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 000000000..4e4ba4a36 --- /dev/null +++ b/.github/workflows/cod-3218-exec-harness-check.yml @@ -0,0 +1,418 @@ +# 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. + +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 + 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. 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" + 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 + 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; } + + # `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 }} + + - 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 diff --git a/.github/workflows/cod-3440-musl-check.yml b/.github/workflows/cod-3440-musl-check.yml new file mode 100644 index 000000000..a0d6ec28d --- /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 diff --git a/Cargo.lock b/Cargo.lock index 20d199c15..16119c9ff 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 f73c631c8..277f0155a 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 bf65ef7e5..8988c463a 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 418af1430..000000000 --- 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 702f18d2a..000000000 --- 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 8bb4eaf44..23d736575 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 2d53804cb..000000000 --- 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 9a47591ce..f982f3954 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 30cb21b46..28e52d58a 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/crates/instrument-hooks-bindings/build.rs b/crates/instrument-hooks-bindings/build.rs index 63b46664e..eb6611e85 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." diff --git a/crates/memtrack/musl/argp.h b/crates/memtrack/musl/argp.h new file mode 100644 index 000000000..26a03a694 --- /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 */ diff --git a/src/executor/config.rs b/src/executor/config.rs index 39f958510..f81302e60 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 ca2dbdf4f..6d3646d8e 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)?;