Merge pull request #766 from RalfJung/sysroot

Sysroot consistency check

Author: RalfJung

README.md

@@ -250,9 +250,12 @@ Several `-Z` flags are relevant for Miri:
 
 Moreover, Miri recognizes some environment variables:
 
-* `MIRI_SYSROOT` (recognized by `miri`, `cargo miri` and the test suite)
-  indicates the sysroot to use.
-* `MIRI_TARGET` (recognized by the test suite) indicates which target
+* `MIRI_LOG`, `MIRI_BACKTRACE` control logging and backtrace printing during
+  Miri executions, also [see above][testing-miri].
+* `MIRI_SYSROOT` (recognized by `cargo miri` and the test suite)
+  indicates the sysroot to use.  To do the same thing with `miri`
+  directly, use the `--sysroot` flag.
+* `MIRI_TEST_TARGET` (recognized by the test suite) indicates which target
   architecture to test against.  `miri` and `cargo miri` accept the `--target`
   flag for the same purpose.
 

miri

@@ -55,7 +55,6 @@ build_sysroot() {
     cargo run $CARGO_BUILD_FLAGS --bin cargo-miri -- miri setup "$@"
     # Call again, to just set env var.
     eval $(cargo run $CARGO_BUILD_FLAGS -q --bin cargo-miri -- miri setup --env "$@")
-    export MIRI_SYSROOT
 }
 
 # Prepare and set MIRI_SYSROOT.  Respects `MIRI_TEST_TARGET` and takes into account
@@ -73,7 +72,7 @@ find_sysroot() {
             build_sysroot --target "$MIRI_TEST_TARGET"
         else
             # Assume we have a proper host libstd in $SYSROOT.
-            true
+            MIRI_SYSROOT="$SYSROOT"
         fi
     else
         # A normal toolchain.  We have to build a sysroot either way.
@@ -83,6 +82,7 @@ find_sysroot() {
             build_sysroot
         fi
     fi
+    export MIRI_SYSROOT
 }
 
 ## Main
@@ -140,7 +140,7 @@ run|run-debug)
     cargo build $CARGO_BUILD_FLAGS
     find_sysroot
     # Then run the actual command.
-    exec cargo run $CARGO_BUILD_FLAGS "$@"
+    exec cargo run $CARGO_BUILD_FLAGS -- --sysroot "$MIRI_SYSROOT" "$@"
     ;;
 *)
     echo "Unknown command: $COMMAND"

src/bin/cargo-miri.rs

