Auto merge of #1406 - RalfJung:miri-as-rustc, r=RalfJung

cargo-miri: never invoke rustc

Always go through 'MIRI_BE_RUSTC=1 miri' instead. This is based on @oli-obk's great idea to add a way to make Miri behave like rustc, which already helped us in https://github.com/rust-lang/miri/pull/1405. Now it means in `cargo-miri` we run *all* crates through the same binary, and use the env var to determine if we compile or interpret them. This makes sure the compiler is consistent.

The `rustc` binary of the current toolchain is now not used at all, only the `miri` binary is. In particular this means we can kill the sysroot consistency check. :)

README.md

@@ -156,7 +156,7 @@ Try deleting `~/.cache/miri`.
 This means the sysroot you are using was not compiled with Miri in mind.  This
 should never happen when you use `cargo miri` because that takes care of setting
 up the sysroot.  If you are using `miri` (the Miri driver) directly, see
-[below][testing-miri] for how to set up the sysroot.
+[CONTRIBUTING.md](CONTRIBUTING.md) for how to use `./miri`.
 
 
 ## Miri `-Z` flags and environment variables

src/bin/cargo-miri.rs

@@ -8,6 +8,8 @@
 use std::process::Command;
 use std::ffi::OsString;
 
+use rustc_version::VersionMeta;
+
 const XARGO_MIN_VERSION: (u32, u32, u32) = (0, 3, 20);
 
 const CARGO_MIRI_HELP: &str = r#"Interprets bin crates and tests in Miri
@@ -97,6 +99,10 @@ fn miri() -> Command {
     Command::new(find_miri())
 }
 
+fn version_info() -> VersionMeta {
+    VersionMeta::for_command(miri()).expect("failed to determine underlying rustc version of Miri")
+}
+
 fn cargo() -> Command {
     Command::new(env::var_os("CARGO").unwrap_or_else(|| OsString::from("cargo")))
 }
