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

Miri subtree update

r? `@ghost`

Author: bors

Cargo.lock

@@ -3308,13 +3308,14 @@ dependencies = [
 
 [[package]]
 name = "rustc-build-sysroot"
-version = "0.4.4"
+version = "0.4.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "39dcf8d82b1f79a179bdb284dc44db440a9666eefa5a6df5ef282d6db930d544"
+checksum = "a26170e1d79ea32f7ccec3188dd13cfc1f18c82764a9cbc1071667c0f865a4ea"
 dependencies = [
  "anyhow",
  "rustc_version",
  "tempfile",
+ "walkdir",
 ]
 
 [[package]]

src/tools/miri/Cargo.lock

@@ -279,7 +279,7 @@ Miri builds and vice-versa.
 
 You may be running `cargo miri` with a different compiler version than the one
 used to build the custom libstd that Miri uses, and Miri failed to detect that.
-Try deleting `~/.cache/miri`.
+Try running `cargo miri clean`.
 
 #### "no mir for `std::rt::lang_start_internal`"
 
@@ -465,7 +465,7 @@ Moreover, Miri recognizes some environment variables:
   must point to the `library` subdirectory of a `rust-lang/rust` repository
   checkout. Note that changing files in that directory does not automatically
   trigger a re-build of the standard library; you have to clear the Miri build
-  cache manually (on Linux, `rm -rf ~/.cache/miri`;
+  cache with `cargo miri clean` or deleting it manually (on Linux, `rm -rf ~/.cache/miri`;
   on Windows, `rmdir /S "%LOCALAPPDATA%\rust-lang\miri\cache"`;
   and on macOS, `rm -rf ~/Library/Caches/org.rust-lang.miri`).
 * `MIRI_SYSROOT` (recognized by `cargo miri` and the Miri driver) indicates the sysroot to use. When

src/tools/miri/README.md

@@ -20,6 +20,7 @@ Subcommands:
     test, t                  Run tests
     nextest                  Run tests with nextest (requires cargo-nextest installed)
     setup                    Only perform automatic setup, but without asking questions (for getting a proper libstd)
+    clean                    Clean the Miri cache & target directory
 
 The cargo options are exactly the same as for `cargo run` and `cargo test`, respectively.
 
@@ -74,14 +75,15 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
     // We cannot know which of those flags take arguments and which do not,
     // so we cannot detect subcommands later.
     let Some(subcommand) = args.next() else {
-        show_error!("`cargo miri` needs to be called with a subcommand (`run`, `test`)");
+        show_error!("`cargo miri` needs to be called with a subcommand (`run`, `test`, `clean`)");
     };
     let subcommand = match &*subcommand {
         "setup" => MiriCommand::Setup,
         "test" | "t" | "run" | "r" | "nextest" => MiriCommand::Forward(subcommand),
+        "clean" => MiriCommand::Clean,
         _ =>
             show_error!(
-                "`cargo miri` supports the following subcommands: `run`, `test`, `nextest`, and `setup`."
+                "`cargo miri` supports the following subcommands: `run`, `test`, `nextest`, `clean`, and `setup`."
             ),
     };
     let verbose = num_arg_flag("-v");
@@ -93,6 +95,16 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
     let target = get_arg_flag_value("--target");
     let target = target.as_ref().unwrap_or(host);
 
+    // If cleaning the the target directory & sysroot cache,
+    // delete them then exit. There is no reason to setup a new
+    // sysroot in this execution.
+    if let MiriCommand::Clean = subcommand {
+        let metadata = get_cargo_metadata();
+        clean_target_dir(&metadata);
+        clean_sysroot();
+        return;
+    }
+
     // We always setup.
     let miri_sysroot = setup(&subcommand, target, &rustc_version, verbose);
 
@@ -110,6 +122,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
     let cargo_cmd = match subcommand {
         MiriCommand::Forward(s) => s,
         MiriCommand::Setup => return, // `cargo miri setup` stops here.
+        MiriCommand::Clean => unreachable!(),
     };
     let metadata = get_cargo_metadata();
     let mut cmd = cargo();
@@ -142,11 +155,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
         .arg(format!("target.'cfg(all())'.runner=[{cargo_miri_path_for_toml}, 'runner']"));
 
     // Set `--target-dir` to `miri` inside the original target directory.
-    let mut target_dir = match get_arg_flag_value("--target-dir") {
-        Some(dir) => PathBuf::from(dir),
-        None => metadata.target_directory.clone().into_std_path_buf(),
-    };
-    target_dir.push("miri");
+    let target_dir = get_target_dir(&metadata);
     cmd.arg("--target-dir").arg(target_dir);
 
     // *After* we set all the flags that need setting, forward everything else. Make sure to skip

src/tools/miri/cargo-miri/Cargo.lock

@@ -67,13 +67,8 @@ pub fn setup(
     }
 
     // Determine where to put the sysroot.
-    let sysroot_dir = match std::env::var_os("MIRI_SYSROOT") {
-        Some(dir) => PathBuf::from(dir),
-        None => {
-            let user_dirs = directories::ProjectDirs::from("org", "rust-lang", "miri").unwrap();
-            user_dirs.cache_dir().to_owned()
-        }
-    };
+    let sysroot_dir = get_sysroot_dir();
+
     // Sysroot configuration and build details.
     let no_std = match std::env::var_os("MIRI_NO_STD") {
         None =>

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

@@ -74,6 +74,8 @@ pub enum MiriCommand {
     Setup,
     /// A command to be forwarded to cargo.
     Forward(String),
+    /// Clean the miri cache
+    Clean,
 }
 
 /// Escapes `s` in a way that is suitable for using it as a string literal in TOML syntax.
@@ -249,3 +251,65 @@ pub fn debug_cmd(prefix: &str, verbose: usize, cmd: &Command) {
     }
     eprintln!("{prefix} running command: {cmd:?}");
 }
+
+/// Get the target directory for miri output.
+///
+/// Either in an argument passed-in, or from cargo metadata.
+pub fn get_target_dir(meta: &Metadata) -> PathBuf {
+    let mut output = match get_arg_flag_value("--target-dir") {
+        Some(dir) => PathBuf::from(dir),
+        None => meta.target_directory.clone().into_std_path_buf(),
+    };
+    output.push("miri");
+    output
+}
+
+/// Determines where the sysroot of this exeuction is
+///
+/// Either in a user-specified spot by an envar, or in a default cache location.
+pub fn get_sysroot_dir() -> PathBuf {
+    match std::env::var_os("MIRI_SYSROOT") {
+        Some(dir) => PathBuf::from(dir),
+        None => {
+            let user_dirs = directories::ProjectDirs::from("org", "rust-lang", "miri").unwrap();
+            user_dirs.cache_dir().to_owned()
+        }
+    }
+}
+
+/// An idempotent version of the stdlib's remove_dir_all
+/// it is considered a success if the directory was not there.
+fn remove_dir_all_idem(dir: &Path) -> std::io::Result<()> {
+    match std::fs::remove_dir_all(dir) {
+        Ok(_) => Ok(()),
+        // If the directory doesn't exist, it is still a success.
+        Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(()),
+        Err(err) => Err(err),
+    }
+}
+
+/// Deletes the Miri sysroot cache
+/// Returns an error if the MIRI_SYSROOT env var is set.
+pub fn clean_sysroot() {
+    if std::env::var_os("MIRI_SYSROOT").is_some() {
+        show_error!(
+            "MIRI_SYSROOT is set. Please clean your custom sysroot cache directory manually."
+        )
+    }
+
+    let sysroot_dir = get_sysroot_dir();
+
+    eprintln!("Cleaning sysroot cache at {}", sysroot_dir.display());
+
+    // Keep it simple, just remove the directory.
+    remove_dir_all_idem(&sysroot_dir).unwrap_or_else(|err| show_error!("{}", err));
+}
+
+/// Deletes the Miri target directory
+pub fn clean_target_dir(meta: &Metadata) {
+    let target_dir = get_target_dir(meta);
+
+    eprintln!("Cleaning target directory at {}", target_dir.display());
+
+    remove_dir_all_idem(&target_dir).unwrap_or_else(|err| show_error!("{}", err))
+}

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

@@ -0,0 +1,10 @@
+:: This makes execution of ./miri on Linux and Windows the same.
+:: Windows will not execute the bash script, and select this.
+@echo off
+set MIRI_SCRIPT_TARGET_DIR=%0\..\miri-script\target
+cargo build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml
+
+:: Forwards all arguments to this file to the executable.
+:: We invoke the binary directly to avoid going through rustup, which would set some extra
+:: env vars that we do not want.
+%MIRI_SCRIPT_TARGET_DIR%\debug\miri-script %*

src/tools/miri/miri-script/Cargo.lock

@@ -1 +1 @@
-4316d0c6252cb1f833e582dfa68adb98efd5ddfb
+c5f69bdd5173a948e0131f934fa7c4cbf5e0b55f

src/tools/miri/miri.bat

@@ -99,18 +99,15 @@ fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) ->
     };
 
     if with_dependencies {
+        // Set the `cargo-miri` binary, which we expect to be in the same folder as the `miri` binary.
+        // (It's a separate crate, so we don't get an env var from cargo.)
+        let mut prog = miri_path();
+        prog.set_file_name("cargo-miri");
+        config.dependency_builder.program = prog;
+        let builder_args = ["miri", "run"]; // There is no `cargo miri build` so we just use `cargo miri run`.
+        config.dependency_builder.args = builder_args.into_iter().map(Into::into).collect();
         config.dependencies_crate_manifest_path =
             Some(Path::new("test_dependencies").join("Cargo.toml"));
-        let mut builder_args = vec!["run".into()];
-        builder_args.extend(flagsplit(&env::var("CARGO_EXTRA_FLAGS").unwrap_or_default()));
-        builder_args.extend([
-            "--manifest-path".into(),
-            "cargo-miri/Cargo.toml".into(),
-            "--".into(),
-            "miri".into(),
-            "run".into(), // There is no `cargo miri build` so we just use `cargo miri run`.
-        ]);
-        config.dependency_builder.args = builder_args.into_iter().map(Into::into).collect();
         // Reset `RUSTFLAGS` to work around <https://github.com/rust-lang/rust/pull/119574#issuecomment-1876878344>.
         config.dependency_builder.envs.push(("RUSTFLAGS".into(), None));
     }

