From 70f83a342c43da584a029d60f79547b175f28947 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 May 2020 11:45:43 +0200 Subject: [PATCH 1/5] re-do cargo-miri host/target detection logic to match rustbuild --- src/bin/cargo-miri.rs | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/bin/cargo-miri.rs b/src/bin/cargo-miri.rs index 4392cb93ddb4..0d73c7cf853a 100644 --- a/src/bin/cargo-miri.rs +++ b/src/bin/cargo-miri.rs @@ -462,6 +462,14 @@ fn in_cargo_miri() { } cmd.arg(arg); } + // We want to always run `cargo` with `--target`. This later helps us detect + // which crates are proc-macro/build-script (host crates) and which crates are + // needed for the program itself. + if get_arg_flag_value("--target").is_none() { + // When no `--target` is given, default to the host. + cmd.arg("--target"); + cmd.arg(rustc_version::version_meta().unwrap().host); + } // Serialize the remaining args into a special environemt variable. // This will be read by `inside_cargo_rustc` when we go to invoke @@ -491,24 +499,21 @@ fn in_cargo_miri() { } fn inside_cargo_rustc() { - /// 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. + /// Determines if we are being invoked (as rustc) to build a crate for + /// the "target" architecture, in contrast to the "host" architecture. + /// Host crates are for build scripts and proc macros and still need to + /// be built like normal; target crates need to be built for or interpreted + /// by Miri. /// - /// Currently, we detect this by checking for "--emit=link", - /// which indicates that Cargo instruced rustc to output - /// a native object. + /// Currently, we detect this by checking for "--target=", which flag is + /// never set for host crates. This matches what rustc bootstrap does, + /// which hopefully makes it "reliable enough". 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")) + get_arg_flag_value("--target").is_some() } /// 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') + /// the final, binary 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 { From e73fc97f0b9b7db87e977c3051699d959c603a6e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 May 2020 11:52:26 +0200 Subject: [PATCH 2/5] cargo-miri: honor RUSTC env var --- src/bin/cargo-miri.rs | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/bin/cargo-miri.rs b/src/bin/cargo-miri.rs index 0d73c7cf853a..44106955e0f4 100644 --- a/src/bin/cargo-miri.rs +++ b/src/bin/cargo-miri.rs @@ -6,6 +6,7 @@ use std::io::{self, BufRead, Write}; use std::ops::Not; use std::path::{Path, PathBuf}; use std::process::Command; +use std::ffi::OsString; const XARGO_MIN_VERSION: (u32, u32, u32) = (0, 3, 20); @@ -85,29 +86,23 @@ fn get_arg_flag_value(name: &str) -> Option { } } -/// Returns the path to the `miri` binary -fn find_miri() -> PathBuf { +/// Returns a command for the right `miri` binary. +fn miri() -> Command { let mut path = std::env::current_exe().expect("current executable path invalid"); path.set_file_name("miri"); - path + Command::new(path) } 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") - } + Command::new(env::var_os("CARGO").unwrap_or_else(|| OsString::from("cargo"))) } fn xargo_check() -> Command { - if let Ok(val) = std::env::var("XARGO_CHECK") { - // Bootstrap tells us where to find xargo - Command::new(val) - } else { - Command::new("xargo-check") - } + Command::new(env::var_os("XARGO_CHECK").unwrap_or_else(|| OsString::from("xargo-check"))) +} + +fn rustc() -> Command { + Command::new(env::var_os("RUSTC").unwrap_or_else(|| OsString::from("rustc"))) } fn list_targets() -> impl Iterator { @@ -188,8 +183,8 @@ fn test_sysroot_consistency() { return; } - let rustc_sysroot = get_sysroot(Command::new("rustc")); - let miri_sysroot = get_sysroot(Command::new(find_miri())); + let rustc_sysroot = get_sysroot(rustc()); + let miri_sysroot = get_sysroot(miri()); if rustc_sysroot != miri_sysroot { show_error(format!( @@ -301,7 +296,7 @@ fn setup(subcommand: MiriCommand) { Ok(val) => PathBuf::from(val), Err(_) => { // Check for `rust-src` rustup component. - let sysroot = Command::new("rustc") + let sysroot = rustc() .args(&["--print", "sysroot"]) .output() .expect("failed to get rustc sysroot") @@ -554,9 +549,9 @@ fn inside_cargo_rustc() { serde_json::from_str(&magic).expect("failed to deserialize MIRI_ARGS"); args.append(&mut user_args); // Run this in Miri. - Command::new(find_miri()) + miri() } else { - Command::new("rustc") + rustc() }; // Run it. From 1ba42b9f55b11a13a507534e9b832b4d754d6435 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 May 2020 11:53:24 +0200 Subject: [PATCH 3/5] Wording Co-authored-by: Oliver Scherer --- src/bin/cargo-miri.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/cargo-miri.rs b/src/bin/cargo-miri.rs index 44106955e0f4..8b238a7f7972 100644 --- a/src/bin/cargo-miri.rs +++ b/src/bin/cargo-miri.rs @@ -500,7 +500,7 @@ fn inside_cargo_rustc() { /// be built like normal; target crates need to be built for or interpreted /// by Miri. /// - /// Currently, we detect this by checking for "--target=", which flag is + /// Currently, we detect this by checking for "--target=", which is /// never set for host crates. This matches what rustc bootstrap does, /// which hopefully makes it "reliable enough". fn is_target_crate() -> bool { From 20097be2feaaa92c3a2843fb1c57c6a28d3dcf29 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 May 2020 11:54:35 +0200 Subject: [PATCH 4/5] more comment --- src/bin/cargo-miri.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bin/cargo-miri.rs b/src/bin/cargo-miri.rs index 8b238a7f7972..8bd9947092a7 100644 --- a/src/bin/cargo-miri.rs +++ b/src/bin/cargo-miri.rs @@ -502,7 +502,8 @@ fn inside_cargo_rustc() { /// /// Currently, we detect this by checking for "--target=", which is /// never set for host crates. This matches what rustc bootstrap does, - /// which hopefully makes it "reliable enough". + /// which hopefully makes it "reliable enough". This relies on us always + /// invoking cargo itself with `--target`, which `in_cargo_miri` ensures. fn is_target_crate() -> bool { get_arg_flag_value("--target").is_some() } From 024cc435f4e19e2d34f8e2099f8da1fb2bf1b952 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 May 2020 11:58:18 +0200 Subject: [PATCH 5/5] avoid env::var which requires valid UTF-8 --- src/bin/cargo-miri.rs | 15 +++++++++------ src/bin/miri.rs | 9 +++++---- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/bin/cargo-miri.rs b/src/bin/cargo-miri.rs index 8bd9947092a7..37ab41b317db 100644 --- a/src/bin/cargo-miri.rs +++ b/src/bin/cargo-miri.rs @@ -269,7 +269,7 @@ fn ask_to_run(mut cmd: Command, ask: bool, text: &str) { /// `MIRI_SYSROOT`. Skipped if `MIRI_SYSROOT` is already set, in which case we expect the user has /// done all this already. fn setup(subcommand: MiriCommand) { - if std::env::var("MIRI_SYSROOT").is_ok() { + if std::env::var_os("MIRI_SYSROOT").is_some() { if subcommand == MiriCommand::Setup { println!("WARNING: MIRI_SYSROOT already set, not doing anything.") } @@ -282,7 +282,7 @@ fn setup(subcommand: MiriCommand) { // First, we need xargo. if xargo_version().map_or(true, |v| v < XARGO_MIN_VERSION) { - if std::env::var("XARGO_CHECK").is_ok() { + if std::env::var_os("XARGO_CHECK").is_some() { // The user manually gave us a xargo binary; don't do anything automatically. show_error(format!("Your xargo is too old; please upgrade to the latest version")) } @@ -292,9 +292,9 @@ fn setup(subcommand: MiriCommand) { } // Determine where the rust sources are located. `XARGO_RUST_SRC` env var trumps everything. - let rust_src = match std::env::var("XARGO_RUST_SRC") { - Ok(val) => PathBuf::from(val), - Err(_) => { + let rust_src = match std::env::var_os("XARGO_RUST_SRC") { + Some(val) => PathBuf::from(val), + None => { // Check for `rust-src` rustup component. let sysroot = rustc() .args(&["--print", "sysroot"]) @@ -522,7 +522,7 @@ fn inside_cargo_rustc() { is_bin || is_test } - let verbose = std::env::var("MIRI_VERBOSE").is_ok(); + let verbose = std::env::var_os("MIRI_VERBOSE").is_some(); let target_crate = is_target_crate(); // Figure out which arguments we need to pass. @@ -531,6 +531,7 @@ fn inside_cargo_rustc() { // other args for target crates - that is, crates which are ultimately // going to get interpreted by Miri. if target_crate { + // FIXME: breaks for non-UTF-8 sysroots (use `var_os` instead). let sysroot = std::env::var("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT"); args.push("--sysroot".to_owned()); @@ -545,6 +546,8 @@ fn inside_cargo_rustc() { // we want to interpret under Miri. We deserialize the user-provided arguments // from the special environment variable "MIRI_ARGS", and feed them // to the 'miri' binary. + // + // `env::var` is okay here, well-formed JSON is always UTF-8. 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"); diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 06101fe24e2d..31f78aa9895a 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -61,7 +61,7 @@ fn init_early_loggers() { // If it is not set, we avoid initializing now so that we can initialize // later with our custom settings, and *not* log anything for what happens before // `miri` gets started. - if env::var("RUSTC_LOG").is_ok() { + if env::var_os("RUSTC_LOG").is_some() { rustc_driver::init_rustc_env_logger(); } } @@ -69,8 +69,9 @@ fn init_early_loggers() { fn init_late_loggers(tcx: TyCtxt<'_>) { // We initialize loggers right before we start evaluation. We overwrite the `RUSTC_LOG` // env var if it is not set, control it based on `MIRI_LOG`. + // (FIXE: use `var_os`, but then we need to manually concatenate instead of `format!`.) if let Ok(var) = env::var("MIRI_LOG") { - if env::var("RUSTC_LOG").is_err() { + if env::var_os("RUSTC_LOG").is_none() { // We try to be a bit clever here: if `MIRI_LOG` is just a single level // used for everything, we only apply it to the parts of rustc that are // CTFE-related. Otherwise, we use it verbatim for `RUSTC_LOG`. @@ -90,8 +91,8 @@ fn init_late_loggers(tcx: TyCtxt<'_>) { // If `MIRI_BACKTRACE` is set and `RUSTC_CTFE_BACKTRACE` is not, set `RUSTC_CTFE_BACKTRACE`. // Do this late, so we ideally only apply this to Miri's errors. - if let Ok(val) = env::var("MIRI_BACKTRACE") { - let ctfe_backtrace = match &*val { + if let Some(val) = env::var_os("MIRI_BACKTRACE") { + let ctfe_backtrace = match &*val.to_string_lossy() { "immediate" => CtfeBacktrace::Immediate, "0" => CtfeBacktrace::Disabled, _ => CtfeBacktrace::Capture,