Skip to content

fix(valgrind): install libc6-dbg during valgrind setup#394

Merged
not-matthias merged 1 commit into
mainfrom
cod-2770-unresolved-symbol-in-exec-cli-simulation-benchmarks
Jun 16, 2026
Merged

fix(valgrind): install libc6-dbg during valgrind setup#394
not-matthias merged 1 commit into
mainfrom
cod-2770-unresolved-symbol-in-exec-cli-simulation-benchmarks

Conversation

@not-matthias

Copy link
Copy Markdown
Member

This takes around 10 seconds, so it shouldn't be an issue that it's not cached + prevents complexities of cache invalidation

@codspeed-hq

codspeed-hq Bot commented Jun 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.

✅ 7 untouched benchmarks


Comparing cod-2770-unresolved-symbol-in-exec-cli-simulation-benchmarks (66037be) with main (7c50010)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented Jun 5, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes valgrind benchmark runs failing on GitHub runners by installing libc6-dbg (required debug symbols not present by default) alongside the valgrind .deb in the same apt::install call, and wires it correctly into the existing caching and idempotency infrastructure.

  • Cache gate fix: is_valgrind_installed() now also checks libc6-dbg via the new apt::is_package_installed helper, so stale caches created before this PR (which lack libc6-dbg) will correctly fall through to a fresh install rather than returning early as satisfied.
  • Single install call: Both valgrind and libc6-dbg are installed together and returned in the package list, so they are saved to and restored from cache atomically.

Confidence Score: 5/5

Safe to merge — the fix is well-scoped and the cache gate is correctly updated to cover both packages.

Both changed files are clean: the new is_package_installed helper is straightforward, and is_valgrind_installed now correctly gates on both valgrind and libc6-dbg. Old caches without libc6-dbg will fail the predicate and trigger a full reinstall, so there is no silent regression path. The install and cache-manifest changes are symmetric and consistent.

No files require special attention.

Important Files Changed

Filename Overview
src/executor/helpers/apt.rs Adds is_package_installed helper that queries dpkg -s to check if a single package is installed; used by the updated is_valgrind_installed predicate.
src/executor/valgrind/setup.rs Extends is_valgrind_installed to also require libc6-dbg and installs it in the same apt::install call and cache manifest as the valgrind .deb, correctly placing it inside the cache gate.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[install_valgrind called] --> B{is_valgrind_installed?\nvalgrind AND libc6-dbg}
    B -- yes --> C[return Ok - nothing to do]
    B -- no --> D{setup_cache_dir\nprovided?}
    D -- yes --> E[restore_from_cache]
    E --> F{is_valgrind_installed?\nvalgrind AND libc6-dbg}
    F -- yes --> G[return Ok - restored from cache]
    F -- no --> H
    D -- no --> H[download valgrind .deb]
    H --> I[apt-get install valgrind.deb libc6-dbg]
    I --> J{cache_dir provided?}
    J -- yes --> K[save_to_cache valgrind + libc6-dbg]
    K --> L[return Ok]
    J -- no --> L
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[install_valgrind called] --> B{is_valgrind_installed?\nvalgrind AND libc6-dbg}
    B -- yes --> C[return Ok - nothing to do]
    B -- no --> D{setup_cache_dir\nprovided?}
    D -- yes --> E[restore_from_cache]
    E --> F{is_valgrind_installed?\nvalgrind AND libc6-dbg}
    F -- yes --> G[return Ok - restored from cache]
    F -- no --> H
    D -- no --> H[download valgrind .deb]
    H --> I[apt-get install valgrind.deb libc6-dbg]
    I --> J{cache_dir provided?}
    J -- yes --> K[save_to_cache valgrind + libc6-dbg]
    K --> L[return Ok]
    J -- no --> L
Loading

Reviews (4): Last reviewed commit: "fix(valgrind): install libc6-dbg during ..." | Re-trigger Greptile

Comment thread src/executor/valgrind/setup.rs Outdated
Comment thread src/executor/valgrind/setup.rs Outdated
Comment thread src/executor/valgrind/setup.rs Outdated
@not-matthias not-matthias force-pushed the cod-2770-unresolved-symbol-in-exec-cli-simulation-benchmarks branch 2 times, most recently from 5a1963a to 23e64a3 Compare June 11, 2026 09:21

@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.

lgtm if cache properly tested with the action

GitHub runners do not ship libc debug symbols by default, which leads
to unresolved symbols in exec CLI simulation benchmarks. Install
libc6-dbg via apt on supported systems after installing valgrind.

Fixes COD-2770
@not-matthias not-matthias force-pushed the cod-2770-unresolved-symbol-in-exec-cli-simulation-benchmarks branch from 23e64a3 to 66037be Compare June 16, 2026 14:40
@not-matthias not-matthias merged commit 66037be into main Jun 16, 2026
21 checks passed
@not-matthias not-matthias deleted the cod-2770-unresolved-symbol-in-exec-cli-simulation-benchmarks branch June 16, 2026 14:52
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