From 1a1d180effb3fc78b98d1a02979df80d96db6bca Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Tue, 3 Jun 2025 04:54:36 +0000 Subject: [PATCH 01/20] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 553d410b2bc7f..15f06b60d0f9c 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -337c11e5932275e7d450c1f2e26f289f0ddfa717 +99426c570eebec8dcba2eaa8f5057265346aaedc From 80a95b745e0d156fd9fce7cb66de0377a4df2781 Mon Sep 17 00:00:00 2001 From: Nia Espera Date: Mon, 2 Jun 2025 21:49:13 +0200 Subject: [PATCH 02/20] native-lib: allow multiple libraries and/or dirs --- src/tools/miri/README.md | 6 +- src/tools/miri/src/alloc_addresses/mod.rs | 4 +- src/tools/miri/src/bin/miri.rs | 19 ++++-- src/tools/miri/src/eval.rs | 7 +-- src/tools/miri/src/machine.rs | 14 ++--- src/tools/miri/src/shims/foreign_items.rs | 2 +- src/tools/miri/src/shims/native_lib.rs | 75 ++++++++++++----------- 7 files changed, 70 insertions(+), 57 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index de521393cd0ab..126a8dc47362a 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -398,9 +398,11 @@ to Miri failing to detect cases of undefined behavior in a program. **unsound** since the fallback body might not be checking for all UB. * `-Zmiri-native-lib=` is an experimental flag for providing support for calling native functions from inside the interpreter via FFI. The flag is supported only on - Unix systems. Functions not provided by that file are still executed via the usual Miri shims. + Unix systems. Functions not provided by that file are still executed via the usual Miri shims. If + a path to a directory is specified, all files in that directory are included nonrecursively. This + flag can be passed multiple times to specify multiple files and/or directories. **WARNING**: If an invalid/incorrect `.so` file is specified, this can cause Undefined Behavior in - Miri itself! And of course, Miri cannot do any checks on the actions taken by the native code. + Miri itself! And of course, Miri often cannot do any checks on the actions taken by the native code. Note that Miri has its own handling of file descriptors, so if you want to replace *some* functions working on file descriptors, you will have to replace *all* of them, or the two kinds of file descriptors will be mixed up. This is **work in progress**; currently, only integer and diff --git a/src/tools/miri/src/alloc_addresses/mod.rs b/src/tools/miri/src/alloc_addresses/mod.rs index 12a320b967678..4a038fe648735 100644 --- a/src/tools/miri/src/alloc_addresses/mod.rs +++ b/src/tools/miri/src/alloc_addresses/mod.rs @@ -132,7 +132,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { assert!(!matches!(info.kind, AllocKind::Dead)); // This allocation does not have a base address yet, pick or reuse one. - if this.machine.native_lib.is_some() { + if !this.machine.native_lib.is_empty() { // In native lib mode, we use the "real" address of the bytes for this allocation. // This ensures the interpreted program and native code have the same view of memory. let params = this.machine.get_default_alloc_params(); @@ -413,7 +413,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, MiriAllocBytes> { let this = self.eval_context_ref(); assert!(this.tcx.try_get_global_alloc(id).is_some()); - if this.machine.native_lib.is_some() { + if !this.machine.native_lib.is_empty() { // In native lib mode, MiriAllocBytes for global allocations are handled via `prepared_alloc_bytes`. // This additional call ensures that some `MiriAllocBytes` are always prepared, just in case // this function gets called before the first time `addr_from_alloc_id` gets called. diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 0121472d330f5..f8168853d3aaf 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -692,11 +692,18 @@ fn main() { }; } else if let Some(param) = arg.strip_prefix("-Zmiri-native-lib=") { let filename = param.to_string(); - if std::path::Path::new(&filename).exists() { - if let Some(other_filename) = miri_config.native_lib { - show_error!("-Zmiri-native-lib is already set to {}", other_filename.display()); + let file_path = std::path::Path::new(&filename); + if file_path.exists() { + // For directories, nonrecursively add all normal files inside + if let Ok(dir) = file_path.read_dir() { + for lib in dir.filter_map(|res| res.ok()) { + if lib.file_type().unwrap().is_file() { + miri_config.native_lib.push(lib.path().to_owned()); + } + } + } else { + miri_config.native_lib.push(filename.into()); } - miri_config.native_lib = Some(filename.into()); } else { show_error!("-Zmiri-native-lib `{}` does not exist", filename); } @@ -731,12 +738,12 @@ fn main() { "Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance" ); } - if miri_config.native_lib.is_some() { + if !miri_config.native_lib.is_empty() { show_error!("Tree Borrows is not compatible with calling native functions"); } } // Native calls and strict provenance are not compatible. - if miri_config.native_lib.is_some() && miri_config.provenance_mode == ProvenanceMode::Strict { + if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict { show_error!("strict provenance is not compatible with calling native functions"); } // You can set either one seed or many. diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 8fe034d258297..1477f103ca5e1 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -148,9 +148,8 @@ pub struct MiriConfig { pub report_progress: Option, /// Whether Stacked Borrows and Tree Borrows retagging should recurse into fields of datatypes. pub retag_fields: RetagFields, - /// The location of a shared object file to load when calling external functions - /// FIXME! consider allowing users to specify paths to multiple files, or to a directory - pub native_lib: Option, + /// The location of the shared object files to load when calling external functions + pub native_lib: Vec, /// Run a garbage collector for BorTags every N basic blocks. pub gc_interval: u32, /// The number of CPUs to be reported by miri. @@ -197,7 +196,7 @@ impl Default for MiriConfig { preemption_rate: 0.01, // 1% report_progress: None, retag_fields: RetagFields::Yes, - native_lib: None, + native_lib: vec![], gc_interval: 10_000, num_cpus: 1, page_size: None, diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 15b3653d7aef9..b221dd8509251 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -558,9 +558,9 @@ pub struct MiriMachine<'tcx> { /// Handle of the optional shared object file for native functions. #[cfg(unix)] - pub native_lib: Option<(libloading::Library, std::path::PathBuf)>, + pub native_lib: Vec<(libloading::Library, std::path::PathBuf)>, #[cfg(not(unix))] - pub native_lib: Option, + pub native_lib: Vec, /// Run a garbage collector for BorTags every N basic blocks. pub(crate) gc_interval: u32, @@ -720,7 +720,7 @@ impl<'tcx> MiriMachine<'tcx> { extern_statics: FxHashMap::default(), rng: RefCell::new(rng), #[cfg(target_os = "linux")] - allocator: if config.native_lib.is_some() { + allocator: if !config.native_lib.is_empty() { Some(Rc::new(RefCell::new(crate::alloc::isolated_alloc::IsolatedAlloc::new()))) } else { None }, tracked_alloc_ids: config.tracked_alloc_ids.clone(), @@ -732,7 +732,7 @@ impl<'tcx> MiriMachine<'tcx> { basic_block_count: 0, monotonic_clock: MonotonicClock::new(config.isolated_op == IsolatedOp::Allow), #[cfg(unix)] - native_lib: config.native_lib.as_ref().map(|lib_file_path| { + native_lib: config.native_lib.iter().map(|lib_file_path| { let host_triple = rustc_session::config::host_tuple(); let target_triple = tcx.sess.opts.target_triple.tuple(); // Check if host target == the session target. @@ -752,11 +752,11 @@ impl<'tcx> MiriMachine<'tcx> { }, lib_file_path.clone(), ) - }), + }).collect(), #[cfg(not(unix))] - native_lib: config.native_lib.as_ref().map(|_| { + native_lib: config.native_lib.iter().map(|_| { panic!("calling functions from native libraries via FFI is only supported on Unix") - }), + }).collect(), gc_interval: config.gc_interval, since_gc: 0, num_cpus: config.num_cpus, diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index b08b522d279a0..81a5046b2260e 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -238,7 +238,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // First deal with any external C functions in linked .so file. #[cfg(unix)] - if this.machine.native_lib.as_ref().is_some() { + if !this.machine.native_lib.is_empty() { use crate::shims::native_lib::EvalContextExt as _; // An Ok(false) here means that the function being called was not exported // by the specified `.so` file; we should continue and check if it corresponds to diff --git a/src/tools/miri/src/shims/native_lib.rs b/src/tools/miri/src/shims/native_lib.rs index 1e6c93333c157..40440bf6da419 100644 --- a/src/tools/miri/src/shims/native_lib.rs +++ b/src/tools/miri/src/shims/native_lib.rs @@ -87,47 +87,52 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Get the pointer to the function of the specified name in the shared object file, - /// if it exists. The function must be in the shared object file specified: we do *not* - /// return pointers to functions in dependencies of the library. + /// if it exists. The function must be in one of the shared object files specified: + /// we do *not* return pointers to functions in dependencies of libraries. fn get_func_ptr_explicitly_from_lib(&mut self, link_name: Symbol) -> Option { let this = self.eval_context_mut(); - // Try getting the function from the shared library. - let (lib, lib_path) = this.machine.native_lib.as_ref().unwrap(); - let func: libloading::Symbol<'_, unsafe extern "C" fn()> = - unsafe { lib.get(link_name.as_str().as_bytes()).ok()? }; - #[expect(clippy::as_conversions)] // fn-ptr to raw-ptr cast needs `as`. - let fn_ptr = *func.deref() as *mut std::ffi::c_void; + // Try getting the function from one of the shared libraries. + for (lib, lib_path) in &this.machine.native_lib { + let Ok(func): Result, _> = + (unsafe { lib.get(link_name.as_str().as_bytes()) }) + else { + continue; + }; + #[expect(clippy::as_conversions)] // fn-ptr to raw-ptr cast needs `as`. + let fn_ptr = *func.deref() as *mut std::ffi::c_void; - // FIXME: this is a hack! - // The `libloading` crate will automatically load system libraries like `libc`. - // On linux `libloading` is based on `dlsym`: https://docs.rs/libloading/0.7.3/src/libloading/os/unix/mod.rs.html#202 - // and `dlsym`(https://linux.die.net/man/3/dlsym) looks through the dependency tree of the - // library if it can't find the symbol in the library itself. - // So, in order to check if the function was actually found in the specified - // `machine.external_so_lib` we need to check its `dli_fname` and compare it to - // the specified SO file path. - // This code is a reimplementation of the mechanism for getting `dli_fname` in `libloading`, - // from: https://docs.rs/libloading/0.7.3/src/libloading/os/unix/mod.rs.html#411 - // using the `libc` crate where this interface is public. - let mut info = std::mem::MaybeUninit::::zeroed(); - unsafe { - if libc::dladdr(fn_ptr, info.as_mut_ptr()) != 0 { - let info = info.assume_init(); - #[cfg(target_os = "cygwin")] - let fname_ptr = info.dli_fname.as_ptr(); - #[cfg(not(target_os = "cygwin"))] - let fname_ptr = info.dli_fname; - assert!(!fname_ptr.is_null()); - if std::ffi::CStr::from_ptr(fname_ptr).to_str().unwrap() - != lib_path.to_str().unwrap() - { - return None; + // FIXME: this is a hack! + // The `libloading` crate will automatically load system libraries like `libc`. + // On linux `libloading` is based on `dlsym`: https://docs.rs/libloading/0.7.3/src/libloading/os/unix/mod.rs.html#202 + // and `dlsym`(https://linux.die.net/man/3/dlsym) looks through the dependency tree of the + // library if it can't find the symbol in the library itself. + // So, in order to check if the function was actually found in the specified + // `machine.external_so_lib` we need to check its `dli_fname` and compare it to + // the specified SO file path. + // This code is a reimplementation of the mechanism for getting `dli_fname` in `libloading`, + // from: https://docs.rs/libloading/0.7.3/src/libloading/os/unix/mod.rs.html#411 + // using the `libc` crate where this interface is public. + let mut info = std::mem::MaybeUninit::::zeroed(); + unsafe { + if libc::dladdr(fn_ptr, info.as_mut_ptr()) != 0 { + let info = info.assume_init(); + #[cfg(target_os = "cygwin")] + let fname_ptr = info.dli_fname.as_ptr(); + #[cfg(not(target_os = "cygwin"))] + let fname_ptr = info.dli_fname; + assert!(!fname_ptr.is_null()); + if std::ffi::CStr::from_ptr(fname_ptr).to_str().unwrap() + != lib_path.to_str().unwrap() + { + return None; + } } } - } - // Return a pointer to the function. - Some(CodePtr(fn_ptr)) + // Return a pointer to the function. + return Some(CodePtr(fn_ptr)); + } + None } } From beff63bb98effefbc9204ff52a66a92545120786 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Wed, 4 Jun 2025 04:54:56 +0000 Subject: [PATCH 03/20] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 15f06b60d0f9c..2bc38eebfc299 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -99426c570eebec8dcba2eaa8f5057265346aaedc +792fc2b033aea7ea7b766e38bdc40f7d6bdce8c3 From 869baeaf9207e86fa9a8e8a3319d7ae080e4a07c Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Thu, 5 Jun 2025 04:55:14 +0000 Subject: [PATCH 04/20] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 2bc38eebfc299..437d4218cfad8 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -792fc2b033aea7ea7b766e38bdc40f7d6bdce8c3 +81a964c23ea4fe9ab52b4449bb166bf280035797 From 8c410ca291a68bf23ca3694b1bbda631323b12cd Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Fri, 6 Jun 2025 04:55:10 +0000 Subject: [PATCH 05/20] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 437d4218cfad8..1e97cdf5bde99 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -81a964c23ea4fe9ab52b4449bb166bf280035797 +cf423712b9e95e9f6ec84b1ecb3d125e55ac8d56 From b1652dc7d53e24196d3745f6196411c61ccd53fa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 5 Jun 2025 18:52:26 +0200 Subject: [PATCH 06/20] use File::lock to implement flock, and add a test for File::lock --- src/tools/miri/Cargo.lock | 1 - src/tools/miri/Cargo.toml | 7 -- src/tools/miri/src/lib.rs | 1 + src/tools/miri/src/shims/unix/fs.rs | 113 +++++--------------------- src/tools/miri/tests/pass/shims/fs.rs | 33 +++++++- 5 files changed, 50 insertions(+), 105 deletions(-) diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index 6f4bd3eab51a2..192d4f444c2ff 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -555,7 +555,6 @@ dependencies = [ "tempfile", "tikv-jemalloc-sys", "ui_test", - "windows-sys", ] [[package]] diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index e4d7abdb0f73e..a314488bb2533 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -40,13 +40,6 @@ libc = "0.2" libffi = "4.0.0" libloading = "0.8" -[target.'cfg(target_family = "windows")'.dependencies] -windows-sys = { version = "0.59", features = [ - "Win32_Foundation", - "Win32_System_IO", - "Win32_Storage_FileSystem", -] } - [dev-dependencies] ui_test = "0.29.1" colored = "2" diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 51ec19af52a07..dfefe2f4b0525 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -16,6 +16,7 @@ #![feature(unqualified_local_imports)] #![feature(derive_coerce_pointee)] #![feature(arbitrary_self_types)] +#![feature(file_lock)] // Configure clippy and other lints #![allow( clippy::collapsible_else_if, diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 31cb269059c16..0f7d453b296c9 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -2,7 +2,8 @@ use std::borrow::Cow; use std::fs::{ - DirBuilder, File, FileType, OpenOptions, ReadDir, read_dir, remove_dir, remove_file, rename, + DirBuilder, File, FileType, OpenOptions, ReadDir, TryLockError, read_dir, remove_dir, + remove_file, rename, }; use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; @@ -89,103 +90,27 @@ impl UnixFileDescription for FileHandle { communicate_allowed: bool, op: FlockOp, ) -> InterpResult<'tcx, io::Result<()>> { - // cfg(bootstrap) - macro_rules! cfg_select_dispatch { - ($($tokens:tt)*) => { - #[cfg(bootstrap)] - cfg_match! { $($tokens)* } - - #[cfg(not(bootstrap))] - cfg_select! { $($tokens)* } - }; - } - assert!(communicate_allowed, "isolation should have prevented even opening a file"); - cfg_select_dispatch! { - all(target_family = "unix", not(target_os = "solaris")) => { - use std::os::fd::AsRawFd; - - use FlockOp::*; - // We always use non-blocking call to prevent interpreter from being blocked - let (host_op, lock_nb) = match op { - SharedLock { nonblocking } => (libc::LOCK_SH | libc::LOCK_NB, nonblocking), - ExclusiveLock { nonblocking } => (libc::LOCK_EX | libc::LOCK_NB, nonblocking), - Unlock => (libc::LOCK_UN, false), - }; - let fd = self.file.as_raw_fd(); - let ret = unsafe { libc::flock(fd, host_op) }; - let res = match ret { - 0 => Ok(()), - -1 => { - let err = io::Error::last_os_error(); - if !lock_nb && err.kind() == io::ErrorKind::WouldBlock { - throw_unsup_format!("blocking `flock` is not currently supported"); - } - Err(err) - } - ret => panic!("Unexpected return value from flock: {ret}"), - }; - interp_ok(res) + use FlockOp::*; + // We must not block the interpreter loop, so we always `try_lock`. + let (res, nonblocking) = match op { + SharedLock { nonblocking } => (self.file.try_lock_shared(), nonblocking), + ExclusiveLock { nonblocking } => (self.file.try_lock(), nonblocking), + Unlock => { + return interp_ok(self.file.unlock()); } - target_family = "windows" => { - use std::os::windows::io::AsRawHandle; - - use windows_sys::Win32::Foundation::{ - ERROR_IO_PENDING, ERROR_LOCK_VIOLATION, FALSE, TRUE, - }; - use windows_sys::Win32::Storage::FileSystem::{ - LOCKFILE_EXCLUSIVE_LOCK, LOCKFILE_FAIL_IMMEDIATELY, LockFileEx, UnlockFile, - }; - - let fh = self.file.as_raw_handle(); - - use FlockOp::*; - let (ret, lock_nb) = match op { - SharedLock { nonblocking } | ExclusiveLock { nonblocking } => { - // We always use non-blocking call to prevent interpreter from being blocked - let mut flags = LOCKFILE_FAIL_IMMEDIATELY; - if matches!(op, ExclusiveLock { .. }) { - flags |= LOCKFILE_EXCLUSIVE_LOCK; - } - let ret = unsafe { LockFileEx(fh, flags, 0, !0, !0, &mut std::mem::zeroed()) }; - (ret, nonblocking) - } - Unlock => { - let ret = unsafe { UnlockFile(fh, 0, 0, !0, !0) }; - (ret, false) - } - }; + }; - let res = match ret { - TRUE => Ok(()), - FALSE => { - let mut err = io::Error::last_os_error(); - // This only runs on Windows hosts so we can use `raw_os_error`. - // We have to be careful not to forward that error code to target code. - let code: u32 = err.raw_os_error().unwrap().try_into().unwrap(); - if matches!(code, ERROR_IO_PENDING | ERROR_LOCK_VIOLATION) { - if lock_nb { - // The io error mapping does not know about these error codes, - // so we translate it to `WouldBlock` manually. - let desc = format!("LockFileEx wouldblock error: {err}"); - err = io::Error::new(io::ErrorKind::WouldBlock, desc); - } else { - throw_unsup_format!("blocking `flock` is not currently supported"); - } - } - Err(err) - } - _ => panic!("Unexpected return value: {ret}"), - }; - interp_ok(res) - } - _ => { - let _ = op; - throw_unsup_format!( - "flock is supported only on UNIX (except Solaris) and Windows hosts" - ); - } + match res { + Ok(()) => interp_ok(Ok(())), + Err(TryLockError::Error(err)) => interp_ok(Err(err)), + Err(TryLockError::WouldBlock) => + if nonblocking { + interp_ok(Err(ErrorKind::WouldBlock.into())) + } else { + throw_unsup_format!("blocking `flock` is not currently supported"); + }, } } } diff --git a/src/tools/miri/tests/pass/shims/fs.rs b/src/tools/miri/tests/pass/shims/fs.rs index 87df43ca7e57f..2f30827c9336b 100644 --- a/src/tools/miri/tests/pass/shims/fs.rs +++ b/src/tools/miri/tests/pass/shims/fs.rs @@ -2,12 +2,12 @@ #![feature(io_error_more)] #![feature(io_error_uncategorized)] +#![feature(file_lock)] use std::collections::BTreeMap; use std::ffi::OsString; use std::fs::{ - File, OpenOptions, canonicalize, create_dir, read_dir, remove_dir, remove_dir_all, remove_file, - rename, + self, File, OpenOptions, create_dir, read_dir, remove_dir, remove_dir_all, remove_file, rename, }; use std::io::{Error, ErrorKind, IsTerminal, Read, Result, Seek, SeekFrom, Write}; use std::path::Path; @@ -33,6 +33,8 @@ fn main() { test_canonicalize(); #[cfg(unix)] test_pread_pwrite(); + #[cfg(not(any(target_os = "solaris", target_os = "illumos")))] + test_flock(); } } @@ -240,7 +242,7 @@ fn test_canonicalize() { let path = dir_path.join("test_file"); drop(File::create(&path).unwrap()); - let p = canonicalize(format!("{}/./test_file", dir_path.to_string_lossy())).unwrap(); + let p = fs::canonicalize(format!("{}/./test_file", dir_path.to_string_lossy())).unwrap(); assert_eq!(p.to_string_lossy().find("/./"), None); remove_dir_all(&dir_path).unwrap(); @@ -351,3 +353,28 @@ fn test_pread_pwrite() { f.read_exact(&mut buf1).unwrap(); assert_eq!(&buf1, b" m"); } + +// This function does seem to exist on Illumos but std does not expose it there. +#[cfg(not(any(target_os = "solaris", target_os = "illumos")))] +fn test_flock() { + let bytes = b"Hello, World!\n"; + let path = utils::prepare_with_content("miri_test_fs_flock.txt", bytes); + let file1 = OpenOptions::new().read(true).write(true).open(&path).unwrap(); + let file2 = OpenOptions::new().read(true).write(true).open(&path).unwrap(); + + // Test that we can apply many shared locks + file1.lock_shared().unwrap(); + file2.lock_shared().unwrap(); + // Test that shared lock prevents exclusive lock + assert!(matches!(file1.try_lock().unwrap_err(), fs::TryLockError::WouldBlock)); + // Unlock shared lock + file1.unlock().unwrap(); + file2.unlock().unwrap(); + // Take exclusive lock + file1.lock().unwrap(); + // Test that shared lock prevents exclusive and shared locks + assert!(matches!(file2.try_lock().unwrap_err(), fs::TryLockError::WouldBlock)); + assert!(matches!(file2.try_lock_shared().unwrap_err(), fs::TryLockError::WouldBlock)); + // Unlock exclusive lock + file1.unlock().unwrap(); +} From cf5aea57cc741ad10a80841a3ec2fde52a939279 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sat, 7 Jun 2025 04:53:58 +0000 Subject: [PATCH 07/20] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 1e97cdf5bde99..84a353bfbcf38 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -cf423712b9e95e9f6ec84b1ecb3d125e55ac8d56 +775e0c8aeb8f63192854b27156f8b05a06b51814 From 4007b1937bdc6994da0847a27db9bbe97a9f69c2 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sat, 7 Jun 2025 05:02:30 +0000 Subject: [PATCH 08/20] fmt --- src/tools/miri/src/shims/backtrace.rs | 10 ++++++++-- src/tools/miri/src/shims/unix/linux_like/epoll.rs | 3 ++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/tools/miri/src/shims/backtrace.rs b/src/tools/miri/src/shims/backtrace.rs index 8606735c913eb..feb83ca8829ab 100644 --- a/src/tools/miri/src/shims/backtrace.rs +++ b/src/tools/miri/src/shims/backtrace.rs @@ -169,8 +169,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { _ => throw_unsup_format!("unknown `miri_resolve_frame` flags {}", flags), } - this.write_scalar(Scalar::from_u32(lineno), &this.project_field(dest, FieldIdx::from_u32(2))?)?; - this.write_scalar(Scalar::from_u32(colno), &this.project_field(dest, FieldIdx::from_u32(3))?)?; + this.write_scalar( + Scalar::from_u32(lineno), + &this.project_field(dest, FieldIdx::from_u32(2))?, + )?; + this.write_scalar( + Scalar::from_u32(colno), + &this.project_field(dest, FieldIdx::from_u32(3))?, + )?; // Support a 4-field struct for now - this is deprecated // and slated for removal. diff --git a/src/tools/miri/src/shims/unix/linux_like/epoll.rs b/src/tools/miri/src/shims/unix/linux_like/epoll.rs index f971fb10b1998..d460abc783dd5 100644 --- a/src/tools/miri/src/shims/unix/linux_like/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux_like/epoll.rs @@ -286,7 +286,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if op == epoll_ctl_add || op == epoll_ctl_mod { // Read event bitmask and data from epoll_event passed by caller. - let mut events = this.read_scalar(&this.project_field(&event, FieldIdx::ZERO)?)?.to_u32()?; + let mut events = + this.read_scalar(&this.project_field(&event, FieldIdx::ZERO)?)?.to_u32()?; let data = this.read_scalar(&this.project_field(&event, FieldIdx::ONE)?)?.to_u64()?; // Unset the flag we support to discover if any unsupported flags are used. From b620d0791d9176232d72b2743e93e054a7849a32 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 7 Jun 2025 08:27:34 +0200 Subject: [PATCH 09/20] test_dup: ensure the FDs remain in sync even after dup'ing --- src/tools/miri/tests/pass-dep/libc/libc-fs.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/tests/pass-dep/libc/libc-fs.rs b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs index 129b18d8e598d..0ff48c389e855 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-fs.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-fs.rs @@ -88,16 +88,17 @@ fn test_dup() { let name_ptr = name.as_bytes().as_ptr().cast::(); unsafe { let fd = libc::open(name_ptr, libc::O_RDONLY); + let new_fd = libc::dup(fd); + let new_fd2 = libc::dup2(fd, 8); + let mut first_buf = [0u8; 4]; libc::read(fd, first_buf.as_mut_ptr() as *mut libc::c_void, 4); assert_eq!(&first_buf, b"dup "); - let new_fd = libc::dup(fd); let mut second_buf = [0u8; 4]; libc::read(new_fd, second_buf.as_mut_ptr() as *mut libc::c_void, 4); assert_eq!(&second_buf, b"and "); - let new_fd2 = libc::dup2(fd, 8); let mut third_buf = [0u8; 4]; libc::read(new_fd2, third_buf.as_mut_ptr() as *mut libc::c_void, 4); assert_eq!(&third_buf, b"dup2"); From f413f157675f585eb3a7cae9a808d54d4ba32a29 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sun, 8 Jun 2025 04:56:16 +0000 Subject: [PATCH 10/20] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 84a353bfbcf38..d1a5ef7b7fce3 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -775e0c8aeb8f63192854b27156f8b05a06b51814 +a5584a8fe16037dc01782064fa41424a6dbe9987 From ee00d0f0514c74fdd09de3e6cbb01009257f2a23 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Mon, 9 Jun 2025 04:56:52 +0000 Subject: [PATCH 11/20] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index d1a5ef7b7fce3..c8721bb36001f 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -a5584a8fe16037dc01782064fa41424a6dbe9987 +c31cccb7b5cc098b1a8c1794ed38d7fdbec0ccb0 From 4e8d5837df7cd7c83d91e14ad107a7422ab0ed36 Mon Sep 17 00:00:00 2001 From: Xinglu Chen Date: Mon, 2 Jun 2025 12:21:39 +0200 Subject: [PATCH 12/20] Add `-Zmiri-tree-borrows-no-precise-interior-mut` flag --- src/tools/miri/README.md | 4 + src/tools/miri/src/bin/miri.rs | 20 ++++- src/tools/miri/src/borrow_tracker/mod.rs | 29 +++++-- .../src/borrow_tracker/tree_borrows/mod.rs | 87 ++++++++++++++----- src/tools/miri/src/diagnostics.rs | 5 +- src/tools/miri/src/lib.rs | 4 +- .../pass/tree_borrows/cell-inside-struct.rs | 34 ++++++++ .../tree_borrows/cell-inside-struct.stderr | 6 ++ 8 files changed, 154 insertions(+), 35 deletions(-) create mode 100644 src/tools/miri/tests/pass/tree_borrows/cell-inside-struct.rs create mode 100644 src/tools/miri/tests/pass/tree_borrows/cell-inside-struct.stderr diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index de521393cd0ab..f89708e0d8f20 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -458,6 +458,10 @@ to Miri failing to detect cases of undefined behavior in a program. This is much less likely with Stacked Borrows. Using Tree Borrows currently implies `-Zmiri-strict-provenance` because integer-to-pointer casts are not supported in this mode, but that may change in the future. +* `-Zmiri-tree-borrows-no-precise-interior-mut` makes Tree Borrows + track interior mutable data on the level of references instead of on the + byte-level as is done by default. Therefore, with this flag, Tree + Borrows will be more permissive. * `-Zmiri-force-page-size=` overrides the default page size for an architecture, in multiples of 1k. `4` is default for most targets. This value should always be a power of 2 and nonzero. diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 0121472d330f5..1d3742cb0e98a 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -35,7 +35,7 @@ use std::sync::{Arc, Once}; use miri::{ BacktraceStyle, BorrowTrackerMethod, GenmcConfig, GenmcCtx, MiriConfig, MiriEntryFnType, - ProvenanceMode, RetagFields, ValidationMode, + ProvenanceMode, RetagFields, TreeBorrowsParams, ValidationMode, }; use rustc_abi::ExternAbi; use rustc_data_structures::sync; @@ -554,8 +554,21 @@ fn main() { } else if arg == "-Zmiri-disable-stacked-borrows" { miri_config.borrow_tracker = None; } else if arg == "-Zmiri-tree-borrows" { - miri_config.borrow_tracker = Some(BorrowTrackerMethod::TreeBorrows); + miri_config.borrow_tracker = + Some(BorrowTrackerMethod::TreeBorrows(TreeBorrowsParams { + precise_interior_mut: true, + })); miri_config.provenance_mode = ProvenanceMode::Strict; + } else if arg == "-Zmiri-tree-borrows-no-precise-interior-mut" { + match &mut miri_config.borrow_tracker { + Some(BorrowTrackerMethod::TreeBorrows(params)) => { + params.precise_interior_mut = false; + } + _ => + show_error!( + "`-Zmiri-tree-borrows` is required before `-Zmiri-tree-borrows-no-precise-interior-mut`" + ), + }; } else if arg == "-Zmiri-disable-data-race-detector" { miri_config.data_race_detector = false; miri_config.weak_memory_emulation = false; @@ -725,7 +738,7 @@ fn main() { } } // Tree Borrows implies strict provenance, and is not compatible with native calls. - if matches!(miri_config.borrow_tracker, Some(BorrowTrackerMethod::TreeBorrows)) { + if matches!(miri_config.borrow_tracker, Some(BorrowTrackerMethod::TreeBorrows { .. })) { if miri_config.provenance_mode != ProvenanceMode::Strict { show_error!( "Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance" @@ -735,6 +748,7 @@ fn main() { show_error!("Tree Borrows is not compatible with calling native functions"); } } + // Native calls and strict provenance are not compatible. if miri_config.native_lib.is_some() && miri_config.provenance_mode == ProvenanceMode::Strict { show_error!("strict provenance is not compatible with calling native functions"); diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index b66c561d2b8ad..36c61053a3205 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -226,7 +226,13 @@ pub enum BorrowTrackerMethod { /// Stacked Borrows, as implemented in borrow_tracker/stacked_borrows StackedBorrows, /// Tree borrows, as implemented in borrow_tracker/tree_borrows - TreeBorrows, + TreeBorrows(TreeBorrowsParams), +} + +/// Parameters that Tree Borrows can take. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub struct TreeBorrowsParams { + pub precise_interior_mut: bool, } impl BorrowTrackerMethod { @@ -237,6 +243,13 @@ impl BorrowTrackerMethod { config.retag_fields, )) } + + pub fn get_tree_borrows_params(self) -> TreeBorrowsParams { + match self { + BorrowTrackerMethod::TreeBorrows(params) => params, + _ => panic!("can only be called when `BorrowTrackerMethod` is `TreeBorrows`"), + } + } } impl GlobalStateInner { @@ -252,7 +265,7 @@ impl GlobalStateInner { AllocState::StackedBorrows(Box::new(RefCell::new(Stacks::new_allocation( id, alloc_size, self, kind, machine, )))), - BorrowTrackerMethod::TreeBorrows => + BorrowTrackerMethod::TreeBorrows { .. } => AllocState::TreeBorrows(Box::new(RefCell::new(Tree::new_allocation( id, alloc_size, self, kind, machine, )))), @@ -271,7 +284,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; match method { BorrowTrackerMethod::StackedBorrows => this.sb_retag_ptr_value(kind, val), - BorrowTrackerMethod::TreeBorrows => this.tb_retag_ptr_value(kind, val), + BorrowTrackerMethod::TreeBorrows { .. } => this.tb_retag_ptr_value(kind, val), } } @@ -284,7 +297,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; match method { BorrowTrackerMethod::StackedBorrows => this.sb_retag_place_contents(kind, place), - BorrowTrackerMethod::TreeBorrows => this.tb_retag_place_contents(kind, place), + BorrowTrackerMethod::TreeBorrows { .. } => this.tb_retag_place_contents(kind, place), } } @@ -293,7 +306,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; match method { BorrowTrackerMethod::StackedBorrows => this.sb_protect_place(place), - BorrowTrackerMethod::TreeBorrows => this.tb_protect_place(place), + BorrowTrackerMethod::TreeBorrows { .. } => this.tb_protect_place(place), } } @@ -302,7 +315,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; match method { BorrowTrackerMethod::StackedBorrows => this.sb_expose_tag(alloc_id, tag), - BorrowTrackerMethod::TreeBorrows => this.tb_expose_tag(alloc_id, tag), + BorrowTrackerMethod::TreeBorrows { .. } => this.tb_expose_tag(alloc_id, tag), } } @@ -319,7 +332,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.tcx.tcx.dcx().warn("Stacked Borrows does not support named pointers; `miri_pointer_name` is a no-op"); interp_ok(()) } - BorrowTrackerMethod::TreeBorrows => + BorrowTrackerMethod::TreeBorrows { .. } => this.tb_give_pointer_debug_name(ptr, nth_parent, name), } } @@ -333,7 +346,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let method = borrow_tracker.borrow().borrow_tracker_method; match method { BorrowTrackerMethod::StackedBorrows => this.print_stacks(alloc_id), - BorrowTrackerMethod::TreeBorrows => this.print_tree(alloc_id, show_unnamed), + BorrowTrackerMethod::TreeBorrows { .. } => this.print_tree(alloc_id, show_unnamed), } } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 411ae89da9060..ce8fe03ee477d 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -312,8 +312,6 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } let span = this.machine.current_span(); - let alloc_extra = this.get_alloc_extra(alloc_id)?; - let mut tree_borrows = alloc_extra.borrow_tracker_tb().borrow_mut(); // Store initial permissions and their corresponding range. let mut perms_map: RangeMap = RangeMap::new( @@ -342,36 +340,83 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { assert!(new_perm.freeze_access); let protected = new_perm.protector.is_some(); - this.visit_freeze_sensitive(place, ptr_size, |range, frozen| { - has_unsafe_cell = has_unsafe_cell || !frozen; - - // We are only ever `Frozen` inside the frozen bits. - let (perm, access) = if frozen { + let precise_interior_mut = this + .machine + .borrow_tracker + .as_mut() + .unwrap() + .get_mut() + .borrow_tracker_method + .get_tree_borrows_params() + .precise_interior_mut; + + let default_perm = if !precise_interior_mut { + // NOTE: Using `ty_is_freeze` doesn't give the same result as going through the range + // and computing `has_unsafe_cell`. This is because of zero-sized `UnsafeCell`, for which + // `has_unsafe_cell` is false, but `!ty_is_freeze` is true. + let ty_is_freeze = place.layout.ty.is_freeze(*this.tcx, this.typing_env()); + let (perm, access) = if ty_is_freeze { (new_perm.freeze_perm, new_perm.freeze_access) } else { (new_perm.nonfreeze_perm, new_perm.nonfreeze_access) }; + let sifa = perm.strongest_idempotent_foreign_access(protected); + let new_loc = if access { + LocationState::new_accessed(perm, sifa) + } else { + LocationState::new_non_accessed(perm, sifa) + }; + + for (_loc_range, loc) in perms_map.iter_mut_all() { + *loc = new_loc; + } + + perm + } else { + this.visit_freeze_sensitive(place, ptr_size, |range, frozen| { + has_unsafe_cell = has_unsafe_cell || !frozen; - // Store initial permissions. - for (_loc_range, loc) in perms_map.iter_mut(range.start, range.size) { + // We are only ever `Frozen` inside the frozen bits. + let (perm, access) = if frozen { + (new_perm.freeze_perm, new_perm.freeze_access) + } else { + (new_perm.nonfreeze_perm, new_perm.nonfreeze_access) + }; let sifa = perm.strongest_idempotent_foreign_access(protected); // NOTE: Currently, `access` is false if and only if `perm` is Cell, so this `if` // doesn't not change whether any code is UB or not. We could just always use // `new_accessed` and everything would stay the same. But that seems conceptually // odd, so we keep the initial "accessed" bit of the `LocationState` in sync with whether // a read access is performed below. - if access { - *loc = LocationState::new_accessed(perm, sifa); + let new_loc = if access { + LocationState::new_accessed(perm, sifa) } else { - *loc = LocationState::new_non_accessed(perm, sifa); + LocationState::new_non_accessed(perm, sifa) + }; + + // Store initial permissions. + for (_loc_range, loc) in perms_map.iter_mut(range.start, range.size) { + *loc = new_loc; } - } - // Some reborrows incur a read access to the parent. - if access { + interp_ok(()) + })?; + + // Allow lazily writing to surrounding data if we found an `UnsafeCell`. + if has_unsafe_cell { new_perm.nonfreeze_perm } else { new_perm.freeze_perm } + }; + + let alloc_extra = this.get_alloc_extra(alloc_id)?; + let mut tree_borrows = alloc_extra.borrow_tracker_tb().borrow_mut(); + + for (perm_range, perm) in perms_map.iter_mut_all() { + if perm.is_accessed() { + // Some reborrows incur a read access to the parent. // Adjust range to be relative to allocation start (rather than to `place`). - let mut range_in_alloc = range; - range_in_alloc.start += base_offset; + let range_in_alloc = AllocRange { + start: Size::from_bytes(perm_range.start) + base_offset, + size: Size::from_bytes(perm_range.end - perm_range.start), + }; tree_borrows.perform_access( orig_tag, @@ -382,7 +427,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { )?; // Also inform the data race model (but only if any bytes are actually affected). - if range.size.bytes() > 0 { + if range_in_alloc.size.bytes() > 0 { if let Some(data_race) = alloc_extra.data_race.as_vclocks_ref() { data_race.read( alloc_id, @@ -394,8 +439,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } } - interp_ok(()) - })?; + } // Record the parent-child pair in the tree. tree_borrows.new_child( @@ -403,8 +447,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { orig_tag, new_tag, perms_map, - // Allow lazily writing to surrounding data if we found an `UnsafeCell`. - if has_unsafe_cell { new_perm.nonfreeze_perm } else { new_perm.freeze_perm }, + default_perm, protected, span, )?; diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 1728a9cfd6de6..58bf163bfadee 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -682,7 +682,10 @@ impl<'tcx> MiriMachine<'tcx> { ), ]; if self.borrow_tracker.as_ref().is_some_and(|b| { - matches!(b.borrow().borrow_tracker_method(), BorrowTrackerMethod::TreeBorrows) + matches!( + b.borrow().borrow_tracker_method(), + BorrowTrackerMethod::TreeBorrows { .. } + ) }) { v.push( note!("Tree Borrows does not support integer-to-pointer casts, so the program is likely to go wrong when this pointer gets used") diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 8802216448f6e..8b457b98017ee 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -112,7 +112,9 @@ pub use crate::borrow_tracker::stacked_borrows::{ EvalContextExt as _, Item, Permission, Stack, Stacks, }; pub use crate::borrow_tracker::tree_borrows::{EvalContextExt as _, Tree}; -pub use crate::borrow_tracker::{BorTag, BorrowTrackerMethod, EvalContextExt as _, RetagFields}; +pub use crate::borrow_tracker::{ + BorTag, BorrowTrackerMethod, EvalContextExt as _, RetagFields, TreeBorrowsParams, +}; pub use crate::clock::{Instant, MonotonicClock}; pub use crate::concurrency::cpu_affinity::MAX_CPUS; pub use crate::concurrency::data_race::{ diff --git a/src/tools/miri/tests/pass/tree_borrows/cell-inside-struct.rs b/src/tools/miri/tests/pass/tree_borrows/cell-inside-struct.rs new file mode 100644 index 0000000000000..fd68685a2f442 --- /dev/null +++ b/src/tools/miri/tests/pass/tree_borrows/cell-inside-struct.rs @@ -0,0 +1,34 @@ +//! The same as `tests/fail/tree-borrows/cell-inside-struct` but with +//! precise tracking of interior mutability disabled. +//@compile-flags: -Zmiri-tree-borrows -Zmiri-tree-borrows-no-precise-interior-mut +#[path = "../../utils/mod.rs"] +#[macro_use] +mod utils; + +use std::cell::Cell; + +struct Foo { + field1: u32, + field2: Cell, +} + +pub fn main() { + let root = Foo { field1: 42, field2: Cell::new(88) }; + unsafe { + let a = &root; + + name!(a as *const Foo, "a"); + + let a: *const Foo = a as *const Foo; + let a: *mut Foo = a as *mut Foo; + + let alloc_id = alloc_id!(a); + print_state!(alloc_id); + + // Writing to `field2`, which is interior mutable, should be allowed. + (*a).field2.set(10); + + // Writing to `field1` should be allowed because it also has the `Cell` permission. + (*a).field1 = 88; + } +} diff --git a/src/tools/miri/tests/pass/tree_borrows/cell-inside-struct.stderr b/src/tools/miri/tests/pass/tree_borrows/cell-inside-struct.stderr new file mode 100644 index 0000000000000..1d939329040fa --- /dev/null +++ b/src/tools/miri/tests/pass/tree_borrows/cell-inside-struct.stderr @@ -0,0 +1,6 @@ +────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 8 +| Act | └─┬── +|?Cel | └──── +────────────────────────────────────────────────── From a096de73ea6f9a41026f6c6ad5a393f9418aab97 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 Jun 2025 21:49:50 +0200 Subject: [PATCH 13/20] native_lib: skip to next .so if function was in dependency of the first --- src/tools/miri/src/shims/native_lib.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/tools/miri/src/shims/native_lib.rs b/src/tools/miri/src/shims/native_lib.rs index 40440bf6da419..acf258f4eedd2 100644 --- a/src/tools/miri/src/shims/native_lib.rs +++ b/src/tools/miri/src/shims/native_lib.rs @@ -114,18 +114,19 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // using the `libc` crate where this interface is public. let mut info = std::mem::MaybeUninit::::zeroed(); unsafe { - if libc::dladdr(fn_ptr, info.as_mut_ptr()) != 0 { - let info = info.assume_init(); - #[cfg(target_os = "cygwin")] - let fname_ptr = info.dli_fname.as_ptr(); - #[cfg(not(target_os = "cygwin"))] - let fname_ptr = info.dli_fname; - assert!(!fname_ptr.is_null()); - if std::ffi::CStr::from_ptr(fname_ptr).to_str().unwrap() - != lib_path.to_str().unwrap() - { - return None; - } + let res = libc::dladdr(fn_ptr, info.as_mut_ptr()); + assert!(res != 0, "failed to load info about function we already loaded"); + let info = info.assume_init(); + #[cfg(target_os = "cygwin")] + let fname_ptr = info.dli_fname.as_ptr(); + #[cfg(not(target_os = "cygwin"))] + let fname_ptr = info.dli_fname; + assert!(!fname_ptr.is_null()); + if std::ffi::CStr::from_ptr(fname_ptr).to_str().unwrap() + != lib_path.to_str().unwrap() + { + // The function is not actually in this .so, check the next one. + continue; } } From 11581df1ba8b8473dd2a1fb6f43d06d01b0d9f20 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 Jun 2025 21:52:43 +0200 Subject: [PATCH 14/20] native-lib: update readme (to mention folders in header) --- src/tools/miri/README.md | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 508228ede9ee2..fc7942a49c0cc 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -396,20 +396,22 @@ to Miri failing to detect cases of undefined behavior in a program. * `-Zmiri-force-intrinsic-fallback` forces the use of the "fallback" body for all intrinsics that have one. This is useful to test the fallback bodies, but should not be used otherwise. It is **unsound** since the fallback body might not be checking for all UB. -* `-Zmiri-native-lib=` is an experimental flag for providing support - for calling native functions from inside the interpreter via FFI. The flag is supported only on - Unix systems. Functions not provided by that file are still executed via the usual Miri shims. If - a path to a directory is specified, all files in that directory are included nonrecursively. This - flag can be passed multiple times to specify multiple files and/or directories. +* `-Zmiri-native-lib=` is an experimental flag for providing + support for calling native functions from inside the interpreter via FFI. The flag is supported + only on Unix systems. Functions not provided by that file are still executed via the usual Miri + shims. If a path to a directory is specified, all files in that directory are included + non-recursively. This flag can be passed multiple times to specify multiple files and/or + directories. **WARNING**: If an invalid/incorrect `.so` file is specified, this can cause Undefined Behavior in Miri itself! And of course, Miri often cannot do any checks on the actions taken by the native code. Note that Miri has its own handling of file descriptors, so if you want to replace *some* functions working on file descriptors, you will have to replace *all* of them, or the two kinds of - file descriptors will be mixed up. This is **work in progress**; currently, only integer and - pointers arguments and return values are supported and memory allocated by the native code cannot - be accessed from Rust (only the other way around). Native code must not spawn threads that keep - running in the background after the call has returned to Rust and that access Rust-allocated - memory. Finally, the flag is **unsound** in the sense that Miri stops tracking details such as + file descriptors will be mixed up. + This is **work in progress**; currently, only integer and pointers arguments and return values are + supported and memory allocated by the native code cannot be accessed from Rust (only the other way + around). Native code must not spawn threads that keep running in the background after the call has + returned to Rust and that access Rust-allocated memory. + Finally, the flag is **unsound** in the sense that Miri stops tracking details such as initialization and provenance on memory shared with native code, so it is easily possible to write code that has UB which is missed by Miri. * `-Zmiri-measureme=` enables `measureme` profiling for the interpreted program. From 5eb5f2a2269c1f83a65cde57fb0ce0562d931be3 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Tue, 10 Jun 2025 04:55:12 +0000 Subject: [PATCH 15/20] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index c8721bb36001f..43632bf86a73b 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -c31cccb7b5cc098b1a8c1794ed38d7fdbec0ccb0 +c6768de2d63de7a41124a0fb8fc78f9e26111c01 From 89db7778d664a88d679ce3a58dbba208dd180629 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Tue, 10 Jun 2025 05:03:44 +0000 Subject: [PATCH 16/20] fmt --- src/tools/miri/tests/pass/float.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tools/miri/tests/pass/float.rs b/src/tools/miri/tests/pass/float.rs index 7ce0bc88517e1..c08a5d9ddaad3 100644 --- a/src/tools/miri/tests/pass/float.rs +++ b/src/tools/miri/tests/pass/float.rs @@ -45,7 +45,6 @@ macro_rules! assert_approx_eq { }; } - /// From IEEE 754 a Signaling NaN for single precision has the following representation: /// ``` /// s | 1111 1111 | 0x..x From fceb8d65107cb081bed799050519ec805ddb8ef7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Jun 2025 08:10:16 +0200 Subject: [PATCH 17/20] sync max_fundamental_align with alloc crate --- src/tools/miri/src/shims/alloc.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/src/shims/alloc.rs b/src/tools/miri/src/shims/alloc.rs index d7bb16f0858d8..f05c5fbbe1d4f 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/src/shims/alloc.rs @@ -15,11 +15,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // This is given by `alignof(max_align_t)`. The following list is taken from // `library/std/src/sys/alloc/mod.rs` (where this is called `MIN_ALIGN`) and should // be kept in sync. + let os = this.tcx.sess.target.os.as_ref(); let max_fundamental_align = match this.tcx.sess.target.arch.as_ref() { - "x86" | "arm" | "loongarch32" | "mips" | "mips32r6" | "powerpc" | "powerpc64" - | "wasm32" => 8, - "x86_64" | "aarch64" | "mips64" | "mips64r6" | "s390x" | "sparc64" | "loongarch64" => - 16, + "riscv32" if matches!(os, "espidf" | "zkvm") => 4, + "xtensa" if matches!(os, "espidf") => 4, + "x86" | "arm" | "m68k" | "csky" | "loongarch32" | "mips" | "mips32r6" | "powerpc" + | "powerpc64" | "sparc" | "wasm32" | "hexagon" | "riscv32" | "xtensa" => 8, + "x86_64" | "aarch64" | "arm64ec" | "loongarch64" | "mips64" | "mips64r6" | "s390x" + | "sparc64" | "riscv64" | "wasm64" => 16, arch => bug!("unsupported target architecture for malloc: `{}`", arch), }; // The C standard only requires sufficient alignment for any *type* with size less than or From 3e980d5f643f840bef74ac96bd657b2a6d7fe71a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Jun 2025 08:45:12 +0200 Subject: [PATCH 18/20] add SmallVec test --- .../miri/tests/pass/both_borrows/smallvec.rs | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 src/tools/miri/tests/pass/both_borrows/smallvec.rs diff --git a/src/tools/miri/tests/pass/both_borrows/smallvec.rs b/src/tools/miri/tests/pass/both_borrows/smallvec.rs new file mode 100644 index 0000000000000..f48815e37be34 --- /dev/null +++ b/src/tools/miri/tests/pass/both_borrows/smallvec.rs @@ -0,0 +1,99 @@ +//! This test represents a core part of `SmallVec`'s `extend_impl`. +//! What makes it interesting as a test is that it relies on Stacked Borrow's "quirk" +//! in a fundamental, hard-to-fix-without-full-trees way. + +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows + +use std::marker::PhantomData; +use std::mem::{ManuallyDrop, MaybeUninit}; +use std::ptr::NonNull; + +#[repr(C)] +pub union RawSmallVec { + inline: ManuallyDrop>, + heap: (NonNull, usize), +} + +impl RawSmallVec { + const fn new() -> Self { + Self::new_inline(MaybeUninit::uninit()) + } + + const fn new_inline(inline: MaybeUninit<[T; N]>) -> Self { + Self { inline: ManuallyDrop::new(inline) } + } + + const fn as_mut_ptr_inline(&mut self) -> *mut T { + (unsafe { &raw mut self.inline }) as *mut T + } + + const unsafe fn as_mut_ptr_heap(&mut self) -> *mut T { + self.heap.0.as_ptr() + } +} + +#[repr(transparent)] +#[derive(Clone, Copy)] +struct TaggedLen(usize); + +impl TaggedLen { + pub const fn new(len: usize, on_heap: bool, is_zst: bool) -> Self { + if is_zst { + debug_assert!(!on_heap); + TaggedLen(len) + } else { + debug_assert!(len < isize::MAX as usize); + TaggedLen((len << 1) | on_heap as usize) + } + } + + pub const fn on_heap(self, is_zst: bool) -> bool { + if is_zst { false } else { (self.0 & 1_usize) == 1 } + } + + pub const fn value(self, is_zst: bool) -> usize { + if is_zst { self.0 } else { self.0 >> 1 } + } +} + +#[repr(C)] +pub struct SmallVec { + len: TaggedLen, + raw: RawSmallVec, + _marker: PhantomData, +} + +impl SmallVec { + pub const fn new() -> SmallVec { + Self { + len: TaggedLen::new(0, false, Self::is_zst()), + raw: RawSmallVec::new(), + _marker: PhantomData, + } + } + + const fn is_zst() -> bool { + size_of::() == 0 + } + + pub const fn as_mut_ptr(&mut self) -> *mut T { + if self.len.on_heap(Self::is_zst()) { + // SAFETY: see above + unsafe { self.raw.as_mut_ptr_heap() } + } else { + self.raw.as_mut_ptr_inline() + } + } + + pub const fn len(&self) -> usize { + self.len.value(Self::is_zst()) + } +} + +fn main() { + let mut v = SmallVec::::new(); + let ptr = v.as_mut_ptr(); + let _len = v.len(); // this call incurs a reborrow which just barely does not invalidate `ptr` + unsafe { ptr.write(0) }; +} From a7153db254acc387e271e75153bdbd3caa2bed89 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 Jun 2025 22:24:58 +0200 Subject: [PATCH 19/20] float tests: test non-determinism for more operations --- src/tools/miri/tests/pass/float.rs | 47 ++++++++++++++++-------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/tools/miri/tests/pass/float.rs b/src/tools/miri/tests/pass/float.rs index c08a5d9ddaad3..383579198bbe5 100644 --- a/src/tools/miri/tests/pass/float.rs +++ b/src/tools/miri/tests/pass/float.rs @@ -1386,7 +1386,6 @@ fn test_non_determinism() { frem_algebraic, frem_fast, fsub_algebraic, fsub_fast, }; use std::{f32, f64}; - // TODO: Also test powi and powf when the non-determinism is implemented for them /// Ensure that the operation is non-deterministic #[track_caller] @@ -1426,21 +1425,23 @@ fn test_non_determinism() { } pub fn test_operations_f32(a: f32, b: f32) { test_operations_f!(a, b); - // FIXME: temporarily disabled as it breaks std tests. - // ensure_nondet(|| a.log(b)); - // ensure_nondet(|| a.exp()); - // ensure_nondet(|| 10f32.exp2()); - // ensure_nondet(|| f32::consts::E.ln()); + // FIXME: some are temporarily disabled as it breaks std tests. + ensure_nondet(|| a.powf(b)); + ensure_nondet(|| a.powi(2)); + ensure_nondet(|| a.log(b)); + ensure_nondet(|| a.exp()); + ensure_nondet(|| 10f32.exp2()); + ensure_nondet(|| f32::consts::E.ln()); + ensure_nondet(|| 10f32.log10()); + ensure_nondet(|| 8f32.log2()); // ensure_nondet(|| 1f32.ln_1p()); - // ensure_nondet(|| 10f32.log10()); - // ensure_nondet(|| 8f32.log2()); // ensure_nondet(|| 27.0f32.cbrt()); // ensure_nondet(|| 3.0f32.hypot(4.0f32)); - // ensure_nondet(|| 1f32.sin()); - // ensure_nondet(|| 0f32.cos()); - // // On i686-pc-windows-msvc , these functions are implemented by calling the `f64` version, - // // which means the little rounding errors Miri introduces are discard by the cast down to `f32`. - // // Just skip the test for them. + ensure_nondet(|| 1f32.sin()); + ensure_nondet(|| 1f32.cos()); + // On i686-pc-windows-msvc , these functions are implemented by calling the `f64` version, + // which means the little rounding errors Miri introduces are discarded by the cast down to + // `f32`. Just skip the test for them. // if !cfg!(all(target_os = "windows", target_env = "msvc", target_arch = "x86")) { // ensure_nondet(|| 1.0f32.tan()); // ensure_nondet(|| 1.0f32.asin()); @@ -1461,18 +1462,20 @@ fn test_non_determinism() { } pub fn test_operations_f64(a: f64, b: f64) { test_operations_f!(a, b); - // FIXME: temporarily disabled as it breaks std tests. - // ensure_nondet(|| a.log(b)); - // ensure_nondet(|| a.exp()); - // ensure_nondet(|| 50f64.exp2()); - // ensure_nondet(|| 3f64.ln()); + // FIXME: some are temporarily disabled as it breaks std tests. + ensure_nondet(|| a.powf(b)); + ensure_nondet(|| a.powi(2)); + ensure_nondet(|| a.log(b)); + ensure_nondet(|| a.exp()); + ensure_nondet(|| 50f64.exp2()); + ensure_nondet(|| 3f64.ln()); + ensure_nondet(|| f64::consts::E.log10()); + ensure_nondet(|| f64::consts::E.log2()); // ensure_nondet(|| 1f64.ln_1p()); - // ensure_nondet(|| f64::consts::E.log10()); - // ensure_nondet(|| f64::consts::E.log2()); // ensure_nondet(|| 27.0f64.cbrt()); // ensure_nondet(|| 3.0f64.hypot(4.0f64)); - // ensure_nondet(|| 1f64.sin()); - // ensure_nondet(|| 0f64.cos()); + ensure_nondet(|| 1f64.sin()); + ensure_nondet(|| 1f64.cos()); // ensure_nondet(|| 1.0f64.tan()); // ensure_nondet(|| 1.0f64.asin()); // ensure_nondet(|| 5.0f64.acos()); From 88e3e682869d6c6f28c86cd2ee38a2164dbcd8d0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Jun 2025 14:01:03 +0200 Subject: [PATCH 20/20] update lockfile --- Cargo.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index ad054e50e6029..93abab8469a7b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2326,7 +2326,6 @@ dependencies = [ "tempfile", "tikv-jemalloc-sys", "ui_test", - "windows-sys 0.59.0", ] [[package]]