proc: fix embedded field search (#2320)

Both structMember and findMethod implemented a depth-first search in
embedded fields but the Go specification requires a breadth-first
search. They also allowed promotion of fields in the concrete type of
embedded interfaces even though this is not allowed by Go.
Furthermore they both lacked protection from infinite recursion
when a type embeds itself and the user requests a non-existent field.

Fixes #2316

_fixtures/testvariables2.go

@@ -106,6 +106,38 @@ type List struct {
 	Next *List
 }
 
+type T struct {
+	F string
+}
+
+type W1 struct {
+	T
+}
+
+func (*W1) M() {}
+
+type I interface{ M() }
+
+type W2 struct {
+	W1
+	T
+}
+
+type W3 struct {
+	I
+	T
+}
+
+type W4 struct {
+	I
+}
+
+type W5 struct {
+	*W5
+}
+
+var _ I = (*W2)(nil)
+
 func main() {
 	i1 := 1
 	i2 := 2
@@ -312,6 +344,12 @@ func main() {
 	ll := &List{0, &List{1, &List{2, &List{3, &List{4, nil}}}}}
 	unread := (*int)(unsafe.Pointer(uintptr(12345)))
 
+	w2 := &W2{W1{T{"T-inside-W1"}}, T{"T-inside-W2"}}
+	w3 := &W3{&W1{T{"T-inside-W1"}}, T{"T-inside-W3"}}
+	w4 := &W4{&W1{T{"T-inside-W1"}}}
+	w5 := &W5{nil}
+	w5.W5 = w5
+
 	var amb1 = 1
 	runtime.Breakpoint()
 	for amb1 := 0; amb1 < 10; amb1++ {
@@ -319,5 +357,5 @@ func main() {
 	}
 
 	runtime.Breakpoint()
-	fmt.Println(i1, i2, i3, p1, 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)
+	fmt.Println(i1, i2, i3, p1, 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)
 }

pkg/proc/eval.go

@@ -1963,73 +1963,79 @@ func (v *Variable) findMethod(mname string) (*Variable, error) {
 		return v.Children[0].findMethod(mname)
 	}
 
-	typ := v.DwarfType
+	queue := []*Variable{v}
-	ptyp, isptr := typ.(*godwarf.PtrType)
+	seen := map[string]struct{}{}
-	if isptr {
-		typ = ptyp.Type
-	}
 
-	typePath := typ.Common().Name
+	for len(queue) > 0 {
-	dot := strings.LastIndex(typePath, ".")
+		v := queue[0]
-	if dot < 0 {
+		queue = append(queue[:0], queue[1:]...)
-		// probably just a C type
+		if _, isseen := seen[v.RealType.String()]; isseen {
-		return nil, nil
+			continue
-	}
-
-	pkg := typePath[:dot]
-	receiver := typePath[dot+1:]
-
-	if fn, ok := v.bi.LookupFunc[fmt.Sprintf("%s.%s.%s", pkg, receiver, mname)]; ok {
-		r, err := functionToVariable(fn, v.bi, v.mem)
-		if err != nil {
-			return nil, err
 		}
+		seen[v.RealType.String()] = struct{}{}
+
+		typ := v.DwarfType
+		ptyp, isptr := typ.(*godwarf.PtrType)
 		if isptr {
-			r.Children = append(r.Children, *(v.maybeDereference()))
+			typ = ptyp.Type
-		} else {
-			r.Children = append(r.Children, *v)
 		}
-		return r, nil
-	}
 
-	if fn, ok := v.bi.LookupFunc[fmt.Sprintf("%s.(*%s).%s", pkg, receiver, mname)]; ok {
+		typePath := typ.Common().Name
-		r, err := functionToVariable(fn, v.bi, v.mem)
+		dot := strings.LastIndex(typePath, ".")
-		if err != nil {
+		if dot < 0 {
-			return nil, err
+			// probably just a C type
+			continue
 		}
-		if isptr {
-			r.Children = append(r.Children, *v)
-		} else {
-			r.Children = append(r.Children, *(v.pointerToVariable()))
-		}
-		return r, nil
-	}
-	return v.tryFindMethodInEmbeddedFields(mname)
-}
 
-func (v *Variable) tryFindMethodInEmbeddedFields(mname string) (*Variable, error) {
+		pkg := typePath[:dot]
-	structVar := v.maybeDereference()
+		receiver := typePath[dot+1:]
-	structVar.Name = v.Name
+
-	if structVar.Unreadable != nil {
+		if fn, ok := v.bi.LookupFunc[fmt.Sprintf("%s.%s.%s", pkg, receiver, mname)]; ok {
-		return structVar, nil
+			r, err := functionToVariable(fn, v.bi, v.mem)
-	}
+			if err != nil {
-	switch t := structVar.RealType.(type) {
+				return nil, err
-	case *godwarf.StructType:
+			}
-		for _, field := range t.Field {
+			if isptr {
-			if field.Embedded {
+				r.Children = append(r.Children, *(v.maybeDereference()))
-				// Recursively check for promoted fields on the embedded field
+			} else {
-				embeddedVar, err := structVar.toField(field)
+				r.Children = append(r.Children, *v)
-				if err != nil {
+			}
-					return nil, err
+			return r, nil
-				}
+		}
-				if embeddedMethod, err := embeddedVar.findMethod(mname); err != nil {
+
-					return nil, err
+		if fn, ok := v.bi.LookupFunc[fmt.Sprintf("%s.(*%s).%s", pkg, receiver, mname)]; ok {
-				} else if embeddedMethod != nil {
+			r, err := functionToVariable(fn, v.bi, v.mem)
-					return embeddedMethod, nil
+			if err != nil {
+				return nil, err
+			}
+			if isptr {
+				r.Children = append(r.Children, *v)
+			} else {
+				r.Children = append(r.Children, *(v.pointerToVariable()))
+			}
+			return r, nil
+		}
+
+		// queue embedded fields for search
+		structVar := v.maybeDereference()
+		structVar.Name = v.Name
+		if structVar.Unreadable != nil {
+			return structVar, nil
+		}
+		switch t := structVar.RealType.(type) {
+		case *godwarf.StructType:
+			for _, field := range t.Field {
+				if field.Embedded {
+					embeddedVar, err := structVar.toField(field)
+					if err != nil {
+						return nil, err
+					}
+					queue = append(queue, embeddedVar)
 				}
 			}
 		}
 	}
+
 	return nil, nil
 }
 

pkg/proc/variables.go

@@ -1060,59 +1060,61 @@ func (v *Variable) structMember(memberName string) (*Variable, error) {
 			v = &v.Children[0]
 		}
 	}
-	structVar := v.maybeDereference()
-	structVar.Name = v.Name
-	if structVar.Unreadable != nil {
-		return structVar, nil
-	}
 
-	switch t := structVar.RealType.(type) {
+	queue := []*Variable{v}
-	case *godwarf.StructType:
+	seen := map[string]struct{}{} // prevent infinite loops
-		for _, field := range t.Field {
+	first := true
-			if field.Name != memberName {
+
-				continue
+	for len(queue) > 0 {
-			}
+		v := queue[0]
-			return structVar.toField(field)
+		queue = append(queue[:0], queue[1:]...)
+		if _, isseen := seen[v.RealType.String()]; isseen {
+			continue
 		}
-		// Check for embedded field only if field was
+		seen[v.RealType.String()] = struct{}{}
-		// not a regular struct member
+
-		for _, field := range t.Field {
+		structVar := v.maybeDereference()
-			isEmbeddedStructMember :=
+		structVar.Name = v.Name
-				field.Embedded ||
+		if structVar.Unreadable != nil {
-					(field.Type.Common().Name == field.Name) ||
+			return structVar, nil
-					(len(field.Name) > 1 &&
+		}
-						field.Name[0] == '*' &&
+
-						field.Type.Common().Name[1:] == field.Name[1:])
+		switch t := structVar.RealType.(type) {
-			if !isEmbeddedStructMember {
+		case *godwarf.StructType:
-				continue
+			for _, field := range t.Field {
-			}
+				if field.Name == memberName {
-			// Check for embedded field referenced by type name
+					return structVar.toField(field)
-			parts := strings.Split(field.Name, ".")
+				}
-			if len(parts) > 1 && parts[1] == memberName {
+				isEmbeddedStructMember :=
+					field.Embedded ||
+						(field.Type.Common().Name == field.Name) ||
+						(len(field.Name) > 1 &&
+							field.Name[0] == '*' &&
+							field.Type.Common().Name[1:] == field.Name[1:])
+				if !isEmbeddedStructMember {
+					continue
+				}
 				embeddedVar, err := structVar.toField(field)
 				if err != nil {
 					return nil, err
 				}
-				return embeddedVar, nil
+				// Check for embedded field referenced by type name
+				parts := strings.Split(field.Name, ".")
+				if len(parts) > 1 && parts[1] == memberName {
+					return embeddedVar, nil
+				}
+				embeddedVar.Name = structVar.Name
+				queue = append(queue, embeddedVar)
 			}
-			// Recursively check for promoted fields on the embedded field
+		default:
-			embeddedVar, err := structVar.toField(field)
+			if first {
-			if err != nil {
+				return nil, fmt.Errorf("%s (type %s) is not a struct", vname, structVar.TypeString())
-				return nil, err
-			}
-			embeddedVar.Name = structVar.Name
-			embeddedField, _ := embeddedVar.structMember(memberName)
-			if embeddedField != nil {
-				return embeddedField, nil
 			}
 		}
-		return nil, fmt.Errorf("%s has no member %s", vname, memberName)
+		first = false
-	default:
-		if v.Name == "" {
-			return nil, fmt.Errorf("type %s is not a struct", structVar.TypeString())
-		}
-		return nil, fmt.Errorf("%s (type %s) is not a struct", vname, structVar.TypeString())
 	}
+
+	return nil, fmt.Errorf("%s has no member %s", vname, memberName)
 }
 
 func readVarEntry(entry *godwarf.Tree, image *Image) (name string, typ godwarf.Type, err error) {

service/dap/server_test.go

@@ -1026,7 +1026,7 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
 				execute: func() {
 					client.StackTraceRequest(1, 0, 20)
 					stack := client.ExpectStackTraceResponse(t)
-					expectStackFrames(t, stack, "main.main", 317, 1000, 3, 3)
+					expectStackFrames(t, stack, "main.main", -1, 1000, 3, 3)
 
 					client.ScopesRequest(1000)
 					scopes := client.ExpectScopesResponse(t)
@@ -1039,7 +1039,7 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
 				execute: func() {
 					client.StackTraceRequest(1, 0, 20)
 					stack := client.ExpectStackTraceResponse(t)
-					expectStackFrames(t, stack, "main.main", 322, 1000, 3, 3)
+					expectStackFrames(t, stack, "main.main", -1, 1000, 3, 3)
 
 					client.ScopesRequest(1000)
 					scopes := client.ExpectScopesResponse(t)

service/test/variables_test.go

@@ -504,6 +504,20 @@ func TestEmbeddedStruct(t *testing.T) {
 			{"b.C.s", true, "\"hello\"", "\"hello\"", "string", nil},
 			{"b.s", true, "\"hello\"", "\"hello\"", "string", nil},
 			{"b2", true, "main.B {A: main.A {val: 42}, C: *main.C nil, a: main.A {val: 47}, ptr: *main.A nil}", "main.B {A: (*main.A)(0x…", "main.B", nil},
+
+			// Issue 2316: field promotion is breadth first and embedded interfaces never get promoted
+			{"w2.W1.T.F", true, `"T-inside-W1"`, `"T-inside-W1"`, "string", nil},
+			{"w2.W1.F", true, `"T-inside-W1"`, `"T-inside-W1"`, "string", nil},
+			{"w2.T.F", true, `"T-inside-W2"`, `"T-inside-W2"`, "string", nil},
+			{"w2.F", true, `"T-inside-W2"`, `"T-inside-W2"`, "string", nil},
+			{"w3.I.T.F", false, `"T-inside-W1"`, `"T-inside-W1"`, "string", nil},
+			{"w3.I.F", false, `"T-inside-W1"`, `"T-inside-W1"`, "string", nil},
+			{"w3.T.F", true, `"T-inside-W3"`, `"T-inside-W3"`, "string", nil},
+			{"w3.F", true, `"T-inside-W3"`, `"T-inside-W3"`, "string", nil},
+			{"w4.I.T.F", false, `"T-inside-W1"`, `"T-inside-W1"`, "string", nil},
+			{"w4.I.F", false, `"T-inside-W1"`, `"T-inside-W1"`, "string", nil},
+			{"w4.F", false, ``, ``, "", errors.New("w4 has no member F")},
+			{"w5.F", false, ``, ``, "", errors.New("w5 has no member F")},
 		}
 		assertNoError(p.Continue(), t, "Continue()")