Skip to content

fix(memory): compare mapped module identity against a mapping, not stat - #563

Merged
not-matthias merged 1 commit into
mainfrom
cod-3746-investigate-unresolved-symbols-and-truncated-memory-call
Oct 6, 2026
Merged

not-matthias merged 1 commit into
mainfrom
cod-3746-investigate-unresolved-symbols-and-truncated-memory-call

Conversation

@not-matthias

@not-matthias not-matthias commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

TLDR: This is a regression from the gen2 macro runner, which uses overlayfs.

Stops memory-mode artifact extraction from rejecting unchanged system libraries on nested overlayfs, which left libc frames unresolved in memory flamegraphs.

  • names_mapped_file compared the perf MMAP2 (dev, ino) with stat(path). On nested overlayfs, stat can return a per-layer pseudo device while mappings carry the overlay's own device, so every system library was treated as changed and shipped no symbols or unwind data.
  • The current identity now comes from a read-only mapping of the file, read back from /proc/self/maps, which the kernel derives the same way as MMAP2.
  • Replaced files still get a different inode and are still rejected.

Evidence

  • Failing report: 28 changed since it was mapped lines for libc, ld.so and others, recorded minor 56 vs stat 58, same inode. Only 4 modules were saved.
  • New root-only test accepts_an_unchanged_file_in_a_nested_overlay builds a nested overlay, asserts stat and the mapping disagree, and checks the file is accepted. It passes in a privileged container. The other module_artifacts tests pass too.

Review notes

  • The nested overlay test is #[ignore] because it needs root to mount.
  • Not yet re-recorded end to end: whether libc symbols resolve and whether truncated stacks get deeper.

Fixes COD-3746

@codspeed

codspeed Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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.

✅ 31 untouched benchmarks
⏩ 6 skipped benchmarks1


Comparing cod-3746-investigate-unresolved-symbols-and-truncated-memory-call (8c603c2) with main (bcad8db)2

Open in CodSpeed

Footnotes

  1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on main (4421efa) during the generation of this report, so bcad8db was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@not-matthias
not-matthias marked this pull request as ready for review October 5, 2026 12:10
@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Refactors module artifact parsing to bind identity checks to file contents.

The PR is not safe to merge until executor tests are separated by their actual modes so memtrack cannot overlap Valgrind.

Summary

The PR changes memory-mode module extraction to compare mapping identity using a mapped file and parse artifacts from those same bytes. Since the prior review, it also adds uretprobe return-address restoration and moves executor integration tests into a Docker-backed CI job.

  • The new test sharder does not reliably separate memory tests from simulation tests, which must not overlap.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Parametrized executor tests] --> B[Generated positional test names]
  B --> C[Name-based mode classifier]
  C --> D[Walltime shard]
  C --> E[Simulation shard]
  D --> F[Concurrent container runs]
  E --> F
  F --> G[Memtrack probes can overlap Valgrind]
Loading

Reviews (5) · Last reviewed commit: "fix(memory): compare mapped module ident..."

Comment thread src/executor/memory/module_artifacts.rs Outdated
Comment thread src/executor/memory/module_artifacts.rs Outdated
Comment thread src/executor/memory/module_artifacts.rs Outdated
@not-matthias
not-matthias force-pushed the cod-3746-investigate-unresolved-symbols-and-truncated-memory-call branch 2 times, most recently from d652369 to 024423c Compare October 5, 2026 14:03
Comment thread src/executor/memory/module_artifacts.rs
@not-matthias
not-matthias force-pushed the cod-3746-investigate-unresolved-symbols-and-truncated-memory-call branch from f88ed24 to 8d9e5b2 Compare October 6, 2026 10:12
@not-matthias
not-matthias changed the base branch from main to cod-3758-memtrack-c-allocation-stacks-stop-at-operator-new-uretprobe October 6, 2026 10:12
@not-matthias
not-matthias added this pull request to stack #568 October 6, 2026 10:20

@GuillaumeLagrange GuillaumeLagrange left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch, lgtm

@not-matthias
not-matthias force-pushed the cod-3746-investigate-unresolved-symbols-and-truncated-memory-call branch from 8d9e5b2 to 7a793c3 Compare October 6, 2026 12:42
Module artifact extraction checks that a mapped path still names the file
that was mapped by comparing the perf MMAP2 (dev, ino) with stat(path).
On nested overlayfs the two disagree for unchanged files: perf and
/proc/PID/maps report the overlay superblock's device, while stat can
return a per-layer pseudo device when the outer overlay cannot encode its
layer in the inode's high bits (the inner overlay's xino already uses them).
Every system library was rejected as changed, so libc and ld.so shipped no
symbols or unwind data and their frames showed as unresolved addresses.

Map each module once and read the device and inode of that mapping from
/proc/self/maps, which the kernel derives the same way as MMAP2. Replaced
files still get a different inode and are still rejected. Symbols, load
bias and unwind data are parsed from the same mapping, so a path replaced
during extraction cannot pair the verified identity with another file's
contents.
@not-matthias
not-matthias force-pushed the cod-3746-investigate-unresolved-symbols-and-truncated-memory-call branch from 7a793c3 to 8c603c2 Compare October 6, 2026 12:50
Base automatically changed from cod-3758-memtrack-c-allocation-stacks-stop-at-operator-new-uretprobe to main October 6, 2026 13:04
@not-matthias
not-matthias merged commit 8c603c2 into main Oct 6, 2026
57 checks passed
@not-matthias
not-matthias deleted the cod-3746-investigate-unresolved-symbols-and-truncated-memory-call branch October 6, 2026 13:06
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