@@ -119,6 +119,41 @@ fn list_targets() -> impl Iterator<Item=cargo_metadata::Target> {
     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.
+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");
+        assert!(out.status.success(), "Bad status code when getting sysroot info");
+        let sysroot = out.stdout.lines().nth(0)
+            .expect("didn't get at least one line for the sysroot").unwrap();
+        PathBuf::from(sysroot).canonicalize()
+            .expect("Failed to canonicalize sysroot")
+    }
+
+    let rustc_sysroot = get_sysroot(Command::new("rustc"));
+    let miri_sysroot = get_sysroot(Command::new(find_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 = Command::new("xargo").arg("--version").output().ok()?;
     if !out.status.success() {
@@ -265,11 +300,11 @@ path = "lib.rs"
         Some(target) => target == rustc_version::version_meta().unwrap().host,
     };
     let sysroot = if is_host { dir.join("HOST") } else { PathBuf::from(dir) };
-    std::env::set_var("MIRI_SYSROOT", &sysroot);
+    std::env::set_var("MIRI_SYSROOT", &sysroot); // pass the env var to the processes we spawn, which will turn it into "--sysroot" flags
     if print_env {
         println!("MIRI_SYSROOT={}", sysroot.display());
     } else if !ask_user {
-        println!("A libstd for Miri is now available in `{}`", sysroot.display());
+        println!("A libstd for Miri is now available in `{}`.", sysroot.display());
     }
 }
 
@@ -313,6 +348,9 @@ fn in_cargo_miri() {
     };
     let verbose = has_arg_flag("-v");
 
+    // Some basic sanity checks
+    test_sysroot_consistency();
+
     // We always setup.
     let ask = subcommand != MiriCommand::Setup;
     setup(ask);
@@ -385,38 +423,13 @@ fn in_cargo_miri() {
 }
 
 fn inside_cargo_rustc() {
-    let home = option_env!("RUSTUP_HOME").or(option_env!("MULTIRUST_HOME"));
-    let toolchain = option_env!("RUSTUP_TOOLCHAIN").or(option_env!("MULTIRUST_TOOLCHAIN"));
-    let sys_root = if let Ok(sysroot) = ::std::env::var("MIRI_SYSROOT") {
-        sysroot
-    } else if let (Some(home), Some(toolchain)) = (home, toolchain) {
-        format!("{}/toolchains/{}", home, toolchain)
-    } else {
-        option_env!("RUST_SYSROOT")
-            .map(|s| s.to_owned())
-            .or_else(|| {
-                Command::new("rustc")
-                    .arg("--print")
-                    .arg("sysroot")
-                    .output()
-                    .ok()
-                    .and_then(|out| String::from_utf8(out.stdout).ok())
-                    .map(|s| s.trim().to_owned())
-            })
-            .expect("need to specify `RUST_SYSROOT` env var during miri compilation, or use rustup or multirust")
-    };
+    let sysroot = std::env::var("MIRI_SYSROOT").expect("The wrapper should have set MIRI_SYSROOT");
 
-    // This conditional check for the `--sysroot` flag is there so that users can call `cargo-miri`
-    // directly without having to pass `--sysroot` or anything.
-    let rustc_args = std::env::args().skip(2);
-    let mut args: Vec<String> = if std::env::args().any(|s| s == "--sysroot") {
-        rustc_args.collect()
-    } else {
-        rustc_args
-            .chain(Some("--sysroot".to_owned()))
-            .chain(Some(sys_root))
-            .collect()
-    };
+    let rustc_args = std::env::args().skip(2); // skip `cargo rustc`
+    let mut args: Vec<String> = rustc_args
+        .chain(Some("--sysroot".to_owned()))
+        .chain(Some(sysroot))
+        .collect();
     args.splice(0..0, miri::miri_default_args().iter().map(ToString::to_string));
 
     // See if we can find the `cargo-miri` markers. Those only get added to the binary we want to
@@ -441,9 +454,7 @@ fn inside_cargo_rustc() {
     };
 
     let mut command = if needs_miri {
-        let mut path = std::env::current_exe().expect("current executable path invalid");
-        path.set_file_name("miri");
-        Command::new(path)
+        Command::new(find_miri())
     } else {
         Command::new("rustc")
     };

src/bin/miri.rs

@@ -100,11 +100,9 @@ fn init_late_loggers() {
     }
 }
 
-fn find_sysroot() -> String {
-    if let Ok(sysroot) = std::env::var("MIRI_SYSROOT") {
-        return sysroot;
-    }
-
+/// Returns the "default sysroot" that Miri will use if no `--sysroot` flag is set.
+/// Should be a compile-time constant.
+fn compile_time_sysroot() -> String {
     // Taken from PR <https://github.com/Manishearth/rust-clippy/pull/911>.
     let home = option_env!("RUSTUP_HOME").or(option_env!("MULTIRUST_HOME"));
     let toolchain = option_env!("RUSTUP_TOOLCHAIN").or(option_env!("MULTIRUST_TOOLCHAIN"));
@@ -167,12 +165,16 @@ fn main() {
         }
     }
 
-    // Determine sysroot and let rustc know about it.
-    let sysroot_flag = String::from("--sysroot");
+    // Determine sysroot.
+    let sysroot_flag = "--sysroot".to_string();
     if !rustc_args.contains(&sysroot_flag) {
+        // We need to *always* set a --sysroot, as the "default" rustc uses is
+        // somewhere in the directory miri was built in.
+        // If no --sysroot is given, fall back to env vars that are read at *compile-time*.
         rustc_args.push(sysroot_flag);
-        rustc_args.push(find_sysroot());
+        rustc_args.push(compile_time_sysroot());
     }
+
     // Finally, add the default flags all the way in the beginning, but after the binary name.
     rustc_args.splice(1..1, miri::miri_default_args().iter().map(ToString::to_string));
 

tests/compiletest.rs

@@ -25,7 +25,15 @@ fn rustc_lib_path() -> PathBuf {
     option_env!("RUSTC_LIB_PATH").unwrap().into()
 }
 
-fn mk_config(mode: &str) -> compiletest::common::ConfigWithTemp {
+fn run_tests(mode: &str, path: &str, target: &str, mut flags: Vec<String>) {
+    // Some flags we always want.
+    flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs
+    flags.push("--edition 2018".to_owned());
+    if let Ok(sysroot) = std::env::var("MIRI_SYSROOT") {
+        flags.push(format!("--sysroot {}", sysroot));
+    }
+
+    // The rest of the configuration.
     let mut config = compiletest::Config::default().tempdir();
     config.mode = mode.parse().expect("Invalid mode");
     config.rustc_path = miri_path();
@@ -35,7 +43,10 @@ fn mk_config(mode: &str) -> compiletest::common::ConfigWithTemp {
     }
     config.filter = env::args().nth(1);
     config.host = get_host();
-    config
+    config.src_base = PathBuf::from(path);
+    config.target = target.to_owned();
+    config.target_rustcflags = Some(flags.join(" "));
+    compiletest::run_tests(&config);
 }
 
 fn compile_fail(path: &str, target: &str, opt: bool) {
@@ -48,20 +59,14 @@ fn compile_fail(path: &str, target: &str, opt: bool) {
     ).green().bold());
 
     let mut flags = Vec::new();
-    flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs
-    flags.push("--edition 2018".to_owned());
     if opt {
         // Optimizing too aggressivley makes UB detection harder, but test at least
         // the default value.
         // FIXME: Opt level 3 ICEs during stack trace generation.
         flags.push("-Zmir-opt-level=1".to_owned());
     }
 
-    let mut config = mk_config("compile-fail");
-    config.src_base = PathBuf::from(path);
-    config.target = target.to_owned();
-    config.target_rustcflags = Some(flags.join(" "));
-    compiletest::run_tests(&config);
+    run_tests("compile-fail", path, target, flags);
 }
 
 fn miri_pass(path: &str, target: &str, opt: bool) {
@@ -74,17 +79,11 @@ fn miri_pass(path: &str, target: &str, opt: bool) {
     ).green().bold());
 
     let mut flags = Vec::new();
-    flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs
-    flags.push("--edition 2018".to_owned());
     if opt {
         flags.push("-Zmir-opt-level=3".to_owned());
     }
 
-    let mut config = mk_config("ui");
-    config.src_base = PathBuf::from(path);
-    config.target = target.to_owned();
-    config.target_rustcflags = Some(flags.join(" "));
-    compiletest::run_tests(&config);
+    run_tests("ui", path, target, flags);
 }
 
 fn get_host() -> String {