gocore: more fixes for Go 1.24 - The isCrashFrame flag during unwinding was never reset, which could be bad. - We're apparently reading a lot of stale constants and not seeing it because a failed read from the constants map just returns the zero value. Oops! Add a much stricter map wrapper type for constants and fix up all the stale constants. This fixes a whole bunch of things, including totally incorrect readings of the liveness maps. - More strict checking of liveness map data. Change-Id: I2cb4901df4e3f473ef8b0c09981ba222e883b0b6 Reviewed-on: https://go-review.googlesource.com/c/debug/+/635835 Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Nicolas Hillegeer <aktau@google.com> Commit-Queue: 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 019b15a..b9fead4 100644 --- a/internal/gocore/dwarf.go +++ b/internal/gocore/dwarf.go
@@ -241,14 +241,28 @@ } } -func readRuntimeConstants(p *core.Process) (map[string]int64, error) { +type constsMap map[string]int64 + +func (c constsMap) get(s string) int64 { + v, ok := c[s] + if !ok { + panic("failed to find constant " + s) + } + return v +} + +func (c constsMap) find(s string) (int64, bool) { + v, ok := c[s] + return v, ok +} + +func readConstants(p *core.Process) (constsMap, error) { d, err := p.DWARF() if err != nil { return nil, fmt.Errorf("failed to read DWARF: %v", err) } consts := map[string]int64{} - // From 1.10, these constants are recorded in DWARF records. r := d.Reader() for e, err := r.Next(); e != nil && err == nil; e, err = r.Next() { if e.Tag != dwarf.TagConstant { @@ -259,10 +273,6 @@ continue } name := f.Val.(string) - if !strings.HasPrefix(name, "runtime.") { - continue - } - name = name[8:] c := e.AttrField(dwarf.AttrConstValue) if c == nil { continue
diff --git a/internal/gocore/module.go b/internal/gocore/module.go index f15b9e0..84994dc 100644 --- a/internal/gocore/module.go +++ b/internal/gocore/module.go
@@ -70,7 +70,7 @@ // readFunc parses a runtime._func and returns a *Func. // r must have type runtime._func. // pcln must have type []byte and represent the module's pcln table region. -func (m *module) readFunc(r region, pctab region, funcnametab region, rtConsts map[string]int64) *Func { +func (m *module) readFunc(r region, pctab region, funcnametab region, rtConsts constsMap) *Func { f := &Func{module: m, r: r} f.entry = m.textAddr(r.Field("entryOff").Uint32()) nameOff := r.Field("nameOff").Int32() @@ -103,7 +103,7 @@ } // Read pcln tables we need. - if stackmap := int(rtConsts["_PCDATA_StackMapIndex"]); stackmap < len(f.pcdata) { + if stackmap := int(rtConsts.get("internal/abi.PCDATA_StackMapIndex")); stackmap < len(f.pcdata) { f.stackMap.read(r.p, pctab.SliceIndex(int64(f.pcdata[stackmap])).a) } else { f.stackMap.setEmpty()
diff --git a/internal/gocore/process.go b/internal/gocore/process.go index fcabdc7..b00d04d 100644 --- a/internal/gocore/process.go +++ b/internal/gocore/process.go
@@ -36,7 +36,7 @@ // Runtime info for easier lookup. rtGlobals map[string]region - rtConsts map[string]int64 + rtConsts constsMap // A module is a loadable unit. Most Go programs have 1, programs // which load plugins will have more. @@ -98,7 +98,7 @@ } p.rtTypeByName[name] = t } - p.rtConsts, err = readRuntimeConstants(proc) + p.rtConsts, err = readConstants(proc) if err != nil { return nil, err } @@ -201,11 +201,11 @@ mheap := p.rtGlobals["mheap_"] var arenas []arena - arenaSize := p.rtConsts["heapArenaBytes"] + arenaSize := p.rtConsts.get("runtime.heapArenaBytes") if arenaSize%heapInfoSize != 0 { panic("arenaSize not a multiple of heapInfoSize") } - arenaBaseOffset := -p.rtConsts["arenaBaseOffsetUintptr"] + arenaBaseOffset := -p.rtConsts.get("runtime.arenaBaseOffsetUintptr") if p.proc.PtrSize() == 4 && arenaBaseOffset != 0 { panic("arenaBaseOffset must be 0 for 32-bit inferior") } @@ -312,17 +312,17 @@ return nil, nil, errors.New("weird mapping " + m.Perm().String()) } } - pageSize := p.rtConsts["_PageSize"] + pageSize := p.rtConsts.get("runtime._PageSize") // Span types. - spanInUse := uint8(p.rtConsts["mSpanInUse"]) - spanManual := uint8(p.rtConsts["mSpanManual"]) - spanDead := uint8(p.rtConsts["mSpanDead"]) + spanInUse := uint8(p.rtConsts.get("runtime.mSpanInUse")) + spanManual := uint8(p.rtConsts.get("runtime.mSpanManual")) + spanDead := uint8(p.rtConsts.get("runtime.mSpanDead")) // Malloc header constants (go 1.22+) - minSizeForMallocHeader := int64(p.rtConsts["minSizeForMallocHeader"]) - mallocHeaderSize := int64(p.rtConsts["mallocHeaderSize"]) - maxSmallSize := int64(p.rtConsts["maxSmallSize"]) + minSizeForMallocHeader := int64(p.rtConsts.get("runtime.minSizeForMallocHeader")) + mallocHeaderSize := int64(p.rtConsts.get("runtime.mallocHeaderSize")) + maxSmallSize := int64(p.rtConsts.get("runtime.maxSmallSize")) abiType := p.tryFindType("internal/abi.Type") @@ -402,7 +402,7 @@ // Process special records. for sp := s.Field("specials"); sp.Address() != 0; sp = sp.Field("next") { sp = sp.Deref() // *special to special - if sp.Field("kind").Uint8() != uint8(p.rtConsts["_KindSpecialFinalizer"]) { + if sp.Field("kind").Uint8() != uint8(p.rtConsts.get("runtime._KindSpecialFinalizer")) { // All other specials (just profile records) can't point into the heap. continue } @@ -466,7 +466,8 @@ } typ := region{p: p.proc, a: typeAddr, typ: abiType} nptrs := int64(typ.Field("PtrBytes").Uintptr()) / int64(heap.ptrSize) - if typ.Field("Kind_").Uint8()&uint8(p.rtConsts["kindGCProg"]) != 0 { + kindGCProg, hasGCProgs := p.rtConsts.find("internal/abi.KindGCProg") + if hasGCProgs && typ.Field("Kind_").Uint8()&uint8(kindGCProg) != 0 { panic("unexpected GC prog on small allocation") } gcdata := typ.Field("GCData").Address() @@ -485,7 +486,8 @@ // is in use. typ := s.Field("largeType").Deref() nptrs := int64(typ.Field("PtrBytes").Uintptr()) / int64(heap.ptrSize) - if typ.Field("Kind_").Uint8()&uint8(p.rtConsts["kindGCProg"]) != 0 { + kindGCProg, hasGCProgs := p.rtConsts.find("internal/abi.KindGCProg") + if hasGCProgs && typ.Field("Kind_").Uint8()&uint8(kindGCProg) != 0 { panic("large object's GCProg was not unrolled") } gcdata := typ.Field("GCData").Address() @@ -513,9 +515,9 @@ // Also keep track of how much has been scavenged. pages := mheap.Field("pages") chunks := pages.Field("chunks") - pallocChunkBytes := p.rtConsts["pallocChunkBytes"] - pallocChunksL1Bits := p.rtConsts["pallocChunksL1Bits"] - pallocChunksL2Bits := p.rtConsts["pallocChunksL2Bits"] + pallocChunkBytes := p.rtConsts.get("runtime.pallocChunkBytes") + pallocChunksL1Bits := p.rtConsts.get("runtime.pallocChunksL1Bits") + pallocChunksL2Bits := p.rtConsts.get("runtime.pallocChunksL2Bits") inuse := pages.Field("inUse") ranges := inuse.Field("ranges") for i := int64(0); i < ranges.SliceLen(); i++ { @@ -621,24 +623,24 @@ } st := r.Field("atomicstatus").Field("value") status := st.Uint32() - status &^= uint32(p.rtConsts["_Gscan"]) + status &^= uint32(p.rtConsts.get("runtime._Gscan")) var sp, pc core.Address switch status { - case uint32(p.rtConsts["_Gidle"]): + case uint32(p.rtConsts.get("runtime._Gidle")): return g, nil - case uint32(p.rtConsts["_Grunnable"]), uint32(p.rtConsts["_Gwaiting"]): + case uint32(p.rtConsts.get("runtime._Grunnable")), uint32(p.rtConsts.get("runtime._Gwaiting")): sched := r.Field("sched") sp = core.Address(sched.Field("sp").Uintptr()) pc = core.Address(sched.Field("pc").Uintptr()) - case uint32(p.rtConsts["_Grunning"]): + case uint32(p.rtConsts.get("runtime._Grunning")): sp = osT.SP() pc = osT.PC() // TODO: back up to the calling frame? - case uint32(p.rtConsts["_Gsyscall"]): + case uint32(p.rtConsts.get("runtime._Gsyscall")): sp = core.Address(r.Field("syscallsp").Uintptr()) pc = core.Address(r.Field("syscallpc").Uintptr()) // TODO: or should we use the osT registers? - case uint32(p.rtConsts["_Gdead"]): + case uint32(p.rtConsts.get("runtime._Gdead")): return nil, nil // TODO: copystack, others? default: @@ -874,6 +876,8 @@ } else { sp = f.max pc = core.Address(p.proc.ReadUintptr(sp - 8)) // TODO:amd64 only + + isCrashFrame = false } if pc == 0 { // TODO: when would this happen? @@ -905,7 +909,7 @@ // Find live ptrs in locals live := map[core.Address]bool{} - if x := int(p.rtConsts["_FUNCDATA_LocalsPointerMaps"]); x < len(f.funcdata) { + if x := int(p.rtConsts.get("internal/abi.FUNCDATA_LocalsPointerMaps")); x < len(f.funcdata) { addr := f.funcdata[x] // TODO: Ideally we should have the same frame size check as // runtime.getStackSize to detect errors when we are missing @@ -918,7 +922,10 @@ if err != nil { return nil, fmt.Errorf("cannot read stack map at pc=%#x: %v", pc, err) } - if idx < 0 { + if idx < -1 { + return nil, fmt.Errorf("cannot read stack map at pc=%#x: invalid stack map index %d", pc, idx) + } + if idx == -1 { idx = 0 } if idx < int64(n) { @@ -934,7 +941,7 @@ } } // Same for args - if x := int(p.rtConsts["_FUNCDATA_ArgsPointerMaps"]); x < len(f.funcdata) { + if x := int(p.rtConsts.get("internal/abi.FUNCDATA_ArgsPointerMaps")); x < len(f.funcdata) { addr := f.funcdata[x] if addr != 0 { args := region{p: p.proc, a: addr, typ: p.findType("runtime.stackmap")} @@ -944,11 +951,14 @@ if err != nil { return nil, fmt.Errorf("cannot read stack map at pc=%#x: %v", pc, err) } - if idx < 0 { + if idx < -1 { + return nil, fmt.Errorf("cannot read stack map at pc=%#x: invalid stack map index %d", pc, idx) + } + if idx == -1 { idx = 0 } if idx < int64(n) { - bits := args.Field("bytedata").a.Add(int64(nbit+7) / 8 * idx) + bits := args.Field("bytedata").a.Add((int64(nbit+7) / 8) * idx) base := frame.max // TODO: add to base for LR archs. for i := int64(0); i < int64(nbit); i++ {
diff --git a/internal/gocore/type.go b/internal/gocore/type.go index c983e64..7f66383 100644 --- a/internal/gocore/type.go +++ b/internal/gocore/type.go
@@ -224,7 +224,7 @@ b := make([]byte, n) p.proc.ReadAt(b, x.Add(i+1)) name = string(b) - if r.TFlag()&uint8(p.rtConsts["tflagExtraStar"]) != 0 { + if r.TFlag()&uint8(p.rtConsts.get("internal/abi.TFlagExtraStar")) != 0 { name = name[1:] } } else { @@ -238,15 +238,16 @@ ptrSize := p.proc.PtrSize() nptrs := int64(r.PtrBytes()) / ptrSize var ptrs []int64 - if r.Kind_()&uint8(p.rtConsts["kindGCProg"]) == 0 { + kindGCProg, hasGCProgs := p.rtConsts.find("internal/abi.KindGCProg") + if hasGCProgs && r.Kind_()&uint8(kindGCProg) != 0 { + // TODO: run GC program. Go 1.23 and earlier only. + } else { gcdata := r.GCData() for i := int64(0); i < nptrs; i++ { if p.proc.ReadUint8(gcdata.Add(i/8))>>uint(i%8)&1 != 0 { ptrs = append(ptrs, i*ptrSize) } } - } else { - // TODO: run GC program to get ptr indexes } // Find a Type that matches this type. @@ -540,7 +541,7 @@ p.typeObject(r.Addr, r.Type, fr, add) } else if r.Addr == 0 && r.Type.Kind == KindPtr && r.Type.Elem != nil { p.typeObject(r.RegValue, r.Type.Elem, fr, add) - } else { + } else if r.Addr != 0 { p.typeObject(r.Addr, r.Type, p.proc, add) } return true @@ -631,7 +632,7 @@ // ifaceIndir reports whether t is stored indirectly in an interface value. func ifaceIndir(t core.Address, p *Process) bool { typr := p.findRuntimeType(t) - return typr.Kind_()&uint8(p.rtConsts["kindDirectIface"]) == 0 + return typr.Kind_()&uint8(p.rtConsts.get("internal/abi.KindDirectIface")) == 0 } // typeObject takes an address and a type for the data at that address.