Skip to content

Commit

Permalink
Auto merge of rust-lang#123317 - RalfJung:test-in-miri, r=m-ou-se,sae…
Browse files Browse the repository at this point in the history
…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...
  • Loading branch information
bors committed Apr 5, 2024
2 parents 4563f70 + 3ad9c83 commit 5958f5e
Show file tree
Hide file tree
Showing 17 changed files with 490 additions and 68 deletions.
5 changes: 5 additions & 0 deletions library/alloc/benches/vec_deque_append.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
4 changes: 4 additions & 0 deletions library/alloc/src/lib.miri.rs
Original file line number Diff line number Diff line change
@@ -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::*;
4 changes: 4 additions & 0 deletions library/core/src/lib.miri.rs
Original file line number Diff line number Diff line change
@@ -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::*;
4 changes: 4 additions & 0 deletions library/std/src/lib.miri.rs
Original file line number Diff line number Diff line change
@@ -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::*;
79 changes: 57 additions & 22 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 => {
Expand Down
22 changes: 20 additions & 2 deletions src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,7 @@ pub enum Kind {
Format,
#[value(alias = "t")]
Test,
Miri,
Bench,
#[value(alias = "d")]
Doc,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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[..]),
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 5 additions & 2 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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 { .. }
Expand Down Expand Up @@ -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![],
Expand Down
28 changes: 25 additions & 3 deletions src/bootstrap/src/core/config/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down Expand Up @@ -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,
Expand All @@ -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![],
Expand All @@ -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 {
Expand Down
12 changes: 8 additions & 4 deletions src/bootstrap/src/utils/render_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(""),
}
);
}

Expand Down Expand Up @@ -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)]
Expand Down
5 changes: 5 additions & 0 deletions src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading

0 comments on commit 5958f5e

Please sign in to comment.