Skip to content

fix(perf): purge inherited mappings on execve to fix unknown symbols#392

Merged
GuillaumeLagrange merged 1 commit into
mainfrom
cod-2779-fix-unknown-symbols-in-walltime-and-add-e2e-gate
Jun 5, 2026
Merged

fix(perf): purge inherited mappings on execve to fix unknown symbols#392
GuillaumeLagrange merged 1 commit into
mainfrom
cod-2779-fix-unknown-symbols-in-walltime-and-add-e2e-gate

Conversation

@GuillaumeLagrange
Copy link
Copy Markdown
Contributor

@GuillaumeLagrange GuillaumeLagrange commented Jun 5, 2026

Purge a process's inherited memory mappings when it execve's a new binary, so wall-time symbolication doesn't resolve frames against the wrong module.

A child forked from a parent (e.g. bash) inherits the parent's memory
mappings. When it then execve's its own binary, the inherited PIE
mappings remain and can collide with the new binary's mappings at the
same base address, leading to wrong symbolication and unknown symbols.

Handle COMM records flagged as execve and purge the exec'ing process's
inherited mappings so only its post-exec mappings are used.

Timing-aware module mapping would be a more general fix but is larger in scope; tracked separately in COD-1377.

Refs COD-2779

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 5, 2026

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 7 untouched benchmarks


Comparing cod-2779-fix-unknown-symbols-in-walltime-and-add-e2e-gate (a54f0af) with main (bfff150)

Open in CodSpeed

Copy link
Copy Markdown
Member

@not-matthias not-matthias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 5, 2026

Greptile Summary

This PR fixes incorrect symbolication when a child process (e.g. a benchmark forked from bash) exec's its own binary by purging stale inherited memory mappings on COMM records flagged as is_execve. A new purge_process_mappings helper iterates all loaded modules and removes the exec'ing PID's entries, ensuring that only post-exec MMAP2 records are used for symbolication.

  • Adds a RecordType::COMM branch that fires purge_process_mappings for any tracked PID whose COMM record carries is_execve, clearing all module mappings inherited via inherit_parent_mappings on fork.
  • Adds a chronological-ordering comment documenting the stream assumption.
  • Adds a focused unit test (purge_process_mappings_removes_only_the_target_pid) that reproduces the fork+exec collision scenario and verifies the purge's selectivity.

Confidence Score: 5/5

Safe to merge; the change is narrowly scoped to purging stale per-PID mappings on execve and cannot affect PIDs that don't exec.

The fix correctly intercepts COMM records with is_execve, gates the purge behind pid_filter.should_include so only tracked processes are affected, and relies on the chronological ordering already required by the rest of the parsing loop. The new unit test covers the exact collision scenario and verifies isolation between PIDs. No correctness issues found.

No files require special attention.

Important Files Changed

Filename Overview
src/executor/wall_time/profiler/perf/parse_perf_file.rs Adds COMM/execve handling to purge inherited mappings for exec'ing PIDs; logic is correct, well-guarded by pid_filter, and covered by a new unit test.

Sequence Diagram

sequenceDiagram
    participant perf as perf stream
    participant parser as parse_for_memmap2
    participant modules as loaded_modules_by_path
    participant filter as pid_filter

    perf->>parser: "FORK(ppid=100, pid=200)"
    parser->>filter: add_child_if_parent_tracked(100, 200)
    filter-->>parser: true (child tracked)
    parser->>modules: inherit_parent_mappings(100 to 200)
    Note over modules: bash mappings copied to pid 200

    perf->>parser: "COMM(pid=200, is_execve=true)"
    parser->>filter: should_include(200)
    filter-->>parser: true
    parser->>modules: "purge_process_mappings(pid=200)"
    Note over modules: All pid 200 entries removed

    perf->>parser: "MMAP2(pid=200, path=/cpp/build/fractal_main)"
    parser->>modules: "process_mmap2_record(pid=200, new binary)"
    Note over modules: Clean post-exec mapping for pid 200
Loading

Reviews (4): Last reviewed commit: "fix(perf): purge inherited mappings on e..." | Re-trigger Greptile

Comment thread src/executor/wall_time/profiler/perf/parse_perf_file.rs Outdated
Comment thread src/executor/wall_time/profiler/perf/parse_perf_file.rs
@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-2779-fix-unknown-symbols-in-walltime-and-add-e2e-gate branch 2 times, most recently from 9bf1fdd to 791f1d8 Compare June 5, 2026 09:47
A child forked from a parent (e.g. bash) inherits the parent's memory
mappings. When it then execve's its own binary, the inherited PIE
mappings remain and can collide with the new binary's mappings at the
same base address, leading to wrong symbolication and unknown symbols.

Handle COMM records flagged as execve and purge the exec'ing process's
inherited mappings so only its post-exec mappings are used.

Refs COD-2779
@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-2779-fix-unknown-symbols-in-walltime-and-add-e2e-gate branch from 791f1d8 to a54f0af Compare June 5, 2026 09:51
@GuillaumeLagrange GuillaumeLagrange merged commit dcbc74c into main Jun 5, 2026
22 checks passed
@GuillaumeLagrange GuillaumeLagrange deleted the cod-2779-fix-unknown-symbols-in-walltime-and-add-e2e-gate branch June 5, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants