diff --git a/_fixtures/testvariables2.go b/_fixtures/testvariables2.go index 4fcee1cf..a3a6deb7 100644 --- a/_fixtures/testvariables2.go +++ b/_fixtures/testvariables2.go @@ -357,6 +357,9 @@ func main() { fmt.Println(amb1) } + longarr := [100]int{} + 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, 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, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5) + 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, 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, errtypednil, emptyslice, emptymap, byteslice, runeslice, bytearray, runearray, longstr, nilstruct, as2, as2.NonPointerRecieverMethod, s4, iface2map, issue1578, ll, unread, w2, w3, w4, w5, longarr, longslice) } diff --git a/service/dap/server.go b/service/dap/server.go index 7069a87f..1fac93d9 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -87,6 +87,17 @@ var defaultArgs = launchAttachArgs{ showGlobalVariables: false, } +// DefaultLoadConfig controls how variables are loaded from the target's memory, borrowing the +// default value from the original vscode-go debug adapter and rpc server. +// TODO(polina): Support setting config via launch/attach args or only rely on on-demand loading? +var DefaultLoadConfig = proc.LoadConfig{ + FollowPointers: true, + MaxVariableRecurse: 1, + MaxStringLen: 64, + MaxArrayValues: 64, + MaxStructFields: -1, +} + // NewServer creates a new DAP Server. It takes an opened Listener // via config and assumes its ownership. config.disconnectChan has to be set; // it will be closed by the server when the client disconnects or requests @@ -825,11 +836,9 @@ func (s *Server) onScopesRequest(request *dap.ScopesRequest) { goid := sf.(stackFrame).goroutineID frame := sf.(stackFrame).frameIndex - // TODO(polina): Support setting config via launch/attach args - cfg := proc.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1} // Retrieve arguments - args, err := s.debugger.FunctionArguments(goid, frame, 0, cfg) + args, err := s.debugger.FunctionArguments(goid, frame, 0, DefaultLoadConfig) if err != nil { s.sendErrorResponse(request.Request, UnableToListArgs, "Unable to list args", err.Error()) return @@ -837,7 +846,7 @@ func (s *Server) onScopesRequest(request *dap.ScopesRequest) { argScope := &fullyQualifiedVariable{&proc.Variable{Name: "Arguments", Children: slicePtrVarToSliceVar(args)}, "", true} // Retrieve local variables - locals, err := s.debugger.LocalVariables(goid, frame, 0, cfg) + locals, err := s.debugger.LocalVariables(goid, frame, 0, DefaultLoadConfig) if err != nil { s.sendErrorResponse(request.Request, UnableToListLocals, "Unable to list locals", err.Error()) return @@ -866,7 +875,7 @@ func (s *Server) onScopesRequest(request *dap.ScopesRequest) { return } currPkgFilter := fmt.Sprintf("^%s\\.", currPkg) - globals, err := s.debugger.PackageVariables(currPkgFilter, cfg) + globals, err := s.debugger.PackageVariables(currPkgFilter, DefaultLoadConfig) if err != nil { s.sendErrorResponse(request.Request, UnableToListGlobals, "Unable to list globals", err.Error()) return @@ -908,8 +917,6 @@ func (s *Server) onVariablesRequest(request *dap.VariablesRequest) { return } children := make([]dap.Variable, 0) - // TODO(polina): check and handle if variable loaded incompletely - // https://github.com/go-delve/delve/blob/master/Documentation/api/ClientHowto.md#looking-into-variables switch v.Kind { case reflect.Map: @@ -1045,6 +1052,11 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s return } typeName := api.PrettyTypeName(v.DwarfType) + + // As per https://github.com/go-delve/delve/blob/master/Documentation/api/ClientHowto.md#looking-into-variables, + // some of the types might be fully or partially not loaded based on LoadConfig. For now, clearly + // communicate when values are not fully loaded. TODO(polina): look into loading on demand. + switch v.Kind { case reflect.UnsafePointer: if len(v.Children) == 0 { @@ -1061,10 +1073,20 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s value = "void" } else { value = fmt.Sprintf("<%s>(%#x)", typeName, v.Children[0].Addr) - variablesReference = maybeCreateVariableHandle(v) + if v.Children[0].OnlyAddr { // Not fully loaded + value += " (value not loaded)" + // Since child is not fully loaded, don't provide a reference to view it. + } else { + value = fmt.Sprintf("<%s>(%#x)", typeName, v.Children[0].Addr) + variablesReference = maybeCreateVariableHandle(v) + } } case reflect.Array: - value = "<" + typeName + ">" + if v.Len > int64(len(v.Children)) { // Not fully loaded + value = fmt.Sprintf("<%s> (length: %d, loaded: %d)", typeName, v.Len, len(v.Children)) + } else { + value = fmt.Sprintf("<%s>", typeName) + } if len(v.Children) > 0 { variablesReference = maybeCreateVariableHandle(v) } @@ -1072,7 +1094,11 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s if v.Base == 0 { value = "nil <" + typeName + ">" } else { - value = fmt.Sprintf("<%s> (length: %d, cap: %d)", typeName, v.Len, v.Cap) + if v.Len > int64(len(v.Children)) { // Not fully loaded + value = fmt.Sprintf("<%s> (length: %d, cap: %d, loaded: %d)", typeName, v.Len, v.Cap, len(v.Children)) + } else { + value = fmt.Sprintf("<%s> (length: %d, cap: %d)", typeName, v.Len, v.Cap) + } if len(v.Children) > 0 { variablesReference = maybeCreateVariableHandle(v) } @@ -1081,7 +1107,11 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s if v.Base == 0 { value = "nil <" + typeName + ">" } else { - value = fmt.Sprintf("<%s> (length: %d)", typeName, v.Len) + if v.Len > int64(len(v.Children)/2) { // Not fully loaded + value = fmt.Sprintf("<%s> (length: %d, loaded: %d)", typeName, v.Len, len(v.Children)/2) + } else { + value = fmt.Sprintf("<%s> (length: %d)", typeName, v.Len) + } if len(v.Children) > 0 { variablesReference = maybeCreateVariableHandle(v) } @@ -1089,7 +1119,7 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s case reflect.String: vvalue := constant.StringVal(v.Value) lenNotLoaded := v.Len - int64(len(vvalue)) - if lenNotLoaded > 0 { + if lenNotLoaded > 0 { // Not fully loaded vvalue += fmt.Sprintf("...+%d more", lenNotLoaded) } value = fmt.Sprintf("%q", vvalue) @@ -1111,21 +1141,35 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s } else if len(v.Children) == 0 || v.Children[0].Kind == reflect.Invalid && v.Children[0].Addr == 0 { value = "nil <" + typeName + ">" } else { - value = "<" + typeName + "(" + v.Children[0].TypeString() + ")" + ">" - // TODO(polina): should we remove one level of indirection and skip "data"? - // Then we will have: - // Before: - // i: - // data: 123 - // After: - // i: 123 - // Before: - // i: - // data: - // field1: ... - // After: - // i: - // field1: ... + if v.Children[0].OnlyAddr { // Not fully loaded + value = "<" + typeName + "(" + v.Children[0].TypeString() + ")" + ">" + " (data not loaded)" + // Since child is not fully loaded, don't provide a reference to view it. + } else { + value = "<" + typeName + "(" + v.Children[0].TypeString() + ")" + ">" + // TODO(polina): should we remove one level of indirection and skip "data"? + // Then we will have: + // Before: + // i: + // data: 123 + // After: + // i: 123 + // Before: + // i: + // data: + // field1: ... + // After: + // i: + // field1: ... + variablesReference = maybeCreateVariableHandle(v) + } + } + case reflect.Struct: + if v.Len > int64(len(v.Children)) { // Not fully loaded + value = fmt.Sprintf("<%s> (fields: %d, loaded: %d)", typeName, v.Len, len(v.Children)) + } else { + value = fmt.Sprintf("<%s>", typeName) + } + if len(v.Children) > 0 { variablesReference = maybeCreateVariableHandle(v) } case reflect.Complex64, reflect.Complex128: @@ -1142,7 +1186,7 @@ func (s *Server) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr s v.Children[1].Kind = reflect.Float64 } fallthrough - default: // Struct, complex, scalar + default: // complex, scalar vvalue := api.VariableValueAsString(v) if vvalue != "" { value = vvalue @@ -1177,9 +1221,6 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) { goid = sf.(stackFrame).goroutineID frame = sf.(stackFrame).frameIndex } - // TODO(polina): Support config settings via launch/attach args vs auto-loading? - apiCfg := &api.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1} - prcCfg := proc.LoadConfig{FollowPointers: true, MaxVariableRecurse: 1, MaxStringLen: 64, MaxArrayValues: 64, MaxStructFields: -1} response := &dap.EvaluateResponse{Response: *newResponse(request.Request)} isCall, err := regexp.MatchString(`^\s*call\s+\S+`, request.Arguments.Expression) @@ -1201,7 +1242,7 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) { } state, err := s.debugger.Command(&api.DebuggerCommand{ Name: api.Call, - ReturnInfoLoadConfig: apiCfg, + ReturnInfoLoadConfig: api.LoadConfigFromProc(&DefaultLoadConfig), Expr: strings.Replace(request.Arguments.Expression, "call ", "", 1), UnsafeCall: false, GoroutineID: goid, @@ -1224,7 +1265,7 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) { 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, prcCfg) + retVars, err = s.debugger.FindThreadReturnValues(t.ID, DefaultLoadConfig) if err != nil { s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", err.Error(), showErrorToUser) return @@ -1264,7 +1305,7 @@ func (s *Server) onEvaluateRequest(request *dap.EvaluateRequest) { } } } else { // {expression} - exprVar, err := s.debugger.EvalVariableInScope(goid, frame, 0, request.Arguments.Expression, prcCfg) + exprVar, err := s.debugger.EvalVariableInScope(goid, frame, 0, request.Arguments.Expression, DefaultLoadConfig) if err != nil { s.sendErrorResponseWithOpts(request.Request, UnableToEvaluateExpression, "Unable to evaluate expression", err.Error(), showErrorToUser) return diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 92c8b05a..64452b8e 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -1022,7 +1022,6 @@ func TestScopesAndVariablesRequests2(t *testing.T) { // Breakpoints are set within the program fixture.Source, []int{}, []onBreakpoint{{ - // Stop at line 317 execute: func() { client.StackTraceRequest(1, 0, 20) stack := client.ExpectStackTraceResponse(t) @@ -1035,7 +1034,6 @@ func TestScopesAndVariablesRequests2(t *testing.T) { }, disconnect: false, }, { - // Stop at line 322 execute: func() { client.StackTraceRequest(1, 0, 20) stack := client.ExpectStackTraceResponse(t) @@ -1275,15 +1273,77 @@ func TestScopesAndVariablesRequests2(t *testing.T) { expectVarRegex(t, val, 0, "^$", `\(\*unread\)`, "unreadable <.+>", noChildren) validateEvaluateName(t, client, val, 0) } + }, + disconnect: true, + }}) + }) +} - // Test that variables are not yet loaded completely. - ref = expectVarExact(t, locals, -1, "m1", "m1", " (length: 66)", hasChildren) +// TestVariablesLoading exposes test cases where variables might be partiall or +// fully unloaded. +func TestVariablesLoading(t *testing.T) { + runTest(t, "testvariables2", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSessionWithBPs(t, client, "launch", + // Launch + func() { + client.LaunchRequest("exec", fixture.Path, !stopOnEntry) + }, + // Breakpoints are set within the program + fixture.Source, []int{}, + []onBreakpoint{{ + execute: func() {}, + disconnect: false, + }, { + execute: func() { + // Change default config values to trigger certain unloaded corner cases + DefaultLoadConfig.MaxStructFields = 5 + defer func() { DefaultLoadConfig.MaxStructFields = -1 }() + + client.StackTraceRequest(1, 0, 0) + client.ExpectStackTraceResponse(t) + + client.ScopesRequest(1000) + client.ExpectScopesResponse(t) + + client.VariablesRequest(1001) // Locals + locals := client.ExpectVariablesResponse(t) + + // String partially missing based on LoadConfig.MaxStringLen + expectVarExact(t, locals, -1, "longstr", "longstr", "\"very long string 0123456789a0123456789b0123456789c0123456789d012...+73 more\"", noChildren) + + // Array partially missing based on LoadConfig.MaxArrayValues + ref := expectVarExact(t, locals, -1, "longarr", "longarr", "<[100]int> (length: 100, loaded: 64)", hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + longarr := client.ExpectVariablesResponse(t) + expectChildren(t, longarr, "longarr", 64) + } + + // Slice partially missing based on LoadConfig.MaxArrayValues + ref = expectVarExact(t, locals, -1, "longslice", "longslice", "<[]int> (length: 100, cap: 100, loaded: 64)", hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + longarr := client.ExpectVariablesResponse(t) + expectChildren(t, longarr, "longslice", 64) + } + + // Map partially missing based on LoadConfig.MaxArrayValues + ref = expectVarExact(t, locals, -1, "m1", "m1", " (length: 66, loaded: 64)", hasChildren) if ref > 0 { client.VariablesRequest(ref) m1 := client.ExpectVariablesResponse(t) - expectChildren(t, m1, "m1", 64) // TODO(polina): should be 66. + expectChildren(t, m1, "m1", 64) } + // Struct partially missing based on LoadConfig.MaxStructFields + ref = expectVarExact(t, locals, -1, "sd", "sd", " (fields: 6, loaded: 5)", hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + sd := client.ExpectVariablesResponse(t) + expectChildren(t, sd, "sd", 5) + } + + // Struct fully missing due to hitting LoadConfig.MaxVariableRecurse (also tests evaluateName) ref = expectVarExact(t, locals, -1, "c1", "c1", "", hasChildren) if ref > 0 { client.VariablesRequest(ref) @@ -1299,12 +1359,13 @@ func TestScopesAndVariablesRequests2(t *testing.T) { client.VariablesRequest(ref) c1sa0 := client.ExpectVariablesResponse(t) expectChildren(t, c1sa0, "c1.sa[0]", 1) - // TODO(polina): there should be a child here once we support auto loading - expectVarExact(t, c1sa0, 0, "", "(*c1.sa[0])", "", noChildren) + // TODO(polina): there should be children here once we support auto loading + expectVarExact(t, c1sa0, 0, "", "(*c1.sa[0])", " (fields: 2, loaded: 0)", noChildren) } } } + // Struct fully missing due to hitting LoadConfig.MaxVariableRecurse (also tests evaluteName) ref = expectVarExact(t, locals, -1, "aas", "aas", "<[]main.a> (length: 1, cap: 1)", hasChildren) if ref > 0 { client.VariablesRequest(ref) @@ -1321,11 +1382,12 @@ func TestScopesAndVariablesRequests2(t *testing.T) { aas0aas := client.ExpectVariablesResponse(t) expectChildren(t, aas0aas, "aas[0].aas", 1) // TODO(polina): there should be a child here once we support auto loading - test for "aas[0].aas[0].aas" - expectVarExact(t, aas0aas, 0, "[0]", "aas[0].aas[0]", "", noChildren) + expectVarExact(t, aas0aas, 0, "[0]", "aas[0].aas[0]", " (fields: 1, loaded: 0)", noChildren) } } } + // Map fully missing due to hitting LoadConfig.MaxVariableRecurse (also tests evaluateName) ref = expectVarExact(t, locals, -1, "tm", "tm", "", hasChildren) if ref > 0 { client.VariablesRequest(ref) @@ -1337,9 +1399,62 @@ func TestScopesAndVariablesRequests2(t *testing.T) { tm_v := client.ExpectVariablesResponse(t) expectChildren(t, tm_v, "tm.v", 1) // TODO(polina): there should be children here once we support auto loading - test for "tm.v[0]["gutters"]" - expectVarExact(t, tm_v, 0, "[0]", "tm.v[0]", " (length: 66)", noChildren) + expectVarExact(t, tm_v, 0, "[0]", "tm.v[0]", " (length: 66, loaded: 0)", noChildren) } } + + // Interface fully missing due to hitting LoadConfig.MaxVariableRecurse + ref = expectVarRegex(t, locals, -1, "ptrinf", "ptrinf", `<\*interface {}>\(0x[0-9a-f]+\)`, hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + ptrinf := client.ExpectVariablesResponse(t) + ref = expectVarExact(t, ptrinf, 0, "", "(*ptrinf)", "", hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + ptrinfVal := client.ExpectVariablesResponse(t) + ref = expectVarRegex(t, ptrinfVal, 0, "", `\(\*ptrinf\)\.\(data\)`, `<\*\*interface {}>\(0x[0-9a-f]+\)`, hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + ptrinfValData := client.ExpectVariablesResponse(t) + ref = expectVarRegex(t, ptrinfValData, 0, "", `\(\*\(\*ptrinf\)\.\(data\)\)`, `<\*interface {}>\(0x[0-9a-f]+\)`, hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + iface6dataValVal := client.ExpectVariablesResponse(t) + expectVarExact(t, iface6dataValVal, 0, "", "(*(*(*ptrinf).(data)))", " (data not loaded)", noChildren) + } + } + } + } + + // Pointer fully missing - see below + }, + disconnect: true, + }}) + }) + runTest(t, "testvariables2", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSessionWithBPs(t, client, "launch", + // Launch + func() { + client.LaunchRequest("exec", fixture.Path, !stopOnEntry) + }, + // Breakpoints are set within the program + fixture.Source, []int{}, + []onBreakpoint{{ + execute: func() { + DefaultLoadConfig.FollowPointers = false + defer func() { DefaultLoadConfig.FollowPointers = true }() + + client.StackTraceRequest(1, 0, 0) + client.ExpectStackTraceResponse(t) + + client.ScopesRequest(1000) + client.ExpectScopesResponse(t) + + client.VariablesRequest(1001) // Locals + locals := client.ExpectVariablesResponse(t) + + // Pointer value not loaded + expectVarRegex(t, locals, -1, "ptrinf", "ptrinf", `<\*interface {}>\(0x[0-9a-f]+\) \(value not loaded\)`, noChildren) }, disconnect: true, }}) @@ -1580,6 +1695,11 @@ func TestEvaluateRequest(t *testing.T) { validateEvaluateName(t, client, a5, 4) } + // Variable lookup that's not fully loaded + client.EvaluateRequest("ba", 1000, "this context will be ignored") + got = client.ExpectEvaluateResponse(t) + expectEval(t, got, "<[]int> (length: 200, cap: 200, loaded: 64)", hasChildren) + // All (binary and unary) on basic types except <-, ++ and -- client.EvaluateRequest("1+1", 1000, "this context will be ignored") got = client.ExpectEvaluateResponse(t) @@ -1598,14 +1718,8 @@ func TestEvaluateRequest(t *testing.T) { // Type casts of integer constants into any pointer type and vice versa client.EvaluateRequest("(*int)(2)", 1000, "this context will be ignored") got = client.ExpectEvaluateResponse(t) - ref = expectEval(t, got, "<*int>(0x2)", hasChildren) - if ref > 0 { - client.VariablesRequest(ref) - expr := client.ExpectVariablesResponse(t) - expectChildren(t, expr, "(*int)(2)", 1) - // TODO(polina): should this be printed as (unknown int) instead? - expectVarExact(t, expr, 0, "", "(*((*int)(2)))", "", noChildren) - } + expectEval(t, got, "<*int>(0x2) (value not loaded)", noChildren) + // Type casts between string, []byte and []rune client.EvaluateRequest("[]byte(\"ABC€\")", 1000, "this context will be ignored") got = client.ExpectEvaluateResponse(t)