src/tools/miri/rust-version

@@ -155,8 +155,9 @@ fn test_mremap() {
 
     // Test all of our error conditions
     // Not aligned
-    let ptr =
-        unsafe { libc::mremap(ptr::without_provenance_mut(1), page_size, page_size, libc::MREMAP_MAYMOVE) };
+    let ptr = unsafe {
+        libc::mremap(ptr::without_provenance_mut(1), page_size, page_size, libc::MREMAP_MAYMOVE)
+    };
     assert_eq!(ptr, libc::MAP_FAILED);
     assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL);
 

src/tools/miri/tests/compiletest.rs

@@ -4,8 +4,8 @@
 #![feature(sync_unsafe_cell)]
 
 use std::cell::SyncUnsafeCell;
-use std::thread;
-use std::{mem, ptr};
+use std::mem::MaybeUninit;
+use std::{mem, ptr, thread};
 
 fn main() {
     test_mutex_libc_init_recursive();
@@ -15,9 +15,10 @@ fn main() {
     #[cfg(target_os = "linux")]
     test_mutex_libc_static_initializer_recursive();
 
-    test_mutex();
+    check_mutex();
     check_rwlock_write();
     check_rwlock_read_no_deadlock();
+    check_cond();
 }
 
 fn test_mutex_libc_init_recursive() {
@@ -119,7 +120,7 @@ impl<T> Clone for SendPtr<T> {
     }
 }
 
-fn test_mutex() {
+fn check_mutex() {
     // Specifically *not* using `Arc` to make sure there is no synchronization apart from the mutex.
     unsafe {
         let data = SyncUnsafeCell::new((libc::PTHREAD_MUTEX_INITIALIZER, 0));
@@ -213,6 +214,53 @@ fn check_rwlock_read_no_deadlock() {
     }
 }
 
+fn check_cond() {
+    unsafe {
+        let mut cond: MaybeUninit<libc::pthread_cond_t> = MaybeUninit::uninit();
+        assert_eq!(libc::pthread_cond_init(cond.as_mut_ptr(), ptr::null()), 0);
+        let cond = SendPtr { ptr: cond.as_mut_ptr() };
+
+        let mut mutex: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER;
+        let mutex = SendPtr { ptr: &mut mutex };
+
+        let mut data = 0;
+        let data = SendPtr { ptr: &mut data };
+
+        let t = thread::spawn(move || {
+            let mutex = mutex; // circumvent per-field closure capture
+            let cond = cond;
+            let data = data;
+            assert_eq!(libc::pthread_mutex_lock(mutex.ptr), 0);
+            assert!(data.ptr.read() == 0);
+            data.ptr.write(1);
+            libc::pthread_cond_wait(cond.ptr, mutex.ptr);
+            assert!(data.ptr.read() == 3);
+            data.ptr.write(4);
+            assert_eq!(libc::pthread_mutex_unlock(mutex.ptr), 0);
+        });
+
+        thread::yield_now();
+
+        assert_eq!(libc::pthread_mutex_lock(mutex.ptr), 0);
+        assert!(data.ptr.read() == 1);
+        data.ptr.write(2);
+        assert_eq!(libc::pthread_cond_signal(cond.ptr), 0);
+        thread::yield_now(); // the other thread wakes up but can't get the lock yet
+        assert!(data.ptr.read() == 2);
+        data.ptr.write(3);
+        assert_eq!(libc::pthread_mutex_unlock(mutex.ptr), 0);
+
+        thread::yield_now(); // now the other thread gets the lock back
+
+        assert_eq!(libc::pthread_mutex_lock(mutex.ptr), 0);
+        assert!(data.ptr.read() == 4);
+        assert_eq!(libc::pthread_cond_broadcast(cond.ptr), 0); // just a smoke test
+        assert_eq!(libc::pthread_mutex_unlock(mutex.ptr), 0);
+
+        t.join().unwrap();
+    }
+}
+
 // std::sync::RwLock does not even used pthread_rwlock any more.
 // Do some smoke testing of the API surface.
 fn test_rwlock_libc_static_initializer() {

src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond.rs renamed to src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_timedwait.rs

@@ -63,10 +63,10 @@ fn check_conditional_variables_timed_wait_timeout() {
     let cvar = Condvar::new();
     let guard = lock.lock().unwrap();
     let now = Instant::now();
-    let (_guard, timeout) = cvar.wait_timeout(guard, Duration::from_millis(100)).unwrap();
+    let (_guard, timeout) = cvar.wait_timeout(guard, Duration::from_millis(10)).unwrap();
     assert!(timeout.timed_out());
     let elapsed_time = now.elapsed().as_millis();
-    assert!(100 <= elapsed_time && elapsed_time <= 1000);
+    assert!(10 <= elapsed_time && elapsed_time <= 1000);
 }
 
 /// Test that signaling a conditional variable when waiting with a timeout works
@@ -79,7 +79,7 @@ fn check_conditional_variables_timed_wait_notimeout() {
     let guard = lock.lock().unwrap();
 
     let handle = thread::spawn(move || {
-        thread::sleep(Duration::from_millis(100)); // Make sure the other thread is waiting by the time we call `notify`.
+        thread::sleep(Duration::from_millis(1)); // Make sure the other thread is waiting by the time we call `notify`.
         let (_lock, cvar) = &*pair2;
         cvar.notify_one();
     });

src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_isolated.rs renamed to src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_timedwait_isolated.rs

@@ -7,10 +7,9 @@
 // Miri does not implement the codegen-time hack that backs `#[rustc_inherit_overflow_checks]`.
 // use std::ops::*;
 
-
 // Disable _compile-time_ overflow linting
 // so that we can test runtime overflow checks
- #![allow(arithmetic_overflow)]
+#![allow(arithmetic_overflow)]
 
 fn main() {
     assert_eq!(-{ -0x80i8 }, -0x80);

src/tools/miri/tests/pass-dep/shims/mmap.rs

@@ -268,15 +268,11 @@ fn simd_mask() {
     }
 
     // This used to cause an ICE. It exercises simd_select_bitmask with an array as input.
-    if cfg!(target_endian = "little") {
-        // FIXME this test currently fails on big-endian:
-        // <https://github.com/rust-lang/portable-simd/issues/379>
-        let bitmask = u8x4::from_array([0b00001101, 0, 0, 0]);
-        assert_eq!(
-            mask32x4::from_bitmask_vector(bitmask),
-            mask32x4::from_array([true, false, true, true]),
-        );
-    }
+    let bitmask = u8x4::from_array([0b00001101, 0, 0, 0]);
+    assert_eq!(
+        mask32x4::from_bitmask_vector(bitmask),
+        mask32x4::from_array([true, false, true, true]),
+    );
     let bitmask = u8x8::from_array([0b01000101, 0, 0, 0, 0, 0, 0, 0]);
     assert_eq!(
         mask32x8::from_bitmask_vector(bitmask),

src/tools/miri/tests/pass-dep/shims/pthread-sync.rs

@@ -0,0 +1,41 @@
+//@only-target-windows: this directly tests windows only random functions
+use core::ffi::c_void;
+use core::mem::size_of_val;
+use core::ptr::null_mut;
+
+// Windows API definitions.
+type NTSTATUS = i32;
+type BOOLEAN = u8;
+const BCRYPT_USE_SYSTEM_PREFERRED_RNG: u32 = 0x00000002;
+const BCRYPT_RNG_ALG_HANDLE: *mut c_void = 0x81 as *mut c_void;
+#[link(name = "bcrypt")]
+extern "system" {
+    fn BCryptGenRandom(
+        halgorithm: *mut c_void,
+        pbbuffer: *mut u8,
+        cbbuffer: u32,
+        dwflags: u32,
+    ) -> NTSTATUS;
+}
+#[link(name = "advapi32")]
+extern "system" {
+    #[link_name = "SystemFunction036"]
+    fn RtlGenRandom(RandomBuffer: *mut u8, RandomBufferLength: u32) -> BOOLEAN;
+}
+
+fn main() {
+    let mut key = [0u8; 24];
+    let len: u32 = size_of_val(&key).try_into().unwrap();
+    let ret = unsafe {
+        BCryptGenRandom(null_mut(), key.as_mut_ptr(), len, BCRYPT_USE_SYSTEM_PREFERRED_RNG)
+    };
+    // NTSTATUS codes use the high bit to indicate an error
+    assert!(ret >= 0);
+
+    let ret = unsafe { BCryptGenRandom(BCRYPT_RNG_ALG_HANDLE, key.as_mut_ptr(), len, 0) };
+    assert!(ret >= 0);
+
+    let ret = unsafe { RtlGenRandom(key.as_mut_ptr(), len) };
+    // RtlGenRandom returns a BOOLEAN where 0 indicates an error
+    assert_ne!(ret, 0);
+}

src/tools/miri/tests/pass/concurrency/sync.rs

@@ -29,7 +29,8 @@ fn slice_of_zst() {
 
     // In a slice of zero-size elements the pointer is meaningless.
     // Ensure iteration still works even if the pointer is at the end of the address space.
-    let slice: &[()] = unsafe { slice::from_raw_parts(ptr::without_provenance(-5isize as usize), 10) };
+    let slice: &[()] =
+        unsafe { slice::from_raw_parts(ptr::without_provenance(-5isize as usize), 10) };
     assert_eq!(slice.len(), 10);
     assert_eq!(slice.iter().count(), 10);