From 5abd69c1bd952dc92b84542ea9f1baa9103bc3fd Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Fri, 4 Sep 2026 17:48:02 +0200 Subject: [PATCH 01/27] chore(memtrack): add argp.h stub for musl builds `libbpf-sys` vendors elfutils, whose `configure` aborts on a musl target because `argp_parse` is a glibc extension that musl does not implement: checking for library containing argp_parse... no configure: error: failed to find argp_parse libelf does not actually need those symbols -- they are used by the elfutils CLI tools for argument parsing, but `configure.ac` checks for them unconditionally, even when only the library is being built. Seeding autoconf's cache (`ac_cv_search_argp_parse="none required"`) skips the check, but the elfutils sources still `#include `, so the header has to exist. Nothing that gets compiled calls into it, hence declarations only. Lives inside the crate, next to `wrapper.h` and `src/ebpf/c`, rather than at the repo root: it is a memtrack build input, and it needs its own directory because the path goes on the include path via `-I`. Refs: https://github.com/libbpf/libbpf-sys/issues/137 Co-Authored-By: Claude Opus 5 (1M context) --- crates/memtrack/musl/argp.h | 43 +++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 crates/memtrack/musl/argp.h diff --git a/crates/memtrack/musl/argp.h b/crates/memtrack/musl/argp.h new file mode 100644 index 00000000..26a03a69 --- /dev/null +++ b/crates/memtrack/musl/argp.h @@ -0,0 +1,43 @@ +/* crates/memtrack/musl/argp.h — stub for musl builds of libbpf-sys' vendored elfutils. + Declarations only: a libelf-only build never calls into argp, but the + elfutils sources still `#include `, which musl does not ship. + If compilation complains about a missing type or macro, add it here. */ +#ifndef CODSPEED_STUB_ARGP_H +#define CODSPEED_STUB_ARGP_H + +#include + +typedef int error_t; + +struct argp_option { + const char *name; + int key; + const char *arg; + int flags; + const char *doc; + int group; +}; + +struct argp_state { + const char *name; +}; + +typedef error_t (*argp_parser_t)(int key, char *arg, struct argp_state *state); + +struct argp { + const struct argp_option *options; + argp_parser_t parser; + const char *args_doc; + const char *doc; + const void *children; + void *help_filter; + const char *argp_domain; +}; + +#define OPTION_ARG_OPTIONAL 0x1 +#define ARGP_HELP_SEE 0x40 +#define ARGP_ERR_UNKNOWN 1 + +int argp_help(const struct argp *argp, FILE *stream, unsigned int flags, char *name); + +#endif /* CODSPEED_STUB_ARGP_H */ From 51bb339607302b76f487069f2d0f334f103a6448 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Fri, 4 Sep 2026 18:00:51 +0200 Subject: [PATCH 02/27] ci: add throwaway COD-3440 musl check workflow Closes the one gap the local COD-3440 spike could not: whether the musl build of memtrack actually loads its BPF programs on a real x86_64 kernel. The spike was done on an aarch64 host, where an x86_64 build can only be cross-compiled -- its skeleton targets the x86_64 ABI and cannot load against an aarch64 kernel. `workflow_dispatch` only, so it never runs on its own, and it touches nothing in `release.yml`, `dist-workspace.toml` or any `Cargo.toml`. Beyond the autoconf cache seeds and the argp.h stub, Debian needs one thing the spike host did not: `LIBBPF_SYS_EXTRA_CFLAGS` with `-idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include`. Its musl-gcc runs with `-nostdinc` and only the musl include directory, so libbpf cannot find the kernel UAPI headers it includes directly: bpf.c:28:10: fatal error: asm/unistd.h: No such file or directory ../include/linux/types.h:12:10: fatal error: asm/types.h: No such file **This must not reach `main`.** Delete it once the question is answered. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/cod-3440-musl-check.yml | 206 ++++++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 .github/workflows/cod-3440-musl-check.yml diff --git a/.github/workflows/cod-3440-musl-check.yml b/.github/workflows/cod-3440-musl-check.yml new file mode 100644 index 00000000..a0d6ec28 --- /dev/null +++ b/.github/workflows/cod-3440-musl-check.yml @@ -0,0 +1,206 @@ +# COD-3440 spike — throwaway workflow, NOT for merging. +# +# Purpose: prove the musl recipe on a real x86_64 kernel, which is the one gap +# the local spike could not close (the local host is aarch64; an x86_64 build +# cross-compiled there cannot load its BPF skeleton against an aarch64 kernel). +# +# To use it: push the spike branch, then +# +# gh workflow run cod-3440-musl-check.yml --ref spike/cod-3440-memtrack-musl +# +# Manual trigger only, so it never fires on its own. Note that the very first +# dispatch of a workflow that lives only on a non-default branch 404s until +# GitHub has registered it; once it has appeared in the Actions list, --ref +# dispatch works. Delete the file once the question is answered -- it must not +# reach main. +# +# Deliberately does NOT touch release.yml, dist-workspace.toml or any +# Cargo.toml — it only adds one manually-triggered job. +# +# Two differences from the local spike recipe, both simplifications: +# - musl-tools provides x86_64-linux-musl-gcc, which cc-rs finds on its own, +# so no compiler wrapper and no -lgcc / -fno-link-libatomic. +# - no zig, so none of the zig artifacts (crt duplication, UBSan trap, +# unknown warning options) apply. +# The only thing carried over is what the spike is actually about: the three +# autoconf cache seeds and the crates/memtrack/musl/argp.h stub. + +name: COD-3440 musl check + +on: + workflow_dispatch: + +env: + TARGET: x86_64-unknown-linux-musl + # Absolute, because the test step runs with working-directory: crates/memtrack, + # where a $PWD-relative path would resolve to the wrong place. + STUB_INCLUDE: ${{ github.workspace }}/crates/memtrack/musl + # libbpf includes and ; Debian's musl-gcc runs with + # -nostdinc and only /usr/include/x86_64-linux-musl on the include path, so the + # kernel UAPI headers from linux-libc-dev have to be added back. -idirafter puts + # them last, behind musl's own headers. build.rs forwards this to libbpf's make; + # CFLAGS alone would not reach it the same way. + LIBBPF_SYS_EXTRA_CFLAGS: -idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include + # Approach A: pre-seed autoconf's cache so the vendored elfutils never runs + # the checks musl cannot satisfy. "none required" = available with no -l flag. + ac_cv_search_argp_parse: none required + ac_cv_search__obstack_free: none required + ac_cv_search_fts_close: none required + +jobs: + build: + name: build ${{ matrix.profile }} + runs-on: ubuntu-latest # x86_64 + strategy: + fail-fast: false + matrix: + profile: [dev, dist] + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: musl-${{ matrix.profile }} + - uses: ./.github/actions/install-bpf-deps + + - name: Install musl toolchain + run: | + sudo apt-get install -y musl-tools pkg-config linux-libc-dev + rustup target add "$TARGET" + + - name: Build + # elfutils only honours CFLAGS/CPPFLAGS, not LIBBPF_SYS_EXTRA_CFLAGS. + # The stub header exists because elfutils #include even though + # a libelf-only build never calls into it. + run: | + export CFLAGS="-I$STUB_INCLUDE" + cargo build -p memtrack --profile ${{ matrix.profile }} --target "$TARGET" + + - name: Verify the artifact is genuinely static + run: | + BIN=target/$TARGET/${{ matrix.profile == 'dev' && 'debug' || matrix.profile }}/codspeed-memtrack + file "$BIN" + ldd "$BIN" || true # expected: "not a dynamic executable" + readelf -d "$BIN" || true # expected: no dynamic section at all + echo "size: $(stat -c %s "$BIN") bytes" + # Fail loudly if anything reintroduced a dynamic dependency or an rpath. + # `if` rather than `grep ... && exit 1`, because a grep that matches + # nothing exits 1 and would fail the step under bash -e. + if readelf -d "$BIN" 2>/dev/null | grep -qE 'NEEDED|RPATH|RUNPATH'; then + echo "unexpected dynamic dependency or rpath" + exit 1 + fi + echo "OK: static, no NEEDED, no RPATH/RUNPATH" + + - name: Compare against the gnu build + if: matrix.profile == 'dist' + run: | + unset CFLAGS + cargo build -p memtrack --profile dist + echo "musl: $(stat -c %s target/$TARGET/dist/codspeed-memtrack) bytes" + echo "gnu: $(stat -c %s target/dist/codspeed-memtrack) bytes" + + - name: Smoke test the BPF path + run: | + BIN=$PWD/target/$TARGET/${{ matrix.profile == 'dev' && 'debug' || matrix.profile }}/codspeed-memtrack + mkdir -p /tmp/memtrack-out + sudo env "RUST_LOG=info" "$BIN" track -o /tmp/memtrack-out "/bin/ls /tmp" + ls -la /tmp/memtrack-out + # A run that loads no probes still exits 0 but writes nothing. + test -n "$(ls -A /tmp/memtrack-out)" || { echo "no artifact written"; exit 1; } + + tests: + name: ${{ matrix.test }} (musl) + runs-on: ubuntu-latest # x86_64 + strategy: + fail-fast: false + # Each memtrack integration test binary runs its cases serially (the eBPF + # tracker can't overlap with itself in one process), so shard by binary. + matrix: + test: [c_tests, cpp_tests, rust_tests, spawn_tests, dlopen_tests, rss_tests] + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + lfs: true + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: musl-${{ matrix.test }} + - uses: ./.github/actions/install-bpf-deps + + - name: Install musl toolchain + run: | + sudo apt-get install -y musl-tools pkg-config linux-libc-dev + rustup target add "$TARGET" + + - name: Install additional allocators + run: sudo apt-get install -y libmimalloc-dev libjemalloc-dev + + # Built separately from the run because test-with's env(GITHUB_ACTIONS) + # gate is evaluated at COMPILE time. GITHUB_ACTIONS is set by the runner, + # so this is automatic here -- but if the tests are ever built outside + # Actions, the sudo-gated cases silently become #[ignore]d and the run + # reports a green "0 passed; N ignored". + - name: Build tests + run: | + export CFLAGS="-I$STUB_INCLUDE" + cargo test -p memtrack --target "$TARGET" --no-run + + - name: Run tests + env: + RUST_LOG: debug + # Ubuntu 26.04 ships sudo-rs, which ignores `-E`; pass the env the + # rustup shims and the test gate need through `env` instead. + run: | + sudo env \ + "HOME=$HOME" \ + "PATH=$PATH" \ + "CARGO_HOME=${CARGO_HOME:-$HOME/.cargo}" \ + "RUSTUP_HOME=${RUSTUP_HOME:-$HOME/.rustup}" \ + "CARGO_INCREMENTAL=$CARGO_INCREMENTAL" \ + "RUST_LOG=$RUST_LOG" \ + "GITHUB_ACTIONS=$GITHUB_ACTIONS" \ + "CFLAGS=-I$STUB_INCLUDE" \ + "LIBBPF_SYS_EXTRA_CFLAGS=$LIBBPF_SYS_EXTRA_CFLAGS" \ + "TARGET=$TARGET" \ + "ac_cv_search_argp_parse=$ac_cv_search_argp_parse" \ + "ac_cv_search__obstack_free=$ac_cv_search__obstack_free" \ + "ac_cv_search_fts_close=$ac_cv_search_fts_close" \ + $(which cargo) test --target "$TARGET" --test ${{ matrix.test }} \ + -- --test-threads 1 --nocapture + working-directory: crates/memtrack + + # Since we ran the tests with sudo, the build artifacts will have root ownership + - name: Clean up + run: sudo chown -R $USER:$USER . ~/.cargo + + unit: + name: unit tests (musl) + runs-on: ubuntu-latest # x86_64 + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: musl-unit + - uses: ./.github/actions/install-bpf-deps + + - name: Install musl toolchain + run: | + sudo apt-get install -y musl-tools pkg-config linux-libc-dev + rustup target add "$TARGET" + + # Split out from the sharded job because of one known failure: + # ebpf::memtrack::tests::libc_allocator_symbols_resolve_to_offsets reads + # /proc/self/maps of the TEST BINARY and requires a mapped libc.so.6, + # which a statically linked musl binary does not have by construction. + # The production path resolves symbols in the *traced* process, so this is + # a test assumption, not a defect (report §4). Drop the --skip to check + # whether it has since been fixed; everything else must stay green. + - name: Run unit tests + run: | + export CFLAGS="-I$STUB_INCLUDE" + cargo test -p memtrack --target "$TARGET" --lib \ + -- --skip libc_allocator_symbols_resolve_to_offsets From 331541e5b39123c966cec020efe68864e84be7d8 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Mon, 7 Sep 2026 12:29:50 +0200 Subject: [PATCH 03/27] feat(exec-harness)!: remove the LD_PRELOAD hack The harness used to inject a `libcodspeed_preload.so` into the benchmark process so the callgrind client requests were issued from inside it. That was only necessary because instrumentation state did not propagate across `fork`, which COD-2349 has since fixed. Instrumentation is now toggled in exec-harness itself, around the spawn of each benchmark command. The benchmarked child inherits the live state across `fork`/`exec`, callgrind records the spawn edge on the dump part live at fork time, and `set_executed_benchmark` names that same part with the benchmark URI, so the backend can attribute the child's whole trace to the benchmark. Dropping the preload removes the "CPU Simulation mode does not support statically linked binaries" limitation, since nothing has to be injected into the benchmarked executable any more. It also unblocks building exec-harness for musl (COD-3440), which a preloaded `.so` made impossible. `--instr-atstart=inherit` becomes unconditional: it is what makes the benchmark measurable at all now, so it can no longer hang off the opt-in `--simulation-track-subprocess`, which keeps its name but from now on only selects `--separate-threads`. Since every way this can go wrong is silent -- the harness runs, the benchmark completes, and the measurement is empty -- the harness now fails loudly when it finds itself uninstrumented. BREAKING CHANGE: the measured region of an exec-harness benchmark now begins in exec-harness before the fork rather than in the child's ELF constructor, so it also covers the fork/exec/wait path and the child's pre-main startup. Absolute numbers shift in a step and history is not comparable across this change. A post-preload exec-harness also requires a runner that passes `--instr-atstart=inherit`, valgrind-codspeed >= iteration 6, and a backend with spawn-chain attribution. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 2 - crates/exec-harness/Cargo.toml | 4 - crates/exec-harness/build.rs | 168 +----------------- .../exec-harness/preload/codspeed_preload.c | 87 --------- .../src/analysis/ld_preload_check.rs | 120 ------------- crates/exec-harness/src/analysis/mod.rs | 84 ++++----- .../src/analysis/preload_lib_file.rs | 46 ----- crates/exec-harness/src/constants.rs | 7 +- crates/exec-harness/src/lib.rs | 7 +- src/cli/shared.rs | 6 +- src/executor/valgrind/measure.rs | 9 +- 11 files changed, 66 insertions(+), 474 deletions(-) delete mode 100644 crates/exec-harness/preload/codspeed_preload.c delete mode 100644 crates/exec-harness/src/analysis/ld_preload_check.rs delete mode 100644 crates/exec-harness/src/analysis/preload_lib_file.rs diff --git a/Cargo.lock b/Cargo.lock index 69c918e5..87b3f0b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1179,13 +1179,11 @@ name = "exec-harness" version = "1.3.0" dependencies = [ "anyhow", - "cc", "clap", "env_logger", "humantime", "instrument-hooks-bindings", "log", - "object", "runner-shared", "serde", "serde_json", diff --git a/crates/exec-harness/Cargo.toml b/crates/exec-harness/Cargo.toml index f73c631c..277f0155 100644 --- a/crates/exec-harness/Cargo.toml +++ b/crates/exec-harness/Cargo.toml @@ -20,10 +20,6 @@ serde = { workspace = true } humantime = "2.3" runner-shared = { path = "../runner-shared" } tempfile = { workspace = true } -object = { workspace = true } - -[build-dependencies] -cc = "1" [package.metadata.dist] targets = ["aarch64-unknown-linux-gnu", "x86_64-unknown-linux-gnu"] diff --git a/crates/exec-harness/build.rs b/crates/exec-harness/build.rs index bf65ef7e..8988c463 100644 --- a/crates/exec-harness/build.rs +++ b/crates/exec-harness/build.rs @@ -1,170 +1,18 @@ //! Build script for exec-harness //! -//! This script compiles the `libcodspeed_preload.so` shared library that is used -//! to inject instrumentation into child processes via LD_PRELOAD. -//! -//! The library is built using the `core.c` and headers from the `instrument-hooks-bindings` -//! crate's `instrument-hooks` directory. - -use std::env; -use std::path::PathBuf; +//! Exports the constants shared between the crate's modules as environment +//! variables, so `src/constants.rs` can read them through `env!()` and there is +//! a single source of truth for the integration identity reported to CodSpeed. -/// Shared constants for the preload library. -/// These are passed as C defines during compilation and exported as environment -/// variables for the Rust code to use via `env!()`. -struct PreloadConstants { - /// Environment variable name for the benchmark URI. - uri_env: &'static str, - /// Integration name reported to CodSpeed. - integration_name: &'static str, - /// Integration version reported to CodSpeed. - integration_version: &'static str, - /// Filename for the preload shared library. - preload_lib_filename: &'static str, -} +/// Integration name reported to CodSpeed. +const INTEGRATION_NAME: &str = "exec-harness"; fn main() { - println!("cargo:rerun-if-changed=preload/codspeed_preload.c"); - println!("cargo:rerun-if-env-changed=CODSPEED_INSTRUMENT_HOOKS_DIR"); - - let preload_constants: PreloadConstants = PreloadConstants::default(); + println!("cargo:rerun-if-changed=build.rs"); - // Export constants as environment variables for the Rust code - println!( - "cargo:rustc-env=CODSPEED_URI_ENV={}", - preload_constants.uri_env - ); - println!( - "cargo:rustc-env=CODSPEED_INTEGRATION_NAME={}", - preload_constants.integration_name - ); + println!("cargo:rustc-env=CODSPEED_INTEGRATION_NAME={INTEGRATION_NAME}"); println!( "cargo:rustc-env=CODSPEED_INTEGRATION_VERSION={}", - preload_constants.integration_version - ); - println!( - "cargo:rustc-env=CODSPEED_PRELOAD_LIB_FILENAME={}", - preload_constants.preload_lib_filename + env!("CARGO_PKG_VERSION") ); - - let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap()); - - // Try to get the instrument-hooks directory from the environment variable first, - // otherwise use the one from the instrument-hooks-bindings crate - let instrument_hooks_dir = manifest_dir - .parent() - .unwrap() - .join("instrument-hooks-bindings/instrument-hooks"); - - // Build the preload shared library - let paths = PreloadBuildPaths { - preload_c: manifest_dir.join("preload/codspeed_preload.c"), - core_c: instrument_hooks_dir.join("dist/core.c"), - includes_dir: instrument_hooks_dir.join("includes"), - }; - println!("cargo:rerun-if-changed={}", paths.core_c.display()); - paths.check_sources_exist(); - build_shared_library(&paths, &preload_constants); -} - -/// Build the shared library using the cc crate -fn build_shared_library(paths: &PreloadBuildPaths, constants: &PreloadConstants) { - let uri_env_val = format!("\"{}\"", constants.uri_env); - let integration_name_val = format!("\"{}\"", constants.integration_name); - let integration_version_val = format!("\"{}\"", constants.integration_version); - let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap()); - let out_file = out_dir.join(constants.preload_lib_filename); - - let mut build = cc::Build::new(); - build - .file(&paths.preload_c) - .file(&paths.core_c) - .include(&paths.includes_dir) - .pic(true) - .opt_level(3) - // There's no need to output cargo metadata as we are just building a shared library - // that will be copied to disk and loaded through LD_PRELOAD at runtime - .cargo_metadata(false) - // Pass constants as C defines - .define("CODSPEED_URI_ENV", uri_env_val.as_str()) - .define("CODSPEED_INTEGRATION_NAME", integration_name_val.as_str()) - .define( - "CODSPEED_INTEGRATION_VERSION", - integration_version_val.as_str(), - ) - .std("gnu11") // need gnu11 instead of just c11 for setenv - // Suppress warnings from generated Zig code - .flag("-Wno-format") - .flag("-Wno-format-security") - .flag("-Wno-unused-but-set-variable") - .flag("-Wno-unused-const-variable") - .flag("-Wno-type-limits") - .flag("-Wno-uninitialized") - .flag("-Wno-overflow") - .flag("-Wno-unused-function") - .flag("-Wno-unterminated-string-initialization"); - - // Compile source files to object files - let objects = build.compile_intermediates(); - - // Link object files into shared library - let compiler = build.get_compiler(); - let mut link_cmd = compiler.to_command(); - link_cmd - .arg("-shared") - .arg("-o") - .arg(&out_file) - .args(&objects) - .arg("-lpthread"); - - let status = link_cmd.status().expect("Failed to run linker"); - if !status.success() { - panic!("Failed to link libcodspeed_preload.so"); - } -} - -impl Default for PreloadConstants { - fn default() -> Self { - Self { - uri_env: "CODSPEED_BENCH_URI", - integration_name: "exec-harness", - integration_version: env!("CARGO_PKG_VERSION"), - preload_lib_filename: "libcodspeed_preload.so", - } - } -} - -/// Paths required to build the preload shared library. -struct PreloadBuildPaths { - /// Path to the preload C source file (codspeed_preload.c). - preload_c: PathBuf, - /// Path to the core C source file from instrument-hooks. - core_c: PathBuf, - /// Path to the includes directory from instrument-hooks. - includes_dir: PathBuf, -} - -impl PreloadBuildPaths { - /// Verify that all required source files and directories exist. - /// Panics with a descriptive message if any path is missing. - fn check_sources_exist(&self) { - if !self.core_c.exists() { - panic!( - "core.c not found at {}. Make sure the instrument hooks submodule is available.", - self.core_c.display() - ); - } - if !self.includes_dir.exists() { - panic!( - "includes directory not found at {}. instrument hooks submodule is available.", - self.includes_dir.display() - ); - } - if !self.preload_c.exists() { - panic!( - "codspeed_preload.c not found at {}", - self.preload_c.display() - ); - } - } } diff --git a/crates/exec-harness/preload/codspeed_preload.c b/crates/exec-harness/preload/codspeed_preload.c deleted file mode 100644 index 418af143..00000000 --- a/crates/exec-harness/preload/codspeed_preload.c +++ /dev/null @@ -1,87 +0,0 @@ -// LD_PRELOAD library for enabling Valgrind instrumentation in child processes -// -// This library is loaded via LD_PRELOAD into benchmark processes spawned by -// exec-harness. It enables callgrind instrumentation on load and disables it on -// exit, allowing exec-harness to measure arbitrary commands without requiring -// them to link against instrument-hooks. -// -// Environment variables: -// CODSPEED_BENCH_URI - The benchmark URI to report (required) - -#include -#include - -#include "core.h" - -#ifndef RUNNING_ON_VALGRIND -// If somehow the core.h did not include the valgrind header, something is -// wrong, but still have a fallback -#warning "RUNNING_ON_VALGRIND not defined, headers may be missing" -#define RUNNING_ON_VALGRIND 0 -#endif - -// These constants are defined by the build script (build.rs) via -D flags -#ifndef CODSPEED_URI_ENV -#error "CODSPEED_URI_ENV must be defined by the build system" -#endif -#ifndef CODSPEED_INTEGRATION_NAME -#error "CODSPEED_INTEGRATION_NAME must be defined by the build system" -#endif -#ifndef CODSPEED_INTEGRATION_VERSION -#error "CODSPEED_INTEGRATION_VERSION must be defined by the build system" -#endif - -static const char *URI_ENV = CODSPEED_URI_ENV; -static const char *INTEGRATION_NAME = CODSPEED_INTEGRATION_NAME; -static const char *INTEGRATION_VERSION = CODSPEED_INTEGRATION_VERSION; - -static InstrumentHooks *g_hooks = NULL; -static const char *g_bench_uri = NULL; - -__attribute__((constructor)) static void codspeed_preload_init(void) { - // Skip initialization if not running under Valgrind yet. - // When using LD_PRELOAD with Valgrind, the constructor runs twice: - // once before Valgrind takes over, and once after. We only want to - // initialize when Valgrind is active. - // - // This is purely empirical, and is not (yet) backed up by documented - // behavior. - if (!RUNNING_ON_VALGRIND) { - return; - } - - g_bench_uri = getenv(URI_ENV); - if (!g_bench_uri) { - return; - } - - g_hooks = instrument_hooks_init(); - if (!g_hooks) { - return; - } - - instrument_hooks_set_integration(g_hooks, INTEGRATION_NAME, - INTEGRATION_VERSION); - - if (instrument_hooks_start_benchmark_inline(g_hooks) != 0) { - instrument_hooks_deinit(g_hooks); - g_hooks = NULL; - return; - } -} - -__attribute__((destructor)) static void codspeed_preload_fini(void) { - // If the process is not the owner of the lock, this means g_hooks was not - // initialized - if (!g_hooks) { - return; - } - - instrument_hooks_stop_benchmark_inline(g_hooks); - - int32_t pid = getpid(); - instrument_hooks_set_executed_benchmark(g_hooks, pid, g_bench_uri); - - instrument_hooks_deinit(g_hooks); - g_hooks = NULL; -} diff --git a/crates/exec-harness/src/analysis/ld_preload_check.rs b/crates/exec-harness/src/analysis/ld_preload_check.rs deleted file mode 100644 index 702f18d2..00000000 --- a/crates/exec-harness/src/analysis/ld_preload_check.rs +++ /dev/null @@ -1,120 +0,0 @@ -use crate::prelude::*; -use std::fs; -use std::path::Path; - -/// Checks if the given executable will honor LD_PRELOAD. -/// -/// Returns `Ok(())` if LD_PRELOAD will work, or an error with a descriptive message if not. -/// -/// LD_PRELOAD works for: -/// - Dynamically linked ELF binaries -/// - Scripts (the interpreter is typically dynamically linked) -/// -/// LD_PRELOAD does NOT work for: -/// - Statically linked ELF binaries (no dynamic linker involved) -pub fn check_ld_preload_compatible(executable: &str) -> Result<()> { - let path = resolve_executable(executable)?; - let data = fs::read(&path) - .with_context(|| format!("Failed to read executable: {}", path.display()))?; - - // Check ELF magic bytes - if data.len() >= 4 && &data[0..4] == b"\x7FELF" { - check_elf_is_dynamic(&data, &path) - } else { - // Not an ELF file - likely a script with a shebang. - // Scripts use an interpreter which is typically dynamically linked. - Ok(()) - } -} - -/// Resolve executable name to its full path using PATH lookup. -fn resolve_executable(executable: &str) -> Result { - let path = Path::new(executable); - - // If it's already an absolute or relative path, use it directly - if path.is_absolute() || executable.contains('/') { - return Ok(path.to_path_buf()); - } - - // Search in PATH - if let Ok(path_env) = std::env::var("PATH") { - for dir in path_env.split(':') { - let candidate = Path::new(dir).join(executable); - if candidate.is_file() { - return Ok(candidate); - } - } - } - - bail!("Executable not found in PATH: {executable}") -} - -/// Check if an ELF binary is dynamically linked. -fn check_elf_is_dynamic(data: &[u8], path: &Path) -> Result<()> { - use object::Endianness; - use object::read::elf::ElfFile; - - // Try parsing as 64-bit ELF first, then 32-bit - if let Ok(elf) = ElfFile::>::parse(data) { - return check_elf_has_interp(elf, path); - } - - if let Ok(elf) = ElfFile::>::parse(data) { - return check_elf_has_interp(elf, path); - } - - bail!("Failed to parse ELF file: {}", path.display()) -} - -/// Check if an ELF file has a PT_INTERP or PT_DYNAMIC segment, indicating dynamic linking. -fn check_elf_has_interp<'data, Elf>( - elf: object::read::elf::ElfFile<'data, Elf>, - path: &Path, -) -> Result<()> -where - Elf: object::read::elf::FileHeader, -{ - use object::read::elf::ProgramHeader; - - let endian = elf.endian(); - - for segment in elf.elf_program_headers() { - let p_type = segment.p_type(endian); - // Either PT_INTERP or PT_DYNAMIC indicates a dynamically linked binary - if p_type == object::elf::PT_INTERP || p_type == object::elf::PT_DYNAMIC { - return Ok(()); - } - } - - // No PT_INTERP found - this is a statically linked binary - bail!( - "The codspeed CLI in CPU Simulation mode does not support statically linked binaries.\n\n\ - Executable '{}' is statically linked.\n\n\ - Please either:\n\ - - Use a dynamically linked executable, or\n\ - - Use a different measurement mode, or\n\ - - Use one of the CodSpeed framework benchmark integrations", - path.display() - ) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_dynamic_binary() { - // /bin/sh or similar should be dynamically linked on most systems - let result = check_ld_preload_compatible("sh"); - assert!( - result.is_ok(), - "sh should be dynamically linked: {result:?}" - ); - } - - #[test] - fn test_nonexistent_binary() { - let result = check_ld_preload_compatible("nonexistent_binary_12345"); - assert!(result.is_err()); - } -} diff --git a/crates/exec-harness/src/analysis/mod.rs b/crates/exec-harness/src/analysis/mod.rs index 8bb4eaf4..23d73657 100644 --- a/crates/exec-harness/src/analysis/mod.rs +++ b/crates/exec-harness/src/analysis/mod.rs @@ -1,25 +1,62 @@ +use crate::MeasurementMode; use crate::constants::INTEGRATION_NAME; use crate::constants::INTEGRATION_VERSION; use crate::prelude::*; use crate::BenchmarkCommand; -use crate::constants; use crate::uri; use instrument_hooks_bindings::InstrumentHooks; use std::process::Command; -mod ld_preload_check; -mod preload_lib_file; - -pub fn perform(commands: Vec) -> Result<()> { +/// Executes the given benchmark commands, measuring each one through the +/// instrument hooks. +/// +/// Instrumentation is toggled in *this* process, around the spawn of each +/// benchmark command. Under Valgrind, the benchmarked child inherits the live +/// instrumentation state across `fork`/`exec`, and callgrind records the spawn +/// edge on the dump part that is live at fork time — the same part that +/// [`InstrumentHooks::set_executed_benchmark`] then names with the benchmark +/// URI. The backend walks that edge to attribute the child's trace to the +/// benchmark, so the measurement covers the whole spawned process tree. +/// +/// This replaces the previous `LD_PRELOAD` shared library, which started +/// instrumentation from inside the benchmark process because the state did not +/// use to propagate across `fork`. Dropping it means statically linked +/// executables are now supported, since nothing has to be injected into them. +pub fn perform(commands: Vec, mode: MeasurementMode) -> Result<()> { let hooks = InstrumentHooks::instance(INTEGRATION_NAME, INTEGRATION_VERSION); + if !hooks.is_instrumented() { + // Every way this mode can go wrong is silent: the harness runs, the + // benchmark completes, and the measurement is empty. Fail loudly + // instead. + // + // Note this only catches the absence of *any* instrument (no + // instrument-hooks support compiled in, or nothing to attach to). It + // cannot tell whether Valgrind will actually honour the instrumentation + // toggles, which depends on the `--instr-atstart` the runner passes. + bail!( + "exec-harness found no instrument to report to, so nothing would be measured.\n\ + This binary is meant to be run by the CodSpeed CLI, which sets up the \ + instrumentation around it." + ); + } + for benchmark_cmd in commands { let name_and_uri = uri::generate_name_and_uri(&benchmark_cmd.name, &benchmark_cmd.command); name_and_uri.print_executing(); let mut cmd = Command::new(&benchmark_cmd.command[0]); cmd.args(&benchmark_cmd.command[1..]); + + if mode == MeasurementMode::Simulation { + // Make sure python and node processes output perf maps, so the + // runner can resolve JIT-ed frames afterwards. For python this is + // usually done by `pytest-codspeed`. + cmd.env("PYTHONPERFSUPPORT", "1"); + crate::node::set_node_options(&mut cmd); + } + hooks.start_benchmark().unwrap(); let status = cmd.status(); hooks.stop_benchmark().unwrap(); @@ -34,40 +71,3 @@ pub fn perform(commands: Vec) -> Result<()> { Ok(()) } - -/// Executes the given benchmark commands using a preload based trick to handle valgrind control. -/// -/// This function is only supported on Unix-like platforms, as it relies on the -/// `LD_PRELOAD` environment variable and Unix file permissions for shared libraries. -/// It will not work on non-Unix platforms or with statically linked binaries. -pub fn perform_with_valgrind(commands: Vec) -> Result<()> { - let preload_lib_path = preload_lib_file::get_preload_lib_path()?; - - for benchmark_cmd in commands { - // Check if the executable will honor LD_PRELOAD before running - ld_preload_check::check_ld_preload_compatible(&benchmark_cmd.command[0])?; - - let name_and_uri = uri::generate_name_and_uri(&benchmark_cmd.name, &benchmark_cmd.command); - name_and_uri.print_executing(); - - let mut cmd = Command::new(&benchmark_cmd.command[0]); - cmd.args(&benchmark_cmd.command[1..]); - // Use LD_PRELOAD to inject instrumentation into the child process - cmd.env("LD_PRELOAD", preload_lib_path); - // Make sure python processes output perf maps. This is usually done by `pytest-codspeed` - cmd.env("PYTHONPERFSUPPORT", "1"); - cmd.env(constants::URI_ENV, &name_and_uri.uri); - - crate::node::set_node_options(&mut cmd); - - let mut child = cmd.spawn().context("Failed to spawn command")?; - - let status = child.wait().context("Failed to execute command")?; - - if !status.success() { - bail!("Command exited with non-zero status: {status}"); - } - } - - Ok(()) -} diff --git a/crates/exec-harness/src/analysis/preload_lib_file.rs b/crates/exec-harness/src/analysis/preload_lib_file.rs deleted file mode 100644 index 2d53804c..00000000 --- a/crates/exec-harness/src/analysis/preload_lib_file.rs +++ /dev/null @@ -1,46 +0,0 @@ -use crate::prelude::*; - -use std::io::Write; -use std::sync::OnceLock; - -/// Filename for the preload shared library. -const PRELOAD_LIB_FILENAME: &str = env!("CODSPEED_PRELOAD_LIB_FILENAME"); - -/// The preload library binary embedded at compile time. -const PRELOAD_LIB_BYTES: &[u8] = include_bytes!(concat!( - env!("OUT_DIR"), - "/", - env!("CODSPEED_PRELOAD_LIB_FILENAME") -)); - -/// Lazily initialized temp file containing the extracted preload library. -/// Kept in a static to prevent cleanup until process exit. -static PRELOAD_LIB_FILE: OnceLock = OnceLock::new(); - -/// Extracts the preload library to a temp file. -fn extract_preload_lib() -> Result { - let mut file = tempfile::Builder::new() - .suffix(PRELOAD_LIB_FILENAME) - .tempfile() - .context("Failed to create temp file for preload library")?; - - file.write_all(PRELOAD_LIB_BYTES) - .context("Failed to write preload library to temp file")?; - - debug!( - "Extracted preload library to temp file: {}", - file.path().display() - ); - - Ok(file) -} - -/// Returns the path to the preload library, extracting it to a temp file if needed. -pub(super) fn get_preload_lib_path() -> Result<&'static std::path::Path> { - if let Some(file) = PRELOAD_LIB_FILE.get() { - return Ok(file.path()); - } - - let file = extract_preload_lib()?; - Ok(PRELOAD_LIB_FILE.get_or_init(|| file).path()) -} diff --git a/crates/exec-harness/src/constants.rs b/crates/exec-harness/src/constants.rs index 9a47591c..f982f395 100644 --- a/crates/exec-harness/src/constants.rs +++ b/crates/exec-harness/src/constants.rs @@ -1,11 +1,8 @@ //! Shared constants for the exec-harness crate. //! //! These constants are defined in the build script (build.rs) and exported as -//! environment variables. The same values are passed to the C preload library -//! as compiler defines, ensuring both Rust and C code use the same source of truth. - -/// Environment variable name for the benchmark URI. -pub const URI_ENV: &str = env!("CODSPEED_URI_ENV"); +//! environment variables, so that the integration identity reported to CodSpeed +//! has a single source of truth. /// Integration name reported to CodSpeed. pub const INTEGRATION_NAME: &str = env!("CODSPEED_INTEGRATION_NAME"); diff --git a/crates/exec-harness/src/lib.rs b/crates/exec-harness/src/lib.rs index 30cb21b4..28e52d58 100644 --- a/crates/exec-harness/src/lib.rs +++ b/crates/exec-harness/src/lib.rs @@ -74,11 +74,8 @@ pub fn execute_benchmarks( Some(MeasurementMode::Walltime) | None => { walltime::perform(commands)?; } - Some(MeasurementMode::Memory) => { - analysis::perform(commands)?; - } - Some(MeasurementMode::Simulation) => { - analysis::perform_with_valgrind(commands)?; + Some(mode @ (MeasurementMode::Memory | MeasurementMode::Simulation)) => { + analysis::perform(commands, mode)?; } } diff --git a/src/cli/shared.rs b/src/cli/shared.rs index 1fe13474..b926af04 100644 --- a/src/cli/shared.rs +++ b/src/cli/shared.rs @@ -135,7 +135,11 @@ pub struct ExecAndRunSharedArgs { )] pub exclude_allocations: bool, - /// Measure the subprocesses spawned by the benchmarked process in simulation mode. + /// Emit per-thread dumps for the benchmarked process in simulation mode. + /// + /// Subprocesses spawned by the benchmarked process are now always measured, + /// so this only controls Valgrind's `--separate-threads`. The flag keeps its + /// name for compatibility; renaming it would break existing invocations. #[arg(long, env = "CODSPEED_SIMULATION_TRACK_SUBPROCESS")] pub simulation_track_subprocess: bool, diff --git a/src/executor/valgrind/measure.rs b/src/executor/valgrind/measure.rs index 62807b92..5c395753 100644 --- a/src/executor/valgrind/measure.rs +++ b/src/executor/valgrind/measure.rs @@ -33,11 +33,16 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec Date: Mon, 7 Sep 2026 12:29:59 +0200 Subject: [PATCH 04/27] fix(instrument-hooks): never fall back to the noop impl on Linux When cc-rs fails to compile the native library, the build script printed a `cargo:warning` and compiled the noop `InstrumentHooks` instead, in which every hook returns `Ok(())`. A build that landed there ran benchmarks and measured nothing, at exit code 0. That is reachable by accident: building for a musl target without a musl C compiler on PATH is enough, which the exec-harness musl port makes a routine thing to do. Make it a build failure on Linux, where we actually measure, and point at the missing toolchain. Other platforms keep the warning so macOS dev builds are unaffected. Co-Authored-By: Claude Opus 5 (1M context) --- crates/instrument-hooks-bindings/build.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/instrument-hooks-bindings/build.rs b/crates/instrument-hooks-bindings/build.rs index 63b46664..eb6611e8 100644 --- a/crates/instrument-hooks-bindings/build.rs +++ b/crates/instrument-hooks-bindings/build.rs @@ -1,3 +1,5 @@ +use std::env; + fn main() { println!("cargo:rustc-check-cfg=cfg(use_instrument_hooks)"); @@ -35,6 +37,24 @@ fn main() { Err(e) => { let compiler = build.try_get_compiler().expect("Failed to get C compiler"); + // Falling back to the noop implementation makes every hook a + // no-op, so a build that lands there runs benchmarks and reports + // no measurement at all, at exit code 0. Linux is where we + // actually measure, so fail the build instead of emitting a + // warning nobody reads. + if env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("linux") { + panic!( + "Failed to compile the instrument-hooks native library with cc-rs.\n\ + A Linux build must not fall back to the noop implementation: it \ + would run benchmarks and measure nothing.\n\ + Make sure a C compiler for the target is installed and reachable \ + by cc-rs (for musl targets, `musl-tools` provides \ + `-linux-musl-gcc`).\n\ + Compiler information: {compiler:?}\n\ + Compilation error: {e}" + ); + } + eprintln!("\n\nWARNING: Failed to compile instrument-hooks native library with cc-rs."); eprintln!( "The library will still compile, but instrument-hooks functionality will be disabled." From 48d370f83c3431d7eb1e6c522f4c3e23664cc07f Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Mon, 7 Sep 2026 12:36:43 +0200 Subject: [PATCH 05/27] ci: add throwaway COD-3218 exec-harness check workflow Removing the preload moves the callgrind client requests out of the benchmark child and up into exec-harness, so the measurement now rests on valgrind propagating instrumentation state across fork/exec and on the spawn edges valgrind-codspeed records. None of that is observable on the dev host (aarch64 Arch, no valgrind, and the CodSpeed .deb is Ubuntu-only), so this runs it on a real x86_64 runner. Asserts on the content of the .out files rather than the exit code, since the failure mode being guarded against is a run that completes happily and measures nothing: a part must carry the benchmark URI, that part must list the spawn edge, and every process in the chain must have its own .out with non-zero cost. The benchmark deliberately nests spawns (exec-harness -> sh -> seq/wc) so the chain is walked, not just one edge. Two guards against the check passing vacuously: the run is bracketed by a hash of the exec-harness on PATH, because the runner silently downloads the released preload build when the local one is missing, and the musl leg asserts the installed binary really is static. A baseline job runs the same benchmark on main to quantify the step change in reported cost that dropping the preload causes. Manual trigger only, and it must not reach main -- the header says so. Co-Authored-By: Claude Opus 5 (1M context) --- .../workflows/cod-3218-exec-harness-check.yml | 298 ++++++++++++++++++ 1 file changed, 298 insertions(+) create mode 100644 .github/workflows/cod-3218-exec-harness-check.yml diff --git a/.github/workflows/cod-3218-exec-harness-check.yml b/.github/workflows/cod-3218-exec-harness-check.yml new file mode 100644 index 00000000..66cbfae5 --- /dev/null +++ b/.github/workflows/cod-3218-exec-harness-check.yml @@ -0,0 +1,298 @@ +# COD-3218 spike — throwaway workflow, NOT for merging. +# +# Purpose: close the one gap the local work could not. Removing the LD_PRELOAD +# hack moves the callgrind client requests from inside the benchmark child up +# into exec-harness, so the measurement now depends on valgrind propagating +# instrumentation state across fork/exec and on the spawn edges recorded by +# valgrind-codspeed. None of that is observable locally: this host is aarch64 +# Arch with no valgrind, and the CodSpeed valgrind .deb is published for Ubuntu +# only. It also runs the whole flow with a *musl* exec-harness, which is the +# COD-3440 half. +# +# Exit code 0 proves nothing here: the harness runs, valgrind runs, the +# benchmark completes, and the measurement can still be empty. So this workflow +# asserts on the CONTENT of the .out files, per the verification bar: +# - a part carries the benchmark URI, and lists `desc: Spawned pid: ` +# - every process in the spawn chain has its own .out with non-zero cost +# - the summed cost is printed, and the baseline job prints the same number +# from main so the step change can be quantified (breaking change #1) +# +# To use it: push this branch, then +# +# gh workflow run cod-3218-exec-harness-check.yml --ref spike/cod-3440-memtrack-musl +# +# Manual trigger only, so it never fires on its own. Note that the very first +# dispatch of a workflow living only on a non-default branch 404s until GitHub +# has registered it; once it shows up in the Actions list, --ref dispatch works. +# Delete the file once the question is answered -- it must not reach main. +# +# Deliberately does NOT touch ci.yml, release.yml, dist-workspace.toml or any +# Cargo.toml: it only adds manually-triggered jobs. + +name: COD-3218 exec-harness check + +on: + workflow_dispatch: + +env: + MUSL_TARGET: x86_64-unknown-linux-musl + # Distinctive so it can be grepped out of the .out files unambiguously. The + # harness derives the URI as `exec_harness::`. + BENCH_NAME: cod3218_probe + # Passed as `sh -c "$BENCH_SCRIPT"`, so it holds no quoting of its own -- an + # env var cannot carry shell quotes through word splitting. + # A nested spawn on purpose: the harness forks `sh`, which forks `seq` and + # `wc`. That exercises the intermediate-forwarding case in the backend's + # spawn-chain walk, not just a single parent -> child edge. + BENCH_SCRIPT: seq 1 50000 | wc -l + +jobs: + # The COD-3440 half: the artifact the preload removal unblocks. + build-musl: + name: musl build is static + runs-on: ubuntu-latest # x86_64 + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: cod3218-musl + + - name: Install musl toolchain + # musl-tools provides x86_64-linux-musl-gcc, which cc-rs finds unaided, + # so instrument-hooks' core.c compiles for the target. Without it the + # bindings build script now fails the build outright rather than + # silently compiling the noop implementation. + run: | + sudo apt-get install -y musl-tools + rustup target add "$MUSL_TARGET" + + - name: Build + run: cargo build -p exec-harness --target "$MUSL_TARGET" + + - name: Verify the artifact is genuinely static + run: | + BIN=target/$MUSL_TARGET/debug/exec-harness + file "$BIN" + ldd "$BIN" || true # expected: "not a dynamic executable" + readelf -d "$BIN" || true # expected: no dynamic section at all + echo "size: $(stat -c %s "$BIN") bytes" + # `if` rather than `grep ... && exit 1`, because a grep that matches + # nothing exits 1 and would fail the step under bash -e. + if readelf -d "$BIN" 2>/dev/null | grep -qE 'NEEDED|RPATH|RUNPATH'; then + echo "unexpected dynamic dependency or rpath" + exit 1 + fi + echo "OK: static, no NEEDED, no RPATH/RUNPATH" + + - name: Verify no preload artifact is produced any more + run: | + if find target -name 'libcodspeed_preload*' | grep .; then + echo "the preload library is still being built" + exit 1 + fi + echo "OK: no preload library in the build output" + + # The COD-3218 half: does the measurement actually land anywhere? + instrumentation: + name: instrumentation (${{ matrix.libc }}) + runs-on: ubuntu-latest # x86_64 + strategy: + fail-fast: false + matrix: + libc: [gnu, musl] + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: cod3218-${{ matrix.libc }} + + - name: Install musl toolchain + if: matrix.libc == 'musl' + run: | + sudo apt-get install -y musl-tools + rustup target add "$MUSL_TARGET" + + # The runner resolves exec-harness off PATH and only downloads the + # released build when `which exec-harness` is missing or reports a + # version other than the pin (src/binary_installer/mod.rs). Installing + # our build first is therefore what makes this job test anything -- see + # the tamper guard in the next step. + - name: Install the exec-harness under test + run: | + if [ "${{ matrix.libc }}" = "musl" ]; then + cargo install --path crates/exec-harness --locked --target "$MUSL_TARGET" + else + cargo install --path crates/exec-harness --locked + fi + BIN=$(which exec-harness) + echo "$BIN" + exec-harness --version + file "$BIN" + # Prove the musl matrix leg is really exercising the musl artifact, + # rather than a leftover gnu build earlier on PATH. + if [ "${{ matrix.libc }}" = "musl" ]; then + file "$BIN" | grep -q 'statically linked' \ + || { echo "the installed exec-harness is not static"; exit 1; } + echo "OK: the exec-harness under test is statically linked" + fi + + - name: Run a simulation-mode benchmark + id: run + run: | + PROFILE_DIR=$RUNNER_TEMP/profile + mkdir -p "$PROFILE_DIR" + echo "profile_dir=$PROFILE_DIR" >> "$GITHUB_OUTPUT" + + # If the runner swapped in the *released* exec-harness, that would be + # the preload build and this whole job would pass while proving + # nothing. Pin the binary's hash across the run. + BEFORE=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) + + CODSPEED_LOG=debug cargo run -- exec \ + -m simulation \ + --skip-upload \ + --profile-folder "$PROFILE_DIR" \ + --name "$BENCH_NAME" \ + -- sh -c "$BENCH_SCRIPT" + + AFTER=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) + if [ "$BEFORE" != "$AFTER" ]; then + echo "the runner replaced exec-harness with the released (preload) build" + echo "=> this run measured the old code path, not the change under test" + exit 1 + fi + echo "OK: the exec-harness under test was the one used" + + - name: Show what was produced + if: always() && steps.run.outputs.profile_dir != '' + run: | + PROFILE_DIR=${{ steps.run.outputs.profile_dir }} + echo "=== files ===" + find "$PROFILE_DIR" -type f -printf '%10s %p\n' | sort -k2 + echo + echo "=== headers of every callgrind out file ===" + # Printed in full and unfiltered on purpose: the exact header spelling + # of a client-request dump is what the backend's parser keys off, and + # eyeballing it here is cheaper than guessing at it from this repo. + find "$PROFILE_DIR" -name '*.out*' -type f | sort | while read -r f; do + echo "--- $f" + grep -nE '^(version|creator|pid|part|desc|cmd|events|summary|totals):' "$f" || true + echo + done + echo "=== valgrind logs ===" + find "$PROFILE_DIR" -name 'valgrind.*.log' -type f -exec tail -n 30 {} + || true + + - name: Verify the URI is attributed and every spawned process has cost + run: | + PROFILE_DIR=${{ steps.run.outputs.profile_dir }} + URI="exec_harness::$BENCH_NAME" + + fail() { echo "FAIL: $*"; exit 1; } + # awk rather than `grep | awk`: awk always exits 0, so a benchmark + # that recorded nothing reaches the explicit check below instead of + # aborting the step through pipefail with no diagnosis. + cost_of() { awk '/^(summary|totals):/ {s+=$2} END {print s+0}' "$@"; } + + mapfile -t OUTS < <(find "$PROFILE_DIR" -name '*.out*' -type f | sort) + echo "found ${#OUTS[@]} callgrind out file(s)" + [ "${#OUTS[@]}" -ge 2 ] || fail \ + "expected at least two out files (exec-harness plus the benchmark child), got ${#OUTS[@]}" + + # 1. Some part must carry the benchmark URI. Without this the cost + # exists but is anonymous, and the backend has nothing to attribute + # it to. + mapfile -t URI_FILES < <(grep -lF -- "$URI" "${OUTS[@]}") + [ "${#URI_FILES[@]}" -ge 1 ] || fail "no out file mentions the benchmark URI '$URI'" + echo "OK: URI '$URI' found in: ${URI_FILES[*]}" + + # 2. That same file must record the spawn edge to the benchmark child. + # Asserted per file rather than per part: splitting parts apart in + # shell is not worth it, and the headers printed above show the + # part association for a human to confirm. + URI_FILE=${URI_FILES[0]} + mapfile -t SPAWNED < <(grep -oiE 'Spawned pid:[[:space:]]*[0-9]+' "$URI_FILE" \ + | grep -oE '[0-9]+' | sort -u) + [ "${#SPAWNED[@]}" -ge 1 ] || fail \ + "the URI-bearing part in $URI_FILE records no 'Spawned pid:' edge, so the child's cost cannot be attributed to the benchmark" + echo "OK: $URI_FILE spawned pid(s): ${SPAWNED[*]}" + + # 3. Walk the whole spawn chain. Each process must have its own out + # file with non-zero cost, and may itself have spawned more + # (here: exec-harness -> sh -> seq/wc). + declare -A SEEN=() + WORK=("${SPAWNED[@]}") + while [ "${#WORK[@]}" -gt 0 ]; do + PID=${WORK[0]} + WORK=("${WORK[@]:1}") + # `if` rather than `[ ... ] && continue`, which returns non-zero on + # the miss and would abort the step under bash -e. + if [ -n "${SEEN[$PID]:-}" ]; then + continue + fi + SEEN[$PID]=1 + + mapfile -t CHILD < <(find "$PROFILE_DIR" -name "$PID.out*" -type f) + [ "${#CHILD[@]}" -ge 1 ] || fail \ + "spawned pid $PID has no out file, so its cost was never recorded" + + COST=$(cost_of "${CHILD[@]}") + [ "$COST" -gt 0 ] || fail "spawned pid $PID recorded zero cost (file: ${CHILD[*]})" + echo "OK: pid $PID -> ${CHILD[*]} (Ir: $COST)" + + mapfile -t MORE < <(grep -hoiE 'Spawned pid:[[:space:]]*[0-9]+' "${CHILD[@]}" \ + | grep -oE '[0-9]+' | sort -u) + if [ "${#MORE[@]}" -gt 0 ]; then + WORK+=("${MORE[@]}") + fi + done + + echo + echo "PASS: URI attributed, ${#SEEN[@]} spawned process(es) all carry cost" + echo "TOTAL_IR(${{ matrix.libc }})=$(cost_of "${OUTS[@]}")" + + # The baseline for breaking change #1: the same benchmark on main, where the + # preload starts the measured region inside the child instead. The delta + # between this number and the one above IS the step change in reported cost. + baseline-main: + name: baseline on main (preload) + runs-on: ubuntu-latest # x86_64 + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: main + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: cod3218-baseline + + - name: Install the exec-harness from main + run: | + cargo install --path crates/exec-harness --locked + exec-harness --version + + - name: Run the same benchmark + # main's exec-harness is the preload build, and its LD_PRELOAD check + # rejects statically linked executables -- so this leg is gnu only. + run: | + PROFILE_DIR=$RUNNER_TEMP/profile + mkdir -p "$PROFILE_DIR" + + CODSPEED_LOG=debug cargo run -- exec \ + -m simulation \ + --skip-upload \ + --profile-folder "$PROFILE_DIR" \ + --name "$BENCH_NAME" \ + -- sh -c "$BENCH_SCRIPT" + + find "$PROFILE_DIR" -type f -printf '%10s %p\n' | sort -k2 + find "$PROFILE_DIR" -name '*.out*' -type f | sort | while read -r f; do + echo "--- $f" + grep -nE '^(pid|part|desc|cmd|summary|totals):' "$f" || true + done + mapfile -t OUTS < <(find "$PROFILE_DIR" -name '*.out*' -type f | sort) + TOTAL=$(awk '/^(summary|totals):/ {s+=$2} END {print s+0}' "${OUTS[@]}") + echo "TOTAL_IR(main-preload)=$TOTAL" From 4edc22863b38b5b8263640f3a24d4297bdd47132 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Mon, 7 Sep 2026 12:50:03 +0200 Subject: [PATCH 06/27] ci: trigger the COD-3218 check on spike branch pushes `gh workflow run` answers "HTTP 404: workflow not found on the default branch" for a workflow_dispatch-only file that has never existed on main, and it stays that way: GitHub does not register such a file on its own, so waiting and retrying the dispatch gets nowhere. A push trigger is what forces registration -- GitHub runs the file on push and assigns it an id, after which --ref dispatch works too. The COD-3440 workflow next door was registered exactly this way; its first run is a `push` one from a commit that temporarily added the same trigger. Recorded in the `on:` block so the next person does not rediscover it. Scoped to the spike branch, and it goes away with the file. Co-Authored-By: Claude Opus 5 (1M context) --- .../workflows/cod-3218-exec-harness-check.yml | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/.github/workflows/cod-3218-exec-harness-check.yml b/.github/workflows/cod-3218-exec-harness-check.yml index 66cbfae5..697ed556 100644 --- a/.github/workflows/cod-3218-exec-harness-check.yml +++ b/.github/workflows/cod-3218-exec-harness-check.yml @@ -17,22 +17,36 @@ # - the summed cost is printed, and the baseline job prints the same number # from main so the step change can be quantified (breaking change #1) # -# To use it: push this branch, then +# To use it: push this branch. The push itself runs the workflow -- see the +# `on:` block for why a push trigger is required rather than optional. After +# that first run has registered the file, it can also be re-run by hand: # # gh workflow run cod-3218-exec-harness-check.yml --ref spike/cod-3440-memtrack-musl # -# Manual trigger only, so it never fires on its own. Note that the very first -# dispatch of a workflow living only on a non-default branch 404s until GitHub -# has registered it; once it shows up in the Actions list, --ref dispatch works. -# Delete the file once the question is answered -- it must not reach main. +# The push trigger is scoped to the spike branch, so it cannot fire anywhere +# else. Delete the file once the question is answered -- it must not reach main. # # Deliberately does NOT touch ci.yml, release.yml, dist-workspace.toml or any -# Cargo.toml: it only adds manually-triggered jobs. +# Cargo.toml: it only adds jobs that run on this branch. name: COD-3218 exec-harness check on: workflow_dispatch: + # The push trigger is what makes this workflow dispatchable at all, and it is + # not optional. A workflow_dispatch-only file that has never existed on the + # default branch is never registered by GitHub: `gh workflow run` answers + # "HTTP 404: workflow ... not found on the default branch" indefinitely, and + # it does NOT register itself over time -- retrying is useless. A push trigger + # forces registration, because GitHub runs the file on push and assigns it an + # id, after which `--ref` dispatch works too. The COD-3440 workflow next door + # was registered exactly this way; its first run is a `push` one from a commit + # that temporarily added this same trigger. + # + # Scoped to the spike branch so it cannot fire anywhere else, and it goes away + # when this throwaway file is deleted. + push: + branches: [spike/cod-3440-memtrack-musl] env: MUSL_TARGET: x86_64-unknown-linux-musl From 59c9756c571186c34ce1f7b1583b1ec984aefdc8 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Mon, 7 Sep 2026 13:01:19 +0200 Subject: [PATCH 07/27] ci: fix three wrong assertions in the COD-3218 check The first run found all three; `instrumentation (gnu)` passed and met the verification bar, so the change itself is fine. 1. The musl leg asserted `file` says "statically linked". rustc emits a static-PIE for x86_64 musl, which `file` calls "static-pie linked"; only aarch64 gets the non-PIE spelling, which is why this passed locally and failed on CI. The binary was static all along -- the `build-musl` job's readelf check confirms no NEEDED/RPATH. Assert through readelf instead: no DT_NEEDED and no interpreter, which is the property we mean. 2. Cost was summed over `summary:` AND `totals:`. Child dumps carry both with near-equal values, so their cost was counted twice -- and only for some files, which inflated the branch total to 11508827 against main's 4819368 and made the comparison meaningless. On `totals:` alone it is 5924410 vs 4688549. Same fix in the baseline job. 3. The URI and the spawn edge were only required to be in the same FILE. The real dumps put both on the same PART, which is the invariant the backend walks: attribution starts at the URI-bearing part and follows its edges, so an edge on a neighbouring part would not attribute anything. Extract the spawn pids from the URI-bearing part itself. Also prints a per-file cost breakdown, so the comparison can be read without digging through the headers. Re-tested against synthetic dumps in the shape the run actually produced: the happy path passes and six failure modes each fail with the right diagnosis, including the new same-file-different-part case that 3. adds. Co-Authored-By: Claude Opus 5 (1M context) --- .../workflows/cod-3218-exec-harness-check.yml | 76 ++++++++++++++----- 1 file changed, 58 insertions(+), 18 deletions(-) diff --git a/.github/workflows/cod-3218-exec-harness-check.yml b/.github/workflows/cod-3218-exec-harness-check.yml index 697ed556..87b64467 100644 --- a/.github/workflows/cod-3218-exec-harness-check.yml +++ b/.github/workflows/cod-3218-exec-harness-check.yml @@ -147,11 +147,22 @@ jobs: exec-harness --version file "$BIN" # Prove the musl matrix leg is really exercising the musl artifact, - # rather than a leftover gnu build earlier on PATH. + # rather than a leftover gnu build earlier on PATH. Asserted through + # readelf and not a `file` string: rustc emits a static-PIE for + # x86_64 musl, which `file` calls "static-pie linked" rather than + # "statically linked" (aarch64 gets the non-PIE spelling), so matching + # that wording tests the wrong axis and fails on a perfectly static + # binary. What matters is that nothing is loaded at runtime. if [ "${{ matrix.libc }}" = "musl" ]; then - file "$BIN" | grep -q 'statically linked' \ - || { echo "the installed exec-harness is not static"; exit 1; } - echo "OK: the exec-harness under test is statically linked" + if readelf -d "$BIN" 2>/dev/null | grep -qE 'NEEDED|RPATH|RUNPATH'; then + echo "the installed exec-harness has a dynamic dependency" + exit 1 + fi + if readelf -lW "$BIN" 2>/dev/null | grep -q 'INTERP'; then + echo "the installed exec-harness requests a dynamic loader" + exit 1 + fi + echo "OK: no NEEDED and no interpreter -- nothing is loaded at runtime" fi - name: Run a simulation-mode benchmark @@ -206,10 +217,30 @@ jobs: URI="exec_harness::$BENCH_NAME" fail() { echo "FAIL: $*"; exit 1; } - # awk rather than `grep | awk`: awk always exits 0, so a benchmark - # that recorded nothing reaches the explicit check below instead of - # aborting the step through pipefail with no diagnosis. - cost_of() { awk '/^(summary|totals):/ {s+=$2} END {print s+0}' "$@"; } + + # `totals:` only, NOT `summary:`. Child dumps carry both, with nearly + # equal values (a part's summary and the file's totals), so summing + # both silently doubles the reported cost -- and it doubles it only + # for some files, which made the branch/main comparison meaningless. + # awk rather than `grep | awk` because awk always exits 0, so a + # benchmark that recorded nothing reaches the explicit check below + # instead of aborting the step through pipefail with no diagnosis. + cost_of() { awk '/^totals:/ {s+=$2} END {print s+0}' "$@"; } + + # Spawned pids of the dump part that carries the URI. This is the + # invariant the backend actually walks: the URI and the spawn edge + # have to be on the SAME part, not merely in the same file, since + # attribution starts from the URI-bearing part and follows its edges. + spawns_of_uri_part() { + awk -v uri="$1" ' + /^part: / { if (hasuri && pids != "") { print pids; found=1; exit } + hasuri=0; pids=""; next } + index($0, "Client Request: " uri) { hasuri=1 } + /^desc: Spawned pid:/ { p=$0; sub(/.*Spawned pid:[[:space:]]*/,"",p) + pids = pids " " p } + END { if (!found && hasuri && pids != "") print pids } + ' "$2" + } mapfile -t OUTS < <(find "$PROFILE_DIR" -name '*.out*' -type f | sort) echo "found ${#OUTS[@]} callgrind out file(s)" @@ -218,21 +249,18 @@ jobs: # 1. Some part must carry the benchmark URI. Without this the cost # exists but is anonymous, and the backend has nothing to attribute - # it to. + # it to. Post-preload this is exec-harness's own dump: the children + # are NOT labelled any more, which is exactly why 2. and 3. matter. mapfile -t URI_FILES < <(grep -lF -- "$URI" "${OUTS[@]}") [ "${#URI_FILES[@]}" -ge 1 ] || fail "no out file mentions the benchmark URI '$URI'" echo "OK: URI '$URI' found in: ${URI_FILES[*]}" - # 2. That same file must record the spawn edge to the benchmark child. - # Asserted per file rather than per part: splitting parts apart in - # shell is not worth it, and the headers printed above show the - # part association for a human to confirm. + # 2. The URI-bearing part must record the spawn edge to the child. URI_FILE=${URI_FILES[0]} - mapfile -t SPAWNED < <(grep -oiE 'Spawned pid:[[:space:]]*[0-9]+' "$URI_FILE" \ - | grep -oE '[0-9]+' | sort -u) + read -r -a SPAWNED <<< "$(spawns_of_uri_part "$URI" "$URI_FILE")" [ "${#SPAWNED[@]}" -ge 1 ] || fail \ - "the URI-bearing part in $URI_FILE records no 'Spawned pid:' edge, so the child's cost cannot be attributed to the benchmark" - echo "OK: $URI_FILE spawned pid(s): ${SPAWNED[*]}" + "the URI-bearing part of $URI_FILE records no 'Spawned pid:' edge, so the child's cost cannot be attributed to the benchmark" + echo "OK: the URI-bearing part of $URI_FILE spawned pid(s): ${SPAWNED[*]}" # 3. Walk the whole spawn chain. Each process must have its own out # file with non-zero cost, and may itself have spawned more @@ -266,6 +294,11 @@ jobs: echo echo "PASS: URI attributed, ${#SEEN[@]} spawned process(es) all carry cost" + # Per-file breakdown, so the branch/main comparison can be read + # without digging through the headers above. + for f in "${OUTS[@]}"; do + printf ' %-14s Ir=%s\n' "$(basename "$f")" "$(cost_of "$f")" + done echo "TOTAL_IR(${{ matrix.libc }})=$(cost_of "${OUTS[@]}")" # The baseline for breaking change #1: the same benchmark on main, where the @@ -308,5 +341,12 @@ jobs: grep -nE '^(pid|part|desc|cmd|summary|totals):' "$f" || true done mapfile -t OUTS < <(find "$PROFILE_DIR" -name '*.out*' -type f | sort) - TOTAL=$(awk '/^(summary|totals):/ {s+=$2} END {print s+0}' "${OUTS[@]}") + # `totals:` only, to match the instrumentation job -- see the comment + # on cost_of there. Summing `summary:` as well doubles the figure for + # some files and not others, which would make this comparison lie. + TOTAL=$(awk '/^totals:/ {s+=$2} END {print s+0}' "${OUTS[@]}") + for f in "${OUTS[@]}"; do + printf ' %-14s Ir=%s\n' "$(basename "$f")" \ + "$(awk '/^totals:/ {s+=$2} END {print s+0}' "$f")" + done echo "TOTAL_IR(main-preload)=$TOTAL" From 11785e49e3ba491bfc2510105ee096cc1df18f6f Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Mon, 7 Sep 2026 14:17:54 +0200 Subject: [PATCH 08/27] ci: sweep benchmark size to test the fixed-overhead model The single fixed-size probe reports +26.4%, which is dominated by fixed per-process startup and so says nothing useful about a real benchmark. Extrapolating ~+7% from the one process that did real work (`seq`) is a guess, not a measurement. This holds the process shape identical across sizes (exec-harness -> sh -> seq) and varies only the work, which makes the model falsifiable: if the shift really is a fixed per-process cost, `branch - main` stays roughly CONSTANT in absolute Ir as N grows while the ratio collapses towards 1. If the delta instead grows with N, the cost is proportional and the "only matters for tiny benchmarks" reading is wrong. Both variants run the same sizes through the same script, at pinned commits (github.sha rather than the branch name, which may move), so the pairs are directly comparable. Carries the same exec-harness tamper guard as the instrumentation job. Throwaway, like the rest of this workflow -- delete once the number is recorded on the ticket. Co-Authored-By: Claude Opus 5 (1M context) --- .../workflows/cod-3218-exec-harness-check.yml | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/.github/workflows/cod-3218-exec-harness-check.yml b/.github/workflows/cod-3218-exec-harness-check.yml index 87b64467..4e4ba4a3 100644 --- a/.github/workflows/cod-3218-exec-harness-check.yml +++ b/.github/workflows/cod-3218-exec-harness-check.yml @@ -350,3 +350,69 @@ jobs: "$(awk '/^totals:/ {s+=$2} END {print s+0}' "$f")" done echo "TOTAL_IR(main-preload)=$TOTAL" + + # Quantifies breaking change #1 properly, which the single fixed-size probe + # above cannot: it reports one number (+26 %) that is dominated by fixed + # per-process startup and so says nothing about a real benchmark. + # + # The process shape is held IDENTICAL across sizes (exec-harness -> sh -> seq) + # and only the work varies, which makes the model falsifiable: if the shift + # really is a fixed per-process cost, then `branch - main` stays roughly + # CONSTANT in absolute Ir as N grows while the ratio collapses towards 1. If + # instead the delta grows with N, the cost is proportional and the whole + # "it only matters for tiny benchmarks" reading is wrong. + # + # Both variants run the same sizes through the same script, so the pairs are + # directly comparable. Delete this job once the number is recorded on the + # ticket. + cost-sweep: + name: cost sweep (${{ matrix.variant }}) + runs-on: ubuntu-latest # x86_64 + strategy: + fail-fast: false + matrix: + variant: [branch, main] + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + # github.sha rather than the branch name: the branch may have moved on + # by the time this runs, and the two variants must be pinned commits + # for the comparison to mean anything. + ref: ${{ matrix.variant == 'main' && 'main' || github.sha }} + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: cod3218-sweep-${{ matrix.variant }} + + - name: Install the exec-harness under test + run: | + cargo install --path crates/exec-harness --locked + exec-harness --version + + - name: Sweep + run: | + # Same tamper guard as the instrumentation job: if the runner swapped + # in the released exec-harness, the sweep would silently measure the + # wrong binary on the branch variant. + BEFORE=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) + + for N in 1000 10000 100000 1000000; do + D=$RUNNER_TEMP/sweep-$N + mkdir -p "$D" + CODSPEED_LOG=warn cargo run -q -- exec \ + -m simulation \ + --skip-upload \ + --profile-folder "$D" \ + --name "sweep_$N" \ + -- sh -c "seq 1 $N > /dev/null" + + mapfile -t OUTS < <(find "$D" -name '*.out*' -type f | sort) + TOTAL=$(awk '/^totals:/ {s+=$2} END {print s+0}' "${OUTS[@]}") + echo "SWEEP variant=${{ matrix.variant }} N=$N files=${#OUTS[@]} TOTAL_IR=$TOTAL" + done + + AFTER=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) + if [ "$BEFORE" != "$AFTER" ]; then + echo "the runner replaced exec-harness mid-sweep; results are not trustworthy" + exit 1 + fi From d7d36bee1ba8eacf135a4d84bc02cbb6e03c84ae Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Mon, 7 Sep 2026 16:16:31 +0200 Subject: [PATCH 09/27] revert: restore measure.rs and shared.rs to their state on main Keeps the change component-local. The goal is to unblock the musl build by removing the preload; forcing `--instr-atstart=inherit` on every simulation run was a bigger behavioural change than that needs, and it reached runs it had no business touching: Entrypoint runs (`cargo codspeed run`, pytest-codspeed) got `inherit` too. Harmless for a benchmark that never forks -- top-level `inherit` starts instrumentation off, same as `no` -- but an entrypoint benchmark that DOES fork would suddenly have its children instrumented and counted, silently changing its numbers. That is presumably why the flag was opt-in to begin with. It may also be unnecessary. Per COD-2349 the instrumentation state crosses `exec` by injecting `--instr-atstart=yes|no` into the child valgrind's argv (`VG_(needs_child_exec_args)`), not via the top-level flag; `inherit` covers the fork-only case. exec-harness spawns with `Command::status()`, i.e. fork + exec, so the child should pick the state up through the argv channel whatever the top level says. The check on this branch will confirm or refute that -- and with `src/` now identical to main, it isolates the exec-harness change on its own. If it turns out the runner does need a nudge, the shape to use is deriving it from `uses_exec_harness` (already threaded to `executor_config_for_command`) rather than hardcoding it here, so entrypoint runs keep their current behaviour. Co-Authored-By: Claude Opus 5 (1M context) --- src/cli/shared.rs | 6 +----- src/executor/valgrind/measure.rs | 9 ++------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/cli/shared.rs b/src/cli/shared.rs index b926af04..1fe13474 100644 --- a/src/cli/shared.rs +++ b/src/cli/shared.rs @@ -135,11 +135,7 @@ pub struct ExecAndRunSharedArgs { )] pub exclude_allocations: bool, - /// Emit per-thread dumps for the benchmarked process in simulation mode. - /// - /// Subprocesses spawned by the benchmarked process are now always measured, - /// so this only controls Valgrind's `--separate-threads`. The flag keeps its - /// name for compatibility; renaming it would break existing invocations. + /// Measure the subprocesses spawned by the benchmarked process in simulation mode. #[arg(long, env = "CODSPEED_SIMULATION_TRACK_SUBPROCESS")] pub simulation_track_subprocess: bool, diff --git a/src/executor/valgrind/measure.rs b/src/executor/valgrind/measure.rs index 5c395753..62807b92 100644 --- a/src/executor/valgrind/measure.rs +++ b/src/executor/valgrind/measure.rs @@ -33,16 +33,11 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec Date: Mon, 7 Sep 2026 16:26:01 +0200 Subject: [PATCH 10/27] fix(valgrind): track subprocesses for exec-harness runs Removing the preload moved the instrumentation toggles out of the benchmark child and into exec-harness, which forks it. Measured on CI: with `--instr-atstart=no` the child dumps a single zero-cost `Trigger: Program termination` part, so the benchmark reports nothing at all -- the parent's live instrumentation state does not reach the child on its own. `--instr-atstart=inherit` is what enables that propagation; the argv-injection channel COD-2349 added for `exec` is not sufficient by itself. Derive it from `uses_exec_harness`, which the orchestrator already threads down to `executor_config_for_command`, rather than making `--instr-atstart=inherit` unconditional in `get_valgrind_args`. That leaves `measure.rs` untouched and keeps entrypoint runs on exactly their current behaviour, which matters: an entrypoint benchmark that forks would otherwise start having its children instrumented and counted, silently changing its numbers. The parameter was already `!uses_exec_harness` at the call site, for `enable_introspection`; it now passes the positive form and both derived values are computed inside. `ExecutorConfig::test()` passes `false`, which reproduces its previous field values exactly -- its target is an entrypoint one. Co-Authored-By: Claude Opus 5 (1M context) --- src/executor/config.rs | 26 ++++++++++++++++++++------ src/executor/orchestrator.rs | 2 +- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/executor/config.rs b/src/executor/config.rs index 39f95851..f81302e6 100644 --- a/src/executor/config.rs +++ b/src/executor/config.rs @@ -193,12 +193,26 @@ impl OrchestratorConfig { /// Produce a per-execution [`ExecutorConfig`] for the given command and mode. /// - /// `enable_introspection` controls whether language-level wrappers (Node.js, Go) - /// are injected into `PATH`. This should be `false` for exec-harness targets. + /// `uses_exec_harness` says whether this run is driven by exec-harness rather + /// than being a plain entrypoint command. Two things are derived from it: + /// + /// - Language-level wrappers (Node.js, Go) are injected into `PATH` only for + /// entrypoint runs. + /// - Subprocess tracking is forced on for exec-harness runs. exec-harness + /// toggles instrumentation in its own process and then forks the benchmark, + /// so the benchmarked child is measured only if valgrind propagates that + /// state across `fork`/`exec` — which is what `--instr-atstart=inherit` + /// enables. Measured: with `--instr-atstart=no` the child dumps a single + /// zero-cost part and the benchmark reports nothing at all. + /// + /// Deriving it here rather than making `--instr-atstart=inherit` + /// unconditional keeps entrypoint runs on their current behaviour. That + /// matters: an entrypoint benchmark that forks would otherwise start having + /// its children instrumented and counted, silently changing its numbers. pub fn executor_config_for_command( &self, command: String, - enable_introspection: bool, + uses_exec_harness: bool, ) -> ExecutorConfig { ExecutorConfig { working_directory: self.working_directory.clone(), @@ -212,11 +226,11 @@ impl OrchestratorConfig { allow_empty: self.allow_empty, go_runner_version: self.go_runner_version.clone(), extra_env: self.extra_env.clone(), - enable_introspection, + enable_introspection: !uses_exec_harness, fair_sched: self.fair_sched, cycle_estimation: self.cycle_estimation, exclude_allocations: self.exclude_allocations, - simulation_track_subprocess: self.simulation_track_subprocess, + simulation_track_subprocess: self.simulation_track_subprocess || uses_exec_harness, memory_track_physical: self.memory_track_physical, } } @@ -262,7 +276,7 @@ impl OrchestratorConfig { impl ExecutorConfig { /// Constructs a new `ExecutorConfig` with default values for testing purposes pub fn test() -> Self { - OrchestratorConfig::test().executor_config_for_command("".into(), true) + OrchestratorConfig::test().executor_config_for_command("".into(), false) } } diff --git a/src/executor/orchestrator.rs b/src/executor/orchestrator.rs index ca2dbdf4..6d3646d8 100644 --- a/src/executor/orchestrator.rs +++ b/src/executor/orchestrator.rs @@ -143,7 +143,7 @@ impl Orchestrator { for (run_part_index, part) in run_parts.into_iter().enumerate() { let config = self .config - .executor_config_for_command(part.command, !part.uses_exec_harness); + .executor_config_for_command(part.command, part.uses_exec_harness); let mut executor = get_executor_from_mode(part.mode, self.config.walltime_profiler); let profile_folder = self.resolve_profile_folder(&executor.name(), run_part_index, total_parts)?; From c43aaba6195121cb71688f5715d073360e914281 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Wed, 16 Sep 2026 12:02:45 -0400 Subject: [PATCH 11/27] build(memtrack): move the portable half of the musl recipe into cargo config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spike proved memtrack builds on musl, but the recipe lived entirely in out-of-band env vars. Split it: what is a property of the repo moves in, what is a property of the build machine stays out. The three `ac_cv_search_*` cache seeds go in unconditionally rather than per-target. `argp_parse`, `_obstack_free` and `fts_close` all live in glibc's libc, so "none required" is the answer a gnu host reaches on its own — seeding it only skips three `configure` probes there and cannot change the outcome. The aarch64 `-lgcc` becomes `[target.aarch64-unknown-linux-musl] rustflags`, which cargo scopes natively. rustc links with `-nodefaultlibs`, so gcc never pulls in libgcc, and libbpf's C code needs the outline-atomic helpers that live there. What is deliberately NOT here is the include flags. `[env]` cannot express `-I` — its `relative = true` form resolves a bare path, with nowhere to put the `-I` — but the real reason is that `-idirafter /usr/include/` describes Debian's header layout, not ours. An Arch host needs different values. Checking one in would just be picking a distro. Co-Authored-By: Claude Opus 5 (1M context) --- .cargo/config.toml | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 00000000..f8b7ffa0 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,31 @@ +# COD-3440 — what a musl build of `memtrack` needs, for the parts that are a +# property of the repo rather than of the machine doing the build. +# +# `libbpf-sys` vendors elfutils, whose `configure` unconditionally looks for +# `argp`, `obstack` and `fts`. musl ships none of them, so the checks fail and +# the build stops before it ever gets to libelf. A libelf-only build never calls +# into any of them, though — the checks are there for the elfutils CLI tools, +# which we do not build. +# +# Pre-seeding autoconf's cache makes `configure` skip those three checks +# entirely. "none required" means "the symbol is available with no extra -l +# flag", which is the answer a glibc host would have reached on its own, so +# these are set unconditionally rather than per-target: they are correct for the +# gnu build too, where they only save three `configure` probes. +# +# This is deliberately the whole of what lives here. The remaining piece of the +# recipe — the include flags that point at the `argp.h` stub in +# `crates/memtrack/musl/` and at the kernel UAPI headers — cannot live in a +# `[env]` table, and should not: see `.github/workflows/cod-3440-musl-check.yml` +# for where it goes and why. +[env] +ac_cv_search_argp_parse = "none required" +ac_cv_search__obstack_free = "none required" +ac_cv_search_fts_close = "none required" + +# rustc links with `-nodefaultlibs`, so gcc does not pull in libgcc. On aarch64, +# libbpf's C code needs the outline-atomic helpers (`__aarch64_ldadd4_sync` and +# friends) that live there, and the link fails without it. x86_64 has no such +# helpers and needs nothing. +[target.aarch64-unknown-linux-musl] +rustflags = ["-C", "link-arg=-lgcc"] From 21525d5c2274370aa0c159e7a2e6db8fd79639ba Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Wed, 16 Sep 2026 12:02:53 -0400 Subject: [PATCH 12/27] test(memtrack): resolve libc symbols in a child, not in the test process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `libc_allocator_symbols_resolve_to_offsets` read `/proc/self/maps` and required a mapped `libc.so.6`. That only ever worked because the test binary happens to be dynamically linked against glibc: under static musl there is no such mapping by construction, and the test failed for a reason that said nothing about symbol resolution — which is why the musl check workflow had to `--skip` it. Read a spawned child's maps instead. That is static-safe, and it is also what the production path does: symbols are resolved in a *traced* process, never in memtrack's own. The child is killed and reaped on the panic path too, so a failed assertion does not leak a `sleep`. Passes on both aarch64 gnu and aarch64 static musl (19/19 `--lib`). Co-Authored-By: Claude Opus 5 (1M context) --- crates/memtrack/src/ebpf/memtrack/mod.rs | 59 ++++++++++++++++++++---- 1 file changed, 51 insertions(+), 8 deletions(-) diff --git a/crates/memtrack/src/ebpf/memtrack/mod.rs b/crates/memtrack/src/ebpf/memtrack/mod.rs index 2ec1f04f..22afdec1 100644 --- a/crates/memtrack/src/ebpf/memtrack/mod.rs +++ b/crates/memtrack/src/ebpf/memtrack/mod.rs @@ -282,19 +282,62 @@ impl Drop for MemtrackBpf { #[cfg(test)] mod tests { use super::*; + use std::process::{Child, Command, Stdio}; + use std::time::{Duration, Instant}; + + /// A spawned child, killed and reaped when it goes out of scope, including + /// on the panic path of a failed assertion. + struct Reaped(Child); + + impl Drop for Reaped { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } + + /// The `libc.so.6` that a freshly spawned child has mapped. + /// + /// Read from a child rather than from `/proc/self/maps`: a static musl + /// build of this binary maps no `libc.so.6` at all, and the production path + /// resolves symbols in a traced process anyway, never in memtrack's own. + fn libc_mapped_by_a_child() -> String { + let child = Reaped( + Command::new("sleep") + .arg("30") + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("could not spawn `sleep`"), + ); + let pid = child.0.id(); + + // The mapping is made by the child's dynamic linker, which has not + // necessarily run by the time `spawn` returns. + let deadline = Instant::now() + Duration::from_secs(5); + loop { + if let Ok(maps) = std::fs::read_to_string(format!("/proc/{pid}/maps")) { + if let Some(path) = maps.lines().find_map(|line| { + let path = line.split_whitespace().last()?; + path.contains("libc.so.6").then(|| path.to_owned()) + }) { + return path; + } + } + assert!( + Instant::now() < deadline, + "pid {pid} mapped no libc.so.6 within 5s — is `sleep` statically \ + linked or built against a non-glibc libc on this host?" + ); + std::thread::sleep(Duration::from_millis(10)); + } + } /// Allocator entry points must resolve to file offsets; a symbol that /// silently fails to resolve attaches nothing and loses all events. #[test] fn libc_allocator_symbols_resolve_to_offsets() { - let maps = std::fs::read_to_string("/proc/self/maps").unwrap(); - let libc_path = maps - .lines() - .find_map(|line| { - let path = line.split_whitespace().last()?; - path.contains("libc.so.6").then(|| path.to_owned()) - }) - .expect("test process has no mapped libc.so.6"); + let libc_path = libc_mapped_by_a_child(); let symbols = resolve_symbol_offsets(Path::new(&libc_path)).unwrap(); for symbol in ["malloc", "calloc", "realloc", "free"] { From 6bb8b751e89b6a6ae902d0bc7a3c9b47846784ca Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Wed, 16 Sep 2026 12:03:03 -0400 Subject: [PATCH 13/27] ci: cover both arches in the COD-3440 musl check, and scope its CFLAGS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Still throwaway, still must not reach main. Three changes. The seeds and `-lgcc` are gone from here — they now live in `.cargo/config.toml`, so the sudo `env` passthrough no longer has to smuggle them into the test run. The include flags move from `CFLAGS` to `CFLAGS_`. cc-rs reads `CFLAGS_`, `TARGET_CFLAGS` and `CFLAGS` and accumulates them, so a target-scoped name reaches the musl build and is invisible to the gnu one; the `unset CFLAGS` dance before the comparison build is gone. It also subsumes `LIBBPF_SYS_EXTRA_CFLAGS`: libbpf-sys forwards `compiler.cflags_env()` to elfutils' configure, to zlib's, and to libbpf's make, appending `LIBBPF_SYS_EXTRA_CFLAGS` only to the last of the three. And every job now runs on both `ubuntu-latest` and `ubuntu-24.04-arm`. `arch` has to be a real matrix dimension rather than something the `include` entries introduce: an include entry whose keys are all new merges into *every* combination, so the second would have overwritten the first and both jobs would have ended up aarch64. The unit job no longer skips `libc_allocator_symbols_resolve_to_offsets`, which no longer assumes a dynamically linked test binary. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/cod-3440-musl-check.yml | 213 +++++++++++++++------- 1 file changed, 149 insertions(+), 64 deletions(-) diff --git a/.github/workflows/cod-3440-musl-check.yml b/.github/workflows/cod-3440-musl-check.yml index a0d6ec28..3e3fc159 100644 --- a/.github/workflows/cod-3440-musl-check.yml +++ b/.github/workflows/cod-3440-musl-check.yml @@ -1,8 +1,17 @@ # COD-3440 spike — throwaway workflow, NOT for merging. # -# Purpose: prove the musl recipe on a real x86_64 kernel, which is the one gap -# the local spike could not close (the local host is aarch64; an x86_64 build -# cross-compiled there cannot load its BPF skeleton against an aarch64 kernel). +# Purpose: prove the musl recipe on real kernels of both architectures. The dev +# host is aarch64, and an x86_64 build cross-compiled there cannot load its BPF +# skeleton against an aarch64 kernel, so x86_64 can only be closed here. +# +# The aarch64 half of the recipe has since been reproduced locally on Ubuntu +# 24.04 aarch64 — same distro family as the runners, unlike the Arch host the +# original spike ran on, which is where the `-fno-link-libatomic` and +# `-mno-outline-atomics` workarounds came from. Neither is needed on Debian: +# `cargo build -p memtrack --target aarch64-unknown-linux-musl`, with +# CFLAGS_ as the only out-of-band variable, produces a fully static +# binary and the whole `--lib` suite passes. What this workflow adds on top is +# the x86_64 kernel, the integration shards, and the BPF load itself. # # To use it: push the spike branch, then # @@ -15,53 +24,81 @@ # reach main. # # Deliberately does NOT touch release.yml, dist-workspace.toml or any -# Cargo.toml — it only adds one manually-triggered job. +# Cargo.toml. +# +# --- what this workflow still carries, and why --- +# +# The three autoconf cache seeds and the aarch64 `-lgcc` link flag have moved to +# `.cargo/config.toml`: they are properties of the repo, portable, and correct +# for the gnu build too. What is left here is the one part that genuinely +# belongs to the build machine — the include flags: +# +# -I/crates/memtrack/musl the argp.h stub, an absolute path, and a +# `[env]` table cannot build one (its +# `relative = true` form yields a bare path, +# with nowhere to put the -I). +# -idirafter /usr/include/ -idirafter /usr/include +# Debian's kernel UAPI headers. musl-gcc runs +# with -nostdinc and only sees +# /usr/include/-linux-musl, so libbpf's +# and have to be +# added back, last, behind musl's own headers. +# These paths are Debian's layout; an Arch or +# Alpine host needs different ones, so no +# value checked into the repo could be right +# for everyone. +# +# Both go into `CFLAGS_` rather than `CFLAGS`. cc-rs reads +# CFLAGS_, CFLAGS_, TARGET_CFLAGS and CFLAGS, +# and accumulates them, so the target-scoped name reaches the musl build and is +# invisible to the gnu one. That is why the "compare against the gnu build" step +# below no longer has to `unset CFLAGS` first. # -# Two differences from the local spike recipe, both simplifications: -# - musl-tools provides x86_64-linux-musl-gcc, which cc-rs finds on its own, -# so no compiler wrapper and no -lgcc / -fno-link-libatomic. -# - no zig, so none of the zig artifacts (crt duplication, UBSan trap, -# unknown warning options) apply. -# The only thing carried over is what the spike is actually about: the three -# autoconf cache seeds and the crates/memtrack/musl/argp.h stub. +# It also replaces LIBBPF_SYS_EXTRA_CFLAGS, which this workflow used to set as +# well. libbpf-sys forwards `compiler.cflags_env()` to elfutils' ./configure, +# to zlib's, and to libbpf's make, appending LIBBPF_SYS_EXTRA_CFLAGS only to the +# last of those — so CFLAGS_ alone covers all three. If libbpf (not +# elfutils) is what fails to find , that assumption is what broke. name: COD-3440 musl check on: workflow_dispatch: -env: - TARGET: x86_64-unknown-linux-musl - # Absolute, because the test step runs with working-directory: crates/memtrack, - # where a $PWD-relative path would resolve to the wrong place. - STUB_INCLUDE: ${{ github.workspace }}/crates/memtrack/musl - # libbpf includes and ; Debian's musl-gcc runs with - # -nostdinc and only /usr/include/x86_64-linux-musl on the include path, so the - # kernel UAPI headers from linux-libc-dev have to be added back. -idirafter puts - # them last, behind musl's own headers. build.rs forwards this to libbpf's make; - # CFLAGS alone would not reach it the same way. - LIBBPF_SYS_EXTRA_CFLAGS: -idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include - # Approach A: pre-seed autoconf's cache so the vendored elfutils never runs - # the checks musl cannot satisfy. "none required" = available with no -l flag. - ac_cv_search_argp_parse: none required - ac_cv_search__obstack_free: none required - ac_cv_search_fts_close: none required - jobs: build: - name: build ${{ matrix.profile }} - runs-on: ubuntu-latest # x86_64 + name: build ${{ matrix.arch }} ${{ matrix.profile }} + runs-on: ${{ matrix.runner }} strategy: fail-fast: false matrix: profile: [dev, dist] + # `arch` has to be a real dimension, not something the `include` entries + # introduce on their own: an include entry whose keys are all new is + # merged into *every* combination, so the second one would overwrite the + # first and both jobs would end up aarch64. Listing it here makes each + # entry match on `arch` and fill in only its own combinations. + arch: [x86_64, aarch64] + include: + - arch: x86_64 + runner: ubuntu-latest + target: x86_64-unknown-linux-musl + cflags_var: CFLAGS_x86_64_unknown_linux_musl + triplet: x86_64-linux-gnu + - arch: aarch64 + runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + cflags_var: CFLAGS_aarch64_unknown_linux_musl + triplet: aarch64-linux-gnu + env: + TARGET: ${{ matrix.target }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: submodules: true - uses: ./.github/actions/install-rust with: - cache-key: musl-${{ matrix.profile }} + cache-key: musl-${{ matrix.arch }}-${{ matrix.profile }} - uses: ./.github/actions/install-bpf-deps - name: Install musl toolchain @@ -69,13 +106,19 @@ jobs: sudo apt-get install -y musl-tools pkg-config linux-libc-dev rustup target add "$TARGET" - - name: Build - # elfutils only honours CFLAGS/CPPFLAGS, not LIBBPF_SYS_EXTRA_CFLAGS. - # The stub header exists because elfutils #include even though - # a libelf-only build never calls into it. + # Exported once here so every later step sees the same value, and so the + # `cargo build` below is a plain one with nothing prepended to it. + - name: Point the musl build at the stub and the UAPI headers run: | - export CFLAGS="-I$STUB_INCLUDE" - cargo build -p memtrack --profile ${{ matrix.profile }} --target "$TARGET" + VALUE="-I$GITHUB_WORKSPACE/crates/memtrack/musl -idirafter /usr/include/${{ matrix.triplet }} -idirafter /usr/include" + # The target-scoped name is what cc-rs reads; MUSL_CFLAGS is a plain + # mirror of it, so later steps can pass the value on without having to + # index the env context by a matrix-computed key. + echo "${{ matrix.cflags_var }}=$VALUE" >> "$GITHUB_ENV" + echo "MUSL_CFLAGS=$VALUE" >> "$GITHUB_ENV" + + - name: Build + run: cargo build -p memtrack --profile ${{ matrix.profile }} --target "$TARGET" - name: Verify the artifact is genuinely static run: | @@ -93,10 +136,12 @@ jobs: fi echo "OK: static, no NEEDED, no RPATH/RUNPATH" + # No `unset CFLAGS` any more: the include flags are scoped to the musl + # target, so a gnu build in the same shell cannot see them. If this step + # fails on a missing or wrong argp.h, that scoping is what leaked. - name: Compare against the gnu build if: matrix.profile == 'dist' run: | - unset CFLAGS cargo build -p memtrack --profile dist echo "musl: $(stat -c %s target/$TARGET/dist/codspeed-memtrack) bytes" echo "gnu: $(stat -c %s target/dist/codspeed-memtrack) bytes" @@ -111,14 +156,28 @@ jobs: test -n "$(ls -A /tmp/memtrack-out)" || { echo "no artifact written"; exit 1; } tests: - name: ${{ matrix.test }} (musl) - runs-on: ubuntu-latest # x86_64 + name: ${{ matrix.test }} (${{ matrix.arch }} musl) + runs-on: ${{ matrix.runner }} strategy: fail-fast: false # Each memtrack integration test binary runs its cases serially (the eBPF # tracker can't overlap with itself in one process), so shard by binary. matrix: test: [c_tests, cpp_tests, rust_tests, spawn_tests, dlopen_tests, rss_tests] + arch: [x86_64, aarch64] + include: + - arch: x86_64 + runner: ubuntu-latest + target: x86_64-unknown-linux-musl + cflags_var: CFLAGS_x86_64_unknown_linux_musl + triplet: x86_64-linux-gnu + - arch: aarch64 + runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + cflags_var: CFLAGS_aarch64_unknown_linux_musl + triplet: aarch64-linux-gnu + env: + TARGET: ${{ matrix.target }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -126,7 +185,7 @@ jobs: submodules: true - uses: ./.github/actions/install-rust with: - cache-key: musl-${{ matrix.test }} + cache-key: musl-${{ matrix.arch }}-${{ matrix.test }} - uses: ./.github/actions/install-bpf-deps - name: Install musl toolchain @@ -137,21 +196,30 @@ jobs: - name: Install additional allocators run: sudo apt-get install -y libmimalloc-dev libjemalloc-dev + - name: Point the musl build at the stub and the UAPI headers + run: | + VALUE="-I$GITHUB_WORKSPACE/crates/memtrack/musl -idirafter /usr/include/${{ matrix.triplet }} -idirafter /usr/include" + # The target-scoped name is what cc-rs reads; MUSL_CFLAGS is a plain + # mirror of it, so later steps can pass the value on without having to + # index the env context by a matrix-computed key. + echo "${{ matrix.cflags_var }}=$VALUE" >> "$GITHUB_ENV" + echo "MUSL_CFLAGS=$VALUE" >> "$GITHUB_ENV" + # Built separately from the run because test-with's env(GITHUB_ACTIONS) # gate is evaluated at COMPILE time. GITHUB_ACTIONS is set by the runner, # so this is automatic here -- but if the tests are ever built outside # Actions, the sudo-gated cases silently become #[ignore]d and the run # reports a green "0 passed; N ignored". - name: Build tests - run: | - export CFLAGS="-I$STUB_INCLUDE" - cargo test -p memtrack --target "$TARGET" --no-run + run: cargo test -p memtrack --target "$TARGET" --no-run - name: Run tests env: RUST_LOG: debug # Ubuntu 26.04 ships sudo-rs, which ignores `-E`; pass the env the - # rustup shims and the test gate need through `env` instead. + # rustup shims and the test gate need through `env` instead. The + # autoconf seeds no longer appear here -- they come from + # .cargo/config.toml, which cargo reads regardless of who invokes it. run: | sudo env \ "HOME=$HOME" \ @@ -161,12 +229,8 @@ jobs: "CARGO_INCREMENTAL=$CARGO_INCREMENTAL" \ "RUST_LOG=$RUST_LOG" \ "GITHUB_ACTIONS=$GITHUB_ACTIONS" \ - "CFLAGS=-I$STUB_INCLUDE" \ - "LIBBPF_SYS_EXTRA_CFLAGS=$LIBBPF_SYS_EXTRA_CFLAGS" \ + "${{ matrix.cflags_var }}=$MUSL_CFLAGS" \ "TARGET=$TARGET" \ - "ac_cv_search_argp_parse=$ac_cv_search_argp_parse" \ - "ac_cv_search__obstack_free=$ac_cv_search__obstack_free" \ - "ac_cv_search_fts_close=$ac_cv_search_fts_close" \ $(which cargo) test --target "$TARGET" --test ${{ matrix.test }} \ -- --test-threads 1 --nocapture working-directory: crates/memtrack @@ -176,15 +240,32 @@ jobs: run: sudo chown -R $USER:$USER . ~/.cargo unit: - name: unit tests (musl) - runs-on: ubuntu-latest # x86_64 + name: unit tests (${{ matrix.arch }} musl) + runs-on: ${{ matrix.runner }} + strategy: + fail-fast: false + matrix: + arch: [x86_64, aarch64] + include: + - arch: x86_64 + runner: ubuntu-latest + target: x86_64-unknown-linux-musl + cflags_var: CFLAGS_x86_64_unknown_linux_musl + triplet: x86_64-linux-gnu + - arch: aarch64 + runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + cflags_var: CFLAGS_aarch64_unknown_linux_musl + triplet: aarch64-linux-gnu + env: + TARGET: ${{ matrix.target }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: submodules: true - uses: ./.github/actions/install-rust with: - cache-key: musl-unit + cache-key: musl-${{ matrix.arch }}-unit - uses: ./.github/actions/install-bpf-deps - name: Install musl toolchain @@ -192,15 +273,19 @@ jobs: sudo apt-get install -y musl-tools pkg-config linux-libc-dev rustup target add "$TARGET" - # Split out from the sharded job because of one known failure: - # ebpf::memtrack::tests::libc_allocator_symbols_resolve_to_offsets reads - # /proc/self/maps of the TEST BINARY and requires a mapped libc.so.6, - # which a statically linked musl binary does not have by construction. - # The production path resolves symbols in the *traced* process, so this is - # a test assumption, not a defect (report §4). Drop the --skip to check - # whether it has since been fixed; everything else must stay green. - - name: Run unit tests + - name: Point the musl build at the stub and the UAPI headers run: | - export CFLAGS="-I$STUB_INCLUDE" - cargo test -p memtrack --target "$TARGET" --lib \ - -- --skip libc_allocator_symbols_resolve_to_offsets + VALUE="-I$GITHUB_WORKSPACE/crates/memtrack/musl -idirafter /usr/include/${{ matrix.triplet }} -idirafter /usr/include" + # The target-scoped name is what cc-rs reads; MUSL_CFLAGS is a plain + # mirror of it, so later steps can pass the value on without having to + # index the env context by a matrix-computed key. + echo "${{ matrix.cflags_var }}=$VALUE" >> "$GITHUB_ENV" + echo "MUSL_CFLAGS=$VALUE" >> "$GITHUB_ENV" + + # `libc_allocator_symbols_resolve_to_offsets` used to be skipped here: it + # read /proc/self/maps of the TEST BINARY and required a mapped libc.so.6, + # which a statically linked musl binary does not have by construction. The + # test now resolves symbols in a spawned child instead, matching what the + # production path does, so the whole --lib suite must pass on musl. + - name: Run unit tests + run: cargo test -p memtrack --target "$TARGET" --lib From 728fddee4375937ab1824ced34155d8feb5a8966 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Wed, 16 Sep 2026 12:16:13 -0400 Subject: [PATCH 14/27] refactor(exec-harness,memtrack): move each CLI into its crate's lib Preparation for bundling both into the `codspeed` binary (COD-3440 phase 1). Follows what samply already does here: `samply::run` drives the library, and the entry point is a wrapper over it. Each crate gains a `cli` module exposing `run_cli(argv)`, and its `main.rs` shrinks to the two things that only make sense when the crate owns the whole process: installing the global logger, and turning the result into an exit code. A bundled subcommand can then call `run_cli` directly and cannot drift from the standalone binary, because there is only one implementation. The logger split is the point, not an accident. Only one global logger can exist per process; when these run bundled, the host CLI has already installed its own. Keeping `env_logger::init` in `main.rs` rather than in `run_cli` is what makes the bundled path safe. memtrack's `run_cli` returns the exit code instead of calling `std::process::exit` itself, so the caller stays in charge of teardown. memtrack's module is gated on `ebpf` for the same reason its `[[bin]]` is: without that feature there is no `Tracker` to drive. No behaviour change. 25 + 19 unit tests pass, clippy and fmt clean. Co-Authored-By: Claude Opus 5 (1M context) --- crates/exec-harness/src/cli.rs | 65 +++++++++++ crates/exec-harness/src/lib.rs | 2 + crates/exec-harness/src/main.rs | 57 ++------- crates/memtrack/src/cli.rs | 201 ++++++++++++++++++++++++++++++++ crates/memtrack/src/lib.rs | 5 + crates/memtrack/src/main.rs | 186 ++--------------------------- 6 files changed, 292 insertions(+), 224 deletions(-) create mode 100644 crates/exec-harness/src/cli.rs create mode 100644 crates/memtrack/src/cli.rs diff --git a/crates/exec-harness/src/cli.rs b/crates/exec-harness/src/cli.rs new file mode 100644 index 00000000..16631518 --- /dev/null +++ b/crates/exec-harness/src/cli.rs @@ -0,0 +1,65 @@ +//! exec-harness's command line, shared by the standalone `exec-harness` binary +//! and by the `codspeed` CLI that bundles it as a hidden subcommand. +//! +//! Keeping the parser and the dispatch here rather than in `main.rs` is what +//! stops the two paths from drifting: the standalone binary is a wrapper over +//! [`run_cli`] and nothing else. It deliberately does **not** install a logger — +//! only one global logger can exist per process, and when exec-harness runs +//! bundled the host CLI has already installed its own. + +use crate::prelude::*; +use crate::walltime::WalltimeExecutionArgs; +use crate::{BenchmarkCommand, MeasurementMode, execute_benchmarks, read_commands_from_stdin}; +use clap::Parser; +use std::ffi::OsString; + +#[derive(Parser, Debug)] +#[command(name = "exec-harness")] +#[command( + version, + about = "CodSpeed exec harness - wraps commands with performance instrumentation" +)] +struct Args { + /// Optional benchmark name, else the command will be used as the name + #[arg(long)] + name: Option, + + /// Set by the runner, should be coherent with the executor being used + #[arg(short, long, global = true, env = "CODSPEED_RUNNER_MODE", hide = true)] + measurement_mode: Option, + + #[command(flatten)] + walltime_args: WalltimeExecutionArgs, + + /// The command and arguments to execute. + /// Use "-" as the only argument to read a JSON payload from stdin. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + command: Vec, +} + +/// Parse `argv` and run the requested benchmarks. +pub fn run_cli(argv: I) -> Result<()> +where + I: IntoIterator, + T: Into + Clone, +{ + debug!("Starting exec-harness with pid {}", std::process::id()); + + let args = Args::parse_from(argv); + let measurement_mode = args.measurement_mode; + + // Determine if we're in stdin mode or CLI mode + let commands = match args.command.as_slice() { + [single] if single == "-" => read_commands_from_stdin()?, + [] => bail!("No command provided"), + _ => vec![BenchmarkCommand { + command: args.command, + name: args.name, + walltime_args: args.walltime_args, + }], + }; + + execute_benchmarks(commands, measurement_mode)?; + + Ok(()) +} diff --git a/crates/exec-harness/src/lib.rs b/crates/exec-harness/src/lib.rs index 28e52d58..52954b78 100644 --- a/crates/exec-harness/src/lib.rs +++ b/crates/exec-harness/src/lib.rs @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize}; use std::io::{self, BufRead}; pub mod analysis; +/// The command line, shared by the standalone binary and the bundled subcommand. +pub mod cli; pub mod constants; pub mod node; pub mod prelude; diff --git a/crates/exec-harness/src/main.rs b/crates/exec-harness/src/main.rs index 99cbf7cd..89bbe8c8 100644 --- a/crates/exec-harness/src/main.rs +++ b/crates/exec-harness/src/main.rs @@ -1,33 +1,12 @@ -use clap::Parser; +//! The standalone `exec-harness` binary. +//! +//! Everything it does lives in [`exec_harness::cli::run_cli`], which the +//! `codspeed` CLI also calls when exec-harness runs as a bundled subcommand. +//! All that is left here is the global logger, which only makes sense when +//! exec-harness owns the whole process. + +use exec_harness::cli::run_cli; use exec_harness::prelude::*; -use exec_harness::walltime::WalltimeExecutionArgs; -use exec_harness::{ - BenchmarkCommand, MeasurementMode, execute_benchmarks, read_commands_from_stdin, -}; - -#[derive(Parser, Debug)] -#[command(name = "exec-harness")] -#[command( - version, - about = "CodSpeed exec harness - wraps commands with performance instrumentation" -)] -struct Args { - /// Optional benchmark name, else the command will be used as the name - #[arg(long)] - name: Option, - - /// Set by the runner, should be coherent with the executor being used - #[arg(short, long, global = true, env = "CODSPEED_RUNNER_MODE", hide = true)] - measurement_mode: Option, - - #[command(flatten)] - walltime_args: WalltimeExecutionArgs, - - /// The command and arguments to execute. - /// Use "-" as the only argument to read a JSON payload from stdin. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - command: Vec, -} fn main() -> Result<()> { env_logger::builder() @@ -38,23 +17,5 @@ fn main() -> Result<()> { }) .init(); - debug!("Starting exec-harness with pid {}", std::process::id()); - - let args = Args::parse(); - let measurement_mode = args.measurement_mode; - - // Determine if we're in stdin mode or CLI mode - let commands = match args.command.as_slice() { - [single] if single == "-" => read_commands_from_stdin()?, - [] => bail!("No command provided"), - _ => vec![BenchmarkCommand { - command: args.command, - name: args.name, - walltime_args: args.walltime_args, - }], - }; - - execute_benchmarks(commands, measurement_mode)?; - - Ok(()) + run_cli(std::env::args_os()) } diff --git a/crates/memtrack/src/cli.rs b/crates/memtrack/src/cli.rs new file mode 100644 index 00000000..86ca66ab --- /dev/null +++ b/crates/memtrack/src/cli.rs @@ -0,0 +1,201 @@ +//! memtrack's command line, shared by the standalone `codspeed-memtrack` +//! binary and by the `codspeed` CLI that bundles it as a hidden subcommand. +//! +//! Keeping the parser and the work here rather than in `main.rs` is what stops +//! the two paths from drifting: the standalone binary is a wrapper over +//! [`run_cli`] and nothing else. It deliberately does **not** install a logger — +//! only one global logger can exist per process, and when memtrack runs bundled +//! the host CLI has already installed its own. `main.rs` installs one because +//! there it is the only thing in the process. + +use crate::prelude::*; +use crate::{MemtrackIpcMessage, Tracker, handle_ipc_message}; +use clap::Parser; +use ipc_channel::ipc; +use runner_shared::artifacts::{ArtifactExt, MemtrackArtifact, encode_events}; +use std::ffi::OsString; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::Arc; +use std::thread; + +#[derive(Parser)] +#[command(name = "memtrack")] +#[command(version, about = "Track memory allocations using eBPF", long_about = None)] +struct Cli { + #[command(subcommand)] + command: Commands, +} + +#[derive(Parser)] +enum Commands { + /// Track memory allocations for a command + Track { + /// Command to execute and track + command: String, + + /// Output folder for the allocations data + #[arg(short, long, default_value = ".")] + output: PathBuf, + + /// Optional IPC server name for receiving control commands + #[arg(long)] + ipc_server: Option, + }, +} + +/// Parse `argv` and run the requested subcommand, returning the exit code the +/// process should end with. +/// +/// The code is returned rather than passed to [`std::process::exit`] so that the +/// caller stays in charge of teardown. Both callers do exit on it: the tracked +/// command's status is the only meaningful result of a `track` run. +pub fn run_cli(argv: I) -> Result +where + I: IntoIterator, + T: Into + Clone, +{ + let cli = Cli::parse_from(argv); + + match cli.command { + Commands::Track { + command, + output: out_dir, + ipc_server, + } => { + debug!("Starting memtrack for command: {command}"); + + let status = + track_command(&command, ipc_server, &out_dir).context("Failed to track command")?; + + Ok(status.code().unwrap_or(1)) + } + } +} + +/// Get the original user's UID and GID when running under sudo. +/// Returns None if not running under sudo or if the environment variables are not set. +fn get_user_uid_gid() -> Option<(u32, u32)> { + let uid = std::env::var("SUDO_UID").ok()?.parse().ok()?; + let gid = std::env::var("SUDO_GID").ok()?.parse().ok()?; + Some((uid, gid)) +} + +fn track_command( + cmd_string: &str, + ipc_server_name: Option, + out_dir: &Path, +) -> anyhow::Result { + // First, establish IPC connection if needed to avoid timeouts on the runner because + // creating the Tracker instance takes some time. + let ipc_channel = if let Some(server_name) = ipc_server_name { + debug!("Connecting to IPC server: {server_name}"); + + let (tx, rx) = ipc::channel::()?; + let sender = ipc::IpcSender::connect(server_name)?; + sender.send(tx)?; + + Some(rx) + } else { + None + }; + + let tracker = Arc::new(Tracker::new()?); + + // Spawn IPC handler thread with the now-available tracker + let ipc_handle = if let Some(rx) = ipc_channel { + let tracker = tracker.clone(); + Some(thread::spawn(move || { + while let Ok(msg) = rx.recv() { + handle_ipc_message(msg, &tracker); + } + })) + } else { + // Without IPC, nothing toggles the tracking_enabled map, so allocator + // events would be dropped by the eBPF is_enabled() check. Enable it up + // front. + tracker.enable_tracking()?; + None + }; + + // Run the target command through bash to handle shell syntax. Drop + // privileges if running under sudo to avoid permission issues when the + // target accesses files owned by the original user. + let mut cmd = Command::new("bash"); + cmd.arg("-c").arg(cmd_string); + let uid_gid = get_user_uid_gid(); + if let Some((uid, gid)) = uid_gid { + debug!("Running under sudo, dropping privileges to uid={uid}, gid={gid}"); + } + + let mut session = tracker + .spawn(&cmd, uid_gid) + .map_err(|e| anyhow!("Failed to spawn child process: {e}"))?; + let root_pid = session.pid(); + let event_rx = session.take_events()?; + debug!("Spawned child with pid {root_pid}"); + + // Generate output file name and create file for streaming events + let file_name = MemtrackArtifact::file_name(Some(root_pid)); + let out_file = std::fs::File::create(out_dir.join(file_name))?; + + // Leave headroom for the ring buffer poll thread and the tracked + // command: encode workers on every core starve the poller during + // allocation bursts, which overflows the kernel ring buffer. + let n_workers = thread::available_parallelism() + .map(|n| n.get().saturating_sub(2).max(1)) + .unwrap_or(4); + + let pipeline_thread = thread::spawn(move || encode_events(event_rx, out_file, n_workers)); + + // Wait for the command to complete + let status = session.wait().context("Failed to wait for command")?; + debug!("Command exited with status: {status}"); + + // Stop allocator-event production before draining: the child has exited, + // so anything still arriving is already in the ring buffer. + if let Err(e) = tracker.disable_tracking() { + warn!("Failed to disable tracking: {e:#}"); + } + + // Dropping the session drops the event poller, which does a final drain of + // the ring buffer and then closes the event channel. Without this the + // encode pipeline join below would block forever. + debug!("Stopping the ring buffer poller"); + drop(session); + + debug!("Waiting for the encode pipeline to finish"); + let total = pipeline_thread + .join() + .map_err(|_| anyhow::anyhow!("Failed to join memtrack encode pipeline"))??; + + info!("Wrote {total} memtrack events to disk"); + + // Stop the attach worker and surface any fatal error it recorded (missed + // exec mappings mean incomplete allocator coverage). + tracker.finish()?; + + // Detach probes explicitly: the IPC thread still holds an Arc clone, so the + // tracker would otherwise never be dropped before process::exit and the + // kernel would close every link fd serially during exit. + tracker.detach(); + + // Read the eBPF dropped-event counter after the run is complete. + // A non-zero value means the ring buffer overflowed and the trace is + // incomplete. + let dropped_events = tracker + .dropped_events_count() + .context("Failed to read memtrack dropped-event counter")?; + if dropped_events > 0 { + bail!( + "Memtrack ring buffer overflowed: {dropped_events} events lost, aborting since the trace is incomplete.\n\ + Try reducing the benchmark's allocation rate (fewer iterations or smaller inputs), \ + or report it at https://github.com/CodSpeedHQ/codspeed/issues." + ); + } + + // IPC thread will exit when channel closes + drop(ipc_handle); + + Ok(status) +} diff --git a/crates/memtrack/src/lib.rs b/crates/memtrack/src/lib.rs index ccd93399..3cc133be 100644 --- a/crates/memtrack/src/lib.rs +++ b/crates/memtrack/src/lib.rs @@ -1,5 +1,10 @@ mod allocators; mod bpf_token; +/// The command line, shared by the standalone binary and the bundled subcommand. +/// Gated on `ebpf` for the same reason the `[[bin]]` is: without it there is no +/// `Tracker` to drive. +#[cfg(feature = "ebpf")] +pub mod cli; #[cfg(feature = "ebpf")] mod ebpf; mod ipc; diff --git a/crates/memtrack/src/main.rs b/crates/memtrack/src/main.rs index 283cff19..231c27c2 100644 --- a/crates/memtrack/src/main.rs +++ b/crates/memtrack/src/main.rs @@ -1,45 +1,12 @@ -use clap::Parser; -use ipc_channel::ipc; +//! The standalone `codspeed-memtrack` binary. +//! +//! Everything it does lives in [`memtrack::cli::run_cli`], which the `codspeed` +//! CLI also calls when memtrack runs as a bundled subcommand. All that is left +//! here is what only makes sense when memtrack owns the whole process: the +//! global logger, and turning the tracked command's exit code into our own. + +use memtrack::cli::run_cli; use memtrack::prelude::*; -use memtrack::{MemtrackIpcMessage, Tracker, handle_ipc_message}; -use runner_shared::artifacts::{ArtifactExt, MemtrackArtifact, encode_events}; -use std::path::{Path, PathBuf}; -use std::process::Command; -use std::sync::Arc; -use std::thread; - -#[derive(Parser)] -#[command(name = "memtrack")] -#[command(version, about = "Track memory allocations using eBPF", long_about = None)] -struct Cli { - #[command(subcommand)] - command: Commands, -} - -#[derive(Parser)] -enum Commands { - /// Track memory allocations for a command - Track { - /// Command to execute and track - command: String, - - /// Output folder for the allocations data - #[arg(short, long, default_value = ".")] - output: PathBuf, - - /// Optional IPC server name for receiving control commands - #[arg(long)] - ipc_server: Option, - }, -} - -/// Get the original user's UID and GID when running under sudo. -/// Returns None if not running under sudo or if the environment variables are not set. -fn get_user_uid_gid() -> Option<(u32, u32)> { - let uid = std::env::var("SUDO_UID").ok()?.parse().ok()?; - let gid = std::env::var("SUDO_GID").ok()?.parse().ok()?; - Some((uid, gid)) -} fn main() -> Result<()> { env_logger::builder() @@ -47,139 +14,6 @@ fn main() -> Result<()> { .format_timestamp(None) .init(); - let cli = Cli::parse(); - - match cli.command { - Commands::Track { - command, - output: out_dir, - ipc_server, - } => { - debug!("Starting memtrack for command: {command}"); - - let status = - track_command(&command, ipc_server, &out_dir).context("Failed to track command")?; - - std::process::exit(status.code().unwrap_or(1)); - } - } -} - -fn track_command( - cmd_string: &str, - ipc_server_name: Option, - out_dir: &Path, -) -> anyhow::Result { - // First, establish IPC connection if needed to avoid timeouts on the runner because - // creating the Tracker instance takes some time. - let ipc_channel = if let Some(server_name) = ipc_server_name { - debug!("Connecting to IPC server: {server_name}"); - - let (tx, rx) = ipc::channel::()?; - let sender = ipc::IpcSender::connect(server_name)?; - sender.send(tx)?; - - Some(rx) - } else { - None - }; - - let tracker = Arc::new(Tracker::new()?); - - // Spawn IPC handler thread with the now-available tracker - let ipc_handle = if let Some(rx) = ipc_channel { - let tracker = tracker.clone(); - Some(thread::spawn(move || { - while let Ok(msg) = rx.recv() { - handle_ipc_message(msg, &tracker); - } - })) - } else { - // Without IPC, nothing toggles the tracking_enabled map, so allocator - // events would be dropped by the eBPF is_enabled() check. Enable it up - // front. - tracker.enable_tracking()?; - None - }; - - // Run the target command through bash to handle shell syntax. Drop - // privileges if running under sudo to avoid permission issues when the - // target accesses files owned by the original user. - let mut cmd = Command::new("bash"); - cmd.arg("-c").arg(cmd_string); - let uid_gid = get_user_uid_gid(); - if let Some((uid, gid)) = uid_gid { - debug!("Running under sudo, dropping privileges to uid={uid}, gid={gid}"); - } - - let mut session = tracker - .spawn(&cmd, uid_gid) - .map_err(|e| anyhow!("Failed to spawn child process: {e}"))?; - let root_pid = session.pid(); - let event_rx = session.take_events()?; - debug!("Spawned child with pid {root_pid}"); - - // Generate output file name and create file for streaming events - let file_name = MemtrackArtifact::file_name(Some(root_pid)); - let out_file = std::fs::File::create(out_dir.join(file_name))?; - - // Leave headroom for the ring buffer poll thread and the tracked - // command: encode workers on every core starve the poller during - // allocation bursts, which overflows the kernel ring buffer. - let n_workers = thread::available_parallelism() - .map(|n| n.get().saturating_sub(2).max(1)) - .unwrap_or(4); - - let pipeline_thread = thread::spawn(move || encode_events(event_rx, out_file, n_workers)); - - // Wait for the command to complete - let status = session.wait().context("Failed to wait for command")?; - debug!("Command exited with status: {status}"); - - // Stop allocator-event production before draining: the child has exited, - // so anything still arriving is already in the ring buffer. - if let Err(e) = tracker.disable_tracking() { - warn!("Failed to disable tracking: {e:#}"); - } - - // Dropping the session drops the event poller, which does a final drain of - // the ring buffer and then closes the event channel. Without this the - // encode pipeline join below would block forever. - debug!("Stopping the ring buffer poller"); - drop(session); - - debug!("Waiting for the encode pipeline to finish"); - let total = pipeline_thread - .join() - .map_err(|_| anyhow::anyhow!("Failed to join memtrack encode pipeline"))??; - - info!("Wrote {total} memtrack events to disk"); - - // Stop the attach worker and surface any fatal error it recorded (missed - // exec mappings mean incomplete allocator coverage). - tracker.finish()?; - - // Detach probes explicitly: the IPC thread still holds an Arc clone, so the - // tracker would otherwise never be dropped before process::exit and the - // kernel would close every link fd serially during exit. - tracker.detach(); - - // Read the eBPF dropped-event counter after the run is complete. - // A non-zero value means the ring buffer overflowed and the trace is - // incomplete. - let dropped_events = tracker - .dropped_events_count() - .context("Failed to read memtrack dropped-event counter")?; - if dropped_events > 0 { - bail!( - "Memtrack ring buffer overflowed: {dropped_events} events lost, aborting since the trace is incomplete.\n\ - Try reducing the benchmark's allocation rate (fewer iterations or smaller inputs), \ - or report it at https://github.com/CodSpeedHQ/codspeed/issues." - ); - } - - // IPC thread will exit when channel closes - drop(ipc_handle); - - Ok(status) + let code = run_cli(std::env::args_os())?; + std::process::exit(code); } From 5c8807b24ae24921ed951835e2233f2c0441fb76 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Wed, 16 Sep 2026 12:59:47 -0400 Subject: [PATCH 15/27] feat(runner): bundle exec-harness as a subcommand instead of downloading it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit COD-3440 phase 1, second half. Follows samply exactly: a hidden `InternalCommands` variant, `get_command_builder()` re-execing the current binary, and `SELF_EXE_ENV_VAR` for the cases where `current_exe` lies. One thing samply did not need: exec-harness is handed its targets through a heredoc, so its invocation is spliced into a string that `bash -c` runs rather than driven by a `CommandBuilder`. Hence `get_shell_command()`, which renders the same re-exec through `shell_words::join`. Two tests pin that down — a self-exe path containing a space has to come back out as one word, and the heredoc delimiter has to stay quoted so nothing in the JSON payload is expanded. Unquoted, the tail of the path would silently become exec-harness's first argument and the run would fail with a parse error rather than a missing-file one. `ensure_binary_installed` for exec-harness is gone, which leaves `EXEC_HARNESS_INSTALLER`, `EXEC_HARNESS_VERSION`, `EXEC_HARNESS_COMMAND` and the `PinnedBinary::ExecHarnessInstaller` variant dead. `clippy -D warnings` rejects dead code, and keeping them behind an `allow` would have advertised a download path that no longer exists, so they are removed. No automation writes `binary_pins.rs`, so nothing else has to move with them. Verified: `codspeed exec-harness --version` reports `exec-harness 1.3.0` from the bundled binary. clippy strict, fmt, and the workspace `--lib` suite (5 binaries, 0 failures) all pass. Co-Authored-By: Claude Opus 5 (1M context) --- src/binary_pins.rs | 16 +----------- src/cli/exec/multi_targets.rs | 46 +++++++++++++++++++++++++++++---- src/cli/exec_harness.rs | 16 ++++++++++++ src/cli/mod.rs | 27 ++++++++++++++++++- src/executor/helpers/command.rs | 7 ++++- src/executor/orchestrator.rs | 19 +++++--------- 6 files changed, 97 insertions(+), 34 deletions(-) create mode 100644 src/cli/exec_harness.rs diff --git a/src/binary_pins.rs b/src/binary_pins.rs index 2b2734a8..caa70ffc 100644 --- a/src/binary_pins.rs +++ b/src/binary_pins.rs @@ -116,13 +116,6 @@ const MEMTRACK_INSTALLER: BinaryPin = BinaryPin { #[cfg(target_os = "linux")] pub const MEMTRACK_VERSION: &str = MEMTRACK_INSTALLER.version; -const EXEC_HARNESS_INSTALLER: BinaryPin = BinaryPin { - version: "1.3.0", - url_template: "https://github.com/CodSpeedHQ/codspeed/releases/download/exec-harness-v{version}/exec-harness-installer.sh", - sha256: "75cbff4fdaefe98927d24fff43fd600c621eb1263b0c40b0fd32c68fa6d88ebd", -}; -pub const EXEC_HARNESS_VERSION: &str = EXEC_HARNESS_INSTALLER.version; - const MONGO_TRACER_INSTALLER: BinaryPin = BinaryPin { version: "cs-mongo-tracer-v0.2.0", url_template: "https://codspeed-public-assets.s3.eu-west-1.amazonaws.com/mongo-tracer/{version}/cs-mongo-tracer-installer.sh", @@ -138,7 +131,6 @@ pub enum PinnedBinary { // Only installed by the Linux-only memory executor. #[cfg_attr(not(target_os = "linux"), allow(dead_code))] MemtrackInstaller, - ExecHarnessInstaller, MongoTracerInstaller, } @@ -147,7 +139,6 @@ impl PinnedBinary { match self { PinnedBinary::ValgrindDeb(target) => target.url(), PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.url(), - PinnedBinary::ExecHarnessInstaller => EXEC_HARNESS_INSTALLER.url(), PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.url(), } } @@ -156,7 +147,6 @@ impl PinnedBinary { match self { PinnedBinary::ValgrindDeb(target) => target.sha256(), PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.sha256, - PinnedBinary::ExecHarnessInstaller => EXEC_HARNESS_INSTALLER.sha256, PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.sha256, } } @@ -170,7 +160,6 @@ mod tests { const INSTALLER_BINARIES: &[PinnedBinary] = &[ PinnedBinary::MemtrackInstaller, - PinnedBinary::ExecHarnessInstaller, PinnedBinary::MongoTracerInstaller, ]; @@ -196,9 +185,7 @@ mod tests { fn assert_installer_variant_is_listed(binary: PinnedBinary) { match binary { PinnedBinary::ValgrindDeb(_) => {} - PinnedBinary::MemtrackInstaller - | PinnedBinary::ExecHarnessInstaller - | PinnedBinary::MongoTracerInstaller => { + PinnedBinary::MemtrackInstaller | PinnedBinary::MongoTracerInstaller => { assert!(INSTALLER_BINARIES.contains(&binary)); } } @@ -215,7 +202,6 @@ mod tests { #[test] fn installer_variant_list_is_exhaustive() { assert_installer_variant_is_listed(PinnedBinary::MemtrackInstaller); - assert_installer_variant_is_listed(PinnedBinary::ExecHarnessInstaller); assert_installer_variant_is_listed(PinnedBinary::MongoTracerInstaller); } diff --git a/src/cli/exec/multi_targets.rs b/src/cli/exec/multi_targets.rs index d24c16b9..0d511a6c 100644 --- a/src/cli/exec/multi_targets.rs +++ b/src/cli/exec/multi_targets.rs @@ -1,5 +1,4 @@ use crate::executor::config::BenchmarkTarget; -use crate::executor::orchestrator::EXEC_HARNESS_COMMAND; use crate::prelude::*; use crate::project_config::{Target, TargetCommand, WalltimeOptions}; use exec_harness::BenchmarkCommand; @@ -69,8 +68,11 @@ pub fn build_benchmark_targets( .collect() } -/// Build a shell command string that pipes BenchmarkTarget::Exec variants to exec-harness via stdin +/// Build a shell command string that pipes BenchmarkTarget::Exec variants to exec-harness via stdin. +/// +/// `exec_harness` is the already shell-quoted invocation of exec-harness. pub fn build_exec_targets_pipe_command( + exec_harness: &str, targets: &[&crate::executor::config::BenchmarkTarget], ) -> Result { let inputs: Vec = targets @@ -92,9 +94,43 @@ pub fn build_exec_targets_pipe_command( .collect::>>()?; let json = serde_json::to_string(&inputs).context("Failed to serialize targets to JSON")?; - Ok(build_pipe_command_from_json(&json)) + Ok(build_pipe_command_from_json(exec_harness, &json)) +} + +fn build_pipe_command_from_json(exec_harness: &str, json: &str) -> String { + format!("{exec_harness} - <<'CODSPEED_EOF'\n{json}\nCODSPEED_EOF") } -fn build_pipe_command_from_json(json: &str) -> String { - format!("{EXEC_HARNESS_COMMAND} - <<'CODSPEED_EOF'\n{json}\nCODSPEED_EOF") +#[cfg(test)] +mod tests { + use super::*; + use crate::cli::exec_harness::ExecHarnessArgs; + use crate::cli::{InternalCommands, SELF_EXE_ENV_VAR}; + + /// The invocation is spliced into a string that `bash -c` runs, so a + /// self-exe path containing a space has to come back out as one word. + #[test] + fn exec_harness_invocation_survives_a_self_exe_path_with_spaces() { + temp_env::with_var(SELF_EXE_ENV_VAR, Some("/opt/my tools/codspeed"), || { + let invocation = InternalCommands::ExecHarness(ExecHarnessArgs { args: vec![] }) + .get_shell_command() + .unwrap(); + + assert_eq!( + shell_words::split(&invocation).unwrap(), + vec!["/opt/my tools/codspeed", "exec-harness"] + ); + }); + } + + /// The delimiter is quoted, so the shell expands nothing inside the body. + #[test] + fn pipe_command_wraps_the_payload_in_an_unexpanded_heredoc() { + let cmd = build_pipe_command_from_json("/bin/codspeed exec-harness", r#"{"a":"$HOME"}"#); + + assert_eq!( + cmd, + "/bin/codspeed exec-harness - <<'CODSPEED_EOF'\n{\"a\":\"$HOME\"}\nCODSPEED_EOF" + ); + } } diff --git a/src/cli/exec_harness.rs b/src/cli/exec_harness.rs new file mode 100644 index 00000000..f8a4b4d2 --- /dev/null +++ b/src/cli/exec_harness.rs @@ -0,0 +1,16 @@ +use crate::prelude::*; + +/// Run the bundled exec-harness. Arguments after `exec-harness` are forwarded +/// verbatim to exec-harness's own CLI parser. +#[derive(Debug, clap::Args)] +pub struct ExecHarnessArgs { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + pub args: Vec, +} + +pub fn run(args: ExecHarnessArgs) -> Result<()> { + // exec-harness's own clap parser expects its name as `argv[0]`, not ours. + let argv = std::iter::once(std::ffi::OsString::from("exec-harness")).chain(args.args); + + ::exec_harness::cli::run_cli(argv) +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 2a9218dd..eae17c2c 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1,5 +1,6 @@ mod auth; pub(crate) mod exec; +pub(crate) mod exec_harness; pub(crate) mod experimental; mod profile; pub(crate) mod run; @@ -110,6 +111,9 @@ pub(crate) enum InternalCommands { /// Run the bundled samply profiler. Args are forwarded to samply. #[command(disable_help_flag = true, disable_help_subcommand = true)] Samply(samply::SamplyArgs), + /// Run the bundled exec-harness. Args are forwarded to exec-harness. + #[command(disable_help_flag = true, disable_help_subcommand = true)] + ExecHarness(exec_harness::ExecHarnessArgs), } /// Overrides the executable used to re-invoke internal subcommands. @@ -134,9 +138,23 @@ impl InternalCommands { builder.arg("samply"); builder.args(args.args.iter().cloned()); } + InternalCommands::ExecHarness(args) => { + builder.arg("exec-harness"); + builder.args(args.args.iter().cloned()); + } } Ok(builder) } + + /// The same re-exec, rendered as a single POSIX-shell command string. + /// + /// Not every call site can use a [`CommandBuilder`]: exec-harness is handed + /// its targets through a heredoc, so its invocation has to be spliced into + /// a string that `bash -c` will run. Quoting goes through + /// `shell_words::join`, so a self-exe path containing spaces survives. + pub fn get_shell_command(&self) -> Result { + Ok(self.get_command_builder()?.as_command_line()) + } } pub async fn run() -> Result<()> { @@ -158,7 +176,13 @@ pub async fn run() -> Result<()> { let setup_cache_dir = setup_cache_dir.as_deref(); match cli.command { - Commands::Run(_) | Commands::Exec(_) | Commands::Internal(InternalCommands::Samply(_)) => {} // these are responsible for their own logger initialization + // These are responsible for their own logger initialization. The + // bundled components must not install one: only one global logger can + // exist per process, and theirs would lose to (or clash with) ours. + Commands::Run(_) + | Commands::Exec(_) + | Commands::Internal(InternalCommands::Samply(_)) + | Commands::Internal(InternalCommands::ExecHarness(_)) => {} _ => { init_local_logger()?; } @@ -211,6 +235,7 @@ pub async fn run() -> Result<()> { Commands::Show => show::run()?, Commands::Update => update::run().await?, Commands::Internal(InternalCommands::Samply(args)) => samply::run(args)?, + Commands::Internal(InternalCommands::ExecHarness(args)) => exec_harness::run(args)?, } Ok(()) } diff --git a/src/executor/helpers/command.rs b/src/executor/helpers/command.rs index 345f9697..72f9b4d1 100644 --- a/src/executor/helpers/command.rs +++ b/src/executor/helpers/command.rs @@ -128,7 +128,12 @@ impl CommandBuilder { self } - /// Returns the command line as a string for debugging/testing purposes + /// The command line as a single shell-quoted string. + /// + /// Used for logging and assertions, and by + /// [`InternalCommands::get_shell_command`](crate::cli::InternalCommands::get_shell_command) + /// to splice a re-exec into a command that `bash -c` runs — so the quoting + /// has to stay correct, not merely readable. pub fn as_command_line(&self) -> String { let mut parts: Vec = vec![self.program.to_string_lossy().into_owned()]; parts.extend( diff --git a/src/executor/orchestrator.rs b/src/executor/orchestrator.rs index 6d3646d8..cdf08212 100644 --- a/src/executor/orchestrator.rs +++ b/src/executor/orchestrator.rs @@ -1,8 +1,8 @@ use super::{ExecutionContext, ExecutorName, get_executor_from_mode, run_executor}; use crate::api_client::CodSpeedAPIClient; -use crate::binary_installer::ensure_binary_installed; -use crate::binary_pins::{self, PinnedBinary}; +use crate::cli::InternalCommands; use crate::cli::exec::multi_targets; +use crate::cli::exec_harness::ExecHarnessArgs; use crate::cli::run::logger::Logger; use crate::executor::config::BenchmarkTarget; use crate::executor::config::OrchestratorConfig; @@ -17,9 +17,6 @@ use serde_json::Value; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; -pub const EXEC_HARNESS_COMMAND: &str = "exec-harness"; -pub const EXEC_HARNESS_VERSION: &str = binary_pins::EXEC_HARNESS_VERSION; - /// Shared orchestration state created once per CLI invocation. /// /// Holds the run-level configuration, environment provider, system info, and logger. @@ -82,14 +79,12 @@ impl Orchestrator { .collect(); if !exec_targets.is_empty() { - ensure_binary_installed( - EXEC_HARNESS_COMMAND, - EXEC_HARNESS_VERSION, - PinnedBinary::ExecHarnessInstaller, - ) - .await?; + // exec-harness is a hidden subcommand of this binary: re-exec ourselves. + let exec_harness = InternalCommands::ExecHarness(ExecHarnessArgs { args: vec![] }) + .get_shell_command()?; - let pipe_cmd = multi_targets::build_exec_targets_pipe_command(&exec_targets)?; + let pipe_cmd = + multi_targets::build_exec_targets_pipe_command(&exec_harness, &exec_targets)?; let label = match exec_targets.as_slice() { [BenchmarkTarget::Exec { command, .. }] => { format!("Running `{}` with exec-harness", command.join(" ")) From 7368b6a1d509f806c68088fb571e13399c43e441 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 05:53:59 -0400 Subject: [PATCH 16/27] feat(runner): bundle memtrack too, and drop the download machinery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit COD-3440 phase 1c. Same shape as samply and exec-harness: a hidden `InternalCommands` variant, `get_command_builder()` re-execing the current binary. The root manifest loses `default-features = false` on memtrack so the tracker itself is linked in, not just its IPC types. Verified rather than assumed. From the **statically linked musl** build: `codspeed memtrack track` loads its BPF programs and writes a 10 249-byte artifact. That closes the roadmap's open risk — memtrack's musl port was only ever validated standalone, never linked into the same binary as the runner. Size went the right way. On the dist profile for aarch64 musl, the merged binary compresses to 11 049 967 bytes against 13 522 798 for the three artifacts published today: one download instead of three, and ~2.5 MB smaller, because the code each binary carried its own copy of costs more than linking everything once. `memtrack_path()` now returns `self_exe()`, resolved by a function shared with `get_command_builder()`. That sharing is deliberate: `setcap` on a path that is not the one later exec'd succeeds and changes nothing, so resolving it twice by hand would be a silent footgun. The consequence is that the five capabilities, `CAP_SYS_ADMIN` among them, now sit on the `codspeed` executable. They are `+ep` with no inheritable set, so a spawned benchmark does not receive them and the elevation stops at the CLI process. `get_memtrack_status` stops probing a version — a bundled subcommand cannot be out of step with its host — and `install_memtrack` becomes a no-op. With the memtrack pin gone, `src/binary_installer/` had no callers left and is deleted; `download_pinned_file` stays, since valgrind and mongo-tracer use it directly. memtrack's apt build dependencies move to the root package, which is now the one being built for release. The memory tests needed `SELF_EXE_ENV_VAR`, as the valgrind and walltime ones already did: under `cargo test`, `current_exe` is the test harness, which rejects `memtrack track --output …` and leaves the executor waiting on an IPC connection that never comes. `MEMORY_INIT` needs it too — `grant_privileges()` runs outside the per-test scope and would otherwise setcap the throwaway test binary. 389 tests pass across the workspace, clippy `-D warnings` and fmt are clean. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 19 ++++- src/binary_installer/mod.rs | 97 ---------------------- src/binary_installer/versions.rs | 134 ------------------------------- src/binary_pins.rs | 21 +---- src/cli/memtrack.rs | 19 +++++ src/cli/mod.rs | 53 ++++++++---- src/executor/memory/executor.rs | 7 +- src/executor/memory/setup.rs | 93 ++++++--------------- src/executor/tests.rs | 66 +++++++++++---- src/lib.rs | 1 - 10 files changed, 156 insertions(+), 354 deletions(-) delete mode 100644 src/binary_installer/mod.rs delete mode 100644 src/binary_installer/versions.rs create mode 100644 src/cli/memtrack.rs diff --git a/Cargo.toml b/Cargo.toml index 7709542c..d88c8eea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -74,7 +74,9 @@ samply = { path = "crates/samply-codspeed/samply" } [target.'cfg(target_os = "linux")'.dependencies] procfs = "0.18" caps = "0.5" -memtrack = { path = "crates/memtrack", default-features = false } +# Default features on purpose: `ebpf` carries the tracker itself, which the +# bundled `memtrack` subcommand needs, not just the IPC types. +memtrack = { path = "crates/memtrack" } ipc-channel = { workspace = true } [dev-dependencies] @@ -139,3 +141,18 @@ targets = ["aarch64-apple-darwin", "aarch64-unknown-linux-musl", "x86_64-unknown binaries.aarch64-apple-darwin = ["codspeed"] binaries.aarch64-unknown-linux-musl = ["codspeed"] binaries.x86_64-unknown-linux-musl = ["codspeed"] + +# Linking memtrack in pulls its vendored libbpf/elfutils build into this +# package, so releasing the CLI needs memtrack's build toolchain. +[package.metadata.dist.dependencies.apt] +build-essential = "*" +pkgconf = "*" +zlib1g-dev = "*" +libbpf-dev = "*" +musl-tools = "*" +linux-libc-dev = "*" + +# Required for the vendored feature +autopoint = "*" +bison = "*" +flex = "*" diff --git a/src/binary_installer/mod.rs b/src/binary_installer/mod.rs deleted file mode 100644 index d8bdb75b..00000000 --- a/src/binary_installer/mod.rs +++ /dev/null @@ -1,97 +0,0 @@ -use crate::binary_pins::PinnedBinary; -use crate::cli::run::helpers::download_pinned_file; -use crate::prelude::*; -use semver::Version; -use std::process::Command; -use tempfile::NamedTempFile; - -mod versions; - -/// Ensure a binary is installed, or install it from a `PinnedBinary` installer script. -/// -/// This function checks if the binary is already installed with the correct version. -/// If not, it downloads and executes the pinned installer script. -/// -/// # Arguments -/// * `binary_name` - The binary command name (e.g., "codspeed-memtrack", "codspeed-exec-harness") -/// * `version` - The version to install (e.g., "4.4.2-alpha.2") -/// * `installer` - The `PinnedBinary` installer to download. -pub async fn ensure_binary_installed( - binary_name: &str, - version: &str, - installer: PinnedBinary, -) -> Result<()> { - if is_command_installed( - binary_name, - Version::parse(version).context("Invalid version format")?, - ) { - debug!("{binary_name} version {version} is already installed"); - return Ok(()); - } - - debug!("Downloading installer for {binary_name}"); - - // Download the installer script to a temporary file (with sha256 verification) - let temp_file = NamedTempFile::new().context("Failed to create temporary file")?; - download_pinned_file(installer, temp_file.path()).await?; - - // Execute the installer script - let output = Command::new("sh") - .arg(temp_file.path()) - .output() - .context("Failed to execute installer command")?; - - if !output.status.success() { - bail!( - "Failed to install {binary_name} version {version}. Installer exited with output: {output:?}", - ); - } - - if !is_command_installed( - binary_name, - Version::parse(version).context("Invalid version format")?, - ) { - bail!( - "Could not veryfy installation of {binary_name} version {version} after running installer" - ); - } - - info!("Successfully installed {binary_name} version {version}"); - Ok(()) -} - -/// Check if the given command is installed and its version matches the expected version. -/// -/// Expects the command to support the `--version` flag and return a version string. -fn is_command_installed(command: &str, expected_version: Version) -> bool { - let is_command_installed = Command::new("which") - .arg(command) - .output() - .is_ok_and(|output| output.status.success()); - - if !is_command_installed { - debug!("{command} is not installed"); - return false; - } - - let Ok(version_output) = Command::new(command).arg("--version").output() else { - return false; - }; - - if !version_output.status.success() { - debug!( - "Failed to get command version. stderr: {}", - String::from_utf8_lossy(&version_output.stderr) - ); - return false; - } - - let version_string = String::from_utf8_lossy(&version_output.stdout); - let Ok(version) = versions::parse_from_output(&version_string) else { - return false; - }; - - debug!("Found {command} version: {version}"); - - versions::is_compatible(command, &version, &expected_version) -} diff --git a/src/binary_installer/versions.rs b/src/binary_installer/versions.rs deleted file mode 100644 index 4d12e7de..00000000 --- a/src/binary_installer/versions.rs +++ /dev/null @@ -1,134 +0,0 @@ -use crate::prelude::*; -use semver::Version; - -/// Parse a version string from command output. -/// -/// Expects the output format to be: "command_name version_string" -/// Example: "codspeed-memtrack 4.4.2" -pub(super) fn parse_from_output(output: &str) -> Result { - let version_str = output - .split_once(" ") - .context("Unexpected version output format: missing space separator")? - .1 - .trim(); - - Version::parse(version_str) - .with_context(|| format!("Failed to parse version from: {version_str}")) -} - -/// Check if an installed version is compatible with the expected version. -/// -/// Returns true if the installed version is greater than or equal to the expected version. -/// Logs warnings for outdated or experimental versions. -pub(super) fn is_compatible(command: &str, installed: &Version, expected: &Version) -> bool { - match installed.cmp(expected) { - std::cmp::Ordering::Less => { - warn!( - "{command} is installed but the version is too old. expecting {expected} or higher but found installed: {installed}", - ); - false - } - std::cmp::Ordering::Greater => { - warn!( - "Using experimental {command} version {installed}. The recommended version is {expected}", - ); - true - } - std::cmp::Ordering::Equal => true, - } -} -#[cfg(test)] -mod tests { - use super::*; - - mod parse_version_from_output { - use super::*; - - #[test] - fn parses_valid_version() { - let output = "codspeed-memtrack 4.4.2"; - let version = parse_from_output(output).unwrap(); - assert_eq!(version, Version::new(4, 4, 2)); - } - - #[test] - fn parses_version_with_prerelease() { - let output = "codspeed-exec-harness 4.4.2-alpha.2"; - let version = parse_from_output(output).unwrap(); - assert_eq!(version.major, 4); - assert_eq!(version.minor, 4); - assert_eq!(version.patch, 2); - assert_eq!(version.pre.as_str(), "alpha.2"); - } - } - - mod is_version_compatible { - use super::*; - - #[test] - fn returns_true_for_equal_versions() { - let installed = Version::new(4, 4, 2); - let expected = Version::new(4, 4, 2); - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn returns_true_for_newer_version() { - let installed = Version::new(4, 5, 0); - let expected = Version::new(4, 4, 2); - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn returns_false_for_older_version() { - let installed = Version::new(4, 3, 0); - let expected = Version::new(4, 4, 2); - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn handles_prerelease_versions() { - let installed = Version::parse("4.4.2-alpha.2").unwrap(); - let expected = Version::new(4, 4, 1); - // 4.4.2-alpha.2 > 4.4.1 because 4.4.2 > 4.4.1 - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn prerelease_different_stage() { - { - let installed = Version::parse("4.4.2-alpha.2").unwrap(); - let expected = Version::new(4, 4, 2); - // 4.4.2-alpha.2 < 4.4.2 - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - - { - let installed = Version::parse("4.4.2-beta.1").unwrap(); - let expected = Version::parse("4.4.2-alpha.1").unwrap(); - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - { - let installed = Version::new(4, 4, 2); - let expected = Version::parse("4.4.2-alpha.2").unwrap(); - // 4.4.2 > 4.4.2-alpha.2 - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - { - let installed = Version::parse("4.4.2-alpha.1").unwrap(); - let expected = Version::parse("4.4.2-beta.1").unwrap(); - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - } - - #[test] - fn prerelease_same_stage() { - let installed = Version::parse("4.4.2-alpha.1").unwrap(); - let expected = Version::parse("4.4.2-alpha.2").unwrap(); - - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - } -} diff --git a/src/binary_pins.rs b/src/binary_pins.rs index caa70ffc..355d725e 100644 --- a/src/binary_pins.rs +++ b/src/binary_pins.rs @@ -108,14 +108,6 @@ impl ValgrindTarget { } } -const MEMTRACK_INSTALLER: BinaryPin = BinaryPin { - version: "1.5.1", - url_template: "https://github.com/CodSpeedHQ/codspeed/releases/download/memtrack-v{version}/memtrack-installer.sh", - sha256: "47d529728d9e2a02fc0773c8ca0ece214f67cbc965d7ae327fe8c213ae2735a7", -}; -#[cfg(target_os = "linux")] -pub const MEMTRACK_VERSION: &str = MEMTRACK_INSTALLER.version; - const MONGO_TRACER_INSTALLER: BinaryPin = BinaryPin { version: "cs-mongo-tracer-v0.2.0", url_template: "https://codspeed-public-assets.s3.eu-west-1.amazonaws.com/mongo-tracer/{version}/cs-mongo-tracer-installer.sh", @@ -128,9 +120,6 @@ const MONGO_TRACER_INSTALLER: BinaryPin = BinaryPin { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum PinnedBinary { ValgrindDeb(ValgrindTarget), - // Only installed by the Linux-only memory executor. - #[cfg_attr(not(target_os = "linux"), allow(dead_code))] - MemtrackInstaller, MongoTracerInstaller, } @@ -138,7 +127,6 @@ impl PinnedBinary { pub fn url(&self) -> String { match self { PinnedBinary::ValgrindDeb(target) => target.url(), - PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.url(), PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.url(), } } @@ -146,7 +134,6 @@ impl PinnedBinary { pub fn sha256(&self) -> &'static str { match self { PinnedBinary::ValgrindDeb(target) => target.sha256(), - PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.sha256, PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.sha256, } } @@ -158,10 +145,7 @@ mod tests { use crate::cli::run::helpers::download_pinned_file; use tempfile::NamedTempFile; - const INSTALLER_BINARIES: &[PinnedBinary] = &[ - PinnedBinary::MemtrackInstaller, - PinnedBinary::MongoTracerInstaller, - ]; + const INSTALLER_BINARIES: &[PinnedBinary] = &[PinnedBinary::MongoTracerInstaller]; const ALL_VALGRIND_TARGETS: &[ValgrindTarget] = &[ ValgrindTarget { @@ -185,7 +169,7 @@ mod tests { fn assert_installer_variant_is_listed(binary: PinnedBinary) { match binary { PinnedBinary::ValgrindDeb(_) => {} - PinnedBinary::MemtrackInstaller | PinnedBinary::MongoTracerInstaller => { + PinnedBinary::MongoTracerInstaller => { assert!(INSTALLER_BINARIES.contains(&binary)); } } @@ -201,7 +185,6 @@ mod tests { #[test] fn installer_variant_list_is_exhaustive() { - assert_installer_variant_is_listed(PinnedBinary::MemtrackInstaller); assert_installer_variant_is_listed(PinnedBinary::MongoTracerInstaller); } diff --git a/src/cli/memtrack.rs b/src/cli/memtrack.rs new file mode 100644 index 00000000..686d7692 --- /dev/null +++ b/src/cli/memtrack.rs @@ -0,0 +1,19 @@ +use crate::prelude::*; + +/// Run the bundled memtrack. Arguments after `memtrack` are forwarded verbatim +/// to memtrack's own CLI parser. +#[derive(Debug, clap::Args)] +pub struct MemtrackArgs { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + pub args: Vec, +} + +pub fn run(args: MemtrackArgs) -> Result<()> { + // memtrack's own clap parser expects its name as `argv[0]`, not ours. + let argv = std::iter::once(std::ffi::OsString::from("memtrack")).chain(args.args); + + // memtrack's exit code is the tracked command's own, and the runner reads + // it to decide whether the benchmark failed, so it has to become ours. + let code = ::memtrack::cli::run_cli(argv)?; + std::process::exit(code); +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index eae17c2c..85a2dd5d 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -2,6 +2,8 @@ mod auth; pub(crate) mod exec; pub(crate) mod exec_harness; pub(crate) mod experimental; +#[cfg(target_os = "linux")] +pub(crate) mod memtrack; mod profile; pub(crate) mod run; pub(crate) mod samply; @@ -114,6 +116,13 @@ pub(crate) enum InternalCommands { /// Run the bundled exec-harness. Args are forwarded to exec-harness. #[command(disable_help_flag = true, disable_help_subcommand = true)] ExecHarness(exec_harness::ExecHarnessArgs), + /// Run the bundled memtrack. Args are forwarded to memtrack. + /// + /// Linux-only, like the memory executor that drives it: memtrack is an eBPF + /// tracker and is not built at all on other platforms. + #[cfg(target_os = "linux")] + #[command(disable_help_flag = true, disable_help_subcommand = true)] + Memtrack(memtrack::MemtrackArgs), } /// Overrides the executable used to re-invoke internal subcommands. @@ -123,16 +132,25 @@ pub(crate) enum InternalCommands { /// a wrapper when the CLI is invoked through a launcher script. pub(crate) const SELF_EXE_ENV_VAR: &str = "CODSPEED_SELF_EXE"; +/// The executable that internal subcommands are re-invoked through. +/// +/// Exposed separately from [`InternalCommands::get_command_builder`] because the +/// memory executor grants file capabilities to this exact path before running +/// it, and `setcap` on a path that is not the one later exec'd succeeds while +/// changing nothing. +pub(crate) fn self_exe() -> Result { + match std::env::var_os(SELF_EXE_ENV_VAR) { + Some(path) => Ok(PathBuf::from(path)), + None => std::env::current_exe() + .context("failed to resolve current executable for internal subcommand"), + } +} + impl InternalCommands { /// Build a [`CommandBuilder`] that re-execs the current binary into this /// internal subcommand. Each variant owns its own arg layout. pub fn get_command_builder(&self) -> Result { - let self_exe = match std::env::var_os(SELF_EXE_ENV_VAR) { - Some(path) => PathBuf::from(path), - None => std::env::current_exe() - .context("failed to resolve current executable for internal subcommand")?, - }; - let mut builder = CommandBuilder::new(self_exe); + let mut builder = CommandBuilder::new(self_exe()?); match self { InternalCommands::Samply(args) => { builder.arg("samply"); @@ -142,16 +160,18 @@ impl InternalCommands { builder.arg("exec-harness"); builder.args(args.args.iter().cloned()); } + #[cfg(target_os = "linux")] + InternalCommands::Memtrack(args) => { + builder.arg("memtrack"); + builder.args(args.args.iter().cloned()); + } } Ok(builder) } - /// The same re-exec, rendered as a single POSIX-shell command string. - /// - /// Not every call site can use a [`CommandBuilder`]: exec-harness is handed - /// its targets through a heredoc, so its invocation has to be spliced into - /// a string that `bash -c` will run. Quoting goes through - /// `shell_words::join`, so a self-exe path containing spaces survives. + /// The same re-exec as a single POSIX-shell command string, for the call + /// sites that splice it into a script rather than spawning it: exec-harness + /// is handed its targets through a heredoc. pub fn get_shell_command(&self) -> Result { Ok(self.get_command_builder()?.as_command_line()) } @@ -176,13 +196,14 @@ pub async fn run() -> Result<()> { let setup_cache_dir = setup_cache_dir.as_deref(); match cli.command { - // These are responsible for their own logger initialization. The - // bundled components must not install one: only one global logger can - // exist per process, and theirs would lose to (or clash with) ours. + // These initialize their own logging. Bundled subcommands must not: + // a process has one global logger, and theirs would clash with ours. Commands::Run(_) | Commands::Exec(_) | Commands::Internal(InternalCommands::Samply(_)) | Commands::Internal(InternalCommands::ExecHarness(_)) => {} + #[cfg(target_os = "linux")] + Commands::Internal(InternalCommands::Memtrack(_)) => {} _ => { init_local_logger()?; } @@ -236,6 +257,8 @@ pub async fn run() -> Result<()> { Commands::Update => update::run().await?, Commands::Internal(InternalCommands::Samply(args)) => samply::run(args)?, Commands::Internal(InternalCommands::ExecHarness(args)) => exec_harness::run(args)?, + #[cfg(target_os = "linux")] + Commands::Internal(InternalCommands::Memtrack(args)) => memtrack::run(args)?, } Ok(()) } diff --git a/src/executor/memory/executor.rs b/src/executor/memory/executor.rs index 6cbb0f97..f9b71e59 100644 --- a/src/executor/memory/executor.rs +++ b/src/executor/memory/executor.rs @@ -1,3 +1,5 @@ +use crate::cli::InternalCommands; +use crate::cli::memtrack::MemtrackArgs; use crate::executor::ExecutorName; use crate::executor::ExecutorSupport; use crate::executor::PrivilegeStatus; @@ -61,8 +63,9 @@ impl MemoryExecutor { let bench_command = get_bench_command(&execution_context.config)?; let (bench_command, env_file) = prefix_command_with_env(&bench_command, &extra_env)?; - // Build the memtrack command - let mut cmd_builder = CommandBuilder::new(MEMTRACK_COMMAND); + // memtrack is a hidden subcommand of this binary: re-exec ourselves. + let mut cmd_builder = + InternalCommands::Memtrack(MemtrackArgs { args: vec![] }).get_command_builder()?; if execution_context.config.memory_track_physical { cmd_builder.env("CODSPEED_MEMTRACK_TRACK_PHYSICAL", "1"); } diff --git a/src/executor/memory/setup.rs b/src/executor/memory/setup.rs index e393e8b1..4881fb85 100644 --- a/src/executor/memory/setup.rs +++ b/src/executor/memory/setup.rs @@ -1,15 +1,14 @@ -use crate::binary_installer::ensure_binary_installed; -use crate::binary_pins::{self, PinnedBinary}; +use crate::cli::self_exe; use crate::executor::helpers::capabilities::binary_has_capabilities; use crate::executor::helpers::run_with_sudo::{is_root_user, run_with_sudo}; use crate::executor::{ToolInstallStatus, ToolStatus}; use crate::prelude::*; use caps::Capability; use std::path::PathBuf; -use std::process::Command; -pub const MEMTRACK_COMMAND: &str = "codspeed-memtrack"; -pub const MEMTRACK_CODSPEED_VERSION: &str = binary_pins::MEMTRACK_VERSION; +/// How memtrack is named in user-facing messages. It is no longer a binary to +/// look up: memtrack is bundled into this executable as a hidden subcommand. +pub const MEMTRACK_COMMAND: &str = "memtrack"; const MEMTRACK_REQUIRED_CAPS: &[Capability] = &[ Capability::CAP_DAC_READ_SEARCH, @@ -37,8 +36,16 @@ fn memtrack_setcap_spec() -> String { format!("{caps}+ep") } +/// The binary that must carry the eBPF capabilities. +/// +/// Since memtrack is bundled, that binary is *this* one. Note what that means: +/// the five capabilities below, `CAP_SYS_ADMIN` among them, end up on the +/// `codspeed` executable itself rather than on a dedicated tracker, so every +/// invocation of the CLI carries them in its permitted and effective sets. +/// They are granted `+ep` and not inheritable, so a spawned benchmark does not +/// receive them — the elevation stops at the CLI process. fn memtrack_path() -> Option { - which::which(MEMTRACK_COMMAND).ok() + self_exe().ok() } /// Whether the installed memtrack binary already carries the required capabilities. @@ -94,73 +101,21 @@ pub fn ensure_memtrack_capabilities() -> Result<()> { } pub fn get_memtrack_status() -> ToolStatus { - let tool_name = MEMTRACK_COMMAND.to_string(); - - let is_available = Command::new("which") - .arg(MEMTRACK_COMMAND) - .output() - .is_ok_and(|output| output.status.success()); - if !is_available { - return ToolStatus { - tool_name, - status: ToolInstallStatus::NotInstalled, - }; - } - - let Ok(version_output) = Command::new(MEMTRACK_COMMAND).arg("--version").output() else { - return ToolStatus { - tool_name, - status: ToolInstallStatus::NotInstalled, - }; - }; - - if !version_output.status.success() { - return ToolStatus { - tool_name, - status: ToolInstallStatus::NotInstalled, - }; - } - - let version = String::from_utf8_lossy(&version_output.stdout) - .trim() - .to_string(); - - // Parse the version number from output like "memtrack 1.2.2" - let expected = semver::Version::parse(MEMTRACK_CODSPEED_VERSION).unwrap(); - if let Some(version_str) = version.split_once(' ').map(|(_, v)| v.trim()) { - if let Ok(installed) = semver::Version::parse(version_str) { - if installed < expected { - return ToolStatus { - tool_name, - status: ToolInstallStatus::IncorrectVersion { - version, - message: format!( - "version too old, expecting {MEMTRACK_CODSPEED_VERSION} or higher", - ), - }, - }; - } - return ToolStatus { - tool_name, - status: ToolInstallStatus::Installed { version }, - }; - } - } - + // Bundled: there is nothing to look up on PATH and no version to compare, + // because memtrack ships inside this binary and cannot be out of step with + // it. What is still worth reporting is whether it can actually run, which + // is a question about privileges, not about installation. ToolStatus { - tool_name, - status: ToolInstallStatus::IncorrectVersion { - version, - message: "could not parse version".to_string(), + tool_name: MEMTRACK_COMMAND.to_string(), + status: ToolInstallStatus::Installed { + version: env!("CARGO_PKG_VERSION").to_string(), }, } } +/// Nothing to install any more: memtrack is part of this binary. Kept as a +/// no-op so the setup flow keeps its shape while the other tools still install. pub async fn install_memtrack() -> Result<()> { - ensure_binary_installed( - MEMTRACK_COMMAND, - MEMTRACK_CODSPEED_VERSION, - PinnedBinary::MemtrackInstaller, - ) - .await + debug!("{MEMTRACK_COMMAND} is bundled into this binary, nothing to install"); + Ok(()) } diff --git a/src/executor/tests.rs b/src/executor/tests.rs index 65507fca..de511e2a 100644 --- a/src/executor/tests.rs +++ b/src/executor/tests.rs @@ -457,10 +457,18 @@ mod memory { MEMORY_INIT .get_or_init(|| async { - let executor = MemoryExecutor; - let system_info = SystemInfo::new().unwrap(); - executor.setup(&system_info, None).await.unwrap(); - executor.grant_privileges().unwrap(); + // `grant_privileges` setcaps the binary memtrack will be run + // from, which since the bundling is `current_exe`. Without the + // override that is the test harness, so the capabilities would + // land on a throwaway binary and the run would still lack them. + let self_exe = codspeed_binary_path().await; + temp_env::async_with_vars(&[(SELF_EXE_ENV_VAR, Some(self_exe))], async { + let executor = MemoryExecutor; + let system_info = SystemInfo::new().unwrap(); + executor.setup(&system_info, None).await.unwrap(); + executor.grant_privileges().unwrap(); + }) + .await; }) .await; @@ -487,12 +495,19 @@ mod memory { async fn test_memory_executor(#[case] cmd: &str) { let (_permit, _lock, mut executor) = get_memory_executor().await; + // memtrack is a subcommand of this binary now, so the executor re-execs + // `current_exe` — which under `cargo test` is the test harness, not a + // CLI. Point it at the real binary, as the other executors' tests do. + let self_exe = codspeed_binary_path().await; // Unset GITHUB_ACTIONS to force LocalProvider which supports repository_override - temp_env::async_with_vars(&[("GITHUB_ACTIONS", None::<&str>)], async { - let config = memory_config(cmd); - let (execution_context, _temp_dir) = create_test_setup(config).await; - executor.run(&execution_context, &None).await.unwrap(); - }) + temp_env::async_with_vars( + &[("GITHUB_ACTIONS", None), (SELF_EXE_ENV_VAR, Some(self_exe))], + async { + let config = memory_config(cmd); + let (execution_context, _temp_dir) = create_test_setup(config).await; + executor.run(&execution_context, &None).await.unwrap(); + }, + ) .await; } @@ -502,8 +517,13 @@ mod memory { let (_permit, _lock, mut executor) = get_memory_executor().await; let (env_var, env_value) = env_case; + let self_exe = codspeed_binary_path().await; temp_env::async_with_vars( - &[(env_var, Some(env_value)), ("GITHUB_ACTIONS", None)], + &[ + (env_var, Some(env_value)), + ("GITHUB_ACTIONS", None), + (SELF_EXE_ENV_VAR, Some(self_exe)), + ], async { let cmd = env_var_validation_script(env_var, env_value); let config = memory_config(&cmd); @@ -533,9 +553,16 @@ fi let (execution_context, _temp_dir) = create_test_setup(config).await; let (_permit, _lock, mut executor) = get_memory_executor().await; - temp_env::async_with_vars(&[("PATH", Some(&modified_path))], async { - executor.run(&execution_context, &None).await.unwrap(); - }) + let self_exe = codspeed_binary_path().await; + temp_env::async_with_vars( + &[ + ("PATH", Some(modified_path.as_str())), + (SELF_EXE_ENV_VAR, Some(self_exe)), + ], + async { + executor.run(&execution_context, &None).await.unwrap(); + }, + ) .await; } @@ -564,9 +591,16 @@ fi let (execution_context, _temp_dir) = create_test_setup(config).await; let (_permit, _lock, mut executor) = get_memory_executor().await; - temp_env::async_with_vars(&[("LD_LIBRARY_PATH", Some(&modified))], async { - executor.run(&execution_context, &None).await.unwrap(); - }) + let self_exe = codspeed_binary_path().await; + temp_env::async_with_vars( + &[ + ("LD_LIBRARY_PATH", Some(modified.as_str())), + (SELF_EXE_ENV_VAR, Some(self_exe)), + ], + async { + executor.run(&execution_context, &None).await.unwrap(); + }, + ) .await; } } diff --git a/src/lib.rs b/src/lib.rs index fe86e3e3..c2926fae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,6 @@ //! CodSpeed Runner library mod api_client; -mod binary_installer; mod binary_pins; pub mod cli; mod config; From 763c8947ec114b25be23792325a68da661ec81e8 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 09:27:59 -0400 Subject: [PATCH 17/27] build(memtrack): put the whole musl recipe in the cargo config A bare `cargo build --target -unknown-linux-musl` now works with no environment variables at all. The include flags used to be exported by the caller, on the grounds that `-idirafter /usr/include/` describes Debian's header layout rather than this repo's; they are in fact harmless everywhere, because a `-idirafter` naming a directory that does not exist is ignored silently, and on a glibc host those directories are already on the search path. Checking both multiarch triplets in costs nothing and removes the last thing a release build would have had to inject. Two mechanisms make it expressible, and both cost a failed build to find. `CPATH` rather than `CFLAGS -I`: `[env]`'s `relative = true` can only make a *bare* path absolute, and a `CFLAGS` value has nowhere to put the `-I`. `CPATH` takes bare directories. It resolves against the project root, the directory holding `.cargo/` and not `.cargo/` itself, which the cargo reference words ambiguously. The argp stub therefore sits on the include path for the gnu build too, so it has to defer to the real wherever one exists. `__has_include_next` is the obvious way to write that and it is wrong: the same config puts `-idirafter /usr/include` on the musl build, which makes glibc's argp.h reachable from a musl compilation, and the build then dies on `__THROW`. Including for and branching on `__GLIBC__` tests the libc instead, which is the thing that actually matters. One caveat is documented rather than fixed: cargo's `[env]` does not override a variable already present in the environment unless the entry sets `force = true`, so a shell exporting `CFLAGS` or `CPATH` loses these values. No CI job exports either, and a caller who sets them deliberately should keep them. The check workflow drops its export steps, which is what proves the config stands on its own, and builds `--bin codspeed` rather than `-p memtrack`, since the merged binary is what the release ships. Refs COD-3440 Co-Authored-By: Claude Opus 5 (1M context) --- .cargo/config.toml | 46 +++++--- .github/workflows/cod-3440-musl-check.yml | 131 +++++++--------------- crates/memtrack/musl/argp.h | 19 +++- 3 files changed, 86 insertions(+), 110 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index f8b7ffa0..9f36b44d 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,28 +1,42 @@ -# COD-3440 — what a musl build of `memtrack` needs, for the parts that are a -# property of the repo rather than of the machine doing the build. +# What a musl build of the bundled `memtrack` needs. Refs COD-3440. # # `libbpf-sys` vendors elfutils, whose `configure` unconditionally looks for # `argp`, `obstack` and `fts`. musl ships none of them, so the checks fail and -# the build stops before it ever gets to libelf. A libelf-only build never calls -# into any of them, though — the checks are there for the elfutils CLI tools, -# which we do not build. +# the build stops before it reaches libelf — even though a libelf-only build +# never calls into any of them. Pre-seeding autoconf's cache skips the three +# checks. "none required" is the answer a glibc host reaches on its own, so +# these are unconditional rather than per-target; on gnu they only save three +# `configure` probes. # -# Pre-seeding autoconf's cache makes `configure` skip those three checks -# entirely. "none required" means "the symbol is available with no extra -l -# flag", which is the answer a glibc host would have reached on its own, so -# these are set unconditionally rather than per-target: they are correct for the -# gnu build too, where they only save three `configure` probes. -# -# This is deliberately the whole of what lives here. The remaining piece of the -# recipe — the include flags that point at the `argp.h` stub in -# `crates/memtrack/musl/` and at the kernel UAPI headers — cannot live in a -# `[env]` table, and should not: see `.github/workflows/cod-3440-musl-check.yml` -# for where it goes and why. +# Applies to everything below: cargo does *not* override a variable already set +# in the environment unless the entry carries `force = true`. A shell exporting +# `CFLAGS` or `CPATH` therefore loses these values, and the musl build fails on +# a missing or . They are left unforced so a caller who +# sets them deliberately keeps them; no CI job does. [env] ac_cv_search_argp_parse = "none required" ac_cv_search__obstack_free = "none required" ac_cv_search_fts_close = "none required" +# Where the `argp.h` stub lives. `CPATH` rather than `CFLAGS -I`, because +# `relative = true` can only make a *bare* path absolute and a `CFLAGS` value +# has nowhere to put the `-I`. It resolves against the project root — the +# directory holding `.cargo/`, not `.cargo/` itself. +# +# Not target-scoped, so the stub is on the gnu build's include path too; the +# header defers to the real whenever it detects glibc. +CPATH = { value = "crates/memtrack/musl", relative = true } + +# libbpf includes and . Debian's musl-gcc runs with +# -nostdinc and only sees /usr/include/-linux-musl, so the kernel UAPI +# headers from linux-libc-dev have to be added back. `-idirafter` puts them last, +# behind musl's own, which is what keeps a glibc build unaffected. +# +# Both Debian multiarch triplets are listed because `[env]` cannot branch on the +# host architecture. A `-idirafter` naming a directory that does not exist is +# ignored silently, so the wrong one does nothing — as do both off Debian. +CFLAGS = "-idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include/aarch64-linux-gnu -idirafter /usr/include" + # rustc links with `-nodefaultlibs`, so gcc does not pull in libgcc. On aarch64, # libbpf's C code needs the outline-atomic helpers (`__aarch64_ldadd4_sync` and # friends) that live there, and the link fails without it. x86_64 has no such diff --git a/.github/workflows/cod-3440-musl-check.yml b/.github/workflows/cod-3440-musl-check.yml index 3e3fc159..de1d4fec 100644 --- a/.github/workflows/cod-3440-musl-check.yml +++ b/.github/workflows/cod-3440-musl-check.yml @@ -1,18 +1,9 @@ # COD-3440 spike — throwaway workflow, NOT for merging. # -# Purpose: prove the musl recipe on real kernels of both architectures. The dev +# Purpose: prove the musl build on real kernels of both architectures. The dev # host is aarch64, and an x86_64 build cross-compiled there cannot load its BPF # skeleton against an aarch64 kernel, so x86_64 can only be closed here. # -# The aarch64 half of the recipe has since been reproduced locally on Ubuntu -# 24.04 aarch64 — same distro family as the runners, unlike the Arch host the -# original spike ran on, which is where the `-fno-link-libatomic` and -# `-mno-outline-atomics` workarounds came from. Neither is needed on Debian: -# `cargo build -p memtrack --target aarch64-unknown-linux-musl`, with -# CFLAGS_ as the only out-of-band variable, produces a fully static -# binary and the whole `--lib` suite passes. What this workflow adds on top is -# the x86_64 kernel, the integration shards, and the BPF load itself. -# # To use it: push the spike branch, then # # gh workflow run cod-3440-musl-check.yml --ref spike/cod-3440-memtrack-musl @@ -23,42 +14,33 @@ # dispatch works. Delete the file once the question is answered -- it must not # reach main. # -# Deliberately does NOT touch release.yml, dist-workspace.toml or any -# Cargo.toml. +# Deliberately does NOT touch release.yml or dist-workspace.toml. # -# --- what this workflow still carries, and why --- +# --- what this workflow no longer carries --- # -# The three autoconf cache seeds and the aarch64 `-lgcc` link flag have moved to -# `.cargo/config.toml`: they are properties of the repo, portable, and correct -# for the gnu build too. What is left here is the one part that genuinely -# belongs to the build machine — the include flags: +# It used to export `CFLAGS_` with the path to the `argp.h` stub and the +# `-idirafter` flags for Debian's kernel UAPI headers. The whole recipe now +# lives in `.cargo/config.toml`, so every `cargo` line below is a plain one: +# that is the point of running it, and a failure here means the checked-in +# config does not stand on its own. # -# -I/crates/memtrack/musl the argp.h stub, an absolute path, and a -# `[env]` table cannot build one (its -# `relative = true` form yields a bare path, -# with nowhere to put the -I). -# -idirafter /usr/include/ -idirafter /usr/include -# Debian's kernel UAPI headers. musl-gcc runs -# with -nostdinc and only sees -# /usr/include/-linux-musl, so libbpf's -# and have to be -# added back, last, behind musl's own headers. -# These paths are Debian's layout; an Arch or -# Alpine host needs different ones, so no -# value checked into the repo could be right -# for everyone. +# Two things make that possible, both of which cost a build to discover: # -# Both go into `CFLAGS_` rather than `CFLAGS`. cc-rs reads -# CFLAGS_, CFLAGS_, TARGET_CFLAGS and CFLAGS, -# and accumulates them, so the target-scoped name reaches the musl build and is -# invisible to the gnu one. That is why the "compare against the gnu build" step -# below no longer has to `unset CFLAGS` first. +# * `CPATH` rather than `CFLAGS -I`. `[env]`'s `relative = true` can +# only make a *bare* path absolute, and there is nowhere in such a value to +# put an `-I`. `CPATH` takes bare directories, so it fits. It resolves +# against the project root — the directory holding `.cargo/`, not `.cargo/` +# itself, which the cargo reference words ambiguously. +# * The stub is therefore on the include path for the gnu build too, and +# defers to the real by testing `__GLIBC__`. Testing the include +# path with `__has_include_next` looks equivalent and is not: the same +# config puts `-idirafter /usr/include` on the *musl* build, which makes +# glibc's argp.h reachable from a musl compilation, and the build then dies +# on `__THROW`. # -# It also replaces LIBBPF_SYS_EXTRA_CFLAGS, which this workflow used to set as -# well. libbpf-sys forwards `compiler.cflags_env()` to elfutils' ./configure, -# to zlib's, and to libbpf's make, appending LIBBPF_SYS_EXTRA_CFLAGS only to the -# last of those — so CFLAGS_ alone covers all three. If libbpf (not -# elfutils) is what fails to find , that assumption is what broke. +# The "compare against the gnu build" step below is what guards the second +# point: it builds gnu with the stub on CPATH, and only passes if the guard +# defers correctly. name: COD-3440 musl check @@ -83,13 +65,9 @@ jobs: - arch: x86_64 runner: ubuntu-latest target: x86_64-unknown-linux-musl - cflags_var: CFLAGS_x86_64_unknown_linux_musl - triplet: x86_64-linux-gnu - arch: aarch64 runner: ubuntu-24.04-arm target: aarch64-unknown-linux-musl - cflags_var: CFLAGS_aarch64_unknown_linux_musl - triplet: aarch64-linux-gnu env: TARGET: ${{ matrix.target }} steps: @@ -106,23 +84,15 @@ jobs: sudo apt-get install -y musl-tools pkg-config linux-libc-dev rustup target add "$TARGET" - # Exported once here so every later step sees the same value, and so the - # `cargo build` below is a plain one with nothing prepended to it. - - name: Point the musl build at the stub and the UAPI headers - run: | - VALUE="-I$GITHUB_WORKSPACE/crates/memtrack/musl -idirafter /usr/include/${{ matrix.triplet }} -idirafter /usr/include" - # The target-scoped name is what cc-rs reads; MUSL_CFLAGS is a plain - # mirror of it, so later steps can pass the value on without having to - # index the env context by a matrix-computed key. - echo "${{ matrix.cflags_var }}=$VALUE" >> "$GITHUB_ENV" - echo "MUSL_CFLAGS=$VALUE" >> "$GITHUB_ENV" - + # The root package is what ships now: it bundles memtrack and + # exec-harness as hidden subcommands, so building `-p memtrack` alone + # would no longer prove anything about the released artifact. - name: Build - run: cargo build -p memtrack --profile ${{ matrix.profile }} --target "$TARGET" + run: cargo build --bin codspeed --profile ${{ matrix.profile }} --target "$TARGET" - name: Verify the artifact is genuinely static run: | - BIN=target/$TARGET/${{ matrix.profile == 'dev' && 'debug' || matrix.profile }}/codspeed-memtrack + BIN=target/$TARGET/${{ matrix.profile == 'dev' && 'debug' || matrix.profile }}/codspeed file "$BIN" ldd "$BIN" || true # expected: "not a dynamic executable" readelf -d "$BIN" || true # expected: no dynamic section at all @@ -136,21 +106,23 @@ jobs: fi echo "OK: static, no NEEDED, no RPATH/RUNPATH" - # No `unset CFLAGS` any more: the include flags are scoped to the musl - # target, so a gnu build in the same shell cannot see them. If this step - # fails on a missing or wrong argp.h, that scoping is what leaked. + # The stub directory is on CPATH for this build too, so a gnu build that + # still succeeds is the proof that the header's `__GLIBC__` guard defers + # to the real . If this fails on `__THROW`, that guard is what + # broke. - name: Compare against the gnu build if: matrix.profile == 'dist' run: | - cargo build -p memtrack --profile dist - echo "musl: $(stat -c %s target/$TARGET/dist/codspeed-memtrack) bytes" - echo "gnu: $(stat -c %s target/dist/codspeed-memtrack) bytes" + cargo build --bin codspeed --profile dist + echo "musl: $(stat -c %s target/$TARGET/dist/codspeed) bytes" + echo "gnu: $(stat -c %s target/dist/codspeed) bytes" - name: Smoke test the BPF path run: | - BIN=$PWD/target/$TARGET/${{ matrix.profile == 'dev' && 'debug' || matrix.profile }}/codspeed-memtrack + BIN=$PWD/target/$TARGET/${{ matrix.profile == 'dev' && 'debug' || matrix.profile }}/codspeed mkdir -p /tmp/memtrack-out - sudo env "RUST_LOG=info" "$BIN" track -o /tmp/memtrack-out "/bin/ls /tmp" + # Through the bundled subcommand, which is how the runner reaches it. + sudo env "RUST_LOG=info" "$BIN" memtrack track -o /tmp/memtrack-out "/bin/ls /tmp" ls -la /tmp/memtrack-out # A run that loads no probes still exits 0 but writes nothing. test -n "$(ls -A /tmp/memtrack-out)" || { echo "no artifact written"; exit 1; } @@ -169,13 +141,9 @@ jobs: - arch: x86_64 runner: ubuntu-latest target: x86_64-unknown-linux-musl - cflags_var: CFLAGS_x86_64_unknown_linux_musl - triplet: x86_64-linux-gnu - arch: aarch64 runner: ubuntu-24.04-arm target: aarch64-unknown-linux-musl - cflags_var: CFLAGS_aarch64_unknown_linux_musl - triplet: aarch64-linux-gnu env: TARGET: ${{ matrix.target }} steps: @@ -196,15 +164,6 @@ jobs: - name: Install additional allocators run: sudo apt-get install -y libmimalloc-dev libjemalloc-dev - - name: Point the musl build at the stub and the UAPI headers - run: | - VALUE="-I$GITHUB_WORKSPACE/crates/memtrack/musl -idirafter /usr/include/${{ matrix.triplet }} -idirafter /usr/include" - # The target-scoped name is what cc-rs reads; MUSL_CFLAGS is a plain - # mirror of it, so later steps can pass the value on without having to - # index the env context by a matrix-computed key. - echo "${{ matrix.cflags_var }}=$VALUE" >> "$GITHUB_ENV" - echo "MUSL_CFLAGS=$VALUE" >> "$GITHUB_ENV" - # Built separately from the run because test-with's env(GITHUB_ACTIONS) # gate is evaluated at COMPILE time. GITHUB_ACTIONS is set by the runner, # so this is automatic here -- but if the tests are ever built outside @@ -229,7 +188,6 @@ jobs: "CARGO_INCREMENTAL=$CARGO_INCREMENTAL" \ "RUST_LOG=$RUST_LOG" \ "GITHUB_ACTIONS=$GITHUB_ACTIONS" \ - "${{ matrix.cflags_var }}=$MUSL_CFLAGS" \ "TARGET=$TARGET" \ $(which cargo) test --target "$TARGET" --test ${{ matrix.test }} \ -- --test-threads 1 --nocapture @@ -250,13 +208,9 @@ jobs: - arch: x86_64 runner: ubuntu-latest target: x86_64-unknown-linux-musl - cflags_var: CFLAGS_x86_64_unknown_linux_musl - triplet: x86_64-linux-gnu - arch: aarch64 runner: ubuntu-24.04-arm target: aarch64-unknown-linux-musl - cflags_var: CFLAGS_aarch64_unknown_linux_musl - triplet: aarch64-linux-gnu env: TARGET: ${{ matrix.target }} steps: @@ -273,15 +227,6 @@ jobs: sudo apt-get install -y musl-tools pkg-config linux-libc-dev rustup target add "$TARGET" - - name: Point the musl build at the stub and the UAPI headers - run: | - VALUE="-I$GITHUB_WORKSPACE/crates/memtrack/musl -idirafter /usr/include/${{ matrix.triplet }} -idirafter /usr/include" - # The target-scoped name is what cc-rs reads; MUSL_CFLAGS is a plain - # mirror of it, so later steps can pass the value on without having to - # index the env context by a matrix-computed key. - echo "${{ matrix.cflags_var }}=$VALUE" >> "$GITHUB_ENV" - echo "MUSL_CFLAGS=$VALUE" >> "$GITHUB_ENV" - # `libc_allocator_symbols_resolve_to_offsets` used to be skipped here: it # read /proc/self/maps of the TEST BINARY and required a mapped libc.so.6, # which a statically linked musl binary does not have by construction. The diff --git a/crates/memtrack/musl/argp.h b/crates/memtrack/musl/argp.h index 26a03a69..87dbd8fd 100644 --- a/crates/memtrack/musl/argp.h +++ b/crates/memtrack/musl/argp.h @@ -1,7 +1,22 @@ /* crates/memtrack/musl/argp.h — stub for musl builds of libbpf-sys' vendored elfutils. Declarations only: a libelf-only build never calls into argp, but the elfutils sources still `#include `, which musl does not ship. - If compilation complains about a missing type or macro, add it here. */ + If compilation complains about a missing type or macro, add it here. + + This directory is on `CPATH` for *every* build, gnu included, so the header + has to defer to a real wherever one exists. + + It branches on the libc rather than on the include path, and the difference + matters: `.cargo/config.toml` also puts `-idirafter /usr/include` on the musl + build, for libbpf's kernel UAPI headers, which makes glibc's argp.h reachable + from a musl compilation. `__has_include_next` would find it and the build + would die on `__THROW`. is included only to pull in , + which defines `__GLIBC__`. */ +#include +#if defined(__GLIBC__) +#include_next +#else + #ifndef CODSPEED_STUB_ARGP_H #define CODSPEED_STUB_ARGP_H @@ -41,3 +56,5 @@ struct argp { int argp_help(const struct argp *argp, FILE *stream, unsigned int flags, char *name); #endif /* CODSPEED_STUB_ARGP_H */ + +#endif /* __GLIBC__ */ From 36afa1820902ff12c0700d9def09616275306ce6 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 09:30:41 -0400 Subject: [PATCH 18/27] build(exec-harness,memtrack): stop releasing the component crates Both ship inside the `codspeed` binary as hidden subcommands, so a standalone artifact for either is something nothing consumes and one more thing that can be out of step. Dropping their `[package.metadata.dist]` makes one tag produce one artifact set. Their `[[bin]]` targets stay: development and the tests still build them to exercise the standalone path. memtrack's apt build dependencies moved to the root package in the bundling commit, which is the one cargo-dist now builds. Its `features = ["libbpf-rs/static"]` is not lost either: libbpf-rs is pulled in with `vendored`, which builds libbpf from source and links it statically anyway, and the merged musl binary has no NEEDED, no RPATH and no RUNPATH. Refs COD-3440 Co-Authored-By: Claude Opus 5 (1M context) --- crates/exec-harness/Cargo.toml | 5 +++-- crates/memtrack/Cargo.toml | 17 +++-------------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/crates/exec-harness/Cargo.toml b/crates/exec-harness/Cargo.toml index 277f0155..04a168ed 100644 --- a/crates/exec-harness/Cargo.toml +++ b/crates/exec-harness/Cargo.toml @@ -21,5 +21,6 @@ humantime = "2.3" runner-shared = { path = "../runner-shared" } tempfile = { workspace = true } -[package.metadata.dist] -targets = ["aarch64-unknown-linux-gnu", "x86_64-unknown-linux-gnu"] +# Deliberately no `[package.metadata.dist]`: exec-harness ships inside the +# `codspeed` binary and is not released on its own. The `[[bin]]` stays for +# development and for the tests, which exercise the standalone path. diff --git a/crates/memtrack/Cargo.toml b/crates/memtrack/Cargo.toml index d97ed4f7..c8e241eb 100644 --- a/crates/memtrack/Cargo.toml +++ b/crates/memtrack/Cargo.toml @@ -50,17 +50,6 @@ test-log = { workspace = true } insta = { workspace = true, features = ["json", "redactions"] } test-with = { workspace = true } -[package.metadata.dist] -targets = ["aarch64-unknown-linux-gnu", "x86_64-unknown-linux-gnu"] -features = ["libbpf-rs/static"] - -[package.metadata.dist.dependencies.apt] -build-essential = "*" -pkgconf = "*" -zlib1g-dev = "*" -libbpf-dev = "*" - -# Required for the vendored feature -autopoint = "*" -bison = "*" -flex = "*" +# Deliberately no `[package.metadata.dist]`: memtrack ships inside the +# `codspeed` binary and is not released on its own. Its apt build dependencies +# belong to the root package, which is what cargo-dist builds. From a05e42e5d20f838983da717c085206deb3acb048 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 09:30:41 -0400 Subject: [PATCH 19/27] ci: catch up with memtrack and exec-harness being bundled Neither is resolved off PATH any more, so three steps were provisioning binaries nothing would use: `cargo install --path crates/memtrack`, the `setup --mode memory` that setcap'd it, and the macOS `cargo install --path crates/exec-harness`. The setcap step is not merely redundant but wrong now that the capabilities belong on `codspeed` itself: it would grant them to whichever `target/debug/codspeed` cargo last wrote, which is not necessarily the one the tests re-exec. The memory tests already grant them in `MEMORY_INIT`, pointed through `CODSPEED_SELF_EXE` at the binary they run. The other direction: linking memtrack in means building its vendored libbpf-sys, so the libbpf toolchain became a build dependency of anything that compiles the runner. `lint` (which runs `generate_config_schema`) and `basic-run-test` build it and had no such step; both get one, gated to Linux on `lint` since memtrack is Linux-only in the root manifest. The COD-3218 check workflow would otherwise have gone green while testing nothing. Its musl leg proved itself by installing a musl exec-harness on PATH and pinning its hash across the run, both of which hinge on a PATH lookup that no longer happens -- so the install was inert, the guard guarded nothing, and the leg ran a gnu `cargo run` runner exactly like the gnu leg. It now builds `codspeed` for the leg's libc, asserts `codspeed exec-harness --version` and the static properties, and invokes that binary directly. In the cost sweep the install and the tamper guard survive but only on the `main` variant, which is still the PATH-based world. Refs COD-3440 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 17 +-- .../workflows/cod-3218-exec-harness-check.yml | 115 +++++++++++------- 2 files changed, 75 insertions(+), 57 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f8fe75ea..6120425b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,9 @@ jobs: - uses: ./.github/actions/install-rust with: components: rustfmt, clippy + # Building the runner builds memtrack's vendored libbpf-sys with it. + - uses: ./.github/actions/install-bpf-deps + if: matrix.os == 'ubuntu-latest' - uses: j178/prek-action@bdca6f102f98e2b4c7029491a53dfd366469e33d # v2.0.4 with: extra-args: --all-files @@ -36,15 +39,10 @@ jobs: - uses: ./.github/actions/install-rust - # Install memtrack for the memory integration tests - uses: ./.github/actions/install-bpf-deps - - name: Install memtrack - run: | - cargo install --path crates/memtrack --locked - - - name: Grant memtrack file capabilities - run: cargo r -- setup --mode memory + # No `setup --mode memory` here: the memory tests grant the capabilities + # themselves, pointed at the binary they actually re-exec. - run: cargo test --all --exclude memtrack --exclude exec-harness exec-harness-tests: @@ -64,6 +62,7 @@ jobs: with: submodules: true - uses: ./.github/actions/install-rust + - uses: ./.github/actions/install-bpf-deps - name: Run tests run: cargo run -- exec -m simulation,walltime,memory --warmup-time 0s --max-rounds 5 -- sleep 1 @@ -74,10 +73,6 @@ jobs: with: submodules: true - uses: ./.github/actions/install-rust - - name: Install exec-harness - run: | - cargo install --path crates/exec-harness --locked - - name: Run tests env: # Profiling system commands (e.g. `ls`) with samply is not yet supported on MacOS diff --git a/.github/workflows/cod-3218-exec-harness-check.yml b/.github/workflows/cod-3218-exec-harness-check.yml index 4e4ba4a3..c958898f 100644 --- a/.github/workflows/cod-3218-exec-harness-check.yml +++ b/.github/workflows/cod-3218-exec-harness-check.yml @@ -28,6 +28,17 @@ # # Deliberately does NOT touch ci.yml, release.yml, dist-workspace.toml or any # Cargo.toml: it only adds jobs that run on this branch. +# +# --- updated for the single binary (COD-3440) --- +# +# exec-harness is no longer resolved off PATH: it is a subcommand of `codspeed`, +# reached by re-execing the running executable. So `cargo install --path +# crates/exec-harness` no longer influences what runs, and the sha256 tamper +# guard built on it guards nothing. Worse, it was the musl leg's only claim to +# testing musl -- the runner itself was always a gnu `cargo run`. The +# instrumentation job now builds the `codspeed` binary for the leg's libc and +# invokes it directly, which is both the real artifact and the only way the musl +# leg still means anything. name: COD-3218 exec-harness check @@ -79,15 +90,19 @@ jobs: # bindings build script now fails the build outright rather than # silently compiling the noop implementation. run: | - sudo apt-get install -y musl-tools + sudo apt-get install -y musl-tools linux-libc-dev rustup target add "$MUSL_TARGET" + # The released artifact is `codspeed`, with exec-harness inside it, so + # that is what has to be static. Building `-p exec-harness` alone would + # still pass while saying nothing about what ships. + - uses: ./.github/actions/install-bpf-deps - name: Build - run: cargo build -p exec-harness --target "$MUSL_TARGET" + run: cargo build --bin codspeed --target "$MUSL_TARGET" - name: Verify the artifact is genuinely static run: | - BIN=target/$MUSL_TARGET/debug/exec-harness + BIN=target/$MUSL_TARGET/debug/codspeed file "$BIN" ldd "$BIN" || true # expected: "not a dynamic executable" readelf -d "$BIN" || true # expected: no dynamic section at all @@ -127,39 +142,46 @@ jobs: - name: Install musl toolchain if: matrix.libc == 'musl' run: | - sudo apt-get install -y musl-tools + sudo apt-get install -y musl-tools linux-libc-dev rustup target add "$MUSL_TARGET" - # The runner resolves exec-harness off PATH and only downloads the - # released build when `which exec-harness` is missing or reports a - # version other than the pin (src/binary_installer/mod.rs). Installing - # our build first is therefore what makes this job test anything -- see - # the tamper guard in the next step. - - name: Install the exec-harness under test + # The root package links memtrack now, so building `codspeed` at all + # means building its vendored libbpf-sys. + - uses: ./.github/actions/install-bpf-deps + + # The binary under test is `codspeed` itself, because exec-harness is a + # subcommand of it. Building it for the leg's libc is what makes the musl + # leg a musl test: with `cargo run` it would be a gnu runner every time, + # whatever was installed on PATH. + - name: Build the codspeed binary under test + id: build run: | if [ "${{ matrix.libc }}" = "musl" ]; then - cargo install --path crates/exec-harness --locked --target "$MUSL_TARGET" + cargo build --bin codspeed --target "$MUSL_TARGET" + BIN=$PWD/target/$MUSL_TARGET/debug/codspeed else - cargo install --path crates/exec-harness --locked + cargo build --bin codspeed + BIN=$PWD/target/debug/codspeed fi - BIN=$(which exec-harness) - echo "$BIN" - exec-harness --version + echo "bin=$BIN" >> "$GITHUB_OUTPUT" file "$BIN" - # Prove the musl matrix leg is really exercising the musl artifact, - # rather than a leftover gnu build earlier on PATH. Asserted through - # readelf and not a `file` string: rustc emits a static-PIE for - # x86_64 musl, which `file` calls "static-pie linked" rather than - # "statically linked" (aarch64 gets the non-PIE spelling), so matching - # that wording tests the wrong axis and fails on a perfectly static - # binary. What matters is that nothing is loaded at runtime. + # Proves the bundling as well as the build: this reports a version + # only if the exec-harness CLI really is linked into this executable. + "$BIN" exec-harness --version + + # Asserted through readelf and not a `file` string: rustc emits a + # static-PIE for x86_64 musl, which `file` calls "static-pie linked" + # rather than "statically linked" (aarch64 gets the non-PIE + # spelling), so matching that wording tests the wrong axis and fails + # on a perfectly static binary. What matters is that nothing is + # loaded at runtime. if [ "${{ matrix.libc }}" = "musl" ]; then if readelf -d "$BIN" 2>/dev/null | grep -qE 'NEEDED|RPATH|RUNPATH'; then - echo "the installed exec-harness has a dynamic dependency" + echo "the codspeed binary has a dynamic dependency" exit 1 fi if readelf -lW "$BIN" 2>/dev/null | grep -q 'INTERP'; then - echo "the installed exec-harness requests a dynamic loader" + echo "the codspeed binary requests a dynamic loader" exit 1 fi echo "OK: no NEEDED and no interpreter -- nothing is loaded at runtime" @@ -172,26 +194,17 @@ jobs: mkdir -p "$PROFILE_DIR" echo "profile_dir=$PROFILE_DIR" >> "$GITHUB_OUTPUT" - # If the runner swapped in the *released* exec-harness, that would be - # the preload build and this whole job would pass while proving - # nothing. Pin the binary's hash across the run. - BEFORE=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) - - CODSPEED_LOG=debug cargo run -- exec \ + # Invoked directly rather than through `cargo run`, so the process + # that re-execs itself into exec-harness is the binary just built and + # asserted on above. No tamper guard is needed any more: there is no + # PATH lookup left for a released build to win. + CODSPEED_LOG=debug "${{ steps.build.outputs.bin }}" exec \ -m simulation \ --skip-upload \ --profile-folder "$PROFILE_DIR" \ --name "$BENCH_NAME" \ -- sh -c "$BENCH_SCRIPT" - AFTER=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) - if [ "$BEFORE" != "$AFTER" ]; then - echo "the runner replaced exec-harness with the released (preload) build" - echo "=> this run measured the old code path, not the change under test" - exit 1 - fi - echo "OK: the exec-harness under test was the one used" - - name: Show what was produced if: always() && steps.run.outputs.profile_dir != '' run: | @@ -384,17 +397,25 @@ jobs: with: cache-key: cod3218-sweep-${{ matrix.variant }} + # Only the `main` variant still resolves exec-harness off PATH; on the + # branch it is bundled, so installing it there would do nothing. - name: Install the exec-harness under test + if: matrix.variant == 'main' run: | cargo install --path crates/exec-harness --locked exec-harness --version + - uses: ./.github/actions/install-bpf-deps + if: matrix.variant != 'main' + - name: Sweep run: | - # Same tamper guard as the instrumentation job: if the runner swapped - # in the released exec-harness, the sweep would silently measure the - # wrong binary on the branch variant. - BEFORE=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) + # Tamper guard, `main` variant only: there the runner can still swap + # in the released exec-harness and the sweep would measure the wrong + # binary. On the branch there is no PATH lookup left to tamper with. + if [ "${{ matrix.variant }}" = "main" ]; then + BEFORE=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) + fi for N in 1000 10000 100000 1000000; do D=$RUNNER_TEMP/sweep-$N @@ -411,8 +432,10 @@ jobs: echo "SWEEP variant=${{ matrix.variant }} N=$N files=${#OUTS[@]} TOTAL_IR=$TOTAL" done - AFTER=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) - if [ "$BEFORE" != "$AFTER" ]; then - echo "the runner replaced exec-harness mid-sweep; results are not trustworthy" - exit 1 + if [ "${{ matrix.variant }}" = "main" ]; then + AFTER=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) + if [ "$BEFORE" != "$AFTER" ]; then + echo "the runner replaced exec-harness mid-sweep; results are not trustworthy" + exit 1 + fi fi From 6fdfd0793660196b4a70018c5dd6294e0f03bf31 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 09:30:41 -0400 Subject: [PATCH 20/27] docs(contributing): drop the component-crate release process `cargo release -p memtrack` and the post-release bump of `MEMTRACK_INSTALLER`, `EXEC_HARNESS_INSTALLER`, `MEMTRACK_VERSION` and `EXEC_HARNESS_VERSION` all describe constants that were deleted with the download machinery. Following those steps today would mean publishing an artifact nothing consumes and editing symbols that are not there. One crate is released now; the other two are linked into its binary and keep their `version` field only as what `--version` reports. `binary_pins.rs` holds the valgrind .deb and the mongo-tracer installer, and the pinned-hash section says so. Refs COD-3440 Co-Authored-By: Claude Opus 5 (1M context) --- CONTRIBUTING.md | 52 +++++++++++++------------------------------------ 1 file changed, 13 insertions(+), 39 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c47e5ace..9d04a714 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,50 +10,28 @@ prek install ## Release Process -This repository is a Cargo workspace containing multiple crates. The release process differs depending on which crate you're releasing. +This repository is a Cargo workspace containing multiple crates, but only one of them is released: the main runner. Everything else is linked into its binary. ### Workspace Structure - **`codspeed-runner`**: The main CLI binary (`codspeed`) -- **`memtrack`**: Memory tracking binary (`codspeed-memtrack`) -- **`exec-harness`**: Execution harness binary +- **`memtrack`**: Memory tracker, built into `codspeed` and reached as `codspeed memtrack` +- **`exec-harness`**: Execution harness, built into `codspeed` and reached as `codspeed exec-harness` - **`runner-shared`**: Shared library used by other crates -### Releasing Support Crates (memtrack, exec-harness, runner-shared) +`memtrack` and `exec-harness` are **not released on their own**. They are linked into the +`codspeed` binary and invoked as hidden subcommands, so one tag produces one artifact set and +there is no version for the runner to be out of step with. Their `[[bin]]` targets remain for +development and for the tests, which build them to exercise the standalone path. -For any crate other than the main runner: - -```bash -cargo release -p --execute -``` - -Where `` is one of: `alpha`, `beta`, `patch`, `minor`, or `major`. - -**Examples:** - -```bash -# Release a new patch version of memtrack -cargo release -p memtrack --execute patch - -# Release a beta version of exec-harness -cargo release -p exec-harness --execute beta -``` - -#### Post-Release: Update Version References - -After releasing `memtrack` or `exec-harness`, you **must** update the version references in the runner code: - -1. **For memtrack**: Update the `MEMTRACK_INSTALLER` pin record in `src/binary_pins.rs` (see [Pinned binary hashes](#pinned-binary-hashes) below). - -2. **For exec-harness**: Update the `EXEC_HARNESS_INSTALLER` pin record in `src/binary_pins.rs`. - -These constants are used by the runner to download and install the correct versions of the binaries from GitHub releases. +Both still keep their own `version` in `Cargo.toml` — that is what +`codspeed exec-harness --version` reports — but bumping it is a plain edit, not a release. ### Pinned binary hashes Every binary the runner downloads at install time is SHA-256-pinned. The pins live in two places: -- **`src/binary_pins.rs`** — the patched valgrind `.deb`, the memtrack installer, the exec-harness installer, and the mongo-tracer installer. Each artifact keeps its version, URL template, and hash together in a pin record. +- **`src/binary_pins.rs`** — the patched valgrind `.deb` and the mongo-tracer installer. Each artifact keeps its version, URL template, and hash together in a pin record. - **`src/executor/helpers/introspected_golang/go.sh`** — the go-runner installer published by [CodSpeedHQ/codspeed-go](https://github.com/CodSpeedHQ/codspeed-go), one ` ` row per release in the `GO_RUNNER_INSTALLER_SHA256S` table. `DEFAULT_GO_RUNNER_VERSION` (just below the table) selects the row used by default. When you bump a pinned version (or add a new go-runner row), update the matching pin record / table row with the new version and its SHA-256. @@ -84,16 +62,12 @@ These tests also run in CI, but running them locally before opening the PR avoid ### Releasing the Main Runner -The main runner (`codspeed-runner`) should be released after ensuring all dependency versions are correct. +The main runner (`codspeed-runner`) is the only crate that is released. #### Pre-Release Check -**Verify binary version references**: Check that version constants in the runner code match the released versions: - -- `MEMTRACK_VERSION` in `src/binary_pins.rs` -- `EXEC_HARNESS_VERSION` in `src/binary_pins.rs` - -Also confirm the SHA-256 entries in the pin records in `src/binary_pins.rs` match the released artifacts. +Confirm the SHA-256 entries in the pin records in `src/binary_pins.rs` match the released +artifacts they point at. #### Release Command From 3ce630583834c49489c6b602bd48ab23a53509ca Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 09:30:41 -0400 Subject: [PATCH 21/27] test(executor): serialize the tests that share the global runner FIFOs `cargo test` never returned: every memory test sat there indefinitely, and two `exec-harness` processes were left sleeping in anon_pipe_read for ten hours, orphaned, holding 3 -> /tmp/runner.ctl.fifo (deleted) 4 -> /tmp/runner.ack.fifo (deleted) `RUNNER_CTL_FIFO` and `RUNNER_ACK_FIFO` are one fixed pair of paths -- a protocol constant shared with the integrations, not something a run can relocate -- and `RunnerFifo::new` unlinks and recreates both. Two executions overlapping pull the FIFO out from under each other: the first keeps its fds on an inode that is now unlinked, the second's child opens the replacement by path, and they never meet again. Nothing there times out, so `handle_fifo_messages` loops waiting for a child that is itself blocked reading an ack, and the test hangs instead of failing. The per-mode semaphores could not prevent it, because the two tests that collide hold different ones: walltime took `WALLTIME_SEMAPHORE`, memory took `MEMORY_SEMAPHORE`, and both then called `RunnerFifo::new`. Replace the pair with a single permit. It keeps what each was for -- perf is not thread-safe, the memory tracker cannot overlap with itself -- and adds the exclusion that was missing. The BPF lock is always taken after the FIFO permit; valgrind takes only the BPF lock and walltime only the FIFO permit, so nothing wants the two in opposite orders. This is a test-level fix for a race that also exists outside the tests: two `codspeed` runs on one machine deadlock each other the same way. Closing that needs either a negotiated per-run FIFO path or a watchdog, neither of which belongs in this commit. Also guards a second way these tests can hang, unrelated to the above and not reachable on a machine with passwordless sudo: `MEMORY_INIT` setcaps the binary, and on a machine that prompts, sudo blocks on a password nobody can see, after every rebuild -- file capabilities are an xattr on the inode and cargo writes a new `codspeed` each relink. Assert up front that the grant needs no prompt and print the exact `setcap` line if it does. `memtrack_setcap_spec` becomes `pub(crate)` so that line cannot drift from the real one. Refs COD-3440 Refs COD-3560 Co-Authored-By: Claude Opus 5 (1M context) --- src/executor/memory/setup.rs | 27 ++++++------- src/executor/tests.rs | 75 +++++++++++++++++++++++++++--------- 2 files changed, 67 insertions(+), 35 deletions(-) diff --git a/src/executor/memory/setup.rs b/src/executor/memory/setup.rs index 4881fb85..e3d67e41 100644 --- a/src/executor/memory/setup.rs +++ b/src/executor/memory/setup.rs @@ -6,8 +6,7 @@ use crate::prelude::*; use caps::Capability; use std::path::PathBuf; -/// How memtrack is named in user-facing messages. It is no longer a binary to -/// look up: memtrack is bundled into this executable as a hidden subcommand. +/// How memtrack is named in user-facing messages. pub const MEMTRACK_COMMAND: &str = "memtrack"; const MEMTRACK_REQUIRED_CAPS: &[Capability] = &[ @@ -27,7 +26,7 @@ fn memtrack_required_caps_mask() -> u64 { /// `setcap` grammar form of [`MEMTRACK_REQUIRED_CAPS`]: the lowercase cap names /// (libcap renders them lowercase) joined with commas and the `+ep` /// effective+permitted flag. Derived from the enum so the two never drift. -fn memtrack_setcap_spec() -> String { +pub(crate) fn memtrack_setcap_spec() -> String { let caps = MEMTRACK_REQUIRED_CAPS .iter() .map(|c| c.to_string().to_lowercase()) @@ -36,14 +35,12 @@ fn memtrack_setcap_spec() -> String { format!("{caps}+ep") } -/// The binary that must carry the eBPF capabilities. +/// The binary that must carry the eBPF capabilities: memtrack runs as a +/// subcommand of this executable, so it is this one. /// -/// Since memtrack is bundled, that binary is *this* one. Note what that means: -/// the five capabilities below, `CAP_SYS_ADMIN` among them, end up on the -/// `codspeed` executable itself rather than on a dedicated tracker, so every -/// invocation of the CLI carries them in its permitted and effective sets. -/// They are granted `+ep` and not inheritable, so a spawned benchmark does not -/// receive them — the elevation stops at the CLI process. +/// Every invocation of the CLI therefore carries [`MEMTRACK_REQUIRED_CAPS`], +/// `CAP_SYS_ADMIN` included. They are granted `+ep` and not inheritable, so a +/// spawned benchmark does not receive them: the elevation stops here. fn memtrack_path() -> Option { self_exe().ok() } @@ -101,10 +98,8 @@ pub fn ensure_memtrack_capabilities() -> Result<()> { } pub fn get_memtrack_status() -> ToolStatus { - // Bundled: there is nothing to look up on PATH and no version to compare, - // because memtrack ships inside this binary and cannot be out of step with - // it. What is still worth reporting is whether it can actually run, which - // is a question about privileges, not about installation. + // memtrack ships inside this binary, so it is installed by construction + // and carries this crate's version. ToolStatus { tool_name: MEMTRACK_COMMAND.to_string(), status: ToolInstallStatus::Installed { @@ -113,8 +108,8 @@ pub fn get_memtrack_status() -> ToolStatus { } } -/// Nothing to install any more: memtrack is part of this binary. Kept as a -/// no-op so the setup flow keeps its shape while the other tools still install. +/// No-op: memtrack is part of this binary. Kept so the setup flow can treat it +/// like the tools that do need installing. pub async fn install_memtrack() -> Result<()> { debug!("{MEMTRACK_COMMAND} is bundled into this binary, nothing to install"); Ok(()) diff --git a/src/executor/tests.rs b/src/executor/tests.rs index de511e2a..d18bd520 100644 --- a/src/executor/tests.rs +++ b/src/executor/tests.rs @@ -175,6 +175,29 @@ fi (execution_context, temp_dir) } + /// Serializes every executor test that drives a benchmark through the runner + /// FIFOs: the walltime and memory tests. + /// + /// `RUNNER_CTL_FIFO` and `RUNNER_ACK_FIFO` are fixed absolute paths shared + /// with the integrations, and `RunnerFifo::new` unlinks and recreates both. + /// Two overlapping executions therefore pull the FIFO out from under each + /// other: the first keeps its fds on a now-unlinked inode, the second's + /// child opens the replacement by path, and they never meet again. Nothing + /// there times out, so the test hangs forever instead of failing. + /// + /// The race exists outside the tests too, where this lock cannot reach it: + /// COD-3560. + pub static RUNNER_FIFO_LOCK: OnceCell = OnceCell::const_new(); + + pub async fn acquire_runner_fifo_lock() -> SemaphorePermit<'static> { + RUNNER_FIFO_LOCK + .get_or_init(|| async { Semaphore::new(1) }) + .await + .acquire() + .await + .unwrap() + } + // Uprobes set by memtrack, lead to crashes in valgrind because they work by setting breakpoints on the first // instruction. Valgrind doesn't rethrow those breakpoint exceptions, which makes the test crash. // @@ -256,14 +279,10 @@ mod walltime { use crate::executor::wall_time::executor::WallTimeExecutor; async fn get_walltime_executor() -> (SemaphorePermit<'static>, WallTimeExecutor) { - static WALLTIME_SEMAPHORE: OnceCell = OnceCell::const_new(); - // We can't execute multiple walltime executors in parallel because perf isn't thread-safe (yet). We have to - // use a semaphore to limit concurrent access. - let semaphore = WALLTIME_SEMAPHORE - .get_or_init(|| async { Semaphore::new(1) }) - .await; - let permit = semaphore.acquire().await.unwrap(); + // use a semaphore to limit concurrent access. The same permit also + // excludes the memory tests, which share the global runner FIFOs. + let permit = acquire_runner_fifo_lock().await; let executor = WallTimeExecutor::new(None); let system_info = SystemInfo::new().unwrap(); @@ -445,7 +464,9 @@ fi #[cfg(target_os = "linux")] mod memory { use super::helpers::*; + use crate::executor::helpers::run_with_sudo::{can_elevate_without_prompt, is_root_user}; use crate::executor::memory::executor::MemoryExecutor; + use crate::executor::memory::setup::{has_memtrack_capabilities, memtrack_setcap_spec}; async fn get_memory_executor() -> ( SemaphorePermit<'static>, @@ -453,16 +474,32 @@ mod memory { MemoryExecutor, ) { static MEMORY_INIT: OnceCell<()> = OnceCell::const_new(); - static MEMORY_SEMAPHORE: OnceCell = OnceCell::const_new(); MEMORY_INIT .get_or_init(|| async { - // `grant_privileges` setcaps the binary memtrack will be run - // from, which since the bundling is `current_exe`. Without the - // override that is the test harness, so the capabilities would - // land on a throwaway binary and the run would still lack them. + // `grant_privileges` setcaps the binary memtrack is run from, + // i.e. `current_exe`. Without the override that is the test + // harness, so the capabilities would land on a throwaway binary + // and the run would still lack them. let self_exe = codspeed_binary_path().await; temp_env::async_with_vars(&[(SELF_EXE_ENV_VAR, Some(self_exe))], async { + // `setcap` goes through sudo, and a sudo password prompt + // under `cargo test` is buried in captured output while the + // read blocks forever: the suite hangs with nothing to say + // what it is waiting for. Check first, fail with the command + // to run. This bites on every rebuild, not once, because + // file capabilities are an xattr and cargo writes a new + // binary on each relink. + let needs_grant = !is_root_user() && !has_memtrack_capabilities(); + assert!( + !needs_grant || can_elevate_without_prompt(), + "The memory tests have to `setcap` {self_exe}, and sudo would prompt for a \ + password here -- a prompt `cargo test` hides and then blocks on forever.\n\ + Cache the credentials first (`sudo -v && cargo test ...`), or grant them \ + by hand:\n sudo setcap {} {self_exe}", + memtrack_setcap_spec(), + ); + let executor = MemoryExecutor; let system_info = SystemInfo::new().unwrap(); executor.setup(&system_info, None).await.unwrap(); @@ -472,12 +509,12 @@ mod memory { }) .await; - let semaphore = MEMORY_SEMAPHORE - .get_or_init(|| async { Semaphore::new(1) }) - .await; - let permit = semaphore.acquire().await.unwrap(); + let permit = acquire_runner_fifo_lock().await; // Memory executor uses heaptrack which uses BPF-based instrumentation, which conflicts with valgrind. + // + // Lock order: always after the FIFO permit. No other test takes both, + // so the two can never be wanted in opposite orders. let _lock = acquire_bpf_instrumentation_lock().await; (permit, _lock, MemoryExecutor) @@ -495,9 +532,9 @@ mod memory { async fn test_memory_executor(#[case] cmd: &str) { let (_permit, _lock, mut executor) = get_memory_executor().await; - // memtrack is a subcommand of this binary now, so the executor re-execs - // `current_exe` — which under `cargo test` is the test harness, not a - // CLI. Point it at the real binary, as the other executors' tests do. + // The executor re-execs `current_exe` to reach the memtrack subcommand, + // and under `cargo test` that is the test harness. Point it at the + // real binary. let self_exe = codspeed_binary_path().await; // Unset GITHUB_ACTIONS to force LocalProvider which supports repository_override temp_env::async_with_vars( From c87836c988694d8e51526dcc889667e22492a30f Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 09:30:41 -0400 Subject: [PATCH 22/27] test(local): fail loudly when a git setup command fails `create_git_repo_with_remote` ran five `git` commands with `.output().unwrap()`, which only unwraps the spawn: a git that runs and exits non-zero was ignored silently. A failing `git commit` then surfaced several frames later as an `UnbornBranch` error on `refs/heads/main`, which says nothing about the cause. Route all five through one helper that asserts on the exit status and prints the command, stdout and stderr on failure. No behaviour change when the commands succeed. Co-Authored-By: Claude Opus 5 (1M context) --- src/run_environment/local/provider.rs | 51 ++++++++++++++------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/run_environment/local/provider.rs b/src/run_environment/local/provider.rs index ab67f69b..7370e8e7 100644 --- a/src/run_environment/local/provider.rs +++ b/src/run_environment/local/provider.rs @@ -425,32 +425,35 @@ mod tests { assert_eq!(FAKE_COMMIT_REF.len(), 40); } - fn create_git_repo_with_remote(dir: &std::path::Path, remote_url: &str) -> String { - std::process::Command::new("git") - .args(["init", "-b", "main"]) - .current_dir(dir) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["config", "user.email", "test@test.com"]) - .current_dir(dir) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["config", "user.name", "Test"]) - .current_dir(dir) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["remote", "add", "origin", remote_url]) - .current_dir(dir) - .output() - .unwrap(); - std::process::Command::new("git") - .args(["commit", "--allow-empty", "-m", "init"]) + /// Run one `git` command in `dir`, failing loudly if it does not succeed. + /// + /// `.output().unwrap()` would only unwrap the *spawn*, letting a git that + /// runs and exits non-zero pass silently — a failed setup then surfaces + /// several frames later as an `UnbornBranch` error on `refs/heads/main`, + /// which says nothing about the cause. + fn git(dir: &std::path::Path, args: &[&str]) { + let output = std::process::Command::new("git") + .args(args) .current_dir(dir) .output() - .unwrap(); + .unwrap_or_else(|e| panic!("failed to spawn `git {}`: {e}", args.join(" "))); + + assert!( + output.status.success(), + "`git {}` failed with {}\nstdout: {}\nstderr: {}", + args.join(" "), + output.status, + String::from_utf8_lossy(&output.stdout).trim(), + String::from_utf8_lossy(&output.stderr).trim(), + ); + } + + fn create_git_repo_with_remote(dir: &std::path::Path, remote_url: &str) -> String { + git(dir, &["init", "-b", "main"]); + git(dir, &["config", "user.email", "test@test.com"]); + git(dir, &["config", "user.name", "Test"]); + git(dir, &["remote", "add", "origin", remote_url]); + git(dir, &["commit", "--allow-empty", "-m", "init"]); format!("{}/", dir.to_string_lossy()) } From 2ec7ceef6ca84810ba66ef01148683970319038e Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 09:56:33 -0400 Subject: [PATCH 23/27] ci: drop the two throwaway spike workflows Both said in their own header that they must not reach main. They have done their job: the COD-3218 check confirmed the preload removal on real x86_64 CI for gnu and static musl, and the COD-3440 check confirmed the musl build and the eBPF load on both architectures, last on the merged binary. Nothing they cover is unique to them any more -- `ci.yml` builds and tests the bundled binary on every PR. Refs COD-3218 Refs COD-3440 Co-Authored-By: Claude Opus 5 (1M context) --- .../workflows/cod-3218-exec-harness-check.yml | 441 ------------------ .github/workflows/cod-3440-musl-check.yml | 236 ---------- 2 files changed, 677 deletions(-) delete mode 100644 .github/workflows/cod-3218-exec-harness-check.yml delete mode 100644 .github/workflows/cod-3440-musl-check.yml diff --git a/.github/workflows/cod-3218-exec-harness-check.yml b/.github/workflows/cod-3218-exec-harness-check.yml deleted file mode 100644 index c958898f..00000000 --- a/.github/workflows/cod-3218-exec-harness-check.yml +++ /dev/null @@ -1,441 +0,0 @@ -# COD-3218 spike — throwaway workflow, NOT for merging. -# -# Purpose: close the one gap the local work could not. Removing the LD_PRELOAD -# hack moves the callgrind client requests from inside the benchmark child up -# into exec-harness, so the measurement now depends on valgrind propagating -# instrumentation state across fork/exec and on the spawn edges recorded by -# valgrind-codspeed. None of that is observable locally: this host is aarch64 -# Arch with no valgrind, and the CodSpeed valgrind .deb is published for Ubuntu -# only. It also runs the whole flow with a *musl* exec-harness, which is the -# COD-3440 half. -# -# Exit code 0 proves nothing here: the harness runs, valgrind runs, the -# benchmark completes, and the measurement can still be empty. So this workflow -# asserts on the CONTENT of the .out files, per the verification bar: -# - a part carries the benchmark URI, and lists `desc: Spawned pid: ` -# - every process in the spawn chain has its own .out with non-zero cost -# - the summed cost is printed, and the baseline job prints the same number -# from main so the step change can be quantified (breaking change #1) -# -# To use it: push this branch. The push itself runs the workflow -- see the -# `on:` block for why a push trigger is required rather than optional. After -# that first run has registered the file, it can also be re-run by hand: -# -# gh workflow run cod-3218-exec-harness-check.yml --ref spike/cod-3440-memtrack-musl -# -# The push trigger is scoped to the spike branch, so it cannot fire anywhere -# else. Delete the file once the question is answered -- it must not reach main. -# -# Deliberately does NOT touch ci.yml, release.yml, dist-workspace.toml or any -# Cargo.toml: it only adds jobs that run on this branch. -# -# --- updated for the single binary (COD-3440) --- -# -# exec-harness is no longer resolved off PATH: it is a subcommand of `codspeed`, -# reached by re-execing the running executable. So `cargo install --path -# crates/exec-harness` no longer influences what runs, and the sha256 tamper -# guard built on it guards nothing. Worse, it was the musl leg's only claim to -# testing musl -- the runner itself was always a gnu `cargo run`. The -# instrumentation job now builds the `codspeed` binary for the leg's libc and -# invokes it directly, which is both the real artifact and the only way the musl -# leg still means anything. - -name: COD-3218 exec-harness check - -on: - workflow_dispatch: - # The push trigger is what makes this workflow dispatchable at all, and it is - # not optional. A workflow_dispatch-only file that has never existed on the - # default branch is never registered by GitHub: `gh workflow run` answers - # "HTTP 404: workflow ... not found on the default branch" indefinitely, and - # it does NOT register itself over time -- retrying is useless. A push trigger - # forces registration, because GitHub runs the file on push and assigns it an - # id, after which `--ref` dispatch works too. The COD-3440 workflow next door - # was registered exactly this way; its first run is a `push` one from a commit - # that temporarily added this same trigger. - # - # Scoped to the spike branch so it cannot fire anywhere else, and it goes away - # when this throwaway file is deleted. - push: - branches: [spike/cod-3440-memtrack-musl] - -env: - MUSL_TARGET: x86_64-unknown-linux-musl - # Distinctive so it can be grepped out of the .out files unambiguously. The - # harness derives the URI as `exec_harness::`. - BENCH_NAME: cod3218_probe - # Passed as `sh -c "$BENCH_SCRIPT"`, so it holds no quoting of its own -- an - # env var cannot carry shell quotes through word splitting. - # A nested spawn on purpose: the harness forks `sh`, which forks `seq` and - # `wc`. That exercises the intermediate-forwarding case in the backend's - # spawn-chain walk, not just a single parent -> child edge. - BENCH_SCRIPT: seq 1 50000 | wc -l - -jobs: - # The COD-3440 half: the artifact the preload removal unblocks. - build-musl: - name: musl build is static - runs-on: ubuntu-latest # x86_64 - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - submodules: true - - uses: ./.github/actions/install-rust - with: - cache-key: cod3218-musl - - - name: Install musl toolchain - # musl-tools provides x86_64-linux-musl-gcc, which cc-rs finds unaided, - # so instrument-hooks' core.c compiles for the target. Without it the - # bindings build script now fails the build outright rather than - # silently compiling the noop implementation. - run: | - sudo apt-get install -y musl-tools linux-libc-dev - rustup target add "$MUSL_TARGET" - - # The released artifact is `codspeed`, with exec-harness inside it, so - # that is what has to be static. Building `-p exec-harness` alone would - # still pass while saying nothing about what ships. - - uses: ./.github/actions/install-bpf-deps - - name: Build - run: cargo build --bin codspeed --target "$MUSL_TARGET" - - - name: Verify the artifact is genuinely static - run: | - BIN=target/$MUSL_TARGET/debug/codspeed - file "$BIN" - ldd "$BIN" || true # expected: "not a dynamic executable" - readelf -d "$BIN" || true # expected: no dynamic section at all - echo "size: $(stat -c %s "$BIN") bytes" - # `if` rather than `grep ... && exit 1`, because a grep that matches - # nothing exits 1 and would fail the step under bash -e. - if readelf -d "$BIN" 2>/dev/null | grep -qE 'NEEDED|RPATH|RUNPATH'; then - echo "unexpected dynamic dependency or rpath" - exit 1 - fi - echo "OK: static, no NEEDED, no RPATH/RUNPATH" - - - name: Verify no preload artifact is produced any more - run: | - if find target -name 'libcodspeed_preload*' | grep .; then - echo "the preload library is still being built" - exit 1 - fi - echo "OK: no preload library in the build output" - - # The COD-3218 half: does the measurement actually land anywhere? - instrumentation: - name: instrumentation (${{ matrix.libc }}) - runs-on: ubuntu-latest # x86_64 - strategy: - fail-fast: false - matrix: - libc: [gnu, musl] - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - submodules: true - - uses: ./.github/actions/install-rust - with: - cache-key: cod3218-${{ matrix.libc }} - - - name: Install musl toolchain - if: matrix.libc == 'musl' - run: | - sudo apt-get install -y musl-tools linux-libc-dev - rustup target add "$MUSL_TARGET" - - # The root package links memtrack now, so building `codspeed` at all - # means building its vendored libbpf-sys. - - uses: ./.github/actions/install-bpf-deps - - # The binary under test is `codspeed` itself, because exec-harness is a - # subcommand of it. Building it for the leg's libc is what makes the musl - # leg a musl test: with `cargo run` it would be a gnu runner every time, - # whatever was installed on PATH. - - name: Build the codspeed binary under test - id: build - run: | - if [ "${{ matrix.libc }}" = "musl" ]; then - cargo build --bin codspeed --target "$MUSL_TARGET" - BIN=$PWD/target/$MUSL_TARGET/debug/codspeed - else - cargo build --bin codspeed - BIN=$PWD/target/debug/codspeed - fi - echo "bin=$BIN" >> "$GITHUB_OUTPUT" - file "$BIN" - # Proves the bundling as well as the build: this reports a version - # only if the exec-harness CLI really is linked into this executable. - "$BIN" exec-harness --version - - # Asserted through readelf and not a `file` string: rustc emits a - # static-PIE for x86_64 musl, which `file` calls "static-pie linked" - # rather than "statically linked" (aarch64 gets the non-PIE - # spelling), so matching that wording tests the wrong axis and fails - # on a perfectly static binary. What matters is that nothing is - # loaded at runtime. - if [ "${{ matrix.libc }}" = "musl" ]; then - if readelf -d "$BIN" 2>/dev/null | grep -qE 'NEEDED|RPATH|RUNPATH'; then - echo "the codspeed binary has a dynamic dependency" - exit 1 - fi - if readelf -lW "$BIN" 2>/dev/null | grep -q 'INTERP'; then - echo "the codspeed binary requests a dynamic loader" - exit 1 - fi - echo "OK: no NEEDED and no interpreter -- nothing is loaded at runtime" - fi - - - name: Run a simulation-mode benchmark - id: run - run: | - PROFILE_DIR=$RUNNER_TEMP/profile - mkdir -p "$PROFILE_DIR" - echo "profile_dir=$PROFILE_DIR" >> "$GITHUB_OUTPUT" - - # Invoked directly rather than through `cargo run`, so the process - # that re-execs itself into exec-harness is the binary just built and - # asserted on above. No tamper guard is needed any more: there is no - # PATH lookup left for a released build to win. - CODSPEED_LOG=debug "${{ steps.build.outputs.bin }}" exec \ - -m simulation \ - --skip-upload \ - --profile-folder "$PROFILE_DIR" \ - --name "$BENCH_NAME" \ - -- sh -c "$BENCH_SCRIPT" - - - name: Show what was produced - if: always() && steps.run.outputs.profile_dir != '' - run: | - PROFILE_DIR=${{ steps.run.outputs.profile_dir }} - echo "=== files ===" - find "$PROFILE_DIR" -type f -printf '%10s %p\n' | sort -k2 - echo - echo "=== headers of every callgrind out file ===" - # Printed in full and unfiltered on purpose: the exact header spelling - # of a client-request dump is what the backend's parser keys off, and - # eyeballing it here is cheaper than guessing at it from this repo. - find "$PROFILE_DIR" -name '*.out*' -type f | sort | while read -r f; do - echo "--- $f" - grep -nE '^(version|creator|pid|part|desc|cmd|events|summary|totals):' "$f" || true - echo - done - echo "=== valgrind logs ===" - find "$PROFILE_DIR" -name 'valgrind.*.log' -type f -exec tail -n 30 {} + || true - - - name: Verify the URI is attributed and every spawned process has cost - run: | - PROFILE_DIR=${{ steps.run.outputs.profile_dir }} - URI="exec_harness::$BENCH_NAME" - - fail() { echo "FAIL: $*"; exit 1; } - - # `totals:` only, NOT `summary:`. Child dumps carry both, with nearly - # equal values (a part's summary and the file's totals), so summing - # both silently doubles the reported cost -- and it doubles it only - # for some files, which made the branch/main comparison meaningless. - # awk rather than `grep | awk` because awk always exits 0, so a - # benchmark that recorded nothing reaches the explicit check below - # instead of aborting the step through pipefail with no diagnosis. - cost_of() { awk '/^totals:/ {s+=$2} END {print s+0}' "$@"; } - - # Spawned pids of the dump part that carries the URI. This is the - # invariant the backend actually walks: the URI and the spawn edge - # have to be on the SAME part, not merely in the same file, since - # attribution starts from the URI-bearing part and follows its edges. - spawns_of_uri_part() { - awk -v uri="$1" ' - /^part: / { if (hasuri && pids != "") { print pids; found=1; exit } - hasuri=0; pids=""; next } - index($0, "Client Request: " uri) { hasuri=1 } - /^desc: Spawned pid:/ { p=$0; sub(/.*Spawned pid:[[:space:]]*/,"",p) - pids = pids " " p } - END { if (!found && hasuri && pids != "") print pids } - ' "$2" - } - - mapfile -t OUTS < <(find "$PROFILE_DIR" -name '*.out*' -type f | sort) - echo "found ${#OUTS[@]} callgrind out file(s)" - [ "${#OUTS[@]}" -ge 2 ] || fail \ - "expected at least two out files (exec-harness plus the benchmark child), got ${#OUTS[@]}" - - # 1. Some part must carry the benchmark URI. Without this the cost - # exists but is anonymous, and the backend has nothing to attribute - # it to. Post-preload this is exec-harness's own dump: the children - # are NOT labelled any more, which is exactly why 2. and 3. matter. - mapfile -t URI_FILES < <(grep -lF -- "$URI" "${OUTS[@]}") - [ "${#URI_FILES[@]}" -ge 1 ] || fail "no out file mentions the benchmark URI '$URI'" - echo "OK: URI '$URI' found in: ${URI_FILES[*]}" - - # 2. The URI-bearing part must record the spawn edge to the child. - URI_FILE=${URI_FILES[0]} - read -r -a SPAWNED <<< "$(spawns_of_uri_part "$URI" "$URI_FILE")" - [ "${#SPAWNED[@]}" -ge 1 ] || fail \ - "the URI-bearing part of $URI_FILE records no 'Spawned pid:' edge, so the child's cost cannot be attributed to the benchmark" - echo "OK: the URI-bearing part of $URI_FILE spawned pid(s): ${SPAWNED[*]}" - - # 3. Walk the whole spawn chain. Each process must have its own out - # file with non-zero cost, and may itself have spawned more - # (here: exec-harness -> sh -> seq/wc). - declare -A SEEN=() - WORK=("${SPAWNED[@]}") - while [ "${#WORK[@]}" -gt 0 ]; do - PID=${WORK[0]} - WORK=("${WORK[@]:1}") - # `if` rather than `[ ... ] && continue`, which returns non-zero on - # the miss and would abort the step under bash -e. - if [ -n "${SEEN[$PID]:-}" ]; then - continue - fi - SEEN[$PID]=1 - - mapfile -t CHILD < <(find "$PROFILE_DIR" -name "$PID.out*" -type f) - [ "${#CHILD[@]}" -ge 1 ] || fail \ - "spawned pid $PID has no out file, so its cost was never recorded" - - COST=$(cost_of "${CHILD[@]}") - [ "$COST" -gt 0 ] || fail "spawned pid $PID recorded zero cost (file: ${CHILD[*]})" - echo "OK: pid $PID -> ${CHILD[*]} (Ir: $COST)" - - mapfile -t MORE < <(grep -hoiE 'Spawned pid:[[:space:]]*[0-9]+' "${CHILD[@]}" \ - | grep -oE '[0-9]+' | sort -u) - if [ "${#MORE[@]}" -gt 0 ]; then - WORK+=("${MORE[@]}") - fi - done - - echo - echo "PASS: URI attributed, ${#SEEN[@]} spawned process(es) all carry cost" - # Per-file breakdown, so the branch/main comparison can be read - # without digging through the headers above. - for f in "${OUTS[@]}"; do - printf ' %-14s Ir=%s\n' "$(basename "$f")" "$(cost_of "$f")" - done - echo "TOTAL_IR(${{ matrix.libc }})=$(cost_of "${OUTS[@]}")" - - # The baseline for breaking change #1: the same benchmark on main, where the - # preload starts the measured region inside the child instead. The delta - # between this number and the one above IS the step change in reported cost. - baseline-main: - name: baseline on main (preload) - runs-on: ubuntu-latest # x86_64 - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - ref: main - submodules: true - - uses: ./.github/actions/install-rust - with: - cache-key: cod3218-baseline - - - name: Install the exec-harness from main - run: | - cargo install --path crates/exec-harness --locked - exec-harness --version - - - name: Run the same benchmark - # main's exec-harness is the preload build, and its LD_PRELOAD check - # rejects statically linked executables -- so this leg is gnu only. - run: | - PROFILE_DIR=$RUNNER_TEMP/profile - mkdir -p "$PROFILE_DIR" - - CODSPEED_LOG=debug cargo run -- exec \ - -m simulation \ - --skip-upload \ - --profile-folder "$PROFILE_DIR" \ - --name "$BENCH_NAME" \ - -- sh -c "$BENCH_SCRIPT" - - find "$PROFILE_DIR" -type f -printf '%10s %p\n' | sort -k2 - find "$PROFILE_DIR" -name '*.out*' -type f | sort | while read -r f; do - echo "--- $f" - grep -nE '^(pid|part|desc|cmd|summary|totals):' "$f" || true - done - mapfile -t OUTS < <(find "$PROFILE_DIR" -name '*.out*' -type f | sort) - # `totals:` only, to match the instrumentation job -- see the comment - # on cost_of there. Summing `summary:` as well doubles the figure for - # some files and not others, which would make this comparison lie. - TOTAL=$(awk '/^totals:/ {s+=$2} END {print s+0}' "${OUTS[@]}") - for f in "${OUTS[@]}"; do - printf ' %-14s Ir=%s\n' "$(basename "$f")" \ - "$(awk '/^totals:/ {s+=$2} END {print s+0}' "$f")" - done - echo "TOTAL_IR(main-preload)=$TOTAL" - - # Quantifies breaking change #1 properly, which the single fixed-size probe - # above cannot: it reports one number (+26 %) that is dominated by fixed - # per-process startup and so says nothing about a real benchmark. - # - # The process shape is held IDENTICAL across sizes (exec-harness -> sh -> seq) - # and only the work varies, which makes the model falsifiable: if the shift - # really is a fixed per-process cost, then `branch - main` stays roughly - # CONSTANT in absolute Ir as N grows while the ratio collapses towards 1. If - # instead the delta grows with N, the cost is proportional and the whole - # "it only matters for tiny benchmarks" reading is wrong. - # - # Both variants run the same sizes through the same script, so the pairs are - # directly comparable. Delete this job once the number is recorded on the - # ticket. - cost-sweep: - name: cost sweep (${{ matrix.variant }}) - runs-on: ubuntu-latest # x86_64 - strategy: - fail-fast: false - matrix: - variant: [branch, main] - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - # github.sha rather than the branch name: the branch may have moved on - # by the time this runs, and the two variants must be pinned commits - # for the comparison to mean anything. - ref: ${{ matrix.variant == 'main' && 'main' || github.sha }} - submodules: true - - uses: ./.github/actions/install-rust - with: - cache-key: cod3218-sweep-${{ matrix.variant }} - - # Only the `main` variant still resolves exec-harness off PATH; on the - # branch it is bundled, so installing it there would do nothing. - - name: Install the exec-harness under test - if: matrix.variant == 'main' - run: | - cargo install --path crates/exec-harness --locked - exec-harness --version - - - uses: ./.github/actions/install-bpf-deps - if: matrix.variant != 'main' - - - name: Sweep - run: | - # Tamper guard, `main` variant only: there the runner can still swap - # in the released exec-harness and the sweep would measure the wrong - # binary. On the branch there is no PATH lookup left to tamper with. - if [ "${{ matrix.variant }}" = "main" ]; then - BEFORE=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) - fi - - for N in 1000 10000 100000 1000000; do - D=$RUNNER_TEMP/sweep-$N - mkdir -p "$D" - CODSPEED_LOG=warn cargo run -q -- exec \ - -m simulation \ - --skip-upload \ - --profile-folder "$D" \ - --name "sweep_$N" \ - -- sh -c "seq 1 $N > /dev/null" - - mapfile -t OUTS < <(find "$D" -name '*.out*' -type f | sort) - TOTAL=$(awk '/^totals:/ {s+=$2} END {print s+0}' "${OUTS[@]}") - echo "SWEEP variant=${{ matrix.variant }} N=$N files=${#OUTS[@]} TOTAL_IR=$TOTAL" - done - - if [ "${{ matrix.variant }}" = "main" ]; then - AFTER=$(sha256sum "$(which exec-harness)" | cut -d' ' -f1) - if [ "$BEFORE" != "$AFTER" ]; then - echo "the runner replaced exec-harness mid-sweep; results are not trustworthy" - exit 1 - fi - fi diff --git a/.github/workflows/cod-3440-musl-check.yml b/.github/workflows/cod-3440-musl-check.yml deleted file mode 100644 index de1d4fec..00000000 --- a/.github/workflows/cod-3440-musl-check.yml +++ /dev/null @@ -1,236 +0,0 @@ -# COD-3440 spike — throwaway workflow, NOT for merging. -# -# Purpose: prove the musl build on real kernels of both architectures. The dev -# host is aarch64, and an x86_64 build cross-compiled there cannot load its BPF -# skeleton against an aarch64 kernel, so x86_64 can only be closed here. -# -# To use it: push the spike branch, then -# -# gh workflow run cod-3440-musl-check.yml --ref spike/cod-3440-memtrack-musl -# -# Manual trigger only, so it never fires on its own. Note that the very first -# dispatch of a workflow that lives only on a non-default branch 404s until -# GitHub has registered it; once it has appeared in the Actions list, --ref -# dispatch works. Delete the file once the question is answered -- it must not -# reach main. -# -# Deliberately does NOT touch release.yml or dist-workspace.toml. -# -# --- what this workflow no longer carries --- -# -# It used to export `CFLAGS_` with the path to the `argp.h` stub and the -# `-idirafter` flags for Debian's kernel UAPI headers. The whole recipe now -# lives in `.cargo/config.toml`, so every `cargo` line below is a plain one: -# that is the point of running it, and a failure here means the checked-in -# config does not stand on its own. -# -# Two things make that possible, both of which cost a build to discover: -# -# * `CPATH` rather than `CFLAGS -I`. `[env]`'s `relative = true` can -# only make a *bare* path absolute, and there is nowhere in such a value to -# put an `-I`. `CPATH` takes bare directories, so it fits. It resolves -# against the project root — the directory holding `.cargo/`, not `.cargo/` -# itself, which the cargo reference words ambiguously. -# * The stub is therefore on the include path for the gnu build too, and -# defers to the real by testing `__GLIBC__`. Testing the include -# path with `__has_include_next` looks equivalent and is not: the same -# config puts `-idirafter /usr/include` on the *musl* build, which makes -# glibc's argp.h reachable from a musl compilation, and the build then dies -# on `__THROW`. -# -# The "compare against the gnu build" step below is what guards the second -# point: it builds gnu with the stub on CPATH, and only passes if the guard -# defers correctly. - -name: COD-3440 musl check - -on: - workflow_dispatch: - -jobs: - build: - name: build ${{ matrix.arch }} ${{ matrix.profile }} - runs-on: ${{ matrix.runner }} - strategy: - fail-fast: false - matrix: - profile: [dev, dist] - # `arch` has to be a real dimension, not something the `include` entries - # introduce on their own: an include entry whose keys are all new is - # merged into *every* combination, so the second one would overwrite the - # first and both jobs would end up aarch64. Listing it here makes each - # entry match on `arch` and fill in only its own combinations. - arch: [x86_64, aarch64] - include: - - arch: x86_64 - runner: ubuntu-latest - target: x86_64-unknown-linux-musl - - arch: aarch64 - runner: ubuntu-24.04-arm - target: aarch64-unknown-linux-musl - env: - TARGET: ${{ matrix.target }} - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - submodules: true - - uses: ./.github/actions/install-rust - with: - cache-key: musl-${{ matrix.arch }}-${{ matrix.profile }} - - uses: ./.github/actions/install-bpf-deps - - - name: Install musl toolchain - run: | - sudo apt-get install -y musl-tools pkg-config linux-libc-dev - rustup target add "$TARGET" - - # The root package is what ships now: it bundles memtrack and - # exec-harness as hidden subcommands, so building `-p memtrack` alone - # would no longer prove anything about the released artifact. - - name: Build - run: cargo build --bin codspeed --profile ${{ matrix.profile }} --target "$TARGET" - - - name: Verify the artifact is genuinely static - run: | - BIN=target/$TARGET/${{ matrix.profile == 'dev' && 'debug' || matrix.profile }}/codspeed - file "$BIN" - ldd "$BIN" || true # expected: "not a dynamic executable" - readelf -d "$BIN" || true # expected: no dynamic section at all - echo "size: $(stat -c %s "$BIN") bytes" - # Fail loudly if anything reintroduced a dynamic dependency or an rpath. - # `if` rather than `grep ... && exit 1`, because a grep that matches - # nothing exits 1 and would fail the step under bash -e. - if readelf -d "$BIN" 2>/dev/null | grep -qE 'NEEDED|RPATH|RUNPATH'; then - echo "unexpected dynamic dependency or rpath" - exit 1 - fi - echo "OK: static, no NEEDED, no RPATH/RUNPATH" - - # The stub directory is on CPATH for this build too, so a gnu build that - # still succeeds is the proof that the header's `__GLIBC__` guard defers - # to the real . If this fails on `__THROW`, that guard is what - # broke. - - name: Compare against the gnu build - if: matrix.profile == 'dist' - run: | - cargo build --bin codspeed --profile dist - echo "musl: $(stat -c %s target/$TARGET/dist/codspeed) bytes" - echo "gnu: $(stat -c %s target/dist/codspeed) bytes" - - - name: Smoke test the BPF path - run: | - BIN=$PWD/target/$TARGET/${{ matrix.profile == 'dev' && 'debug' || matrix.profile }}/codspeed - mkdir -p /tmp/memtrack-out - # Through the bundled subcommand, which is how the runner reaches it. - sudo env "RUST_LOG=info" "$BIN" memtrack track -o /tmp/memtrack-out "/bin/ls /tmp" - ls -la /tmp/memtrack-out - # A run that loads no probes still exits 0 but writes nothing. - test -n "$(ls -A /tmp/memtrack-out)" || { echo "no artifact written"; exit 1; } - - tests: - name: ${{ matrix.test }} (${{ matrix.arch }} musl) - runs-on: ${{ matrix.runner }} - strategy: - fail-fast: false - # Each memtrack integration test binary runs its cases serially (the eBPF - # tracker can't overlap with itself in one process), so shard by binary. - matrix: - test: [c_tests, cpp_tests, rust_tests, spawn_tests, dlopen_tests, rss_tests] - arch: [x86_64, aarch64] - include: - - arch: x86_64 - runner: ubuntu-latest - target: x86_64-unknown-linux-musl - - arch: aarch64 - runner: ubuntu-24.04-arm - target: aarch64-unknown-linux-musl - env: - TARGET: ${{ matrix.target }} - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - lfs: true - submodules: true - - uses: ./.github/actions/install-rust - with: - cache-key: musl-${{ matrix.arch }}-${{ matrix.test }} - - uses: ./.github/actions/install-bpf-deps - - - name: Install musl toolchain - run: | - sudo apt-get install -y musl-tools pkg-config linux-libc-dev - rustup target add "$TARGET" - - - name: Install additional allocators - run: sudo apt-get install -y libmimalloc-dev libjemalloc-dev - - # Built separately from the run because test-with's env(GITHUB_ACTIONS) - # gate is evaluated at COMPILE time. GITHUB_ACTIONS is set by the runner, - # so this is automatic here -- but if the tests are ever built outside - # Actions, the sudo-gated cases silently become #[ignore]d and the run - # reports a green "0 passed; N ignored". - - name: Build tests - run: cargo test -p memtrack --target "$TARGET" --no-run - - - name: Run tests - env: - RUST_LOG: debug - # Ubuntu 26.04 ships sudo-rs, which ignores `-E`; pass the env the - # rustup shims and the test gate need through `env` instead. The - # autoconf seeds no longer appear here -- they come from - # .cargo/config.toml, which cargo reads regardless of who invokes it. - run: | - sudo env \ - "HOME=$HOME" \ - "PATH=$PATH" \ - "CARGO_HOME=${CARGO_HOME:-$HOME/.cargo}" \ - "RUSTUP_HOME=${RUSTUP_HOME:-$HOME/.rustup}" \ - "CARGO_INCREMENTAL=$CARGO_INCREMENTAL" \ - "RUST_LOG=$RUST_LOG" \ - "GITHUB_ACTIONS=$GITHUB_ACTIONS" \ - "TARGET=$TARGET" \ - $(which cargo) test --target "$TARGET" --test ${{ matrix.test }} \ - -- --test-threads 1 --nocapture - working-directory: crates/memtrack - - # Since we ran the tests with sudo, the build artifacts will have root ownership - - name: Clean up - run: sudo chown -R $USER:$USER . ~/.cargo - - unit: - name: unit tests (${{ matrix.arch }} musl) - runs-on: ${{ matrix.runner }} - strategy: - fail-fast: false - matrix: - arch: [x86_64, aarch64] - include: - - arch: x86_64 - runner: ubuntu-latest - target: x86_64-unknown-linux-musl - - arch: aarch64 - runner: ubuntu-24.04-arm - target: aarch64-unknown-linux-musl - env: - TARGET: ${{ matrix.target }} - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - submodules: true - - uses: ./.github/actions/install-rust - with: - cache-key: musl-${{ matrix.arch }}-unit - - uses: ./.github/actions/install-bpf-deps - - - name: Install musl toolchain - run: | - sudo apt-get install -y musl-tools pkg-config linux-libc-dev - rustup target add "$TARGET" - - # `libc_allocator_symbols_resolve_to_offsets` used to be skipped here: it - # read /proc/self/maps of the TEST BINARY and required a mapped libc.so.6, - # which a statically linked musl binary does not have by construction. The - # test now resolves symbols in a spawned child instead, matching what the - # production path does, so the whole --lib suite must pass on musl. - - name: Run unit tests - run: cargo test -p memtrack --target "$TARGET" --lib From 069fbabf8a14336303376b4a0ee12b7016a4a136 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 10:32:42 -0400 Subject: [PATCH 24/27] docs(exec-harness,executor): cut two comments that narrate the change Both explain the diff rather than the code: a paragraph on what the `LD_PRELOAD` shared library used to do, and a paragraph weighing an alternative that was not taken. What a reader of these two functions needs stays -- that nothing is injected into the benchmarked executable, and the measured consequence of `--instr-atstart=no`. Kept out of the commits that introduced them because those sit below two merges of main, and folding would have flattened them. Refs COD-3440 Co-Authored-By: Claude Opus 5 (1M context) --- crates/exec-harness/src/analysis/mod.rs | 6 ++---- src/executor/config.rs | 21 +++++++++------------ 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/crates/exec-harness/src/analysis/mod.rs b/crates/exec-harness/src/analysis/mod.rs index 23d73657..21f91d1e 100644 --- a/crates/exec-harness/src/analysis/mod.rs +++ b/crates/exec-harness/src/analysis/mod.rs @@ -19,10 +19,8 @@ use std::process::Command; /// URI. The backend walks that edge to attribute the child's trace to the /// benchmark, so the measurement covers the whole spawned process tree. /// -/// This replaces the previous `LD_PRELOAD` shared library, which started -/// instrumentation from inside the benchmark process because the state did not -/// use to propagate across `fork`. Dropping it means statically linked -/// executables are now supported, since nothing has to be injected into them. +/// Nothing is injected into the benchmarked executable, so statically linked +/// ones work. pub fn perform(commands: Vec, mode: MeasurementMode) -> Result<()> { let hooks = InstrumentHooks::instance(INTEGRATION_NAME, INTEGRATION_VERSION); diff --git a/src/executor/config.rs b/src/executor/config.rs index f81302e6..bc2584de 100644 --- a/src/executor/config.rs +++ b/src/executor/config.rs @@ -193,22 +193,19 @@ impl OrchestratorConfig { /// Produce a per-execution [`ExecutorConfig`] for the given command and mode. /// - /// `uses_exec_harness` says whether this run is driven by exec-harness rather - /// than being a plain entrypoint command. Two things are derived from it: + /// `uses_exec_harness` says whether this run is driven by exec-harness + /// rather than being a plain entrypoint command. Two things follow from it: /// /// - Language-level wrappers (Node.js, Go) are injected into `PATH` only for /// entrypoint runs. - /// - Subprocess tracking is forced on for exec-harness runs. exec-harness - /// toggles instrumentation in its own process and then forks the benchmark, - /// so the benchmarked child is measured only if valgrind propagates that - /// state across `fork`/`exec` — which is what `--instr-atstart=inherit` - /// enables. Measured: with `--instr-atstart=no` the child dumps a single - /// zero-cost part and the benchmark reports nothing at all. + /// - Subprocess tracking is forced on for exec-harness runs, because + /// exec-harness toggles instrumentation in its own process and then forks + /// the benchmark. The child is measured only if valgrind propagates that + /// state across `fork`/`exec`, which `--instr-atstart=inherit` enables; + /// with `no` it dumps a single zero-cost part and reports nothing. /// - /// Deriving it here rather than making `--instr-atstart=inherit` - /// unconditional keeps entrypoint runs on their current behaviour. That - /// matters: an entrypoint benchmark that forks would otherwise start having - /// its children instrumented and counted, silently changing its numbers. + /// Entrypoint runs must keep the opposite default, or a benchmark that forks + /// would silently start counting its children. pub fn executor_config_for_command( &self, command: String, From 99827e64c2f09c7a01932fbb214ca43505dff827 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 11:03:10 -0400 Subject: [PATCH 25/27] fix(cli): stop honouring CODSPEED_SELF_EXE outside tests `self_exe()` resolves the binary that internal subcommands are re-invoked through, and the memory executor hands that same path to `sudo setcap +ep` so the capabilities land on the binary that is actually exec'd. Reading an environment variable there means anyone able to set one variable chooses which file receives CAP_SYS_ADMIN and CAP_BPF. The override exists for the tests, where `current_exe()` is the test harness and cannot dispatch a subcommand. Nothing in production sets it -- the doc comment justified it with a launcher scenario that has no caller. Putting it behind `cfg(test)`, constant included, removes the escalation path outright while keeping the tests working; a release build now always resolves `current_exe()`. Reported by Greptile on #531. Refs COD-3440 Co-Authored-By: Claude Opus 5 (1M context) --- src/cli/mod.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 85a2dd5d..631f698a 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -125,11 +125,14 @@ pub(crate) enum InternalCommands { Memtrack(memtrack::MemtrackArgs), } -/// Overrides the executable used to re-invoke internal subcommands. +/// Test-only override for the executable that internal subcommands are +/// re-invoked through. Under `cargo test` [`std::env::current_exe`] is the test +/// harness, which rejects their arguments. /// -/// [`std::env::current_exe`] is not always a binary that can dispatch them: it -/// resolves to the host executable when this crate is linked into one, and to -/// a wrapper when the CLI is invoked through a launcher script. +/// Deliberately `cfg(test)`: this path is handed to `sudo setcap +ep`, so +/// honouring it in a release build would let anyone who can set one environment +/// variable pick which file receives `CAP_SYS_ADMIN`. +#[cfg(test)] pub(crate) const SELF_EXE_ENV_VAR: &str = "CODSPEED_SELF_EXE"; /// The executable that internal subcommands are re-invoked through. @@ -139,11 +142,12 @@ pub(crate) const SELF_EXE_ENV_VAR: &str = "CODSPEED_SELF_EXE"; /// it, and `setcap` on a path that is not the one later exec'd succeeds while /// changing nothing. pub(crate) fn self_exe() -> Result { - match std::env::var_os(SELF_EXE_ENV_VAR) { - Some(path) => Ok(PathBuf::from(path)), - None => std::env::current_exe() - .context("failed to resolve current executable for internal subcommand"), + #[cfg(test)] + if let Some(path) = std::env::var_os(SELF_EXE_ENV_VAR) { + return Ok(PathBuf::from(path)); } + + std::env::current_exe().context("failed to resolve current executable for internal subcommand") } impl InternalCommands { From c862935158e4c05efd54d7c1f35d396bb976d213 Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 11:03:10 -0400 Subject: [PATCH 26/27] fix(cli): dispatch bundled subcommands before any runner setup `codspeed memtrack` and `codspeed exec-harness` are a re-exec of this binary and share nothing with the runner, but they were dispatched at the bottom of `run()` -- after the profile config is loaded, after the API client is built, and after `DiscoveredProjectConfig::discover_and_load` walks the filesystem. That last one is the problem: the re-exec runs in the benchmark's working directory, which is the user's project. A malformed `codspeed.yaml` there aborts the subcommand, so a measurement fails for a reason that has nothing to do with the measurement, and a `--config` given to the outer run is not forwarded to the inner one to override it. Move them into `run_internal`, called right after `Cli::parse()`. The logger match loses its internal arms for the same reason it had them. Reported by Greptile on #531. Refs COD-3440 Co-Authored-By: Claude Opus 5 (1M context) --- src/cli/mod.rs | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 631f698a..3ebaccd0 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -181,8 +181,28 @@ impl InternalCommands { } } +/// Dispatch a bundled subcommand. +/// +/// These are a re-exec of this binary and share nothing with the runner: no +/// profile, no project config, no API client, no logger. They also run in the +/// benchmark's working directory, so anything the runner discovers from the +/// filesystem could abort a measurement for a reason unrelated to it. +fn run_internal(command: InternalCommands) -> Result<()> { + match command { + InternalCommands::Samply(args) => samply::run(args), + InternalCommands::ExecHarness(args) => exec_harness::run(args), + #[cfg(target_os = "linux")] + InternalCommands::Memtrack(args) => memtrack::run(args), + } +} + pub async fn run() -> Result<()> { let cli = Cli::parse(); + + if let Commands::Internal(command) = cli.command { + return run_internal(command); + } + let codspeed_config = load_config(&cli)?; let mut api_client = build_api_client(&cli, &codspeed_config); @@ -200,14 +220,8 @@ pub async fn run() -> Result<()> { let setup_cache_dir = setup_cache_dir.as_deref(); match cli.command { - // These initialize their own logging. Bundled subcommands must not: - // a process has one global logger, and theirs would clash with ours. - Commands::Run(_) - | Commands::Exec(_) - | Commands::Internal(InternalCommands::Samply(_)) - | Commands::Internal(InternalCommands::ExecHarness(_)) => {} - #[cfg(target_os = "linux")] - Commands::Internal(InternalCommands::Memtrack(_)) => {} + // These initialize their own logging. + Commands::Run(_) | Commands::Exec(_) => {} _ => { init_local_logger()?; } @@ -259,10 +273,9 @@ pub async fn run() -> Result<()> { Commands::Use(args) => use_mode::run(args)?, Commands::Show => show::run()?, Commands::Update => update::run().await?, - Commands::Internal(InternalCommands::Samply(args)) => samply::run(args)?, - Commands::Internal(InternalCommands::ExecHarness(args)) => exec_harness::run(args)?, - #[cfg(target_os = "linux")] - Commands::Internal(InternalCommands::Memtrack(args)) => memtrack::run(args)?, + Commands::Internal(_) => { + unreachable!("internal subcommands are dispatched before runner setup") + } } Ok(()) } From 8e2d7e331ba88a44bbd5b5ec41df95a79eecc08b Mon Sep 17 00:00:00 2001 From: moha-bekh Date: Thu, 17 Sep 2026 11:03:10 -0400 Subject: [PATCH 27/27] ci: build both musl targets on every pull request The released Linux artifacts are `aarch64-unknown-linux-musl` and `x86_64-unknown-linux-musl`, and nothing in CI built either: a break in the argp stub, in the kernel-header paths or in the aarch64 `-lgcc` link flag would have surfaced for the first time during a tag-triggered release. The throwaway spike workflow used to cover this and was deleted with the spike. Both legs build on a native runner, with no environment variables, which is also what keeps `.cargo/config.toml` honest -- it has to carry the whole recipe on its own. The assertions are `readelf`-based rather than a `file` string, since rustc emits a static-PIE for x86_64 musl and spells it differently from aarch64, and `codspeed exec-harness --version` / `codspeed memtrack --version` answer only if both CLIs really are linked in. Reported by Greptile on #531. Refs COD-3440 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6120425b..1226106e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -148,6 +148,56 @@ jobs: mode: ${{ matrix.mode }} run: cargo codspeed run -p runner-shared + # The released Linux artifacts are musl, and nothing else here builds them, so + # a break in the argp stub, the kernel-header paths or the aarch64 `-lgcc` + # link flag would otherwise surface for the first time in a release. + musl-build: + runs-on: ${{ matrix.runner }} + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu-latest + target: x86_64-unknown-linux-musl + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: ${{ matrix.target }} + - uses: ./.github/actions/install-bpf-deps + - name: Install the musl toolchain + run: | + sudo apt-get install -y musl-tools linux-libc-dev + rustup target add "${{ matrix.target }}" + + # No environment variables: the whole recipe lives in `.cargo/config.toml`, + # and a plain build is what proves it still stands on its own. + - name: Build + run: cargo build --bin codspeed --target "${{ matrix.target }}" + + - name: Assert the artifact is static and carries both subcommands + run: | + BIN=target/${{ matrix.target }}/debug/codspeed + file "$BIN" + # Asserted through readelf rather than a `file` string: rustc emits a + # static-PIE for x86_64 musl, which `file` spells differently from the + # aarch64 one. What matters is that nothing is loaded at runtime. + if readelf -d "$BIN" 2>/dev/null | grep -qE 'NEEDED|RPATH|RUNPATH'; then + echo "the musl binary has a dynamic dependency" + exit 1 + fi + if readelf -lW "$BIN" 2>/dev/null | grep -q 'INTERP'; then + echo "the musl binary requests a dynamic loader" + exit 1 + fi + # These answer only if the two CLIs really are linked in. + "$BIN" exec-harness --version + "$BIN" memtrack --version + check: runs-on: ubuntu-latest if: always() @@ -158,6 +208,7 @@ jobs: - basic-run-test - macos-basic-run-test - bpf-tests + - musl-build - benchmarks steps: - uses: re-actors/alls-green@release/v1