gocore: record global pointers not described in DWARF as roots
Currently globals are entirely defined by what we can find in DWARF, but
there are some globals (for example, runtime..interfaceSwitch.0) which
have no corresponding DWARF. This is problematic, because markObjects
uses the full set of global pointers *instead of* the roots we found to
mark objects. This is more precise, but creates a discrepancy, and
callers (such as the dominator functionality) can't actually find a path
to all listed objects.
Fix this by walking all these undescribed global pointers and recording
them explicitly as "unknown" roots. This makes viewcore much more robust
to future changes, at the expense of not being able to type the
undescribed things it finds.
This change also refactors ForEachRootPtr's internals into something
that could be executed from markObjects, and refactors markObjects to be
simpler and use the Process APIs.
Goroutine stack roots already do this, but globals do not, causing
certain intuitive invariants (like that each object in ForEachObject
should be reachable from ForEachRoot) to be violated.
Change-Id: Ie170098e5e5df929bbef5dee47b068de213a89a4
Reviewed-on: https://go-review.googlesource.com/c/debug/+/637876
Auto-Submit: Nicolas Hillegeer <aktau@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicolas Hillegeer <aktau@google.com>
diff --git a/internal/gocore/dwarf.go b/internal/gocore/dwarf.go
index aba997a..e22766f 100644
--- a/internal/gocore/dwarf.go
+++ b/internal/gocore/dwarf.go
@@ -273,7 +273,7 @@
return v, ok
}
-func readConstants(p *core.Process) (constsMap, error) {
+func readDWARFConstants(p *core.Process) (constsMap, error) {
d, err := p.DWARF()
if err != nil {
return nil, fmt.Errorf("failed to read DWARF: %v", err)
@@ -299,7 +299,7 @@
return consts, nil
}
-func readGlobals(p *core.Process, nRoots *int, dwarfTypeMap map[dwarf.Type]*Type) ([]*Root, error) {
+func readDWARFGlobals(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)
diff --git a/internal/gocore/gocore_test.go b/internal/gocore/gocore_test.go
index ac0ace5..081db20 100644
--- a/internal/gocore/gocore_test.go
+++ b/internal/gocore/gocore_test.go
@@ -222,14 +222,9 @@
t.Errorf("stat[%q].Size == 0, want >0", heapName)
}
- // TODO(aktau): Adding package os to the test binary causes the dominator test
- // to panic. We suspect the dominator code may not be working right even if it
- // doesn't crash. This needs a fixup and dedicated tests at a later time.
- if false {
- lt := runLT(p)
- if !checkDominator(t, lt) {
- t.Errorf("sanityCheckDominator(...) = false, want true")
- }
+ lt := runLT(p)
+ if !checkDominator(t, lt) {
+ t.Errorf("sanityCheckDominator(...) = false, want true")
}
}
diff --git a/internal/gocore/object.go b/internal/gocore/object.go
index 5638430..8218d7a 100644
--- a/internal/gocore/object.go
+++ b/internal/gocore/object.go
@@ -7,7 +7,6 @@
import (
"iter"
"math/bits"
- "strings"
"golang.org/x/debug/internal/core"
)
@@ -54,46 +53,20 @@
// Note that we don't just use the DWARF roots, just in case DWARF isn't complete.
// Instead we use exactly what the runtime uses.
- // Goroutine roots
- //
// BUG: If the process dumps core with a thread in mallocgc at the point
// where an object is reachable from the stack but it hasn't been zeroed
- // yet, it's possible for us to observe stale pointers here and follow them.
+ // yet, it's possible for us to observe stale pointers in a goroutine stack
+ // frame and follow them.
+ //
// 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 _, f := range g.frames {
- for a := range f.Live {
- add(p.proc.ReadPtr(a))
- }
- }
- }
-
- // Global roots
- for _, m := range p.modules {
- for _, s := range [2]string{"data", "bss"} {
- min := core.Address(m.r.Field(s).Uintptr())
- max := core.Address(m.r.Field("e" + s).Uintptr())
- gc := m.r.Field("gc" + s + "mask").Field("bytedata").Address()
- num := max.Sub(min) / ptrSize
- for i := int64(0); i < num; i++ {
- if p.proc.ReadUint8(gc.Add(i/8))>>uint(i%8)&1 != 0 {
- add(p.proc.ReadPtr(min.Add(i * ptrSize)))
- }
- }
- }
- }
-
- // Finalizers
- for _, r := range p.globals {
- if !strings.HasPrefix(r.Name, "finalizer for ") {
- continue
- }
- p.ForEachRootPtr(r, func(_ int64, o Object, _ int64) bool {
- add(core.Address(o))
+ p.ForEachRoot(func(r *Root) bool {
+ p.forEachRootPtr(r, func(_ int64, ptr core.Address) bool {
+ add(ptr)
return true
})
- }
+ return true
+ })
// Expand root set to all reachable objects.
for len(q) > 0 {
@@ -278,6 +251,20 @@
// 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) {
+ p.forEachRootPtr(r, func(off int64, ptr core.Address) bool {
+ dst, off2 := p.FindObject(ptr)
+ if dst != 0 {
+ if !fn(off, dst, off2) {
+ return false
+ }
+ }
+ return true
+ })
+}
+
+// forEachRootPtr walks all the pointers of the root, even if they don't point to
+// a valid object.
+func (p *Process) forEachRootPtr(r *Root, fn func(int64, core.Address) bool) {
ptrBuf := make([]byte, 8)
walkRootTypePtrs(p, r, ptrBuf, 0, r.Type, fn)
}
diff --git a/internal/gocore/process.go b/internal/gocore/process.go
index 5e86ace..74e71c0 100644
--- a/internal/gocore/process.go
+++ b/internal/gocore/process.go
@@ -5,12 +5,14 @@
package gocore
import (
+ "cmp"
"debug/dwarf"
"encoding/binary"
"errors"
"fmt"
"iter"
"math/bits"
+ "slices"
"sort"
"strings"
"sync"
@@ -98,11 +100,11 @@
p.rtTypeByName[name] = t
}
}
- p.rtConsts, err = readConstants(proc)
+ p.rtConsts, err = readDWARFConstants(proc)
if err != nil {
return nil, err
}
- p.globals, err = readGlobals(proc, &p.nRoots, p.dwarfTypeMap)
+ p.globals, err = readDWARFGlobals(proc, &p.nRoots, p.dwarfTypeMap)
if err != nil {
return nil, err
}
@@ -130,6 +132,12 @@
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)
+ if err != nil {
+ return nil, err
+ }
+
// Initialize the heap data structures.
p.heap, p.stats, err = readHeap(p)
if err != nil {
@@ -147,8 +155,10 @@
if err != nil {
return nil, err
}
+ // From this point on, all roots are found, initialized, and ready to use.
- p.markObjects() // needs to be after readGlobals, readGs.
+ // Find all the objects from the roots.
+ p.markObjects()
return p, nil
}
@@ -180,6 +190,48 @@
return p.funcTab.find(pc)
}
+func forEachGlobalPtr(p *core.Process, modules []*module, f func(core.Address) bool) {
+ for _, m := range modules {
+ for _, s := range [2]string{"data", "bss"} {
+ min := core.Address(m.r.Field(s).Uintptr())
+ max := core.Address(m.r.Field("e" + s).Uintptr())
+ gc := m.r.Field("gc" + s + "mask").Field("bytedata").Address()
+ num := max.Sub(min) / p.PtrSize()
+ for i := int64(0); i < num; i++ {
+ if p.ReadUint8(gc.Add(i/8))>>uint(i%8)&1 != 0 {
+ f(min.Add(i * p.PtrSize()))
+ }
+ }
+ }
+ }
+}
+
+func fixUpGlobals(p *core.Process, modules []*module, globals []*Root, unsafePtrType *Type, nRoots *int) ([]*Root, error) {
+ slices.SortFunc(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 {
+ // Skip over globals that are already accounted for.
+ i, ok := slices.BinarySearchFunc(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) {
+ return true
+ }
+ }
+ root := makeMemRoot(nRoots, "unk", unsafePtrType, nil, addr)
+ extraGlobals = append(extraGlobals, root)
+ return true
+ })
+ return append(globals, extraGlobals...), nil
+}
+
// arena is a summary of the size of components of a heapArena.
type arena struct {
heapMin core.Address
diff --git a/internal/gocore/root.go b/internal/gocore/root.go
index 8d4ac99..2a56957 100644
--- a/internal/gocore/root.go
+++ b/internal/gocore/root.go
@@ -127,7 +127,7 @@
// walkRootTypePtrs 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 walkRootTypePtrs(p *Process, r *Root, ptrBuf []byte, off int64, t *Type, fn func(int64, Object, int64) bool) bool {
+func walkRootTypePtrs(p *Process, r *Root, ptrBuf []byte, off int64, t *Type, fn func(int64, core.Address) bool) bool {
switch t.Kind {
case KindBool, KindInt, KindUint, KindFloat, KindComplex:
// no edges here
@@ -144,11 +144,8 @@
} else {
ptr = core.Address(binary.LittleEndian.Uint64(ptrBuf[:]))
}
- dst, off2 := p.FindObject(ptr)
- if dst != 0 {
- if !fn(off, dst, off2) {
- return false
- }
+ if !fn(off, ptr) {
+ return false
}
}
// Treat second word like a pointer.
@@ -163,11 +160,8 @@
} else {
ptr = core.Address(binary.LittleEndian.Uint64(ptrBuf[:]))
}
- dst, off2 := p.FindObject(ptr)
- if dst != 0 {
- if !fn(off, dst, off2) {
- return false
- }
+ if !fn(off, ptr) {
+ return false
}
}
case KindArray:
