From 99f7b97313ffccff9c6e9ce1858eeac2bee53e5f Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Mon, 16 Mar 2026 16:23:27 +0000 Subject: [PATCH 01/15] feat: add iterateMappings api --- process/coredump.go | 9 ++++ process/process.go | 89 +++++++++++++++++++++++------------ process/types.go | 5 ++ processmanager/processinfo.go | 17 ++++++- 4 files changed, 88 insertions(+), 32 deletions(-) diff --git a/process/coredump.go b/process/coredump.go index 385ce52d2..29d42f35b 100644 --- a/process/coredump.go +++ b/process/coredump.go @@ -274,6 +274,15 @@ func (cd *CoredumpProcess) GetMappings() ([]Mapping, uint32, error) { return cd.mappings, 0, nil } +func (cd *CoredumpProcess) IterateMappings(fn func(m Mapping) bool) (uint32, error) { + for _, m := range cd.mappings { + if !fn(m) { + break + } + } + return 0, nil +} + // GetThreadInfo implements the Process interface. func (cd *CoredumpProcess) GetThreads() ([]ThreadInfo, error) { return cd.threadInfo, nil diff --git a/process/process.go b/process/process.go index 6901a95e3..324b71780 100644 --- a/process/process.go +++ b/process/process.go @@ -189,13 +189,12 @@ func trimMappingPath(path string) string { return path } -func parseMappings(mapsFile io.Reader) ([]Mapping, uint32, error) { +func iterateMappings(mapsFile io.Reader, fn func(m Mapping) bool) (uint32, error) { numParseErrors := uint32(0) - mappings := make([]Mapping, 0, 32) scanner := bufio.NewScanner(mapsFile) scanBuf := bufPool.Get().(*[]byte) if scanBuf == nil { - return mappings, 0, errors.New("failed to get memory from sync pool") + return 0, errors.New("failed to get memory from sync pool") } defer func() { // Reset memory and return it for reuse. @@ -295,7 +294,6 @@ func parseMappings(mapsFile io.Reader) ([]Mapping, uint32, error) { numParseErrors++ continue } - length := vend - vaddr fileOffset, err := strconv.ParseUint(fields[2], 16, 64) if err != nil { @@ -304,37 +302,54 @@ func parseMappings(mapsFile io.Reader) ([]Mapping, uint32, error) { continue } - mappings = append(mappings, Mapping{ + if !fn(Mapping{ Vaddr: vaddr, - Length: length, + Length: vend - vaddr, Flags: flags, FileOffset: fileOffset, Device: device, Inode: inode, Path: path, - }) + }) { + return numParseErrors, scanner.Err() + } } - return mappings, numParseErrors, scanner.Err() + return numParseErrors, scanner.Err() } -// GetMappings will process the mappings file from proc. Additionally, -// a reverse map from mapping filename to a Mapping node is built to allow -// OpenELF opening ELF files using the corresponding proc map_files entry. -// WARNING: This implementation does not support calling GetMappings -// concurrently with itself, or with OpenELF. -func (sp *systemProcess) GetMappings() ([]Mapping, uint32, error) { +func parseMappings(mapsFile io.Reader) ([]Mapping, uint32, error) { + mappings := make([]Mapping, 0, 32) + numParseErrors, err := iterateMappings(mapsFile, func(m Mapping) bool { + mappings = append(mappings, m) + return true + }) + + return mappings, numParseErrors, err +} + +func (sp *systemProcess) IterateMappings(fn func(m Mapping) bool) (uint32, error) { mapsFile, err := os.Open(fmt.Sprintf("/proc/%d/maps", sp.pid)) if err != nil { - return nil, 0, err + return 0, err } defer mapsFile.Close() - mappings, numParseErrors, err := parseMappings(mapsFile) + var namedMappings []Mapping + gotMappings := false + wrappedFn := func(m Mapping) bool { + gotMappings = true + if m.Path != libpf.NullString { + namedMappings = append(namedMappings, m) + } + return fn(m) + } + + numParseErrors, err := iterateMappings(mapsFile, wrappedFn) if err != nil { - return mappings, numParseErrors, err + return numParseErrors, err } - if len(mappings) == 0 { + if !gotMappings { // We could test for main thread exit here by checking for zombie state // in /proc/sp.pid/stat but it's simpler to assume that this is the case // and try extracting mappings for a different thread. Since we stopped @@ -344,7 +359,7 @@ func (sp *systemProcess) GetMappings() ([]Mapping, uint32, error) { sp.mainThreadExit = true if sp.pid == sp.tid { - return mappings, numParseErrors, ErrNoMappings + return numParseErrors, ErrNoMappings } log.Debugf("TID: %v extracting mappings", sp.tid) @@ -356,24 +371,38 @@ func (sp *systemProcess) GetMappings() ([]Mapping, uint32, error) { // the agent to unload process metadata when a thread exits but the process is still // alive). if err != nil { - return mappings, numParseErrors, ErrNoMappings + return numParseErrors, ErrNoMappings } defer mapsFileAlt.Close() - mappings, numParseErrors, err = parseMappings(mapsFileAlt) - if err != nil || len(mappings) == 0 { - return mappings, numParseErrors, ErrNoMappings + fallbackErrors, err := iterateMappings(mapsFileAlt, wrappedFn) + numParseErrors += fallbackErrors + if err != nil || !gotMappings { + return numParseErrors, ErrNoMappings } } - fileToMapping := make(map[string]*Mapping) - for idx := range mappings { - m := &mappings[idx] - if m.Path != libpf.NullString { - fileToMapping[m.Path.String()] = m - } + fileToMapping := make(map[string]*Mapping, len(namedMappings)) + for i := range namedMappings { + fileToMapping[namedMappings[i].Path.String()] = &namedMappings[i] } + sp.fileToMapping = fileToMapping - return mappings, numParseErrors, nil + return numParseErrors, nil +} + +// GetMappings will process the mappings file from proc. Additionally, +// a reverse map from mapping filename to a Mapping node is built to allow +// OpenELF opening ELF files using the corresponding proc map_files entry. +// WARNING: This implementation does not support calling GetMappings +// concurrently with itself, or with OpenELF. +func (sp *systemProcess) GetMappings() ([]Mapping, uint32, error) { + mappings := make([]Mapping, 0, 32) + numParseErrors, err := sp.IterateMappings(func(m Mapping) bool { + mappings = append(mappings, m) + return true + }) + + return mappings, numParseErrors, err } func (sp *systemProcess) GetThreads() ([]ThreadInfo, error) { diff --git a/process/types.go b/process/types.go index 54efc0dea..d32751cce 100644 --- a/process/types.go +++ b/process/types.go @@ -127,6 +127,11 @@ type Process interface { // GetMappings reads and parses process memory mappings. GetMappings() ([]Mapping, uint32, error) + // IterateMappings parses process memory mappings and calls fn for each + // valid mapping. Parsing stops early if fn returns false. + // The returned uint32 is the number of parse errors encountered. + IterateMappings(fn func(m Mapping) bool) (uint32, error) + // GetThreads reads the process thread states. GetThreads() ([]ThreadInfo, error) diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index f1efde54c..0eb0f7189 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -18,6 +18,7 @@ import ( "path" "slices" "sort" + "strings" "syscall" "time" @@ -608,7 +609,18 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { pm.mappingStats.numProcAttempts.Add(1) start := time.Now() - mappings, numParseErrors, err := pr.GetMappings() + + executableMappings := make([]process.Mapping, 0, 32) + var dllMappings []process.Mapping + numParseErrors, err := pr.IterateMappings(func(m process.Mapping) bool { + if m.IsExecutable() || m.IsAnonymous() { + executableMappings = append(executableMappings, m) + } else if strings.HasSuffix(m.Path.String(), ".dll") { + dllMappings = append(dllMappings, m) + } + return true + }) + elapsed := time.Since(start) pm.mappingStats.numProcParseErrors.Add(numParseErrors) @@ -650,7 +662,8 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { util.AtomicUpdateMaxUint32(&pm.mappingStats.maxProcParseUsec, uint32(elapsed.Microseconds())) pm.mappingStats.totalProcParseUsec.Add(uint32(elapsed.Microseconds())) - if pm.synchronizeMappings(pr, mappings) { + allMappings := slices.Concat(executableMappings, dllMappings) + if pm.synchronizeMappings(pr, allMappings) { log.Debugf("+ PID: %v", pid) // TODO: Fine-grained reported_pids handling (evaluate per-PID mapping // synchronization based on per-PID state such as time since last From 79061347001abfe14179b45c246ecd385842279e Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Mon, 16 Mar 2026 16:23:55 +0000 Subject: [PATCH 02/15] feat: add new tests --- process/process_test.go | 176 +++++++++++++++++++++++----------------- 1 file changed, 100 insertions(+), 76 deletions(-) diff --git a/process/process_test.go b/process/process_test.go index b7c943c9e..ce39d4ae8 100644 --- a/process/process_test.go +++ b/process/process_test.go @@ -30,87 +30,111 @@ var testMappings = `55fe82710000-55fe8273c000 r--p 00000000 fd:01 1068432 7f63c8eef000 r-xp 0001c000 1fd:01 1075944 7f8b929f0000-7f8b92a00000 r-xp 00000000 00:00 0 ` +var allExpectedMappings = []Mapping{ + { + Vaddr: 0x55fe82710000, + Device: 0xfd01, + Flags: elf.PF_R, + Inode: 1068432, + Length: 0x2c000, + FileOffset: 0, + Path: libpf.Intern("/tmp/usr_bin_seahorse"), + }, + { + Vaddr: 0x55fe8273c000, + Device: 0xfd01, + Flags: elf.PF_R + elf.PF_X, + Inode: 1068432, + Length: 0x82000, + FileOffset: 0x2c000, + Path: libpf.Intern("/tmp/usr_bin_seahorse"), + }, + { + Vaddr: 0x55fe827be000, + Device: 0xfd01, + Flags: elf.PF_R, + Inode: 1068432, + Length: 0x78000, + FileOffset: 0xae000, + Path: libpf.Intern("/tmp/usr_bin_seahorse"), + }, + { + Vaddr: 0x55fe82836000, + Device: 0xfd01, + Flags: elf.PF_R, + Inode: 1068432, + Length: 0x7000, + FileOffset: 0x125000, + Path: libpf.Intern("/tmp/usr_bin_seahorse"), + }, + { + Vaddr: 0x55fe8283d000, + Device: 0xfd01, + Flags: elf.PF_R + elf.PF_W, + Inode: 1068432, + Length: 0x1000, + FileOffset: 0x12c000, + Path: libpf.Intern("/tmp/usr_bin_seahorse"), + }, + { + Vaddr: 0x7f63c8c3e000, + Device: 0x0801, + Flags: elf.PF_R + elf.PF_X, + Inode: 1048922, + Length: 0x1A2000, + FileOffset: 544768, + Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libcrypto.so.1.1"), + }, + { + Vaddr: 0x7f63c8ebf000, + Device: 0x1fd01, + Flags: elf.PF_R + elf.PF_X, + Inode: 1075944, + Length: 0x130000, + FileOffset: 114688, + Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0"), + }, + { + Vaddr: 0x7f8b929f0000, + Device: 0x0, + Flags: elf.PF_R + elf.PF_X, + Inode: 0, + Length: 0x10000, + FileOffset: 0, + Path: libpf.NullString, + }, +} + func TestParseMappings(t *testing.T) { mappings, numParseErrors, err := parseMappings(strings.NewReader(testMappings)) require.NoError(t, err) require.Equal(t, uint32(4), numParseErrors) - assert.NotNil(t, mappings) + assert.Equal(t, allExpectedMappings, mappings) +} - expected := []Mapping{ - { - Vaddr: 0x55fe82710000, - Device: 0xfd01, - Flags: elf.PF_R, - Inode: 1068432, - Length: 0x2c000, - FileOffset: 0, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), - }, - { - Vaddr: 0x55fe8273c000, - Device: 0xfd01, - Flags: elf.PF_R + elf.PF_X, - Inode: 1068432, - Length: 0x82000, - FileOffset: 0x2c000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), - }, - { - Vaddr: 0x55fe827be000, - Device: 0xfd01, - Flags: elf.PF_R, - Inode: 1068432, - Length: 0x78000, - FileOffset: 0xae000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), - }, - { - Vaddr: 0x55fe82836000, - Device: 0xfd01, - Flags: elf.PF_R, - Inode: 1068432, - Length: 0x7000, - FileOffset: 0x125000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), - }, - { - Vaddr: 0x55fe8283d000, - Device: 0xfd01, - Flags: elf.PF_R + elf.PF_W, - Inode: 1068432, - Length: 0x1000, - FileOffset: 0x12c000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), - }, - { - Vaddr: 0x7f63c8c3e000, - Device: 0x0801, - Flags: elf.PF_R + elf.PF_X, - Inode: 1048922, - Length: 0x1A2000, - FileOffset: 544768, - Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libcrypto.so.1.1"), - }, - { - Vaddr: 0x7f63c8ebf000, - Device: 0x1fd01, - Flags: elf.PF_R + elf.PF_X, - Inode: 1075944, - Length: 0x130000, - FileOffset: 114688, - Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0"), - }, - { - Vaddr: 0x7f8b929f0000, - Device: 0x0, - Flags: elf.PF_R + elf.PF_X, - Inode: 0, - Length: 0x10000, - FileOffset: 0, - Path: libpf.NullString, - }, - } - assert.Equal(t, expected, mappings) +func TestIterateMappings(t *testing.T) { + t.Run("collects all mappings", func(t *testing.T) { + var got []Mapping + numParseErrors, err := iterateMappings(strings.NewReader(testMappings), func(m Mapping) bool { + got = append(got, m) + return true + }) + require.NoError(t, err) + require.Equal(t, uint32(4), numParseErrors) + assert.Equal(t, allExpectedMappings, got) + }) + + t.Run("stops early when callback returns false", func(t *testing.T) { + var got []Mapping + numParseErrors, err := iterateMappings(strings.NewReader(testMappings), func(m Mapping) bool { + got = append(got, m) + return len(got) < 3 + }) + require.NoError(t, err) + assert.Equal(t, uint32(0), numParseErrors) + assert.Len(t, got, 3) + assert.Equal(t, allExpectedMappings[:3], got) + }) } func TestNewPIDOfSelf(t *testing.T) { From 6deb93a6f22737ce5dafd053530972be940bcbc6 Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Wed, 18 Mar 2026 08:58:24 +0000 Subject: [PATCH 03/15] fix: regorganize methods and remove libf.String --- interpreter/dotnet/instance.go | 2 +- interpreter/dotnet/pe.go | 2 +- process/coredump.go | 17 ++---- process/process.go | 71 +++++++++--------------- process/process_test.go | 97 +++++++++++++++++++++++++++------ process/types.go | 46 +++++++++++----- processmanager/processinfo.go | 19 +++++-- tools/coredump/new.go | 6 +- tools/coredump/storecoredump.go | 2 +- 9 files changed, 164 insertions(+), 98 deletions(-) diff --git a/interpreter/dotnet/instance.go b/interpreter/dotnet/instance.go index c59304247..de9d9c673 100644 --- a/interpreter/dotnet/instance.go +++ b/interpreter/dotnet/instance.go @@ -599,7 +599,7 @@ func (i *dotnetInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler, if m.IsAnonymous() { continue } - if !strings.HasSuffix(m.Path.String(), ".dll") { + if !strings.HasSuffix(m.Path, ".dll") { continue } diff --git a/interpreter/dotnet/pe.go b/interpreter/dotnet/pe.go index 952a92e76..824bc936f 100644 --- a/interpreter/dotnet/pe.go +++ b/interpreter/dotnet/pe.go @@ -1243,7 +1243,7 @@ func (pc *peCache) Get(pr process.Process, mapping *process.Mapping) *peInfo { if info.err == nil { mf := libpf.NewFrameMappingFile(libpf.FrameMappingFileData{ FileID: fileID, - FileName: libpf.Intern(path.Base(mapping.Path.String())), + FileName: libpf.Intern(path.Base(mapping.Path)), GnuBuildID: info.guid, }) info.mapping = libpf.NewFrameMapping(libpf.FrameMappingData{ diff --git a/process/coredump.go b/process/coredump.go index 29d42f35b..9fa96e4a7 100644 --- a/process/coredump.go +++ b/process/coredump.go @@ -269,14 +269,9 @@ func (cd *CoredumpProcess) GetExe() (libpf.String, error) { return cd.fname, nil } -// GetMappings implements the Process interface. -func (cd *CoredumpProcess) GetMappings() ([]Mapping, uint32, error) { - return cd.mappings, 0, nil -} - -func (cd *CoredumpProcess) IterateMappings(fn func(m Mapping) bool) (uint32, error) { +func (cd *CoredumpProcess) IterateMappings(callback func(m Mapping) bool) (uint32, error) { for _, m := range cd.mappings { - if !fn(m) { + if !callback(m) { break } } @@ -308,7 +303,7 @@ func (cd *CoredumpProcess) CalculateMappingFileID(m *Mapping) (libpf.FileID, err h := fnv.New128a() _, _ = h.Write(vaddr) - _, _ = h.Write([]byte(m.Path.String())) + _, _ = h.Write([]byte(m.Path)) return libpf.FileIDFromBytes(h.Sum(nil)) } @@ -408,7 +403,7 @@ func (cd *CoredumpProcess) parseMappings(desc []byte, cf.Mappings = append(cf.Mappings, cm) mapping := &cd.mappings[m.mappingIndex] - mapping.Path = cf.Name + mapping.Path = cf.Name.String() mapping.FileOffset = entry.FileOffset * hdr.PageSize // Synthesize non-zero device and inode indicating this is a filebacked mapping. mapping.Device = 1 @@ -424,7 +419,7 @@ func (cd *CoredumpProcess) parseMappings(desc []byte, FileOffset: entry.FileOffset * hdr.PageSize, Device: 1, Inode: cf.inode, - Path: cf.Name, + Path: cf.Name.String(), }) } strs = strs[fnlen+1:] @@ -447,7 +442,7 @@ func (cd *CoredumpProcess) parseAuxVector(desc []byte, vaddrToMappings map[uint6 vm.Inode = vdsoInode vm.Path = VdsoPathName - cf := cd.getFile(vm.Path.String()) + cf := cd.getFile(vm.Path) cm := CoredumpMapping{ Prog: m.prog, File: cf, diff --git a/process/process.go b/process/process.go index 324b71780..670a8312e 100644 --- a/process/process.go +++ b/process/process.go @@ -189,7 +189,7 @@ func trimMappingPath(path string) string { return path } -func iterateMappings(mapsFile io.Reader, fn func(m Mapping) bool) (uint32, error) { +func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, error) { numParseErrors := uint32(0) scanner := bufio.NewScanner(mapsFile) scanBuf := bufPool.Get().(*[]byte) @@ -210,6 +210,9 @@ func iterateMappings(mapsFile io.Reader, fn func(m Mapping) bool) (uint32, error var addrs [2]string var devs [2]string + // WARNING: line (and all substrings derived from it, including + // mappingPath) points into scanBuf which is recycled after iteration. + // Callbacks that store mappings must call m.Retain() to clone Path. line := pfunsafe.ToString(scanner.Bytes()) if stringutil.FieldsN(line, fields[:]) < 5 { numParseErrors++ @@ -265,21 +268,20 @@ func iterateMappings(mapsFile io.Reader, fn func(m Mapping) bool) (uint32, error } device := major<<8 + minor - var path libpf.String + var path string if inode == 0 { if fields[5] == "[vdso]" { - // Map to something filename looking with synthesized inode path = VdsoPathName device = 0 inode = vdsoInode } else if fields[5] == "" { - // This is an anonymous mapping, keep it + // Anonymous mapping, keep it with empty path } else { // Ignore other mappings that are invalid, non-existent or are special pseudo-files continue } } else { - path = libpf.Intern(trimMappingPath(fields[5])) + path = trimMappingPath(fields[5]) } vaddr, err := strconv.ParseUint(addrs[0], 16, 64) @@ -294,6 +296,7 @@ func iterateMappings(mapsFile io.Reader, fn func(m Mapping) bool) (uint32, error numParseErrors++ continue } + length := vend - vaddr fileOffset, err := strconv.ParseUint(fields[2], 16, 64) if err != nil { @@ -302,9 +305,9 @@ func iterateMappings(mapsFile io.Reader, fn func(m Mapping) bool) (uint32, error continue } - if !fn(Mapping{ + if !callback(Mapping{ Vaddr: vaddr, - Length: vend - vaddr, + Length: length, Flags: flags, FileOffset: fileOffset, Device: device, @@ -317,34 +320,26 @@ func iterateMappings(mapsFile io.Reader, fn func(m Mapping) bool) (uint32, error return numParseErrors, scanner.Err() } -func parseMappings(mapsFile io.Reader) ([]Mapping, uint32, error) { - mappings := make([]Mapping, 0, 32) - numParseErrors, err := iterateMappings(mapsFile, func(m Mapping) bool { - mappings = append(mappings, m) - return true - }) - - return mappings, numParseErrors, err -} - -func (sp *systemProcess) IterateMappings(fn func(m Mapping) bool) (uint32, error) { +func (sp *systemProcess) IterateMappings(callback func(m Mapping) bool) (uint32, error) { mapsFile, err := os.Open(fmt.Sprintf("/proc/%d/maps", sp.pid)) if err != nil { return 0, err } defer mapsFile.Close() - var namedMappings []Mapping + var elfMappings []Mapping gotMappings := false - wrappedFn := func(m Mapping) bool { + + collectForOpenELF := func(m Mapping) bool { gotMappings = true - if m.Path != libpf.NullString { - namedMappings = append(namedMappings, m) + if m.IsExecutable() || m.IsVDSO() { + m.Retain() + elfMappings = append(elfMappings, m) } - return fn(m) + return callback(m) } - numParseErrors, err := iterateMappings(mapsFile, wrappedFn) + numParseErrors, err := iterateMappings(mapsFile, collectForOpenELF) if err != nil { return numParseErrors, err } @@ -374,35 +369,21 @@ func (sp *systemProcess) IterateMappings(fn func(m Mapping) bool) (uint32, error return numParseErrors, ErrNoMappings } defer mapsFileAlt.Close() - fallbackErrors, err := iterateMappings(mapsFileAlt, wrappedFn) + fallbackErrors, err := iterateMappings(mapsFileAlt, collectForOpenELF) numParseErrors += fallbackErrors if err != nil || !gotMappings { return numParseErrors, ErrNoMappings } } - fileToMapping := make(map[string]*Mapping, len(namedMappings)) - for i := range namedMappings { - fileToMapping[namedMappings[i].Path.String()] = &namedMappings[i] + fileToMapping := make(map[string]*Mapping, len(elfMappings)) + for i := range elfMappings { + m := &elfMappings[i] + fileToMapping[m.Path] = m } - sp.fileToMapping = fileToMapping - return numParseErrors, nil -} -// GetMappings will process the mappings file from proc. Additionally, -// a reverse map from mapping filename to a Mapping node is built to allow -// OpenELF opening ELF files using the corresponding proc map_files entry. -// WARNING: This implementation does not support calling GetMappings -// concurrently with itself, or with OpenELF. -func (sp *systemProcess) GetMappings() ([]Mapping, uint32, error) { - mappings := make([]Mapping, 0, 32) - numParseErrors, err := sp.IterateMappings(func(m Mapping) bool { - mappings = append(mappings, m) - return true - }) - - return mappings, numParseErrors, err + return numParseErrors, nil } func (sp *systemProcess) GetThreads() ([]ThreadInfo, error) { @@ -436,7 +417,7 @@ func (sp *systemProcess) getMappingFile(m *Mapping) string { // nor /proc/sp.pid/root exist if main thread has exited, so we use the // mapping path directly under the sp.tid root. rootPath := fmt.Sprintf("/proc/%v/task/%v/root", sp.pid, sp.tid) - return path.Join(rootPath, m.Path.String()) + return path.Join(rootPath, m.Path) } return fmt.Sprintf("/proc/%v/map_files/%x-%x", sp.pid, m.Vaddr, m.Vaddr+m.Length) } diff --git a/process/process_test.go b/process/process_test.go index ce39d4ae8..5606c806f 100644 --- a/process/process_test.go +++ b/process/process_test.go @@ -5,6 +5,7 @@ package process import ( "debug/elf" + "io" "os" "runtime" "strings" @@ -38,7 +39,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x2c000, FileOffset: 0, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x55fe8273c000, @@ -47,7 +48,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x82000, FileOffset: 0x2c000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x55fe827be000, @@ -56,7 +57,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x78000, FileOffset: 0xae000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x55fe82836000, @@ -65,7 +66,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x7000, FileOffset: 0x125000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x55fe8283d000, @@ -74,7 +75,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x1000, FileOffset: 0x12c000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x7f63c8c3e000, @@ -83,7 +84,7 @@ var allExpectedMappings = []Mapping{ Inode: 1048922, Length: 0x1A2000, FileOffset: 544768, - Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libcrypto.so.1.1"), + Path: "/tmp/usr_lib_x86_64-linux-gnu_libcrypto.so.1.1", }, { Vaddr: 0x7f63c8ebf000, @@ -92,7 +93,7 @@ var allExpectedMappings = []Mapping{ Inode: 1075944, Length: 0x130000, FileOffset: 114688, - Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0"), + Path: "/tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0", }, { Vaddr: 0x7f8b929f0000, @@ -101,12 +102,39 @@ var allExpectedMappings = []Mapping{ Inode: 0, Length: 0x10000, FileOffset: 0, - Path: libpf.NullString, }, } +func getTestMappings(t *testing.T, mapsFile io.Reader) ([]Mapping, uint32, error) { + t.Helper() + + mappings := make([]Mapping, 0, 32) + collectAll := func(m Mapping) bool { + m.Retain() + mappings = append(mappings, m) + return true + } + + numParseErrors, err := iterateMappings(mapsFile, collectAll) + return mappings, numParseErrors, err +} + +func getTestMappingsFromProcess(t *testing.T, process Process) ([]Mapping, uint32, error) { + t.Helper() + + mappings := make([]Mapping, 0, 32) + collectAll := func(m Mapping) bool { + m.Retain() + mappings = append(mappings, m) + return true + } + + numParseErrors, err := process.IterateMappings(collectAll) + return mappings, numParseErrors, err +} + func TestParseMappings(t *testing.T) { - mappings, numParseErrors, err := parseMappings(strings.NewReader(testMappings)) + mappings, numParseErrors, err := getTestMappings(t, strings.NewReader(testMappings)) require.NoError(t, err) require.Equal(t, uint32(4), numParseErrors) assert.Equal(t, allExpectedMappings, mappings) @@ -114,22 +142,20 @@ func TestParseMappings(t *testing.T) { func TestIterateMappings(t *testing.T) { t.Run("collects all mappings", func(t *testing.T) { - var got []Mapping - numParseErrors, err := iterateMappings(strings.NewReader(testMappings), func(m Mapping) bool { - got = append(got, m) - return true - }) + mappings, numParseErrors, err := getTestMappings(t, strings.NewReader(testMappings)) require.NoError(t, err) require.Equal(t, uint32(4), numParseErrors) - assert.Equal(t, allExpectedMappings, got) + assert.Equal(t, allExpectedMappings, mappings) }) t.Run("stops early when callback returns false", func(t *testing.T) { var got []Mapping - numParseErrors, err := iterateMappings(strings.NewReader(testMappings), func(m Mapping) bool { + collectThree := func(m Mapping) bool { + m.Retain() got = append(got, m) return len(got) < 3 - }) + } + numParseErrors, err := iterateMappings(strings.NewReader(testMappings), collectThree) require.NoError(t, err) assert.Equal(t, uint32(0), numParseErrors) assert.Len(t, got, 3) @@ -137,6 +163,41 @@ func TestIterateMappings(t *testing.T) { }) } +func TestMappingPredicates(t *testing.T) { + tests := []struct { + name string + m Mapping + wantAnon bool + wantFile bool + wantMemFD bool + wantVDSO bool + }{ + {"anonymous", Mapping{}, true, false, false, false}, + {"file-backed", Mapping{Path: "/usr/lib/foo.so"}, false, true, false, false}, + {"memfd", Mapping{Path: "/memfd:jit"}, true, false, true, false}, + {"vdso", Mapping{Path: VdsoPathName}, false, false, false, true}, + {"/dev/zero normalized", Mapping{Inode: 42, Device: 1}, true, false, false, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantAnon, tt.m.IsAnonymous(), "IsAnonymous") + assert.Equal(t, tt.wantFile, tt.m.IsFileBacked(), "IsFileBacked") + assert.Equal(t, tt.wantMemFD, tt.m.IsMemFD(), "IsMemFD") + assert.Equal(t, tt.wantVDSO, tt.m.IsVDSO(), "IsVDSO") + }) + } +} + +func TestMappingRetain(t *testing.T) { + m := Mapping{Path: "/usr/lib/foo.so"} + assert.True(t, m.IsFileBacked()) + + m.Retain() + assert.Equal(t, "/usr/lib/foo.so", m.Path) + assert.True(t, m.IsFileBacked()) + assert.False(t, m.IsAnonymous()) +} + func TestNewPIDOfSelf(t *testing.T) { if runtime.GOOS != "linux" { t.Skipf("unsupported os %s", runtime.GOOS) @@ -145,7 +206,7 @@ func TestNewPIDOfSelf(t *testing.T) { pr := New(pid, pid) assert.NotNil(t, pr) - mappings, numParseErrors, err := pr.GetMappings() + mappings, numParseErrors, err := getTestMappingsFromProcess(t, pr) require.NoError(t, err) require.Equal(t, uint32(0), numParseErrors) assert.NotEmpty(t, mappings) diff --git a/process/types.go b/process/types.go index d32751cce..236a244b5 100644 --- a/process/types.go +++ b/process/types.go @@ -16,8 +16,8 @@ import ( "go.opentelemetry.io/ebpf-profiler/util" ) -// VdsoPathName is the path to use for VDSO mappings. -var VdsoPathName = libpf.Intern("linux-vdso.1.so") +// VdsoPathName is the path used for VDSO mappings. +const VdsoPathName = "linux-vdso.1.so" // vdsoInode is the synthesized inode number for VDSO mappings. const vdsoInode = 50 @@ -36,26 +36,46 @@ type Mapping struct { Device uint64 // Inode holds the mapped file's inode number. Inode uint64 - // Path contains the file name for file backed mappings. - Path libpf.String + // Path is the file path for file-backed and special mappings. + // For live processes parsed from /proc/pid/maps, this string may + // reference an internal buffer that is recycled. Callers must call + // Retain() on any mapping they store beyond the iteration callback. + Path string } func (m *Mapping) IsExecutable() bool { return m.Flags&elf.PF_X == elf.PF_X } +// IsAnonymous returns true for mappings without a backing file. +// This includes memfd mappings and /dev/zero. func (m *Mapping) IsAnonymous() bool { - return m.Path == libpf.NullString || m.IsMemFD() + return !m.IsFileBacked() && !m.IsVDSO() +} + +// IsFileBacked returns true for mappings backed by a regular file on disk. +// Excludes memfd, vdso, and anonymous mappings. +func (m *Mapping) IsFileBacked() bool { + return m.Path != "" && !m.IsVDSO() && !m.IsMemFD() } func (m *Mapping) IsMemFD() bool { - return strings.HasPrefix(m.Path.String(), "/memfd:") + return strings.HasPrefix(m.Path, "/memfd:") } func (m *Mapping) IsVDSO() bool { return m.Path == VdsoPathName } +// Retain makes a copy of Path so the mapping can safely outlive the +// parser's internal buffer. Must be called inside the IterateMappings +// callback for any mapping that will be stored beyond the callback. +// Safe to call on mappings from implementations that don't use buffer +// recycling (e.g. coredump) -- it's a no-op clone in that case. +func (m *Mapping) Retain() { + m.Path = strings.Clone(m.Path) +} + func (m *Mapping) GetOnDiskFileIdentifier() util.OnDiskFileIdentifier { return util.OnDiskFileIdentifier{ DeviceID: m.Device, @@ -124,13 +144,13 @@ type Process interface { // GetExe returns the executable path of the process. GetExe() (libpf.String, error) - // GetMappings reads and parses process memory mappings. - GetMappings() ([]Mapping, uint32, error) - - // IterateMappings parses process memory mappings and calls fn for each - // valid mapping. Parsing stops early if fn returns false. - // The returned uint32 is the number of parse errors encountered. - IterateMappings(fn func(m Mapping) bool) (uint32, error) + // IterateMappings parses process memory mappings and calls callback for + // each valid mapping. Path is set to the raw file path for file-backed + // mappings. The callback must call m.Retain() on any mapping it stores + // beyond the callback scope, because Path may reference an internal + // buffer that is recycled after iteration. Parsing stops early if + // callback returns false. + IterateMappings(callback func(m Mapping) bool) (uint32, error) // GetThreads reads the process thread states. GetThreads() ([]ThreadInfo, error) diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 0eb0f7189..1422f159e 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -225,7 +225,7 @@ func (pm *ProcessManager) getELFInfo(pr process.Process, mapping *process.Mappin return info } - baseName := path.Base(mapping.Path.String()) + baseName := path.Base(mapping.Path) if baseName == "/" { // There are circumstances where there is no filename. // E.g. kernel module 'bpfilter_umh' before Linux 5.9-rc1 uses @@ -333,7 +333,7 @@ func (pm *ProcessManager) processRemovedInterpreters(pid libpf.PID, var errInvalidVirtualAddress = errors.New("invalid ELF virtual address") func (pm *ProcessManager) newFrameMapping(pr process.Process, m *process.Mapping) (libpf.FrameMapping, error) { - elfRef := pfelf.NewReference(m.Path.String(), pr) + elfRef := pfelf.NewReference(m.Path, pr) defer elfRef.Close() info := pm.getELFInfo(pr, m, elfRef) @@ -612,14 +612,23 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { executableMappings := make([]process.Mapping, 0, 32) var dllMappings []process.Mapping - numParseErrors, err := pr.IterateMappings(func(m process.Mapping) bool { + + // filterMappings keeps only mappings relevant for profiling: + // executable, anonymous (JIT), and .dll (dotnet PE assemblies). + // Non-executable file-backed mappings (e.g. read-only data files) + // are discarded without interning their path. + filterMappings := func(m process.Mapping) bool { if m.IsExecutable() || m.IsAnonymous() { + m.Retain() executableMappings = append(executableMappings, m) - } else if strings.HasSuffix(m.Path.String(), ".dll") { + } else if m.IsFileBacked() && strings.HasSuffix(m.Path, ".dll") { + m.Retain() dllMappings = append(dllMappings, m) } return true - }) + } + + numParseErrors, err := pr.IterateMappings(filterMappings) elapsed := time.Since(start) pm.mappingStats.numProcParseErrors.Add(numParseErrors) diff --git a/tools/coredump/new.go b/tools/coredump/new.go index 73b382d82..c5087eac2 100644 --- a/tools/coredump/new.go +++ b/tools/coredump/new.go @@ -73,7 +73,7 @@ func (tc *trackedCoredump) warnMissing(fileName string) { func (tc *trackedCoredump) CalculateMappingFileID(m *process.Mapping) (libpf.FileID, error) { if !m.IsVDSO() && !m.IsAnonymous() { - file := m.Path.String() + file := m.Path fid, err := libpf.FileIDFromExecutableFile(path.Join(tc.prefix, file)) if err == nil { tc.seen[file] = libpf.Void{} @@ -86,7 +86,7 @@ func (tc *trackedCoredump) CalculateMappingFileID(m *process.Mapping) (libpf.Fil func (tc *trackedCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCloser, error) { if !m.IsVDSO() && !m.IsAnonymous() { - file := m.Path.String() + file := m.Path rac, err := os.Open(path.Join(tc.prefix, file)) if err == nil { tc.seen[file] = libpf.Void{} @@ -98,7 +98,7 @@ func (tc *trackedCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCl } func (tc *trackedCoredump) OpenELF(fileName string) (*pfelf.File, error) { - if fileName != process.VdsoPathName.String() { + if fileName != process.VdsoPathName { f, err := pfelf.Open(path.Join(tc.prefix, fileName)) if err == nil { tc.seen[fileName] = libpf.Void{} diff --git a/tools/coredump/storecoredump.go b/tools/coredump/storecoredump.go index 954c700a1..2c3149f0a 100644 --- a/tools/coredump/storecoredump.go +++ b/tools/coredump/storecoredump.go @@ -46,7 +46,7 @@ func (scd *StoreCoredump) openFile(path string) (process.ReadAtCloser, error) { } func (scd *StoreCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCloser, error) { - return scd.openFile(m.Path.String()) + return scd.openFile(m.Path) } func (scd *StoreCoredump) OpenELF(path string) (*pfelf.File, error) { From 9fd4079db4f47787f0d749e6b431db2643a0d861 Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Wed, 18 Mar 2026 09:59:37 +0000 Subject: [PATCH 04/15] fix: rollback to libpf.Intern but with new intermediate type RawMapping for better perf --- interpreter/dotnet/instance.go | 2 +- interpreter/dotnet/pe.go | 2 +- process/coredump.go | 23 +++++--- process/process.go | 22 ++++---- process/process_test.go | 82 +++++++++++++++++++--------- process/types.go | 96 +++++++++++++++++++++++---------- processmanager/processinfo.go | 12 ++--- tools/coredump/new.go | 4 +- tools/coredump/storecoredump.go | 2 +- 9 files changed, 162 insertions(+), 83 deletions(-) diff --git a/interpreter/dotnet/instance.go b/interpreter/dotnet/instance.go index de9d9c673..c59304247 100644 --- a/interpreter/dotnet/instance.go +++ b/interpreter/dotnet/instance.go @@ -599,7 +599,7 @@ func (i *dotnetInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler, if m.IsAnonymous() { continue } - if !strings.HasSuffix(m.Path, ".dll") { + if !strings.HasSuffix(m.Path.String(), ".dll") { continue } diff --git a/interpreter/dotnet/pe.go b/interpreter/dotnet/pe.go index 824bc936f..952a92e76 100644 --- a/interpreter/dotnet/pe.go +++ b/interpreter/dotnet/pe.go @@ -1243,7 +1243,7 @@ func (pc *peCache) Get(pr process.Process, mapping *process.Mapping) *peInfo { if info.err == nil { mf := libpf.NewFrameMappingFile(libpf.FrameMappingFileData{ FileID: fileID, - FileName: libpf.Intern(path.Base(mapping.Path)), + FileName: libpf.Intern(path.Base(mapping.Path.String())), GnuBuildID: info.guid, }) info.mapping = libpf.NewFrameMapping(libpf.FrameMappingData{ diff --git a/process/coredump.go b/process/coredump.go index 9fa96e4a7..1c9ef06b7 100644 --- a/process/coredump.go +++ b/process/coredump.go @@ -269,9 +269,18 @@ func (cd *CoredumpProcess) GetExe() (libpf.String, error) { return cd.fname, nil } -func (cd *CoredumpProcess) IterateMappings(callback func(m Mapping) bool) (uint32, error) { +func (cd *CoredumpProcess) IterateMappings(callback func(m RawMapping) bool) (uint32, error) { for _, m := range cd.mappings { - if !callback(m) { + raw := RawMapping{ + Vaddr: m.Vaddr, + Length: m.Length, + Flags: m.Flags, + FileOffset: m.FileOffset, + Device: m.Device, + Inode: m.Inode, + Path: m.Path.String(), + } + if !callback(raw) { break } } @@ -303,7 +312,7 @@ func (cd *CoredumpProcess) CalculateMappingFileID(m *Mapping) (libpf.FileID, err h := fnv.New128a() _, _ = h.Write(vaddr) - _, _ = h.Write([]byte(m.Path)) + _, _ = h.Write([]byte(m.Path.String())) return libpf.FileIDFromBytes(h.Sum(nil)) } @@ -403,7 +412,7 @@ func (cd *CoredumpProcess) parseMappings(desc []byte, cf.Mappings = append(cf.Mappings, cm) mapping := &cd.mappings[m.mappingIndex] - mapping.Path = cf.Name.String() + mapping.Path = cf.Name mapping.FileOffset = entry.FileOffset * hdr.PageSize // Synthesize non-zero device and inode indicating this is a filebacked mapping. mapping.Device = 1 @@ -419,7 +428,7 @@ func (cd *CoredumpProcess) parseMappings(desc []byte, FileOffset: entry.FileOffset * hdr.PageSize, Device: 1, Inode: cf.inode, - Path: cf.Name.String(), + Path: cf.Name, }) } strs = strs[fnlen+1:] @@ -440,9 +449,9 @@ func (cd *CoredumpProcess) parseAuxVector(desc []byte, vaddrToMappings map[uint6 vm := &cd.mappings[m.mappingIndex] vm.Inode = vdsoInode - vm.Path = VdsoPathName + vm.Path = libpf.Intern(VdsoPathName) - cf := cd.getFile(vm.Path) + cf := cd.getFile(vm.Path.String()) cm := CoredumpMapping{ Prog: m.prog, File: cf, diff --git a/process/process.go b/process/process.go index 670a8312e..32159b3e2 100644 --- a/process/process.go +++ b/process/process.go @@ -27,7 +27,7 @@ import ( "go.opentelemetry.io/ebpf-profiler/stringutil" ) -// GetMappings returns this error when no mappings can be extracted. +// IterateMappings returns this error when no mappings can be extracted. var ErrNoMappings = errors.New("no mappings") const ( @@ -189,7 +189,7 @@ func trimMappingPath(path string) string { return path } -func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, error) { +func iterateMappings(mapsFile io.Reader, callback func(m RawMapping) bool) (uint32, error) { numParseErrors := uint32(0) scanner := bufio.NewScanner(mapsFile) scanBuf := bufPool.Get().(*[]byte) @@ -211,8 +211,9 @@ func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, var devs [2]string // WARNING: line (and all substrings derived from it, including - // mappingPath) points into scanBuf which is recycled after iteration. - // Callbacks that store mappings must call m.Retain() to clone Path. + // the Path field of the emitted RawMapping) points into scanBuf + // which is recycled after iteration. Callers must use + // m.ToMapping() to produce a safe copy before storing. line := pfunsafe.ToString(scanner.Bytes()) if stringutil.FieldsN(line, fields[:]) < 5 { numParseErrors++ @@ -305,7 +306,7 @@ func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, continue } - if !callback(Mapping{ + if !callback(RawMapping{ Vaddr: vaddr, Length: length, Flags: flags, @@ -320,7 +321,7 @@ func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, return numParseErrors, scanner.Err() } -func (sp *systemProcess) IterateMappings(callback func(m Mapping) bool) (uint32, error) { +func (sp *systemProcess) IterateMappings(callback func(m RawMapping) bool) (uint32, error) { mapsFile, err := os.Open(fmt.Sprintf("/proc/%d/maps", sp.pid)) if err != nil { return 0, err @@ -330,11 +331,10 @@ func (sp *systemProcess) IterateMappings(callback func(m Mapping) bool) (uint32, var elfMappings []Mapping gotMappings := false - collectForOpenELF := func(m Mapping) bool { + collectForOpenELF := func(m RawMapping) bool { gotMappings = true if m.IsExecutable() || m.IsVDSO() { - m.Retain() - elfMappings = append(elfMappings, m) + elfMappings = append(elfMappings, m.ToMapping()) } return callback(m) } @@ -379,7 +379,7 @@ func (sp *systemProcess) IterateMappings(callback func(m Mapping) bool) (uint32, fileToMapping := make(map[string]*Mapping, len(elfMappings)) for i := range elfMappings { m := &elfMappings[i] - fileToMapping[m.Path] = m + fileToMapping[m.Path.String()] = m } sp.fileToMapping = fileToMapping @@ -417,7 +417,7 @@ func (sp *systemProcess) getMappingFile(m *Mapping) string { // nor /proc/sp.pid/root exist if main thread has exited, so we use the // mapping path directly under the sp.tid root. rootPath := fmt.Sprintf("/proc/%v/task/%v/root", sp.pid, sp.tid) - return path.Join(rootPath, m.Path) + return path.Join(rootPath, m.Path.String()) } return fmt.Sprintf("/proc/%v/map_files/%x-%x", sp.pid, m.Vaddr, m.Vaddr+m.Length) } diff --git a/process/process_test.go b/process/process_test.go index 5606c806f..888608140 100644 --- a/process/process_test.go +++ b/process/process_test.go @@ -39,7 +39,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x2c000, FileOffset: 0, - Path: "/tmp/usr_bin_seahorse", + Path: libpf.Intern("/tmp/usr_bin_seahorse"), }, { Vaddr: 0x55fe8273c000, @@ -48,7 +48,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x82000, FileOffset: 0x2c000, - Path: "/tmp/usr_bin_seahorse", + Path: libpf.Intern("/tmp/usr_bin_seahorse"), }, { Vaddr: 0x55fe827be000, @@ -57,7 +57,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x78000, FileOffset: 0xae000, - Path: "/tmp/usr_bin_seahorse", + Path: libpf.Intern("/tmp/usr_bin_seahorse"), }, { Vaddr: 0x55fe82836000, @@ -66,7 +66,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x7000, FileOffset: 0x125000, - Path: "/tmp/usr_bin_seahorse", + Path: libpf.Intern("/tmp/usr_bin_seahorse"), }, { Vaddr: 0x55fe8283d000, @@ -75,7 +75,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x1000, FileOffset: 0x12c000, - Path: "/tmp/usr_bin_seahorse", + Path: libpf.Intern("/tmp/usr_bin_seahorse"), }, { Vaddr: 0x7f63c8c3e000, @@ -84,7 +84,7 @@ var allExpectedMappings = []Mapping{ Inode: 1048922, Length: 0x1A2000, FileOffset: 544768, - Path: "/tmp/usr_lib_x86_64-linux-gnu_libcrypto.so.1.1", + Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libcrypto.so.1.1"), }, { Vaddr: 0x7f63c8ebf000, @@ -93,7 +93,7 @@ var allExpectedMappings = []Mapping{ Inode: 1075944, Length: 0x130000, FileOffset: 114688, - Path: "/tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0", + Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0"), }, { Vaddr: 0x7f8b929f0000, @@ -102,6 +102,7 @@ var allExpectedMappings = []Mapping{ Inode: 0, Length: 0x10000, FileOffset: 0, + Path: libpf.NullString, }, } @@ -109,9 +110,8 @@ func getTestMappings(t *testing.T, mapsFile io.Reader) ([]Mapping, uint32, error t.Helper() mappings := make([]Mapping, 0, 32) - collectAll := func(m Mapping) bool { - m.Retain() - mappings = append(mappings, m) + collectAll := func(m RawMapping) bool { + mappings = append(mappings, m.ToMapping()) return true } @@ -123,9 +123,8 @@ func getTestMappingsFromProcess(t *testing.T, process Process) ([]Mapping, uint3 t.Helper() mappings := make([]Mapping, 0, 32) - collectAll := func(m Mapping) bool { - m.Retain() - mappings = append(mappings, m) + collectAll := func(m RawMapping) bool { + mappings = append(mappings, m.ToMapping()) return true } @@ -150,9 +149,8 @@ func TestIterateMappings(t *testing.T) { t.Run("stops early when callback returns false", func(t *testing.T) { var got []Mapping - collectThree := func(m Mapping) bool { - m.Retain() - got = append(got, m) + collectThree := func(m RawMapping) bool { + got = append(got, m.ToMapping()) return len(got) < 3 } numParseErrors, err := iterateMappings(strings.NewReader(testMappings), collectThree) @@ -163,6 +161,31 @@ func TestIterateMappings(t *testing.T) { }) } +func TestRawMappingPredicates(t *testing.T) { + tests := []struct { + name string + m RawMapping + wantAnon bool + wantFile bool + wantMemFD bool + wantVDSO bool + }{ + {"anonymous", RawMapping{}, true, false, false, false}, + {"file-backed", RawMapping{Path: "/usr/lib/foo.so"}, false, true, false, false}, + {"memfd", RawMapping{Path: "/memfd:jit"}, true, false, true, false}, + {"vdso", RawMapping{Path: VdsoPathName}, false, false, false, true}, + {"/dev/zero normalized", RawMapping{Inode: 42, Device: 1}, true, false, false, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantAnon, tt.m.IsAnonymous(), "IsAnonymous") + assert.Equal(t, tt.wantFile, tt.m.IsFileBacked(), "IsFileBacked") + assert.Equal(t, tt.wantMemFD, tt.m.IsMemFD(), "IsMemFD") + assert.Equal(t, tt.wantVDSO, tt.m.IsVDSO(), "IsVDSO") + }) + } +} + func TestMappingPredicates(t *testing.T) { tests := []struct { name string @@ -173,9 +196,9 @@ func TestMappingPredicates(t *testing.T) { wantVDSO bool }{ {"anonymous", Mapping{}, true, false, false, false}, - {"file-backed", Mapping{Path: "/usr/lib/foo.so"}, false, true, false, false}, - {"memfd", Mapping{Path: "/memfd:jit"}, true, false, true, false}, - {"vdso", Mapping{Path: VdsoPathName}, false, false, false, true}, + {"file-backed", Mapping{Path: libpf.Intern("/usr/lib/foo.so")}, false, true, false, false}, + {"memfd", Mapping{Path: libpf.Intern("/memfd:jit")}, true, false, true, false}, + {"vdso", Mapping{Path: libpf.Intern(VdsoPathName)}, false, false, false, true}, {"/dev/zero normalized", Mapping{Inode: 42, Device: 1}, true, false, false, false}, } for _, tt := range tests { @@ -188,14 +211,21 @@ func TestMappingPredicates(t *testing.T) { } } -func TestMappingRetain(t *testing.T) { - m := Mapping{Path: "/usr/lib/foo.so"} - assert.True(t, m.IsFileBacked()) - - m.Retain() - assert.Equal(t, "/usr/lib/foo.so", m.Path) +func TestToMapping(t *testing.T) { + raw := RawMapping{ + Vaddr: 0x1000, + Length: 0x2000, + Flags: elf.PF_R + elf.PF_X, + Path: "/usr/lib/foo.so", + Device: 1, + Inode: 42, + } + m := raw.ToMapping() + assert.Equal(t, libpf.Intern("/usr/lib/foo.so"), m.Path) + assert.Equal(t, raw.Vaddr, m.Vaddr) + assert.Equal(t, raw.Length, m.Length) assert.True(t, m.IsFileBacked()) - assert.False(t, m.IsAnonymous()) + assert.True(t, m.IsExecutable()) } func TestNewPIDOfSelf(t *testing.T) { diff --git a/process/types.go b/process/types.go index 236a244b5..25d1da4b2 100644 --- a/process/types.go +++ b/process/types.go @@ -22,8 +22,12 @@ const VdsoPathName = "linux-vdso.1.so" // vdsoInode is the synthesized inode number for VDSO mappings. const vdsoInode = 50 -// Mapping contains information about a memory mapping. -type Mapping struct { +// RawMapping is the ephemeral representation of a memory mapping as parsed +// from /proc/pid/maps. Path is a plain string that may reference the +// parser's internal buffer and must not be stored beyond the +// IterateMappings callback. Use ToMapping() to produce a Mapping with +// an interned Path that is safe to store long-term. +type RawMapping struct { // Vaddr is the virtual memory start for this mapping. Vaddr uint64 // Length is the length of the mapping. @@ -37,43 +41,81 @@ type Mapping struct { // Inode holds the mapped file's inode number. Inode uint64 // Path is the file path for file-backed and special mappings. - // For live processes parsed from /proc/pid/maps, this string may - // reference an internal buffer that is recycled. Callers must call - // Retain() on any mapping they store beyond the iteration callback. + // May reference an internal buffer recycled after iteration. Path string } -func (m *Mapping) IsExecutable() bool { +func (m *RawMapping) IsExecutable() bool { return m.Flags&elf.PF_X == elf.PF_X } -// IsAnonymous returns true for mappings without a backing file. -// This includes memfd mappings and /dev/zero. -func (m *Mapping) IsAnonymous() bool { +func (m *RawMapping) IsAnonymous() bool { return !m.IsFileBacked() && !m.IsVDSO() } -// IsFileBacked returns true for mappings backed by a regular file on disk. -// Excludes memfd, vdso, and anonymous mappings. -func (m *Mapping) IsFileBacked() bool { +func (m *RawMapping) IsFileBacked() bool { return m.Path != "" && !m.IsVDSO() && !m.IsMemFD() } -func (m *Mapping) IsMemFD() bool { +func (m *RawMapping) IsMemFD() bool { return strings.HasPrefix(m.Path, "/memfd:") } -func (m *Mapping) IsVDSO() bool { +func (m *RawMapping) IsVDSO() bool { return m.Path == VdsoPathName } -// Retain makes a copy of Path so the mapping can safely outlive the -// parser's internal buffer. Must be called inside the IterateMappings -// callback for any mapping that will be stored beyond the callback. -// Safe to call on mappings from implementations that don't use buffer -// recycling (e.g. coredump) -- it's a no-op clone in that case. -func (m *Mapping) Retain() { - m.Path = strings.Clone(m.Path) +// ToMapping converts to a Mapping with an interned Path. Only call this +// for mappings you intend to keep. +func (m *RawMapping) ToMapping() Mapping { + return Mapping{ + Vaddr: m.Vaddr, + Length: m.Length, + Flags: m.Flags, + FileOffset: m.FileOffset, + Device: m.Device, + Inode: m.Inode, + Path: libpf.Intern(m.Path), + } +} + +// Mapping is the stable representation of a memory mapping with an +// interned Path. Produced by RawMapping.ToMapping() after filtering. +type Mapping struct { + // Vaddr is the virtual memory start for this mapping. + Vaddr uint64 + // Length is the length of the mapping. + Length uint64 + // Flags contains the mapping flags and permissions. + Flags elf.ProgFlag + // FileOffset contains for file backed mappings the offset from the file start. + FileOffset uint64 + // Device holds the device ID where the file is located. + Device uint64 + // Inode holds the mapped file's inode number. + Inode uint64 + // Path is the interned file path for file-backed and special mappings. + Path libpf.String +} + +func (m *Mapping) IsExecutable() bool { + return m.Flags&elf.PF_X == elf.PF_X +} + +func (m *Mapping) IsAnonymous() bool { + return !m.IsFileBacked() && !m.IsVDSO() +} + +func (m *Mapping) IsFileBacked() bool { + return m.Path != libpf.NullString && !m.IsVDSO() && !m.IsMemFD() +} + +func (m *Mapping) IsMemFD() bool { + return strings.HasPrefix(m.Path.String(), "/memfd:") +} + +func (m *Mapping) IsVDSO() bool { + return m.Path.String() == VdsoPathName } func (m *Mapping) GetOnDiskFileIdentifier() util.OnDiskFileIdentifier { @@ -144,13 +186,13 @@ type Process interface { // GetExe returns the executable path of the process. GetExe() (libpf.String, error) - // IterateMappings parses process memory mappings and calls callback for - // each valid mapping. Path is set to the raw file path for file-backed - // mappings. The callback must call m.Retain() on any mapping it stores - // beyond the callback scope, because Path may reference an internal - // buffer that is recycled after iteration. Parsing stops early if + // IterateMappings parses process memory mappings and calls callback + // for each valid mapping. The callback receives a RawMapping whose + // Path may reference an internal buffer recycled after iteration. + // Use m.ToMapping() inside the callback to produce a safe Mapping + // for any mapping you intend to keep. Parsing stops early if // callback returns false. - IterateMappings(callback func(m Mapping) bool) (uint32, error) + IterateMappings(callback func(m RawMapping) bool) (uint32, error) // GetThreads reads the process thread states. GetThreads() ([]ThreadInfo, error) diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 1422f159e..3095261d8 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -225,7 +225,7 @@ func (pm *ProcessManager) getELFInfo(pr process.Process, mapping *process.Mappin return info } - baseName := path.Base(mapping.Path) + baseName := path.Base(mapping.Path.String()) if baseName == "/" { // There are circumstances where there is no filename. // E.g. kernel module 'bpfilter_umh' before Linux 5.9-rc1 uses @@ -333,7 +333,7 @@ func (pm *ProcessManager) processRemovedInterpreters(pid libpf.PID, var errInvalidVirtualAddress = errors.New("invalid ELF virtual address") func (pm *ProcessManager) newFrameMapping(pr process.Process, m *process.Mapping) (libpf.FrameMapping, error) { - elfRef := pfelf.NewReference(m.Path, pr) + elfRef := pfelf.NewReference(m.Path.String(), pr) defer elfRef.Close() info := pm.getELFInfo(pr, m, elfRef) @@ -617,13 +617,11 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { // executable, anonymous (JIT), and .dll (dotnet PE assemblies). // Non-executable file-backed mappings (e.g. read-only data files) // are discarded without interning their path. - filterMappings := func(m process.Mapping) bool { + filterMappings := func(m process.RawMapping) bool { if m.IsExecutable() || m.IsAnonymous() { - m.Retain() - executableMappings = append(executableMappings, m) + executableMappings = append(executableMappings, m.ToMapping()) } else if m.IsFileBacked() && strings.HasSuffix(m.Path, ".dll") { - m.Retain() - dllMappings = append(dllMappings, m) + dllMappings = append(dllMappings, m.ToMapping()) } return true } diff --git a/tools/coredump/new.go b/tools/coredump/new.go index c5087eac2..539eb8272 100644 --- a/tools/coredump/new.go +++ b/tools/coredump/new.go @@ -73,7 +73,7 @@ func (tc *trackedCoredump) warnMissing(fileName string) { func (tc *trackedCoredump) CalculateMappingFileID(m *process.Mapping) (libpf.FileID, error) { if !m.IsVDSO() && !m.IsAnonymous() { - file := m.Path + file := m.Path.String() fid, err := libpf.FileIDFromExecutableFile(path.Join(tc.prefix, file)) if err == nil { tc.seen[file] = libpf.Void{} @@ -86,7 +86,7 @@ func (tc *trackedCoredump) CalculateMappingFileID(m *process.Mapping) (libpf.Fil func (tc *trackedCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCloser, error) { if !m.IsVDSO() && !m.IsAnonymous() { - file := m.Path + file := m.Path.String() rac, err := os.Open(path.Join(tc.prefix, file)) if err == nil { tc.seen[file] = libpf.Void{} diff --git a/tools/coredump/storecoredump.go b/tools/coredump/storecoredump.go index 2c3149f0a..954c700a1 100644 --- a/tools/coredump/storecoredump.go +++ b/tools/coredump/storecoredump.go @@ -46,7 +46,7 @@ func (scd *StoreCoredump) openFile(path string) (process.ReadAtCloser, error) { } func (scd *StoreCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCloser, error) { - return scd.openFile(m.Path) + return scd.openFile(m.Path.String()) } func (scd *StoreCoredump) OpenELF(path string) (*pfelf.File, error) { From cf618b91e0926ac5bb30d5ceedd96c3bc07bfe88 Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Wed, 18 Mar 2026 14:21:59 +0000 Subject: [PATCH 05/15] feat: refacto synchronizeMappings into SynchronizeProcess in order to allow for one pass loop --- process/process_test.go | 22 --- processmanager/processinfo.go | 328 ++++++++++++++++------------------ 2 files changed, 157 insertions(+), 193 deletions(-) diff --git a/process/process_test.go b/process/process_test.go index 888608140..326d28854 100644 --- a/process/process_test.go +++ b/process/process_test.go @@ -139,28 +139,6 @@ func TestParseMappings(t *testing.T) { assert.Equal(t, allExpectedMappings, mappings) } -func TestIterateMappings(t *testing.T) { - t.Run("collects all mappings", func(t *testing.T) { - mappings, numParseErrors, err := getTestMappings(t, strings.NewReader(testMappings)) - require.NoError(t, err) - require.Equal(t, uint32(4), numParseErrors) - assert.Equal(t, allExpectedMappings, mappings) - }) - - t.Run("stops early when callback returns false", func(t *testing.T) { - var got []Mapping - collectThree := func(m RawMapping) bool { - got = append(got, m.ToMapping()) - return len(got) < 3 - } - numParseErrors, err := iterateMappings(strings.NewReader(testMappings), collectThree) - require.NoError(t, err) - assert.Equal(t, uint32(0), numParseErrors) - assert.Len(t, got, 3) - assert.Equal(t, allExpectedMappings[:3], got) - }) -} - func TestRawMappingPredicates(t *testing.T) { tests := []struct { name string diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 3095261d8..95c0876fd 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -395,154 +395,6 @@ func compareMapping(a, b Mapping) int { return 0 } -// synchronizeMappings synchronizes executable mappings for the given PID. -// This method will be called when a PID is first encountered or when the eBPF -// code encounters an address in an executable mapping that HA has no information -// on. Therefore, executable mapping synchronization takes place lazily on-demand, -// and map/unmap operations are not precisely tracked (reduce processing load). -// This means that at any point, we may have cached stale (or miss) executable -// mappings. The expectation is that stale mappings will disappear and new -// mappings cached at the next synchronization triggered by process exit or -// unknown address encountered. -// -// TODO: Periodic synchronization of mappings for every tracked PID. -func (pm *ProcessManager) synchronizeMappings(pr process.Process, - processMappings []process.Mapping) bool { - pid := pr.PID() - - // Get current executable name - exe, err := pr.GetExe() - if err != nil && !os.IsNotExist(err) { - // The /proc/PID/exe returns "not exists" error also in - // the case of main thread exit. Ignore it. - log.Warnf("Failed to get executable of process %d: %v", pid, err) - } - - pm.mu.Lock() - info := pm.getPidInformation(pid, pr) - if info == nil { - pm.mu.Unlock() - return false - } - // Check if process meta needs an update - updateProcessMeta := exe != libpf.NullString && exe != info.meta.Executable - - // Get existing info - oldMappings := info.mappings - newProcess := len(info.mappings) == 0 - var numInterpreters int - if intrp, ok := pm.interpreters[pid]; ok { - numInterpreters = len(intrp) - } - pm.mu.Unlock() - - // Create a lookup map for the old mappings - mpRemove := make(map[uint64]*Mapping, len(oldMappings)) - for idx := range oldMappings { - m := &oldMappings[idx] - mpRemove[uint64(m.Vaddr)] = m - } - - // Generate the list of new processmanager mappings and interpreters. - // Reuse existing mappings if possible. - mappings := make([]Mapping, 0, len(processMappings)) - mpAdd := make([]*Mapping, 0, len(processMappings)) - interpretersValid := make(libpf.Set[util.OnDiskFileIdentifier], numInterpreters) - for idx := range processMappings { - m := &processMappings[idx] - if !m.IsExecutable() || m.IsAnonymous() { - continue - } - - var fm libpf.FrameMapping - if oldm, ok := mpRemove[m.Vaddr]; ok { - if oldm.Length == m.Length && oldm.Device == m.Device && oldm.Inode == m.Inode { - delete(mpRemove, m.Vaddr) - fm = oldm.FrameMapping - } - } - newMapping := false - if !fm.Valid() { - newMapping = true - var err error - fm, err = pm.newFrameMapping(pr, m) - if err != nil { - // newFrameMapping logged the message if needed - continue - } - } - - key := m.GetOnDiskFileIdentifier() - interpretersValid[key] = libpf.Void{} - - mappings = append(mappings, Mapping{ - Vaddr: libpf.Address(m.Vaddr), - Length: m.Length, - Device: m.Device, - Inode: m.Inode, - FrameMapping: fm, - }) - if newMapping { - mpAdd = append(mpAdd, &mappings[len(mappings)-1]) - } - } - - // Detach removed interpreters and remove old mappings - numChanges := uint64(0) - for _, m := range mpRemove { - numChanges += pm.processRemovedMapping(pid, m) - } - pm.pidPageToMappingInfoSize -= numChanges - pm.mu.Lock() - pm.processRemovedInterpreters(pid, interpretersValid) - pm.mu.Unlock() - - // Add new mappings - numChanges = 0 - for _, m := range mpAdd { - numChanges += pm.processNewMapping(pid, m) - } - pm.pidPageToMappingInfoSize += numChanges - - // Update metadata of the process. - var meta process.ProcessMeta - if updateProcessMeta { - meta = pr.GetProcessMeta(process.MetaConfig{IncludeEnvVars: pm.includeEnvVars}) - } - - // Sort and publish the new mappings and meta - slices.SortFunc(mappings, compareMapping) - pm.mu.Lock() - info = pm.getPidInformation(pid, pr) - if info != nil { - info.mappings = mappings - if updateProcessMeta { - info.meta = meta - } - } - interpreters := pm.interpreters[pid] - pm.mu.Unlock() - - // Synchronize all interpreters with updated mappings - for _, instance := range interpreters { - err := instance.SynchronizeMappings(pm.ebpf, pm.exeReporter, pr, processMappings) - if err != nil { - if alive, _ := isPIDLive(pid); alive { - log.Errorf("Failed to handle new anonymous mapping for PID %d: %v", pid, err) - } else { - log.Debugf("Failed to handle new anonymous mapping for PID %d: process exited", - pid) - } - } - } - - if len(mpAdd) > 0 || len(mpRemove) > 0 || len(interpreters) > 0 { - log.Debugf("Added %v mappings, removed %v mappings for PID %v with %d interpreters", - len(mpAdd), len(mpRemove), pid, len(interpreters)) - } - return newProcess -} - // processPIDExit informs the ProcessManager that a process exited and no longer will be scheduled. // exitKTime is stored for later processing in ProcessedUntil, when traces up to this time have been // processed. There can be a race condition if we can not clean up the references for this process @@ -591,7 +443,18 @@ func (pm *ProcessManager) processPIDExit(pid libpf.PID) { } // SynchronizeProcess triggers ProcessManager to update its internal information -// about a process. This includes process exit information as well as changed memory mappings. +// about a process. It synchronizes executable mappings for the given PID by +// parsing /proc/PID/maps and building the internal mapping state directly in +// a single pass. This method will be called when a PID is first encountered or +// when the eBPF code encounters an address in an executable mapping that HA has +// no information on. Therefore, executable mapping synchronization takes place +// lazily on-demand, and map/unmap operations are not precisely tracked (reduce +// processing load). This means that at any point, we may have cached stale (or +// miss) executable mappings. The expectation is that stale mappings will +// disappear and new mappings cached at the next synchronization triggered by +// process exit or unknown address encountered. +// +// TODO: Periodic synchronization of mappings for every tracked PID. func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { pid := pr.PID() log.Debugf("= PID: %v", pid) @@ -607,26 +470,84 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { return } + // Read existing mapping state for diffing. We only read here (no creation) + // so that if IterateMappings fails, we haven't created any state to leak. + var oldMappings []Mapping + var numInterpreters int + + pm.mu.RLock() + if info, ok := pm.pidToProcessInfo[pid]; ok { + oldMappings = info.mappings + } + if intrp, ok := pm.interpreters[pid]; ok { + numInterpreters = len(intrp) + } + pm.mu.RUnlock() + + // Create a lookup map for the old mappings + mpRemove := make(map[uint64]*Mapping, len(oldMappings)) + for idx := range oldMappings { + m := &oldMappings[idx] + mpRemove[uint64(m.Vaddr)] = m + } + + interpreterMappings := make([]process.Mapping, 0, 8) + interpretersValid := make(libpf.Set[util.OnDiskFileIdentifier], numInterpreters) + capHeuristic := max(32, len(oldMappings)) + mappings := make([]Mapping, 0, capHeuristic) + mpAdd := make([]*Mapping, 0, capHeuristic) + pm.mappingStats.numProcAttempts.Add(1) start := time.Now() - executableMappings := make([]process.Mapping, 0, 32) - var dllMappings []process.Mapping - - // filterMappings keeps only mappings relevant for profiling: - // executable, anonymous (JIT), and .dll (dotnet PE assemblies). - // Non-executable file-backed mappings (e.g. read-only data files) - // are discarded without interning their path. - filterMappings := func(m process.RawMapping) bool { - if m.IsExecutable() || m.IsAnonymous() { - executableMappings = append(executableMappings, m.ToMapping()) - } else if m.IsFileBacked() && strings.HasSuffix(m.Path, ".dll") { - dllMappings = append(dllMappings, m.ToMapping()) + numParseErrors, err := pr.IterateMappings(func(raw process.RawMapping) bool { + // executable mappings and VDSO converted directly to libpf.FrameMapping + if raw.IsExecutable() && !raw.IsAnonymous() { + m := raw.ToMapping() + + var fm libpf.FrameMapping + if oldm, ok := mpRemove[m.Vaddr]; ok { + if oldm.Length == m.Length && oldm.Device == m.Device && oldm.Inode == m.Inode { + delete(mpRemove, m.Vaddr) + fm = oldm.FrameMapping + } + } + newMapping := false + if !fm.Valid() { + newMapping = true + var err error + fm, err = pm.newFrameMapping(pr, &m) + if err != nil { + return true // Ignore error, continue with next mapping + } + } + + key := m.GetOnDiskFileIdentifier() + interpretersValid[key] = libpf.Void{} + + mappings = append(mappings, Mapping{ + Vaddr: libpf.Address(m.Vaddr), + Length: m.Length, + Device: m.Device, + Inode: m.Inode, + FrameMapping: fm, + }) + if newMapping { + mpAdd = append(mpAdd, &mappings[len(mappings)-1]) + } } - return true - } - numParseErrors, err := pr.IterateMappings(filterMappings) + // Interpreters specific mappings + // Needed by V8 and BEAM to retrieve JIT mappings + if raw.IsExecutable() && raw.IsAnonymous() { + interpreterMappings = append(interpreterMappings, raw.ToMapping()) + } + // Needed by .NET to retrieve PE assembly mappings + if !raw.IsAnonymous() && strings.HasSuffix(raw.Path, ".dll") { + interpreterMappings = append(interpreterMappings, raw.ToMapping()) + } + return true + }) elapsed := time.Since(start) pm.mappingStats.numProcParseErrors.Add(numParseErrors) @@ -669,8 +590,78 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { util.AtomicUpdateMaxUint32(&pm.mappingStats.maxProcParseUsec, uint32(elapsed.Microseconds())) pm.mappingStats.totalProcParseUsec.Add(uint32(elapsed.Microseconds())) - allMappings := slices.Concat(executableMappings, dllMappings) - if pm.synchronizeMappings(pr, allMappings) { + exe, exeErr := pr.GetExe() + if exeErr != nil && !os.IsNotExist(exeErr) { + log.Warnf("Failed to get executable of process %d: %v", pid, exeErr) + } + + pm.mu.Lock() + info := pm.getPidInformation(pid, pr) + if info == nil { + pm.mu.Unlock() + return + } + + // Check if process meta needs an update + updateProcessMeta := exe != libpf.NullString && exe != info.meta.Executable + newProcess := len(info.mappings) == 0 + pm.mu.Unlock() + + // Detach removed interpreters and remove old mappings + numChanges := uint64(0) + for _, m := range mpRemove { + numChanges += pm.processRemovedMapping(pid, m) + } + pm.pidPageToMappingInfoSize -= numChanges + pm.mu.Lock() + pm.processRemovedInterpreters(pid, interpretersValid) + pm.mu.Unlock() + + // Add new mappings + numChanges = 0 + for _, m := range mpAdd { + numChanges += pm.processNewMapping(pid, m) + } + pm.pidPageToMappingInfoSize += numChanges + + // Update metadata of the process. + var meta process.ProcessMeta + if updateProcessMeta { + meta = pr.GetProcessMeta(process.MetaConfig{IncludeEnvVars: pm.includeEnvVars}) + } + + // Sort and publish the new mappings and meta + slices.SortFunc(mappings, compareMapping) + pm.mu.Lock() + info = pm.getPidInformation(pid, pr) + if info != nil { + info.mappings = mappings + if updateProcessMeta { + info.meta = meta + } + } + interpreters := pm.interpreters[pid] + pm.mu.Unlock() + + // Synchronize all interpreters with updated mappings + for _, instance := range interpreters { + err := instance.SynchronizeMappings(pm.ebpf, pm.exeReporter, pr, interpreterMappings) + if err != nil { + if alive, _ := isPIDLive(pid); alive { + log.Errorf("Failed to handle new anonymous mapping for PID %d: %v", pid, err) + } else { + log.Debugf("Failed to handle new anonymous mapping for PID %d: process exited", + pid) + } + } + } + + if len(mpAdd) > 0 || len(mpRemove) > 0 || len(interpreters) > 0 { + log.Debugf("Added %v mappings, removed %v mappings for PID %v with %d interpreters", + len(mpAdd), len(mpRemove), pid, len(interpreters)) + } + + if newProcess { log.Debugf("+ PID: %v", pid) // TODO: Fine-grained reported_pids handling (evaluate per-PID mapping // synchronization based on per-PID state such as time since last @@ -678,11 +669,6 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { // if it's a new process and on process exit. This limits // the frequency of PID mapping synchronizations to PID lifetime in // reported_pids (which is dictated by REPORTED_PIDS_TIMEOUT in eBPF). - - // We're immediately removing a new PID from reported_pids, to cover - // corner cases where processes load on startup in quick-succession - // additional code (e.g. plugins, Asterisk). - // Also see: Unified PID Events design doc pm.ebpf.RemoveReportedPID(pid) } } From 3bd7429f76bace6bfe750fb946f9a55c90e0ca31 Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Wed, 18 Mar 2026 14:38:37 +0000 Subject: [PATCH 06/15] chore: clean code and comments --- process/coredump.go | 3 ++- process/process.go | 10 ++++++++-- process/process_test.go | 4 ++-- process/types.go | 15 +++++++-------- processmanager/processinfo.go | 5 +++++ tools/coredump/new.go | 2 +- 6 files changed, 25 insertions(+), 14 deletions(-) diff --git a/process/coredump.go b/process/coredump.go index 1c9ef06b7..fa127cdee 100644 --- a/process/coredump.go +++ b/process/coredump.go @@ -269,6 +269,7 @@ func (cd *CoredumpProcess) GetExe() (libpf.String, error) { return cd.fname, nil } +// IterateMappings implements the Process interface. func (cd *CoredumpProcess) IterateMappings(callback func(m RawMapping) bool) (uint32, error) { for _, m := range cd.mappings { raw := RawMapping{ @@ -449,7 +450,7 @@ func (cd *CoredumpProcess) parseAuxVector(desc []byte, vaddrToMappings map[uint6 vm := &cd.mappings[m.mappingIndex] vm.Inode = vdsoInode - vm.Path = libpf.Intern(VdsoPathName) + vm.Path = VdsoPathName cf := cd.getFile(vm.Path.String()) cm := CoredumpMapping{ diff --git a/process/process.go b/process/process.go index 32159b3e2..04787a9bb 100644 --- a/process/process.go +++ b/process/process.go @@ -272,11 +272,12 @@ func iterateMappings(mapsFile io.Reader, callback func(m RawMapping) bool) (uint var path string if inode == 0 { if fields[5] == "[vdso]" { - path = VdsoPathName + // Map to something filename looking with synthesized inode + path = VdsoPathName.String() device = 0 inode = vdsoInode } else if fields[5] == "" { - // Anonymous mapping, keep it with empty path + // This is an anonymous mapping, keep it } else { // Ignore other mappings that are invalid, non-existent or are special pseudo-files continue @@ -321,6 +322,11 @@ func iterateMappings(mapsFile io.Reader, callback func(m RawMapping) bool) (uint return numParseErrors, scanner.Err() } +// IterateMappings parses process memory mappings and calls callback +// for each mapping. The callback receives a RawMapping whose Path +// may reference an internal buffer recycled after iteration; use +// ToMapping() to produce a Mapping safe to store long-term. +// The callback is responsible for filtering out unwanted mappings. func (sp *systemProcess) IterateMappings(callback func(m RawMapping) bool) (uint32, error) { mapsFile, err := os.Open(fmt.Sprintf("/proc/%d/maps", sp.pid)) if err != nil { diff --git a/process/process_test.go b/process/process_test.go index 326d28854..cd633655b 100644 --- a/process/process_test.go +++ b/process/process_test.go @@ -151,7 +151,7 @@ func TestRawMappingPredicates(t *testing.T) { {"anonymous", RawMapping{}, true, false, false, false}, {"file-backed", RawMapping{Path: "/usr/lib/foo.so"}, false, true, false, false}, {"memfd", RawMapping{Path: "/memfd:jit"}, true, false, true, false}, - {"vdso", RawMapping{Path: VdsoPathName}, false, false, false, true}, + {"vdso", RawMapping{Path: VdsoPathName.String()}, false, false, false, true}, {"/dev/zero normalized", RawMapping{Inode: 42, Device: 1}, true, false, false, false}, } for _, tt := range tests { @@ -176,7 +176,7 @@ func TestMappingPredicates(t *testing.T) { {"anonymous", Mapping{}, true, false, false, false}, {"file-backed", Mapping{Path: libpf.Intern("/usr/lib/foo.so")}, false, true, false, false}, {"memfd", Mapping{Path: libpf.Intern("/memfd:jit")}, true, false, true, false}, - {"vdso", Mapping{Path: libpf.Intern(VdsoPathName)}, false, false, false, true}, + {"vdso", Mapping{Path: VdsoPathName}, false, false, false, true}, {"/dev/zero normalized", Mapping{Inode: 42, Device: 1}, true, false, false, false}, } for _, tt := range tests { diff --git a/process/types.go b/process/types.go index 25d1da4b2..906e6930c 100644 --- a/process/types.go +++ b/process/types.go @@ -17,7 +17,7 @@ import ( ) // VdsoPathName is the path used for VDSO mappings. -const VdsoPathName = "linux-vdso.1.so" +var VdsoPathName = libpf.Intern("linux-vdso.1.so") // vdsoInode is the synthesized inode number for VDSO mappings. const vdsoInode = 50 @@ -62,7 +62,7 @@ func (m *RawMapping) IsMemFD() bool { } func (m *RawMapping) IsVDSO() bool { - return m.Path == VdsoPathName + return m.Path == VdsoPathName.String() } // ToMapping converts to a Mapping with an interned Path. Only call this @@ -115,7 +115,7 @@ func (m *Mapping) IsMemFD() bool { } func (m *Mapping) IsVDSO() bool { - return m.Path.String() == VdsoPathName + return m.Path == VdsoPathName } func (m *Mapping) GetOnDiskFileIdentifier() util.OnDiskFileIdentifier { @@ -187,11 +187,10 @@ type Process interface { GetExe() (libpf.String, error) // IterateMappings parses process memory mappings and calls callback - // for each valid mapping. The callback receives a RawMapping whose - // Path may reference an internal buffer recycled after iteration. - // Use m.ToMapping() inside the callback to produce a safe Mapping - // for any mapping you intend to keep. Parsing stops early if - // callback returns false. + // for each mapping. The callback receives a RawMapping whose Path + // may reference an internal buffer recycled after iteration; use + // ToMapping() to produce a Mapping safe to store long-term. + // The callback is responsible for filtering out unwanted mappings. IterateMappings(callback func(m RawMapping) bool) (uint32, error) // GetThreads reads the process thread states. diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 95c0876fd..e8696189b 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -669,6 +669,11 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { // if it's a new process and on process exit. This limits // the frequency of PID mapping synchronizations to PID lifetime in // reported_pids (which is dictated by REPORTED_PIDS_TIMEOUT in eBPF). + + // We're immediately removing a new PID from reported_pids, to cover + // corner cases where processes load on startup in quick-succession + // additional code (e.g. plugins, Asterisk). + // Also see: Unified PID Events design doc pm.ebpf.RemoveReportedPID(pid) } } diff --git a/tools/coredump/new.go b/tools/coredump/new.go index 539eb8272..73b382d82 100644 --- a/tools/coredump/new.go +++ b/tools/coredump/new.go @@ -98,7 +98,7 @@ func (tc *trackedCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCl } func (tc *trackedCoredump) OpenELF(fileName string) (*pfelf.File, error) { - if fileName != process.VdsoPathName { + if fileName != process.VdsoPathName.String() { f, err := pfelf.Open(path.Join(tc.prefix, fileName)) if err == nil { tc.seen[fileName] = libpf.Void{} From 86af6ae2baaffc19e04881ec53de3b030c75ce8f Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Wed, 18 Mar 2026 14:42:28 +0000 Subject: [PATCH 07/15] chore: clean code and comments --- process/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/process/types.go b/process/types.go index 906e6930c..bcbfa125f 100644 --- a/process/types.go +++ b/process/types.go @@ -16,7 +16,7 @@ import ( "go.opentelemetry.io/ebpf-profiler/util" ) -// VdsoPathName is the path used for VDSO mappings. +// VdsoPathName is the path to use for VDSO mappings. var VdsoPathName = libpf.Intern("linux-vdso.1.so") // vdsoInode is the synthesized inode number for VDSO mappings. From ccc8720b79f6d67a1aa2bc3140566d66d399a77a Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Wed, 18 Mar 2026 14:57:53 +0000 Subject: [PATCH 08/15] chore: remove fallbackerror to follow old implem, rename capHeuristic --- process/process.go | 3 +-- processmanager/processinfo.go | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/process/process.go b/process/process.go index 04787a9bb..df2efe70b 100644 --- a/process/process.go +++ b/process/process.go @@ -375,8 +375,7 @@ func (sp *systemProcess) IterateMappings(callback func(m RawMapping) bool) (uint return numParseErrors, ErrNoMappings } defer mapsFileAlt.Close() - fallbackErrors, err := iterateMappings(mapsFileAlt, collectForOpenELF) - numParseErrors += fallbackErrors + numParseErrors, err := iterateMappings(mapsFileAlt, collectForOpenELF) if err != nil || !gotMappings { return numParseErrors, ErrNoMappings } diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index e8696189b..9a6ae17f2 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -493,9 +493,9 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { interpreterMappings := make([]process.Mapping, 0, 8) interpretersValid := make(libpf.Set[util.OnDiskFileIdentifier], numInterpreters) - capHeuristic := max(32, len(oldMappings)) - mappings := make([]Mapping, 0, capHeuristic) - mpAdd := make([]*Mapping, 0, capHeuristic) + capHint := max(32, len(oldMappings)) + mappings := make([]Mapping, 0, capHint) + mpAdd := make([]*Mapping, 0, capHint) pm.mappingStats.numProcAttempts.Add(1) start := time.Now() From 37662d6129a16ff12cea3ca7f99265c9a83c4546 Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Wed, 18 Mar 2026 16:05:52 +0000 Subject: [PATCH 09/15] fix: resolve coredump tests (python, perl, dotnet) --- processmanager/processinfo.go | 82 ++++++++++++++++------------------- 1 file changed, 38 insertions(+), 44 deletions(-) diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 9a6ae17f2..d6de937f0 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -470,19 +470,31 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { return } - // Read existing mapping state for diffing. We only read here (no creation) - // so that if IterateMappings fails, we haven't created any state to leak. - var oldMappings []Mapping - var numInterpreters int + // Get current executable name + exe, exeErr := pr.GetExe() + if exeErr != nil && !os.IsNotExist(exeErr) { + // The /proc/PID/exe returns "not exists" error also in + // the case of main thread exit. Ignore it. + log.Warnf("Failed to get executable of process %d: %v", pid, exeErr) + } - pm.mu.RLock() - if info, ok := pm.pidToProcessInfo[pid]; ok { - oldMappings = info.mappings + pm.mu.Lock() + info := pm.getPidInformation(pid, pr) + if info == nil { + pm.mu.Unlock() + return } + // Check if process meta needs an update + updateProcessMeta := exe != libpf.NullString && exe != info.meta.Executable + + // Get existing info + oldMappings := info.mappings + newProcess := len(info.mappings) == 0 + var numInterpreters int if intrp, ok := pm.interpreters[pid]; ok { numInterpreters = len(intrp) } - pm.mu.RUnlock() + pm.mu.Unlock() // Create a lookup map for the old mappings mpRemove := make(map[uint64]*Mapping, len(oldMappings)) @@ -493,7 +505,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { interpreterMappings := make([]process.Mapping, 0, 8) interpretersValid := make(libpf.Set[util.OnDiskFileIdentifier], numInterpreters) - capHint := max(32, len(oldMappings)) + capHint := max(32, min(len(oldMappings), 256)) mappings := make([]Mapping, 0, capHint) mpAdd := make([]*Mapping, 0, capHint) @@ -515,25 +527,24 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { newMapping := false if !fm.Valid() { newMapping = true - var err error - fm, err = pm.newFrameMapping(pr, &m) - if err != nil { - return true // Ignore error, continue with next mapping - } + // Error is expected for non-ELF files (e.g. PE .dll); + // fm will be invalid and the mapping skipped below but will enter the interpreter mappings block. + fm, _ = pm.newFrameMapping(pr, &m) } - - key := m.GetOnDiskFileIdentifier() - interpretersValid[key] = libpf.Void{} - - mappings = append(mappings, Mapping{ - Vaddr: libpf.Address(m.Vaddr), - Length: m.Length, - Device: m.Device, - Inode: m.Inode, - FrameMapping: fm, - }) - if newMapping { - mpAdd = append(mpAdd, &mappings[len(mappings)-1]) + if fm.Valid() { + key := m.GetOnDiskFileIdentifier() + interpretersValid[key] = libpf.Void{} + + mappings = append(mappings, Mapping{ + Vaddr: libpf.Address(m.Vaddr), + Length: m.Length, + Device: m.Device, + Inode: m.Inode, + FrameMapping: fm, + }) + if newMapping { + mpAdd = append(mpAdd, &mappings[len(mappings)-1]) + } } } @@ -590,23 +601,6 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { util.AtomicUpdateMaxUint32(&pm.mappingStats.maxProcParseUsec, uint32(elapsed.Microseconds())) pm.mappingStats.totalProcParseUsec.Add(uint32(elapsed.Microseconds())) - exe, exeErr := pr.GetExe() - if exeErr != nil && !os.IsNotExist(exeErr) { - log.Warnf("Failed to get executable of process %d: %v", pid, exeErr) - } - - pm.mu.Lock() - info := pm.getPidInformation(pid, pr) - if info == nil { - pm.mu.Unlock() - return - } - - // Check if process meta needs an update - updateProcessMeta := exe != libpf.NullString && exe != info.meta.Executable - newProcess := len(info.mappings) == 0 - pm.mu.Unlock() - // Detach removed interpreters and remove old mappings numChanges := uint64(0) for _, m := range mpRemove { From abffc57b261146ee82e0a92a81326d569c71439c Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Thu, 19 Mar 2026 13:46:01 +0000 Subject: [PATCH 10/15] fix: apply review --- interpreter/dotnet/instance.go | 2 +- interpreter/dotnet/pe.go | 2 +- interpreter/types.go | 16 ++++-- process/coredump.go | 23 +++------ process/process.go | 45 +++++++--------- process/process_test.go | 80 +++++++--------------------- process/types.go | 92 +++++++++------------------------ processmanager/processinfo.go | 48 ++++++++++------- tools/coredump/new.go | 6 +-- tools/coredump/storecoredump.go | 2 +- 10 files changed, 117 insertions(+), 199 deletions(-) diff --git a/interpreter/dotnet/instance.go b/interpreter/dotnet/instance.go index c59304247..de9d9c673 100644 --- a/interpreter/dotnet/instance.go +++ b/interpreter/dotnet/instance.go @@ -599,7 +599,7 @@ func (i *dotnetInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler, if m.IsAnonymous() { continue } - if !strings.HasSuffix(m.Path.String(), ".dll") { + if !strings.HasSuffix(m.Path, ".dll") { continue } diff --git a/interpreter/dotnet/pe.go b/interpreter/dotnet/pe.go index 952a92e76..824bc936f 100644 --- a/interpreter/dotnet/pe.go +++ b/interpreter/dotnet/pe.go @@ -1243,7 +1243,7 @@ func (pc *peCache) Get(pr process.Process, mapping *process.Mapping) *peInfo { if info.err == nil { mf := libpf.NewFrameMappingFile(libpf.FrameMappingFileData{ FileID: fileID, - FileName: libpf.Intern(path.Base(mapping.Path.String())), + FileName: libpf.Intern(path.Base(mapping.Path)), GnuBuildID: info.guid, }) info.mapping = libpf.NewFrameMapping(libpf.FrameMappingData{ diff --git a/interpreter/types.go b/interpreter/types.go index 991ec65a8..cfa055c44 100644 --- a/interpreter/types.go +++ b/interpreter/types.go @@ -137,9 +137,19 @@ type Instance interface { // simple interpreters can use the global Data also as the Instance implementation. Detach(ebpf EbpfHandler, pid libpf.PID) error - // SynchronizeMappings is called when the processmanager has reread process memory - // mappings. Interpreters not needing to process these events can simply ignore them - // by just returning a nil. + // SynchronizeMappings is called when the processmanager has reread process + // memory mappings. The mappings slice contains only the subset of mappings + // that are relevant to interpreters: executable anonymous mappings (for JIT + // engines like HotSpot, V8, BEAM) and .dll file-backed mappings (for .NET + // PE assemblies). The processmanager decides which mappings are included; + // this can be made more dynamic in the future if needed. + // + // The mappings are in /proc/PID/maps order (ascending by virtual address) + // but are NOT sorted by any other criteria. Interpreters that need a + // specific ordering must sort locally. + // + // Interpreters not needing to process these events can simply ignore them + // by returning nil. SynchronizeMappings(ebpf EbpfHandler, exeReporter reporter.ExecutableReporter, pr process.Process, mappings []process.Mapping) error diff --git a/process/coredump.go b/process/coredump.go index fa127cdee..349f74389 100644 --- a/process/coredump.go +++ b/process/coredump.go @@ -270,19 +270,10 @@ func (cd *CoredumpProcess) GetExe() (libpf.String, error) { } // IterateMappings implements the Process interface. -func (cd *CoredumpProcess) IterateMappings(callback func(m RawMapping) bool) (uint32, error) { +func (cd *CoredumpProcess) IterateMappings(callback func(m Mapping) bool) (uint32, error) { for _, m := range cd.mappings { - raw := RawMapping{ - Vaddr: m.Vaddr, - Length: m.Length, - Flags: m.Flags, - FileOffset: m.FileOffset, - Device: m.Device, - Inode: m.Inode, - Path: m.Path.String(), - } - if !callback(raw) { - break + if !callback(m) { + return 0, ErrCallbackStopped } } return 0, nil @@ -313,7 +304,7 @@ func (cd *CoredumpProcess) CalculateMappingFileID(m *Mapping) (libpf.FileID, err h := fnv.New128a() _, _ = h.Write(vaddr) - _, _ = h.Write([]byte(m.Path.String())) + _, _ = h.Write([]byte(m.Path)) return libpf.FileIDFromBytes(h.Sum(nil)) } @@ -413,7 +404,7 @@ func (cd *CoredumpProcess) parseMappings(desc []byte, cf.Mappings = append(cf.Mappings, cm) mapping := &cd.mappings[m.mappingIndex] - mapping.Path = cf.Name + mapping.Path = cf.Name.String() mapping.FileOffset = entry.FileOffset * hdr.PageSize // Synthesize non-zero device and inode indicating this is a filebacked mapping. mapping.Device = 1 @@ -429,7 +420,7 @@ func (cd *CoredumpProcess) parseMappings(desc []byte, FileOffset: entry.FileOffset * hdr.PageSize, Device: 1, Inode: cf.inode, - Path: cf.Name, + Path: cf.Name.String(), }) } strs = strs[fnlen+1:] @@ -452,7 +443,7 @@ func (cd *CoredumpProcess) parseAuxVector(desc []byte, vaddrToMappings map[uint6 vm.Inode = vdsoInode vm.Path = VdsoPathName - cf := cd.getFile(vm.Path.String()) + cf := cd.getFile(vm.Path) cm := CoredumpMapping{ Prog: m.prog, File: cf, diff --git a/process/process.go b/process/process.go index df2efe70b..a7aafbcc1 100644 --- a/process/process.go +++ b/process/process.go @@ -27,9 +27,13 @@ import ( "go.opentelemetry.io/ebpf-profiler/stringutil" ) -// IterateMappings returns this error when no mappings can be extracted. +// ErrNoMappings is returned when no mappings can be extracted. var ErrNoMappings = errors.New("no mappings") +// ErrCallbackStopped is returned when the IterateMappings callback returns +// false, signaling that iteration was intentionally interrupted. +var ErrCallbackStopped = errors.New("iteration stopped by callback") + const ( containerSource = "[0-9a-f]{64}" taskSource = "[0-9a-f]{32}-\\d+" @@ -189,7 +193,7 @@ func trimMappingPath(path string) string { return path } -func iterateMappings(mapsFile io.Reader, callback func(m RawMapping) bool) (uint32, error) { +func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, error) { numParseErrors := uint32(0) scanner := bufio.NewScanner(mapsFile) scanBuf := bufPool.Get().(*[]byte) @@ -210,10 +214,10 @@ func iterateMappings(mapsFile io.Reader, callback func(m RawMapping) bool) (uint var addrs [2]string var devs [2]string - // WARNING: line (and all substrings derived from it, including - // the Path field of the emitted RawMapping) points into scanBuf - // which is recycled after iteration. Callers must use - // m.ToMapping() to produce a safe copy before storing. + // WARNING: line (and all substrings derived from it, including the + // Path field of the emitted Mapping) points into scanBuf which is + // recycled after iteration. Callers must copy Path (strings.Clone) + // or intern it (libpf.Intern) before storing. line := pfunsafe.ToString(scanner.Bytes()) if stringutil.FieldsN(line, fields[:]) < 5 { numParseErrors++ @@ -273,7 +277,7 @@ func iterateMappings(mapsFile io.Reader, callback func(m RawMapping) bool) (uint if inode == 0 { if fields[5] == "[vdso]" { // Map to something filename looking with synthesized inode - path = VdsoPathName.String() + path = VdsoPathName device = 0 inode = vdsoInode } else if fields[5] == "" { @@ -307,7 +311,7 @@ func iterateMappings(mapsFile io.Reader, callback func(m RawMapping) bool) (uint continue } - if !callback(RawMapping{ + if !callback(Mapping{ Vaddr: vaddr, Length: length, Flags: flags, @@ -316,31 +320,28 @@ func iterateMappings(mapsFile io.Reader, callback func(m RawMapping) bool) (uint Inode: inode, Path: path, }) { - return numParseErrors, scanner.Err() + return numParseErrors, ErrCallbackStopped } } return numParseErrors, scanner.Err() } -// IterateMappings parses process memory mappings and calls callback -// for each mapping. The callback receives a RawMapping whose Path -// may reference an internal buffer recycled after iteration; use -// ToMapping() to produce a Mapping safe to store long-term. -// The callback is responsible for filtering out unwanted mappings. -func (sp *systemProcess) IterateMappings(callback func(m RawMapping) bool) (uint32, error) { +func (sp *systemProcess) IterateMappings(callback func(m Mapping) bool) (uint32, error) { mapsFile, err := os.Open(fmt.Sprintf("/proc/%d/maps", sp.pid)) if err != nil { return 0, err } defer mapsFile.Close() - var elfMappings []Mapping + fileToMapping := make(map[string]*Mapping) gotMappings := false - collectForOpenELF := func(m RawMapping) bool { + collectForOpenELF := func(m Mapping) bool { gotMappings = true if m.IsExecutable() || m.IsVDSO() { - elfMappings = append(elfMappings, m.ToMapping()) + stored := m + stored.Path = strings.Clone(m.Path) + fileToMapping[stored.Path] = &stored } return callback(m) } @@ -381,13 +382,7 @@ func (sp *systemProcess) IterateMappings(callback func(m RawMapping) bool) (uint } } - fileToMapping := make(map[string]*Mapping, len(elfMappings)) - for i := range elfMappings { - m := &elfMappings[i] - fileToMapping[m.Path.String()] = m - } sp.fileToMapping = fileToMapping - return numParseErrors, nil } @@ -422,7 +417,7 @@ func (sp *systemProcess) getMappingFile(m *Mapping) string { // nor /proc/sp.pid/root exist if main thread has exited, so we use the // mapping path directly under the sp.tid root. rootPath := fmt.Sprintf("/proc/%v/task/%v/root", sp.pid, sp.tid) - return path.Join(rootPath, m.Path.String()) + return path.Join(rootPath, m.Path) } return fmt.Sprintf("/proc/%v/map_files/%x-%x", sp.pid, m.Vaddr, m.Vaddr+m.Length) } diff --git a/process/process_test.go b/process/process_test.go index cd633655b..2d3fdf6cc 100644 --- a/process/process_test.go +++ b/process/process_test.go @@ -39,7 +39,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x2c000, FileOffset: 0, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x55fe8273c000, @@ -48,7 +48,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x82000, FileOffset: 0x2c000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x55fe827be000, @@ -57,7 +57,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x78000, FileOffset: 0xae000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x55fe82836000, @@ -66,7 +66,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x7000, FileOffset: 0x125000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x55fe8283d000, @@ -75,7 +75,7 @@ var allExpectedMappings = []Mapping{ Inode: 1068432, Length: 0x1000, FileOffset: 0x12c000, - Path: libpf.Intern("/tmp/usr_bin_seahorse"), + Path: "/tmp/usr_bin_seahorse", }, { Vaddr: 0x7f63c8c3e000, @@ -84,7 +84,7 @@ var allExpectedMappings = []Mapping{ Inode: 1048922, Length: 0x1A2000, FileOffset: 544768, - Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libcrypto.so.1.1"), + Path: "/tmp/usr_lib_x86_64-linux-gnu_libcrypto.so.1.1", }, { Vaddr: 0x7f63c8ebf000, @@ -93,7 +93,7 @@ var allExpectedMappings = []Mapping{ Inode: 1075944, Length: 0x130000, FileOffset: 114688, - Path: libpf.Intern("/tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0"), + Path: "/tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0", }, { Vaddr: 0x7f8b929f0000, @@ -102,7 +102,7 @@ var allExpectedMappings = []Mapping{ Inode: 0, Length: 0x10000, FileOffset: 0, - Path: libpf.NullString, + Path: "", }, } @@ -110,12 +110,11 @@ func getTestMappings(t *testing.T, mapsFile io.Reader) ([]Mapping, uint32, error t.Helper() mappings := make([]Mapping, 0, 32) - collectAll := func(m RawMapping) bool { - mappings = append(mappings, m.ToMapping()) + numParseErrors, err := iterateMappings(mapsFile, func(m Mapping) bool { + m.Path = strings.Clone(m.Path) + mappings = append(mappings, m) return true - } - - numParseErrors, err := iterateMappings(mapsFile, collectAll) + }) return mappings, numParseErrors, err } @@ -123,12 +122,11 @@ func getTestMappingsFromProcess(t *testing.T, process Process) ([]Mapping, uint3 t.Helper() mappings := make([]Mapping, 0, 32) - collectAll := func(m RawMapping) bool { - mappings = append(mappings, m.ToMapping()) + numParseErrors, err := process.IterateMappings(func(m Mapping) bool { + m.Path = strings.Clone(m.Path) + mappings = append(mappings, m) return true - } - - numParseErrors, err := process.IterateMappings(collectAll) + }) return mappings, numParseErrors, err } @@ -139,31 +137,6 @@ func TestParseMappings(t *testing.T) { assert.Equal(t, allExpectedMappings, mappings) } -func TestRawMappingPredicates(t *testing.T) { - tests := []struct { - name string - m RawMapping - wantAnon bool - wantFile bool - wantMemFD bool - wantVDSO bool - }{ - {"anonymous", RawMapping{}, true, false, false, false}, - {"file-backed", RawMapping{Path: "/usr/lib/foo.so"}, false, true, false, false}, - {"memfd", RawMapping{Path: "/memfd:jit"}, true, false, true, false}, - {"vdso", RawMapping{Path: VdsoPathName.String()}, false, false, false, true}, - {"/dev/zero normalized", RawMapping{Inode: 42, Device: 1}, true, false, false, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.wantAnon, tt.m.IsAnonymous(), "IsAnonymous") - assert.Equal(t, tt.wantFile, tt.m.IsFileBacked(), "IsFileBacked") - assert.Equal(t, tt.wantMemFD, tt.m.IsMemFD(), "IsMemFD") - assert.Equal(t, tt.wantVDSO, tt.m.IsVDSO(), "IsVDSO") - }) - } -} - func TestMappingPredicates(t *testing.T) { tests := []struct { name string @@ -174,8 +147,8 @@ func TestMappingPredicates(t *testing.T) { wantVDSO bool }{ {"anonymous", Mapping{}, true, false, false, false}, - {"file-backed", Mapping{Path: libpf.Intern("/usr/lib/foo.so")}, false, true, false, false}, - {"memfd", Mapping{Path: libpf.Intern("/memfd:jit")}, true, false, true, false}, + {"file-backed", Mapping{Path: "/usr/lib/foo.so"}, false, true, false, false}, + {"memfd", Mapping{Path: "/memfd:jit"}, true, false, true, false}, {"vdso", Mapping{Path: VdsoPathName}, false, false, false, true}, {"/dev/zero normalized", Mapping{Inode: 42, Device: 1}, true, false, false, false}, } @@ -189,23 +162,6 @@ func TestMappingPredicates(t *testing.T) { } } -func TestToMapping(t *testing.T) { - raw := RawMapping{ - Vaddr: 0x1000, - Length: 0x2000, - Flags: elf.PF_R + elf.PF_X, - Path: "/usr/lib/foo.so", - Device: 1, - Inode: 42, - } - m := raw.ToMapping() - assert.Equal(t, libpf.Intern("/usr/lib/foo.so"), m.Path) - assert.Equal(t, raw.Vaddr, m.Vaddr) - assert.Equal(t, raw.Length, m.Length) - assert.True(t, m.IsFileBacked()) - assert.True(t, m.IsExecutable()) -} - func TestNewPIDOfSelf(t *testing.T) { if runtime.GOOS != "linux" { t.Skipf("unsupported os %s", runtime.GOOS) diff --git a/process/types.go b/process/types.go index bcbfa125f..fdf19cfaa 100644 --- a/process/types.go +++ b/process/types.go @@ -17,17 +17,19 @@ import ( ) // VdsoPathName is the path to use for VDSO mappings. -var VdsoPathName = libpf.Intern("linux-vdso.1.so") +const VdsoPathName = "linux-vdso.1.so" // vdsoInode is the synthesized inode number for VDSO mappings. const vdsoInode = 50 -// RawMapping is the ephemeral representation of a memory mapping as parsed -// from /proc/pid/maps. Path is a plain string that may reference the -// parser's internal buffer and must not be stored beyond the -// IterateMappings callback. Use ToMapping() to produce a Mapping with -// an interned Path that is safe to store long-term. -type RawMapping struct { +// Mapping represents a memory mapping parsed from /proc/pid/maps or a coredump. +// +// WARNING: When produced by the systemProcess IterateMappings implementation, +// Path may reference an internal scanner buffer that is recycled after the +// iteration completes. Callers that need to store the mapping beyond the +// callback scope must copy the Path (e.g. via strings.Clone). For long-lived +// references where deduplication matters, prefer libpf.Intern instead. +type Mapping struct { // Vaddr is the virtual memory start for this mapping. Vaddr uint64 // Length is the length of the mapping. @@ -41,63 +43,13 @@ type RawMapping struct { // Inode holds the mapped file's inode number. Inode uint64 // Path is the file path for file-backed and special mappings. - // May reference an internal buffer recycled after iteration. + // When received from IterateMappings, this may point into an internal + // buffer. The caller is responsible for copying it (e.g. strings.Clone) + // before storing the mapping long-term. For long-lived references where + // deduplication matters, prefer libpf.Intern over strings.Clone. Path string } -func (m *RawMapping) IsExecutable() bool { - return m.Flags&elf.PF_X == elf.PF_X -} - -func (m *RawMapping) IsAnonymous() bool { - return !m.IsFileBacked() && !m.IsVDSO() -} - -func (m *RawMapping) IsFileBacked() bool { - return m.Path != "" && !m.IsVDSO() && !m.IsMemFD() -} - -func (m *RawMapping) IsMemFD() bool { - return strings.HasPrefix(m.Path, "/memfd:") -} - -func (m *RawMapping) IsVDSO() bool { - return m.Path == VdsoPathName.String() -} - -// ToMapping converts to a Mapping with an interned Path. Only call this -// for mappings you intend to keep. -func (m *RawMapping) ToMapping() Mapping { - return Mapping{ - Vaddr: m.Vaddr, - Length: m.Length, - Flags: m.Flags, - FileOffset: m.FileOffset, - Device: m.Device, - Inode: m.Inode, - Path: libpf.Intern(m.Path), - } -} - -// Mapping is the stable representation of a memory mapping with an -// interned Path. Produced by RawMapping.ToMapping() after filtering. -type Mapping struct { - // Vaddr is the virtual memory start for this mapping. - Vaddr uint64 - // Length is the length of the mapping. - Length uint64 - // Flags contains the mapping flags and permissions. - Flags elf.ProgFlag - // FileOffset contains for file backed mappings the offset from the file start. - FileOffset uint64 - // Device holds the device ID where the file is located. - Device uint64 - // Inode holds the mapped file's inode number. - Inode uint64 - // Path is the interned file path for file-backed and special mappings. - Path libpf.String -} - func (m *Mapping) IsExecutable() bool { return m.Flags&elf.PF_X == elf.PF_X } @@ -107,11 +59,11 @@ func (m *Mapping) IsAnonymous() bool { } func (m *Mapping) IsFileBacked() bool { - return m.Path != libpf.NullString && !m.IsVDSO() && !m.IsMemFD() + return m.Path != "" && !m.IsVDSO() && !m.IsMemFD() } func (m *Mapping) IsMemFD() bool { - return strings.HasPrefix(m.Path.String(), "/memfd:") + return strings.HasPrefix(m.Path, "/memfd:") } func (m *Mapping) IsVDSO() bool { @@ -186,12 +138,14 @@ type Process interface { // GetExe returns the executable path of the process. GetExe() (libpf.String, error) - // IterateMappings parses process memory mappings and calls callback - // for each mapping. The callback receives a RawMapping whose Path - // may reference an internal buffer recycled after iteration; use - // ToMapping() to produce a Mapping safe to store long-term. - // The callback is responsible for filtering out unwanted mappings. - IterateMappings(callback func(m RawMapping) bool) (uint32, error) + // IterateMappings parses process memory mappings and calls the + // callback for each mapping. The Mapping's Path field may reference + // an internal buffer that is recycled after the iteration completes; + // callers must copy the Path (e.g. strings.Clone, or libpf.Intern for + // long-lived references needing deduplication) before storing the + // mapping beyond the callback scope. Returning false from the callback + // stops iteration and causes ErrCallbackStopped to be returned. + IterateMappings(callback func(m Mapping) bool) (uint32, error) // GetThreads reads the process thread states. GetThreads() ([]ThreadInfo, error) diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index d6de937f0..9683e9d91 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -225,7 +225,7 @@ func (pm *ProcessManager) getELFInfo(pr process.Process, mapping *process.Mappin return info } - baseName := path.Base(mapping.Path.String()) + baseName := path.Base(mapping.Path) if baseName == "/" { // There are circumstances where there is no filename. // E.g. kernel module 'bpfilter_umh' before Linux 5.9-rc1 uses @@ -333,7 +333,7 @@ func (pm *ProcessManager) processRemovedInterpreters(pid libpf.PID, var errInvalidVirtualAddress = errors.New("invalid ELF virtual address") func (pm *ProcessManager) newFrameMapping(pr process.Process, m *process.Mapping) (libpf.FrameMapping, error) { - elfRef := pfelf.NewReference(m.Path.String(), pr) + elfRef := pfelf.NewReference(m.Path, pr) defer elfRef.Close() info := pm.getELFInfo(pr, m, elfRef) @@ -503,6 +503,9 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { mpRemove[uint64(m.Vaddr)] = m } + // interpreterMappings collects the subset of mappings relevant to interpreters: + // executable anonymous mappings (JIT) and .dll file-backed mappings (.NET PE). + // They are in /proc/PID/maps order (ascending Vaddr), not sorted otherwise. interpreterMappings := make([]process.Mapping, 0, 8) interpretersValid := make(libpf.Set[util.OnDiskFileIdentifier], numInterpreters) capHint := max(32, min(len(oldMappings), 256)) @@ -512,11 +515,22 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { pm.mappingStats.numProcAttempts.Add(1) start := time.Now() - numParseErrors, err := pr.IterateMappings(func(raw process.RawMapping) bool { - // executable mappings and VDSO converted directly to libpf.FrameMapping - if raw.IsExecutable() && !raw.IsAnonymous() { - m := raw.ToMapping() + numParseErrors, err := pr.IterateMappings(func(m process.Mapping) bool { + // Executable mappings and VDSO, converted directly to libpf.FrameMapping + mappingNeeded := m.IsExecutable() && !m.IsAnonymous() + // Needed for JIT mappings (Hotspot, V8, BEAM, etc.) + interpreterNeeded := m.IsExecutable() && m.IsAnonymous() + // Needed by .NET to retrieve PE assembly mappings + interpreterNeeded = interpreterNeeded || strings.HasSuffix(m.Path, ".dll") + if !mappingNeeded && !interpreterNeeded { + return true + } + if m.Path != "" { + m.Path = strings.Clone(m.Path) + } + + if mappingNeeded { var fm libpf.FrameMapping if oldm, ok := mpRemove[m.Vaddr]; ok { if oldm.Length == m.Length && oldm.Device == m.Device && oldm.Inode == m.Inode { @@ -548,14 +562,8 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { } } - // Interpreters specific mappings - // Needed by V8 and BEAM to retrieve JIT mappings - if raw.IsExecutable() && raw.IsAnonymous() { - interpreterMappings = append(interpreterMappings, raw.ToMapping()) - } - // Needed by .NET to retrieve PE assembly mappings - if !raw.IsAnonymous() && strings.HasSuffix(raw.Path, ".dll") { - interpreterMappings = append(interpreterMappings, raw.ToMapping()) + if interpreterNeeded { + interpreterMappings = append(interpreterMappings, m) } return true }) @@ -564,16 +572,18 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { pm.mappingStats.numProcParseErrors.Add(numParseErrors) if err != nil { - if os.IsPermission(err) { + if errors.Is(err, process.ErrCallbackStopped) { + // Defensive: the current callback does not stop early, but the + // IterateMappings contract allows it. Treat as non-fatal. + err = nil + } else if os.IsPermission(err) { // Ignore the synchronization completely in case of permission // error. This implies the process is still alive, but we cannot // inspect it. Exiting here keeps the PID in the eBPF maps so // we avoid a notification flood to resynchronize. pm.mappingStats.errProcPerm.Add(1) return - } - - if errors.Is(err, process.ErrNoMappings) { + } else if errors.Is(err, process.ErrNoMappings) { // When no mappings can be extracted but the process is still alive, // do not trigger a process exit to avoid unloading process metadata. // As it's likely that a future iteration can extract mappings from a @@ -581,7 +591,9 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { pm.ebpf.RemoveReportedPID(pid) return } + } + if err != nil { // All other errors imply that the process has exited. if os.IsNotExist(err) { // Since listing /proc and opening files in there later is inherently racy, diff --git a/tools/coredump/new.go b/tools/coredump/new.go index 73b382d82..c5087eac2 100644 --- a/tools/coredump/new.go +++ b/tools/coredump/new.go @@ -73,7 +73,7 @@ func (tc *trackedCoredump) warnMissing(fileName string) { func (tc *trackedCoredump) CalculateMappingFileID(m *process.Mapping) (libpf.FileID, error) { if !m.IsVDSO() && !m.IsAnonymous() { - file := m.Path.String() + file := m.Path fid, err := libpf.FileIDFromExecutableFile(path.Join(tc.prefix, file)) if err == nil { tc.seen[file] = libpf.Void{} @@ -86,7 +86,7 @@ func (tc *trackedCoredump) CalculateMappingFileID(m *process.Mapping) (libpf.Fil func (tc *trackedCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCloser, error) { if !m.IsVDSO() && !m.IsAnonymous() { - file := m.Path.String() + file := m.Path rac, err := os.Open(path.Join(tc.prefix, file)) if err == nil { tc.seen[file] = libpf.Void{} @@ -98,7 +98,7 @@ func (tc *trackedCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCl } func (tc *trackedCoredump) OpenELF(fileName string) (*pfelf.File, error) { - if fileName != process.VdsoPathName.String() { + if fileName != process.VdsoPathName { f, err := pfelf.Open(path.Join(tc.prefix, fileName)) if err == nil { tc.seen[fileName] = libpf.Void{} diff --git a/tools/coredump/storecoredump.go b/tools/coredump/storecoredump.go index 954c700a1..2c3149f0a 100644 --- a/tools/coredump/storecoredump.go +++ b/tools/coredump/storecoredump.go @@ -46,7 +46,7 @@ func (scd *StoreCoredump) openFile(path string) (process.ReadAtCloser, error) { } func (scd *StoreCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCloser, error) { - return scd.openFile(m.Path.String()) + return scd.openFile(m.Path) } func (scd *StoreCoredump) OpenELF(path string) (*pfelf.File, error) { From 8dd2dcc3c961d0b8abc20d60443d4d2418b8febd Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Thu, 19 Mar 2026 13:58:27 +0000 Subject: [PATCH 11/15] fix: remove uneeded guard --- processmanager/processinfo.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 9683e9d91..4c50c541a 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -526,9 +526,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { return true } - if m.Path != "" { - m.Path = strings.Clone(m.Path) - } + m.Path = strings.Clone(m.Path) if mappingNeeded { var fm libpf.FrameMapping From fcf0cb78b189325553c3945154225e36fbb2777a Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Fri, 20 Mar 2026 13:48:42 +0000 Subject: [PATCH 12/15] rename process.Mappings to RawMappings and replace Clone by Intern --- interpreter/beam/beam.go | 2 +- interpreter/dotnet/instance.go | 4 ++-- interpreter/dotnet/pe.go | 2 +- interpreter/hotspot/instance.go | 2 +- interpreter/instancestubs.go | 2 +- interpreter/multi.go | 2 +- interpreter/nodev8/v8.go | 8 +++---- interpreter/php/opcache.go | 2 +- interpreter/types.go | 2 +- process/coredump.go | 16 +++++++------- process/process.go | 30 +++++++++++++------------- process/process_test.go | 30 +++++++++++++------------- process/types.go | 38 ++++++++++++++++----------------- processmanager/processinfo.go | 10 ++++----- reporter/iface.go | 4 ++-- tools/coredump/new.go | 6 +++--- tools/coredump/storecoredump.go | 2 +- 17 files changed, 80 insertions(+), 82 deletions(-) diff --git a/interpreter/beam/beam.go b/interpreter/beam/beam.go index ab72f280f..47d392b39 100644 --- a/interpreter/beam/beam.go +++ b/interpreter/beam/beam.go @@ -337,7 +337,7 @@ func (d *beamData) Attach(ebpf interpreter.EbpfHandler, pid libpf.PID, bias libp func (d *beamData) Unload(_ interpreter.EbpfHandler) { } -func (i *beamInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler, _ reporter.ExecutableReporter, pr process.Process, mappings []process.Mapping) error { +func (i *beamInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler, _ reporter.ExecutableReporter, pr process.Process, mappings []process.RawMapping) error { pid := pr.PID() i.mappingGeneration++ for idx := range mappings { diff --git a/interpreter/dotnet/instance.go b/interpreter/dotnet/instance.go index de9d9c673..0da42883f 100644 --- a/interpreter/dotnet/instance.go +++ b/interpreter/dotnet/instance.go @@ -141,7 +141,7 @@ type dotnetInstance struct { virtualCallStubManagerManagerPtr libpf.Address // mappings contains the PE mappings to process memory space. Multiple individual - // consecutive process.Mappings may be merged to one mapping per PE file. + // consecutive process.RawMappings may be merged to one mapping per PE file. mappings []dotnetMapping ranges map[libpf.Address]dotnetRangeSection @@ -552,7 +552,7 @@ func (i *dotnetInstance) getDacSlotPtr(slot uint) libpf.Address { func (i *dotnetInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler, exeReporter reporter.ExecutableReporter, pr process.Process, - mappings []process.Mapping, + mappings []process.RawMapping, ) error { // get introspection data cdac, err := i.d.GetOrInit(func() (dotnetCdac, error) { return i.d.newVMData(i.rm, i.bias) }) diff --git a/interpreter/dotnet/pe.go b/interpreter/dotnet/pe.go index 824bc936f..fde83feb4 100644 --- a/interpreter/dotnet/pe.go +++ b/interpreter/dotnet/pe.go @@ -1209,7 +1209,7 @@ func (pc *peCache) init() { pc.peInfoCache = peInfoCache } -func (pc *peCache) Get(pr process.Process, mapping *process.Mapping) *peInfo { +func (pc *peCache) Get(pr process.Process, mapping *process.RawMapping) *peInfo { key := mapping.GetOnDiskFileIdentifier() lastModified := pr.GetMappingFileLastModified(mapping) if info, ok := pc.peInfoCache.Get(key); ok && info.lastModified == lastModified { diff --git a/interpreter/hotspot/instance.go b/interpreter/hotspot/instance.go index ce0802f68..e116b772e 100644 --- a/interpreter/hotspot/instance.go +++ b/interpreter/hotspot/instance.go @@ -854,7 +854,7 @@ func (d *hotspotInstance) updateStubMappings(vmd *hotspotVMData, } func (d *hotspotInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler, - _ reporter.ExecutableReporter, pr process.Process, _ []process.Mapping, + _ reporter.ExecutableReporter, pr process.Process, _ []process.RawMapping, ) error { vmd, err := d.d.GetOrInit(func() (hotspotVMData, error) { return d.d.newVMData(d.rm, d.bias) }) if err != nil { diff --git a/interpreter/instancestubs.go b/interpreter/instancestubs.go index 662eb74d8..1eca53dea 100644 --- a/interpreter/instancestubs.go +++ b/interpreter/instancestubs.go @@ -17,7 +17,7 @@ type InstanceStubs struct { } func (is *InstanceStubs) SynchronizeMappings(EbpfHandler, reporter.ExecutableReporter, - process.Process, []process.Mapping) error { + process.Process, []process.RawMapping) error { return nil } diff --git a/interpreter/multi.go b/interpreter/multi.go index 08634384b..d976228d2 100644 --- a/interpreter/multi.go +++ b/interpreter/multi.go @@ -92,7 +92,7 @@ func (m *MultiInstance) Detach(ebpf EbpfHandler, pid libpf.PID) error { // SynchronizeMappings synchronizes mappings for all interpreter instances. func (m *MultiInstance) SynchronizeMappings(ebpf EbpfHandler, - exeReporter reporter.ExecutableReporter, pr process.Process, mappings []process.Mapping, + exeReporter reporter.ExecutableReporter, pr process.Process, mappings []process.RawMapping, ) error { var errs []error for _, instance := range m.instances { diff --git a/interpreter/nodev8/v8.go b/interpreter/nodev8/v8.go index 12cffa30a..62d9a8e6c 100644 --- a/interpreter/nodev8/v8.go +++ b/interpreter/nodev8/v8.go @@ -500,8 +500,8 @@ type v8Instance struct { addrToSource *freelru.LRU[libpf.Address, *v8Source] addrToType *freelru.LRU[libpf.Address, uint16] - // mappings is indexed by the Mapping to its generation - mappings map[process.Mapping]*uint32 + // mappings is indexed by the RawMapping to its generation + mappings map[process.RawMapping]*uint32 // prefixes is indexed by the prefix added to ebpf maps (to be cleaned up) to its generation prefixes map[lpm.Prefix]*uint32 // mappingGeneration is the current generation (so old entries can be pruned) @@ -555,7 +555,7 @@ func (i *v8Instance) Detach(ebpf interpreter.EbpfHandler, pid libpf.PID) error { } func (i *v8Instance) SynchronizeMappings(ebpf interpreter.EbpfHandler, - _ reporter.ExecutableReporter, pr process.Process, mappings []process.Mapping, + _ reporter.ExecutableReporter, pr process.Process, mappings []process.RawMapping, ) error { pid := pr.PID() i.mappingGeneration++ @@ -1841,7 +1841,7 @@ func (d *v8Data) Attach(ebpf interpreter.EbpfHandler, pid libpf.PID, _ libpf.Add return &v8Instance{ d: d, rm: rm, - mappings: make(map[process.Mapping]*uint32), + mappings: make(map[process.RawMapping]*uint32), prefixes: make(map[lpm.Prefix]*uint32), addrToString: addrToString, addrToCode: addrToCode, diff --git a/interpreter/php/opcache.go b/interpreter/php/opcache.go index 380e05120..2e8f5afdb 100644 --- a/interpreter/php/opcache.go +++ b/interpreter/php/opcache.go @@ -193,7 +193,7 @@ func (i *opcacheInstance) Detach(ebpf interpreter.EbpfHandler, pid libpf.PID) er } func (i *opcacheInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler, - _ reporter.ExecutableReporter, pr process.Process, _ []process.Mapping, + _ reporter.ExecutableReporter, pr process.Process, _ []process.RawMapping, ) error { if i.prefixes != nil { // Already attached diff --git a/interpreter/types.go b/interpreter/types.go index cfa055c44..aae0537cb 100644 --- a/interpreter/types.go +++ b/interpreter/types.go @@ -151,7 +151,7 @@ type Instance interface { // Interpreters not needing to process these events can simply ignore them // by returning nil. SynchronizeMappings(ebpf EbpfHandler, exeReporter reporter.ExecutableReporter, - pr process.Process, mappings []process.Mapping) error + pr process.Process, mappings []process.RawMapping) error // UpdateLibcInfo is called when the process C-library related // introspection data has been updated. diff --git a/process/coredump.go b/process/coredump.go index 349f74389..053fa7ccb 100644 --- a/process/coredump.go +++ b/process/coredump.go @@ -46,7 +46,7 @@ type CoredumpProcess struct { machineData MachineData // mappings contains the parsed mappings. - mappings []Mapping + mappings []RawMapping // threadInfo contains the parsed thread info. threadInfo []ThreadInfo @@ -145,7 +145,7 @@ func OpenCoredumpFile(f *pfelf.File) (*CoredumpProcess, error) { cd := &CoredumpProcess{ File: f, files: make(map[string]*CoredumpFile), - mappings: make([]Mapping, 0, len(f.Progs)), + mappings: make([]RawMapping, 0, len(f.Progs)), threadInfo: make([]ThreadInfo, 0, 8), } cd.machineData.Machine = cd.Machine @@ -157,7 +157,7 @@ func OpenCoredumpFile(f *pfelf.File) (*CoredumpProcess, error) { for i := range f.Progs { p := &f.Progs[i] if p.Type == elf.PT_LOAD && p.Flags != 0 { - m := Mapping{ + m := RawMapping{ Vaddr: p.Vaddr, Length: p.Memsz, Flags: p.Flags, @@ -270,7 +270,7 @@ func (cd *CoredumpProcess) GetExe() (libpf.String, error) { } // IterateMappings implements the Process interface. -func (cd *CoredumpProcess) IterateMappings(callback func(m Mapping) bool) (uint32, error) { +func (cd *CoredumpProcess) IterateMappings(callback func(m RawMapping) bool) (uint32, error) { for _, m := range cd.mappings { if !callback(m) { return 0, ErrCallbackStopped @@ -285,18 +285,18 @@ func (cd *CoredumpProcess) GetThreads() ([]ThreadInfo, error) { } // OpenMappingFile implements the Process interface. -func (cd *CoredumpProcess) OpenMappingFile(_ *Mapping) (ReadAtCloser, error) { +func (cd *CoredumpProcess) OpenMappingFile(_ *RawMapping) (ReadAtCloser, error) { // Coredumps do not contain the original backing files. return nil, errors.New("coredump does not support opening backing file") } // GetMappingFileLastModified implements the Process interface. -func (cd *CoredumpProcess) GetMappingFileLastModified(_ *Mapping) int64 { +func (cd *CoredumpProcess) GetMappingFileLastModified(_ *RawMapping) int64 { return 0 } // CalculateMappingFileID implements the Process interface. -func (cd *CoredumpProcess) CalculateMappingFileID(m *Mapping) (libpf.FileID, error) { +func (cd *CoredumpProcess) CalculateMappingFileID(m *RawMapping) (libpf.FileID, error) { // It is not possible to calculate the real FileID as the section headers // are likely missing. So just return a synthesized FileID. vaddr := make([]byte, 8) @@ -413,7 +413,7 @@ func (cd *CoredumpProcess) parseMappings(desc []byte, // This file backed mapping is not in the coredump LOAD tables // Likely a executable mapping excluded by core_filter. Construct // the mappings assuming R+X. - cd.mappings = append(cd.mappings, Mapping{ + cd.mappings = append(cd.mappings, RawMapping{ Vaddr: entry.Start, Length: entry.End - entry.Start, Flags: elf.PF_R + elf.PF_X, diff --git a/process/process.go b/process/process.go index a7aafbcc1..d2c59c452 100644 --- a/process/process.go +++ b/process/process.go @@ -59,7 +59,7 @@ type systemProcess struct { mainThreadExit bool remoteMemory remotememory.RemoteMemory - fileToMapping map[string]*Mapping + fileToMapping map[string]*RawMapping } var _ Process = &systemProcess{} @@ -193,7 +193,7 @@ func trimMappingPath(path string) string { return path } -func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, error) { +func iterateMappings(mapsFile io.Reader, callback func(m RawMapping) bool) (uint32, error) { numParseErrors := uint32(0) scanner := bufio.NewScanner(mapsFile) scanBuf := bufPool.Get().(*[]byte) @@ -215,9 +215,9 @@ func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, var devs [2]string // WARNING: line (and all substrings derived from it, including the - // Path field of the emitted Mapping) points into scanBuf which is - // recycled after iteration. Callers must copy Path (strings.Clone) - // or intern it (libpf.Intern) before storing. + // Path field of the emitted RawMapping) points into scanBuf which is + // recycled after iteration. Callers must intern Path (libpf.Intern) + // before storing. line := pfunsafe.ToString(scanner.Bytes()) if stringutil.FieldsN(line, fields[:]) < 5 { numParseErrors++ @@ -311,7 +311,7 @@ func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, continue } - if !callback(Mapping{ + if !callback(RawMapping{ Vaddr: vaddr, Length: length, Flags: flags, @@ -326,21 +326,21 @@ func iterateMappings(mapsFile io.Reader, callback func(m Mapping) bool) (uint32, return numParseErrors, scanner.Err() } -func (sp *systemProcess) IterateMappings(callback func(m Mapping) bool) (uint32, error) { +func (sp *systemProcess) IterateMappings(callback func(m RawMapping) bool) (uint32, error) { mapsFile, err := os.Open(fmt.Sprintf("/proc/%d/maps", sp.pid)) if err != nil { return 0, err } defer mapsFile.Close() - fileToMapping := make(map[string]*Mapping) + fileToMapping := make(map[string]*RawMapping) gotMappings := false - collectForOpenELF := func(m Mapping) bool { + collectForOpenELF := func(m RawMapping) bool { gotMappings = true if m.IsExecutable() || m.IsVDSO() { stored := m - stored.Path = strings.Clone(m.Path) + stored.Path = libpf.Intern(m.Path).String() fileToMapping[stored.Path] = &stored } return callback(m) @@ -398,7 +398,7 @@ func (sp *systemProcess) GetRemoteMemory() remotememory.RemoteMemory { return sp.remoteMemory } -func (sp *systemProcess) extractMapping(m *Mapping) (*bytes.Reader, error) { +func (sp *systemProcess) extractMapping(m *RawMapping) (*bytes.Reader, error) { data := make([]byte, m.Length) _, err := sp.remoteMemory.ReadAt(data, int64(m.Vaddr)) if err != nil { @@ -408,7 +408,7 @@ func (sp *systemProcess) extractMapping(m *Mapping) (*bytes.Reader, error) { return bytes.NewReader(data), nil } -func (sp *systemProcess) getMappingFile(m *Mapping) string { +func (sp *systemProcess) getMappingFile(m *RawMapping) string { if m.IsAnonymous() || m.IsVDSO() { return "" } @@ -422,7 +422,7 @@ func (sp *systemProcess) getMappingFile(m *Mapping) string { return fmt.Sprintf("/proc/%v/map_files/%x-%x", sp.pid, m.Vaddr, m.Vaddr+m.Length) } -func (sp *systemProcess) OpenMappingFile(m *Mapping) (ReadAtCloser, error) { +func (sp *systemProcess) OpenMappingFile(m *RawMapping) (ReadAtCloser, error) { filename := sp.getMappingFile(m) if filename == "" { return nil, errors.New("no backing file for anonymous memory") @@ -430,7 +430,7 @@ func (sp *systemProcess) OpenMappingFile(m *Mapping) (ReadAtCloser, error) { return os.Open(filename) } -func (sp *systemProcess) GetMappingFileLastModified(m *Mapping) int64 { +func (sp *systemProcess) GetMappingFileLastModified(m *RawMapping) int64 { filename := sp.getMappingFile(m) if filename != "" { var st unix.Stat_t @@ -445,7 +445,7 @@ func (sp *systemProcess) GetMappingFileLastModified(m *Mapping) int64 { // VDSO for the system. var vdsoFileID libpf.FileID -func (sp *systemProcess) CalculateMappingFileID(m *Mapping) (libpf.FileID, error) { +func (sp *systemProcess) CalculateMappingFileID(m *RawMapping) (libpf.FileID, error) { if m.IsVDSO() { if vdsoFileID != (libpf.FileID{}) { return vdsoFileID, nil diff --git a/process/process_test.go b/process/process_test.go index 2d3fdf6cc..e67ac2593 100644 --- a/process/process_test.go +++ b/process/process_test.go @@ -31,7 +31,7 @@ var testMappings = `55fe82710000-55fe8273c000 r--p 00000000 fd:01 1068432 7f63c8eef000 r-xp 0001c000 1fd:01 1075944 7f8b929f0000-7f8b92a00000 r-xp 00000000 00:00 0 ` -var allExpectedMappings = []Mapping{ +var allExpectedMappings = []RawMapping{ { Vaddr: 0x55fe82710000, Device: 0xfd01, @@ -106,24 +106,24 @@ var allExpectedMappings = []Mapping{ }, } -func getTestMappings(t *testing.T, mapsFile io.Reader) ([]Mapping, uint32, error) { +func getTestMappings(t *testing.T, mapsFile io.Reader) ([]RawMapping, uint32, error) { t.Helper() - mappings := make([]Mapping, 0, 32) - numParseErrors, err := iterateMappings(mapsFile, func(m Mapping) bool { - m.Path = strings.Clone(m.Path) + mappings := make([]RawMapping, 0, 32) + numParseErrors, err := iterateMappings(mapsFile, func(m RawMapping) bool { + m.Path = libpf.Intern(m.Path).String() mappings = append(mappings, m) return true }) return mappings, numParseErrors, err } -func getTestMappingsFromProcess(t *testing.T, process Process) ([]Mapping, uint32, error) { +func getTestMappingsFromProcess(t *testing.T, process Process) ([]RawMapping, uint32, error) { t.Helper() - mappings := make([]Mapping, 0, 32) - numParseErrors, err := process.IterateMappings(func(m Mapping) bool { - m.Path = strings.Clone(m.Path) + mappings := make([]RawMapping, 0, 32) + numParseErrors, err := process.IterateMappings(func(m RawMapping) bool { + m.Path = libpf.Intern(m.Path).String() mappings = append(mappings, m) return true }) @@ -140,17 +140,17 @@ func TestParseMappings(t *testing.T) { func TestMappingPredicates(t *testing.T) { tests := []struct { name string - m Mapping + m RawMapping wantAnon bool wantFile bool wantMemFD bool wantVDSO bool }{ - {"anonymous", Mapping{}, true, false, false, false}, - {"file-backed", Mapping{Path: "/usr/lib/foo.so"}, false, true, false, false}, - {"memfd", Mapping{Path: "/memfd:jit"}, true, false, true, false}, - {"vdso", Mapping{Path: VdsoPathName}, false, false, false, true}, - {"/dev/zero normalized", Mapping{Inode: 42, Device: 1}, true, false, false, false}, + {"anonymous", RawMapping{}, true, false, false, false}, + {"file-backed", RawMapping{Path: "/usr/lib/foo.so"}, false, true, false, false}, + {"memfd", RawMapping{Path: "/memfd:jit"}, true, false, true, false}, + {"vdso", RawMapping{Path: VdsoPathName}, false, false, false, true}, + {"/dev/zero normalized", RawMapping{Inode: 42, Device: 1}, true, false, false, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/process/types.go b/process/types.go index fdf19cfaa..f838586e9 100644 --- a/process/types.go +++ b/process/types.go @@ -22,14 +22,14 @@ const VdsoPathName = "linux-vdso.1.so" // vdsoInode is the synthesized inode number for VDSO mappings. const vdsoInode = 50 -// Mapping represents a memory mapping parsed from /proc/pid/maps or a coredump. +// RawMapping represents a memory mapping parsed from /proc/pid/maps or a coredump. // // WARNING: When produced by the systemProcess IterateMappings implementation, // Path may reference an internal scanner buffer that is recycled after the // iteration completes. Callers that need to store the mapping beyond the -// callback scope must copy the Path (e.g. via strings.Clone). For long-lived -// references where deduplication matters, prefer libpf.Intern instead. -type Mapping struct { +// callback scope must intern the Path via libpf.Intern to detach it from the +// buffer and deduplicate identical paths across mappings. +type RawMapping struct { // Vaddr is the virtual memory start for this mapping. Vaddr uint64 // Length is the length of the mapping. @@ -44,33 +44,32 @@ type Mapping struct { Inode uint64 // Path is the file path for file-backed and special mappings. // When received from IterateMappings, this may point into an internal - // buffer. The caller is responsible for copying it (e.g. strings.Clone) - // before storing the mapping long-term. For long-lived references where - // deduplication matters, prefer libpf.Intern over strings.Clone. + // buffer. The caller must use libpf.Intern to detach it before storing + // the mapping long-term. Path string } -func (m *Mapping) IsExecutable() bool { +func (m *RawMapping) IsExecutable() bool { return m.Flags&elf.PF_X == elf.PF_X } -func (m *Mapping) IsAnonymous() bool { +func (m *RawMapping) IsAnonymous() bool { return !m.IsFileBacked() && !m.IsVDSO() } -func (m *Mapping) IsFileBacked() bool { +func (m *RawMapping) IsFileBacked() bool { return m.Path != "" && !m.IsVDSO() && !m.IsMemFD() } -func (m *Mapping) IsMemFD() bool { +func (m *RawMapping) IsMemFD() bool { return strings.HasPrefix(m.Path, "/memfd:") } -func (m *Mapping) IsVDSO() bool { +func (m *RawMapping) IsVDSO() bool { return m.Path == VdsoPathName } -func (m *Mapping) GetOnDiskFileIdentifier() util.OnDiskFileIdentifier { +func (m *RawMapping) GetOnDiskFileIdentifier() util.OnDiskFileIdentifier { return util.OnDiskFileIdentifier{ DeviceID: m.Device, InodeNum: m.Inode, @@ -139,13 +138,12 @@ type Process interface { GetExe() (libpf.String, error) // IterateMappings parses process memory mappings and calls the - // callback for each mapping. The Mapping's Path field may reference + // callback for each mapping. The RawMapping's Path field may reference // an internal buffer that is recycled after the iteration completes; - // callers must copy the Path (e.g. strings.Clone, or libpf.Intern for - // long-lived references needing deduplication) before storing the + // callers must use libpf.Intern to detach the Path before storing the // mapping beyond the callback scope. Returning false from the callback // stops iteration and causes ErrCallbackStopped to be returned. - IterateMappings(callback func(m Mapping) bool) (uint32, error) + IterateMappings(callback func(m RawMapping) bool) (uint32, error) // GetThreads reads the process thread states. GetThreads() ([]ThreadInfo, error) @@ -154,14 +152,14 @@ type Process interface { GetRemoteMemory() remotememory.RemoteMemory // OpenMappingFile returns ReadAtCloser accessing the backing file of the mapping. - OpenMappingFile(*Mapping) (ReadAtCloser, error) + OpenMappingFile(*RawMapping) (ReadAtCloser, error) // GetMappingFileLastModifed returns the timestamp when the backing file was last modified // or zero if an error occurs or mapping file is not accessible via filesystem. - GetMappingFileLastModified(*Mapping) int64 + GetMappingFileLastModified(*RawMapping) int64 // CalculateMappingFileID calculates FileID of the backing file. - CalculateMappingFileID(*Mapping) (libpf.FileID, error) + CalculateMappingFileID(*RawMapping) (libpf.FileID, error) io.Closer diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 4c50c541a..182ca2ae4 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -190,7 +190,7 @@ func (pm *ProcessManager) handleNewInterpreter(pr process.Process, bias libpf.Ad return nil } -func (pm *ProcessManager) getELFInfo(pr process.Process, mapping *process.Mapping, +func (pm *ProcessManager) getELFInfo(pr process.Process, mapping *process.RawMapping, elfRef *pfelf.Reference, ) elfInfo { key := mapping.GetOnDiskFileIdentifier() @@ -332,7 +332,7 @@ func (pm *ProcessManager) processRemovedInterpreters(pid libpf.PID, var errInvalidVirtualAddress = errors.New("invalid ELF virtual address") -func (pm *ProcessManager) newFrameMapping(pr process.Process, m *process.Mapping) (libpf.FrameMapping, error) { +func (pm *ProcessManager) newFrameMapping(pr process.Process, m *process.RawMapping) (libpf.FrameMapping, error) { elfRef := pfelf.NewReference(m.Path, pr) defer elfRef.Close() @@ -506,7 +506,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { // interpreterMappings collects the subset of mappings relevant to interpreters: // executable anonymous mappings (JIT) and .dll file-backed mappings (.NET PE). // They are in /proc/PID/maps order (ascending Vaddr), not sorted otherwise. - interpreterMappings := make([]process.Mapping, 0, 8) + interpreterMappings := make([]process.RawMapping, 0, 8) interpretersValid := make(libpf.Set[util.OnDiskFileIdentifier], numInterpreters) capHint := max(32, min(len(oldMappings), 256)) mappings := make([]Mapping, 0, capHint) @@ -515,7 +515,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { pm.mappingStats.numProcAttempts.Add(1) start := time.Now() - numParseErrors, err := pr.IterateMappings(func(m process.Mapping) bool { + numParseErrors, err := pr.IterateMappings(func(m process.RawMapping) bool { // Executable mappings and VDSO, converted directly to libpf.FrameMapping mappingNeeded := m.IsExecutable() && !m.IsAnonymous() // Needed for JIT mappings (Hotspot, V8, BEAM, etc.) @@ -526,7 +526,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { return true } - m.Path = strings.Clone(m.Path) + m.Path = libpf.Intern(m.Path).String() if mappingNeeded { var fm libpf.FrameMapping diff --git a/reporter/iface.go b/reporter/iface.go index d26b3f11e..f5291ef19 100644 --- a/reporter/iface.go +++ b/reporter/iface.go @@ -40,9 +40,9 @@ type ExecutableMetadata struct { // Process is the interface to the process holding the file. Process process.Process - // Mapping is the process.Mapping file. Process.OpenMappingFile can be used + // Mapping is the process.RawMapping file. Process.OpenMappingFile can be used // to open the file if needed. - Mapping *process.Mapping + Mapping *process.RawMapping // DebuglinkFileName is the path to the matching debug file // from the .gnu.debuglink, if any. The caller should diff --git a/tools/coredump/new.go b/tools/coredump/new.go index c5087eac2..3df9499f3 100644 --- a/tools/coredump/new.go +++ b/tools/coredump/new.go @@ -60,7 +60,7 @@ func newTrackedCoredump(corePath, filePrefix string) (*trackedCoredump, error) { }, nil } -func (tc *trackedCoredump) GetMappingFileLastModified(_ *process.Mapping) int64 { +func (tc *trackedCoredump) GetMappingFileLastModified(_ *process.RawMapping) int64 { return 0 } @@ -71,7 +71,7 @@ func (tc *trackedCoredump) warnMissing(fileName string) { } } -func (tc *trackedCoredump) CalculateMappingFileID(m *process.Mapping) (libpf.FileID, error) { +func (tc *trackedCoredump) CalculateMappingFileID(m *process.RawMapping) (libpf.FileID, error) { if !m.IsVDSO() && !m.IsAnonymous() { file := m.Path fid, err := libpf.FileIDFromExecutableFile(path.Join(tc.prefix, file)) @@ -84,7 +84,7 @@ func (tc *trackedCoredump) CalculateMappingFileID(m *process.Mapping) (libpf.Fil return tc.CoredumpProcess.CalculateMappingFileID(m) } -func (tc *trackedCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCloser, error) { +func (tc *trackedCoredump) OpenMappingFile(m *process.RawMapping) (process.ReadAtCloser, error) { if !m.IsVDSO() && !m.IsAnonymous() { file := m.Path rac, err := os.Open(path.Join(tc.prefix, file)) diff --git a/tools/coredump/storecoredump.go b/tools/coredump/storecoredump.go index 2c3149f0a..b1ecef55d 100644 --- a/tools/coredump/storecoredump.go +++ b/tools/coredump/storecoredump.go @@ -45,7 +45,7 @@ func (scd *StoreCoredump) openFile(path string) (process.ReadAtCloser, error) { return file, nil } -func (scd *StoreCoredump) OpenMappingFile(m *process.Mapping) (process.ReadAtCloser, error) { +func (scd *StoreCoredump) OpenMappingFile(m *process.RawMapping) (process.ReadAtCloser, error) { return scd.openFile(m.Path) } From ffbec5c711e54db11e5b4bc47b86b133675326de Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Fri, 27 Mar 2026 08:42:30 +0000 Subject: [PATCH 13/15] apply review --- interpreter/types.go | 2 +- process/process.go | 5 +++-- processmanager/processinfo.go | 5 ++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/interpreter/types.go b/interpreter/types.go index aae0537cb..4b2e219c2 100644 --- a/interpreter/types.go +++ b/interpreter/types.go @@ -140,7 +140,7 @@ type Instance interface { // SynchronizeMappings is called when the processmanager has reread process // memory mappings. The mappings slice contains only the subset of mappings // that are relevant to interpreters: executable anonymous mappings (for JIT - // engines like HotSpot, V8, BEAM) and .dll file-backed mappings (for .NET + // engines like HotSpot, V8, BEAM) and DLL file-backed mappings (for .NET // PE assemblies). The processmanager decides which mappings are included; // this can be made more dynamic in the future if needed. // diff --git a/process/process.go b/process/process.go index d2c59c452..185d5aabb 100644 --- a/process/process.go +++ b/process/process.go @@ -32,7 +32,7 @@ var ErrNoMappings = errors.New("no mappings") // ErrCallbackStopped is returned when the IterateMappings callback returns // false, signaling that iteration was intentionally interrupted. -var ErrCallbackStopped = errors.New("iteration stopped by callback") +var ErrCallbackStopped = errors.New("IterateMappings stopped by callback") const ( containerSource = "[0-9a-f]{64}" @@ -409,7 +409,8 @@ func (sp *systemProcess) extractMapping(m *RawMapping) (*bytes.Reader, error) { } func (sp *systemProcess) getMappingFile(m *RawMapping) string { - if m.IsAnonymous() || m.IsVDSO() { + + if !m.IsFileBacked() { return "" } if sp.mainThreadExit { diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 182ca2ae4..0b7600a16 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -475,7 +475,6 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { if exeErr != nil && !os.IsNotExist(exeErr) { // The /proc/PID/exe returns "not exists" error also in // the case of main thread exit. Ignore it. - log.Warnf("Failed to get executable of process %d: %v", pid, exeErr) } pm.mu.Lock() @@ -504,7 +503,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { } // interpreterMappings collects the subset of mappings relevant to interpreters: - // executable anonymous mappings (JIT) and .dll file-backed mappings (.NET PE). + // executable anonymous mappings (JIT) and DLL file-backed mappings (.NET PE). // They are in /proc/PID/maps order (ascending Vaddr), not sorted otherwise. interpreterMappings := make([]process.RawMapping, 0, 8) interpretersValid := make(libpf.Set[util.OnDiskFileIdentifier], numInterpreters) @@ -539,7 +538,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { newMapping := false if !fm.Valid() { newMapping = true - // Error is expected for non-ELF files (e.g. PE .dll); + // Error is expected for non-ELF files (e.g. PE DLL); // fm will be invalid and the mapping skipped below but will enter the interpreter mappings block. fm, _ = pm.newFrameMapping(pr, &m) } From d5131ea1a7c432e0b2c9a0d36206e0b0920b21b0 Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Fri, 27 Mar 2026 15:36:55 +0000 Subject: [PATCH 14/15] chore: lint --- process/process.go | 1 - 1 file changed, 1 deletion(-) diff --git a/process/process.go b/process/process.go index 185d5aabb..46e341945 100644 --- a/process/process.go +++ b/process/process.go @@ -409,7 +409,6 @@ func (sp *systemProcess) extractMapping(m *RawMapping) (*bytes.Reader, error) { } func (sp *systemProcess) getMappingFile(m *RawMapping) string { - if !m.IsFileBacked() { return "" } From f02222ca1dde57029ae0ad517d49bc8a675833f0 Mon Sep 17 00:00:00 2001 From: Martin Levesque Date: Mon, 30 Mar 2026 08:09:40 +0000 Subject: [PATCH 15/15] refactor: replace if/else statements with a switch block and add comments --- processmanager/processinfo.go | 40 +++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/processmanager/processinfo.go b/processmanager/processinfo.go index 0b7600a16..6df63f09d 100644 --- a/processmanager/processinfo.go +++ b/processmanager/processinfo.go @@ -514,6 +514,9 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { pm.mappingStats.numProcAttempts.Add(1) start := time.Now() + // This callback processes each memory mapping, keeping only executable + // file-backed mappings and anonymous executable/DLL mappings needed by interpreters. + // All other mappings are skipped. numParseErrors, err := pr.IterateMappings(func(m process.RawMapping) bool { // Executable mappings and VDSO, converted directly to libpf.FrameMapping mappingNeeded := m.IsExecutable() && !m.IsAnonymous() @@ -569,42 +572,43 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) { pm.mappingStats.numProcParseErrors.Add(numParseErrors) if err != nil { - if errors.Is(err, process.ErrCallbackStopped) { + switch { + case errors.Is(err, process.ErrCallbackStopped): // Defensive: the current callback does not stop early, but the - // IterateMappings contract allows it. Treat as non-fatal. + // IterateMappings contract allows it. Treat as non-fatal and + // continue with whatever mappings were collected so far. err = nil - } else if os.IsPermission(err) { + case os.IsPermission(err): // Ignore the synchronization completely in case of permission // error. This implies the process is still alive, but we cannot // inspect it. Exiting here keeps the PID in the eBPF maps so // we avoid a notification flood to resynchronize. pm.mappingStats.errProcPerm.Add(1) return - } else if errors.Is(err, process.ErrNoMappings) { + case errors.Is(err, process.ErrNoMappings): // When no mappings can be extracted but the process is still alive, // do not trigger a process exit to avoid unloading process metadata. // As it's likely that a future iteration can extract mappings from a // different thread in the process, notify eBPF to enable further notifications. pm.ebpf.RemoveReportedPID(pid) return - } - } - - if err != nil { - // All other errors imply that the process has exited. - if os.IsNotExist(err) { + case os.IsNotExist(err): // Since listing /proc and opening files in there later is inherently racy, // we expect to lose the race sometimes and thus expect to hit os.IsNotExist. pm.mappingStats.errProcNotExist.Add(1) - } else if e, ok := err.(*os.PathError); ok && e.Err == syscall.ESRCH { - // If the process exits while reading its /proc/$PID/maps, the kernel will - // return ESRCH. Handle it as if the process did not exist. - pm.mappingStats.errProcESRCH.Add(1) + log.Debugf("removing pid due to mappings read error: %v", err) + pm.processPIDExit(pid) + return + default: + if e, ok := err.(*os.PathError); ok && e.Err == syscall.ESRCH { + // If the process exits while reading its /proc/$PID/maps, the kernel will + // return ESRCH. Handle it as if the process did not exist. + pm.mappingStats.errProcESRCH.Add(1) + } + log.Debugf("removing pid due to mappings read error: %v", err) + pm.processPIDExit(pid) + return } - // Clean up, and notify eBPF. - log.Debugf("removing pid due to mappings read error: %v", err) - pm.processPIDExit(pid) - return } util.AtomicUpdateMaxUint32(&pm.mappingStats.maxProcParseUsec, uint32(elapsed.Microseconds()))