Rollup merge of #63162 - RalfJung:miri-xargo, r=alexcrichton

Miri tests: use xargo to build separate libstd

This uses `cargo miri setup` to prepare the libstd that is used for testing Miri, instead of adjusting the entire bootstrap process to make sure the libstd that already gets built is fit for Miri.

The issue with our current approach is that with `test-miri = true`, libstd and the test suite get built with `--cfg miri`, which e.g. means hashbrown uses no SIMD, and not all things are tested. Such global side-effects seem like footguns waiting to go off.

On the other hand, the new approach means we install xargo as a side-effect of doing `./x.py test src/tools/miri`, which might be surprising, and we also both have to build xargo and another libstd which costs some extra time. Not sure if the tools builders have enough time budget for that. Maybe there is a way to cache xargo?

We have to first first land https://github.com/rust-lang/miri/pull/870 in Miri and then update this PR to include that change (also to get CI to test Miri before bors), but I wanted to get the review started here.

Cc @oli-obk (for Miri) @alexcrichton (for CI) @Mark-Simulacrum (for bootstrap)

Fixes https://github.com/rust-lang/rust/issues/61833, fixes https://github.com/rust-lang/rust/issues/63219

9 files changed, 94 insertions, 58 deletions

diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs
index 23f81c2c876..54b689fb062 100644
--- a/src/bootstrap/bin/rustc.rs
+++ b/src/bootstrap/bin/rustc.rs
@@ -143,8 +143,11 @@ fn main() {
 
     if let Some(target) = target {
         // The stage0 compiler has a special sysroot distinct from what we
-        // actually downloaded, so we just always pass the `--sysroot` option.
-        cmd.arg("--sysroot").arg(&sysroot);
+        // actually downloaded, so we just always pass the `--sysroot` option,
+        // unless one is already set.
+        if !args.iter().any(|arg| arg == "--sysroot") {
+            cmd.arg("--sysroot").arg(&sysroot);
+        }
 
         cmd.arg("-Zexternal-macro-backtrace");
 
@@ -285,20 +288,6 @@ fn main() {
             }
         }
 
-        // When running miri tests, we need to generate MIR for all libraries
-        if env::var("TEST_MIRI").ok().map_or(false, |val| val == "true") {
-            // The flags here should be kept in sync with `add_miri_default_args`
-            // in miri's `src/lib.rs`.
-            cmd.arg("-Zalways-encode-mir");
-            cmd.arg("--cfg=miri");
-            // These options are preferred by miri, to be able to perform better validation,
-            // but the bootstrap compiler might not understand them.
-            if stage != "0" {
-                cmd.arg("-Zmir-emit-retag");
-                cmd.arg("-Zmir-opt-level=0");
-            }
-        }
-
         if let Ok(map) = env::var("RUSTC_DEBUGINFO_MAP") {
             cmd.arg("--remap-path-prefix").arg(&map);
         }
diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs
index 5a75497173e..e54c9360bae 100644
--- a/src/bootstrap/builder.rs
+++ b/src/bootstrap/builder.rs
@@ -543,15 +543,6 @@ impl<'a> Builder<'a> {
             parent: Cell::new(None),
         };
 
-        if kind == Kind::Dist {
-            assert!(
-                !builder.config.test_miri,
-                "Do not distribute with miri enabled.\n\
-                The distributed libraries would include all MIR (increasing binary size).
-                The distributed MIR would include validation statements."
-            );
-        }
-
         builder
     }
 
@@ -981,7 +972,6 @@ impl<'a> Builder<'a> {
                     PathBuf::from("/path/to/nowhere/rustdoc/not/required")
                 },
             )
