Auto merge of #123041 - RalfJung:miri, r=RalfJung

Miri subtree update

r? `@ghost`

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:

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 <filename>``.  
+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",
     ],
 }
 ```

src/tools/miri/cargo-miri/src/main.rs

@@ -11,10 +11,18 @@
 
 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)
         }
-        _ => {
+        _ if looks_like_rustdoc() => {
-            // Everything else must be rustdoc. But we need to get `first` "back onto the iterator",
+            // 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()),
+            );
+        }
     }
 }

src/tools/miri/cargo-miri/src/phases.rs

@@ -620,7 +620,7 @@ pub fn phase_rustdoc(mut args: impl Iterator<Item = String>) {
 
     // 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());

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
+  # <https://github.com/rust-lang/miri/pull/3402>). 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

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 "$@"

src/tools/miri/miri-script/src/commands.rs

@@ -178,7 +178,7 @@ fn toolchain(flags: Vec<OsString>) -> Result<()> {
             .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 @@ fn toolchain(flags: Vec<OsString>) -> Result<()> {
             }
         };
         // 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 @@ fn toolchain(flags: Vec<OsString>) -> Result<()> {
         // 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 @@ fn many_seeds(command: Vec<OsString>) -> Result<()> {
                 .env("MIRIFLAGS", miriflags)
                 .quiet()
                 .run();
-            if status.is_err() {
+            if let Err(err) = status {
                 println!("Failing seed: {seed}");
-                break;
+                return Err(err.into());
             }
         }
         Ok(())

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

src/tools/miri/rust-version

@@ -1 +1 @@
-c3b05c6e5b5b59613350b8c2875b0add67ed74df
+cb7c63606e53715f94f3ba04d38e50772e4cd23d

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(|_| {

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",

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"

src/tools/miri/tests/pass-dep/shims/libc-fs.rs

@@ -16,6 +16,10 @@
 fn main() {
     test_dup_stdout_stderr();
     test_canonicalize_too_long();
+    test_rename();
+    test_ftruncate::<libc::off_t>(libc::ftruncate);
+    #[cfg(target_os = "linux")]
+    test_ftruncate::<libc::off64_t>(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<T: From<i32>>(
+    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};

src/tools/miri/tests/ui.rs

@@ -4,8 +4,11 @@
 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::color_eyre::eyre::{Context, Result};
-use ui_test::{status_emitter, CommandBuilder, Format, RustfixMode};
+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 {