Additional improvements

weiznich · weiznich · commit 17ab5191e5ba · 2025-12-06T17:59:02.000+01:00
This commit brings better error reporting and some additional cleanups.
diff --git a/src/alloc_addresses/mod.rs b/src/alloc_addresses/mod.rs
@@ -8,10 +8,11 @@ use std::cell::RefCell;
 
 use rustc_abi::{Align, Size};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_middle::ty::{InstanceKind, TyCtxt};
+use rustc_middle::ty::TyCtxt;
 
 pub use self::address_generator::AddressGenerator;
 use self::reuse_pool::ReusePool;
+use crate::alloc::MiriAllocParams;
 use crate::concurrency::VClock;
 use crate::diagnostics::SpanDedupDiagnostic;
 use crate::*;
@@ -162,48 +163,41 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                         this.get_alloc_bytes_unchecked_raw(alloc_id)?
                     }
                 }
-                AllocKind::Function | AllocKind::VTable => {
-                    // Allocate some dummy memory to get a unique address for this function/vtable.
-                    let alloc_bytes = MiriAllocBytes::from_bytes(
-                        &[0u8; 1],
-                        Align::from_bytes(1).unwrap(),
-                        params,
-                    );
-                    let mut ptr = alloc_bytes.as_ptr();
-                    // Leak the underlying memory to ensure it remains unique.
-                    std::mem::forget(alloc_bytes);
+                #[cfg(all(unix, feature = "native-lib"))]
+                AllocKind::Function => {
                     if let Some(GlobalAlloc::Function { instance, .. }) =
                         this.tcx.try_get_global_alloc(alloc_id)
+                        && let rustc_middle::ty::InstanceKind::Item(def_id) = instance.def
+                        && let Some(closure) = crate::shims::native_lib::build_libffi_closure(
+                            this,
+                            this.tcx.fn_sig(def_id).skip_binder().skip_binder(),
+                        )
                     {
-                        #[cfg(all(unix, feature = "native-lib"))]
-                        if let InstanceKind::Item(def_id) = instance.def {
-                            let sig = this.tcx.fn_sig(def_id).skip_binder().skip_binder();
-                            let closure =
-                                crate::shims::native_lib::build_libffi_closure(this, sig)?;
-                            if let Some(closure) = closure {
-                                let closure = Box::leak(Box::new(closure));
-                                // Libffi returns a **reference** to a function ptr here
-                                // (The actual argument type doesn't matter)
-                                let fn_ptr = unsafe {
-                                    closure.instantiate_code_ptr::<unsafe extern "C" fn(*const std::ffi::c_void)>()
-                                };
-                                // Therefore we need to dereference the reference to get the actual function pointer
-                                let fn_ptr = *fn_ptr;
-                                #[expect(
-                                    clippy::as_conversions,
-                                    reason = "No better way to cast a function ptr to a ptr"
-                                )]
-                                {
-                                    // After that we need to cast the function pointer to the
-                                    // expected pointer type. At this point we don't actually care about the
-                                    // type of this pointer
-                                    ptr = (fn_ptr as *const std::ffi::c_void).cast();
-                                }
-                            }
+                        let closure = Box::leak(Box::new(closure));
+                        // Libffi returns a **reference** to a function ptr here
+                        // (The actual argument type doesn't matter)
+                        let fn_ptr = unsafe {
+                            closure.instantiate_code_ptr::<unsafe extern "C" fn(*const std::ffi::c_void)>()
+                        };
+                        // Therefore we need to dereference the reference to get the actual function pointer
+                        let fn_ptr = *fn_ptr;
+                        #[expect(
+                            clippy::as_conversions,
+                            reason = "No better way to cast a function ptr to a ptr"
+                        )]
+                        {
+                            // After that we need to cast the function pointer to the
+                            // expected pointer type. At this point we don't actually care about the
+                            // type of this pointer
+                            (fn_ptr as *const std::ffi::c_void).cast()
                         }
+                    } else {
+                        dummy_alloc(params)
                     }
-                    ptr
                 }
+                #[cfg(not(all(unix, feature = "native-lib")))]
+                AllocKind::Function => dummy_alloc(params),
+                AllocKind::VTable => dummy_alloc(params),
                 AllocKind::TypeId | AllocKind::Dead => unreachable!(),
             };
             // We don't have to expose this pointer yet, we do that in `prepare_for_native_call`.
@@ -235,6 +229,15 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
     }
 }
 
