Skip to content

Commit 4bf107f

Browse files
derekparkerclaude
andcommitted
proc: address PR review feedback for non-Go binary debugging
- Deduplicate auxv parsing by extracting searchAuxv helper - Rename IsNonGo to IsGo (positive flag is easier to reason about) - Move Go image detection into loadDebugInfoMaps to avoid reparsing debug_info; uses DW_AT_language == DW_LANG_Go which is already checked there for cu.isgo - Remove hasGoProducer function (no longer needed) - Remove pkg/proc import from pkg/terminal/command.go; use string literal for stop reason comparison instead - Update CLAUDE.md with DWARF parsing and layer boundary guidance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8741a62 commit 4bf107f

8 files changed

Lines changed: 33 additions & 70 deletions

File tree

CLAUDE.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,10 @@ Fixes #1234
282282
4. **Memory safety** - Always handle errors when reading process memory.
283283
Process may die, memory may be unmapped, or addresses invalid.
284284

285+
5. **Don't reparse DWARF sections** - `loadDebugInfoMaps` already
286+
iterates all compile units. Add per-CU checks there instead of
287+
making separate passes over `debug_info`.
288+
285289
## File Organization
286290

287291
```

pkg/proc/bininfo.go

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,9 +1023,9 @@ type Image struct {
10231023
symTable *gosym.Table
10241024
Trimpath bool // trimpath used
10251025

1026-
// IsNonGo is true when the image has been confirmed to not be a Go binary.
1027-
// This is set when both DWARF loading and Go runtime symbol table loading fail.
1028-
IsNonGo bool
1026+
// IsGo is true when the image has been confirmed to be a Go binary,
1027+
// e.g. by checking for a Go producer string in DWARF compile units.
1028+
IsGo bool
10291029

10301030
typeCache map[dwarf.Offset]godwarf.Type
10311031

@@ -1060,7 +1060,7 @@ func (image *Image) Stripped() bool {
10601060
// successfully loaded debug info.
10611061
func (bi *BinaryInfo) HasGoImage() bool {
10621062
for _, image := range bi.Images {
1063-
if !image.IsNonGo && image.loadErr == nil {
1063+
if image.IsGo && image.loadErr == nil {
10641064
return true
10651065
}
10661066
}
@@ -1446,27 +1446,6 @@ func (bi *Image) findCompileUnitForOffset(off dwarf.Offset) *compileUnit {
14461446
return bi.compileUnits[i]
14471447
}
14481448

1449-
// hasGoProducer reports whether d contains any compilation unit with a Go
1450-
// producer string (DW_AT_producer starting with "Go "). This is used during
1451-
// image loading to distinguish Go binaries from non-Go binaries that happen
1452-
// to have DWARF debug info.
1453-
func hasGoProducer(d *dwarf.Data) bool {
1454-
reader := d.Reader()
1455-
for {
1456-
entry, err := reader.Next()
1457-
if err != nil || entry == nil {
1458-
return false
1459-
}
1460-
if entry.Tag == dwarf.TagCompileUnit {
1461-
producer, _ := entry.Val(dwarf.AttrProducer).(string)
1462-
if strings.HasPrefix(producer, "Go ") {
1463-
return true
1464-
}
1465-
reader.SkipChildren()
1466-
}
1467-
}
1468-
}
1469-
14701449
// Producer returns the value of DW_AT_producer.
14711450
func (bi *BinaryInfo) Producer() string {
14721451
for _, cu := range bi.Images[0].compileUnits {
@@ -1737,9 +1716,9 @@ func loadBinaryInfoElf(bi *BinaryInfo, image *Image, path string, addr uint64, w
17371716
}
17381717
err := loadBinaryInfoGoRuntimeElf(bi, image, path, elfFile)
17391718
if err != nil {
1740-
image.IsNonGo = true
17411719
return fmt.Errorf("could not read debug info (%v) and could not read go symbol table (%v)", dwerr, err)
17421720
}
1721+
image.IsGo = true
17431722
return nil
17441723
}
17451724
image.sepDebugCloser = sepFile
@@ -1749,14 +1728,6 @@ func loadBinaryInfoElf(bi *BinaryInfo, image *Image, path string, addr uint64, w
17491728
}
17501729
}
17511730

1752-
// Mark images that have DWARF but no Go producer string as non-Go.
1753-
// Without this check, shared libraries that have DWARF debug info would
1754-
// be incorrectly treated as Go images by HasGoImage(), since IsNonGo is
1755-
// only set when both DWARF and Go symbol table loading fail.
1756-
if !hasGoProducer(image.dwarf) {
1757-
image.IsNonGo = true
1758-
}
1759-
17601731
debugInfoBytes, err = godwarf.GetDebugSectionElf(dwarfFile, "info")
17611732
if err != nil {
17621733
return err
@@ -2650,6 +2621,7 @@ func (bi *BinaryInfo) loadDebugInfoMaps(image *Image, debugInfoBytes, debugLineB
26502621
cu.Version = ctxt.offsetToVersion[cu.offset]
26512622
if lang, _ := entry.Val(dwarf.AttrLanguage).(int64); lang == dwarfGoLanguage {
26522623
cu.isgo = true
2624+
image.IsGo = true
26532625
}
26542626
cu.name, _ = entry.Val(dwarf.AttrName).(string)
26552627
compdir, _ := entry.Val(dwarf.AttrCompDir).(string)

pkg/proc/linutil/auxv.go

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,38 +11,14 @@ const (
1111
_AT_ENTRY = 9
1212
)
1313

14-
// BaseFromAuxv searches the elf auxiliary vector for the AT_BASE value,
15-
// which is the base address of the dynamic linker (interpreter).
16-
func BaseFromAuxv(auxv []byte, ptrSize int) uint64 {
17-
rd := bytes.NewBuffer(auxv)
18-
19-
for {
20-
tag, err := readUintRaw(rd, binary.LittleEndian, ptrSize)
21-
if err != nil {
22-
return 0
23-
}
24-
val, err := readUintRaw(rd, binary.LittleEndian, ptrSize)
25-
if err != nil {
26-
return 0
27-
}
28-
29-
switch tag {
30-
case _AT_NULL:
31-
return 0
32-
case _AT_BASE:
33-
return val
34-
}
35-
}
36-
}
37-
38-
// EntryPointFromAuxv searches the elf auxiliary vector for the entry point
39-
// address.
14+
// searchAuxv searches the elf auxiliary vector for the given tag and returns
15+
// its value. Returns 0 if the tag is not found.
4016
// For a description of the auxiliary vector (auxv) format see:
4117
// System V Application Binary Interface, AMD64 Architecture Processor
4218
// Supplement, section 3.4.3.
4319
// System V Application Binary Interface, Intel386 Architecture Processor
4420
// Supplement (fourth edition), section 3-28.
45-
func EntryPointFromAuxv(auxv []byte, ptrSize int) uint64 {
21+
func searchAuxv(auxv []byte, ptrSize int, target uint64) uint64 {
4622
rd := bytes.NewBuffer(auxv)
4723

4824
for {
@@ -58,8 +34,20 @@ func EntryPointFromAuxv(auxv []byte, ptrSize int) uint64 {
5834
switch tag {
5935
case _AT_NULL:
6036
return 0
61-
case _AT_ENTRY:
37+
case target:
6238
return val
6339
}
6440
}
6541
}
42+
43+
// BaseFromAuxv searches the elf auxiliary vector for the AT_BASE value,
44+
// which is the base address of the dynamic linker (interpreter).
45+
func BaseFromAuxv(auxv []byte, ptrSize int) uint64 {
46+
return searchAuxv(auxv, ptrSize, _AT_BASE)
47+
}
48+
49+
// EntryPointFromAuxv searches the elf auxiliary vector for the entry point
50+
// address.
51+
func EntryPointFromAuxv(auxv []byte, ptrSize int) uint64 {
52+
return searchAuxv(auxv, ptrSize, _AT_ENTRY)
53+
}

pkg/proc/native/proc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ func (dbp *nativeProcess) initialize(path string, debugInfoDirs []string) (*proc
388388
return nil, err
389389
}
390390
if dbp.bi.Arch.Name == "arm64" || dbp.bi.Arch.Name == "ppc64le" || dbp.bi.Arch.Name == "riscv64" || dbp.bi.Arch.Name == "loong64" {
391-
if len(dbp.bi.Images) > 0 && !dbp.bi.Images[0].IsNonGo {
391+
if len(dbp.bi.Images) > 0 && dbp.bi.Images[0].IsGo {
392392
dbp.iscgo = tgt.IsCgo()
393393
}
394394
}

pkg/proc/native/proc_linux.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ func setupSharedLibBreakpoint(dbp *nativeProcess, grp *proc.TargetGroup) {
187187
return
188188
}
189189
bi := grp.Selected.BinInfo()
190-
if len(bi.Images) == 0 || !bi.Images[0].IsNonGo {
190+
if len(bi.Images) == 0 || bi.Images[0].IsGo {
191191
return
192192
}
193193
rBrkAddr, err := linutil.ElfFindRBrk(grp.Selected)

pkg/proc/proc_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6090,8 +6090,8 @@ func TestNonGoBinaryWithGoDlopen(t *testing.T) {
60906090
if len(p.BinInfo().Images) == 0 {
60916091
t.Fatal("no images loaded")
60926092
}
6093-
if !p.BinInfo().Images[0].IsNonGo {
6094-
t.Error("expected Images[0].IsNonGo to be true")
6093+
if p.BinInfo().Images[0].IsGo {
6094+
t.Error("expected Images[0].IsGo to be false")
60956095
}
60966096
if p.BinInfo().HasGoImage() {
60976097
t.Error("expected HasGoImage to be false before dlopen")

pkg/proc/target.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,14 @@ func (grp *TargetGroup) newTarget(p ProcessInternal, pid int, currentThread Thre
183183
if err != nil {
184184
// If this is a non-Go binary, log a warning and continue.
185185
// A Go shared object may be loaded later via dlopen.
186-
if len(p.BinInfo().Images) > 0 && p.BinInfo().Images[0].IsNonGo {
186+
if len(p.BinInfo().Images) > 0 && !p.BinInfo().Images[0].IsGo {
187187
logflags.DebuggerLogger().Warnf("Initial binary is not a Go binary: %v", err)
188188
} else {
189189
return nil, &ErrBadBinaryInfo{Err: err}
190190
}
191191
}
192192
for _, image := range p.BinInfo().Images {
193-
if image.loadErr != nil && !image.IsNonGo {
193+
if image.loadErr != nil && image.IsGo {
194194
return nil, &ErrBadBinaryInfo{Err: image.loadErr}
195195
}
196196
}

pkg/terminal/command.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
"github.com/cosiner/argv"
3131
"github.com/go-delve/delve/pkg/config"
3232
"github.com/go-delve/delve/pkg/locspec"
33-
"github.com/go-delve/delve/pkg/proc"
3433
"github.com/go-delve/delve/pkg/proc/debuginfod"
3534
"github.com/go-delve/delve/service"
3635
"github.com/go-delve/delve/service/api"
@@ -2798,7 +2797,7 @@ func printcontext(t *Term, state *api.DebuggerState) {
27982797
return
27992798
}
28002799

2801-
if state.StopReason == proc.StopSharedLibLoaded.String() {
2800+
if state.StopReason == "shared library loaded" {
28022801
fmt.Fprintln(t.stdout, "Go shared library loaded. You can now set breakpoints in Go code.")
28032802
}
28042803

0 commit comments

Comments
 (0)