@@ -105,10 +111,6 @@ fn xargo_check() -> Command {
     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<Item = cargo_metadata::Target> {
     // We need to get the manifest, and then the metadata, to enumerate targets.
     let manifest_path =
@@ -153,55 +155,6 @@ fn list_targets() -> impl Iterator<Item = cargo_metadata::Target> {
     package.targets.into_iter()
 }
 
-/// 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.
-fn test_sysroot_consistency() {
-    fn get_sysroot(mut cmd: Command) -> PathBuf {
-        let out = cmd
-            .arg("--print")
-            .arg("sysroot")
-            .output()
-            .expect("Failed to run rustc to get sysroot info");
-        let stdout = String::from_utf8(out.stdout).expect("stdout is not valid UTF-8");
-        let stderr = String::from_utf8(out.stderr).expect("stderr is not valid UTF-8");
-        assert!(
-            out.status.success(),
-            "Bad status code {} when getting sysroot info via {:?}.\nstdout:\n{}\nstderr:\n{}",
-            out.status,
-            cmd,
-            stdout,
-            stderr,
-        );
-        let stdout = stdout.trim();
-        PathBuf::from(stdout)
-            .canonicalize()
-            .unwrap_or_else(|_| panic!("Failed to canonicalize sysroot: {}", stdout))
-    }
-
-    // Do not check sysroots if we got built as part of a Rust distribution.
-    // During `bootstrap`, the sysroot does not match anyway, and then some distros
-    // play symlink tricks so the sysroots may be different even for the final stage
-    // (see <https://github.com/mozilla/nixpkgs-mozilla/issues/198>).
-    if option_env!("RUSTC_STAGE").is_some() {
-        return;
-    }
-
-    let rustc_sysroot = get_sysroot(rustc());
-    let miri_sysroot = get_sysroot(miri());
-
-    if rustc_sysroot != miri_sysroot {
-        show_error(format!(
-            "miri was built for a different sysroot than the rustc in your current toolchain.\n\
-             Make sure you use the same toolchain to run miri that you used to build it!\n\
-             rustc sysroot: `{}`\n\
-             miri sysroot: `{}`",
-            rustc_sysroot.display(),
-            miri_sysroot.display()
-        ));
-    }
-}
-
 fn xargo_version() -> Option<(u32, u32, u32)> {
     let out = xargo_check().arg("--version").output().ok()?;
     if !out.status.success() {
@@ -300,10 +253,10 @@ fn setup(subcommand: MiriCommand) {
         Some(val) => PathBuf::from(val),
         None => {
             // Check for `rust-src` rustup component.
-            let sysroot = rustc()
+            let sysroot = miri()
                 .args(&["--print", "sysroot"])
                 .output()
-                .expect("failed to get rustc sysroot")
+                .expect("failed to determine sysroot")
                 .stdout;
             let sysroot = std::str::from_utf8(&sysroot).unwrap();
             let sysroot = Path::new(sysroot.trim_end_matches('\n'));
@@ -316,7 +269,7 @@ fn setup(subcommand: MiriCommand) {
                 ask_to_run(
                     cmd,
                     ask_user,
-                    "install the rustc-src component for the selected toolchain",
+                    "install the `rust-src` component for the selected toolchain",
                 );
             }
             rustup_src
@@ -368,7 +321,7 @@ fn setup(subcommand: MiriCommand) {
 
     // Determine architectures.
     // We always need to set a target so rustc bootstrap can tell apart host from target crates.
-    let host = rustc_version::version_meta().unwrap().host;
+    let host = version_info().host;
     let target = get_arg_flag_value("--target");
     let target = target.as_ref().unwrap_or(&host);
     // Now invoke xargo.
@@ -389,7 +342,6 @@ fn setup(subcommand: MiriCommand) {
         command.env("RUSTC", find_miri());
     }
     command.env("MIRI_BE_RUSTC", "1");
-    command.env("RUSTFLAGS", miri::miri_default_args().join(" "));
     // Finally run it!
     if command.status().expect("failed to run xargo").success().not() {
         show_error(format!("Failed to run xargo"));
@@ -424,9 +376,6 @@ fn in_cargo_miri() {
     };
     let verbose = has_arg_flag("-v");
 
-    // Some basic sanity checks
-    test_sysroot_consistency();
-
     // We always setup.
     setup(subcommand);
     if subcommand == MiriCommand::Setup {
@@ -478,7 +427,7 @@ fn in_cargo_miri() {
         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);
+            cmd.arg(version_info().host);
         }
 
         // Serialize the remaining args into a special environemt variable.
@@ -540,51 +489,46 @@ fn is_runnable_crate() -> bool {
     let verbose = std::env::var_os("MIRI_VERBOSE").is_some();
     let target_crate = is_target_crate();
 
-    // Figure out which arguments we need to pass.
-    let mut args: Vec<String> = 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.
+    let mut cmd = miri();
+    // Forward arguments.
+    cmd.args(std::env::args().skip(2)); // skip `cargo-miri rustc`
+
+    // We make sure to only specify our custom Xargo sysroot 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());
-        args.push(sysroot);
-        args.splice(0..0, miri::miri_default_args().iter().map(ToString::to_string));
+            env::var_os("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT");
+        cmd.arg("--sysroot");
+        cmd.arg(sysroot);
     }
 
-    // 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_ARGS", and feed them
-        // to the 'miri' binary.
+    // If this is a runnable target crate, we want Miri to start interpretation;
+    // otherwise we want Miri to behave like rustc and build the crate as usual.
+    if target_crate && is_runnable_crate() {
+        // This is the binary or test crate that we want to interpret under Miri.
+        // We deserialize the arguments that are meant for Miri 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<String> =
+        let miri_args: Vec<String> =
             serde_json::from_str(&magic).expect("failed to deserialize MIRI_ARGS");
-        args.append(&mut user_args);
-        // Run this in Miri.
-        miri()
+        cmd.args(miri_args);
     } else {
-        rustc()
+        // We want to compile, not interpret.
+        cmd.env("MIRI_BE_RUSTC", "1");
     };
 
     // Run it.
-    command.args(&args);
     if verbose {
-        eprintln!("+ {:?}", command);
+        eprintln!("+ {:?}", cmd);
     }
-
-    match command.status() {
+    match cmd.status() {
         Ok(exit) =>
             if !exit.success() {
                 std::process::exit(exit.code().unwrap_or(42));
             },
-        Err(e) => panic!("error running {:?}:\n{:?}", command, e),
+        Err(e) => panic!("error running {:?}:\n{:?}", cmd, e),
     }
 }
 
@@ -609,6 +553,6 @@ fn main() {
         // dependencies get dispatched to `rustc`, the final test/binary to `miri`.
         inside_cargo_rustc();
     } else {
-        show_error(format!("must be called with either `miri` or `rustc` as first argument."))
+        show_error(format!("`cargo-miri` must be called with either `miri` or `rustc` as first argument."))
     }
 }

src/bin/miri.rs

@@ -139,6 +139,10 @@ fn run_compiler(mut args: Vec<String>, callbacks: &mut (dyn rustc_driver::Callba
         }
     }
 
+    // Some options have different defaults in Miri than in plain rustc; apply those by making
+    // them the first arguments after the binary name (but later arguments can overwrite them).
+    args.splice(1..1, miri::miri_default_args().iter().map(ToString::to_string));
+
     // Invoke compiler, and handle return code.
     let result = rustc_driver::catch_fatal_errors(move || {
         rustc_driver::run_compiler(&args, callbacks, None, None)
@@ -182,10 +186,6 @@ fn main() {
         if rustc_args.is_empty() {
             // Very first arg: binary name.
             rustc_args.push(arg);
-            // After this, push Miri default args (before everything else so they can be overwritten).
-            for arg in miri::miri_default_args().iter() {
-                rustc_args.push(arg.to_string());
-            }
         } else if after_dashdash {
             // Everything that comes after `--` is forwarded to the interpreted crate.
             crate_args.push(arg);