From 6580a22726abf9544c0752a3cd26860643b8aa5d Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Wed, 8 May 2024 14:43:33 -0400 Subject: [PATCH 1/4] Allow test targets to be set via CLI args --- src/tools/miri/miri-script/src/commands.rs | 57 +++++++++++++++------- src/tools/miri/miri-script/src/main.rs | 41 +++++++++++++--- 2 files changed, 75 insertions(+), 23 deletions(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 6d07455c0de..968dc62cd98 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -23,7 +23,9 @@ const JOSH_PORT: &str = "42042"; impl MiriEnv { /// Returns the location of the sysroot. - fn build_miri_sysroot(&mut self, quiet: bool) -> Result { + /// + /// 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 { if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") { // Sysroot already set, use that. return Ok(miri_sysroot.into()); @@ -35,26 +37,27 @@ impl MiriEnv { self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?; self.build(&manifest_path, &[], quiet)?; - let target = &match self.sh.var("MIRI_TEST_TARGET") { - Ok(target) => vec!["--target".into(), target], - Err(_) => vec![], - }; + let target_flag = + &if let Some(target) = target { vec!["--target", target] } else { vec![] }; + if !quiet { - match self.sh.var("MIRI_TEST_TARGET") { - Ok(target) => eprintln!("$ (building Miri sysroot for {target})"), - Err(_) => eprintln!("$ (building Miri sysroot)"), + if let Some(target) = target { + eprintln!("$ (building Miri sysroot for {target})"); + } else { + eprintln!("$ (building Miri sysroot)"); } } + let output = cmd!(self.sh, "cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- - miri setup --print-sysroot {target...}" + miri setup --print-sysroot {target_flag...}" ).read(); let Ok(output) = output else { // Run it again (without `--print-sysroot` or `--quiet`) so the user can see the error. cmd!( self.sh, "cargo +{toolchain} run {cargo_extra_flags...} --manifest-path {manifest_path} -- - miri setup {target...}" + miri setup {target_flag...}" ) .run() .with_context(|| "`cargo miri setup` failed")?; @@ -161,7 +164,7 @@ impl Command { Command::Install { flags } => Self::install(flags), Command::Build { flags } => Self::build(flags), Command::Check { flags } => Self::check(flags), - Command::Test { bless, flags } => Self::test(bless, flags), + Command::Test { bless, flags, target } => Self::test(bless, flags, target), Command::Run { dep, verbose, many_seeds, flags } => Self::run(dep, verbose, many_seeds, flags), Command::Fmt { flags } => Self::fmt(flags), @@ -446,16 +449,23 @@ impl Command { Ok(()) } - fn test(bless: bool, flags: Vec) -> Result<()> { + fn test(bless: bool, 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)?; + 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. if bless { e.sh.set_var("RUSTC_BLESS", "Gesundheit"); } + e.test(path!(e.miri_dir / "Cargo.toml"), &flags)?; Ok(()) } @@ -476,14 +486,26 @@ impl Command { .take_while(|arg| *arg != "--") .tuple_windows() .find(|(first, _)| *first == "--target"); - if let Some((_, target)) = 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!("invalid target triple encoding"))?; + 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.into()); - } + flags.push(target.clone().into()); + + Some(target) + } else { + None + }; + // 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"); @@ -492,7 +514,8 @@ impl Command { } // Prepare a sysroot, and add it to the flags. - let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose)?; + let miri_sysroot = + e.build_miri_sysroot(/* quiet */ !verbose, target_triple.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 f0ebbc84690..c92513a0fa7 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -33,6 +33,9 @@ pub enum Command { 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, }, /// Build miri, set up a sysroot and then run the driver with the given . /// (Also respects MIRIFLAGS environment variable.) @@ -84,9 +87,9 @@ Just build miri. are passed to `cargo build`. ./miri check : Just check miri. are passed to `cargo check`. -./miri test [--bless] : +./miri test [--bless] [--target] : Build miri, set up a sysroot and then run the test suite. are passed -to the final `cargo test` invocation. +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 . @@ -147,12 +150,38 @@ fn main() -> Result<()> { Some("build") => Command::Build { flags: args.collect() }, Some("check") => Command::Check { flags: args.collect() }, Some("test") => { - let bless = args.peek().is_some_and(|a| a.to_str() == Some("--bless")); - if bless { - // Consume the flag. + let mut target = std::env::var("MIRI_TEST_TARGET").ok(); + let mut bless = false; + + while let Some(arg) = args.peek().and_then(|s| s.to_str()) { + match arg { + "--bless" => bless = true, + "--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!("invalid target triple encoding"))?; + Some(target_str) + } else { + bail!("no target triple found") + } + } + // Only parse the leading flags. + _ => break, + } + + // Consume the flag, look at the next one. args.next().unwrap(); } - Command::Test { bless, flags: args.collect() } + + // 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") => { let mut dep = false; From 6e564ed9fdce38a56efc3feeaccf171bc91d92c9 Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Wed, 8 May 2024 14:44:06 -0400 Subject: [PATCH 2/4] Update CI script for the miri-script test changes --- src/tools/miri/ci/ci.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index c28a6f183b3..18f1bf18c7d 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -59,7 +59,7 @@ 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 tests/{pass,panic} fi if [ -n "${MANY_SEEDS-}" ]; then # Also run some many-seeds tests. @@ -107,7 +107,7 @@ function run_tests_minimal { exit 1 fi - time ./miri test -- "$@" + time ./miri test "$@" # 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} From 620bf348e1d793e2b1e13ea0b4af52f883d911ae Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Wed, 8 May 2024 16:37:48 -0400 Subject: [PATCH 3/4] Update documentation for miri-script test changes --- src/tools/miri/CONTRIBUTING.md | 9 ++++----- src/tools/miri/README.md | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 60bc1d5282d..39aed51f5df 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -72,14 +72,13 @@ For example: You can (cross-)run the entire test suite using: -``` -./miri test -MIRI_TEST_TARGET=i686-unknown-linux-gnu ./miri test +```sh +./miri test --target i686-unknown-linux-gnu ``` `./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). These filters are passed -to `cargo test`, so for multiple filters you need to use `./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`. #### Fine grained logging diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 2c76749fbca..6e0c96499e2 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -464,7 +464,7 @@ by all intended entry points, i.e. `cargo miri` and `./miri {test,run}`): 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. `miri` and `cargo miri` accept the `--target` flag for the same + 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. From d43cb7121e8d651e9e2d4f48ead98c173f26af7d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 9 May 2024 09:18:14 +0200 Subject: [PATCH 4/4] minor tweaks --- src/tools/miri/CONTRIBUTING.md | 1 + src/tools/miri/miri-script/src/commands.rs | 6 ++---- src/tools/miri/miri-script/src/main.rs | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 39aed51f5df..57682e60c37 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -73,6 +73,7 @@ For example: You can (cross-)run the entire test suite using: ```sh +./miri test ./miri test --target i686-unknown-linux-gnu ``` diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 968dc62cd98..43abf180d3e 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -491,10 +491,8 @@ impl Command { // Found it! e.sh.set_var("MIRI_TEST_TARGET", target); - let triple = target - .clone() - .into_string() - .map_err(|_| anyhow!("invalid target triple encoding"))?; + 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`. diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs index c92513a0fa7..c19c4c91c65 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -87,7 +87,7 @@ Just build miri. are passed to `cargo build`. ./miri check : Just check miri. are passed to `cargo check`. -./miri test [--bless] [--target] : +./miri test [--bless] [--target ] : Build miri, set up a sysroot and then run the test suite. are passed to the test harness. @@ -165,7 +165,7 @@ fn main() -> Result<()> { let target_str = value .clone() .into_string() - .map_err(|_| anyhow!("invalid target triple encoding"))?; + .map_err(|_| anyhow!("target triple is not UTF-8"))?; Some(target_str) } else { bail!("no target triple found")