diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000000000..bede22322 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,24 @@ +# Nothing here is `force = true`, so a shell exporting `CFLAGS` or `CPATH` wins +# and breaks the musl build. +# +# `libbpf-sys` vendors elfutils, whose `configure` looks for `argp`, `obstack` +# and `fts`; musl has none of them, so it stops before libelf even though a +# libelf-only build never calls them. Pre-seeding the cache skips the checks. +[env] +ac_cv_search_argp_parse = "none required" +ac_cv_search__obstack_free = "none required" +ac_cv_search_fts_close = "none required" + +# The `argp.h` stub. `CPATH` and not `CFLAGS -I`, because `relative = true` +# only works on a bare path. The stub defers to the real header under glibc. +CPATH = { value = "crates/memtrack/musl", relative = true } + +# Debian's musl-gcc runs with -nostdinc, so the kernel UAPI headers libbpf needs +# have to be added back, last so a glibc build is unaffected. Both triplets are +# listed because `[env]` cannot branch on the arch; a missing one is ignored. +CFLAGS = "-idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include/aarch64-linux-gnu -idirafter /usr/include" + +# rustc links with `-nodefaultlibs`; libbpf's C code needs the outline-atomic +# helpers from libgcc on aarch64. +[target.aarch64-unknown-linux-musl] +rustflags = ["-C", "link-arg=-lgcc"] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f8fe75eac..f5082edcc 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,14 +39,7 @@ 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 - run: cargo test --all --exclude memtrack --exclude exec-harness @@ -64,6 +60,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 +71,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 @@ -153,6 +146,50 @@ jobs: mode: ${{ matrix.mode }} run: cargo codspeed run -p runner-shared + # The released Linux artifacts are musl, and nothing else here builds them. + 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 }}" + + - 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" + # Not a `file` string: x86_64 musl is a static-PIE and `file` spells + # it differently from aarch64. + 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 + "$BIN" exec-harness --version + "$BIN" memtrack --version + check: runs-on: ubuntu-latest if: always() @@ -163,6 +200,7 @@ jobs: - basic-run-test - macos-basic-run-test - bpf-tests + - musl-build - benchmarks steps: - uses: re-actors/alls-green@release/v1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c47e5ace9..6bdf718a3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,50 +10,24 @@ 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, but only the main runner is released. The other crates +are 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, reached as `codspeed memtrack` +- **`exec-harness`**: Execution harness, reached as `codspeed exec-harness` - **`runner-shared`**: Shared library used by other crates -### Releasing Support Crates (memtrack, exec-harness, runner-shared) - -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. +`memtrack` and `exec-harness` keep a `version` in their `Cargo.toml` — 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 +58,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 diff --git a/Cargo.lock b/Cargo.lock index 69c918e5c..87b3f0b37 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/Cargo.toml b/Cargo.toml index 7709542c4..4b8d9a50c 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: `ebpf` carries the tracker the bundled `memtrack` +# subcommand needs, not just the IPC types. +memtrack = { path = "crates/memtrack" } ipc-channel = { workspace = true } [dev-dependencies] @@ -139,3 +141,17 @@ 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"] + +# memtrack's vendored libbpf/elfutils build runs as part of this package. +[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/crates/exec-harness/Cargo.toml b/crates/exec-harness/Cargo.toml index f73c631c8..db523d92f 100644 --- a/crates/exec-harness/Cargo.toml +++ b/crates/exec-harness/Cargo.toml @@ -20,10 +20,3 @@ 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..490eeef76 100644 --- a/crates/exec-harness/build.rs +++ b/crates/exec-harness/build.rs @@ -1,170 +1,11 @@ -//! 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; - -/// 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, -} +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..6eefdcf76 100644 --- a/crates/exec-harness/src/analysis/mod.rs +++ b/crates/exec-harness/src/analysis/mod.rs @@ -1,17 +1,19 @@ +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<()> { +/// Runs each benchmark command with the instrumentation toggled around its spawn. +/// +/// The toggles are in *this* process: the child inherits the state across +/// `fork`/`exec`, and callgrind records the spawn edge on the dump part that +/// [`InstrumentHooks::set_executed_benchmark`] names with the benchmark URI. +pub fn perform(commands: Vec, mode: MeasurementMode) -> Result<()> { let hooks = InstrumentHooks::instance(INTEGRATION_NAME, INTEGRATION_VERSION); for benchmark_cmd in commands { @@ -20,6 +22,13 @@ pub fn perform(commands: Vec) -> Result<()> { let mut cmd = Command::new(&benchmark_cmd.command[0]); cmd.args(&benchmark_cmd.command[1..]); + + if mode == MeasurementMode::Simulation { + // Perf maps, so the runner can resolve JIT-ed frames afterwards. + 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 +43,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/cli.rs b/crates/exec-harness/src/cli.rs new file mode 100644 index 000000000..8d3455479 --- /dev/null +++ b/crates/exec-harness/src/cli.rs @@ -0,0 +1,55 @@ +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; + + 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/constants.rs b/crates/exec-harness/src/constants.rs index 9a47591ce..3e152710b 100644 --- a/crates/exec-harness/src/constants.rs +++ b/crates/exec-harness/src/constants.rs @@ -1,12 +1,3 @@ -//! 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"); - /// 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..92bd0e79b 100644 --- a/crates/exec-harness/src/lib.rs +++ b/crates/exec-harness/src/lib.rs @@ -4,6 +4,7 @@ use serde::{Deserialize, Serialize}; use std::io::{self, BufRead}; pub mod analysis; +pub mod cli; pub mod constants; pub mod node; pub mod prelude; @@ -74,11 +75,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/exec-harness/src/main.rs b/crates/exec-harness/src/main.rs index 99cbf7cd2..a1ef670f9 100644 --- a/crates/exec-harness/src/main.rs +++ b/crates/exec-harness/src/main.rs @@ -1,33 +1,5 @@ -use clap::Parser; +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 +10,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/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/Cargo.toml b/crates/memtrack/Cargo.toml index d97ed4f79..84a5983f1 100644 --- a/crates/memtrack/Cargo.toml +++ b/crates/memtrack/Cargo.toml @@ -49,18 +49,3 @@ rstest = { workspace = true } 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 = "*" diff --git a/crates/memtrack/musl/argp.h b/crates/memtrack/musl/argp.h new file mode 100644 index 000000000..87dbd8fde --- /dev/null +++ b/crates/memtrack/musl/argp.h @@ -0,0 +1,60 @@ +/* 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. + + 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 + +#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 */ + +#endif /* __GLIBC__ */ diff --git a/crates/memtrack/src/cli.rs b/crates/memtrack/src/cli.rs new file mode 100644 index 000000000..1412d9f3f --- /dev/null +++ b/crates/memtrack/src/cli.rs @@ -0,0 +1,189 @@ +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. +/// +/// Returns the tracked command's exit code instead of calling +/// [`std::process::exit`], so the caller stays in charge of teardown. +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/ebpf/memtrack/mod.rs b/crates/memtrack/src/ebpf/memtrack/mod.rs index 2ec1f04f5..96c9bed29 100644 --- a/crates/memtrack/src/ebpf/memtrack/mod.rs +++ b/crates/memtrack/src/ebpf/memtrack/mod.rs @@ -285,16 +285,13 @@ mod tests { /// Allocator entry points must resolve to file offsets; a symbol that /// silently fails to resolve attaches nothing and loses all events. + /// + /// CI-only: the path is the Ubuntu one, and a static musl build of this + /// binary has no libc of its own to look at. + #[test_with::env(GITHUB_ACTIONS)] #[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 = format!("/lib/{}-linux-gnu/libc.so.6", std::env::consts::ARCH); let symbols = resolve_symbol_offsets(Path::new(&libc_path)).unwrap(); for symbol in ["malloc", "calloc", "realloc", "free"] { diff --git a/crates/memtrack/src/lib.rs b/crates/memtrack/src/lib.rs index ccd93399f..a8491dd2f 100644 --- a/crates/memtrack/src/lib.rs +++ b/crates/memtrack/src/lib.rs @@ -1,6 +1,8 @@ mod allocators; mod bpf_token; #[cfg(feature = "ebpf")] +pub mod cli; +#[cfg(feature = "ebpf")] mod ebpf; mod ipc; mod kernel; diff --git a/crates/memtrack/src/main.rs b/crates/memtrack/src/main.rs index 283cff194..0118cdc78 100644 --- a/crates/memtrack/src/main.rs +++ b/crates/memtrack/src/main.rs @@ -1,45 +1,5 @@ -use clap::Parser; -use ipc_channel::ipc; +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 +7,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); } diff --git a/src/binary_installer/mod.rs b/src/binary_installer/mod.rs deleted file mode 100644 index d8bdb75bf..000000000 --- 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 4d12e7de7..000000000 --- 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 2b2734a82..355d725e8 100644 --- a/src/binary_pins.rs +++ b/src/binary_pins.rs @@ -108,21 +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 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", @@ -135,10 +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, - ExecHarnessInstaller, MongoTracerInstaller, } @@ -146,8 +127,6 @@ impl PinnedBinary { pub fn url(&self) -> String { match self { PinnedBinary::ValgrindDeb(target) => target.url(), - PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.url(), - PinnedBinary::ExecHarnessInstaller => EXEC_HARNESS_INSTALLER.url(), PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.url(), } } @@ -155,8 +134,6 @@ impl PinnedBinary { pub fn sha256(&self) -> &'static str { match self { PinnedBinary::ValgrindDeb(target) => target.sha256(), - PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.sha256, - PinnedBinary::ExecHarnessInstaller => EXEC_HARNESS_INSTALLER.sha256, PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.sha256, } } @@ -168,11 +145,7 @@ mod tests { use crate::cli::run::helpers::download_pinned_file; use tempfile::NamedTempFile; - const INSTALLER_BINARIES: &[PinnedBinary] = &[ - PinnedBinary::MemtrackInstaller, - PinnedBinary::ExecHarnessInstaller, - PinnedBinary::MongoTracerInstaller, - ]; + const INSTALLER_BINARIES: &[PinnedBinary] = &[PinnedBinary::MongoTracerInstaller]; const ALL_VALGRIND_TARGETS: &[ValgrindTarget] = &[ ValgrindTarget { @@ -196,9 +169,7 @@ mod tests { fn assert_installer_variant_is_listed(binary: PinnedBinary) { match binary { PinnedBinary::ValgrindDeb(_) => {} - PinnedBinary::MemtrackInstaller - | PinnedBinary::ExecHarnessInstaller - | PinnedBinary::MongoTracerInstaller => { + PinnedBinary::MongoTracerInstaller => { assert!(INSTALLER_BINARIES.contains(&binary)); } } @@ -214,8 +185,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 d24c16b93..0d511a6cf 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 000000000..f8a4b4d2b --- /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/memtrack.rs b/src/cli/memtrack.rs new file mode 100644 index 000000000..686d7692e --- /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 2a9218ddc..6102dcb7c 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1,6 +1,9 @@ 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; @@ -108,39 +111,86 @@ enum Commands { #[derive(Subcommand, Debug)] pub(crate) enum InternalCommands { /// Run the bundled samply profiler. Args are forwarded to samply. - #[command(disable_help_flag = true, disable_help_subcommand = true)] + #[command(hide = true, disable_help_flag = true, disable_help_subcommand = true)] Samply(samply::SamplyArgs), + /// Run the bundled exec-harness. Args are forwarded to exec-harness. + #[command(hide = true, disable_help_flag = true, disable_help_subcommand = true)] + ExecHarness(exec_harness::ExecHarnessArgs), + /// Run the bundled memtrack. Args are forwarded to memtrack. + #[cfg(target_os = "linux")] + #[command(hide = true, disable_help_flag = true, disable_help_subcommand = true)] + Memtrack(memtrack::MemtrackArgs), } -/// Overrides the executable used to re-invoke internal subcommands. +/// Test-only override for the executable internal subcommands re-exec: under +/// `cargo test` [`std::env::current_exe`] is the test harness. /// -/// [`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. +/// `cfg(test)` because this path goes to `sudo setcap +ep`. +#[cfg(test)] pub(crate) const SELF_EXE_ENV_VAR: &str = "CODSPEED_SELF_EXE"; +/// The executable that internal subcommands are re-invoked through. +/// +/// The memory executor `setcap`s 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 { + #[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 { /// 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"); builder.args(args.args.iter().cloned()); } + InternalCommands::ExecHarness(args) => { + 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 as a single POSIX-shell command string, for the call + /// sites that splice it into a script rather than spawning it. + pub fn get_shell_command(&self) -> Result { + Ok(self.get_command_builder()?.as_command_line()) + } +} + +/// Dispatch a bundled subcommand, before any runner setup: these run in the +/// benchmark's working directory, where a stray `codspeed.yaml` would otherwise +/// abort the measurement. +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); @@ -158,7 +208,8 @@ 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 initialize their own logging. + Commands::Run(_) | Commands::Exec(_) => {} _ => { init_local_logger()?; } @@ -210,7 +261,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(_) => { + unreachable!("internal subcommands are dispatched before runner setup") + } } Ok(()) } diff --git a/src/executor/config.rs b/src/executor/config.rs index 39f958510..835a726f4 100644 --- a/src/executor/config.rs +++ b/src/executor/config.rs @@ -131,6 +131,9 @@ pub struct ExecutorConfig { /// Whether to enable language-level introspection (Node.js, Go wrappers in PATH). /// Disabled for exec-harness targets since they don't need it. pub enable_introspection: bool, + /// Whether this execution is driven by exec-harness rather than by a plain + /// entrypoint command. + pub uses_exec_harness: bool, /// Enable valgrind's --fair-sched option. pub fair_sched: bool, /// Enable valgrind's --cycle-estimation option. @@ -193,12 +196,13 @@ 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 by a plain entrypoint command. It gates the language-level wrappers + /// (Node.js, Go) in `PATH`, and valgrind's `--instr-atstart`. 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,7 +216,8 @@ 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, + uses_exec_harness, fair_sched: self.fair_sched, cycle_estimation: self.cycle_estimation, exclude_allocations: self.exclude_allocations, @@ -262,7 +267,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/memory/executor.rs b/src/executor/memory/executor.rs index 6cbb0f97f..1a7f9c7b2 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; @@ -32,7 +34,6 @@ use tokio::time::{Duration, timeout}; use super::setup::{ MEMTRACK_COMMAND, ensure_memtrack_capabilities, get_memtrack_status, has_memtrack_capabilities, - install_memtrack, }; pub struct MemoryExecutor; @@ -61,8 +62,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"); } @@ -146,7 +148,8 @@ impl Executor for MemoryExecutor { _system_info: &SystemInfo, _setup_cache_dir: Option<&Path>, ) -> Result<()> { - install_memtrack().await + // memtrack ships inside this binary, nothing to install. + Ok(()) } fn grant_privileges(&self) -> Result<()> { diff --git a/src/executor/memory/setup.rs b/src/executor/memory/setup.rs index e393e8b1b..ee8d30556 100644 --- a/src/executor/memory/setup.rs +++ b/src/executor/memory/setup.rs @@ -1,15 +1,13 @@ -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. +pub const MEMTRACK_COMMAND: &str = "memtrack"; const MEMTRACK_REQUIRED_CAPS: &[Capability] = &[ Capability::CAP_DAC_READ_SEARCH, @@ -28,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()) @@ -37,8 +35,11 @@ fn memtrack_setcap_spec() -> String { format!("{caps}+ep") } +/// The binary that carries the eBPF capabilities: memtrack is a subcommand of +/// this executable. Granted `+ep`, not inheritable, so a benchmark spawned from +/// here does not receive them. fn memtrack_path() -> Option { - which::which(MEMTRACK_COMMAND).ok() + self_exe().ok() } /// Whether the installed memtrack binary already carries the required capabilities. @@ -94,73 +95,11 @@ 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 }, - }; - } - } - + // memtrack ships inside this binary, so it is installed by construction. 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(), }, } } - -pub async fn install_memtrack() -> Result<()> { - ensure_binary_installed( - MEMTRACK_COMMAND, - MEMTRACK_CODSPEED_VERSION, - PinnedBinary::MemtrackInstaller, - ) - .await -} diff --git a/src/executor/orchestrator.rs b/src/executor/orchestrator.rs index ca2dbdf4f..cdf082120 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(" ")) @@ -143,7 +138,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)?; diff --git a/src/executor/tests.rs b/src/executor/tests.rs index 65507fcac..b70680e68 100644 --- a/src/executor/tests.rs +++ b/src/executor/tests.rs @@ -154,10 +154,7 @@ fi .await } - /// Path to the `exec-harness` binary, built on first use. - /// - /// Production runs install a pinned release and invoke it by name, which - /// would make the tests depend on what is installed on the machine. + /// Path to the standalone `exec-harness` binary, built on first use. pub async fn exec_harness_binary_path() -> &'static str { static BINARY: OnceCell = OnceCell::const_new(); @@ -445,7 +442,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>, @@ -457,10 +456,28 @@ 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 `current_exe`, the test harness here. + let self_exe = codspeed_binary_path().await; + temp_env::async_with_vars(&[(SELF_EXE_ENV_VAR, Some(self_exe))], async { + // `cargo test` hides the sudo prompt and then blocks on it + // forever. Capabilities are an xattr, so this is needed on + // every relink, not once. + 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(); + executor.grant_privileges().unwrap(); + }) + .await; }) .await; @@ -487,12 +504,17 @@ mod memory { async fn test_memory_executor(#[case] cmd: &str) { let (_permit, _lock, mut executor) = get_memory_executor().await; + // The executor re-execs `current_exe`, the test harness here. + 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 +524,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 +560,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 +598,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/executor/valgrind/measure.rs b/src/executor/valgrind/measure.rs index 62807b925..30159eadb 100644 --- a/src/executor/valgrind/measure.rs +++ b/src/executor/valgrind/measure.rs @@ -33,11 +33,18 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec