diff --git a/src/diagnostics.rs b/src/diagnostics.rs index d3486dcbb1..1c3de9035c 100644 --- a/src/diagnostics.rs +++ b/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/shims/native_lib/mod.rs b/src/shims/native_lib/mod.rs index 47102c30bc..eb473de487 100644 --- a/src/shims/native_lib/mod.rs +++ b/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/shims/native_lib/trace/parent.rs b/src/shims/native_lib/trace/parent.rs index 3ae98259ab..335188b331 100644 --- a/src/shims/native_lib/trace/parent.rs +++ b/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/tests/native-lib/pass/ptr_read_access.notrace.stderr b/tests/native-lib/pass/ptr_read_access.notrace.stderr index 04a3025bae..200eba8050 100644 --- a/tests/native-lib/pass/ptr_read_access.notrace.stderr +++ b/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/tests/native-lib/pass/ptr_read_access.rs b/tests/native-lib/pass/ptr_read_access.rs index bab73f7cf1..36eff04a03 100644 --- a/tests/native-lib/pass/ptr_read_access.rs +++ b/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/tests/native-lib/pass/ptr_read_access.trace.stderr b/tests/native-lib/pass/ptr_read_access.trace.stderr index c2a4508b7f..5c0e954deb 100644 --- a/tests/native-lib/pass/ptr_read_access.trace.stderr +++ b/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/tests/native-lib/ptr_read_access.c b/tests/native-lib/ptr_read_access.c index 021eb6adca..5f071ca3d4 100644 --- a/tests/native-lib/ptr_read_access.c +++ b/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 +}