From c8934dc33bb9afedb175ba35670f277bec0aa5f9 Mon Sep 17 00:00:00 2001 From: Hyang-Ah Hana Kim Date: Thu, 20 May 2021 13:05:47 -0400 Subject: [PATCH] dap: handle SetVariable requests (#2440) * dap: handle SetVariable requests The handler invokes debugger.SetVariableInScope, except for string type variables. For which, we rely on the `call` command. Moved the call expression handling logic to the new `doCall` function, so it can be reused by the SetVariable requenst handler. With this PR, every successful SetVariable request triggers a StoppedEvent - that's a hack to reset the variablesHandle map internally and notify the client of this change. It will be nice if we can just update cached data corresponding to the updated variable. But I cannot find an easy and safe way to achieve this yet. Also fixed a small bug in the call expression evaluation - Previously, dlv dap returned an error "Unable to evaluate expression: call stopped" if the call expression is for variable assignment. (e.g. "call animal = "rabbit"). * dap: address comments from aarzilli resetHandlesForStop & sendStoppedEvent unconditionally after call command is left as a TODO - This is an existing code path (just refactored) and an preexisting bug. Fixing it here requires updates in TestEvaluateCallRequest and I prefer addressing it in a separate cl. Disabled call injection testing on arm64. Separated TestSetVariable into two, one that doesn't involve call injection and another that may involve call injection. Fixed variableByName by removing unnecessary recursion. * dap: address polina's comments - removed the hard reset for every variable set - added tests for various variable types - added tests that involves interrupted function calls. (breakpoint/panic) And, - changed to utilize EvalVariableInScope to access the variable instead of searching the children by name. - changed to utilize evaluate requests when verifying whether the variable is changed as expected in testing. Since now we avoid resetting the variable handles after variable reset, either we need to trigger scope changes explicitly, or stop depending on the variables request. * dap: address comments - Discuss the problem around the current doCall implementation and the implication. - Refine the description on how VS Code handles after setVariable and evaluate request (there could be followup scopes/evaluate requests). - Use the explicit line numbers for breakpoints in the SetVariable tests. - Do not use errors.Is - we could've used golang.org/x/xerrors polyfill but that's an additional dependency, and we will remove this check once tests that depend on old behavior are fixed. * dap: remove errTerminated and adjust the test * dap: evaluate in the outer frame, instead of advancing to the next bp --- _fixtures/testvariables2.go | 4 +- service/dap/daptest/client.go | 9 +- service/dap/error_ids.go | 1 + service/dap/server.go | 323 +++++++++++++++++++++------- service/dap/server_test.go | 394 +++++++++++++++++++++++++++++++++- 5 files changed, 638 insertions(+), 93 deletions(-) diff --git a/_fixtures/testvariables2.go b/_fixtures/testvariables2.go index f5988e3a..d350262b 100644 --- a/_fixtures/testvariables2.go +++ b/_fixtures/testvariables2.go @@ -331,8 +331,8 @@ func main() { var nilstruct *astruct = nil + val := A{val: 1} // val vs val.val var as2 astruct - s4 := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 0} var iface2map interface{} = map[string]interface{}{ @@ -363,5 +363,5 @@ func main() { longslice := make([]int, 100, 100) runtime.Breakpoint() - fmt.Println(i1, i2, i3, p1, pp1, amb1, s1, s3, a0, a1, p2, p3, s2, as1, str1, f1, fn1, fn2, nilslice, nilptr, ch1, chnil, m1, mnil, m2, m3, m4, m5, upnil, up1, i4, i5, i6, err1, err2, errnil, iface1, iface2, ifacenil, arr1, parr, cpx1, const1, iface3, iface4, recursive1, recursive1.x, iface5, iface2fn1, iface2fn2, bencharr, benchparr, mapinf, mainMenu, b, b2, sd, anonstruct1, anonstruct2, anoniface1, anonfunc, mapanonstruct1, ifacearr, efacearr, ni8, ni16, ni32, ni64, pinf, ninf, nan, zsvmap, zsslice, zsvar, tm, rettm, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice) + fmt.Println(i1, i2, i3, p1, pp1, amb1, s1, s3, a0, a1, p2, p3, s2, as1, str1, f1, fn1, fn2, nilslice, nilptr, ch1, chnil, m1, mnil, m2, m3, m4, m5, upnil, up1, i4, i5, i6, err1, err2, errnil, iface1, iface2, ifacenil, arr1, parr, cpx1, const1, iface3, iface4, recursive1, recursive1.x, iface5, iface2fn1, iface2fn2, bencharr, benchparr, mapinf, mainMenu, b, b2, sd, anonstruct1, anonstruct2, anoniface1, anonfunc, mapanonstruct1, ifacearr, efacearr, ni8, ni16, ni32, ni64, pinf, ninf, nan, zsvmap, zsslice, zsvar, tm, rettm, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice, val) } diff --git a/service/dap/daptest/client.go b/service/dap/daptest/client.go index 180b6161..cb2036b7 100644 --- a/service/dap/daptest/client.go +++ b/service/dap/daptest/client.go @@ -101,6 +101,7 @@ func (c *Client) ExpectInitializeResponseAndCapabilities(t *testing.T) *dap.Init SupportsDelayedStackTraceLoading: true, SupportTerminateDebuggee: true, SupportsExceptionInfoRequest: true, + SupportsSetVariable: true, SupportsFunctionBreakpoints: true, SupportsEvaluateForHovers: true, } @@ -353,8 +354,12 @@ func (c *Client) ReverseContinueRequest() { } // SetVariableRequest sends a 'setVariable' request. -func (c *Client) SetVariableRequest() { - c.send(&dap.ReverseContinueRequest{Request: *c.newRequest("setVariable")}) +func (c *Client) SetVariableRequest(variablesRef int, name, value string) { + request := &dap.SetVariableRequest{Request: *c.newRequest("setVariable")} + request.Arguments.VariablesReference = variablesRef + request.Arguments.Name = name + request.Arguments.Value = value + c.send(request) } // RestartFrameRequest sends a 'restartFrame' request. diff --git a/service/dap/error_ids.go b/service/dap/error_ids.go index 2ae0f330..23d9d3c1 100644 --- a/service/dap/error_ids.go +++ b/service/dap/error_ids.go @@ -23,6 +23,7 @@ const ( UnableToEvaluateExpression = 2009 UnableToHalt = 2010 UnableToGetExceptionInfo = 2011 + UnableToSetVariable = 2012 // Add more codes as we support more requests DebuggeeIsRunning = 4000 DisconnectError = 5000 diff --git a/service/dap/server.go b/service/dap/server.go index 497d579a..e678a480 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -12,8 +12,10 @@ import ( "bufio" "bytes" "encoding/json" + "errors" "fmt" "go/constant" + "go/parser" "io" "net" "os" @@ -685,9 +687,8 @@ func (s *Server) onInitializeRequest(request *dap.InitializeRequest) { response.Body.SupportTerminateDebuggee = true response.Body.SupportsFunctionBreakpoints = true response.Body.SupportsExceptionInfoRequest = true + response.Body.SupportsSetVariable = true response.Body.SupportsEvaluateForHovers = true - // TODO(polina): support this to match vscode-go functionality - response.Body.SupportsSetVariable = false // TODO(polina): support these requests in addition to vscode-go feature parity response.Body.SupportsTerminateRequest = false response.Body.SupportsRestartRequest = false @@ -1448,12 +1449,38 @@ func slicePtrVarToSliceVar(vars []*proc.Variable) []proc.Variable { // onVariablesRequest handles 'variables' requests. // This is a mandatory request to support. func (s *Server) onVariablesRequest(request *dap.VariablesRequest) { - v, ok := s.variableHandles.get(request.Arguments.VariablesReference) + ref := request.Arguments.VariablesReference + v, ok := s.variableHandles.get(ref) if !ok { - s.sendErrorResponse(request.Request, UnableToLookupVariable, "Unable to lookup variable", fmt.Sprintf("unknown reference %d", request.Arguments.VariablesReference)) + s.sendErrorResponse(request.Request, UnableToLookupVariable, "Unable to lookup variable", fmt.Sprintf("unknown reference %d", ref)) return } - children := make([]dap.Variable, 0) + + children, err := s.childrenToDAPVariables(v) + if err != nil { + s.sendErrorResponse(request.Request, UnableToLookupVariable, "Unable to lookup variable", err.Error()) + return + } + if !s.clientCapabilities.supportsVariableType { + // If the client does not support variable type + // we cannot set the Type field in the response. + for i := range children { + children[i].Type = "" + } + } + response := &dap.VariablesResponse{ + Response: *newResponse(request.Request), + Body: dap.VariablesResponseBody{Variables: children}, + } + s.send(response) +} + +// childrenToDAPVariables returns the DAP presentation of the referenced variable's children. +func (s *Server) childrenToDAPVariables(v *fullyQualifiedVariable) ([]dap.Variable, error) { + // TODO(polina): consider convertVariableToString instead of convertVariable + // and avoid unnecessary creation of variable handles when this is called to + // compute evaluate names when this is called from onSetVariableRequest. + var children []dap.Variable switch v.Kind { case reflect.Map: @@ -1573,18 +1600,7 @@ func (s *Server) onVariablesRequest(request *dap.VariablesRequest) { } } } - if !s.clientCapabilities.supportsVariableType { - // If the client does not support variable type - // we cannot set the Type field in the response. - for i := range children { - children[i].Type = "" - } - } - response := &dap.VariablesResponse{ - Response: *newResponse(request.Request), - Body: dap.VariablesResponseBody{Variables: children}, - } - s.send(response) + return children, nil } func (s *Server) getTypeIfSupported(v *proc.Variable) string { @@ -1781,71 +1797,12 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) { response := &dap.EvaluateResponse{Response: *newResponse(request.Request)} isCall, err := regexp.MatchString(`^\s*call\s+\S+`, request.Arguments.Expression) if err == nil && isCall { // call {expression} - // This call might be evaluated in the context of the frame that is not topmost - // if the editor is set to view the variables for one of the parent frames. - // If the call expression refers to any of these variables, unlike regular - // expressions, it will evaluate them in the context of the topmost frame, - // and the user will get an unexpected result or an unexpected symbol error. - // We prevent this but disallowing any frames other than topmost. - if frame > 0 { - s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", "call is only supported with topmost stack frame", showErrorToUser) - return - } - stateBeforeCall, err := s.debugger.State( /*nowait*/ true) + expr := strings.Replace(request.Arguments.Expression, "call ", "", 1) + _, retVars, err := s.doCall(goid, frame, expr) if err != nil { s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", err.Error(), showErrorToUser) return } - // TODO(polina): since call will resume execution of all goroutines, - // we should do this asynchronously and send a continued event to the - // editor, followed by a stop event when the call completes. - state, err := s.debugger.Command(&api.DebuggerCommand{ - Name: api.Call, - ReturnInfoLoadConfig: api.LoadConfigFromProc(&DefaultLoadConfig), - Expr: strings.Replace(request.Arguments.Expression, "call ", "", 1), - UnsafeCall: false, - GoroutineID: goid, - }, nil) - if _, isexited := err.(proc.ErrProcessExited); isexited || err == nil && state.Exited { - e := &dap.TerminatedEvent{Event: *newEvent("terminated")} - s.send(e) - return - } - if err != nil { - s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", err.Error(), showErrorToUser) - return - } - // After the call is done, the goroutine where we injected the call should - // return to the original stopped line with return values. However, - // it is not guaranteed to be selected due to the possibility of the - // of simultaenous breakpoints. Therefore, we check all threads. - var retVars []*proc.Variable - for _, t := range state.Threads { - if t.GoroutineID == stateBeforeCall.SelectedGoroutine.ID && - t.Line == stateBeforeCall.SelectedGoroutine.CurrentLoc.Line && t.CallReturn { - // The call completed. Get the return values. - retVars, err = s.debugger.FindThreadReturnValues(t.ID, DefaultLoadConfig) - if err != nil { - s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", err.Error(), showErrorToUser) - return - } - break - } - } - if retVars == nil { - // The call got interrupted by a stop (e.g. breakpoint in injected - // function call or in another goroutine) - s.resetHandlesForStoppedEvent() - stopped := &dap.StoppedEvent{Event: *newEvent("stopped")} - stopped.Body.AllThreadsStopped = true - stopped.Body.ThreadId = stoppedGoroutineID(state) - stopped.Body.Reason = s.debugger.StopReason().String() - s.send(stopped) - // TODO(polina): once this is asynchronous, we could wait to reply until the user - // continues, call ends, original stop point is hit and return values are available. - s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", "call stopped", showErrorToUser) - return - } // The call completed and we can reply with its return values (if any) if len(retVars) > 0 { // Package one or more return values in a single scope-like nameless variable @@ -1875,6 +1832,102 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) { s.send(response) } +func (s *Server) doCall(goid, frame int, expr string) (*api.DebuggerState, []*proc.Variable, error) { + // This call might be evaluated in the context of the frame that is not topmost + // if the editor is set to view the variables for one of the parent frames. + // If the call expression refers to any of these variables, unlike regular + // expressions, it will evaluate them in the context of the topmost frame, + // and the user will get an unexpected result or an unexpected symbol error. + // We prevent this but disallowing any frames other than topmost. + if frame > 0 { + return nil, nil, fmt.Errorf("call is only supported with topmost stack frame") + } + stateBeforeCall, err := s.debugger.State( /*nowait*/ true) + if err != nil { + return nil, nil, err + } + // TODO(polina): since call will resume execution of all goroutines, + // we should do this asynchronously and send a continued event to the + // editor, followed by a stop event when the call completes. + state, err := s.debugger.Command(&api.DebuggerCommand{ + Name: api.Call, + ReturnInfoLoadConfig: api.LoadConfigFromProc(&DefaultLoadConfig), + Expr: expr, + UnsafeCall: false, + GoroutineID: goid, + }, nil) + if _, isexited := err.(proc.ErrProcessExited); isexited || err == nil && state.Exited { + e := &dap.TerminatedEvent{Event: *newEvent("terminated")} + s.send(e) + return nil, nil, errors.New("terminated") + } + if err != nil { + return nil, nil, err + } + + // After the call is done, the goroutine where we injected the call should + // return to the original stopped line with return values. However, + // it is not guaranteed to be selected due to the possibility of the + // of simultaenous breakpoints. Therefore, we check all threads. + var retVars []*proc.Variable + found := false + for _, t := range state.Threads { + if t.GoroutineID == stateBeforeCall.SelectedGoroutine.ID && + t.Line == stateBeforeCall.SelectedGoroutine.CurrentLoc.Line && t.CallReturn { + found = true + // The call completed. Get the return values. + retVars, err = s.debugger.FindThreadReturnValues(t.ID, DefaultLoadConfig) + if err != nil { + return nil, nil, err + } + break + } + } + // Normal function calls expect return values, but call commands + // used for variable assignments do not return a value when they succeed. + // In go '=' is not an operator. Check if go/parser complains. + // If the above Call command passed but the expression is not a valid + // go expression, we just handled a variable assignment request. + isAssignment := false + if _, err := parser.ParseExpr(expr); err != nil { + isAssignment = true + } + + // note: as described in https://github.com/golang/go/issues/25578, function call injection + // causes to resume the entire Go process. Due to this limitation, there is no guarantee + // that the process is in the same state even after the injected call returns normally + // without any surprises such as breakpoints or panic. To handle this correctly we need + // to reset all the handles (both variables and stack frames). + // + // We considered sending a stopped event after each call unconditionally, but a stopped + // event can be expensive and can interact badly with the client-side optimization + // to refresh information. For example, VS Code reissues scopes/evaluate (for watch) after + // completing a setVariable or evaluate request for repl context. Thus, for now, we + // do not trigger a stopped event and hope editors to refetch the updated state as soon + // as the user resumes debugging. + + if !found || !isAssignment && retVars == nil { + // The call got interrupted by a stop (e.g. breakpoint in injected + // function call or in another goroutine). + s.resetHandlesForStoppedEvent() + s.sendStoppedEvent(state) + + // TODO(polina): once this is asynchronous, we could wait to reply until the user + // continues, call ends, original stop point is hit and return values are available + // instead of returning an error 'call stopped' here. + return nil, nil, errors.New("call stopped") + } + return state, retVars, nil +} + +func (s *Server) sendStoppedEvent(state *api.DebuggerState) { + stopped := &dap.StoppedEvent{Event: *newEvent("stopped")} + stopped.Body.AllThreadsStopped = true + stopped.Body.ThreadId = stoppedGoroutineID(state) + stopped.Body.Reason = s.debugger.StopReason().String() + s.send(stopped) +} + // onTerminateRequest sends a not-yet-implemented error response. // Capability 'supportsTerminateRequest' is not set in 'initialize' response. func (s *Server) onTerminateRequest(request *dap.TerminateRequest) { @@ -1995,10 +2048,116 @@ func (s *Server) onReverseContinueRequest(request *dap.ReverseContinueRequest) { s.sendNotYetImplementedErrorResponse(request.Request) } -// onSetVariableRequest sends a not-yet-implemented error response. -// Capability 'supportsSetVariable' is not set 'initialize' response. -func (s *Server) onSetVariableRequest(request *dap.SetVariableRequest) { // TODO V0 - s.sendNotYetImplementedErrorResponse(request.Request) +// computeEvaluateName finds the named child, and computes its evaluate name. +func (s *Server) computeEvaluateName(v *fullyQualifiedVariable, cname string) (string, error) { + children, err := s.childrenToDAPVariables(v) + if err != nil { + return "", err + } + for _, c := range children { + if c.Name == cname { + if c.EvaluateName != "" { + return c.EvaluateName, nil + } + return "", errors.New("cannot set the variable without evaluate name") + } + } + return "", errors.New("failed to find the named variable") +} + +// onSetVariableRequest handles 'setVariable' requests. +func (s *Server) onSetVariableRequest(request *dap.SetVariableRequest) { + arg := request.Arguments + + v, ok := s.variableHandles.get(arg.VariablesReference) + if !ok { + s.sendErrorResponse(request.Request, UnableToSetVariable, "Unable to lookup variable", fmt.Sprintf("unknown reference %d", arg.VariablesReference)) + return + } + // We need to translate the arg.Name to its evaluateName if the name + // refers to a field or element of a variable. + // https://github.com/microsoft/vscode/issues/120774 + evaluateName, err := s.computeEvaluateName(v, arg.Name) + if err != nil { + s.sendErrorResponse(request.Request, UnableToSetVariable, "Unable to set variable", err.Error()) + return + } + + // By running EvalVariableInScope, we get the type info of the variable + // that can be accessed with the evaluateName, and ensure the variable we are + // trying to update is valid and accessible from the top most frame & the + // current goroutine. + goid, frame := -1, 0 + evaluated, err := s.debugger.EvalVariableInScope(goid, frame, 0, evaluateName, DefaultLoadConfig) + if err != nil { + s.sendErrorResponse(request.Request, UnableToSetVariable, "Unable to lookup variable", err.Error()) + return + } + + useFnCall := false + switch evaluated.Kind { + case reflect.String: + useFnCall = true + default: + // TODO(hyangah): it's possible to set a non-string variable using (`call i = fn()`) + // and we don't support it through the Set Variable request yet. + // If we want to support it for non-string types, we need to parse arg.Value. + } + + if useFnCall { + // TODO(hyangah): function call injection currentlly allows to assign return values of + // a function call to variables. So, curious users would find set variable + // on string would accept expression like `fn()`. + if state, retVals, err := s.doCall(goid, frame, fmt.Sprintf("%v=%v", evaluateName, arg.Value)); err != nil { + s.sendErrorResponse(request.Request, UnableToSetVariable, "Unable to set variable", err.Error()) + return + } else if retVals != nil { + // The assignment expression isn't supposed to return values, but we got them. + // That indicates something went wrong (e.g. panic). + // TODO: isn't it simpler to do this in s.doCall? + s.resetHandlesForStoppedEvent() + s.sendStoppedEvent(state) + + var r []string + for _, v := range retVals { + r = append(r, s.convertVariableToString(v)) + } + msg := "interrupted" + if len(r) > 0 { + msg = "interrupted:" + strings.Join(r, ", ") + } + + s.sendErrorResponse(request.Request, UnableToSetVariable, "Unable to set variable", msg) + return + } + } else { + if err := s.debugger.SetVariableInScope(goid, frame, 0, evaluateName, arg.Value); err != nil { + s.sendErrorResponse(request.Request, UnableToSetVariable, "Unable to set variable", err.Error()) + return + } + } + // * Note on inconsistent state after set variable: + // + // The variable handles may be in inconsistent state - for example, + // let's assume there are two aliased variables pointing to the same + // memory and both are already loaded and cached in the variable handle. + // VSCode tries to locally update the UI when the set variable + // request succeeds, and may issue additional scopes or evaluate requests + // to update the variable/watch sections if necessary. + // + // More complicated situation is when the set variable involves call + // injection - after the injected call is completed, the debugee can + // be in a completely different state (see the note in doCall) due to + // how the call injection is implemented. Ideally, we need to also refresh + // the stack frames but that is complicated. For now we don't try to actively + // invalidate this state hoping that the editors will refetch the state + // as soon as the user resumes debugging. + + response := &dap.SetVariableResponse{Response: *newResponse(request.Request)} + response.Body.Value = arg.Value + // TODO(hyangah): instead of arg.Value, reload the variable and return + // the presentation of the new value. + s.send(response) } // onSetExpression sends a not-yet-implemented error response. diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 8430a823..233b7769 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -2802,8 +2802,22 @@ func TestEvaluateCallRequest(t *testing.T) { if erres.Body.Error.Format != "Unable to evaluate expression: not enough arguments" { t.Errorf("\ngot %#v\nwant Format=\"Unable to evaluate expression: not enough arguments\"", erres) } - // Call can exit + + // Assignment - expect no error, but no return value. + client.EvaluateRequest("call one = two", 1000, "this context will be ignored") + got = client.ExpectEvaluateResponse(t) + expectEval(t, got, "", noChildren) + // Check one=two was applied. + client.EvaluateRequest("one", 1000, "repl") + got = client.ExpectEvaluateResponse(t) + expectEval(t, got, "2", noChildren) + + // Call can exit. client.EvaluateRequest("call callexit()", 1000, "this context will be ignored") + client.ExpectTerminatedEvent(t) + if res := client.ExpectVisibleErrorResponse(t); !strings.Contains(res.Body.Error.Format, "terminated") { + t.Errorf("\ngot %#v\nwant Format=.*terminated.*", res) + } }, terminated: true, disconnect: true, @@ -3297,9 +3311,6 @@ func runDebugSessionWithBPs(t *testing.T, client *daptest.Client, cmd string, cm client.ExpectStoppedEvent(t) onBP.execute() if onBP.disconnect { - if onBP.terminated { - client.ExpectTerminatedEvent(t) - } client.DisconnectRequestWithKillOption(true) if onBP.terminated { client.ExpectOutputEventProcessExited(t, 0) @@ -3621,6 +3632,378 @@ func TestUnupportedCommandResponses(t *testing.T) { }) } +type helperForSetVariable struct { + t *testing.T + c *daptest.Client +} + +func (h *helperForSetVariable) expectSetVariableAndStop(ref int, name, value string) { + h.t.Helper() + h.expectSetVariable0(ref, name, value, true) +} +func (h *helperForSetVariable) expectSetVariable(ref int, name, value string) { + h.t.Helper() + h.expectSetVariable0(ref, name, value, false) +} + +func (h *helperForSetVariable) failSetVariable(ref int, name, value, wantErrInfo string) { + h.t.Helper() + h.failSetVariable0(ref, name, value, wantErrInfo, false) +} + +func (h *helperForSetVariable) failSetVariableAndStop(ref int, name, value, wantErrInfo string) { + h.t.Helper() + h.failSetVariable0(ref, name, value, wantErrInfo, true) +} + +func (h *helperForSetVariable) evaluate(expr, want string, hasRef bool) { + h.t.Helper() + h.c.EvaluateRequest(expr, 1000, "whatever") + got := h.c.ExpectEvaluateResponse(h.t) + expectEval(h.t, got, want, hasRef) +} + +func (h *helperForSetVariable) evaluateRegex(expr, want string, hasRef bool) { + h.t.Helper() + h.c.EvaluateRequest(expr, 1000, "whatever") + got := h.c.ExpectEvaluateResponse(h.t) + expectEvalRegex(h.t, got, want, hasRef) +} + +func (h *helperForSetVariable) expectSetVariable0(ref int, name, value string, wantStop bool) { + h.t.Helper() + + h.c.SetVariableRequest(ref, name, value) + if wantStop { + h.c.ExpectStoppedEvent(h.t) + } + if got, want := h.c.ExpectSetVariableResponse(h.t), value; got.Success != true || got.Body.Value != want { + h.t.Errorf("SetVariableRequest(%v, %v)=%#v, want {Success=true, Body.Value=%q", name, value, got, want) + } +} + +func (h *helperForSetVariable) failSetVariable0(ref int, name, value, wantErrInfo string, wantStop bool) { + h.t.Helper() + + h.c.SetVariableRequest(ref, name, value) + if wantStop { + h.c.ExpectStoppedEvent(h.t) + } + resp := h.c.ExpectErrorResponse(h.t) + if got := resp.Body.Error.Format; !strings.Contains(got, wantErrInfo) { + h.t.Errorf("got %#v, want error string containing %v", got, wantErrInfo) + } +} + +func (h *helperForSetVariable) variables(ref int) *dap.VariablesResponse { + h.t.Helper() + h.c.VariablesRequest(ref) + return h.c.ExpectVariablesResponse(h.t) +} + +// TestSetVariable tests SetVariable features that do not need function call support. +func TestSetVariable(t *testing.T) { + runTest(t, "testvariables", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSessionWithBPs(t, client, "launch", + func() { + client.LaunchRequestWithArgs(map[string]interface{}{ + "mode": "exec", "program": fixture.Path, "showGlobalVariables": true, + }) + }, + fixture.Source, []int{}, // breakpoints are set within the program. + []onBreakpoint{{ + execute: func() { + tester := &helperForSetVariable{t, client} + + startLineno := 66 // after runtime.Breakpoint + if runtime.GOOS == "windows" && goversion.VersionAfterOrEqual(runtime.Version(), 1, 15) { + // Go1.15 on windows inserts a NOP after the call to + // runtime.Breakpoint and marks it same line as the + // runtime.Breakpoint call, making this flaky, so skip the line check. + startLineno = -1 + } + + handleStop(t, client, 1, "main.foobar", startLineno) + + // Args of foobar(baz string, bar FooBar) + args := tester.variables(1000) + + expectVarExact(t, args, 1, "bar", "bar", `main.FooBar {Baz: 10, Bur: "lorem"}`, "main.FooBar", hasChildren) + tester.failSetVariable(1000, "bar", `main.FooBar {Baz: 42, Bur: "ipsum"}`, "*ast.CompositeLit not implemented") + + // Nested field. + barRef := expectVarExact(t, args, 1, "bar", "bar", `main.FooBar {Baz: 10, Bur: "lorem"}`, "main.FooBar", hasChildren) + tester.expectSetVariable(barRef, "Baz", "42") + tester.evaluate("bar", `main.FooBar {Baz: 42, Bur: "lorem"}`, hasChildren) + + tester.failSetVariable(barRef, "Baz", `"string"`, "can not convert") + + // Local variables + locals := tester.variables(1001) + + // int + expectVarExact(t, locals, -1, "a2", "a2", "6", "int", noChildren) + tester.expectSetVariable(1001, "a2", "42") + tester.evaluate("a2", "42", noChildren) + + tester.failSetVariable(1001, "a2", "false", "can not convert") + + // float + expectVarExact(t, locals, -1, "a3", "a3", "7.23", "float64", noChildren) + tester.expectSetVariable(1001, "a3", "-0.1") + tester.evaluate("a3", "-0.1", noChildren) + + // array of int + a4Ref := expectVarExact(t, locals, -1, "a4", "a4", "[2]int [1,2]", "[2]int", hasChildren) + tester.expectSetVariable(a4Ref, "[1]", "-7") + tester.evaluate("a4", "[2]int [1,-7]", hasChildren) + + tester.failSetVariable(1001, "a4", "[2]int{3, 4}", "not implemented") + + // slice of int + a5Ref := expectVarExact(t, locals, -1, "a5", "a5", "[]int len: 5, cap: 5, [1,2,3,4,5]", "[]int", hasChildren) + tester.expectSetVariable(a5Ref, "[3]", "100") + tester.evaluate("a5", "[]int len: 5, cap: 5, [1,2,3,100,5]", hasChildren) + + // composite literal and its nested fields. + a7Ref := expectVarExact(t, locals, -1, "a7", "a7", `*main.FooBar {Baz: 5, Bur: "strum"}`, "*main.FooBar", hasChildren) + a7Val := tester.variables(a7Ref) + a7ValRef := expectVarExact(t, a7Val, -1, "", "(*a7)", `main.FooBar {Baz: 5, Bur: "strum"}`, "main.FooBar", hasChildren) + tester.expectSetVariable(a7ValRef, "Baz", "7") + tester.evaluate("(*a7)", `main.FooBar {Baz: 7, Bur: "strum"}`, hasChildren) + + // pointer + expectVarExact(t, locals, -1, "a9", "a9", `*main.FooBar nil`, "*main.FooBar", noChildren) + tester.expectSetVariable(1001, "a9", "&a6") + tester.evaluate("a9", `*main.FooBar {Baz: 8, Bur: "word"}`, hasChildren) + + // slice of pointers + a13Ref := expectVarExact(t, locals, -1, "a13", "a13", `[]*main.FooBar len: 3, cap: 3, [*{Baz: 6, Bur: "f"},*{Baz: 7, Bur: "g"},*{Baz: 8, Bur: "h"}]`, "[]*main.FooBar", hasChildren) + a13 := tester.variables(a13Ref) + a13c0Ref := expectVarExact(t, a13, -1, "[0]", "a13[0]", `*main.FooBar {Baz: 6, Bur: "f"}`, "*main.FooBar", hasChildren) + a13c0 := tester.variables(a13c0Ref) + a13c0valRef := expectVarExact(t, a13c0, -1, "", "(*a13[0])", `main.FooBar {Baz: 6, Bur: "f"}`, "main.FooBar", hasChildren) + tester.expectSetVariable(a13c0valRef, "Baz", "777") + tester.evaluate("a13[0]", `*main.FooBar {Baz: 777, Bur: "f"}`, hasChildren) + + // complex + tester.evaluate("c64", `(1 + 2i)`, hasChildren) + tester.expectSetVariable(1001, "c64", "(2 + 3i)") + tester.evaluate("c64", `(2 + 3i)`, hasChildren) + // note: complex's real, imaginary part can't be directly mutable. + + // + // Global variables + // p1 = 10 + client.VariablesRequest(1002) + globals := client.ExpectVariablesResponse(t) + + expectVarExact(t, globals, -1, "p1", "main.p1", "10", "int", noChildren) + tester.expectSetVariable(1002, "p1", "-10") + tester.evaluate("p1", "-10", noChildren) + tester.failSetVariable(1002, "p1", "0.1", "can not convert") + }, + disconnect: true, + }}) + }) + + runTest(t, "testvariables2", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSessionWithBPs(t, client, "launch", + func() { + client.LaunchRequestWithArgs(map[string]interface{}{ + "mode": "exec", "program": fixture.Path, "showGlobalVariables": true, + }) + }, + fixture.Source, []int{}, // breakpoints are set within the program. + []onBreakpoint{{ + execute: func() { + tester := &helperForSetVariable{t, client} + + startLineno := 358 // after runtime.Breakpoint + if runtime.GOOS == "windows" && goversion.VersionAfterOrEqual(runtime.Version(), 1, 15) { + startLineno = -1 + } + + handleStop(t, client, 1, "main.main", startLineno) + locals := tester.variables(1001) + + // channel + tester.evaluate("chnil", "chan int nil", noChildren) + tester.expectSetVariable(1001, "chnil", "ch1") + tester.evaluate("chnil", "chan int 4/11", hasChildren) + + // func + tester.evaluate("fn2", "nil", noChildren) + tester.expectSetVariable(1001, "fn2", "fn1") + tester.evaluate("fn2", "main.afunc", noChildren) + + // interface + tester.evaluate("ifacenil", "interface {} nil", noChildren) + tester.expectSetVariable(1001, "ifacenil", "iface1") + tester.evaluate("ifacenil", "interface {}(*main.astruct) *{A: 1, B: 2}", hasChildren) + + // interface.(data) + iface1Ref := expectVarExact(t, locals, -1, "iface1", "iface1", "interface {}(*main.astruct) *{A: 1, B: 2}", "interface {}", hasChildren) + iface1 := tester.variables(iface1Ref) + iface1DataRef := expectVarExact(t, iface1, -1, "data", "iface1.(data)", "*main.astruct {A: 1, B: 2}", "*main.astruct", hasChildren) + iface1Data := tester.variables(iface1DataRef) + iface1DataValueRef := expectVarExact(t, iface1Data, -1, "", "(*iface1.(data))", "main.astruct {A: 1, B: 2}", "main.astruct", hasChildren) + tester.expectSetVariable(iface1DataValueRef, "A", "2021") + tester.evaluate("iface1", "interface {}(*main.astruct) *{A: 2021, B: 2}", hasChildren) + + // map: string -> struct + tester.evaluate(`m1["Malone"]`, "main.astruct {A: 2, B: 3}", hasChildren) + m1Ref := expectVarRegex(t, locals, -1, "m1", "m1", `.*map\[string\]main\.astruct.*`, `map\[string\]main\.astruct`, hasChildren) + m1 := tester.variables(m1Ref) + elem1 := m1.Body.Variables[0] + tester.expectSetVariable(elem1.VariablesReference, "A", "-9999") + tester.expectSetVariable(elem1.VariablesReference, "B", "10000") + tester.evaluate(elem1.EvaluateName, "main.astruct {A: -9999, B: 10000}", hasChildren) + + // map: struct -> int + m3Ref := expectVarExact(t, locals, -1, "m3", "m3", "map[main.astruct]int [{A: 1, B: 1}: 42, {A: 2, B: 2}: 43, ]", "map[main.astruct]int", hasChildren) + tester.expectSetVariable(m3Ref, "main.astruct {A: 1, B: 1}", "8888") + // note: updating keys is possible, but let's not promise anything. + tester.evaluateRegex("m3", `.*\[\{A: 1, B: 1\}: 8888,.*`, hasChildren) + + // map: struct -> struct + m4Ref := expectVarRegex(t, locals, -1, "m4", "m4", `map\[main\.astruct]main\.astruct.*\[\{A: 1, B: 1\}: \{A: 11, B: 11\}.*`, `map\[main\.astruct\]main\.astruct`, hasChildren) + m4 := tester.variables(m4Ref) + m4Val1Ref := expectVarRegex(t, m4, -1, "[val 0]", `.*0x[0-9a-f]+.*`, `main.astruct.*`, `main\.astruct`, hasChildren) + tester.expectSetVariable(m4Val1Ref, "A", "-9999") + tester.evaluateRegex("m4", `.*A: -9999,.*`, hasChildren) + + // unsigned pointer + expectVarRegex(t, locals, -1, "up1", "up1", `unsafe\.Pointer\(0x[0-9a-f]+\)`, "unsafe.Pointer", noChildren) + tester.expectSetVariable(1001, "up1", "unsafe.Pointer(0x0)") + tester.evaluate("up1", "unsafe.Pointer(0x0)", noChildren) + + // val := A{val: 1} + valRef := expectVarExact(t, locals, -1, "val", "val", `main.A {val: 1}`, "main.A", hasChildren) + tester.expectSetVariable(valRef, "val", "3") + tester.evaluate("val", `main.A {val: 3}`, hasChildren) + }, + disconnect: true, + }}) + }) +} + +// TestSetVariableWithCall tests SetVariable features that do not depend on function calls support. +func TestSetVariableWithCall(t *testing.T) { + protest.MustSupportFunctionCalls(t, testBackend) + + runTest(t, "testvariables", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSessionWithBPs(t, client, "launch", + func() { + client.LaunchRequestWithArgs(map[string]interface{}{ + "mode": "exec", "program": fixture.Path, "showGlobalVariables": true, + }) + }, + fixture.Source, []int{66, 67}, + []onBreakpoint{{ + execute: func() { + tester := &helperForSetVariable{t, client} + + startLineno := 66 + if runtime.GOOS == "windows" && goversion.VersionAfterOrEqual(runtime.Version(), 1, 15) { + // Go1.15 on windows inserts a NOP after the call to + // runtime.Breakpoint and marks it same line as the + // runtime.Breakpoint call, making this flaky, so skip the line check. + startLineno = -1 + } + + handleStop(t, client, 1, "main.foobar", startLineno) + + // Args of foobar(baz string, bar FooBar) + args := tester.variables(1000) + + expectVarExact(t, args, 0, "baz", "baz", `"bazburzum"`, "string", noChildren) + tester.expectSetVariable(1000, "baz", `"BazBurZum"`) + tester.evaluate("baz", `"BazBurZum"`, noChildren) + + args = tester.variables(1000) + barRef := expectVarExact(t, args, 1, "bar", "bar", `main.FooBar {Baz: 10, Bur: "lorem"}`, "main.FooBar", hasChildren) + tester.expectSetVariable(barRef, "Bur", `"ipsum"`) + tester.evaluate("bar", `main.FooBar {Baz: 10, Bur: "ipsum"}`, hasChildren) + + // Local variables + locals := tester.variables(1001) + + expectVarExact(t, locals, -1, "a1", "a1", `"foofoofoofoofoofoo"`, "string", noChildren) + tester.expectSetVariable(1001, "a1", `"barbarbar"`) + tester.evaluate("a1", `"barbarbar"`, noChildren) + + a6Ref := expectVarExact(t, locals, -1, "a6", "a6", `main.FooBar {Baz: 8, Bur: "word"}`, "main.FooBar", hasChildren) + tester.failSetVariable(a6Ref, "Bur", "false", "can not convert") + + tester.expectSetVariable(a6Ref, "Bur", `"sentence"`) + tester.evaluate("a6", `main.FooBar {Baz: 8, Bur: "sentence"}`, hasChildren) + }, + }, { + // Stop at second breakpoint and set a1. + execute: func() { + tester := &helperForSetVariable{t, client} + + handleStop(t, client, 1, "main.barfoo", -1) + // Test: set string 'a1' in main.barfoo. + // This shouldn't affect 'a1' in main.foobar - we will check that in the next breakpoint. + locals := tester.variables(1001) + expectVarExact(t, locals, -1, "a1", "a1", `"bur"`, "string", noChildren) + tester.expectSetVariable(1001, "a1", `"fur"`) + tester.evaluate("a1", `"fur"`, noChildren) + // We will check a1 in main.foobar isn't affected from the next breakpoint. + + client.StackTraceRequest(1, 1, 20) + res := client.ExpectStackTraceResponse(t) + if len(res.Body.StackFrames) < 1 { + t.Fatalf("stack trace response = %#v, wanted at least one stack frame", res) + } + outerFrame := res.Body.StackFrames[0].Id + client.EvaluateRequest("a1", outerFrame, "whatever_context") + evalRes := client.ExpectEvaluateResponse(t) + expectEval(t, evalRes, `"barbarbar"`, noChildren) + }, + disconnect: true, + }}) + }) + + runTest(t, "fncall", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSessionWithBPs(t, client, "launch", + func() { + client.LaunchRequestWithArgs(map[string]interface{}{ + "mode": "exec", "program": fixture.Path, "showGlobalVariables": true, + }) + }, + fixture.Source, []int{}, // breakpoints are set within the program. + []onBreakpoint{{ + // Stop at second breakpoint and set a1. + execute: func() { + tester := &helperForSetVariable{t, client} + + handleStop(t, client, 1, "main.main", 197) + + _ = tester.variables(1001) + + // successful variable set using a function call. + tester.expectSetVariable(1001, "str", `callstacktrace()`) + tester.evaluateRegex("str", `.*in main.callstacktrace at.*`, noChildren) + + tester.failSetVariableAndStop(1001, "str", `callpanic()`, `callpanic panicked`) + handleStop(t, client, 1, "main.main", 197) + + // breakpoint during a function call. + tester.failSetVariableAndStop(1001, "str", `callbreak()`, "call stopped") + + // TODO(hyangah): continue after this causes runtime error while resuming + // unfinished injected call. + // runtime error: can not convert %!s() constant to string + // This can be reproducible with dlv cli. (`call str = callbreak(); continue`) + }, + disconnect: true, + }}) + }) +} + func TestOptionalNotYetImplementedResponses(t *testing.T) { var got *dap.ErrorResponse runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { @@ -3646,9 +4029,6 @@ func TestOptionalNotYetImplementedResponses(t *testing.T) { client.ReverseContinueRequest() expectNotYetImplemented("reverseContinue") - client.SetVariableRequest() - expectNotYetImplemented("setVariable") - client.SetExpressionRequest() expectNotYetImplemented("setExpression")