diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 1097fc6db7293..2c5868d5b37a3 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -30,11 +30,16 @@ jobs: host_target: i686-pc-windows-msvc runs-on: ${{ matrix.os }} env: - RUST_BACKTRACE: 1 HOST_TARGET: ${{ matrix.host_target }} steps: - uses: actions/checkout@v3 + - name: Show Rust version (stable toolchain) + run: | + rustup show + rustc -Vv + cargo -V + # Cache the global cargo directory, but NOT the local `target` directory which # we cannot reuse anyway when the nightly changes (and it grows quite large # over time). @@ -58,7 +63,7 @@ jobs: if: ${{ steps.cache.outputs.cache-hit != 'true' }} run: cargo install -f rustup-toolchain-install-master - - name: Install "master" toolchain + - name: Install miri toolchain run: | if [[ ${{ github.event_name }} == 'schedule' ]]; then echo "Building against latest rustc git version" @@ -66,13 +71,13 @@ jobs: fi ./miri toolchain --host ${{ matrix.host_target }} - - name: Show Rust version + - name: Show Rust version (miri toolchain) run: | rustup show rustc -Vv cargo -V - - name: Test + - name: Test Miri run: ./ci/ci.sh style: diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index f2f3a642e0a03..3416fb0d9ba09 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -64,19 +64,22 @@ For example, you can (cross-)run the driver on a particular file by doing ./miri run tests/pass/hello.rs --target i686-unknown-linux-gnu ``` -and you can (cross-)run the entire test suite using: +Tests in ``pass-dep`` need to be run using ``./miri run --dep ``. +For example: +```sh +./miri run --dep tests/pass-dep/shims/libc-fs.rs +``` + +You can (cross-)run the entire test suite using: ``` ./miri test MIRI_TEST_TARGET=i686-unknown-linux-gnu ./miri test ``` -If your target doesn't support libstd that should usually just work. However, if you are using a -custom target file, you might have to set `MIRI_NO_STD=1`. - `./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 filers you need to use `./miri test -- FILTER1 FILTER2`. +to `cargo test`, so for multiple filters you need to use `./miri test -- FILTER1 FILTER2`. #### Fine grained logging @@ -178,6 +181,7 @@ to `.vscode/settings.json` in your local Miri clone: "cargo", "clippy", // make this `check` when working with a locally built rustc "--message-format=json", + "--all-targets", ], // Contrary to what the name suggests, this also affects proc macros. "rust-analyzer.cargo.buildScripts.overrideCommand": [ @@ -187,6 +191,7 @@ to `.vscode/settings.json` in your local Miri clone: "cargo", "check", "--message-format=json", + "--all-targets", ], } ``` diff --git a/src/tools/miri/cargo-miri/src/main.rs b/src/tools/miri/cargo-miri/src/main.rs index c5fada6fe55b8..9fdd4c3e47043 100644 --- a/src/tools/miri/cargo-miri/src/main.rs +++ b/src/tools/miri/cargo-miri/src/main.rs @@ -11,10 +11,18 @@ use std::{env, iter}; use crate::phases::*; +/// Returns `true` if our flags look like they may be for rustdoc, i.e., this is cargo calling us to +/// be rustdoc. It's hard to be sure as cargo does not have a RUSTDOC_WRAPPER or an env var that +/// would let us get a clear signal. +fn looks_like_rustdoc() -> bool { + // The `--test-run-directory` flag only exists for rustdoc and cargo always passes it. Perfect! + env::args().any(|arg| arg == "--test-run-directory") +} + fn main() { // Rustc does not support non-UTF-8 arguments so we make no attempt either. // (We do support non-UTF-8 environment variables though.) - let mut args = std::env::args(); + let mut args = env::args(); // Skip binary name. args.next().unwrap(); @@ -91,10 +99,16 @@ fn main() { // (see https://github.com/rust-lang/cargo/issues/10886). phase_rustc(args, RustcPhase::Build) } - _ => { - // Everything else must be rustdoc. But we need to get `first` "back onto the iterator", + _ if looks_like_rustdoc() => { + // This is probably rustdoc. But we need to get `first` "back onto the iterator", // it is some part of the rustdoc invocation. phase_rustdoc(iter::once(first).chain(args)); } + _ => { + show_error!( + "`cargo-miri` failed to recognize which phase of the build process this is, please report a bug.\nThe command-line arguments were: {:#?}", + Vec::from_iter(env::args()), + ); + } } } diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 81ff68545ccd3..e547599d954a4 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -620,7 +620,7 @@ pub fn phase_rustdoc(mut args: impl Iterator) { // The `--test-builder` and `--runtool` arguments are unstable rustdoc features, // which are disabled by default. We first need to enable them explicitly: - cmd.arg("-Z").arg("unstable-options"); + cmd.arg("-Zunstable-options"); // rustdoc needs to know the right sysroot. cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 9d2c3f362e6d1..54c5d3087fde1 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -13,6 +13,15 @@ function endgroup { begingroup "Building Miri" +# Special Windows hacks +if [ "$HOST_TARGET" = i686-pc-windows-msvc ]; then + # The $BASH variable is `/bin/bash` here, but that path does not actually work. There are some + # hacks in place somewhere to try to paper over this, but the hacks dont work either (see + # ). So we hard-code the correct location for Github + # CI instead. + BASH="C:/Program Files/Git/usr/bin/bash" +fi + # Determine configuration for installed build echo "Installing release version of Miri" export RUSTFLAGS="-D warnings" @@ -58,12 +67,13 @@ function run_tests { MIRIFLAGS="${MIRIFLAGS:-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test -- tests/{pass,panic} # Also run some many-seeds tests. 64 seeds means this takes around a minute per test. + # (Need to invoke via explicit `bash -c` for Windows.) for FILE in tests/many-seeds/*.rs; do - MIRI_SEEDS=64 ./miri many-seeds ./miri run "$FILE" + MIRI_SEEDS=64 ./miri many-seeds "$BASH" -c "./miri run '$FILE'" done # Check that the benchmarks build and run, but without actually benchmarking. - HYPERFINE="bash -c" ./miri bench + HYPERFINE="'$BASH' -c" ./miri bench fi ## test-cargo-miri diff --git a/src/tools/miri/miri b/src/tools/miri/miri index 169f4521f2d7d..5f71fc9443839 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -3,5 +3,5 @@ set -e # Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through # rustup (that sets it's own environmental variables), which is undesirable. MIRI_SCRIPT_TARGET_DIR="$(dirname "$0")"/miri-script/target -cargo build $CARGO_EXTRA_FLAGS -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml +cargo +stable build $CARGO_EXTRA_FLAGS -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml "$MIRI_SCRIPT_TARGET_DIR"/debug/miri-script "$@" diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 7fff6ae80b429..66f323290b2b0 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -178,7 +178,7 @@ impl Command { .context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?; let sh = Shell::new()?; sh.change_dir(miri_dir()?); - let new_commit = Some(sh.read_file("rust-version")?.trim().to_owned()); + let new_commit = sh.read_file("rust-version")?.trim().to_owned(); let current_commit = { let rustc_info = cmd!(sh, "rustc +miri --version -v").read(); if rustc_info.is_err() { @@ -193,7 +193,7 @@ impl Command { } }; // Check if we already are at that commit. - if current_commit == new_commit { + if current_commit.as_ref() == Some(&new_commit) { if active_toolchain()? != "miri" { cmd!(sh, "rustup override set miri").run()?; } @@ -202,7 +202,7 @@ impl Command { // Install and setup new toolchain. cmd!(sh, "rustup toolchain uninstall miri").run()?; - cmd!(sh, "rustup-toolchain-install-master -n miri -c cargo -c rust-src -c rustc-dev -c llvm-tools -c rustfmt -c clippy {flags...} -- {new_commit...}").run()?; + cmd!(sh, "rustup-toolchain-install-master -n miri -c cargo -c rust-src -c rustc-dev -c llvm-tools -c rustfmt -c clippy {flags...} -- {new_commit}").run()?; cmd!(sh, "rustup override set miri").run()?; // Cleanup. cmd!(sh, "cargo clean").run()?; @@ -380,9 +380,9 @@ impl Command { .env("MIRIFLAGS", miriflags) .quiet() .run(); - if status.is_err() { + if let Err(err) = status { println!("Failing seed: {seed}"); - break; + return Err(err.into()); } } Ok(()) diff --git a/src/tools/miri/miri.bat b/src/tools/miri/miri.bat index 959e54d8844dc..18baa683f6517 100644 --- a/src/tools/miri/miri.bat +++ b/src/tools/miri/miri.bat @@ -5,7 +5,7 @@ set MIRI_SCRIPT_TARGET_DIR=%0\..\miri-script\target :: If any other steps are added, the "|| exit /b" must be appended to early :: return from the script. If not, it will continue execution. -cargo build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml || exit /b +cargo +stable build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml || exit /b :: Forwards all arguments to this file to the executable. :: We invoke the binary directly to avoid going through rustup, which would set some extra diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 9b89f016a7702..fa06a069d541e 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -c3b05c6e5b5b59613350b8c2875b0add67ed74df +cb7c63606e53715f94f3ba04d38e50772e4cd23d diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index c12382527e7bf..4904a489eae8f 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -276,7 +276,7 @@ fn run_compiler( // If no `--sysroot` is given, the `MIRI_SYSROOT` env var is consulted to find where // that sysroot lives, and that is passed to rustc. let sysroot_flag = "--sysroot"; - if !args.iter().any(|e| e == sysroot_flag) { + if !args.iter().any(|e| e.starts_with(sysroot_flag)) { // Using the built-in default here would be plain wrong, so we *require* // the env var to make sure things make sense. let miri_sysroot = env::var("MIRI_SYSROOT").unwrap_or_else(|_| { diff --git a/src/tools/miri/test-cargo-miri/Cargo.lock b/src/tools/miri/test-cargo-miri/Cargo.lock index d5e57a66a8d61..f75d68f4e2983 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.lock +++ b/src/tools/miri/test-cargo-miri/Cargo.lock @@ -2,6 +2,12 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "anyhow" +version = "1.0.81" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0952808a6c2afd1aa8947271f3a60f1a6763c7b912d210184c5149b5cf147247" + [[package]] name = "autocfg" version = "1.1.0" @@ -24,6 +30,7 @@ checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" name = "cargo-miri-test" version = "0.1.0" dependencies = [ + "anyhow", "autocfg", "byteorder 0.5.3", "byteorder 1.4.3", diff --git a/src/tools/miri/test-cargo-miri/Cargo.toml b/src/tools/miri/test-cargo-miri/Cargo.toml index 1688096fd9a4b..58c451741bb50 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.toml +++ b/src/tools/miri/test-cargo-miri/Cargo.toml @@ -22,6 +22,9 @@ issue_rust_86261 = { path = "issue-rust-86261" } byteorder_2 = { package = "byteorder", version = "0.5" } # to test dev-dependencies behave as expected, with renaming # Not actually used, but exercises some unique code path (`--extern` .so file). serde_derive = "1.0.185" +# Not actually used, but uses a custom build probe so let's make sure that works. +# (Ideally we'd check if the probe was successful, but that's not easily possible.) +anyhow = "1.0" [build-dependencies] autocfg = "1" diff --git a/src/tools/miri/tests/pass-dep/shims/libc-fs.rs b/src/tools/miri/tests/pass-dep/shims/libc-fs.rs index fafeb9e05585c..4cfc1843a7a2f 100644 --- a/src/tools/miri/tests/pass-dep/shims/libc-fs.rs +++ b/src/tools/miri/tests/pass-dep/shims/libc-fs.rs @@ -16,6 +16,10 @@ mod utils; fn main() { test_dup_stdout_stderr(); test_canonicalize_too_long(); + test_rename(); + test_ftruncate::(libc::ftruncate); + #[cfg(target_os = "linux")] + test_ftruncate::(libc::ftruncate64); test_readlink(); test_file_open_unix_allow_two_args(); test_file_open_unix_needs_three_args(); @@ -133,6 +137,65 @@ fn test_readlink() { assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound); } +fn test_rename() { + let path1 = prepare("miri_test_libc_fs_source.txt"); + let path2 = prepare("miri_test_libc_fs_rename_destination.txt"); + + let file = File::create(&path1).unwrap(); + drop(file); + + let c_path1 = CString::new(path1.as_os_str().as_bytes()).expect("CString::new failed"); + let c_path2 = CString::new(path2.as_os_str().as_bytes()).expect("CString::new failed"); + + // Renaming should succeed + unsafe { libc::rename(c_path1.as_ptr(), c_path2.as_ptr()) }; + // Check that old file path isn't present + assert_eq!(ErrorKind::NotFound, path1.metadata().unwrap_err().kind()); + // Check that the file has moved successfully + assert!(path2.metadata().unwrap().is_file()); + + // Renaming a nonexistent file should fail + let res = unsafe { libc::rename(c_path1.as_ptr(), c_path2.as_ptr()) }; + assert_eq!(res, -1); + assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound); + + remove_file(&path2).unwrap(); +} + +fn test_ftruncate>( + ftruncate: unsafe extern "C" fn(fd: libc::c_int, length: T) -> libc::c_int, +) { + // libc::off_t is i32 in target i686-unknown-linux-gnu + // https://docs.rs/libc/latest/i686-unknown-linux-gnu/libc/type.off_t.html + + let bytes = b"hello"; + let path = prepare("miri_test_libc_fs_ftruncate.txt"); + let mut file = File::create(&path).unwrap(); + file.write(bytes).unwrap(); + file.sync_all().unwrap(); + assert_eq!(file.metadata().unwrap().len(), 5); + + let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed"); + let fd = unsafe { libc::open(c_path.as_ptr(), libc::O_RDWR) }; + + // Truncate to a bigger size + let mut res = unsafe { ftruncate(fd, T::from(10)) }; + assert_eq!(res, 0); + assert_eq!(file.metadata().unwrap().len(), 10); + + // Write after truncate + file.write(b"dup").unwrap(); + file.sync_all().unwrap(); + assert_eq!(file.metadata().unwrap().len(), 10); + + // Truncate to smaller size + res = unsafe { ftruncate(fd, T::from(2)) }; + assert_eq!(res, 0); + assert_eq!(file.metadata().unwrap().len(), 2); + + remove_file(&path).unwrap(); +} + #[cfg(target_os = "linux")] fn test_o_tmpfile_flag() { use std::fs::{create_dir, OpenOptions}; diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 7f363ccdfe59c..129d1dfd73260 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -4,8 +4,11 @@ use std::ffi::OsString; use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::{env, process::Command}; -use ui_test::{color_eyre::Result, Config, Match, Mode, OutputConflictHandling}; -use ui_test::{status_emitter, CommandBuilder, Format, RustfixMode}; +use ui_test::color_eyre::eyre::{Context, Result}; +use ui_test::{ + status_emitter, CommandBuilder, Config, Format, Match, Mode, OutputConflictHandling, + RustfixMode, +}; fn miri_path() -> PathBuf { PathBuf::from(option_env!("MIRI").unwrap_or(env!("CARGO_BIN_EXE_miri"))) @@ -124,6 +127,9 @@ fn run_tests( // Let the tests know where to store temp files (they might run for a different target, which can make this hard to find). config.program.envs.push(("MIRI_TEMP".into(), Some(tmpdir.to_owned().into()))); + // If a test ICEs, we want to see a backtrace. + config.program.envs.push(("RUST_BACKTRACE".into(), Some("1".into()))); + // Handle command-line arguments. let args = ui_test::Args::test()?; let default_bless = env::var_os("RUSTC_BLESS").is_some_and(|v| v != "0"); @@ -223,7 +229,7 @@ fn ui( with_dependencies: Dependencies, tmpdir: &Path, ) -> Result<()> { - let msg = format!("## Running ui tests in {path} against miri for {target}"); + let msg = format!("## Running ui tests in {path} for {target}"); eprintln!("{}", msg.green().bold()); let with_dependencies = match with_dependencies { @@ -231,6 +237,7 @@ fn ui( WithoutDependencies => false, }; run_tests(mode, path, target, with_dependencies, tmpdir) + .with_context(|| format!("ui tests in {path} for {target} failed")) } fn get_target() -> String {