gocore: overhaul roots, expand testing

This change overhauls how roots work to prepare for supporting composite values (roots "de-structured" and broken up into pieces, despite representing a single type). It also expands testing a little bit to prevent backsliding from this point. Roots now do not have an address identity at all, and callers do not rely on that fact anymore. They have a forged identity which is useful for some calculations, but otherwise can only be identified by a *Root. Pointers may now be found in composite roots, which is nice, so all destructured values work correctly for the sake of iterating over all pointers. Finally, this fixes a small, old bug in typeObject where the type pointer would be interrogated before we filtered out whether a interface was nil or not. Because we filter dead pointers by making them appear nil, this means we could end up looking at a bogus, clobbered type pointer. (This may be a bug in the compiler's DWARF generation, the type shouldn't be clobbered.) Change-Id: I09217b2070dc6f4bf4b1dbd28d52f570ba61ca4a Reviewed-on: https://go-review.googlesource.com/c/debug/+/635857 Auto-Submit: Michael Knyszek <[email protected]> Reviewed-by: Nicolas Hillegeer <[email protected]> Reviewed-by: Michael Pratt <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
golang · Dec 16, 2024 · 71db946 · 71db946
1 parent 5f2bbc1
commit 71db946
Show file tree

Hide file tree

Showing 12 changed files with 480 additions and 219 deletions.
diff --git a/cmd/viewcore/html.go b/cmd/viewcore/html.go
@@ -170,7 +170,9 @@ func serveHTML(c *gocore.Process, port int, async bool) {
 			fmt.Fprintf(w, "<table>\n")
 			fmt.Fprintf(w, "<tr><th align=left>field</th><th align=left colspan=\"2\">type</th><th align=left>value</th></tr>\n")
 			for _, r := range f.Roots() {
-				htmlObject(w, c, r.Name, r.Addr, r.Type, f.Live)
+				if r.HasAddress() {
+					htmlObject(w, c, r.Name, r.Addr(), r.Type, f.Live)
+				}
 			}
 			fmt.Fprintf(w, "</table>\n")
 		}
@@ -181,7 +183,9 @@ func serveHTML(c *gocore.Process, port int, async bool) {
 		fmt.Fprintf(w, "<table>\n")
 		fmt.Fprintf(w, "<tr><th align=left>field</th><th align=left colspan=\"2\">type</th><th align=left>value</th></tr>\n")
 		for _, r := range c.Globals() {
-			htmlObject(w, c, r.Name, r.Addr, r.Type, nil)
+			if r.HasAddress() {
+				htmlObject(w, c, r.Name, r.Addr(), r.Type, nil)
+			}
 		}
 		fmt.Fprintf(w, "</table>\n")
 	})

