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) + } +}