diff --git a/src/executor/helpers/apt.rs b/src/executor/helpers/apt.rs index dfcf7adc9..04357c0bc 100644 --- a/src/executor/helpers/apt.rs +++ b/src/executor/helpers/apt.rs @@ -32,6 +32,8 @@ pub fn is_system_compatible(system_info: &SystemInfo) -> bool { /// 4. Run the install closure to install and get package names /// 5. Save installed packages to cache (if cache_dir provided) /// +/// Separate calls can share a cache directory: package files and metadata accumulate. +/// /// # Example /// /// ```rust,ignore @@ -203,9 +205,26 @@ fn save_to_cache(system_info: &SystemInfo, cache_dir: &Path, packages: &[&str]) bail!("Failed to save packages to cache"); } - // Create metadata file containing the installed packages + // Separate installs share the cache, so retain previously cached package names. let metadata_path = cache_dir.join(METADATA_FILENAME); - let metadata_content = packages.join("\n"); // TODO: add package versions as well, by using the output of the install command for example + let existing_metadata = match std::fs::read_to_string(&metadata_path) { + Ok(content) => content, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(error) => { + warn!( + "Failed to read cache metadata at {}: {error}", + metadata_path.display() + ); + return Ok(()); + } + }; + let mut cached_packages: Vec<&str> = existing_metadata + .lines() + .chain(packages.iter().copied()) + .collect(); + cached_packages.sort_unstable(); + cached_packages.dedup(); + let metadata_content = cached_packages.join("\n"); if let Ok(()) = std::fs::create_dir_all(metadata_path.parent().unwrap()) { if let Ok(()) = std::fs::write(&metadata_path, metadata_content) .context("Failed to write metadata file") diff --git a/src/executor/helpers/libc_debug_info.rs b/src/executor/helpers/libc_debug_info.rs new file mode 100644 index 000000000..f6d41f676 --- /dev/null +++ b/src/executor/helpers/libc_debug_info.rs @@ -0,0 +1,122 @@ +use super::apt; +use super::debug_file; +use super::run_with_sudo::can_elevate_without_prompt; +use crate::prelude::*; +use crate::system::SystemInfo; +use std::path::{Path, PathBuf}; + +/// Debian/Ubuntu package that ships the separate debug file of the system libc. +const LIBC_DEBUG_PACKAGE: &str = "libc6-dbg"; + +/// Path of the system libc, following the Debian multiarch layout. +fn system_libc_path(system_info: &SystemInfo) -> Option { + let triplet = match system_info.arch.as_str() { + "x86_64" => "x86_64-linux-gnu", + "aarch64" => "aarch64-linux-gnu", + arch => { + debug!("No known multiarch triplet for {arch}"); + return None; + } + }; + Some(PathBuf::from(format!("/lib/{triplet}/libc.so.6"))) +} + +/// Whether a separate debug file can be resolved for `binary`, through the same +/// build-id and `.gnu_debuglink` lookup that GDB and valgrind perform. +fn has_debug_symbols(binary: &Path) -> bool { + let data = match std::fs::read(binary) { + Ok(data) => data, + Err(e) => { + debug!("Failed to read {}: {e}", binary.display()); + return false; + } + }; + let object = match object::File::parse(data.as_slice()) { + Ok(object) => object, + Err(e) => { + debug!("Failed to parse {} as ELF: {e}", binary.display()); + return false; + } + }; + + match debug_file::find_debug_file(&object, binary) { + Some(debug_path) => { + debug!( + "Resolved debug file for {}: {}", + binary.display(), + debug_path.display() + ); + true + } + None => { + debug!("No debug file found for {}", binary.display()); + false + } + } +} + +/// Whether the system libc has the separate debug file `libc6-dbg` provides. +/// +/// Probed by file rather than through dpkg, because the setup cache restores package +/// files without touching dpkg's database. +fn has_libc_debug_symbols(system_info: &SystemInfo) -> bool { + system_libc_path(system_info).is_some_and(|libc| has_debug_symbols(&libc)) +} + +/// Warn when the system libc has no debug symbols: they sharpen the output but are not +/// needed to run. +fn warn_on_missing_libc_debug_symbols(system_info: &SystemInfo) { + if !apt::is_system_compatible(system_info) { + debug!("Skipping the libc debug symbol check on a non-apt-based system"); + return; + } + + if !has_libc_debug_symbols(system_info) { + warn!( + "Debug info for the system libc not found. Frames of the system libc can show up \ + as addresses in the flamegraph. Install {LIBC_DEBUG_PACKAGE} (Debian/Ubuntu) or \ + glibc-debuginfo (Fedora/RHEL) for more accurate results" + ); + } +} + +/// Install the libc debug symbols so frames of the system libc resolve to their real names. +/// +/// Best effort: they sharpen the output but are not needed to run, so a failure (no sudo, +/// no network) is a warning instead of an error. Non-apt systems are skipped; the warning +/// raised while saving the module artifacts still points those users to the package. +pub async fn ensure_libc_debug_symbols(system_info: &SystemInfo, setup_cache_dir: Option<&Path>) { + if !apt::is_system_compatible(system_info) { + debug!("Skipping the libc debug symbol install on a non-apt-based system"); + return; + } + + if has_libc_debug_symbols(system_info) { + debug!("The libc debug symbols are already installed"); + return; + } + + // An optional install must not stop to ask for a password, and a failing sudo logs an + // error. Only elevate when that works silently (root, or passwordless sudo). + if !can_elevate_without_prompt() { + debug!("Not installing {LIBC_DEBUG_PACKAGE}: it needs root or passwordless sudo"); + warn_on_missing_libc_debug_symbols(system_info); + return; + } + + let result = apt::install_cached( + system_info, + setup_cache_dir, + || has_libc_debug_symbols(system_info), + || async { + debug!("Installing {LIBC_DEBUG_PACKAGE}"); + apt::install(system_info, &[LIBC_DEBUG_PACKAGE])?; + Ok(vec![LIBC_DEBUG_PACKAGE.to_string()]) + }, + ) + .await; + + if let Err(error) = result { + warn!("Could not install {LIBC_DEBUG_PACKAGE}: {error}"); + } +} diff --git a/src/executor/helpers/mod.rs b/src/executor/helpers/mod.rs index 084302a1b..423db36f0 100644 --- a/src/executor/helpers/mod.rs +++ b/src/executor/helpers/mod.rs @@ -12,6 +12,7 @@ pub mod harvest_perf_maps_for_pids; pub mod homebrew; pub mod introspected_golang; pub mod introspected_nodejs; +pub mod libc_debug_info; pub mod linux_sysctl; pub mod profile_folder; pub mod run_command_with_log_pipe; diff --git a/src/executor/memory/executor.rs b/src/executor/memory/executor.rs index e599c056c..0e381f736 100644 --- a/src/executor/memory/executor.rs +++ b/src/executor/memory/executor.rs @@ -5,6 +5,7 @@ use crate::executor::ToolStatus; use crate::executor::helpers::command::CommandBuilder; use crate::executor::helpers::env::{build_path_env, get_base_injected_env}; use crate::executor::helpers::get_bench_command::get_bench_command; +use crate::executor::helpers::libc_debug_info::ensure_libc_debug_symbols; use crate::executor::helpers::run_command_with_log_pipe::run_command_with_log_pipe_and_callback; use crate::executor::helpers::run_with_env::prefix_command_with_env; use crate::executor::helpers::run_with_sudo::is_root_user; @@ -149,11 +150,15 @@ impl Executor for MemoryExecutor { async fn setup( &self, - _system_info: &SystemInfo, - _setup_cache_dir: Option<&Path>, + system_info: &SystemInfo, + setup_cache_dir: Option<&Path>, _run_environment: &RunEnvironment, ) -> Result<()> { - install_memtrack().await + install_memtrack().await?; + // FIXME(COD-2794): resolve libc symbols through debuginfod once it is back up, instead + // of installing libc6-dbg. + ensure_libc_debug_symbols(system_info, setup_cache_dir).await; + Ok(()) } fn grant_privileges(&self) -> Result<()> { diff --git a/src/executor/valgrind/executor.rs b/src/executor/valgrind/executor.rs index 303b55632..ccb36ec57 100644 --- a/src/executor/valgrind/executor.rs +++ b/src/executor/valgrind/executor.rs @@ -3,6 +3,7 @@ use std::path::Path; use crate::executor::Executor; use crate::executor::ToolStatus; +use crate::executor::helpers::libc_debug_info::ensure_libc_debug_symbols; use crate::executor::{ExecutionContext, ExecutorName, ExecutorSupport}; use crate::instruments::mongo_tracer::MongoTracer; use crate::prelude::*; @@ -62,6 +63,8 @@ impl Executor for ValgrindExecutor { } } + ensure_libc_debug_symbols(system_info, setup_cache_dir).await; + if let Err(error) = venv_compat::symlink_libpython(None) { warn!( "Failed to symlink libpython into the virtual environment, flamegraphs may contain artifacts: {error}" diff --git a/src/executor/valgrind/setup.rs b/src/executor/valgrind/setup.rs index 439392f95..a1cab1d8d 100644 --- a/src/executor/valgrind/setup.rs +++ b/src/executor/valgrind/setup.rs @@ -5,17 +5,12 @@ use crate::binary_pins::{ }; use crate::cli::run::helpers::download_pinned_file; use crate::executor::helpers::apt; -use crate::executor::helpers::debug_file; use crate::executor::{ToolInstallStatus, ToolStatus}; use crate::prelude::*; use crate::run_environment::RunEnvironment; use crate::system::{LinuxDistribution, SupportedOs, SystemInfo}; use semver::Version; -use std::{ - env, - path::{Path, PathBuf}, - process::Command, -}; +use std::{env, path::Path, process::Command}; fn get_codspeed_valgrind_target(system_info: &SystemInfo) -> Result { let SupportedOs::Linux(distro) = &system_info.os else { @@ -186,53 +181,6 @@ fn classify_valgrind_version(version: String) -> ToolInstallStatus { ToolInstallStatus::Installed { version } } -/// Path of the system libc, following the Debian multiarch layout. -fn system_libc_path(system_info: &SystemInfo) -> Option { - let triplet = match system_info.arch.as_str() { - "x86_64" => "x86_64-linux-gnu", - "aarch64" => "aarch64-linux-gnu", - arch => { - debug!("No known multiarch triplet for {arch}"); - return None; - } - }; - Some(PathBuf::from(format!("/lib/{triplet}/libc.so.6"))) -} - -/// Whether a separate debug file can be resolved for `binary`, through the same -/// build-id and `.gnu_debuglink` lookup that GDB and valgrind perform. -fn has_debug_symbols(binary: &Path) -> bool { - let data = match std::fs::read(binary) { - Ok(data) => data, - Err(e) => { - debug!("Failed to read {}: {e}", binary.display()); - return false; - } - }; - let object = match object::File::parse(data.as_slice()) { - Ok(object) => object, - Err(e) => { - debug!("Failed to parse {} as ELF: {e}", binary.display()); - return false; - } - }; - - match debug_file::find_debug_file(&object, binary) { - Some(debug_path) => { - debug!( - "Resolved debug file for {}: {}", - binary.display(), - debug_path.display() - ); - true - } - None => { - debug!("No debug file found for {}", binary.display()); - false - } - } -} - /// Whether a valgrind-codspeed recent enough for this runner is on `PATH`. fn is_valgrind_installed() -> bool { matches!( @@ -241,30 +189,6 @@ fn is_valgrind_installed() -> bool { ) } -/// Whether the system libc has the separate debug file `libc6-dbg` provides. -/// -/// Probed by file rather than through dpkg, because the setup cache restores package -/// files without touching dpkg's database. -fn has_libc_debug_symbols(system_info: &SystemInfo) -> bool { - system_libc_path(system_info).is_some_and(|libc| has_debug_symbols(&libc)) -} - -/// Warn when valgrind will run against a libc it has no debug symbols for: they sharpen -/// its output but are not needed to run it. -fn warn_on_missing_libc_debug_symbols(system_info: &SystemInfo) { - if !apt::is_system_compatible(system_info) { - debug!("Skipping the libc debug symbol check on a non-apt-based system"); - return; - } - - if !has_libc_debug_symbols(system_info) { - warn!( - "Debug info for the system libc not found. Install libc6-dbg (Debian/Ubuntu) \ - or glibc-debuginfo (Fedora/RHEL) for more accurate valgrind results" - ); - } -} - /// Provide valgrind on the systems we publish no package for: take the installation the /// user brings, or build one from source, and otherwise ask for a manual installation. pub(super) async fn try_install_from_source( @@ -276,7 +200,6 @@ pub(super) async fn try_install_from_source( "Using the valgrind installation already present on {}", system_info.os ); - warn_on_missing_libc_debug_symbols(system_info); return Ok(()); } @@ -293,7 +216,6 @@ pub(super) async fn try_install_from_source( match build_from_source::build_and_install(run_environment).await { Ok(()) if is_valgrind_installed() => { info!("valgrind-codspeed has been built and installed from source"); - warn_on_missing_libc_debug_symbols(system_info); return Ok(()); } Ok(()) => warn!("The freshly built valgrind is not usable, see the logs above"), @@ -316,21 +238,16 @@ pub(super) async fn install_valgrind_from_package( apt::install_cached( system_info, setup_cache_dir, - // The libc debug symbols are part of what this path installs, so a cache restore that - // brought back only valgrind must still count as incomplete. - || is_valgrind_installed() && has_libc_debug_symbols(system_info), + is_valgrind_installed, || async { debug!("Installing valgrind"); let binary = get_codspeed_valgrind_binary(system_info)?; let deb_path = env::temp_dir().join("valgrind-codspeed.deb"); download_pinned_file(binary, &deb_path).await?; - // Install libc debug symbols alongside valgrind, as GitHub runners - // do not include them by default. Keeping them in the same install - // call puts them under the same caching and idempotency logic. - apt::install(system_info, &[deb_path.to_str().unwrap(), "libc6-dbg"])?; + apt::install(system_info, &[deb_path.to_str().unwrap()])?; - // Return package names for caching + // Include libc6-dbg, which the Valgrind package installs as a dependency. Ok(vec!["valgrind".to_string(), "libc6-dbg".to_string()]) }, ) diff --git a/src/executor/wall_time/executor.rs b/src/executor/wall_time/executor.rs index 2cf4cc319..26c116c0d 100644 --- a/src/executor/wall_time/executor.rs +++ b/src/executor/wall_time/executor.rs @@ -10,6 +10,7 @@ use crate::executor::config::WalltimeProfiler; use crate::executor::helpers::command::CommandBuilder; use crate::executor::helpers::env::{build_path_env, get_base_injected_env}; use crate::executor::helpers::get_bench_command::get_bench_command; +use crate::executor::helpers::libc_debug_info::ensure_libc_debug_symbols; use crate::executor::helpers::linux_sysctl::{LinuxSysctl, ensure_linux_profiling_sysctls}; use crate::executor::helpers::run_command_with_log_pipe::run_command_with_log_pipe; use crate::executor::helpers::run_command_with_log_pipe::run_command_with_log_pipe_and_callback; @@ -122,6 +123,9 @@ impl Executor for WallTimeExecutor { }; profiler.setup(system_info, setup_cache_dir).await?; + // FIXME(COD-2794): resolve libc symbols through debuginfod once it is back up, instead + // of installing libc6-dbg. + ensure_libc_debug_symbols(system_info, setup_cache_dir).await; if self.profiling_sysctls.get().is_none() { let sysctls = ensure_linux_profiling_sysctls()?; self.profiling_sysctls