diff --git a/internal/gocore/dominator.go b/internal/gocore/dominator.go
@@ -228,7 +228,7 @@ func (d *ltDom) calculate() {
 			d.p.ForEachReversePtr(obj, func(x Object, r *Root, _, _ int64) bool {
 				var v int
 				if r != nil {
-					v = d.p.findRootIndex(r) + 1
+					v = r.id + 1
 				} else {
 					v, _ = d.p.findObjectIndex(d.p.Addr(x))
 					v += d.nRoots + 1
@@ -397,7 +397,7 @@ func (d *ltDom) dot(w io.Writer) {
 			if len(typeName) > 30 {
 				typeName = typeName[:30]
 			}
-			label = fmt.Sprintf("root %s %#x (type %s)", root.Name, root.Addr, typeName)
+			label = fmt.Sprintf("root %s (type %s)", root.Name, typeName)
 		default:
 			typ, _ := d.p.Type(obj)
 			var typeName string

diff --git a/internal/gocore/dominator_test.go b/internal/gocore/dominator_test.go
@@ -123,7 +123,7 @@ func checkDominator(t *testing.T, d ltDom) bool {
 		case n == pseudoRoot:
 			return &pRoot
 		case r != nil:
-			return &roots[d.p.findRootIndex(r)]
+			return &roots[r.id]
 		default:
 			idx, _ := d.p.findObjectIndex(d.p.Addr(o))
 			return &objects[idx]
@@ -168,7 +168,7 @@ type sanityVertex struct {
 func (v *sanityVertex) String(p *Process) string {
 	switch {
 	case v.root != nil:
-		return fmt.Sprintf("root %s %#x (type %s)", v.root.Name, v.root.Addr, v.root.Type)
+		return fmt.Sprintf("root %s (type %s)", v.root.Name, v.root.Type)
 	case v.obj != 0:
 		typ, _ := p.Type(v.obj)
 		var typeName string

diff --git a/internal/gocore/dwarf.go b/internal/gocore/dwarf.go
@@ -299,7 +299,7 @@ func readConstants(p *core.Process) (constsMap, error) {
 	return consts, nil
 }
 
-func readGlobals(p *core.Process, dwarfTypeMap map[dwarf.Type]*Type) ([]*Root, error) {
+func readGlobals(p *core.Process, nRoots *int, dwarfTypeMap map[dwarf.Type]*Type) ([]*Root, error) {
 	d, err := p.DWARF()
 	if err != nil {
 		return nil, fmt.Errorf("failed to read DWARF: %v", err)
@@ -354,12 +354,8 @@ func readGlobals(p *core.Process, dwarfTypeMap map[dwarf.Type]*Type) ([]*Root, e
 		if nf == nil {
 			continue
 		}
-		roots = append(roots, &Root{
-			Name:  nf.Val.(string),
-			Addr:  a,
-			Type:  dwarfTypeMap[dt],
-			Frame: nil,
-		})
+		typ := dwarfTypeMap[dt]
+		roots = append(roots, makeMemRoot(nRoots, nf.Val.(string), typ, nil, a))
 	}
 	return roots, nil
 }

diff --git a/internal/gocore/gocore_test.go b/internal/gocore/gocore_test.go
@@ -253,24 +253,55 @@ func TestObjects(t *testing.T) {
 			{"-buildmode=pie"},
 		} {
 			t.Run(strings.Join(buildFlags, ","), func(t *testing.T) {
+				const largeObjectThreshold = 32768
+
 				p := loadExampleGenerated(t, buildFlags...)
+
+				// Statistics to check.
 				n := 0
-				const largeObjectThreshold = 32768
-				large := 0 // Number of objects larger than (or equal to largeObjectThreshold)
+				largeObjects := 0 // Number of objects larger than (or equal to largeObjectThreshold)
+				myPairObjects := 0
+				anyNodeObjects := 0
+				typeSafeNodeObjects := 0
+
 				p.ForEachObject(func(x Object) bool {
 					siz := p.Size(x)
-					t.Logf("%s size=%d", typeName(p, x), p.Size(x))
+					typ := typeName(p, x)
+					t.Logf("%s size=%d", typ, p.Size(x))
 					if siz >= largeObjectThreshold {
-						large++
+						largeObjects++
+					}
+					switch typ {
+					case "main.myPair":
+						myPairObjects++
+					case "main.anyNode":
+						anyNodeObjects++
+					case "main.typeSafeNode[main.myPair]":
+						typeSafeNodeObjects++
 					}
 					n++
 					return true
 				})
 				if n < 10 {
 					t.Errorf("#objects = %d, want >10", n)
 				}
-				if large != 1 {
-					t.Errorf("expected exactly one object larger than %d, found %d", largeObjectThreshold, large)
+				if largeObjects != 1 {
+					t.Errorf("expected exactly one object larger than %d, found %d", largeObjectThreshold, largeObjects)
+				}
+
+				// Check object counts.
+				const depth = 5
+				const tsTrees = 3
+				const anTrees = 2
+				const nodes = 1<<depth - 1
+				if want := tsTrees*nodes + anTrees*nodes*2; myPairObjects != want {
+					t.Errorf("expected exactly %d main.myPair objects, found %d", want, myPairObjects)
+				}
+				if want := anTrees * nodes; anyNodeObjects != want {
+					t.Errorf("expected exactly %d main.anyNode objects, found %d", want, anyNodeObjects)
+				}
+				if want := tsTrees * nodes; typeSafeNodeObjects != want {
+					t.Errorf("expected exactly %d main.typeSafeNode[main.myPair] objects, found %d", want, typeSafeNodeObjects)
 				}
 			})
 		}

diff --git a/internal/gocore/goroutine.go b/internal/gocore/goroutine.go
@@ -13,9 +13,6 @@ type Goroutine struct {
 	stackSize int64  // current stack allocation
 	frames    []*Frame
 
-	// Registers containing live pointers.
-	regRoots []*Root
-
 	// TODO: defers, in-progress panics
 }
 

diff --git a/internal/gocore/object.go b/internal/gocore/object.go
@@ -62,9 +62,6 @@ func (p *Process) markObjects() {
 	// Not a huge deal given that we'll just ignore outright bad pointers, but
 	// we may accidentally mark some objects as live erroneously.
 	for _, g := range p.goroutines {
-		for _, r := range g.regRoots {
-			add(core.Address(r.RegValue))
-		}
 		for _, f := range g.frames {
 			for a := range f.Live {
 				add(p.proc.ReadPtr(a))
@@ -92,11 +89,10 @@ func (p *Process) markObjects() {
 		if !strings.HasPrefix(r.Name, "finalizer for ") {
 			continue
 		}
-		for _, f := range r.Type.Fields {
-			if f.Type.Kind == KindPtr {
-				add(p.proc.ReadPtr(r.Addr.Add(f.Off)))
-			}
-		}
+		p.ForEachRootPtr(r, func(_ int64, o Object, _ int64) bool {
+			add(core.Address(o))
+			return true
+		})
 	}
 
 	// Expand root set to all reachable objects.
@@ -224,11 +220,6 @@ func (p *Process) ForEachRoot(fn func(r *Root) bool) {
 		}
 	}
 	for _, g := range p.goroutines {
-		for _, r := range g.regRoots {
-			if !fn(r) {
-				return
-			}
-		}
 		for _, f := range g.frames {
 			for _, r := range f.roots {
 				if !fn(r) {
@@ -287,65 +278,8 @@ func (p *Process) ForEachPtr(x Object, fn func(int64, Object, int64) bool) {
 
 // ForEachRootPtr behaves like ForEachPtr but it starts with a Root instead of an Object.
 func (p *Process) ForEachRootPtr(r *Root, fn func(int64, Object, int64) bool) {
-	edges1(p, r, 0, r.Type, fn)
-}
-
-// edges1 calls fn for the edges found in an object of type t living at offset off in the root r.
-// If fn returns false, return immediately with false.
-func edges1(p *Process, r *Root, off int64, t *Type, fn func(int64, Object, int64) bool) bool {
-	switch t.Kind {
-	case KindBool, KindInt, KindUint, KindFloat, KindComplex:
-		// no edges here
-	case KindIface, KindEface:
-		// The first word is a type or itab.
-		// Itabs are never in the heap.
-		// Types might be, though.
-		a := r.Addr.Add(off)
-		if r.Frame == nil || r.Frame.Live[a] {
-			dst, off2 := p.FindObject(p.proc.ReadPtr(a))
-			if dst != 0 {
-				if !fn(off, dst, off2) {
-					return false
-				}
-			}
-		}
-		// Treat second word like a pointer.
-		off += p.proc.PtrSize()
-		fallthrough
-	case KindPtr, KindString, KindSlice, KindFunc:
-		if t.Kind == KindPtr && r.Addr == 0 {
-			dst, off2 := p.FindObject(core.Address(r.RegValue))
-			if dst != 0 {
-				if !fn(off, dst, off2) {
-					return false
-				}
-			}
-			break
-		}
-		a := r.Addr.Add(off)
-		if r.Frame == nil || r.Frame.Live[a] {
-			dst, off2 := p.FindObject(p.proc.ReadPtr(a))
-			if dst != 0 {
-				if !fn(off, dst, off2) {
-					return false
-				}
-			}
-		}
-	case KindArray:
-		s := t.Elem.Size
-		for i := int64(0); i < t.Count; i++ {
-			if !edges1(p, r, off+i*s, t.Elem, fn) {
-				return false
-			}
-		}
-	case KindStruct:
-		for _, f := range t.Fields {
-			if !edges1(p, r, off+f.Off, f.Type, fn) {
-				return false
-			}
-		}
-	}
-	return true
+	ptrBuf := make([]byte, 8)
+	walkRootTypePtrs(p, r, ptrBuf, 0, r.Type, fn)
 }
 
 const heapInfoSize = 512