Auto merge of rust-lang#123317 - RalfJung:test-in-miri, r=m-ou-se,sae…

…thlin,onur-ozkan

Support running library tests in Miri

This adds a new bootstrap subcommand `./x.py miri` which can test libraries in Miri. This is in preparation for eventually doing that as part of bors CI, but this PR only adds the infrastructure, and doesn't enable it yet.

`@rust-lang/bootstrap` should this be `x.py test --miri library/core` or `x.py miri library/core`? The flag has the advantage that we don't have to copy all the arguments from `Subcommand::Test`. It has the disadvantage that most test steps just ignore `--miri` and still run tests the regular way. For clippy you went the route of making it a separate subcommand. ~~I went with a flag now as that seemed easier, but I can change this.~~ I made it a new subcommand. Note however that the regular cargo invocation would be `cargo miri test ...`, so `x.py` is still going to be different in that the `test` is omitted. That said, we could also make it `./x.py miri-test` to make that difference smaller -- that's in fact more consistent with the internal name of the command when bootstrap invokes cargo.

`@rust-lang/libs` ~~unfortunately this PR does some unholy things to the `lib.rs` files of our library crates.~~
`@m-ou-se` found a way that entirely avoids library-level hacks, except for some new small `lib.miri.rs` files that hopefully you will never have to touch. There's a new hack in cargo-miri but there it is in good company...

library/alloc/benches/vec_deque_append.rs

