diff --git a/Cargo.lock b/Cargo.lock index 35d10596710ef..3f0a61fd7c0cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2106,9 +2106,9 @@ dependencies = [ [[package]] name = "libffi" -version = "4.1.1" +version = "5.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7681c6fab541f799a829e44a445a0666cf8d8a6cfebf89419e6aed52c604e87" +checksum = "0444124f3ffd67e1b0b0c661a7f81a278a135eb54aaad4078e79fbc8be50c8a5" dependencies = [ "libc", "libffi-sys", @@ -2116,9 +2116,9 @@ dependencies = [ [[package]] name = "libffi-sys" -version = "3.3.2" +version = "4.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b0d828d367b4450ed08e7d510dc46636cd660055f50d67ac943bfe788767c29" +checksum = "3d722da8817ea580d0669da6babe2262d7b86a1af1103da24102b8bb9c101ce7" dependencies = [ "cc", ] diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index 8792d90ac5366..d66529a464b69 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -794,9 +794,9 @@ checksum = "1171693293099992e19cddea4e8b849964e9846f4acee11b3948bcc337be8776" [[package]] name = "libffi" -version = "4.1.1" +version = "5.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7681c6fab541f799a829e44a445a0666cf8d8a6cfebf89419e6aed52c604e87" +checksum = "0444124f3ffd67e1b0b0c661a7f81a278a135eb54aaad4078e79fbc8be50c8a5" dependencies = [ "libc", "libffi-sys", @@ -804,9 +804,9 @@ dependencies = [ [[package]] name = "libffi-sys" -version = "3.3.2" +version = "4.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b0d828d367b4450ed08e7d510dc46636cd660055f50d67ac943bfe788767c29" +checksum = "3d722da8817ea580d0669da6babe2262d7b86a1af1103da24102b8bb9c101ce7" dependencies = [ "cc", ] diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index 12123c260da0a..8bb4f1c160934 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -39,7 +39,7 @@ features = ['unprefixed_malloc_on_supported_platforms'] [target.'cfg(unix)'.dependencies] libc = "0.2" # native-lib dependencies -libffi = { version = "4.1.1", optional = true } +libffi = { version = "5.0.0", optional = true } libloading = { version = "0.8", optional = true } serde = { version = "1.0.219", features = ["derive"], optional = true } diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index f877706520ea1..f4bdbbff8ea7c 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -36e4f5d1fe1d63953a5bf1758ce2b64172623e2e +235a4c083eb2a2bfe8779d211c3232f39396de00 diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index d3486dcbb19c9..1c3de9035cf86 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -139,6 +139,7 @@ pub enum NonHaltingDiagnostic { NativeCallSharedMem { tracing: bool, }, + NativeCallFnPtr, WeakMemoryOutdatedLoad { ptr: Pointer, }, @@ -644,6 +645,11 @@ impl<'tcx> MiriMachine<'tcx> { Int2Ptr { .. } => ("integer-to-pointer cast".to_string(), DiagLevel::Warning), NativeCallSharedMem { .. } => ("sharing memory with a native function".to_string(), DiagLevel::Warning), + NativeCallFnPtr => + ( + "sharing a function pointer with a native function".to_string(), + DiagLevel::Warning, + ), ExternTypeReborrow => ("reborrow of reference to `extern type`".to_string(), DiagLevel::Warning), GenmcCompareExchangeWeak | GenmcCompareExchangeOrderingMismatch { .. } => @@ -682,6 +688,8 @@ impl<'tcx> MiriMachine<'tcx> { Int2Ptr { .. } => format!("integer-to-pointer cast"), NativeCallSharedMem { .. } => format!("sharing memory with a native function called via FFI"), + NativeCallFnPtr => + format!("sharing a function pointer with a native function called via FFI"), WeakMemoryOutdatedLoad { ptr } => format!("weak memory emulation: outdated value returned from load at {ptr}"), ExternTypeReborrow => @@ -779,6 +787,11 @@ impl<'tcx> MiriMachine<'tcx> { ), ] }, + NativeCallFnPtr => { + vec![note!( + "calling Rust functions from C is not supported and will, in the best case, crash the program" + )] + } ExternTypeReborrow => { assert!(self.borrow_tracker.as_ref().is_some_and(|b| { matches!( diff --git a/src/tools/miri/src/shims/native_lib/ffi.rs b/src/tools/miri/src/shims/native_lib/ffi.rs index 0badf22bb7651..196f43c6f6a6c 100644 --- a/src/tools/miri/src/shims/native_lib/ffi.rs +++ b/src/tools/miri/src/shims/native_lib/ffi.rs @@ -9,11 +9,9 @@ use libffi::middle::{Arg as ArgPtr, Cif, Type as FfiType}; /// /// The safety invariants of the foreign function being called must be upheld (if any). pub unsafe fn call(fun: CodePtr, args: &mut [OwnedArg]) -> R { - let arg_ptrs: Vec<_> = args.iter().map(|arg| arg.ptr()).collect(); let cif = Cif::new(args.iter_mut().map(|arg| arg.ty.take().unwrap()), R::reify().into_middle()); - // SAFETY: Caller upholds that the function is safe to call, and since we - // were passed a slice reference we know the `OwnedArg`s won't have been - // dropped by this point. + let arg_ptrs: Vec<_> = args.iter().map(|arg| ArgPtr::new(&*arg.bytes)).collect(); + // SAFETY: Caller upholds that the function is safe to call. unsafe { cif.call(fun, &arg_ptrs) } } @@ -31,16 +29,4 @@ impl OwnedArg { pub fn new(ty: FfiType, bytes: Box<[u8]>) -> Self { Self { ty: Some(ty), bytes } } - - /// Creates a libffi argument pointer pointing to this argument's bytes. - /// NB: Since `libffi::middle::Arg` ignores the lifetime of the reference - /// it's derived from, it is up to the caller to ensure the `OwnedArg` is - /// not dropped before unsafely calling `libffi::middle::Cif::call()`! - fn ptr(&self) -> ArgPtr { - // FIXME: Using `&self.bytes[0]` to reference the whole array is - // definitely unsound under SB, but we're waiting on - // https://github.com/libffi-rs/libffi-rs/commit/112a37b3b6ffb35bd75241fbcc580de40ba74a73 - // to land in a release so that we don't need to do this. - ArgPtr::new(&self.bytes[0]) - } } diff --git a/src/tools/miri/src/shims/native_lib/mod.rs b/src/tools/miri/src/shims/native_lib/mod.rs index 47102c30bc3b1..eb473de487408 100644 --- a/src/tools/miri/src/shims/native_lib/mod.rs +++ b/src/tools/miri/src/shims/native_lib/mod.rs @@ -6,7 +6,7 @@ use std::sync::atomic::AtomicBool; use libffi::low::CodePtr; use libffi::middle::Type as FfiType; use rustc_abi::{HasDataLayout, Size}; -use rustc_middle::ty::layout::HasTypingEnv; +use rustc_middle::ty::layout::TyAndLayout; use rustc_middle::ty::{self, IntTy, Ty, UintTy}; use rustc_span::Symbol; use serde::{Deserialize, Serialize}; @@ -277,7 +277,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // This should go first so that we emit unsupported before doing a bunch // of extra work for types that aren't supported yet. - let ty = this.ty_to_ffitype(v.layout.ty)?; + let ty = this.ty_to_ffitype(v.layout)?; // Helper to print a warning when a pointer is shared with the native code. let expose = |prov: Provenance| -> InterpResult<'tcx> { @@ -386,34 +386,44 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_ref(); let mut fields = vec![]; for field in &adt_def.non_enum_variant().fields { - fields.push(this.ty_to_ffitype(field.ty(*this.tcx, args))?); + let layout = this.layout_of(field.ty(*this.tcx, args))?; + fields.push(this.ty_to_ffitype(layout)?); } interp_ok(FfiType::structure(fields)) } /// Gets the matching libffi type for a given Ty. - fn ty_to_ffitype(&self, ty: Ty<'tcx>) -> InterpResult<'tcx, FfiType> { - let this = self.eval_context_ref(); - interp_ok(match ty.kind() { - ty::Int(IntTy::I8) => FfiType::i8(), - ty::Int(IntTy::I16) => FfiType::i16(), - ty::Int(IntTy::I32) => FfiType::i32(), - ty::Int(IntTy::I64) => FfiType::i64(), - ty::Int(IntTy::Isize) => FfiType::isize(), - ty::Uint(UintTy::U8) => FfiType::u8(), - ty::Uint(UintTy::U16) => FfiType::u16(), - ty::Uint(UintTy::U32) => FfiType::u32(), - ty::Uint(UintTy::U64) => FfiType::u64(), - ty::Uint(UintTy::Usize) => FfiType::usize(), - ty::RawPtr(pointee_ty, _mut) => { - if !pointee_ty.is_sized(*this.tcx, this.typing_env()) { - throw_unsup_format!("passing a pointer to an unsized type over FFI: {}", ty); - } - FfiType::pointer() - } - ty::Adt(adt_def, args) => self.adt_to_ffitype(ty, *adt_def, args)?, - _ => throw_unsup_format!("unsupported argument type for native call: {}", ty), + fn ty_to_ffitype(&self, layout: TyAndLayout<'tcx>) -> InterpResult<'tcx, FfiType> { + use rustc_abi::{AddressSpace, BackendRepr, Integer, Primitive}; + + // `BackendRepr::Scalar` is also a signal to pass this type as a scalar in the ABI. This + // matches what codegen does. This does mean that we support some types whose ABI is not + // stable, but that's fine -- we are anyway quite conservative in native-lib mode. + if let BackendRepr::Scalar(s) = layout.backend_repr { + // Simple sanity-check: this cannot be `repr(C)`. + assert!(!layout.ty.ty_adt_def().is_some_and(|adt| adt.repr().c())); + return interp_ok(match s.primitive() { + Primitive::Int(Integer::I8, /* signed */ true) => FfiType::i8(), + Primitive::Int(Integer::I16, /* signed */ true) => FfiType::i16(), + Primitive::Int(Integer::I32, /* signed */ true) => FfiType::i32(), + Primitive::Int(Integer::I64, /* signed */ true) => FfiType::i64(), + Primitive::Int(Integer::I8, /* signed */ false) => FfiType::u8(), + Primitive::Int(Integer::I16, /* signed */ false) => FfiType::u16(), + Primitive::Int(Integer::I32, /* signed */ false) => FfiType::u32(), + Primitive::Int(Integer::I64, /* signed */ false) => FfiType::u64(), + Primitive::Pointer(AddressSpace::ZERO) => FfiType::pointer(), + _ => + throw_unsup_format!( + "unsupported scalar argument type for native call: {}", + layout.ty + ), + }); + } + interp_ok(match layout.ty.kind() { + // Scalar types have already been handled above. + ty::Adt(adt_def, args) => self.adt_to_ffitype(layout.ty, *adt_def, args)?, + _ => throw_unsup_format!("unsupported argument type for native call: {}", layout.ty), }) } } @@ -454,6 +464,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // pointer was passed as argument). Uninitialised memory is left as-is, but any data // exposed this way is garbage anyway. this.visit_reachable_allocs(this.exposed_allocs(), |this, alloc_id, info| { + if matches!(info.kind, AllocKind::Function) { + static DEDUP: AtomicBool = AtomicBool::new(false); + if !DEDUP.swap(true, std::sync::atomic::Ordering::Relaxed) { + // Newly set, so first time we get here. + this.emit_diagnostic(NonHaltingDiagnostic::NativeCallFnPtr); + } + } // If there is no data behind this pointer, skip this. if !matches!(info.kind, AllocKind::LiveData) { return interp_ok(()); diff --git a/src/tools/miri/src/shims/native_lib/trace/child.rs b/src/tools/miri/src/shims/native_lib/trace/child.rs index 95b0617a0261b..795ad4a32076d 100644 --- a/src/tools/miri/src/shims/native_lib/trace/child.rs +++ b/src/tools/miri/src/shims/native_lib/trace/child.rs @@ -228,9 +228,10 @@ pub unsafe fn init_sv() -> Result<(), SvInitError> { match init { // The "Ok" case means that we couldn't ptrace. Ok(e) => return Err(e), - Err(p) => { + Err(_p) => { eprintln!( - "Supervisor process panicked!\n{p:?}\n\nTry running again without using the native-lib tracer." + "Supervisor process panicked!\n\" + Try running again without `-Zmiri-native-lib-enable-tracing`." ); std::process::exit(1); } diff --git a/src/tools/miri/src/shims/native_lib/trace/parent.rs b/src/tools/miri/src/shims/native_lib/trace/parent.rs index 3ae98259ab35b..335188b331838 100644 --- a/src/tools/miri/src/shims/native_lib/trace/parent.rs +++ b/src/tools/miri/src/shims/native_lib/trace/parent.rs @@ -530,7 +530,18 @@ fn handle_segfault( // Don't use wait_for_signal here since 1 instruction doesn't give room // for any uncertainty + we don't want it `cont()`ing randomly by accident // Also, don't let it continue with unprotected memory if something errors! - let _ = wait::waitid(wait::Id::Pid(pid), WAIT_FLAGS).map_err(|_| ExecEnd(None))?; + let stat = wait::waitid(wait::Id::Pid(pid), WAIT_FLAGS).map_err(|_| ExecEnd(None))?; + match stat { + wait::WaitStatus::Signaled(_, s, _) + | wait::WaitStatus::Stopped(_, s) + | wait::WaitStatus::PtraceEvent(_, s, _) => + assert!( + !matches!(s, signal::SIGSEGV), + "native code segfaulted when re-trying memory access\n\ + is the native code trying to call a Rust function?" + ), + _ => (), + } // Zero out again to be safe for a in (ch_stack..ch_stack.strict_add(CALLBACK_STACK_SIZE)).step_by(ARCH_WORD_SIZE) { diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index d04ef5eac541b..1f8e60484edaa 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -815,7 +815,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { "pthread_cond_timedwait" => { let [cond, mutex, abstime] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - this.pthread_cond_timedwait(cond, mutex, abstime, dest, /* macos_relative_np */ false)?; + this.pthread_cond_timedwait( + cond, mutex, abstime, dest, /* macos_relative_np */ false, + )?; } "pthread_cond_destroy" => { let [cond] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; diff --git a/src/tools/miri/src/shims/unix/macos/foreign_items.rs b/src/tools/miri/src/shims/unix/macos/foreign_items.rs index 0754eb45a2d29..1b273593de63e 100644 --- a/src/tools/miri/src/shims/unix/macos/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/macos/foreign_items.rs @@ -310,7 +310,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { "pthread_cond_timedwait_relative_np" => { let [cond, mutex, reltime] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?; - this.pthread_cond_timedwait(cond, mutex, reltime, dest, /* macos_relative_np */ true)?; + this.pthread_cond_timedwait( + cond, mutex, reltime, dest, /* macos_relative_np */ true, + )?; } _ => return interp_ok(EmulateItemResult::NotSupported), diff --git a/src/tools/miri/tests/native-lib/pass/ptr_read_access.notrace.stderr b/src/tools/miri/tests/native-lib/pass/ptr_read_access.notrace.stderr index 04a3025baefa9..200eba8050bb5 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_read_access.notrace.stderr +++ b/src/tools/miri/tests/native-lib/pass/ptr_read_access.notrace.stderr @@ -16,3 +16,18 @@ note: inside `main` LL | test_access_pointer(); | ^^^^^^^^^^^^^^^^^^^^^ +warning: sharing a function pointer with a native function called via FFI + --> tests/native-lib/pass/ptr_read_access.rs:LL:CC + | +LL | pass_fn_ptr(Some(nop)); // this one is not + | ^^^^^^^^^^^^^^^^^^^^^^ sharing a function pointer with a native function + | + = help: calling Rust functions from C is not supported and will, in the best case, crash the program + = note: BACKTRACE: + = note: inside `pass_fn_ptr` at tests/native-lib/pass/ptr_read_access.rs:LL:CC +note: inside `main` + --> tests/native-lib/pass/ptr_read_access.rs:LL:CC + | +LL | pass_fn_ptr(); + | ^^^^^^^^^^^^^ + diff --git a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs index bab73f7cf1782..36eff04a03c05 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs @@ -3,11 +3,14 @@ //@[trace] compile-flags: -Zmiri-native-lib-enable-tracing //@compile-flags: -Zmiri-permissive-provenance +use std::ptr::NonNull; + fn main() { test_access_pointer(); test_access_simple(); test_access_nested(); test_access_static(); + pass_fn_ptr(); } /// Test function that dereferences an int pointer and prints its contents from C. @@ -30,11 +33,15 @@ fn test_access_simple() { extern "C" { fn access_simple(s_ptr: *const Simple) -> i32; + fn access_simple2(s_ptr: NonNull) -> i32; + fn access_simple3(s_ptr: Option>) -> i32; } let simple = Simple { field: -42 }; assert_eq!(unsafe { access_simple(&simple) }, -42); + assert_eq!(unsafe { access_simple2(NonNull::from(&simple)) }, -42); + assert_eq!(unsafe { access_simple3(Some(NonNull::from(&simple))) }, -42); } /// Test function that dereferences nested struct pointers and accesses fields. @@ -75,3 +82,16 @@ fn test_access_static() { assert_eq!(unsafe { access_static(&STATIC) }, 9001); } + +fn pass_fn_ptr() { + extern "C" { + fn pass_fn_ptr(s: Option); + } + + extern "C" fn nop() {} + + unsafe { + pass_fn_ptr(None); // this one is fine + pass_fn_ptr(Some(nop)); // this one is not + } +} diff --git a/src/tools/miri/tests/native-lib/pass/ptr_read_access.trace.stderr b/src/tools/miri/tests/native-lib/pass/ptr_read_access.trace.stderr index c2a4508b7fcc7..5c0e954deb9b6 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_read_access.trace.stderr +++ b/src/tools/miri/tests/native-lib/pass/ptr_read_access.trace.stderr @@ -17,3 +17,18 @@ note: inside `main` LL | test_access_pointer(); | ^^^^^^^^^^^^^^^^^^^^^ +warning: sharing a function pointer with a native function called via FFI + --> tests/native-lib/pass/ptr_read_access.rs:LL:CC + | +LL | pass_fn_ptr(Some(nop)); // this one is not + | ^^^^^^^^^^^^^^^^^^^^^^ sharing a function pointer with a native function + | + = help: calling Rust functions from C is not supported and will, in the best case, crash the program + = note: BACKTRACE: + = note: inside `pass_fn_ptr` at tests/native-lib/pass/ptr_read_access.rs:LL:CC +note: inside `main` + --> tests/native-lib/pass/ptr_read_access.rs:LL:CC + | +LL | pass_fn_ptr(); + | ^^^^^^^^^^^^^ + diff --git a/src/tools/miri/tests/native-lib/ptr_read_access.c b/src/tools/miri/tests/native-lib/ptr_read_access.c index 021eb6adca4f6..5f071ca3d424d 100644 --- a/src/tools/miri/tests/native-lib/ptr_read_access.c +++ b/src/tools/miri/tests/native-lib/ptr_read_access.c @@ -19,6 +19,13 @@ typedef struct Simple { EXPORT int32_t access_simple(const Simple *s_ptr) { return s_ptr->field; } +// Some copies so Rust can import them at different types. +EXPORT int32_t access_simple2(const Simple *s_ptr) { + return s_ptr->field; +} +EXPORT int32_t access_simple3(const Simple *s_ptr) { + return s_ptr->field; +} /* Test: test_access_nested */ @@ -55,3 +62,9 @@ EXPORT int32_t access_static(const Static *s_ptr) { EXPORT uintptr_t do_one_deref(const int32_t ***ptr) { return (uintptr_t)*ptr; } + +/* Test: pass_fn_ptr */ + +EXPORT void pass_fn_ptr(void f(void)) { + (void)f; // suppress unused warning +} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs index 531d637d1f24d..7105b076d652d 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-mem.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-mem.rs @@ -1,6 +1,10 @@ #![feature(pointer_is_aligned_to)] use std::{mem, ptr, slice}; +#[path = "../../utils/mod.rs"] +mod utils; +use utils::check_nondet; + fn test_memcpy() { unsafe { let src = [1i8, 2, 3]; @@ -120,13 +124,12 @@ fn test_memset() { } fn test_malloc() { - // Test that small allocations sometimes *are* not very aligned. - let saw_unaligned = (0..64).any(|_| unsafe { + // Test that small allocations sometimes are *not* very aligned (and sometimes they are). + check_nondet(|| unsafe { let p = libc::malloc(3); libc::free(p); - (p as usize) % 4 != 0 // find any that this is *not* 4-aligned + (p as usize) % 4 == 0 }); - assert!(saw_unaligned); unsafe { let p1 = libc::malloc(20); diff --git a/src/tools/miri/tests/pass/dyn-traits.rs b/src/tools/miri/tests/pass/dyn-traits.rs index f6220120968f7..47f913adae053 100644 --- a/src/tools/miri/tests/pass/dyn-traits.rs +++ b/src/tools/miri/tests/pass/dyn-traits.rs @@ -1,3 +1,7 @@ +#[path = "../utils/mod.rs"] +mod utils; +use utils::check_nondet; + fn ref_box_dyn() { struct Struct(i32); @@ -147,7 +151,7 @@ fn vtable_ptr_eq() { // We don't always get the same vtable when casting this to a wide pointer. let x = &2; let x_wide = x as &dyn fmt::Display; - assert!((0..256).any(|_| !ptr::eq(x as &dyn fmt::Display, x_wide))); + check_nondet(|| ptr::eq(x as &dyn fmt::Display, x_wide)); } fn main() { diff --git a/src/tools/miri/tests/pass/function_pointers.rs b/src/tools/miri/tests/pass/function_pointers.rs index 7145be334a13e..96f6f2568e98e 100644 --- a/src/tools/miri/tests/pass/function_pointers.rs +++ b/src/tools/miri/tests/pass/function_pointers.rs @@ -2,6 +2,10 @@ use std::mem; +#[path = "../utils/mod.rs"] +mod utils; +use utils::check_nondet; + trait Answer { fn answer() -> Self; } @@ -30,6 +34,7 @@ fn i() -> i32 { 73 } +#[inline(never)] // optimizations mask the non-determinism we test for below fn return_fn_ptr(f: fn() -> i32) -> fn() -> i32 { f } @@ -85,7 +90,7 @@ fn main() { assert!(return_fn_ptr(i) as unsafe fn() -> i32 == i as fn() -> i32 as unsafe fn() -> i32); // Miri gives different addresses to different reifications of a generic function. // at least if we try often enough. - assert!((0..256).any(|_| return_fn_ptr(f) != f)); + check_nondet(|| return_fn_ptr(f) == f); // However, if we only turn `f` into a function pointer and use that pointer, // it is equal to itself. let f2 = f as fn() -> i32; diff --git a/src/tools/miri/tests/pass/shims/fs.rs b/src/tools/miri/tests/pass/shims/fs.rs index 022dcc5dcbafa..43fbf6b085f75 100644 --- a/src/tools/miri/tests/pass/shims/fs.rs +++ b/src/tools/miri/tests/pass/shims/fs.rs @@ -13,6 +13,7 @@ use std::path::Path; #[path = "../../utils/mod.rs"] mod utils; +use utils::check_nondet; fn main() { test_path_conversion(); @@ -81,25 +82,25 @@ fn test_file() { } fn test_file_partial_reads_writes() { - let path = utils::prepare_with_content("miri_test_fs_file.txt", b"abcdefg"); + let path1 = utils::prepare_with_content("miri_test_fs_file1.txt", b"abcdefg"); + let path2 = utils::prepare_with_content("miri_test_fs_file2.txt", b"abcdefg"); // Ensure we sometimes do incomplete writes. - let got_short_write = (0..16).any(|_| { - let _ = remove_file(&path); // FIXME(win, issue #4483): errors if the file already exists - let mut file = File::create(&path).unwrap(); - file.write(&[0; 4]).unwrap() != 4 + check_nondet(|| { + let _ = remove_file(&path1); // FIXME(win, issue #4483): errors if the file already exists + let mut file = File::create(&path1).unwrap(); + file.write(&[0; 4]).unwrap() == 4 }); - assert!(got_short_write); // Ensure we sometimes do incomplete reads. - let got_short_read = (0..16).any(|_| { - let mut file = File::open(&path).unwrap(); + check_nondet(|| { + let mut file = File::open(&path2).unwrap(); let mut buf = [0; 4]; - file.read(&mut buf).unwrap() != 4 + file.read(&mut buf).unwrap() == 4 }); - assert!(got_short_read); // Clean up - remove_file(&path).unwrap(); + remove_file(&path1).unwrap(); + remove_file(&path2).unwrap(); } fn test_file_clone() { diff --git a/src/tools/miri/tests/utils/mod.rs b/src/tools/miri/tests/utils/mod.rs index 37f9996216343..f855f115e43dc 100644 --- a/src/tools/miri/tests/utils/mod.rs +++ b/src/tools/miri/tests/utils/mod.rs @@ -55,7 +55,10 @@ pub fn check_all_outcomes( /// Check that the operation is non-deterministic #[track_caller] pub fn check_nondet(f: impl Fn() -> T) { - let rounds = 50; + // We test some rather unlikely events with this, such as two global allocations getting the + // same "salt" (1/32 chance). So give this *many* shots before we consider the test to have + // failed. + let rounds = 500; let first = f(); for _ in 1..rounds { if f() != first {