From e16f46cdab0c853a9d1fab15e4746947de72a202 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 9 May 2024 11:13:54 +0200 Subject: [PATCH 1/2] make MIRI_TEST_TARGET entirely an internal thing --- src/tools/miri/CONTRIBUTING.md | 45 +++++++++++-- src/tools/miri/README.md | 34 ---------- src/tools/miri/ci/ci.sh | 67 ++++++++++--------- src/tools/miri/miri-script/src/commands.rs | 77 +++++++++------------- src/tools/miri/miri-script/src/main.rs | 57 ++++++++++------ src/tools/miri/miri-script/src/util.rs | 24 +++++++ src/tools/miri/test-cargo-miri/run-test.py | 14 ++-- 7 files changed, 175 insertions(+), 143 deletions(-) diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 57682e60c370f..5a747e7935e34 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -78,8 +78,8 @@ You can (cross-)run the entire test suite using: ``` `./miri test FILTER` only runs those tests that contain `FILTER` in their filename (including the -base directory, e.g. `./miri test fail` will run all compile-fail tests). Multiple filters -are supported: `./miri test FILTER1 FILTER2`. +base directory, e.g. `./miri test fail` will run all compile-fail tests). Multiple filters are +supported: `./miri test FILTER1 FILTER2` runs all tests that contain either string. #### Fine grained logging @@ -139,9 +139,8 @@ and then you can use it as if it was installed by `rustup` as a component of the in the `miri` toolchain's sysroot to prevent conflicts with other toolchains. The Miri binaries in the `cargo` bin directory (usually `~/.cargo/bin`) are managed by rustup. -There's a test for the cargo wrapper in the `test-cargo-miri` directory; run -`./run-test.py` in there to execute it. Like `./miri test`, this respects the -`MIRI_TEST_TARGET` environment variable to execute the test for another target. +There's a test for the cargo wrapper in the `test-cargo-miri` directory; run `./run-test.py` in +there to execute it. You can pass `--target` to execute the test for another target. ### Using a modified standard library @@ -287,3 +286,39 @@ https. Add the following to your `.gitconfig`: [url "git@github.com:"] pushInsteadOf = https://github.com/ ``` + +## Internal environment variables + +The following environment variables are *internal* and must not be used by +anyone but Miri itself. They are used to communicate between different Miri +binaries, and as such worth documenting: + +* `CARGO_EXTRA_FLAGS` is understood by `./miri` and passed to all host cargo invocations. +* `MIRI_BE_RUSTC` can be set to `host` or `target`. It tells the Miri driver to + actually not interpret the code but compile it like rustc would. With `target`, Miri sets + some compiler flags to prepare the code for interpretation; with `host`, this is not done. + This environment variable is useful to be sure that the compiled `rlib`s are compatible + with Miri. +* `MIRI_CALLED_FROM_SETUP` is set during the Miri sysroot build, + which will re-invoke `cargo-miri` as the `rustc` to use for this build. +* `MIRI_CALLED_FROM_RUSTDOC` when set to any value tells `cargo-miri` that it is + running as a child process of `rustdoc`, which invokes it twice for each doc-test + and requires special treatment, most notably a check-only build before interpretation. + This is set by `cargo-miri` itself when running as a `rustdoc`-wrapper. +* `MIRI_CWD` when set to any value tells the Miri driver to change to the given + directory after loading all the source files, but before commencing + interpretation. This is useful if the interpreted program wants a different + working directory at run-time than at build-time. +* `MIRI_LOCAL_CRATES` is set by `cargo-miri` to tell the Miri driver which + crates should be given special treatment in diagnostics, in addition to the + crate currently being compiled. +* `MIRI_ORIG_RUSTDOC` is set and read by different phases of `cargo-miri` to remember the + value of `RUSTDOC` from before it was overwritten. +* `MIRI_REPLACE_LIBRS_IF_NOT_TEST` when set to any value enables a hack that helps bootstrap + run the standard library tests in Miri. +* `MIRI_TEST_TARGET` is set by `./miri test` (and `./x.py test miri`) to tell the test harness about + the chosen target. +* `MIRI_VERBOSE` when set to any value tells the various `cargo-miri` phases to + perform verbose logging. +* `MIRI_HOST_SYSROOT` is set by bootstrap to tell `cargo-miri` which sysroot to use for *host* + operations. diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 6e0c96499e206..1bdfc4ac1a78f 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -463,9 +463,6 @@ by all intended entry points, i.e. `cargo miri` and `./miri {test,run}`): * `MIRI_SYSROOT` indicates the sysroot to use. When using `cargo miri`, this skips the automatic setup -- only set this if you do not want to use the automatically created sysroot. When invoking `cargo miri setup`, this indicates where the sysroot will be put. -* `MIRI_TEST_TARGET` (recognized by `./miri {test,run}`) indicates which target - architecture to test against. The `--target` flag may be used for the same - purpose. * `MIRI_TEST_THREADS` (recognized by `./miri test`): set the number of threads to use for running tests. By default, the number of cores is used. * `MIRI_NO_STD` makes sure that the target's sysroot is built without libstd. This allows testing @@ -476,37 +473,6 @@ by all intended entry points, i.e. `cargo miri` and `./miri {test,run}`): * `MIRI_SKIP_UI_CHECKS` (recognized by `./miri test`): don't check whether the `stderr` or `stdout` files match the actual output. -The following environment variables are *internal* and must not be used by -anyone but Miri itself. They are used to communicate between different Miri -binaries, and as such worth documenting: - -* `MIRI_BE_RUSTC` can be set to `host` or `target`. It tells the Miri driver to - actually not interpret the code but compile it like rustc would. With `target`, Miri sets - some compiler flags to prepare the code for interpretation; with `host`, this is not done. - This environment variable is useful to be sure that the compiled `rlib`s are compatible - with Miri. -* `MIRI_CALLED_FROM_SETUP` is set during the Miri sysroot build, - which will re-invoke `cargo-miri` as the `rustc` to use for this build. -* `MIRI_CALLED_FROM_RUSTDOC` when set to any value tells `cargo-miri` that it is - running as a child process of `rustdoc`, which invokes it twice for each doc-test - and requires special treatment, most notably a check-only build before interpretation. - This is set by `cargo-miri` itself when running as a `rustdoc`-wrapper. -* `MIRI_CWD` when set to any value tells the Miri driver to change to the given - directory after loading all the source files, but before commencing - interpretation. This is useful if the interpreted program wants a different - working directory at run-time than at build-time. -* `MIRI_LOCAL_CRATES` is set by `cargo-miri` to tell the Miri driver which - crates should be given special treatment in diagnostics, in addition to the - crate currently being compiled. -* `MIRI_ORIG_RUSTDOC` is set and read by different phases of `cargo-miri` to remember the - value of `RUSTDOC` from before it was overwritten. -* `MIRI_REPLACE_LIBRS_IF_NOT_TEST` when set to any value enables a hack that helps bootstrap - run the standard library tests in Miri. -* `MIRI_VERBOSE` when set to any value tells the various `cargo-miri` phases to - perform verbose logging. -* `MIRI_HOST_SYSROOT` is set by bootstrap to tell `cargo-miri` which sysroot to use for *host* - operations. - [testing-miri]: CONTRIBUTING.md#testing-the-miri-driver ## Miri `extern` functions diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 18f1bf18c7d23..f5fbb05d89622 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -31,24 +31,26 @@ time ./miri build --all-targets # the build that all the `./miri test` below wil endgroup # Run tests. Recognizes these variables: -# - MIRI_TEST_TARGET: the target to test. Empty for host target. +# - TEST_TARGET: the target to test. Empty for host target. # - GC_STRESS: if non-empty, run the GC stress test for the main test suite. # - MIR_OPT: if non-empty, re-run test `pass` tests with mir-opt-level=4 # - MANY_SEEDS: if set to N, run the "many-seeds" tests N times # - TEST_BENCH: if non-empty, check that the benchmarks all build # - CARGO_MIRI_ENV: if non-empty, set some env vars and config to potentially confuse cargo-miri function run_tests { - if [ -n "${MIRI_TEST_TARGET-}" ]; then - begingroup "Testing foreign architecture $MIRI_TEST_TARGET" + if [ -n "${TEST_TARGET-}" ]; then + begingroup "Testing foreign architecture $TEST_TARGET" + TARGET_FLAG="--target $TEST_TARGET" else begingroup "Testing host architecture" + TARGET_FLAG="" fi ## ui test suite if [ -n "${GC_STRESS-}" ]; then - time MIRIFLAGS="${MIRIFLAGS-} -Zmiri-provenance-gc=1" ./miri test + time MIRIFLAGS="${MIRIFLAGS-} -Zmiri-provenance-gc=1" ./miri test $TARGET_FLAG else - time ./miri test + time ./miri test $TARGET_FLAG fi ## advanced tests @@ -59,17 +61,17 @@ function run_tests { # them. Also error locations change so we don't run the failing tests. # We explicitly enable debug-assertions here, they are disabled by -O but we have tests # which exist to check that we panic on debug assertion failures. - time MIRIFLAGS="${MIRIFLAGS-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test tests/{pass,panic} + time MIRIFLAGS="${MIRIFLAGS-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test $TARGET_FLAG tests/{pass,panic} fi if [ -n "${MANY_SEEDS-}" ]; then # Also run some many-seeds tests. time for FILE in tests/many-seeds/*.rs; do - ./miri run "--many-seeds=0..$MANY_SEEDS" "$FILE" + ./miri run "--many-seeds=0..$MANY_SEEDS" $TARGET_FLAG "$FILE" done fi if [ -n "${TEST_BENCH-}" ]; then # Check that the benchmarks build and run, but only once. - time HYPERFINE="hyperfine -w0 -r1" ./miri bench + time HYPERFINE="hyperfine -w0 -r1" ./miri bench $TARGET_FLAG fi ## test-cargo-miri @@ -91,7 +93,7 @@ function run_tests { echo 'build.rustc-wrapper = "thisdoesnotexist"' > .cargo/config.toml fi # Run the actual test - time ${PYTHON} test-cargo-miri/run-test.py + time ${PYTHON} test-cargo-miri/run-test.py $TARGET_FLAG # Clean up unset RUSTC MIRI rm -rf .cargo @@ -100,17 +102,18 @@ function run_tests { } function run_tests_minimal { - if [ -n "${MIRI_TEST_TARGET-}" ]; then - begingroup "Testing MINIMAL foreign architecture $MIRI_TEST_TARGET: only testing $@" + if [ -n "${TEST_TARGET-}" ]; then + begingroup "Testing MINIMAL foreign architecture $TEST_TARGET: only testing $@" + TARGET_FLAG="--target $TEST_TARGET" else - echo "run_tests_minimal requires MIRI_TEST_TARGET to be set" + echo "run_tests_minimal requires TEST_TARGET to be set" exit 1 fi - time ./miri test "$@" + time ./miri test $TARGET_FLAG "$@" # Ensure that a small smoke test of cargo-miri works. - time cargo miri run --manifest-path test-cargo-miri/no-std-smoke/Cargo.toml --target ${MIRI_TEST_TARGET-$HOST_TARGET} + time cargo miri run --manifest-path test-cargo-miri/no-std-smoke/Cargo.toml $TARGET_FLAG endgroup } @@ -126,33 +129,33 @@ case $HOST_TARGET in # Extra tier 1 # With reduced many-seed count to avoid spending too much time on that. # (All OSes and ABIs are run with 64 seeds at least once though via the macOS runner.) - MANY_SEEDS=16 MIRI_TEST_TARGET=i686-unknown-linux-gnu run_tests - MANY_SEEDS=16 MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests - MANY_SEEDS=16 MIRI_TEST_TARGET=x86_64-apple-darwin run_tests - MANY_SEEDS=16 MIRI_TEST_TARGET=x86_64-pc-windows-gnu run_tests + MANY_SEEDS=16 TEST_TARGET=i686-unknown-linux-gnu run_tests + MANY_SEEDS=16 TEST_TARGET=aarch64-unknown-linux-gnu run_tests + MANY_SEEDS=16 TEST_TARGET=x86_64-apple-darwin run_tests + MANY_SEEDS=16 TEST_TARGET=x86_64-pc-windows-gnu run_tests ;; aarch64-apple-darwin) # Host (tier 2) GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 - MANY_SEEDS=64 MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests - MANY_SEEDS=64 MIRI_TEST_TARGET=x86_64-pc-windows-msvc CARGO_MIRI_ENV=1 run_tests + MANY_SEEDS=64 TEST_TARGET=i686-pc-windows-gnu run_tests + MANY_SEEDS=64 TEST_TARGET=x86_64-pc-windows-msvc CARGO_MIRI_ENV=1 run_tests # Extra tier 2 - MIRI_TEST_TARGET=arm-unknown-linux-gnueabi run_tests - MIRI_TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice + TEST_TARGET=arm-unknown-linux-gnueabi run_tests + TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice # Partially supported targets (tier 2) VERY_BASIC="integer vec string btreemap" # common things we test on all of them (if they have std), requires no target-specific shims BASIC="$VERY_BASIC hello hashmap alloc align" # ensures we have the shims for stdout and basic data structures - MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus - MIRI_TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus - MIRI_TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $VERY_BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random - MIRI_TEST_TARGET=x86_64-pc-solaris run_tests_minimal $VERY_BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random - MIRI_TEST_TARGET=aarch64-linux-android run_tests_minimal $VERY_BASIC hello panic/panic - MIRI_TEST_TARGET=wasm32-wasi run_tests_minimal $VERY_BASIC wasm - MIRI_TEST_TARGET=wasm32-unknown-unknown run_tests_minimal $VERY_BASIC wasm - MIRI_TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std + TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus + TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC panic/panic concurrency/simple atomic threadname libc-mem libc-misc libc-random libc-time fs env num_cpus + TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $VERY_BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random + TEST_TARGET=x86_64-pc-solaris run_tests_minimal $VERY_BASIC hello panic/panic concurrency/simple pthread-sync libc-mem libc-misc libc-random + TEST_TARGET=aarch64-linux-android run_tests_minimal $VERY_BASIC hello panic/panic + TEST_TARGET=wasm32-wasi run_tests_minimal $VERY_BASIC wasm + TEST_TARGET=wasm32-unknown-unknown run_tests_minimal $VERY_BASIC wasm + TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std # Custom target JSON file - MIRI_TEST_TARGET=tests/avr.json MIRI_NO_STD=1 run_tests_minimal no_std + TEST_TARGET=tests/avr.json MIRI_NO_STD=1 run_tests_minimal no_std ;; i686-pc-windows-msvc) # Host @@ -162,7 +165,7 @@ case $HOST_TARGET in # Extra tier 1 # We really want to ensure a Linux target works on a Windows host, # and a 64bit target works on a 32bit host. - MIRI_TEST_TARGET=x86_64-unknown-linux-gnu run_tests + TEST_TARGET=x86_64-unknown-linux-gnu run_tests ;; *) echo "FATAL: unknown host target: $HOST_TARGET" diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 43abf180d3e75..8e2b07ad8052a 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -1,5 +1,5 @@ use std::env; -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::io::Write; use std::ops::Not; use std::ops::Range; @@ -25,7 +25,7 @@ impl MiriEnv { /// Returns the location of the sysroot. /// /// If the target is None the sysroot will be built for the host machine. - fn build_miri_sysroot(&mut self, quiet: bool, target: Option<&str>) -> Result { + fn build_miri_sysroot(&mut self, quiet: bool, target: Option<&OsStr>) -> Result { if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") { // Sysroot already set, use that. return Ok(miri_sysroot.into()); @@ -38,11 +38,12 @@ impl MiriEnv { self.build(&manifest_path, &[], quiet)?; let target_flag = - &if let Some(target) = target { vec!["--target", target] } else { vec![] }; + if let Some(target) = target { vec![OsStr::new("--target"), target] } else { vec![] }; + let target_flag = &target_flag; if !quiet { if let Some(target) = target { - eprintln!("$ (building Miri sysroot for {target})"); + eprintln!("$ (building Miri sysroot for {})", target.to_string_lossy()); } else { eprintln!("$ (building Miri sysroot)"); } @@ -170,7 +171,7 @@ impl Command { Command::Fmt { flags } => Self::fmt(flags), Command::Clippy { flags } => Self::clippy(flags), Command::Cargo { flags } => Self::cargo(flags), - Command::Bench { benches } => Self::bench(benches), + Command::Bench { target, benches } => Self::bench(target, benches), Command::Toolchain { flags } => Self::toolchain(flags), Command::RustcPull { commit } => Self::rustc_pull(commit.clone()), Command::RustcPush { github_user, branch } => Self::rustc_push(github_user, branch), @@ -372,7 +373,7 @@ impl Command { Ok(()) } - fn bench(benches: Vec) -> Result<()> { + fn bench(target: Option, benches: Vec) -> Result<()> { // The hyperfine to use let hyperfine = env::var("HYPERFINE"); let hyperfine = hyperfine.as_deref().unwrap_or("hyperfine -w 1 -m 5 --shell=none"); @@ -380,8 +381,6 @@ impl Command { let Some((program_name, args)) = hyperfine.split_first() else { bail!("expected HYPERFINE environment variable to be non-empty"); }; - // Extra flags to pass to cargo. - let cargo_extra_flags = std::env::var("CARGO_EXTRA_FLAGS").unwrap_or_default(); // Make sure we have an up-to-date Miri installed and selected the right toolchain. Self::install(vec![])?; @@ -397,6 +396,14 @@ impl Command { } else { benches.to_owned() }; + let target_flag = if let Some(target) = target { + let mut flag = OsString::from("--target="); + flag.push(target); + flag + } else { + OsString::new() + }; + let target_flag = &target_flag; // Run the requested benchmarks for bench in benches { let current_bench = path!(benches_dir / bench / "Cargo.toml"); @@ -404,7 +411,7 @@ impl Command { // That seems to make Windows CI happy. cmd!( sh, - "{program_name} {args...} 'cargo miri run '{cargo_extra_flags}' --manifest-path \"'{current_bench}'\"'" + "{program_name} {args...} 'cargo miri run '{target_flag}' --manifest-path \"'{current_bench}'\"'" ) .run()?; } @@ -449,23 +456,26 @@ impl Command { Ok(()) } - fn test(bless: bool, flags: Vec, target: Option) -> Result<()> { + fn test(bless: bool, mut flags: Vec, target: Option) -> Result<()> { let mut e = MiriEnv::new()?; - if let Some(target) = target.as_deref() { - // Tell the sysroot which target to test. - e.sh.set_var("MIRI_TEST_TARGET", target); - } - // Prepare a sysroot. e.build_miri_sysroot(/* quiet */ false, target.as_deref())?; - // Then test, and let caller control flags. - // Only in root project as `cargo-miri` has no tests. + // Forward information to test harness. if bless { e.sh.set_var("RUSTC_BLESS", "Gesundheit"); } + if let Some(target) = target { + // Tell the harness which target to test. + e.sh.set_var("MIRI_TEST_TARGET", target); + } + // Make sure the flags are going to the test harness, not cargo. + flags.insert(0, "--".into()); + + // Then test, and let caller control flags. + // Only in root project as `cargo-miri` has no tests. e.test(path!(e.miri_dir / "Cargo.toml"), &flags)?; Ok(()) } @@ -477,43 +487,16 @@ impl Command { mut flags: Vec, ) -> Result<()> { let mut e = MiriEnv::new()?; - // Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so - // that we set the MIRI_SYSROOT up the right way. We must make sure that - // MIRI_TEST_TARGET and `--target` are in sync. - use itertools::Itertools; - let target = flags - .iter() - .take_while(|arg| *arg != "--") - .tuple_windows() - .find(|(first, _)| *first == "--target"); - - let target_triple = if let Some((_, target)) = target { - // Found it! - e.sh.set_var("MIRI_TEST_TARGET", target); - - let triple = - target.clone().into_string().map_err(|_| anyhow!("target triple is not UTF-8"))?; - Some(triple) - } else if let Ok(target) = std::env::var("MIRI_TEST_TARGET") { - // Convert `MIRI_TEST_TARGET` into `--target`. - flags.push("--target".into()); - flags.push(target.clone().into()); - - Some(target) - } else { - None - }; + let target = arg_flag_value(&flags, "--target"); // Scan for "--edition", set one ourselves if that flag is not present. - let have_edition = - flags.iter().take_while(|arg| *arg != "--").any(|arg| *arg == "--edition"); + let have_edition = arg_flag_value(&flags, "--edition").is_some(); if !have_edition { flags.push("--edition=2021".into()); // keep in sync with `tests/ui.rs`.` } // Prepare a sysroot, and add it to the flags. - let miri_sysroot = - e.build_miri_sysroot(/* quiet */ !verbose, target_triple.as_deref())?; + let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref())?; flags.push("--sysroot".into()); flags.push(miri_sysroot.into()); diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs index c19c4c91c6519..a8626ceb45d75 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -31,11 +31,11 @@ pub enum Command { /// Build miri, set up a sysroot and then run the test suite. Test { bless: bool, - /// Flags that are passed through to `cargo test`. - flags: Vec, /// The cross-interpretation target. /// If none then the host is the target. - target: Option, + target: Option, + /// Flags that are passed through to the test harness. + flags: Vec, }, /// Build miri, set up a sysroot and then run the driver with the given . /// (Also respects MIRIFLAGS environment variable.) @@ -61,6 +61,7 @@ pub enum Command { Cargo { flags: Vec }, /// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. Bench { + target: Option, /// List of benchmarks to run. By default all benchmarks are run. benches: Vec, }, @@ -88,8 +89,8 @@ Just build miri. are passed to `cargo build`. Just check miri. are passed to `cargo check`. ./miri test [--bless] [--target ] : -Build miri, set up a sysroot and then run the test suite. are passed -to the test harness. +Build miri, set up a sysroot and then run the test suite. + are passed to the test harness. ./miri run [--dep] [-v|--verbose] [--many-seeds|--many-seeds=..to|--many-seeds=from..to] : Build miri, set up a sysroot and then run the driver with the given . @@ -113,7 +114,7 @@ install`. Sets up the rpath such that the installed binary should work in any working directory. Note that the binaries are placed in the `miri` toolchain sysroot, to prevent conflicts with other toolchains. -./miri bench : +./miri bench [--target ] : Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. can explicitly list the benchmarks to run; by default, all of them are run. @@ -150,7 +151,7 @@ fn main() -> Result<()> { Some("build") => Command::Build { flags: args.collect() }, Some("check") => Command::Check { flags: args.collect() }, Some("test") => { - let mut target = std::env::var("MIRI_TEST_TARGET").ok(); + let mut target = None; let mut bless = false; while let Some(arg) = args.peek().and_then(|s| s.to_str()) { @@ -159,17 +160,11 @@ fn main() -> Result<()> { "--target" => { // Skip "--target" args.next().unwrap(); - - // Check that there is a target triple, and that it is unicode. - target = if let Some(value) = args.peek() { - let target_str = value - .clone() - .into_string() - .map_err(|_| anyhow!("target triple is not UTF-8"))?; - Some(target_str) - } else { - bail!("no target triple found") - } + // Next argument is the target triple. + let val = args.peek().ok_or_else(|| { + anyhow!("`--target` must be followed by target triple") + })?; + target = Some(val.to_owned()); } // Only parse the leading flags. _ => break, @@ -179,8 +174,6 @@ fn main() -> Result<()> { args.next().unwrap(); } - // Prepend a "--" so that the rest of the arguments are passed to the test driver. - let args = std::iter::once(OsString::from("--")).chain(args); Command::Test { bless, flags: args.collect(), target } } Some("run") => { @@ -217,7 +210,29 @@ fn main() -> Result<()> { Some("clippy") => Command::Clippy { flags: args.collect() }, Some("cargo") => Command::Cargo { flags: args.collect() }, Some("install") => Command::Install { flags: args.collect() }, - Some("bench") => Command::Bench { benches: args.collect() }, + Some("bench") => { + let mut target = None; + while let Some(arg) = args.peek().and_then(|s| s.to_str()) { + match arg { + "--target" => { + // Skip "--target" + args.next().unwrap(); + // Next argument is the target triple. + let val = args.peek().ok_or_else(|| { + anyhow!("`--target` must be followed by target triple") + })?; + target = Some(val.to_owned()); + } + // Only parse the leading flags. + _ => break, + } + + // Consume the flag, look at the next one. + args.next().unwrap(); + } + + Command::Bench { target, benches: args.collect() } + } Some("toolchain") => Command::Toolchain { flags: args.collect() }, Some("rustc-pull") => { let commit = args.next().map(|a| a.to_string_lossy().into_owned()); diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index 23b5e936edd81..2b5791f3ea53d 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -27,6 +27,30 @@ pub fn flagsplit(flags: &str) -> Vec { flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect() } +pub fn arg_flag_value( + args: impl IntoIterator>, + flag: &str, +) -> Option { + let mut args = args.into_iter(); + while let Some(arg) = args.next() { + let arg = arg.as_ref(); + if arg == "--" { + return None; + } + let Some(arg) = arg.to_str() else { + // Skip non-UTF-8 arguments. + continue; + }; + if arg == flag { + // Next one is the value. + return Some(args.next()?.as_ref().to_owned()); + } else if let Some(val) = arg.strip_prefix(flag).and_then(|s| s.strip_prefix("=")) { + return Some(val.to_owned().into()); + } + } + None +} + /// Some extra state we track for building Miri, such as the right RUSTFLAGS. pub struct MiriEnv { /// miri_dir is the root of the miri repository checkout we are working in. diff --git a/src/tools/miri/test-cargo-miri/run-test.py b/src/tools/miri/test-cargo-miri/run-test.py index 2639d29b73de6..8bab66f3af9d8 100755 --- a/src/tools/miri/test-cargo-miri/run-test.py +++ b/src/tools/miri/test-cargo-miri/run-test.py @@ -10,6 +10,7 @@ import re import subprocess import sys +import argparse CGREEN = '\33[32m' CBOLD = '\33[1m' @@ -25,8 +26,8 @@ def cargo_miri(cmd, quiet = True): args = ["cargo", "miri", cmd] + CARGO_EXTRA_FLAGS if quiet: args += ["-q"] - if 'MIRI_TEST_TARGET' in os.environ: - args += ["--target", os.environ['MIRI_TEST_TARGET']] + if ARGS.target: + args += ["--target", ARGS.target] return args def normalize_stdout(str): @@ -133,7 +134,7 @@ def test_cargo_miri_run(): def test_cargo_miri_test(): # rustdoc is not run on foreign targets - is_foreign = 'MIRI_TEST_TARGET' in os.environ + is_foreign = ARGS.target is not None default_ref = "test.cross-target.stdout.ref" if is_foreign else "test.default.stdout.ref" filter_ref = "test.filter.cross-target.stdout.ref" if is_foreign else "test.filter.stdout.ref" @@ -182,16 +183,21 @@ def test_cargo_miri_test(): env={'MIRIFLAGS': "-Zmiri-permissive-provenance"}, ) +args_parser = argparse.ArgumentParser(description='`cargo miri` testing') +args_parser.add_argument('--target', help='the target to test') +ARGS = args_parser.parse_args() + os.chdir(os.path.dirname(os.path.realpath(__file__))) os.environ["CARGO_TARGET_DIR"] = "target" # this affects the location of the target directory that we need to check os.environ["RUST_TEST_NOCAPTURE"] = "0" # this affects test output, so make sure it is not set os.environ["RUST_TEST_THREADS"] = "1" # avoid non-deterministic output due to concurrent test runs -target_str = " for target {}".format(os.environ['MIRI_TEST_TARGET']) if 'MIRI_TEST_TARGET' in os.environ else "" +target_str = " for target {}".format(ARGS.target) if ARGS.target else "" print(CGREEN + CBOLD + "## Running `cargo miri` tests{}".format(target_str) + CEND) test_cargo_miri_run() test_cargo_miri_test() + # Ensure we did not create anything outside the expected target dir. for target_dir in ["target", "custom-run", "custom-test", "config-cli"]: if os.listdir(target_dir) != ["miri"]: From cb448439006c2cd068b4ab667175fdcbe929f0ec Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 9 May 2024 11:21:10 +0200 Subject: [PATCH 2/2] make RUSTC_BLESS entirely an internal thing --- src/tools/miri/CONTRIBUTING.md | 2 ++ src/tools/miri/README.md | 2 -- src/tools/miri/test-cargo-miri/run-test.py | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 5a747e7935e34..092ad46a7cad3 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -322,3 +322,5 @@ binaries, and as such worth documenting: perform verbose logging. * `MIRI_HOST_SYSROOT` is set by bootstrap to tell `cargo-miri` which sysroot to use for *host* operations. +* `RUSTC_BLESS` is set by `./miri test` (and `./x.py test miri`) to indicate bless-mode to the test + harness. diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 1bdfc4ac1a78f..92cb7b37820b9 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -468,8 +468,6 @@ by all intended entry points, i.e. `cargo miri` and `./miri {test,run}`): * `MIRI_NO_STD` makes sure that the target's sysroot is built without libstd. This allows testing and running no_std programs. (Miri has a heuristic to detect no-std targets based on the target name; this environment variable is only needed when that heuristic fails.) -* `RUSTC_BLESS` (recognized by `./miri test` and `cargo-miri-test/run-test.py`): overwrite all - `stderr` and `stdout` files instead of checking whether the output matches. * `MIRI_SKIP_UI_CHECKS` (recognized by `./miri test`): don't check whether the `stderr` or `stdout` files match the actual output. diff --git a/src/tools/miri/test-cargo-miri/run-test.py b/src/tools/miri/test-cargo-miri/run-test.py index 8bab66f3af9d8..83f3e4c919b9a 100755 --- a/src/tools/miri/test-cargo-miri/run-test.py +++ b/src/tools/miri/test-cargo-miri/run-test.py @@ -36,7 +36,7 @@ def normalize_stdout(str): return str def check_output(actual, path, name): - if os.environ.get("RUSTC_BLESS", "0") != "0": + if ARGS.bless: # Write the output only if bless is set open(path, mode='w').write(actual) return True @@ -185,6 +185,7 @@ def test_cargo_miri_test(): args_parser = argparse.ArgumentParser(description='`cargo miri` testing') args_parser.add_argument('--target', help='the target to test') +args_parser.add_argument('--bless', help='bless the reference files', action='store_true') ARGS = args_parser.parse_args() os.chdir(os.path.dirname(os.path.realpath(__file__)))