@@ -5,6 +5,11 @@ const WARMUP_N: usize = 100;
 const BENCH_N: usize = 1000;
 
 fn main() {
+    if cfg!(miri) {
+        // Don't benchmark Miri...
+        // (Due to bootstrap quirks, this gets picked up by `x.py miri library/alloc --no-doc`.)
+        return;
+    }
     let a: VecDeque<i32> = (0..VECDEQUE_LEN).collect();
     let b: VecDeque<i32> = (0..VECDEQUE_LEN).collect();
 

library/alloc/src/lib.miri.rs

@@ -0,0 +1,4 @@
+//! Grep bootstrap for `MIRI_REPLACE_LIBRS_IF_NOT_TEST` to learn what this is about.
+#![no_std]
+extern crate alloc as realalloc;
+pub use realalloc::*;

library/core/src/lib.miri.rs

@@ -0,0 +1,4 @@
+//! Grep bootstrap for `MIRI_REPLACE_LIBRS_IF_NOT_TEST` to learn what this is about.
+#![no_std]
+extern crate core as realcore;
+pub use realcore::*;

library/std/src/lib.miri.rs

@@ -0,0 +1,4 @@
+//! Grep bootstrap for `MIRI_REPLACE_LIBRS_IF_NOT_TEST` to learn what this is about.
+#![no_std]
+extern crate std as realstd;
+pub use realstd::*;

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

@@ -595,7 +595,7 @@ impl Step for Miri {
         // 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);
+        let host_sysroot = builder.sysroot(target_compiler);
 
         // # Run `cargo test`.
         // This is with the Miri crate, so it uses the host compiler.
@@ -618,7 +618,7 @@ impl Step for Miri {
 
         // miri tests need to know about the stage sysroot
         cargo.env("MIRI_SYSROOT", &miri_sysroot);
-        cargo.env("MIRI_HOST_SYSROOT", &sysroot);
+        cargo.env("MIRI_HOST_SYSROOT", &host_sysroot);
         cargo.env("MIRI", &miri);
 
         // Set the target.
@@ -681,10 +681,6 @@ impl Step for Miri {
             }
         }
 
-        // Tell `cargo miri` where to find the sysroots.
-        cargo.env("MIRI_SYSROOT", &miri_sysroot);
-        cargo.env("MIRI_HOST_SYSROOT", sysroot);
-
         // Finally, pass test-args and run everything.
         cargo.arg("--").args(builder.config.test_args());
         let mut cargo = Command::from(cargo);
@@ -2540,9 +2536,14 @@ fn prepare_cargo_test(
     //
     // Note that to run the compiler we need to run with the *host* libraries,
     // but our wrapper scripts arrange for that to be the case anyway.
-    let mut dylib_path = dylib_path();
-    dylib_path.insert(0, PathBuf::from(&*builder.sysroot_libdir(compiler, target)));
-    cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());
+    //
+    // We skip everything on Miri as then this overwrites the libdir set up
+    // by `Cargo::new` and that actually makes things go wrong.
+    if builder.kind != Kind::Miri {
+        let mut dylib_path = dylib_path();
+        dylib_path.insert(0, PathBuf::from(&*builder.sysroot_libdir(compiler, target)));
+        cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());
+    }
 
     if builder.remote_tested(target) {
         cargo.env(
@@ -2598,28 +2599,62 @@ impl Step for Crate {
         let target = self.target;
         let mode = self.mode;
 
+        // Prepare sysroot
         // See [field@compile::Std::force_recompile].
         builder.ensure(compile::Std::force_recompile(compiler, compiler.host));
 
-        if builder.config.build != target {
-            builder.ensure(compile::Std::force_recompile(compiler, target));
-            builder.ensure(RemoteCopyLibs { compiler, target });
-        }
-
         // If we're not doing a full bootstrap but we're testing a stage2
         // version of libstd, then what we're actually testing is the libstd
         // produced in stage1. Reflect that here by updating the compiler that
         // we're working with automatically.
         let compiler = builder.compiler_for(compiler.stage, compiler.host, target);
 
-        let mut cargo = builder::Cargo::new(
-            builder,
-            compiler,
-            mode,
-            SourceType::InTree,
-            target,
-            builder.kind.as_str(),
-        );
+        let mut cargo = if builder.kind == Kind::Miri {
+            if builder.top_stage == 0 {
+                eprintln!("ERROR: `x.py miri` requires stage 1 or higher");
+                std::process::exit(1);
+            }
+
+            // Build `cargo miri test` command
+            // (Implicitly prepares target sysroot)
+            let mut cargo = builder::Cargo::new(
+                builder,
+                compiler,
+                mode,
+                SourceType::InTree,
+                target,
+                "miri-test",
+            );
+            // This hack helps bootstrap run standard library tests in Miri. The issue is as
+            // follows: when running `cargo miri test` on libcore, cargo builds a local copy of core
+            // and makes it a dependency of the integration test crate. This copy duplicates all the
+            // lang items, so the build fails. (Regular testing avoids this because the sysroot is a
+            // literal copy of what `cargo build` produces, but since Miri builds its own sysroot
+            // this does not work for us.) So we need to make it so that the locally built libcore
+            // contains all the items from `core`, but does not re-define them -- we want to replace
+            // the entire crate but a re-export of the sysroot crate. We do this by swapping out the
+            // source file: if `MIRI_REPLACE_LIBRS_IF_NOT_TEST` is set and we are building a
+            // `lib.rs` file, and a `lib.miri.rs` file exists in the same folder, we build that
+            // instead. But crucially we only do that for the library, not the test builds.
+            cargo.env("MIRI_REPLACE_LIBRS_IF_NOT_TEST", "1");
+            cargo
+        } else {
+            // Also prepare a sysroot for the target.
+            if builder.config.build != target {
+                builder.ensure(compile::Std::force_recompile(compiler, target));
+                builder.ensure(RemoteCopyLibs { compiler, target });
+            }
+
+            // Build `cargo test` command
+            builder::Cargo::new(
+                builder,
+                compiler,
+                mode,
+                SourceType::InTree,
+                target,
+                builder.kind.as_str(),
+            )
+        };
 
         match mode {
             Mode::Std => {

src/bootstrap/src/core/builder.rs

@@ -631,6 +631,7 @@ pub enum Kind {
     Format,
     #[value(alias = "t")]
     Test,
+    Miri,
     Bench,
     #[value(alias = "d")]
     Doc,
@@ -673,6 +674,7 @@ impl Kind {
             Kind::Fix => "fix",
             Kind::Format => "fmt",
             Kind::Test => "test",
+            Kind::Miri => "miri",
             Kind::Bench => "bench",
             Kind::Doc => "doc",
             Kind::Clean => "clean",
@@ -822,6 +824,7 @@ impl<'a> Builder<'a> {
                 // Run run-make last, since these won't pass without make on Windows
                 test::RunMake,
             ),
+            Kind::Miri => describe!(test::Crate),
             Kind::Bench => describe!(test::Crate, test::CrateLibrustc),
             Kind::Doc => describe!(
                 doc::UnstableBook,
@@ -970,6 +973,7 @@ impl<'a> Builder<'a> {
             Subcommand::Fix => (Kind::Fix, &paths[..]),
             Subcommand::Doc { .. } => (Kind::Doc, &paths[..]),
             Subcommand::Test { .. } => (Kind::Test, &paths[..]),
+            Subcommand::Miri { .. } => (Kind::Miri, &paths[..]),
             Subcommand::Bench { .. } => (Kind::Bench, &paths[..]),
             Subcommand::Dist => (Kind::Dist, &paths[..]),
             Subcommand::Install => (Kind::Install, &paths[..]),
@@ -1226,6 +1230,7 @@ impl<'a> Builder<'a> {
         assert!(run_compiler.stage > 0, "miri can not be invoked at stage 0");
         let build_compiler = self.compiler(run_compiler.stage - 1, self.build.build);
 
+        // Prepare the tools
         let miri = self.ensure(tool::Miri {
             compiler: build_compiler,
             target: self.build.build,
@@ -1236,7 +1241,7 @@ impl<'a> Builder<'a> {
             target: self.build.build,
             extra_features: Vec::new(),
         });
-        // Invoke cargo-miri, make sure we can find miri and cargo.
+        // Invoke cargo-miri, make sure it can find miri and cargo.
         let mut cmd = Command::new(cargo_miri);
         cmd.env("MIRI", &miri);
         cmd.env("CARGO", &self.initial_cargo);
@@ -1299,7 +1304,11 @@ impl<'a> Builder<'a> {
         if cmd == "clippy" {
             cargo = self.cargo_clippy_cmd(compiler);
             cargo.arg(cmd);
-        } else if let Some(subcmd) = cmd.strip_prefix("miri-") {
+        } else if let Some(subcmd) = cmd.strip_prefix("miri") {
+            // Command must be "miri-X".
+            let subcmd = subcmd
+                .strip_prefix("-")
+                .unwrap_or_else(|| panic!("expected `miri-$subcommand`, but got {}", cmd));
             cargo = self.cargo_miri_cmd(compiler);
             cargo.arg("miri").arg(subcmd);
         } else {
@@ -1702,6 +1711,15 @@ impl<'a> Builder<'a> {
             cargo.env("RUSTC_WRAPPER_REAL", existing_wrapper);
         }
 
+        // If this is for `miri-test`, prepare the sysroots.
+        if cmd == "miri-test" {
+            self.ensure(compile::Std::new(compiler, compiler.host));
+            let host_sysroot = self.sysroot(compiler);
+            let miri_sysroot = test::Miri::build_miri_sysroot(self, compiler, target);
+            cargo.env("MIRI_SYSROOT", &miri_sysroot);
+            cargo.env("MIRI_HOST_SYSROOT", &host_sysroot);
+        }
+
         cargo.env(profile_var("STRIP"), self.config.rust_strip.to_string());
 
         if let Some(stack_protector) = &self.config.rust_stack_protector {

src/bootstrap/src/core/config/config.rs

@@ -2022,7 +2022,7 @@ impl Config {
             Subcommand::Build { .. } => {
                 flags.stage.or(build_stage).unwrap_or(if download_rustc { 2 } else { 1 })
             }
-            Subcommand::Test { .. } => {
+            Subcommand::Test { .. } | Subcommand::Miri { .. } => {
                 flags.stage.or(test_stage).unwrap_or(if download_rustc { 2 } else { 1 })
             }
             Subcommand::Bench { .. } => flags.stage.or(bench_stage).unwrap_or(2),
@@ -2044,6 +2044,7 @@ impl Config {
         if flags.stage.is_none() && crate::CiEnv::current() != crate::CiEnv::None {
             match config.cmd {
                 Subcommand::Test { .. }
+                | Subcommand::Miri { .. }
                 | Subcommand::Doc { .. }
                 | Subcommand::Build { .. }
                 | Subcommand::Bench { .. }
@@ -2099,7 +2100,9 @@ impl Config {
 
     pub(crate) fn test_args(&self) -> Vec<&str> {
         let mut test_args = match self.cmd {
-            Subcommand::Test { ref test_args, .. } | Subcommand::Bench { ref test_args, .. } => {
+            Subcommand::Test { ref test_args, .. }
+            | Subcommand::Bench { ref test_args, .. }
+            | Subcommand::Miri { ref test_args, .. } => {
                 test_args.iter().flat_map(|s| s.split_whitespace()).collect()
             }
             _ => vec![],

src/bootstrap/src/core/config/flags.rs

@@ -379,6 +379,25 @@ pub enum Subcommand {
         /// `/<build_base>/rustfix_missing_coverage.txt`
         rustfix_coverage: bool,
     },
+    /// Build and run some test suites *in Miri*
+    Miri {
+        #[arg(long)]
+        /// run all tests regardless of failure
+        no_fail_fast: bool,
+        #[arg(long, value_name = "ARGS", allow_hyphen_values(true))]
+        /// extra arguments to be passed for the test tool being used
+        /// (e.g. libtest, compiletest or rustdoc)
+        test_args: Vec<String>,
+        /// extra options to pass the compiler when running tests
+        #[arg(long, value_name = "ARGS", allow_hyphen_values(true))]
+        rustc_args: Vec<String>,
+        #[arg(long)]
+        /// do not run doc tests
+        no_doc: bool,
+        #[arg(long)]
+        /// only run doc tests
+        doc: bool,
+    },
     /// Build and run some benchmarks
     Bench {
         #[arg(long, allow_hyphen_values(true))]
@@ -450,6 +469,7 @@ impl Subcommand {
             Subcommand::Fix { .. } => Kind::Fix,
             Subcommand::Format { .. } => Kind::Format,
             Subcommand::Test { .. } => Kind::Test,
+            Subcommand::Miri { .. } => Kind::Miri,
             Subcommand::Clean { .. } => Kind::Clean,
             Subcommand::Dist { .. } => Kind::Dist,
             Subcommand::Install { .. } => Kind::Install,
@@ -461,7 +481,7 @@ impl Subcommand {
 
     pub fn rustc_args(&self) -> Vec<&str> {
         match *self {
-            Subcommand::Test { ref rustc_args, .. } => {
+            Subcommand::Test { ref rustc_args, .. } | Subcommand::Miri { ref rustc_args, .. } => {
                 rustc_args.iter().flat_map(|s| s.split_whitespace()).collect()
             }
             _ => vec![],
@@ -470,14 +490,16 @@ impl Subcommand {
 
     pub fn fail_fast(&self) -> bool {
         match *self {
-            Subcommand::Test { no_fail_fast, .. } => !no_fail_fast,
+            Subcommand::Test { no_fail_fast, .. } | Subcommand::Miri { no_fail_fast, .. } => {
+                !no_fail_fast
+            }
             _ => false,
         }
     }
 
     pub fn doc_tests(&self) -> DocTests {
         match *self {
-            Subcommand::Test { doc, no_doc, .. } => {
+            Subcommand::Test { doc, no_doc, .. } | Subcommand::Miri { no_doc, doc, .. } => {
                 if doc {
                     DocTests::Only
                 } else if no_doc {

src/bootstrap/src/utils/render_tests.rs

@@ -231,14 +231,16 @@ impl<'a> Renderer<'a> {
         print!("\ntest result: ");
         self.builder.colored_stdout(|stdout| outcome.write_long(stdout)).unwrap();
         println!(
-            ". {} passed; {} failed; {} ignored; {} measured; {} filtered out; \
-             finished in {:.2?}\n",
+            ". {} passed; {} failed; {} ignored; {} measured; {} filtered out{time}\n",
             suite.passed,
             suite.failed,
             suite.ignored,
             suite.measured,
             suite.filtered_out,
-            Duration::from_secs_f64(suite.exec_time)
+            time = match suite.exec_time {
+                Some(t) => format!("; finished in {:.2?}", Duration::from_secs_f64(t)),
+                None => format!(""),
+            }
         );
     }
 
@@ -374,7 +376,9 @@ struct SuiteOutcome {
     ignored: usize,
     measured: usize,
     filtered_out: usize,
-    exec_time: f64,
+    /// The time it took to execute this test suite, or `None` if time measurement was not possible
+    /// (e.g. due to running inside Miri).
+    exec_time: Option<f64>,
 }
 
 #[derive(serde_derive::Deserialize)]

src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh

@@ -62,3 +62,8 @@ case $HOST_TARGET in
     exit 1
     ;;
 esac
+# Also smoke-test `x.py miri`. This doesn't run any actual tests (that would take too long),
+# but it ensures that the crates build properly when tested with Miri.
+python3 "$X_PY" miri --stage 2 library/core --test-args notest
+python3 "$X_PY" miri --stage 2 library/alloc --test-args notest
+python3 "$X_PY" miri --stage 2 library/std --test-args notest