+fn dummy_alloc(params: MiriAllocParams) -> *const u8 {
+    // Allocate some dummy memory to get a unique address for this function/vtable.
+    let alloc_bytes = MiriAllocBytes::from_bytes(&[0u8; 1], Align::from_bytes(1).unwrap(), params);
+    let ptr = alloc_bytes.as_ptr();
+    // Leak the underlying memory to ensure it remains unique.
+    std::mem::forget(alloc_bytes);
+    ptr
+}
+
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     // Returns the `AllocId` that corresponds to the specified addr,
diff --git a/src/shims/native_lib/mod.rs b/src/shims/native_lib/mod.rs
@@ -1,7 +1,9 @@
 //! Implements calling functions from a native library.
 
+use std::cell::Cell;
 use std::ops::Deref;
 use std::os::raw::c_void;
+use std::ptr::NonNull;
 use std::sync::atomic::AtomicBool;
 
 use libffi::low::CodePtr;
@@ -17,6 +19,10 @@ use self::helpers::ToSoft;
 
 mod ffi;
 
+thread_local! {
+    static INTERP_CTX: Cell<Option<NonNull<c_void>>> = const { Cell::new(None) };
+}
+
 #[cfg_attr(
     not(all(
         target_os = "linux",
@@ -28,6 +34,7 @@ mod ffi;
 pub mod trace;
 
 use self::ffi::OwnedArg;
+use crate::diagnostics::DiagLevel;
 use crate::*;
 
 /// The final results of an FFI trace, containing every relevant event detected
@@ -95,6 +102,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         trace::Supervisor::do_ffi(alloc, || {
             // Call the function (`ptr`) with arguments `libffi_args`, and obtain the return value
             // as the specified primitive integer type
+            INTERP_CTX.set(NonNull::new(
+                (this as *const MiriInterpCx<'tcx> as *mut MiriInterpCx<'tcx>).cast(),
+            ));
             let scalar = match dest.layout.ty.kind() {
                 // ints
                 ty::Int(IntTy::I8) => {
@@ -455,21 +465,21 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
 pub fn build_libffi_closure<'tcx, 'this>(
     this: &'this MiriInterpCx<'tcx>,
     fn_ptr: rustc_middle::ty::FnSig<'tcx>,
-) -> InterpResult<'tcx, Option<libffi::middle::Closure<'this>>> {
+) -> Option<libffi::middle::Closure<'this>> {
     let mut args = Vec::new();
     for input in fn_ptr.inputs().iter() {
         let layout = match this.layout_of(*input) {
             Ok(layout) => layout,
             Err(e) => {
                 tracing::info!(?e, "Skip closure");
-                return interp_ok(None);
+                return None;
             }
         };
         let ty = match this.ty_to_ffitype(layout).report_err() {
             Ok(ty) => ty,
             Err(e) => {
                 tracing::info!(?e, "Skip closure");
-                return interp_ok(None);
+                return None;
             }
         };
         args.push(ty);
@@ -480,27 +490,22 @@ pub fn build_libffi_closure<'tcx, 'this>(
             Ok(layout) => layout,
             Err(e) => {
                 tracing::info!(?e, "Skip closure");
-                return interp_ok(None);
+                return None;
             }
         };
         match this.ty_to_ffitype(layout).report_err() {
             Ok(ty) => ty,
             Err(e) => {
                 tracing::info!(?e, "Skip closure");
-                return interp_ok(None);
+                return None;
             }
         }
     };
     let closure_builder = libffi::middle::Builder::new().args(args).res(res_type);
-    let data = CallbackData {
-        args: fn_ptr.inputs().to_vec(),
-        result: fn_ptr.output(),
-        this,
-        //link_name: *link_name,
-    };
+    let data = CallbackData {};
     let data = Box::leak(Box::new(data));
     let closure = closure_builder.into_closure(callback_callback, data);
-    interp_ok(Some(closure))
+    Some(closure)
 }
 
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
@@ -596,44 +601,37 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     }
 }
 
-struct CallbackData<'a, 'tcx> {
-    args: Vec<Ty<'tcx>>,
-    #[expect(dead_code, reason = "It's there for later")]
-    result: Ty<'tcx>,
-    this: &'a MiriInterpCx<'tcx>,
-}
+struct CallbackData {}
 
 unsafe extern "C" fn callback_callback(
-    cif: &libffi::low::ffi_cif,
+    _cif: &libffi::low::ffi_cif,
     _result: &mut c_void,
-    args: *const *const c_void,
-    infos: &CallbackData<'_, '_>,
+    _args: *const *const c_void,
+    _infos: &CallbackData,
 ) {
-    debug_assert_eq!(cif.nargs.try_into(), Ok(infos.args.len()));
-    let mut rust_args = Vec::with_capacity(infos.args.len());
-    // We cast away the pointer to pointer to get a pointer to the actual argument
-    let mut args = args.cast::<c_void>();
-    for arg in &infos.args {
-        let scalar = match arg.kind() {
-            ty::RawPtr(..) => {
-                let ptr = StrictPointer::new(Provenance::Wildcard, Size::from_bytes(args.addr()));
-                // This offset moves the pointer to the next argument
-                args = unsafe { args.offset(1) };
-                Scalar::from_pointer(ptr, infos.this)
-            }
-            // the other types
-            _ => todo!(),
-        };
-        rust_args.push(scalar);
+    if let Some(this) = INTERP_CTX.get() {
+        let ctx = unsafe { this.cast::<MiriInterpCx<'_>>().as_ref() };
+        let stacktrace = ctx.generate_stacktrace();
+        crate::diagnostics::report_msg(
+            DiagLevel::Error,
+            "tried to call a function pointer through the FFI boundary".into(),
+            vec!["this function called a function pointer calling back into Rust".into()],
+            vec![(None, "this is not supported yet by miri".into())],
+            Vec::new(),
+            &stacktrace,
+            None,
+            &ctx.machine,
+        );
+    } else {
+        // There is no interpctx set which likely means we are running in a different thread
+        // than that one that called into the native library. At this point we fall back
+        // to just reporting an error via eprintln before aborting.
+        eprintln!(
+            "Tried to call a function pointer via FFI boundary. \
+            That's not supported yet by miri"
+        );
     }
-
     // We abort the execution at this point as we cannot return the
     // expected value here.
-    eprintln!(
-        "Tried to call a function pointer via FFI boundary. \
-         That's not supported yet by miri\nThis function pointer was registered by a call to `{}` \
-         using an argument of the type `{}`",
-        "todo: fill in name", "todo: fill in type"
-    );
     std::process::exit(1);
 }
diff --git a/tests/native-lib/fail/call_function_ptr.notrace.stderr b/tests/native-lib/fail/call_function_ptr.notrace.stderr
@@ -1,7 +1,7 @@
 warning: sharing memory with a native function called via FFI
   --> tests/native-lib/fail/call_function_ptr.rs:LL:CC
    |
-LL |         call_fn_ptr(Some(nop)); // this one is not
+LL |         call_fn_ptr(Some(nop));
    |         ^^^^^^^^^^^^^^^^^^^^^^ sharing memory with a native function
    |
    = help: when memory is shared with a native function call, Miri stops tracking initialization and provenance for that memory
@@ -19,7 +19,7 @@ LL |     pass_fn_ptr()
 warning: sharing a function pointer with a native function called via FFI
   --> tests/native-lib/fail/call_function_ptr.rs:LL:CC
    |
-LL |         call_fn_ptr(Some(nop)); // this one is not
+LL |         call_fn_ptr(Some(nop));
    |         ^^^^^^^^^^^^^^^^^^^^^^ 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
@@ -31,5 +31,31 @@ note: inside `main`
 LL |     pass_fn_ptr()
    |     ^^^^^^^^^^^^^
 
-Tried to call a function pointer via FFI boundary. That's not supported yet by miri
-This function pointer was registered by a call to `todo: fill in name` using an argument of the type `todo: fill in type`
+error: tried to call a function pointer through the FFI boundary
+  --> tests/native-lib/fail/call_function_ptr.rs:LL:CC
+   |
+LL |         call_fn_ptr(Some(nop));
+   |         ^^^^^^^^^^^^^^^^^^^^^^ this function called a function pointer calling back into Rust
+   |
+   = note: this is not supported yet by miri
+   = note: BACKTRACE:
+   = note: inside `pass_fn_ptr` at tests/native-lib/fail/call_function_ptr.rs:LL:CC
+note: inside `main`
+  --> tests/native-lib/fail/call_function_ptr.rs:LL:CC
+   |
+LL |     pass_fn_ptr()
+   |     ^^^^^^^^^^^^^
+   = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at RUSTLIB/core/src/ops/function.rs:LL:CC
+   = note: inside `std::sys::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at RUSTLIB/std/src/sys/backtrace.rs:LL:CC
+   = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC
+   = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at RUSTLIB/core/src/ops/function.rs:LL:CC
+   = note: inside `std::panicking::catch_unwind::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at RUSTLIB/std/src/panicking.rs:LL:CC
+   = note: inside `std::panicking::catch_unwind::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at RUSTLIB/std/src/panicking.rs:LL:CC
+   = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at RUSTLIB/std/src/panic.rs:LL:CC
+   = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC
+   = note: inside `std::panicking::catch_unwind::do_call::<{closure@std::rt::lang_start_internal::{closure#0}}, isize>` at RUSTLIB/std/src/panicking.rs:LL:CC
+   = note: inside `std::panicking::catch_unwind::<isize, {closure@std::rt::lang_start_internal::{closure#0}}>` at RUSTLIB/std/src/panicking.rs:LL:CC
+   = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#0}}, isize>` at RUSTLIB/std/src/panic.rs:LL:CC
+   = note: inside `std::rt::lang_start_internal` at RUSTLIB/std/src/rt.rs:LL:CC
+   = note: inside `std::rt::lang_start::<()>` at RUSTLIB/std/src/rt.rs:LL:CC
+
diff --git a/tests/native-lib/fail/call_function_ptr.rs b/tests/native-lib/fail/call_function_ptr.rs
@@ -16,6 +16,6 @@ fn pass_fn_ptr() {
 
     unsafe {
         call_fn_ptr(None); // this one is fine
-        call_fn_ptr(Some(nop)); // this one is not
+        call_fn_ptr(Some(nop)); //~ ERROR: tried to call a function pointer through the FFI boundary
     }
 }
diff --git a/tests/native-lib/fail/call_function_ptr.trace.stderr b/tests/native-lib/fail/call_function_ptr.trace.stderr
@@ -1,7 +1,7 @@
 warning: sharing memory with a native function called via FFI
   --> tests/native-lib/fail/call_function_ptr.rs:LL:CC
    |
-LL |         call_fn_ptr(Some(nop)); // this one is not
+LL |         call_fn_ptr(Some(nop));
    |         ^^^^^^^^^^^^^^^^^^^^^^ sharing memory with a native function
    |
    = help: when memory is shared with a native function call, Miri can only track initialisation and provenance on a best-effort basis
@@ -20,7 +20,7 @@ LL |     pass_fn_ptr()
 warning: sharing a function pointer with a native function called via FFI
   --> tests/native-lib/fail/call_function_ptr.rs:LL:CC
    |
-LL |         call_fn_ptr(Some(nop)); // this one is not
+LL |         call_fn_ptr(Some(nop));
    |         ^^^^^^^^^^^^^^^^^^^^^^ 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
@@ -32,5 +32,31 @@ note: inside `main`
 LL |     pass_fn_ptr()
    |     ^^^^^^^^^^^^^
 
-Tried to call a function pointer via FFI boundary. That's not supported yet by miri
-This function pointer was registered by a call to `todo: fill in name` using an argument of the type `todo: fill in type`
+error: tried to call a function pointer through the FFI boundary
+  --> tests/native-lib/fail/call_function_ptr.rs:LL:CC
+   |
+LL |         call_fn_ptr(Some(nop));
+   |         ^^^^^^^^^^^^^^^^^^^^^^ this function called a function pointer calling back into Rust
+   |
+   = note: this is not supported yet by miri
+   = note: BACKTRACE:
+   = note: inside `pass_fn_ptr` at tests/native-lib/fail/call_function_ptr.rs:LL:CC
+note: inside `main`
+  --> tests/native-lib/fail/call_function_ptr.rs:LL:CC
+   |
+LL |     pass_fn_ptr()
+   |     ^^^^^^^^^^^^^
+   = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at RUSTLIB/core/src/ops/function.rs:LL:CC
+   = note: inside `std::sys::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at RUSTLIB/std/src/sys/backtrace.rs:LL:CC
+   = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC
+   = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at RUSTLIB/core/src/ops/function.rs:LL:CC
+   = note: inside `std::panicking::catch_unwind::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at RUSTLIB/std/src/panicking.rs:LL:CC
+   = note: inside `std::panicking::catch_unwind::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at RUSTLIB/std/src/panicking.rs:LL:CC
+   = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at RUSTLIB/std/src/panic.rs:LL:CC
+   = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC
+   = note: inside `std::panicking::catch_unwind::do_call::<{closure@std::rt::lang_start_internal::{closure#0}}, isize>` at RUSTLIB/std/src/panicking.rs:LL:CC
+   = note: inside `std::panicking::catch_unwind::<isize, {closure@std::rt::lang_start_internal::{closure#0}}>` at RUSTLIB/std/src/panicking.rs:LL:CC
+   = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#0}}, isize>` at RUSTLIB/std/src/panic.rs:LL:CC
+   = note: inside `std::rt::lang_start_internal` at RUSTLIB/std/src/rt.rs:LL:CC
+   = note: inside `std::rt::lang_start::<()>` at RUSTLIB/std/src/rt.rs:LL:CC
+

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,6 @@ fn pass_fn_ptr() {`
`16`	`16`
`17`	`17`	`unsafe {`
`18`	`18`	`call_fn_ptr(None); // this one is fine`
`19`		`- call_fn_ptr(Some(nop)); // this one is not`
	`19`	`+ call_fn_ptr(Some(nop)); //~ ERROR: tried to call a function pointer through the FFI boundary`
`20`	`20`	`}`
`21`	`21`	`}`