set --sysroot outside the driver rather than messing with the arguments passed to the driver

Author: RalfJung

README.md

@@ -464,10 +464,9 @@ Moreover, Miri recognizes some environment variables:
   standard library that it will build and use for interpretation. This directory
   must point to the `library` subdirectory of a `rust-lang/rust` repository
   checkout.
-* `MIRI_SYSROOT` (recognized by `cargo miri` and the Miri driver) indicates the sysroot to use. When
+* `MIRI_SYSROOT` (recognized by `cargo miri` and the test suite) indicates the sysroot to use. When
   using `cargo miri`, this skips the automatic setup -- only set this if you do not want to use the
-  automatically created sysroot. For directly invoking the Miri driver, this variable (or a
-  `--sysroot` flag) is mandatory. When invoking `cargo miri setup`, this indicates where the sysroot
+  automatically created sysroot. When invoking `cargo miri setup`, this indicates where the sysroot
   will be put.
 * `MIRI_TEST_TARGET` (recognized by the test suite and the `./miri` script) indicates which target
   architecture to test against.  `miri` and `cargo miri` accept the `--target` flag for the same

cargo-miri/src/phases.rs

@@ -168,8 +168,6 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
     // Forward all further arguments (not consumed by `ArgSplitFlagValue`) to cargo.
     cmd.args(args);
 
-    // Let it know where the Miri sysroot lives.
-    cmd.env("MIRI_SYSROOT", miri_sysroot);
     // 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. (That extra argument is why we prefer this over setting `RUSTC`.)
@@ -204,6 +202,8 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
     // Set rustdoc to us as well, so we can run doctests.
     cmd.env("RUSTDOC", &cargo_miri_path);
 
+    // Forward some crucial information to our own re-invocations.
+    cmd.env("MIRI_SYSROOT", miri_sysroot);
     cmd.env("MIRI_LOCAL_CRATES", local_crates(&metadata));
     if verbose > 0 {
         cmd.env("MIRI_VERBOSE", verbose.to_string()); // This makes the other phases verbose.
@@ -393,6 +393,8 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
     // Arguments are treated very differently depending on whether this crate is
     // for interpretation by Miri, or for use by a build script / proc macro.
     if !info_query && target_crate {
+        // Set the sysroot.
+        cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap());
         // Forward arguments, but remove "link" from "--emit" to make this a check-only build.
         let emit_flag = "--emit";
         while let Some(arg) = args.next() {
@@ -531,6 +533,12 @@ pub fn phase_runner(mut binary_args: impl Iterator<Item = String>, phase: Runner
         cmd.env(name, val);
     }
 
+    if phase != RunnerPhase::Rustdoc {
+        // Set the sysroot. Not necessary in rustdoc, where we already set the sysroot when invoking
+        // rustdoc itself, which will forward that flag when invoking rustc (i.e., us), so the flag
+        // is present in `info.args`.
+        cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap());
+    }
     // Forward rustc arguments.
     // We need to patch "--extern" filenames because we forced a check-only
     // build without cargo knowing about that: replace `.rlib` suffix by

miri-script/src/commands.rs

@@ -2,6 +2,7 @@ use std::env;
 use std::ffi::OsString;
 use std::io::Write;
 use std::ops::Not;
+use std::path::PathBuf;
 use std::process;
 use std::thread;
 use std::time;
@@ -20,10 +21,11 @@ const JOSH_FILTER: &str =
 const JOSH_PORT: &str = "42042";
 
 impl MiriEnv {
-    fn build_miri_sysroot(&mut self, quiet: bool) -> Result<()> {
-        if self.sh.var("MIRI_SYSROOT").is_ok() {
+    /// Returns the location of the sysroot.
+    fn build_miri_sysroot(&mut self, quiet: bool) -> Result<PathBuf> {
+        if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") {
             // Sysroot already set, use that.
-            return Ok(());
+            return Ok(miri_sysroot.into());
         }
         let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml");
         let Self { toolchain, cargo_extra_flags, .. } = &self;
@@ -57,8 +59,8 @@ impl MiriEnv {
             .with_context(|| "`cargo miri setup` failed")?;
             panic!("`cargo miri setup` didn't fail again the 2nd time?");
         };
-        self.sh.set_var("MIRI_SYSROOT", output);
-        Ok(())
+        self.sh.set_var("MIRI_SYSROOT", &output);
+        Ok(output.into())
     }
 }
 
@@ -502,7 +504,7 @@ impl Command {
             flags.iter().take_while(|arg| *arg != "--").any(|arg| *arg == "--edition");
 
         // Prepare a sysroot.
-        e.build_miri_sysroot(/* quiet */ true)?;
+        let miri_sysroot = e.build_miri_sysroot(/* quiet */ true)?;
 
         // Then run the actual command.
         let miri_manifest = path!(e.miri_dir / "Cargo.toml");
@@ -514,12 +516,12 @@ impl Command {
         if dep {
             cmd!(
                 e.sh,
-                "cargo +{toolchain} --quiet test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode {miri_flags...} {edition_flags...} {flags...}"
+                "cargo +{toolchain} --quiet test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode --sysroot {miri_sysroot} {miri_flags...} {edition_flags...} {flags...}"
             ).quiet().run()?;
         } else {
             cmd!(
                 e.sh,
-                "cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {edition_flags...} {flags...}"
+                "cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- --sysroot {miri_sysroot} {miri_flags...} {edition_flags...} {flags...}"
             ).quiet().run()?;
         }
         Ok(())

src/bin/miri.rs

@@ -271,25 +271,6 @@ fn run_compiler(
     callbacks: &mut (dyn rustc_driver::Callbacks + Send),
     using_internal_features: std::sync::Arc<std::sync::atomic::AtomicBool>,
 ) -> ! {
-    if target_crate {
-        // Miri needs a custom sysroot for target crates.
-        // If no `--sysroot` is given, the `MIRI_SYSROOT` env var is consulted to find where
-        // that sysroot lives, and that is passed to rustc.
-        let sysroot_flag = "--sysroot";
-        if !args.iter().any(|e| e == sysroot_flag) {
-            // Using the built-in default here would be plain wrong, so we *require*
-            // the env var to make sure things make sense.
-            let miri_sysroot = env::var("MIRI_SYSROOT").unwrap_or_else(|_| {
-                show_error!(
-                    "Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set"
-                    )
-            });
-
-            args.push(sysroot_flag.to_owned());
-            args.push(miri_sysroot);
-        }
-    }
-
     // Don't insert `MIRI_DEFAULT_ARGS`, in particular, `--cfg=miri`, if we are building
     // a "host" crate. That may cause procedural macros (and probably build scripts) to
     // depend on Miri-only symbols, such as `miri_resolve_frame`:

tests/ui.rs

@@ -60,6 +60,13 @@ fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) ->
     program.program = miri_path();
 
     // Add some flags we always want.
+    program.args.push(
+        format!(
+            "--sysroot={}",
+            env::var("MIRI_SYSROOT").expect("MIRI_SYSROOT must be set to run the ui test suite")
+        )
+        .into(),
+    );
     program.args.push("-Dwarnings".into());
     program.args.push("-Dunused".into());
     program.args.push("-Ainternal_features".into());