symtab/elf: bound symbols by st_size to avoid misattributing stripped code

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

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
 }

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