Auto merge of #123192 - RalfJung:bootstrap-test-miri, r=onur-ozkan

Refactor the way bootstrap invokes `cargo miri`

Instead of basically doing `cargo run --manifest-path=<cargo-miri's manifest> -- miri`, let's invoke the `cargo-miri` binary directly. That means less indirections, and also makes it easier to e.g. run the libcore test suite in Miri. (But there are still other issues with that.)

Also also adjusted Miri's stage numbering so that it is consistent with rustc/rustdoc.

This also makes `./x.py test miri` honor `--no-doc`.

And this fixes #123177 by moving where we handle parallel_compiler.

src/bootstrap/src/bin/rustc.rs

@@ -150,7 +150,7 @@ fn main() {
         {
             cmd.arg("-Ztls-model=initial-exec");
         }
-    } else if std::env::var("MIRI").is_err() {
+    } else {
         // Find any host flags that were passed by bootstrap.
         // The flags are stored in a RUSTC_HOST_FLAGS variable, separated by spaces.
         if let Ok(flags) = std::env::var("RUSTC_HOST_FLAGS") {

src/bootstrap/src/core/build_steps/compile.rs

@@ -1130,12 +1130,6 @@ pub fn rustc_cargo_env(
         cargo.env("CFG_DEFAULT_LINKER", s);
     }
 
-    if builder.config.rustc_parallel {
-        // keep in sync with `bootstrap/lib.rs:Build::rustc_features`
-        // `cfg` option for rustc, `features` option for cargo, for conditional compilation
-        cargo.rustflag("--cfg=parallel_compiler");
-        cargo.rustdocflag("--cfg=parallel_compiler");
-    }
     if builder.config.rust_verify_llvm_ir {
         cargo.env("RUSTC_VERIFY_LLVM_IR", "1");
     }

src/bootstrap/src/core/build_steps/run.rs

@@ -121,8 +121,6 @@ impl Step for ReplaceVersionPlaceholder {
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Miri {
-    stage: u32,
-    host: TargetSelection,
     target: TargetSelection,
 }
 
@@ -135,29 +133,35 @@ impl Step for Miri {
     }
 
     fn make_run(run: RunConfig<'_>) {
-        run.builder.ensure(Miri {
-            stage: run.builder.top_stage,
-            host: run.build_triple(),
-            target: run.target,
-        });
+        run.builder.ensure(Miri { target: run.target });
     }
 
     fn run(self, builder: &Builder<'_>) {
-        let stage = self.stage;
-        let host = self.host;
+        let host = builder.build.build;
         let target = self.target;
-        let compiler = builder.compiler(stage, host);
-
-        let miri =
-            builder.ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() });
-        let miri_sysroot = test::Miri::build_miri_sysroot(builder, compiler, &miri, target);
+        let stage = builder.top_stage;
+        if stage == 0 {
+            eprintln!("miri cannot be run at stage 0");
+            std::process::exit(1);
+        }
+
+        // This compiler runs on the host, we'll just use it for the target.
+        let target_compiler = builder.compiler(stage, host);
+        // Similar to `compile::Assemble`, build with the previous stage's compiler. Otherwise
+        // we'd have stageN/bin/rustc and stageN/bin/rustdoc be effectively different stage
+        // compilers, which isn't what we want. Rustdoc should be linked in the same way as the
+        // rustc compiler it's paired with, so it must be built with the previous stage compiler.
+        let host_compiler = builder.compiler(stage - 1, host);
+
+        // Get a target sysroot for Miri.
+        let miri_sysroot = test::Miri::build_miri_sysroot(builder, target_compiler, target);
 
         // # Run miri.
         // Running it via `cargo run` as that figures out the right dylib path.
         // add_rustc_lib_path does not add the path that contains librustc_driver-<...>.so.
         let mut miri = tool::prepare_tool_cargo(
             builder,
-            compiler,
+            host_compiler,
             Mode::ToolRustc,
             host,
             "run",

src/bootstrap/src/core/build_steps/test.rs

@@ -493,8 +493,6 @@ impl Step for RustDemangler {
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Miri {
-    stage: u32,
-    host: TargetSelection,
     target: TargetSelection,
 }
 
@@ -503,41 +501,26 @@ impl Miri {
     pub fn build_miri_sysroot(
         builder: &Builder<'_>,
         compiler: Compiler,
-        miri: &Path,
         target: TargetSelection,
     ) -> String {
         let miri_sysroot = builder.out.join(compiler.host.triple).join("miri-sysroot");
-        let mut cargo = tool::prepare_tool_cargo(
+        let mut cargo = builder::Cargo::new(
             builder,
             compiler,
-            Mode::ToolRustc,
-            compiler.host,
-            "run",
-            "src/tools/miri/cargo-miri",
-            SourceType::InTree,
-            &[],
+            Mode::Std,
+            SourceType::Submodule,
+            target,
+            "miri-setup",
         );
-        cargo.add_rustc_lib_path(builder);
-        cargo.arg("--").arg("miri").arg("setup");
-        cargo.arg("--target").arg(target.rustc_target_arg());
 
         // Tell `cargo miri setup` where to find the sources.
         cargo.env("MIRI_LIB_SRC", builder.src.join("library"));
-        // Tell it where to find Miri.
-        cargo.env("MIRI", miri);
         // Tell it where to put the sysroot.
         cargo.env("MIRI_SYSROOT", &miri_sysroot);
-        // Debug things.
-        cargo.env("RUST_BACKTRACE", "1");
 
         let mut cargo = Command::from(cargo);
-        let _guard = builder.msg(
-            Kind::Build,
-            compiler.stage + 1,
-            "miri sysroot",
-            compiler.host,
-            compiler.host,
-        );
+        let _guard =
+            builder.msg(Kind::Build, compiler.stage, "miri sysroot", compiler.host, target);
         builder.run(&mut cargo);
 
         // # Determine where Miri put its sysroot.
@@ -574,68 +557,75 @@ impl Step for Miri {
     }
 
     fn make_run(run: RunConfig<'_>) {
-        run.builder.ensure(Miri {
-            stage: run.builder.top_stage,
-            host: run.build_triple(),
-            target: run.target,
-        });
+        run.builder.ensure(Miri { target: run.target });
     }
 
     /// Runs `cargo test` for miri.
     fn run(self, builder: &Builder<'_>) {
-        let stage = self.stage;
-        let host = self.host;
+        let host = builder.build.build;
         let target = self.target;
-        let compiler = builder.compiler(stage, host);
-        // We need the stdlib for the *next* stage, as it was built with this compiler that also built Miri.
-        // Except if we are at stage 2, the bootstrap loop is complete and we can stick with our current stage.
-        let compiler_std = builder.compiler(if stage < 2 { stage + 1 } else { stage }, host);
-
-        let miri =
-            builder.ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() });
-        let _cargo_miri = builder.ensure(tool::CargoMiri {
-            compiler,
-            target: self.host,
+        let stage = builder.top_stage;
+        if stage == 0 {
+            eprintln!("miri cannot be tested at stage 0");
+            std::process::exit(1);
+        }
+
+        // This compiler runs on the host, we'll just use it for the target.
+        let target_compiler = builder.compiler(stage, host);
+        // Similar to `compile::Assemble`, build with the previous stage's compiler. Otherwise
+        // we'd have stageN/bin/rustc and stageN/bin/rustdoc be effectively different stage
+        // compilers, which isn't what we want. Rustdoc should be linked in the same way as the
+        // rustc compiler it's paired with, so it must be built with the previous stage compiler.
+        let host_compiler = builder.compiler(stage - 1, host);
+
+        // Build our tools.
+        let miri = builder.ensure(tool::Miri {
+            compiler: host_compiler,
+            target: host,
+            extra_features: Vec::new(),
+        });
+        // the ui tests also assume cargo-miri has been built
+        builder.ensure(tool::CargoMiri {
+            compiler: host_compiler,
+            target: host,
             extra_features: Vec::new(),
         });
-        // The stdlib we need might be at a different stage. And just asking for the
-        // sysroot does not seem to populate it, so we do that first.
-        builder.ensure(compile::Std::new(compiler_std, host));
-        let sysroot = builder.sysroot(compiler_std);
-        // We also need a Miri sysroot.
-        let miri_sysroot = Miri::build_miri_sysroot(builder, compiler, &miri, target);
+
+        // We also need sysroots, for Miri and for the host (the latter for build scripts).
+        // This is for the tests so everything is done with the target compiler.
+        let miri_sysroot = Miri::build_miri_sysroot(builder, target_compiler, target);
+        builder.ensure(compile::Std::new(target_compiler, host));
+        let sysroot = builder.sysroot(target_compiler);
 
         // # Run `cargo test`.
+        // This is with the Miri crate, so it uses the host compiler.
         let mut cargo = tool::prepare_tool_cargo(
             builder,
-            compiler,
+            host_compiler,
             Mode::ToolRustc,
             host,
             "test",
             "src/tools/miri",
             SourceType::InTree,
             &[],
         );
-        let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "miri", host, target);
 
         cargo.add_rustc_lib_path(builder);
 
+        // We can NOT use `run_cargo_test` since Miri's integration tests do not use the usual test
+        // harness and therefore do not understand the flags added by `add_flags_and_try_run_test`.
+        let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", host_compiler, host, builder);
+
         // miri tests need to know about the stage sysroot
         cargo.env("MIRI_SYSROOT", &miri_sysroot);
         cargo.env("MIRI_HOST_SYSROOT", &sysroot);
         cargo.env("MIRI", &miri);
-        if builder.config.locked_deps {
-            // enforce lockfiles
-            cargo.env("CARGO_EXTRA_FLAGS", "--locked");
-        }
 
         // Set the target.
         cargo.env("MIRI_TEST_TARGET", target.rustc_target_arg());
 
-        // This can NOT be `run_cargo_test` since the Miri test runner
-        // does not understand the flags added by `add_flags_and_try_run_test`.
-        let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", compiler, target, builder);
         {
+            let _guard = builder.msg_sysroot_tool(Kind::Test, stage, "miri", host, target);
             let _time = helpers::timeit(builder);
             builder.run(&mut cargo);
         }
@@ -650,8 +640,14 @@ impl Step for Miri {
             // Optimizations can change error locations and remove UB so don't run `fail` tests.
             cargo.args(["tests/pass", "tests/panic"]);
 
-            let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", compiler, target, builder);
             {
+                let _guard = builder.msg_sysroot_tool(
+                    Kind::Test,
+                    stage,
+                    "miri (mir-opt-level 4)",
+                    host,
+                    target,
+                );
                 let _time = helpers::timeit(builder);
                 builder.run(&mut cargo);
             }
@@ -660,42 +656,40 @@ impl Step for Miri {
         // # Run `cargo miri test`.
         // This is just a smoke test (Miri's own CI invokes this in a bunch of different ways and ensures
         // that we get the desired output), but that is sufficient to make sure that the libtest harness
-        // itself executes properly under Miri.
+        // itself executes properly under Miri, and that all the logic in `cargo-miri` does not explode.
+        // This is running the build `cargo-miri` for the given target, so we need the target compiler.
         let mut cargo = tool::prepare_tool_cargo(
             builder,
-            compiler,
-            Mode::ToolRustc,
-            host,
-            "run",
-            "src/tools/miri/cargo-miri",
+            target_compiler,
+            Mode::ToolStd, // it's unclear what to use here, we're not building anything just doing a smoke test!
+            target,
+            "miri-test",
+            "src/tools/miri/test-cargo-miri",
             SourceType::Submodule,
             &[],
         );
-        cargo.add_rustc_lib_path(builder);
-        cargo.arg("--").arg("miri").arg("test");
-        if builder.config.locked_deps {
-            cargo.arg("--locked");
-        }
-        cargo
-            .arg("--manifest-path")
-            .arg(builder.src.join("src/tools/miri/test-cargo-miri/Cargo.toml"));
-        cargo.arg("--target").arg(target.rustc_target_arg());
-        cargo.arg("--").args(builder.config.test_args());
 
-        // `prepare_tool_cargo` sets RUSTDOC to the bootstrap wrapper and RUSTDOC_REAL to a dummy path as this is a "run", not a "test".
-        // Also, we want the rustdoc from the "next" stage for the same reason that we build a std from the next stage.
-        // So let's just set that here, and bypass bootstrap's RUSTDOC (just like cargo-miri already ignores bootstrap's RUSTC_WRAPPER).
-        cargo.env("RUSTDOC", builder.rustdoc(compiler_std));
+        // We're not using `prepare_cargo_test` so we have to do this ourselves.
+        // (We're not using that as the test-cargo-miri crate is not known to bootstrap.)
+        match builder.doc_tests {
+            DocTests::Yes => {}
+            DocTests::No => {
+                cargo.args(["--lib", "--bins", "--examples", "--tests", "--benches"]);
+            }
+            DocTests::Only => {
+                cargo.arg("--doc");
+            }
+        }
 
-        // Tell `cargo miri` where to find things.
+        // Tell `cargo miri` where to find the sysroots.
         cargo.env("MIRI_SYSROOT", &miri_sysroot);
         cargo.env("MIRI_HOST_SYSROOT", sysroot);
-        cargo.env("MIRI", &miri);
-        // Debug things.
-        cargo.env("RUST_BACKTRACE", "1");
 
+        // Finally, pass test-args and run everything.
+        cargo.arg("--").args(builder.config.test_args());
         let mut cargo = Command::from(cargo);
         {
+            let _guard = builder.msg_sysroot_tool(Kind::Test, stage, "cargo-miri", host, target);
             let _time = helpers::timeit(builder);
             builder.run(&mut cargo);
         }

src/bootstrap/src/core/build_steps/tool.rs

@@ -469,7 +469,7 @@ impl Step for Rustdoc {
             features.push("jemalloc".to_string());
         }
 
-        let mut cargo = prepare_tool_cargo(
+        let cargo = prepare_tool_cargo(
             builder,
             build_compiler,
             Mode::ToolRustc,
@@ -480,10 +480,6 @@ impl Step for Rustdoc {
             features.as_slice(),
         );
 
-        if builder.config.rustc_parallel {
-            cargo.rustflag("--cfg=parallel_compiler");
-        }
-
         let _guard = builder.msg_tool(
             Mode::ToolRustc,
             "rustdoc",
@@ -732,7 +728,7 @@ impl Step for LlvmBitcodeLinker {
         builder.ensure(compile::Std::new(self.compiler, self.compiler.host));
         builder.ensure(compile::Rustc::new(self.compiler, self.target));
 
-        let mut cargo = prepare_tool_cargo(
+        let cargo = prepare_tool_cargo(
             builder,
             self.compiler,
             Mode::ToolRustc,
@@ -743,10 +739,6 @@ impl Step for LlvmBitcodeLinker {
             &self.extra_features,
         );
 
-        if builder.config.rustc_parallel {
-            cargo.rustflag("--cfg=parallel_compiler");
-        }
-
         builder.run(&mut cargo.into());
 
         let tool_out = builder