diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab26768..b93aa75 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,8 +23,10 @@ jobs: with: toolchain: stable components: rustfmt, clippy + - run: rustup toolchain install nightly # we need nightly for some tests which try to detect rustflags - run: cargo clippy --all-targets --all-features -- -D warnings - run: cargo fmt -- --check + - run: cargo test msrv: runs-on: ubuntu-latest diff --git a/Cargo.lock b/Cargo.lock index fa91551..f44b0b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -343,6 +343,12 @@ version = "0.8.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" +[[package]] +name = "current_platform" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a74858bcfe44b22016cb49337d7b6f04618c58e5dbfdef61b06b8c434324a0bc" + [[package]] name = "dashmap" version = "6.2.1" @@ -450,11 +456,13 @@ dependencies = [ "cargo_metadata", "clap", "clap_complete", + "current_platform", "indicatif", "inferno", "opener", "quick-xml 0.40.1", "rustc-demangle", + "serde_json", "shlex", "signal-hook", ] diff --git a/Cargo.toml b/Cargo.toml index 4e96246..13de0aa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,8 +29,12 @@ opener = "0.8.1" shlex = "2.0.1" rustc-demangle = { version = "0.1", features = ["std"] } +[dev-dependencies] +current_platform = "0.2" + [target.'cfg(unix)'.dependencies] signal-hook = "0.4.1" +serde_json = "1" [target.'cfg(windows)'.dependencies] blondie = "0.5.2" diff --git a/README.md b/README.md index 8efb9cb..6d334d3 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,9 @@ your cargo binary directory. On most systems this is something like `~/.cargo/bi ## Linux -**Note**: If you're using lld (which is the default since Rust 1.90.0) or mold on Linux, you must use the `--no-rosegment` flag. Otherwise perf will not be able to generate accurate stack traces ([explanation](https://crbug.com/919499#c16)). +**Note**: If you're using lld (which is the default since Rust 1.90.0) or mold on Linux, you may need to use the `--no-rosegment` flag. Otherwise perf will not be able to generate accurate stack traces ([explanation](https://crbug.com/919499#c16)). `cargo-flamegraph` tries to add this flag automatically but your setup may require extra configuration. + +
For example, Rust 1.90.0 and later: @@ -64,6 +66,8 @@ linker = "clang" rustflags = ["-Clink-arg=-fuse-ld=/usr/local/bin/mold", "-Clink-arg=-Wl,--no-rosegment"] ``` +
+ #### Debian (x86 and aarch) **Note**: Debian bullseye packages an outdated version of Rust which does not meet flamegraph's requirements. You should use [rustup](https://rustup.rs/) to install an up-to-date version of Rust, or upgrade to Debian bookworm or newer. diff --git a/src/bin/cargo-flamegraph.rs b/src/bin/cargo-flamegraph.rs index ccf8c9e..89f3e82 100644 --- a/src/bin/cargo-flamegraph.rs +++ b/src/bin/cargo-flamegraph.rs @@ -1,4 +1,7 @@ -use std::path::{Path, PathBuf}; +use std::{ + path::{Path, PathBuf}, + process::{Command, Output, Stdio}, +}; use anyhow::{anyhow, Context}; use cargo_metadata::{Artifact, ArtifactDebuginfo, Message, MetadataCommand, Package, TargetKind}; @@ -97,7 +100,6 @@ enum Cli { } fn build(opt: &Opt, kind: Vec) -> anyhow::Result> { - use std::process::{Command, Output, Stdio}; let mut cmd = Command::new("cargo"); // This will build benchmarks with the `bench` profile. This is needed @@ -187,6 +189,24 @@ fn build(opt: &Opt, kind: Vec) -> anyhow::Result> { println!("build command: {:?}", cmd); } + #[cfg(any(target_os = "linux", target_os = "android"))] + { + let rustflags_env_var = std::env::var("RUSTFLAGS").ok(); + if rosegment_detection::should_add_no_rosegment_flag( + "+nightly", + rustflags_env_var.as_deref(), + )? { + cmd.env( + "RUSTFLAGS", + format!( + "{} -C{}", + rustflags_env_var.as_ref().map_or("", String::as_str), + rosegment_detection::NO_ROSEGMENT_LINK_ARG + ), + ); + } + } + let Output { status, stdout, .. } = cmd .stderr(Stdio::inherit()) .output() @@ -205,6 +225,205 @@ fn build(opt: &Opt, kind: Vec) -> anyhow::Result> { .collect() } +#[cfg(any(target_os = "linux", target_os = "android"))] +mod rosegment_detection { + use super::*; + use cargo_metadata::semver; + + pub static NO_ROSEGMENT_LINK_ARG: &str = "link-arg=-Wl,--no-rosegment"; + + pub fn should_add_no_rosegment_flag( + toolchain_specifier: &'static str, + rustflags_env_var: Option<&str>, + ) -> anyhow::Result { + // `cargo metadata` doesn't provide this, so the `cargo_metadata` crate isn't a help here. + let cargo_version_stdout = Command::new("cargo") + .arg("version") + .stdout(Stdio::piped()) + .spawn() + // .spawn and .wait_with_output don't have distinct enough fail conditions for us to + // provide special error messages for each one + .and_then(|c| c.wait_with_output()) + .context("`cargo version` failed to run")? + .stdout; + + let cargo_version_utf8 = std::str::from_utf8(&cargo_version_stdout) + .context("`cargo version`'s output was not valid utf8")?; + + let cargo_version = cargo_version_utf8 + .split(' ') + .nth(1) + .ok_or_else(|| anyhow!("`cargo version` provided an answer in a format unlike the expected 'cargo ( )' (got {cargo_version_utf8:?})"))?; + + let cargo_semver = semver::Version::parse(cargo_version) + .context("cargo's version was not a valid semver")?; + + let at_least_1_90 = cargo_semver >= semver::Version::new(1, 90, 0); + let mut using_gold = false; + let mut specified_no_rosegment = false; + let mut using_linker_that_needs_flag = false; + + if let Some(flags) = rustflags_env_var { + detect_linker_settings( + flags.split(' '), + &mut using_gold, + &mut specified_no_rosegment, + &mut using_linker_that_needs_flag, + ); + } + + if let Some(rustflags) = get_rustc_target_from_printing_spec(toolchain_specifier)? + .and_then(|target| get_rustflags_from_cargo(&target, toolchain_specifier)) + { + detect_linker_settings( + rustflags.iter().map(String::as_str), + &mut using_gold, + &mut specified_no_rosegment, + &mut using_linker_that_needs_flag, + ); + } + + let should_add = ((at_least_1_90 && !using_gold) || using_linker_that_needs_flag) + && !specified_no_rosegment; + Ok(should_add) + } + + pub fn get_rustc_target_from_printing_spec( + toolchain_specifier: &'static str, + ) -> anyhow::Result> { + let Ok(rustc_print_target_output) = Command::new("rustc") + .args([ + toolchain_specifier, + "-Z", + "unstable-options", + "--print", + "target-spec-json", + ]) + .env("RUSTUP_AUTO_INSTALL", "0") + .stdout(Stdio::piped()) + .spawn() + .and_then(|c| c.wait_with_output()) + else { + // If this doesn't run correctly, that just means that they don't have nightly installed. + // And that's fine. + return Ok(None); + }; + + if !rustc_print_target_output.status.success() { + return Ok(None); + } + + let rustc_target_json = serde_json::from_slice::(&rustc_print_target_output.stdout) + .context("`rustc +nightly -Z unstable-options --print target-spec-json` provided non-json output despite exiting with an OK exit code")?; + + // It's an unstable feature, so it makes sense it wouldn't stay the same - we should + // probably warn here or smth if it changes, though, so that someone can report it to us. Or + // maybe so that CI can catch it. + Ok(rustc_target_json + .as_object() + .and_then(|obj| obj.get("llvm-target")) + .and_then(|llvm_target| llvm_target.as_str()) + .map(::to_string)) + } + + pub fn get_rustflags_from_cargo( + rustc_target: &str, + toolchain_specifier: &'static str, + ) -> Option> { + Command::new("cargo") + .args([ + toolchain_specifier, + "-Z", + "unstable-options", + "config", + "get", + ]) + .stdout(Stdio::piped()) + .env("RUSTUP_AUTO_INSTALL", "0") + .spawn() + .and_then(|c| c.wait_with_output()) + // If it exits with a non-zero code, that's fine 'cause it's not installed. + .ok() + .and_then(|output| { + get_rustflags_from_cargo_config_output(rustc_target, &output) + .map(|flags| flags.into_iter().map(::to_string).collect()) + }) + } + + pub fn get_rustflags_from_cargo_config_output<'a>( + rustc_target: &str, + cargo_config_output: &'a Output, + ) -> Option> { + // theoretically, we should be able to run nightly options with `cargo` if we can with + // `rustc`, but I guess we should be tolerant of if we can't. + if !cargo_config_output.status.success() { + return None; + } + + // it's nightly, it could change I guess. Really shouldn't be non-utf8 but we can't + // guarantee anything. + let cargo_opts_utf8 = std::str::from_utf8(&cargo_config_output.stdout).ok()?; + + // This command outputs a bunch of lines like: + // ``` + // profile.perf.debug = true + // profile.perf.inherits = "release" + // target.aarch64-unknown-linux-gnu.rustflags = ["-C", "linker=clang"] + // ``` + // So the lines after `target.{triple}.rustflags = ` should be valid json. + // Theoretically. I guess they can change the format at any point. + cargo_opts_utf8 + .lines() + .find_map(|l| { + // need to do splitn 'cause there shouldn't be spaces within the key but there will + // probably be spaces within the value, and we just want to get the stuff after the + // equals + let mut splits = l.splitn(3, ' '); + splits.next().and_then(|config_name| { + if config_name.starts_with("target.") + && config_name.contains(rustc_target) + && config_name.ends_with(".rustflags") + { + // nth(1) because we've already moved over the first one with the + // `.next()` + splits.nth(1) + } else { + None + } + }) + }) + // If it's not a json array, anymore, we don't want this to start throwing + // errors since it's not stabilized. + .and_then(|toml_json| serde_json::from_str::>(toml_json).ok()) + } + + pub fn detect_linker_settings<'a>( + flags: impl IntoIterator, + using_gold: &mut bool, + specified_no_rosegment: &mut bool, + using_linker_that_needs_flag: &mut bool, + ) { + for flag in flags { + if flag.starts_with("link-arg=-fuse-ld=") || flag.starts_with("-Clink-arg=-fuse-ld=") { + if !*using_gold { + *using_gold = flag.ends_with("/ld") || flag.ends_with("/gold"); + } + + if !*using_linker_that_needs_flag { + // does wild need this flag? are there other linkers we should include? + *using_linker_that_needs_flag = flag.ends_with("/wild") + || flag.ends_with("/lld") + || flag.ends_with("/mold"); + } + } + + if !*specified_no_rosegment { + *specified_no_rosegment = flag.ends_with(NO_ROSEGMENT_LINK_ARG); + } + } + } +} + fn workload(opt: &Opt, artifacts: &[Artifact]) -> anyhow::Result> { let mut trailing_arguments = opt.trailing_arguments.clone(); @@ -492,3 +711,126 @@ fn main() -> anyhow::Result<()> { let workload = workload(&opt, &artifacts)?; flamegraph::generate_flamegraph_for_workload(Workload::Command(workload), opt.graph) } + +#[cfg(all(test, any(target_os = "linux", target_os = "android")))] +mod tests { + use super::rosegment_detection::*; + use std::process::{ExitStatus, Output}; + + macro_rules! pass_if_not_ci { + () => {{ + if std::env::var("CI").is_err() { + println!("Silently passing CI-only test since it depends on specific environment details. If you'd like to run it, ensure that you're on a unix machine with the stable and nightly toolchains installed, no target-specific config settings in ~/.cargo/config.toml, and no special RUSTFLAGS set."); + return; + } + }} + } + + #[test] + fn linker_settings_detected() { + let mut using_gold = false; + let mut specified_no_rosegment = false; + let mut using_linker_that_needs_flag = false; + + detect_linker_settings( + ["-C", "linker=clang", "-Clink-arg=-fuse-ld=/usr/bin/wild"], + &mut using_gold, + &mut specified_no_rosegment, + &mut using_linker_that_needs_flag, + ); + + assert!(!using_gold); + assert!(!specified_no_rosegment); + assert!(using_linker_that_needs_flag); + + detect_linker_settings( + [ + "-Clinker=clang", + "-C", + "link-arg=-fuse-ld=/usr/bin/ld", + "-Clink-arg=-Wl,--no-rosegment", + ], + &mut using_gold, + &mut specified_no_rosegment, + &mut using_linker_that_needs_flag, + ); + + assert!(using_gold); + assert!(specified_no_rosegment); + assert!(using_linker_that_needs_flag); + } + + #[test] + fn ci_gets_target_correctly() { + pass_if_not_ci!(); + + let expected_target = current_platform::CURRENT_PLATFORM; + assert_eq!( + get_rustc_target_from_printing_spec("+nightly") + .unwrap() + .as_deref(), + Some(expected_target) + ); + } + + #[test] + fn ci_rustflags_is_empty() { + pass_if_not_ci!(); + + let target = current_platform::CURRENT_PLATFORM; + assert_eq!(get_rustflags_from_cargo(target, "+nightly"), None); + } + + #[test] + fn cargo_config_output_works_for_currently_nightly_format() { + let target = "x86_64-unknown-linux-gnu"; + let cargo_stdout = br#"profile.perf.debug = true +profile.perf.inherits = "release" +target.aarch64-unknown-linux-gnu.linker = "clang" +target.aarch64-unknown-linux-gnu.rustdocflags = ["-C", "linker=clang", "-C", "link-arg=-fuse-ld=/usr/bin/mold"] +target.aarch64-unknown-linux-gnu.rustflags = ["-C", "link-arg=-fuse-ld=/usr/bin/mold", "-Clink-arg=-Wl,--no-rosegment"] +target.wasm32-unknown-unknown.rustflags = ["-C", "target-feature=+bulk-memory"] +target.x86_64-pc-windows-gnu.linker = "x86_64-w64-mingw32-gcc" +target.x86_64-unknown-linux-gnu.linker = "clang" +target.x86_64-unknown-linux-gnu.rustdocflags = ["-C", "linker=clang", "-C", "link-arg=-fuse-ld=/usr/bin/wild"] +target.x86_64-unknown-linux-gnu.rustflags = ["-C", "link-arg=-fuse-ld=/usr/bin/wild"]"#; + + let output = Output { + stdout: cargo_stdout.to_vec(), + stderr: Vec::new(), + status: ExitStatus::default(), + }; + assert_eq!( + get_rustflags_from_cargo_config_output(target, &output), + Some(vec!["-C", "link-arg=-fuse-ld=/usr/bin/wild"]) + ); + } + + #[test] + fn ci_no_error_when_using_uninstalled_toolchain() { + pass_if_not_ci!(); + + assert!(should_add_no_rosegment_flag("+beta", None).unwrap()); + } + + #[test] + fn ci_rustflags_var_parsed() { + pass_if_not_ci!(); + + let should_add = should_add_no_rosegment_flag( + "+nightly", + Some("-Clinker=clang -C link-arg=-fuse-ld=/usr/bin/ld"), + ) + .unwrap(); + // We shouldn't add it because we're specifying the ld linker, which doesn't need it. + assert!(!should_add); + + let should_add = should_add_no_rosegment_flag( + "+nightly", + Some("-Clinker=clang -C link-arg=-fuse-ld=/usr/bin/mold -Clink-arg=-Wl,--no-rosegment"), + ) + .unwrap(); + // We shouldn't add it because we're already specifying it + assert!(!should_add); + } +}