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: NPolina Sokolova <polinasok@users.noreply.github.com>

_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

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")

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:

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]", "<map[string]main.astruct> (length: 66, loaded: 0)", noChildren)
+							expectVarExact(t, tmV, 0, "[0]", "tm.v[0]", "<map[string]main.astruct> (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)", "<interface {}(**interface {})>", 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)))", "<interface {}(**interface {})> (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]", "<interface {}([]interface {})>", 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]", "<interface {}(int)>", 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")

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},