From 90fb0a535fbe3035095f13d580dd2358a85d0cb0 Mon Sep 17 00:00:00 2001 From: polinasok <51177946+polinasok@users.noreply.github.com> Date: Mon, 8 Mar 2021 09:41:47 -0800 Subject: [PATCH] service/dap: support auto-loading of unloaded interfaces (#2362) * service/dap: support auto-loading of unloaded interfaces * Make DeepSource happy * Don't set reference if data failed to auto-load * Use frame-less expressions * Refine interface recursion capping test case Co-authored-by: Polina Sokolova --- _fixtures/testvariables.go | 5 +- pkg/terminal/command_test.go | 6 +-- service/dap/server.go | 18 +++++-- service/dap/server_test.go | 91 ++++++++++++++++++++-------------- service/test/variables_test.go | 1 + 5 files changed, 74 insertions(+), 47 deletions(-) diff --git a/_fixtures/testvariables.go b/_fixtures/testvariables.go index d957d29e..41bbf429 100644 --- a/_fixtures/testvariables.go +++ b/_fixtures/testvariables.go @@ -56,14 +56,15 @@ func foobar(baz string, bar FooBar) { c128 = complex(float64(2), float64(3)) i32 = [2]int32{1, 2} f = barfoo - ms = Nest{0, &Nest{1, &Nest{2, &Nest{3, &Nest{4, nil}}}}} // Test recursion capping + ms = Nest{0, &Nest{1, &Nest{2, &Nest{3, &Nest{4, nil}}}}} // Test recursion capping on structs + ni = []interface{}([]interface{}{[]interface{}{123}}) // Test recursion capping on interfaces ba = make([]int, 200, 200) // Test array size capping mp = map[int]interface{}{1: 42, 2: 43} ) runtime.Breakpoint() barfoo() - fmt.Println(a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, b1, b2, baz, neg, i8, u8, u16, u32, u64, up, f32, c64, c128, i32, bar, f, ms, ba, p1, mp) + fmt.Println(a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, b1, b2, baz, neg, i8, u8, u16, u32, u64, up, f32, c64, c128, i32, bar, f, ms, ni, ba, p1, mp) } var p1 = 10 diff --git a/pkg/terminal/command_test.go b/pkg/terminal/command_test.go index 5669c98d..f4c2a514 100644 --- a/pkg/terminal/command_test.go +++ b/pkg/terminal/command_test.go @@ -690,9 +690,9 @@ func TestListCmd(t *testing.T) { term.MustExec("continue") term.MustExec("continue") listIsAt(t, term, "list", 27, 22, 32) - listIsAt(t, term, "list 69", 69, 64, 73) - listIsAt(t, term, "frame 1 list", 65, 60, 70) - listIsAt(t, term, "frame 1 list 69", 69, 64, 73) + listIsAt(t, term, "list 69", 69, 64, 74) + listIsAt(t, term, "frame 1 list", 66, 61, 71) + listIsAt(t, term, "frame 1 list 69", 69, 64, 74) _, err := term.Exec("frame 50 list") if err == nil { t.Fatalf("Expected error requesting 50th frame") diff --git a/service/dap/server.go b/service/dap/server.go index 1f0eade0..29fa9068 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1150,11 +1150,21 @@ 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() + ")" + ">" 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() + ")" + ">" + loadExpr := fmt.Sprintf("*(*%q)(%#x)", typeName, v.Addr) + // We might be loading variables from the frame that's not topmost, so use + // frame-independent address-based expression. + s.log.Debugf("loading %s (type %s) with %s", qualifiedNameOrExpr, typeName, loadExpr) + vLoaded, err := s.debugger.EvalVariableInScope(-1, 0, 0, loadExpr, DefaultLoadConfig) + if err != nil { + value += fmt.Sprintf(" - FAILED TO LOAD: %s", err) + } else { + v.Children = vLoaded.Children + } + } + // Provide a reference to the child only if it fully loaded + if !v.Children[0].OnlyAddr { // TODO(polina): should we remove one level of indirection and skip "data"? // Then we will have: // Before: diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 02310022..45a071a4 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -751,7 +751,7 @@ func TestScopesAndVariablesRequests(t *testing.T) { client.StackTraceRequest(1, 0, 20) stack := client.ExpectStackTraceResponse(t) - startLineno := 65 + 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 @@ -794,7 +794,7 @@ func TestScopesAndVariablesRequests(t *testing.T) { client.VariablesRequest(1001) locals := client.ExpectVariablesResponse(t) - expectChildren(t, locals, "Locals", 30) + expectChildren(t, locals, "Locals", 31) // reflect.Kind == Bool expectVarExact(t, locals, -1, "b1", "b1", "true", noChildren) @@ -1397,42 +1397,17 @@ func TestVariablesLoading(t *testing.T) { ref = expectVarExact(t, tm, 0, "v", "tm.v", "<[]map[string]main.astruct> (length: 1, cap: 1)", hasChildren) if ref > 0 { client.VariablesRequest(ref) - tm_v := client.ExpectVariablesResponse(t) - expectChildren(t, tm_v, "tm.v", 1) + tmV := client.ExpectVariablesResponse(t) + expectChildren(t, tmV, "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, loaded: 0)", noChildren) + expectVarExact(t, tmV, 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) { + runTest(t, "testvariables", func(client *daptest.Client, fixture protest.Fixture) { runDebugSessionWithBPs(t, client, "launch", // Launch func() { @@ -1448,14 +1423,54 @@ func TestVariablesLoading(t *testing.T) { client.StackTraceRequest(1, 0, 0) client.ExpectStackTraceResponse(t) - client.ScopesRequest(1000) - client.ExpectScopesResponse(t) + var loadvars = func(frame int) { + client.ScopesRequest(frame) + scopes := client.ExpectScopesResponse(t) + localsRef := 0 + for _, s := range scopes.Body.Scopes { + if s.Name == "Locals" { + localsRef = s.VariablesReference + } + } - client.VariablesRequest(1001) // Locals - locals := client.ExpectVariablesResponse(t) + client.VariablesRequest(localsRef) + locals := client.ExpectVariablesResponse(t) + + // Interface auto-loaded when hitting LoadConfig.MaxVariableRecurse=1 - // Pointer value not loaded - expectVarRegex(t, locals, -1, "ptrinf", "ptrinf", `<\*interface {}>\(0x[0-9a-f]+\) \(value not loaded\)`, noChildren) + ref := expectVarExact(t, locals, -1, "ni", "ni", "<[]interface {}> (length: 1, cap: 1)", hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + ni := client.ExpectVariablesResponse(t) + ref = expectVarExact(t, ni, 0, "[0]", "ni[0]", "", hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + niI1 := client.ExpectVariablesResponse(t) + ref = expectVarExact(t, niI1, 0, "data", "ni[0].(data)", "<[]interface {}> (length: 1, cap: 1)", hasChildren) + if ref > 0 { + // Auto-loading happens here + client.VariablesRequest(ref) + niI1Data := client.ExpectVariablesResponse(t) + ref = expectVarExact(t, niI1Data, 0, "[0]", "ni[0].(data)[0]", "", hasChildren) + if ref > 0 { + client.VariablesRequest(ref) + niI1DataI2 := client.ExpectVariablesResponse(t) + expectVarExact(t, niI1DataI2, 0, "data", "ni[0].(data)[0].(data)", "123", noChildren) + } + } + } + } + + // Pointer value not loaded with LoadConfig.FollowPointers=false + expectVarRegex(t, locals, -1, "a7", "a7", `<\*main.FooBar>\(0x[0-9a-f]+\) \(value not loaded\)`, noChildren) + } + loadvars(1000 /*first topmost frame*/) + // step into another function + client.StepInRequest(1) + client.ExpectStepInResponse(t) + client.ExpectStoppedEvent(t) + handleStop(t, client, 1, "main.barfoo", 24) + loadvars(1001 /*second frame here is same as topmost above*/) }, disconnect: true, }}) @@ -1676,7 +1691,7 @@ func TestEvaluateRequest(t *testing.T) { fixture.Source, []int{}, // Breakpoint set in the program []onBreakpoint{{ // Stop at first breakpoint execute: func() { - handleStop(t, client, 1, "main.foobar", 65) + handleStop(t, client, 1, "main.foobar", 66) // Variable lookup client.EvaluateRequest("a2", 1000, "this context will be ignored") diff --git a/service/test/variables_test.go b/service/test/variables_test.go index baea4fa9..c2087930 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -446,6 +446,7 @@ func TestLocalVariables(t *testing.T) { {"mp", true, "map[int]interface {} [1: 42, 2: 43, ]", "", "map[int]interface {}", nil}, {"ms", true, "main.Nest {Level: 0, Nest: *main.Nest {Level: 1, Nest: *(*main.Nest)…", "", "main.Nest", nil}, {"neg", true, "-1", "", "int", nil}, + {"ni", true, "[]interface {} len: 1, cap: 1, [[]interface {} len: 1, cap: 1, [*(*interface {})…", "", "[]interface {}", nil}, {"u16", true, "65535", "", "uint16", nil}, {"u32", true, "4294967295", "", "uint32", nil}, {"u64", true, "18446744073709551615", "", "uint64", nil}, -- GitLab