shift Miri's stage so that it matches other rustc-based tools

RalfJung · RalfJung · commit fb8abe5fcfca · 2024-03-31T19:44:54.000+02:00
diff --git a/src/bootstrap/src/core/build_steps/run.rs b/src/bootstrap/src/core/build_steps/run.rs
@@ -121,7 +121,6 @@ impl Step for ReplaceVersionPlaceholder {
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Miri {
-    host: TargetSelection,
     target: TargetSelection,
 }
 
@@ -134,24 +133,35 @@ impl Step for Miri {
     }
 
     fn make_run(run: RunConfig<'_>) {
-        run.builder.ensure(Miri { host: run.build_triple(), target: run.target });
+        run.builder.ensure(Miri { target: run.target });
     }
 
     fn run(self, builder: &Builder<'_>) {
-        let stage = builder.top_stage;
-        let host = self.host;
+        let host = builder.build.build;
         let target = self.target;
-        let compiler = builder.compiler(stage, host);
-
-        let compiler_std = builder.compiler(if stage < 2 { stage + 1 } else { stage }, host);
-        let miri_sysroot = test::Miri::build_miri_sysroot(builder, compiler_std, 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",
diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs
@@ -493,21 +493,20 @@ impl Step for RustDemangler {
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Miri {
-    host: TargetSelection,
     target: TargetSelection,
 }
 
 impl Miri {
     /// Run `cargo miri setup` for the given target, return where the Miri sysroot was put.
     pub fn build_miri_sysroot(
         builder: &Builder<'_>,
-        compiler_std: Compiler,
+        compiler: Compiler,
         target: TargetSelection,
     ) -> String {
-        let miri_sysroot = builder.out.join(compiler_std.host.triple).join("miri-sysroot");
+        let miri_sysroot = builder.out.join(compiler.host.triple).join("miri-sysroot");
         let mut cargo = builder::Cargo::new(
             builder,
-            compiler_std, // this is compiler+1; cargo_miri_cmd will do -1 again
+            compiler,
             Mode::Std,
             SourceType::Submodule,
             target,
@@ -520,13 +519,8 @@ impl Miri {
         cargo.env("MIRI_SYSROOT", &miri_sysroot);
 
         let mut cargo = Command::from(cargo);
-        let _guard = builder.msg(
-            Kind::Build,
-            compiler_std.stage,
-            "miri sysroot",
-            compiler_std.host,
-            compiler_std.host,
-        );
+        let _guard =
+            builder.msg(Kind::Build, compiler.stage, "miri sysroot", compiler.host, target);
         builder.run(&mut cargo);
 
         // # Determine where Miri put its sysroot.
@@ -563,34 +557,51 @@ impl Step for Miri {
     }
 
     fn make_run(run: RunConfig<'_>) {
-        run.builder.ensure(Miri { 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 = builder.top_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: host, extra_features: Vec::new() });
+        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, 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_std, target);
+        builder.ensure(tool::CargoMiri {
+            compiler: host_compiler,
+            target: host,
+            extra_features: Vec::new(),
+        });
+
+        // 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",
@@ -603,7 +614,7 @@ impl Step for Miri {
 
         // 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", compiler, target, builder);
+        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);
@@ -618,7 +629,7 @@ impl Step for Miri {
         cargo.env("MIRI_TEST_TARGET", target.rustc_target_arg());
 
         {
-            let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "miri", host, target);
+            let _guard = builder.msg_sysroot_tool(Kind::Test, stage, "miri", host, target);
             let _time = helpers::timeit(builder);
             builder.run(&mut cargo);
         }
@@ -636,7 +647,7 @@ impl Step for Miri {
             {
                 let _guard = builder.msg_sysroot_tool(
                     Kind::Test,
-                    compiler.stage,
+                    stage,
                     "miri (mir-opt-level 4)",
                     host,
                     target,
@@ -650,10 +661,10 @@ impl Step for Miri {
         // 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, and that all the logic in `cargo-miri` does not explode.
-        // Everything here needs `compiler_std` to be actually testing the Miri in the current stage.
+        // 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_std,  // this is compiler+1; cargo_miri_cmd will do -1 again
+            target_compiler,
             Mode::ToolStd, // it's unclear what to use here, we're not building anything just doing a smoke test!
             target,
             "miri-test",
@@ -662,16 +673,12 @@ impl Step for Miri {
             &[],
         );
 
-        // `prepare_tool_cargo` sets RUSTDOC to the bootstrap wrapper and RUSTDOC_REAL to a dummy path as this is a "miri", 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).
-        if builder.doc_tests != DocTests::No {
-            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.arg("--tests");
+                cargo.args(["--lib", "--bins", "--examples", "--tests", "--benches"]);
             }
             DocTests::Only => {
                 cargo.arg("--doc");
@@ -686,8 +693,7 @@ impl Step for Miri {
         cargo.arg("--").args(builder.config.test_args());
         let mut cargo = Command::from(cargo);
         {
-            let _guard =
-                builder.msg_sysroot_tool(Kind::Test, compiler.stage, "cargo-miri", host, target);
+            let _guard = builder.msg_sysroot_tool(Kind::Test, stage, "cargo-miri", host, target);
             let _time = helpers::timeit(builder);
             builder.run(&mut cargo);
         }
diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs
@@ -1031,10 +1031,10 @@ impl<'a> Builder<'a> {
         StepDescription::run(v, self, paths);
     }
 
-    /// Obtain a compiler at a given stage and for a given host. Explicitly does
-    /// not take `Compiler` since all `Compiler` instances are meant to be
-    /// obtained through this function, since it ensures that they are valid
-    /// (i.e., built and assembled).
+    /// Obtain a compiler at a given stage and for a given host (i.e., this is the target that the
+    /// compiler will run on, *not* the target it will build code for). Explicitly does not take
+    /// `Compiler` since all `Compiler` instances are meant to be obtained through this function,
+    /// since it ensures that they are valid (i.e., built and assembled).
     pub fn compiler(&self, stage: u32, host: TargetSelection) -> Compiler {
         self.ensure(compile::Assemble { target_compiler: Compiler { stage, host } })
     }
@@ -1703,7 +1703,8 @@ impl<'a> Builder<'a> {
             .env("RUSTDOC", self.bootstrap_out.join("rustdoc"))
             .env(
                 "RUSTDOC_REAL",
-                if cmd == "doc" || cmd == "rustdoc" || (cmd == "test" && want_rustdoc) {
+                // Make sure to handle both `test` and `miri-test` commands.
+                if cmd == "doc" || cmd == "rustdoc" || (cmd.ends_with("test") && want_rustdoc) {
                     self.rustdoc(compiler)
                 } else {
                     PathBuf::from("/path/to/nowhere/rustdoc/not/required")
