internal/gocore: have single entry point for makeMemRoot (refactor)

I was reading through the code and found that getting all references
(places where roots are made) was made a little more difficult by having
a separate `p.makeMemRoot` versus `makeMemRoot`.

I can understand why it was done: to make `gocore.Core(...)` function
rely more on `core.Process` and make it clearer what the dependencies
are. But the `nRoots` passing is awkward.

Change-Id: I3d8f6f8d9c46a09740a4d28fbe742e0af27b806a
Reviewed-on: https://go-review.googlesource.com/c/debug/+/748440
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Nicolas Hillegeer <aktau@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>

internal/gocore/dwarf.go

diff --git a/internal/gocore/dwarf.go b/internal/gocore/dwarf.go
index e6ade63..cc632c8 100644
--- a/internal/gocore/dwarf.go
+++ b/internal/gocore/dwarf.go
@@ -301,8 +301,8 @@
 	return consts, nil
 }
 
-func readDWARFGlobals(p *core.Process, nRoots *int, dwarfTypeMap map[dwarf.Type]*Type) ([]*Root, error) {
-	d, err := p.DWARF()
+func readDWARFGlobals(p *Process, dwarfTypeMap map[dwarf.Type]*Type) ([]*Root, error) {
+	d, err := p.proc.DWARF()
 	if err != nil {
 		return nil, fmt.Errorf("failed to read DWARF: %v", err)
 	}
@@ -330,13 +330,13 @@
 			continue
 		}
 		var a core.Address
-		if p.PtrSize() == 8 {
-			a = core.Address(p.ByteOrder().Uint64(loc[1:]))
+		if p.proc.PtrSize() == 8 {
+			a = core.Address(p.proc.ByteOrder().Uint64(loc[1:]))
 		} else {
-			a = core.Address(p.ByteOrder().Uint32(loc[1:]))
+			a = core.Address(p.proc.ByteOrder().Uint32(loc[1:]))
 		}
-		a = a.Add(int64(p.StaticBase()))
-		if !p.Writeable(a) {
+		a = a.Add(int64(p.proc.StaticBase()))
+		if !p.proc.Writeable(a) {
 			// Read-only globals can't have heap pointers.
 			// TODO: keep roots around anyway?
 			continue
@@ -357,7 +357,7 @@
 			continue
 		}
 		typ := dwarfTypeMap[dt]
-		roots = append(roots, makeMemRoot(nRoots, nf.Val.(string), typ, nil, a))
+		roots = append(roots, p.makeMemRoot(nf.Val.(string), typ, nil, a))
 	}
 	return roots, nil
 }

internal/gocore/process.go

diff --git a/internal/gocore/process.go b/internal/gocore/process.go
index 03606b9..7226369 100644
--- a/internal/gocore/process.go
+++ b/internal/gocore/process.go
@@ -74,7 +74,7 @@
 	ridx             []int64
 	// Sorted list of all roots, sorted by id.
 	rootIdx []*Root
-	nRoots  int
+	nRoots  int // ID of next root.
 }
 
 type reverseEdge struct {
@@ -104,7 +104,7 @@
 	if err != nil {
 		return nil, err
 	}
-	p.globals, err = readDWARFGlobals(proc, &p.nRoots, p.dwarfTypeMap)
+	p.globals, err = readDWARFGlobals(p, p.dwarfTypeMap)
 	if err != nil {
 		return nil, err
 	}
@@ -132,8 +132,8 @@
 		return nil, err
 	}
 
-	// Fix up global roots that don't appear in DWARF, but still have live pointers.
-	p.globals, err = fixUpGlobals(proc, p.modules, p.globals, p.rtTypeByName["unsafe.Pointer"], &p.nRoots)
+	// Fix up global roots (p.globals] that don't appear in DWARF, but still have live pointers.
+	err = fixUpGlobals(p, p.rtTypeByName["unsafe.Pointer"])
 	if err != nil {
 		return nil, err
 	}
@@ -206,30 +206,31 @@
 	}
 }
 
-func fixUpGlobals(p *core.Process, modules []*module, globals []*Root, unsafePtrType *Type, nRoots *int) ([]*Root, error) {
-	slices.SortFunc(globals, func(a, b *Root) int {
+func fixUpGlobals(p *Process, unsafePtrType *Type) error {
+	slices.SortFunc(p.globals, func(a, b *Root) int {
 		// Assume all globals have an address.
 		return cmp.Compare(a.Addr(), b.Addr())
 	})
 	var extraGlobals []*Root // Extra global roots not described by DWARF.
-	forEachGlobalPtr(p, modules, func(addr core.Address) bool {
-		if len(globals) > 0 {
+	forEachGlobalPtr(p.proc, p.modules, func(addr core.Address) bool {
+		if len(p.globals) > 0 {
 			// Skip over globals that are already accounted for.
-			i, ok := slices.BinarySearchFunc(globals, addr, func(a *Root, b core.Address) int {
+			i, ok := slices.BinarySearchFunc(p.globals, addr, func(a *Root, b core.Address) int {
 				return cmp.Compare(a.Addr(), b)
 			})
 			if i >= 0 && !ok {
 				i--
 			}
-			if i >= 0 && globals[i].Addr() != 0 && addr >= globals[i].Addr() && addr < globals[i].Addr().Add(globals[i].Type.Size) {
+			if i >= 0 && p.globals[i].Addr() != 0 && addr >= p.globals[i].Addr() && addr < p.globals[i].Addr().Add(p.globals[i].Type.Size) {
 				return true
 			}
 		}
-		root := makeMemRoot(nRoots, "unk", unsafePtrType, nil, addr)
+		root := p.makeMemRoot("unk", unsafePtrType, nil, addr)
 		extraGlobals = append(extraGlobals, root)
 		return true
 	})
-	return append(globals, extraGlobals...), nil
+	p.globals = append(p.globals, extraGlobals...)
+	return nil
 }
 
 // arena is a summary of the size of components of a heapArena.

internal/gocore/root.go

diff --git a/internal/gocore/root.go b/internal/gocore/root.go
index 42eb067..f16be6f 100644
--- a/internal/gocore/root.go
+++ b/internal/gocore/root.go
@@ -41,19 +41,7 @@
 }
 
 func (p *Process) makeMemRoot(name string, typ *Type, fr *Frame, addr core.Address) *Root {
-	return makeMemRoot(&p.nRoots, name, typ, fr, addr)
-}
-
-func makeMemRoot(nRoots *int, name string, typ *Type, fr *Frame, addr core.Address) *Root {
-	r := &Root{
-		Name:   name,
-		Type:   typ,
-		Frame:  fr,
-		pieces: []rootPiece{{size: typ.Size, kind: addrPiece, value: uint64(addr)}},
-		id:     *nRoots,
-	}
-	*nRoots += 1
-	return r
+	return p.makeCompositeRoot(name, typ, fr, []rootPiece{{size: typ.Size, kind: addrPiece, value: uint64(addr)}})
 }
 
 func (p *Process) makeCompositeRoot(name string, typ *Type, fr *Frame, pieces []rootPiece) *Root {