-            .env("TEST_MIRI", self.config.test_miri.to_string())
             .env("RUSTC_ERROR_METADATA_DST", self.extended_error_dir());
 
         if let Some(host_linker) = self.linker(compiler.host) {
diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs
index 5a5f4ac7252..a5bfafdfdb4 100644
--- a/src/bootstrap/config.rs
+++ b/src/bootstrap/config.rs
@@ -128,7 +128,6 @@ pub struct Config {
     pub low_priority: bool,
     pub channel: String,
     pub verbose_tests: bool,
-    pub test_miri: bool,
     pub save_toolstates: Option<PathBuf>,
     pub print_step_timings: bool,
     pub missing_tools: bool,
@@ -315,7 +314,6 @@ struct Rust {
     debug: Option<bool>,
     dist_src: Option<bool>,
     verbose_tests: Option<bool>,
-    test_miri: Option<bool>,
     incremental: Option<bool>,
     save_toolstates: Option<String>,
     codegen_backends: Option<Vec<String>>,
@@ -375,7 +373,6 @@ impl Config {
         config.codegen_tests = true;
         config.ignore_git = false;
         config.rust_dist_src = true;
-        config.test_miri = false;
         config.rust_codegen_backends = vec![INTERNER.intern_str("llvm")];
         config.rust_codegen_backends_dir = "codegen-backends".to_owned();
         config.deny_warnings = true;
@@ -557,7 +554,6 @@ impl Config {
             set(&mut config.channel, rust.channel.clone());
             set(&mut config.rust_dist_src, rust.dist_src);
             set(&mut config.verbose_tests, rust.verbose_tests);
-            set(&mut config.test_miri, rust.test_miri);
             // in the case "false" is set explicitly, do not overwrite the command line args
             if let Some(true) = rust.incremental {
                 config.incremental = true;
diff --git a/src/bootstrap/configure.py b/src/bootstrap/configure.py
index 298330869b6..346f0cb2039 100755
--- a/src/bootstrap/configure.py
+++ b/src/bootstrap/configure.py
@@ -36,7 +36,6 @@ o("docs", "build.docs", "build standard library documentation")
 o("compiler-docs", "build.compiler-docs", "build compiler documentation")
 o("optimize-tests", "rust.optimize-tests", "build tests with optimizations")
 o("parallel-compiler", "rust.parallel-compiler", "build a multi-threaded rustc")
-o("test-miri", "rust.test-miri", "run miri's test suite")
 o("verbose-tests", "rust.verbose-tests", "enable verbose output when running tests")
 o("ccache", "llvm.ccache", "invoke gcc/clang via ccache to reuse object files between builds")
 o("sccache", None, "invoke gcc/clang via sccache to reuse object files between builds")
diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs
index c8ea3157dc9..b72aa78f3de 100644
--- a/src/bootstrap/lib.rs
+++ b/src/bootstrap/lib.rs
@@ -540,9 +540,7 @@ impl Build {
             Mode::Rustc => "-rustc",
             Mode::Codegen => "-codegen",
             Mode::ToolBootstrap => "-bootstrap-tools",
-            Mode::ToolStd => "-tools",
-            Mode::ToolTest => "-tools",
-            Mode::ToolRustc => "-tools",
+            Mode::ToolStd | Mode::ToolTest | Mode::ToolRustc => "-tools",
         };
         self.out.join(&*compiler.host)
                 .join(format!("stage{}{}", compiler.stage, suffix))
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index e5b0a46ba6f..c2c134bfd1d 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -363,11 +363,9 @@ pub struct Miri {
 impl Step for Miri {
     type Output = ();
     const ONLY_HOSTS: bool = true;
-    const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        let test_miri = run.builder.config.test_miri;
-        run.path("src/tools/miri").default_condition(test_miri)
+        run.path("src/tools/miri")
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -389,26 +387,92 @@ impl Step for Miri {
             extra_features: Vec::new(),
         });
         if let Some(miri) = miri {
-            let mut cargo = tool::prepare_tool_cargo(builder,
-                                                 compiler,
-                                                 Mode::ToolRustc,
-                                                 host,
-                                                 "test",
-                                                 "src/tools/miri",
-                                                 SourceType::Submodule,
-                                                 &[]);
+            // # Run `cargo miri setup`.
+            // As a side-effect, this will install xargo.
+            let mut cargo = tool::prepare_tool_cargo(
+                builder,
+                compiler,
+                Mode::ToolRustc,
+                host,
+                "run",
+                "src/tools/miri",
+                SourceType::Submodule,
+                &[],
+            );
+            cargo
+                .arg("--bin")
+                .arg("cargo-miri")
+                .arg("--")
+                .arg("miri")
+                .arg("setup");
+
+            // Tell `cargo miri` not to worry about the sysroot mismatch (we built with
+            // stage1 but run with stage2).
+            cargo.env("MIRI_SKIP_SYSROOT_CHECK", "1");
+            // Tell `cargo miri setup` where to find the sources.
+            cargo.env("XARGO_RUST_SRC", builder.src.join("src"));
+            // Debug things.
+            cargo.env("RUST_BACKTRACE", "1");
+            // Configure `cargo install` path, and let cargo-miri know that that's where
+            // xargo ends up.
+            cargo.env("CARGO_INSTALL_ROOT", &builder.out); // cargo adds a `bin/`
+            cargo.env("XARGO", builder.out.join("bin").join("xargo"));
+
+            if !try_run(builder, &mut cargo) {
+                return;
+            }
+
+            // # Determine where Miri put its sysroot.
+            // To this end, we run `cargo miri setup --env` and capture the output.
+            // (We do this separately from the above so that when the setup actually
+            // happens we get some output.)
+            // We re-use the `cargo` from above.
+            cargo.arg("--env");
+
+            // FIXME: Is there a way in which we can re-use the usual `run` helpers?
+            let miri_sysroot = if builder.config.dry_run {
+                String::new()
+            } else {
+                builder.verbose(&format!("running: {:?}", cargo));
+                let out = cargo.output()
+                    .expect("We already ran `cargo miri setup` before and that worked");
+                assert!(out.status.success(), "`cargo miri setup` returned with non-0 exit code");
+                // Output is "MIRI_SYSROOT=<str>\n".
+                let stdout = String::from_utf8(out.stdout)
+                    .expect("`cargo miri setup` stdout is not valid UTF-8");
+                let stdout = stdout.trim();
+                builder.verbose(&format!("`cargo miri setup --env` returned: {:?}", stdout));
+                let sysroot = stdout.splitn(2, '=')
+                    .nth(1).expect("`cargo miri setup` stdout did not contain '='");
+                sysroot.to_owned()
+            };
+
+            // # Run `cargo test`.
+            let mut cargo = tool::prepare_tool_cargo(
+                builder,
+                compiler,
+                Mode::ToolRustc,
+                host,
+                "test",
+                "src/tools/miri",
+                SourceType::Submodule,
+                &[],
+            );
 
             // miri tests need to know about the stage sysroot
-            cargo.env("MIRI_SYSROOT", builder.sysroot(compiler));
+            cargo.env("MIRI_SYSROOT", miri_sysroot);
             cargo.env("RUSTC_TEST_SUITE", builder.rustc(compiler));
             cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(compiler));
             cargo.env("MIRI_PATH", miri);
 
             builder.add_rustc_lib_path(compiler, &mut cargo);
 
-            if try_run(builder, &mut cargo) {
-                builder.save_toolstate("miri", ToolState::TestPass);
+            if !try_run(builder, &mut cargo) {
+                return;
             }
+
+            // # Done!
+            builder.save_toolstate("miri", ToolState::TestPass);
         } else {
             eprintln!("failed to test miri: could not build");
         }
diff --git a/src/ci/azure-pipelines/auto.yml b/src/ci/azure-pipelines/auto.yml
index 6ed1585c516..687856cca6b 100644
--- a/src/ci/azure-pipelines/auto.yml
+++ b/src/ci/azure-pipelines/auto.yml
@@ -254,7 +254,7 @@ jobs:
       x86_64-msvc-tools:
         MSYS_BITS: 64
         SCRIPT: src/ci/docker/x86_64-gnu-tools/checktools.sh x.py /tmp/toolstates.json windows
-        RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-msvc --save-toolstates=/tmp/toolstates.json --enable-test-miri
+        RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-msvc --save-toolstates=/tmp/toolstates.json
 
       # 32/64-bit MinGW builds.
       #
diff --git a/src/ci/docker/x86_64-gnu-tools/Dockerfile b/src/ci/docker/x86_64-gnu-tools/Dockerfile
index bab9145cbcb..f11ae7a34cb 100644
--- a/src/ci/docker/x86_64-gnu-tools/Dockerfile
+++ b/src/ci/docker/x86_64-gnu-tools/Dockerfile
@@ -23,6 +23,5 @@ COPY x86_64-gnu-tools/repo.sh /tmp/
 
 ENV RUST_CONFIGURE_ARGS \
   --build=x86_64-unknown-linux-gnu \
-  --enable-test-miri \
   --save-toolstates=/tmp/toolstates.json
 ENV SCRIPT /tmp/checktools.sh ../x.py /tmp/toolstates.json linux
diff --git a/src/tools/miri b/src/tools/miri
-Subproject 39a524f694e42cfb178639d490d3fdbbaf8569d
+Subproject b12ebfc3de853abf6b4260c44a71cd51323803c