Rollup of 7 pull requests

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