From 8b34a069fd2743554794ce18b20feab189a42f21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=A1l=20Gy=C3=B6rgy?= Date: Mon, 6 Jul 2026 22:42:01 +0200 Subject: [PATCH 1/2] symtab/elf: bound symbols by st_size to avoid misattributing stripped code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SymbolTable.Resolve mapped a PC to the symbol with the greatest value <= pc with no upper bound. In a stripped binary that keeps only a handful of exported .dynsym symbols (e.g. an Envoy sidecar that exports symbols for dlopen'd Go/Lua modules while .symtab is stripped), the nearest surviving symbol can sit arbitrarily far below the PC. As a result, samples that actually land in stripped-out functions were attributed to an unrelated exported symbol, and removing that symbol just shifted the samples to the next surviving one. st_size was already parsed out of each Elf symbol but discarded. Keep it and bound each symbol to [Value, Value+Size): when the size is known and the PC falls past the symbol's end, resolve to "" (unknown) instead of the nearest symbol. Symbols with size 0 (unknown) keep the previous nearest-symbol behavior, so nothing regresses for binaries that omit sizes. Also populate Value/Size in the 32-bit symbol path, which previously left the symbol value unset. Signed-off-by: Gaál György --- ebpf/symtab/elf/elf_sym.go | 21 +++++- ebpf/symtab/elf/symbol_table.go | 22 ++++++ ebpf/symtab/elf/symbol_table_resolve_test.go | 73 ++++++++++++++++++++ 3 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 ebpf/symtab/elf/symbol_table_resolve_test.go diff --git a/ebpf/symtab/elf/elf_sym.go b/ebpf/symtab/elf/elf_sym.go index 81a604a439..ef950b50bf 100644 --- a/ebpf/symtab/elf/elf_sym.go +++ b/ebpf/symtab/elf/elf_sym.go @@ -51,6 +51,18 @@ func (f *InMemElfFile) getSymbols(typ elf.SectionType, opt *SymbolsOptions) ([]S // if there is no such section in the File. var ErrNoSymbols = errors.New("no symbol section") +// clampSymSize narrows a 64-bit st_size to the uint32 stored per symbol. +// Function sizes never approach 4 GiB in practice; clamping keeps the value +// nonzero (so the [Value, Value+Size) bound still applies) while avoiding +// overflow wraparound. +func clampSymSize(size uint64) uint32 { + const maxU32 = uint64(^uint32(0)) + if size > maxU32 { + return uint32(maxU32) + } + return uint32(size) +} + func (f *InMemElfFile) getSymbols64(typ elf.SectionType, opt *SymbolsOptions) ([]SymbolIndex, uint32, error) { symtabSection := f.sectionByType(typ) if symtabSection == nil { @@ -88,7 +100,7 @@ func (f *InMemElfFile) getSymbols64(typ elf.SectionType, opt *SymbolsOptions) ([ //Other: rawSym[5], //Shndx: f.ByteOrder.Uint16(rawSym[6:8]), // not used Value: f.ByteOrder.Uint64(rawSym[8:16]), - //Size: f.ByteOrder.Uint64(rawSym[16:24]), // not used + Size: f.ByteOrder.Uint64(rawSym[16:24]), } if sym.Value != 0 && sym.Info&0xf == byte(elf.STT_FUNC) { @@ -101,6 +113,7 @@ func (f *InMemElfFile) getSymbols64(typ elf.SectionType, opt *SymbolsOptions) ([ } symbols[i].Value = pc symbols[i].Name = NewName(sym.Name, linkIndex) + symbols[i].Size = clampSymSize(sym.Size) i++ } } @@ -142,8 +155,8 @@ func (f *InMemElfFile) getSymbols32(typ elf.SectionType, opt *SymbolsOptions) ([ sym = elf.Sym32{ Name: f.ByteOrder.Uint32(rawSym[:4]), Value: f.ByteOrder.Uint32(rawSym[4:8]), - //Size: f.ByteOrder.Uint32(rawSym[8:12]), - Info: rawSym[12], + Size: f.ByteOrder.Uint32(rawSym[8:12]), + Info: rawSym[12], //Other: rawSym[13], //Shndx: f.ByteOrder.Uint16(rawSym[14:16]), } @@ -156,7 +169,9 @@ func (f *InMemElfFile) getSymbols32(typ elf.SectionType, opt *SymbolsOptions) ([ if pc >= opt.FilterFrom && pc < opt.FilterTo { continue } + symbols[i].Value = pc symbols[i].Name = NewName(sym.Name, linkIndex) + symbols[i].Size = sym.Size i++ } } diff --git a/ebpf/symtab/elf/symbol_table.go b/ebpf/symtab/elf/symbol_table.go index 826446fd05..a0a0225b95 100644 --- a/ebpf/symtab/elf/symbol_table.go +++ b/ebpf/symtab/elf/symbol_table.go @@ -27,6 +27,11 @@ type SymbolTableInterface interface { type SymbolIndex struct { Name Name Value uint64 + // Size is the symbol's st_size (function length in bytes), or 0 when the + // binary does not record it. It bounds a symbol to [Value, Value+Size) so + // that a PC in a gap between symbols is not misattributed to the nearest + // preceding symbol. See SymbolTable.Resolve. + Size uint32 } type SectionLinkIndex uint8 @@ -52,6 +57,10 @@ type FlatSymbolIndex struct { Links []elf.SectionHeader Names []Name Values gosym.PCIndex + // Sizes is parallel to Names/Values: Sizes[i] is the st_size of symbol i, + // or 0 when unknown. Used by Resolve to reject PCs that fall past a + // symbol's end. + Sizes []uint32 } type SymbolTable struct { Index FlatSymbolIndex @@ -104,6 +113,17 @@ func (st *SymbolTable) Resolve(addr uint64) string { if i == -1 { return "" } + // FindIndex returns the symbol with the greatest Value <= addr, without an + // upper bound. In a stripped binary that keeps only a few .dynsym entries + // (e.g. an Envoy sidecar that exports symbols for dlopen'd modules), that + // nearest symbol can be arbitrarily far below addr, so a PC inside a + // stripped-out function gets attributed to an unrelated exported symbol. + // When the symbol's size is known, reject addresses beyond its end so such + // PCs resolve to "" (unknown) instead. Size 0 means unknown -> keep the + // legacy nearest-symbol behavior. + if size := st.Index.Sizes[i]; size > 0 && addr >= st.Index.Values.Value(i)+uint64(size) { + return "" + } name, _ := st.symbolName(i) return name } @@ -144,6 +164,7 @@ func (f *InMemElfFile) NewSymbolTable(opt *SymbolsOptions, symReader ElfSymbolRe }, Names: make([]Name, total), Values: gosym.NewPCIndex(total), + Sizes: make([]uint32, total), }, hasSection: map[elf.SectionType]bool{ elf.SHT_SYMTAB: len(sym) > 0, @@ -156,6 +177,7 @@ func (f *InMemElfFile) NewSymbolTable(opt *SymbolsOptions, symReader ElfSymbolRe for i := range all { res.Index.Names[i] = all[i].Name res.Index.Values.Set(i, all[i].Value) + res.Index.Sizes[i] = all[i].Size } return res, nil } diff --git a/ebpf/symtab/elf/symbol_table_resolve_test.go b/ebpf/symtab/elf/symbol_table_resolve_test.go new file mode 100644 index 0000000000..2dcbfb40b1 --- /dev/null +++ b/ebpf/symtab/elf/symbol_table_resolve_test.go @@ -0,0 +1,73 @@ +package elf + +import ( + "debug/elf" + "testing" + + "github.com/grafana/pyroscope/ebpf/symtab/gosym" + "github.com/ianlancetaylor/demangle" + "github.com/stretchr/testify/require" +) + +// stubStrings is a minimal ElfSymbolReader that maps a string-table offset to a +// symbol name, so SymbolTable.Resolve can be exercised without a real ELF file. +type stubStrings map[int]string + +func (s stubStrings) getString(start int, _ []demangle.Option) (string, bool) { + n, ok := s[start] + return n, ok +} + +// TestSymbolTableResolveBounds reproduces coroot/coroot-node-agent#262: a +// stripped binary (e.g. an Envoy sidecar) keeps only a handful of exported +// .dynsym symbols. Before this fix, a PC inside a stripped-out function was +// attributed to the nearest surviving symbol below it. Now, when the symbol's +// size is known, such PCs resolve to "" (unknown) instead. +func TestSymbolTableResolveBounds(t *testing.T) { + type sym struct { + value uint64 + size uint32 + name string + } + input := []sym{ + {0x1000, 0x20, "exported_low"}, // like envoyGoFilterLogLevel + {0x2000, 0x20, "exported_high"}, // like luaopen_jit + {0x3000, 0, "asm_no_size"}, // st_size unknown -> legacy behavior + } + + strings := stubStrings{} + st := &SymbolTable{Index: FlatSymbolIndex{ + Links: []elf.SectionHeader{{Offset: 0}, {Offset: 0}}, + Names: make([]Name, len(input)), + Values: gosym.NewPCIndex(len(input)), + Sizes: make([]uint32, len(input)), + }} + for i, s := range input { + nameIdx := uint32(i + 1) + st.Index.Names[i] = NewName(nameIdx, sectionTypeDynSym) + st.Index.Values.Set(i, s.value) + st.Index.Sizes[i] = s.size + strings[int(nameIdx)] = s.name + } + st.SymReader = strings + + cases := []struct { + pc uint64 + want string + desc string + }{ + {0x0fff, "", "below the first symbol"}, + {0x1000, "exported_low", "start of exported_low"}, + {0x101f, "exported_low", "last byte of exported_low"}, + {0x1020, "", "one past exported_low end (previously misattributed)"}, + {0x1800, "", "gap between the two exported symbols"}, + {0x1fff, "", "stripped-out code just below exported_high"}, + {0x2000, "exported_high", "start of exported_high"}, + {0x2020, "", "past exported_high end"}, + {0x3000, "asm_no_size", "size-0 symbol resolves at its start"}, + {0x4000, "asm_no_size", "size-0 symbol keeps legacy nearest-symbol behavior"}, + } + for _, c := range cases { + require.Equalf(t, c.want, st.Resolve(c.pc), "pc=0x%x (%s)", c.pc, c.desc) + } +} From 419b46a7472e11dff1e0ebc50317be4c7b1023bc Mon Sep 17 00:00:00 2001 From: Nikolay Sivko Date: Tue, 4 Aug 2026 17:43:29 -0300 Subject: [PATCH 2/2] symtab/elf: use end-of-symbol sentinels instead of a per-symbol Sizes array --- ebpf/symtab/elf/elf_sym.go | 4 -- ebpf/symtab/elf/symbol_table.go | 48 +++++++------ ebpf/symtab/elf/symbol_table_resolve_test.go | 73 -------------------- 3 files changed, 26 insertions(+), 99 deletions(-) delete mode 100644 ebpf/symtab/elf/symbol_table_resolve_test.go diff --git a/ebpf/symtab/elf/elf_sym.go b/ebpf/symtab/elf/elf_sym.go index ef950b50bf..e549d84812 100644 --- a/ebpf/symtab/elf/elf_sym.go +++ b/ebpf/symtab/elf/elf_sym.go @@ -51,10 +51,6 @@ func (f *InMemElfFile) getSymbols(typ elf.SectionType, opt *SymbolsOptions) ([]S // if there is no such section in the File. var ErrNoSymbols = errors.New("no symbol section") -// clampSymSize narrows a 64-bit st_size to the uint32 stored per symbol. -// Function sizes never approach 4 GiB in practice; clamping keeps the value -// nonzero (so the [Value, Value+Size) bound still applies) while avoiding -// overflow wraparound. func clampSymSize(size uint64) uint32 { const maxU32 = uint64(^uint32(0)) if size > maxU32 { diff --git a/ebpf/symtab/elf/symbol_table.go b/ebpf/symtab/elf/symbol_table.go index a0a0225b95..4f3db4b9dc 100644 --- a/ebpf/symtab/elf/symbol_table.go +++ b/ebpf/symtab/elf/symbol_table.go @@ -25,12 +25,9 @@ type SymbolTableInterface interface { } type SymbolIndex struct { - Name Name Value uint64 - // Size is the symbol's st_size (function length in bytes), or 0 when the - // binary does not record it. It bounds a symbol to [Value, Value+Size) so - // that a PC in a gap between symbols is not misattributed to the nearest - // preceding symbol. See SymbolTable.Resolve. + Name Name + // st_size, or 0 when unknown; build-time only (see addEndOfSymbolSentinels) Size uint32 } @@ -53,14 +50,30 @@ func (n *Name) LinkIndex() SectionLinkIndex { return SectionLinkIndex(*n >> 31) } +const sentinelNameIndex = 0x7fffffff + +const minGapSentinel = 64 + +func addEndOfSymbolSentinels(all []SymbolIndex) []SymbolIndex { + out := make([]SymbolIndex, 0, len(all)+len(all)/64+1) + for i := range all { + out = append(out, all[i]) + if all[i].Size == 0 { + continue + } + end := all[i].Value + uint64(all[i].Size) + if i+1 < len(all) && all[i+1].Value < end+minGapSentinel { + continue + } + out = append(out, SymbolIndex{Name: NewName(sentinelNameIndex, sectionTypeSym), Value: end}) + } + return out +} + type FlatSymbolIndex struct { Links []elf.SectionHeader Names []Name Values gosym.PCIndex - // Sizes is parallel to Names/Values: Sizes[i] is the st_size of symbol i, - // or 0 when unknown. Used by Resolve to reject PCs that fall past a - // symbol's end. - Sizes []uint32 } type SymbolTable struct { Index FlatSymbolIndex @@ -113,15 +126,7 @@ func (st *SymbolTable) Resolve(addr uint64) string { if i == -1 { return "" } - // FindIndex returns the symbol with the greatest Value <= addr, without an - // upper bound. In a stripped binary that keeps only a few .dynsym entries - // (e.g. an Envoy sidecar that exports symbols for dlopen'd modules), that - // nearest symbol can be arbitrarily far below addr, so a PC inside a - // stripped-out function gets attributed to an unrelated exported symbol. - // When the symbol's size is known, reject addresses beyond its end so such - // PCs resolve to "" (unknown) instead. Size 0 means unknown -> keep the - // legacy nearest-symbol behavior. - if size := st.Index.Sizes[i]; size > 0 && addr >= st.Index.Values.Value(i)+uint64(size) { + if st.Index.Names[i].NameIndex() == sentinelNameIndex { return "" } name, _ := st.symbolName(i) @@ -156,15 +161,15 @@ func (f *InMemElfFile) NewSymbolTable(opt *SymbolsOptions, symReader ElfSymbolRe } return all[i].Value < all[j].Value }) + all = addEndOfSymbolSentinels(all) res := &SymbolTable{Index: FlatSymbolIndex{ Links: []elf.SectionHeader{ f.Sections[sectionSym], // should be at 0 - SectionTypeSym f.Sections[sectionDynSym], // should be at 1 - SectionTypeDynSym }, - Names: make([]Name, total), - Values: gosym.NewPCIndex(total), - Sizes: make([]uint32, total), + Names: make([]Name, len(all)), + Values: gosym.NewPCIndex(len(all)), }, hasSection: map[elf.SectionType]bool{ elf.SHT_SYMTAB: len(sym) > 0, @@ -177,7 +182,6 @@ func (f *InMemElfFile) NewSymbolTable(opt *SymbolsOptions, symReader ElfSymbolRe for i := range all { res.Index.Names[i] = all[i].Name res.Index.Values.Set(i, all[i].Value) - res.Index.Sizes[i] = all[i].Size } return res, nil } diff --git a/ebpf/symtab/elf/symbol_table_resolve_test.go b/ebpf/symtab/elf/symbol_table_resolve_test.go deleted file mode 100644 index 2dcbfb40b1..0000000000 --- a/ebpf/symtab/elf/symbol_table_resolve_test.go +++ /dev/null @@ -1,73 +0,0 @@ -package elf - -import ( - "debug/elf" - "testing" - - "github.com/grafana/pyroscope/ebpf/symtab/gosym" - "github.com/ianlancetaylor/demangle" - "github.com/stretchr/testify/require" -) - -// stubStrings is a minimal ElfSymbolReader that maps a string-table offset to a -// symbol name, so SymbolTable.Resolve can be exercised without a real ELF file. -type stubStrings map[int]string - -func (s stubStrings) getString(start int, _ []demangle.Option) (string, bool) { - n, ok := s[start] - return n, ok -} - -// TestSymbolTableResolveBounds reproduces coroot/coroot-node-agent#262: a -// stripped binary (e.g. an Envoy sidecar) keeps only a handful of exported -// .dynsym symbols. Before this fix, a PC inside a stripped-out function was -// attributed to the nearest surviving symbol below it. Now, when the symbol's -// size is known, such PCs resolve to "" (unknown) instead. -func TestSymbolTableResolveBounds(t *testing.T) { - type sym struct { - value uint64 - size uint32 - name string - } - input := []sym{ - {0x1000, 0x20, "exported_low"}, // like envoyGoFilterLogLevel - {0x2000, 0x20, "exported_high"}, // like luaopen_jit - {0x3000, 0, "asm_no_size"}, // st_size unknown -> legacy behavior - } - - strings := stubStrings{} - st := &SymbolTable{Index: FlatSymbolIndex{ - Links: []elf.SectionHeader{{Offset: 0}, {Offset: 0}}, - Names: make([]Name, len(input)), - Values: gosym.NewPCIndex(len(input)), - Sizes: make([]uint32, len(input)), - }} - for i, s := range input { - nameIdx := uint32(i + 1) - st.Index.Names[i] = NewName(nameIdx, sectionTypeDynSym) - st.Index.Values.Set(i, s.value) - st.Index.Sizes[i] = s.size - strings[int(nameIdx)] = s.name - } - st.SymReader = strings - - cases := []struct { - pc uint64 - want string - desc string - }{ - {0x0fff, "", "below the first symbol"}, - {0x1000, "exported_low", "start of exported_low"}, - {0x101f, "exported_low", "last byte of exported_low"}, - {0x1020, "", "one past exported_low end (previously misattributed)"}, - {0x1800, "", "gap between the two exported symbols"}, - {0x1fff, "", "stripped-out code just below exported_high"}, - {0x2000, "exported_high", "start of exported_high"}, - {0x2020, "", "past exported_high end"}, - {0x3000, "asm_no_size", "size-0 symbol resolves at its start"}, - {0x4000, "asm_no_size", "size-0 symbol keeps legacy nearest-symbol behavior"}, - } - for _, c := range cases { - require.Equalf(t, c.want, st.Resolve(c.pc), "pc=0x%x (%s)", c.pc, c.desc) - } -}