From 443163f93071587f72014018eeb45a2225fdff79 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 Feb 2020 14:17:19 +0100 Subject: [PATCH] refactor cargo-miri a bit --- src/bin/cargo-miri.rs | 168 +++++++++++++++++++++--------------------- 1 file changed, 85 insertions(+), 83 deletions(-) diff --git a/src/bin/cargo-miri.rs b/src/bin/cargo-miri.rs index 55503c635dba..17063b6ff67f 100644 --- a/src/bin/cargo-miri.rs +++ b/src/bin/cargo-miri.rs @@ -23,7 +23,7 @@ Common options: --features Features to compile for the package -V, --version Print version info and exit -Other [options] are the same as `cargo rustc`. Everything after the first "--" is +Other [options] are the same as `cargo check`. Everything after the first "--" is passed verbatim to Miri, which will pass everything after the second "--" verbatim to the interpreted program. "#; @@ -84,33 +84,30 @@ fn get_arg_flag_value(name: &str) -> Option { } } - -/// Determines if we are being invoked (as rustc) to build a runnable -/// executable. We run "cargo check", so this should only happen when -/// we are trying to compile a build script or build script dependency, -/// which actually needs to be executed on the host platform. -/// -/// Currently, we detect this by checking for "--emit=link", -/// which indicates that Cargo instruced rustc to output -/// a native object. -fn is_build_dep() -> bool { - std::env::args().any(|arg| arg.starts_with("--emit=") && arg.contains("link")) +/// Returns the path to the `miri` binary +fn find_miri() -> PathBuf { + let mut path = std::env::current_exe().expect("current executable path invalid"); + path.set_file_name("miri"); + path } -/// Returns whether or not Cargo invoked the wrapper (this binary) to compile -/// the final, target crate (either a test for 'cargo test', or a binary for 'cargo run') -/// Cargo does not give us this information directly, so we need to check -/// various command-line flags. -fn is_target_crate(is_build_script: bool) -> bool { - let is_bin = get_arg_flag_value("--crate-type").as_deref() == Some("bin"); - let is_test = std::env::args().find(|arg| arg == "--test").is_some(); - - // The final runnable (under Miri) crate will either be a binary crate - // or a test crate. We make sure to exclude build scripts here, since - // they are also build with "--crate-type bin" - (is_bin || is_test) && !is_build_script +fn cargo() -> Command { + if let Ok(val) = std::env::var("CARGO") { + // Bootstrap tells us where to find cargo + Command::new(val) + } else { + Command::new("cargo") + } } +fn xargo() -> Command { + if let Ok(val) = std::env::var("XARGO") { + // Bootstrap tells us where to find xargo + Command::new(val) + } else { + Command::new("xargo-check") + } +} fn list_targets() -> impl Iterator { // We need to get the manifest, and then the metadata, to enumerate targets. @@ -155,13 +152,6 @@ fn list_targets() -> impl Iterator { package.targets.into_iter() } -/// Returns the path to the `miri` binary -fn find_miri() -> PathBuf { - let mut path = std::env::current_exe().expect("current executable path invalid"); - path.set_file_name("miri"); - path -} - /// Make sure that the `miri` and `rustc` binary are from the same sysroot. /// This can be violated e.g. when miri is locally built and installed with a different /// toolchain than what is used when `cargo miri` is run. @@ -211,24 +201,6 @@ fn test_sysroot_consistency() { } } -fn cargo() -> Command { - if let Ok(val) = std::env::var("CARGO") { - // Bootstrap tells us where to find cargo - Command::new(val) - } else { - Command::new("cargo") - } -} - -fn xargo() -> Command { - if let Ok(val) = std::env::var("XARGO") { - // Bootstrap tells us where to find xargo - Command::new(val) - } else { - Command::new("xargo-check") - } -} - fn xargo_version() -> Option<(u32, u32, u32)> { let out = xargo().arg("--version").output().ok()?; if !out.status.success() { @@ -385,6 +357,7 @@ features = ["panic_unwind"] ) .unwrap(); // The boring bits: a dummy project for xargo. + // FIXME: With xargo-check, can we avoid doing this? File::create(dir.join("Cargo.toml")) .unwrap() .write_all( @@ -447,12 +420,12 @@ fn main() { } if let Some("miri") = std::env::args().nth(1).as_ref().map(AsRef::as_ref) { - // This arm is for when `cargo miri` is called. We call `cargo rustc` for each applicable target, + // This arm is for when `cargo miri` is called. We call `cargo check` for each applicable target, // but with the `RUSTC` env var set to the `cargo-miri` binary so that we come back in the other branch, // and dispatch the invocations to `rustc` and `miri`, respectively. in_cargo_miri(); } else if let Some("rustc") = std::env::args().nth(1).as_ref().map(AsRef::as_ref) { - // This arm is executed when `cargo-miri` runs `cargo rustc` with the `RUSTC_WRAPPER` env var set to itself: + // This arm is executed when `cargo-miri` runs `cargo check` with the `RUSTC_WRAPPER` env var set to itself: // dependencies get dispatched to `rustc`, the final test/binary to `miri`. inside_cargo_rustc(); } else { @@ -491,7 +464,7 @@ fn in_cargo_miri() { .kind .get(0) .expect("badly formatted cargo metadata: target::kind is an empty array"); - // Now we run `cargo rustc $FLAGS $ARGS`, giving the user the + // Now we run `cargo check $FLAGS $ARGS`, giving the user the // change to add additional arguments. `FLAGS` is set to identify // this target. The user gets to control what gets actually passed to Miri. let mut cmd = cargo(); @@ -515,7 +488,7 @@ fn in_cargo_miri() { // The remaining targets we do not even want to build. _ => continue, } - // Add user-defined args until first `--`. + // Forward user-defined `cargo` args until first `--`. while let Some(arg) = args.next() { if arg == "--" { break; @@ -523,17 +496,21 @@ fn in_cargo_miri() { cmd.arg(arg); } - // Serialize our actual args into a special environemt variable. + // Serialize the remaining args into a special environemt variable. // This will be read by `inside_cargo_rustc` when we go to invoke // our actual target crate (the binary or the test we are running). // Since we're using "cargo check", we have no other way of passing // these arguments. let args_vec: Vec = args.collect(); - cmd.env("MIRI_MAGIC_ARGS", serde_json::to_string(&args_vec).expect("failed to serialize args")); + cmd.env("MIRI_ARGS", serde_json::to_string(&args_vec).expect("failed to serialize args")); + // Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation, + // i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish + // the two codepaths. let path = std::env::current_exe().expect("current executable path invalid"); cmd.env("RUSTC_WRAPPER", path); if verbose { + cmd.env("MIRI_VERBOSE", ""); // this makes `inside_cargo_rustc` verbose. eprintln!("+ {:?}", cmd); } @@ -547,45 +524,71 @@ fn in_cargo_miri() { } fn inside_cargo_rustc() { - let sysroot = std::env::var("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT"); + /// Determines if we are being invoked (as rustc) to build a runnable + /// executable. We run "cargo check", so this should only happen when + /// we are trying to compile a build script or build script dependency, + /// which actually needs to be executed on the host platform. + /// + /// Currently, we detect this by checking for "--emit=link", + /// which indicates that Cargo instruced rustc to output + /// a native object. + fn is_target_crate() -> bool { + // `--emit` is sometimes missing, e.g. cargo calls rustc for "--print". + // That is definitely not a target crate. + // If `--emit` is present, then host crates are built ("--emit=link,...), + // while the rest is only checked. + get_arg_flag_value("--emit").map_or(false, |emit| !emit.contains("link")) + } - let rustc_args = std::env::args().skip(2); // skip `cargo rustc` + /// Returns whether or not Cargo invoked the wrapper (this binary) to compile + /// the final, target crate (either a test for 'cargo test', or a binary for 'cargo run') + /// Cargo does not give us this information directly, so we need to check + /// various command-line flags. + fn is_runnable_crate() -> bool { + let is_bin = get_arg_flag_value("--crate-type").as_deref() == Some("bin"); + let is_test = has_arg_flag("--test"); - let in_build_script = is_build_dep(); + // The final runnable (under Miri) crate will either be a binary crate + // or a test crate. We make sure to exclude build scripts here, since + // they are also build with "--crate-type bin" + is_bin || is_test + } - // Build scripts need to be compiled to actual runnable executables, - // and therefore completely bypass Miri. We make sure to only specify - // our custom Xargo sysroot for non-build-script crate - that is, - // crates which are ultimately going to get interpreted by Miri. - let mut args = if in_build_script { - rustc_args.collect() - } else { - let mut args: Vec = rustc_args - .chain(Some("--sysroot".to_owned())) - .chain(Some(sysroot)) - .collect(); + let verbose = std::env::var("MIRI_VERBOSE").is_ok(); + let target_crate = is_target_crate(); + + // Figure out which arguments we need to pass. + let mut args: Vec = std::env::args().skip(2).collect(); // skip `cargo-miri rustc` + // We make sure to only specify our custom Xargo sysroot and + // other args for target crates - that is, crates which are ultimately + // going to get interpreted by Miri. + if target_crate { + let sysroot = std::env::var("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT"); + args.push("--sysroot".to_owned()); + args.push(sysroot); args.splice(0..0, miri::miri_default_args().iter().map(ToString::to_string)); - args - }; + } - let needs_miri = - if is_target_crate(in_build_script) { - // This is the 'target crate '- the binary or test crate that + // Figure out the binary we need to call. If this is a runnable target crate, we want to call + // Miri to start interpretation; otherwise we want to call rustc to build the crate as usual. + let mut command = + if target_crate && is_runnable_crate() { + // This is the 'target crate' - the binary or test crate that // we want to interpret under Miri. We deserialize the user-provided arguments - // from the special environment variable "MIRI_MAGIC_ARGS", and feed them + // from the special environment variable "MIRI_ARGS", and feed them // to the 'miri' binary. - let magic = std::env::var("MIRI_MAGIC_ARGS").expect("missing MIRI_MAGIC_ARGS"); - let mut user_args: Vec = serde_json::from_str(&magic).expect("failed to deserialize args"); + let magic = std::env::var("MIRI_ARGS").expect("missing MIRI_ARGS"); + let mut user_args: Vec = serde_json::from_str(&magic).expect("failed to deserialize MIRI_ARGS"); args.append(&mut user_args); // Run this in Miri. - true + Command::new(find_miri()) } else { - false + Command::new("rustc") }; - let mut command = if needs_miri { Command::new(find_miri()) } else { Command::new("rustc") }; + // Run it. command.args(&args); - if has_arg_flag("-v") { + if verbose { eprintln!("+ {:?}", command); } @@ -594,7 +597,6 @@ fn inside_cargo_rustc() { if !exit.success() { std::process::exit(exit.code().unwrap_or(42)); }, - Err(ref e) if needs_miri => panic!("error during miri run: {:?}", e), - Err(ref e) => panic!("error during rustc call: {:?}", e), + Err(ref e) => panic!("error running {:?}:\n{:?}", command, e), } }