miri subtree update

Cargo.lock

@@ -2106,19 +2106,19 @@ 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",
 ]
 
 [[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",
 ]

src/tools/miri/Cargo.lock

@@ -794,19 +794,19 @@ 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",
 ]
 
 [[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",
 ]

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 }
 

src/tools/miri/rust-version

@@ -1 +1 @@
-36e4f5d1fe1d63953a5bf1758ce2b64172623e2e
+235a4c083eb2a2bfe8779d211c3232f39396de00

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!(

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<R: libffi::high::CType>(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])
-    }
 }

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(());

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);
                     }

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) {

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)?;

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

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();
+   |     ^^^^^^^^^^^^^
+

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<Simple>) -> i32;
+        fn access_simple3(s_ptr: Option<NonNull<Simple>>) -> 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()>);
+    }
+
+    extern "C" fn nop() {}
+
+    unsafe {
+        pass_fn_ptr(None); // this one is fine
+        pass_fn_ptr(Some(nop)); // this one is not
+    }
+}

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();
+   |     ^^^^^^^^^^^^^
+

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
+}