interpret: ensure that calls via no-unwind ABIs do not unwind

compiler/rustc_const_eval/src/interpret/call.rs

@@ -454,8 +454,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         // compile time.
         M::check_fn_target_features(self, instance)?;
 
+        // If the signature says this cannot unwind, reflect this in the unwind destination so that
+        // we don't have to check this later. (`init_fn_call` already did this for the caller so
+        // here we only have to check the callee.)
         if !callee_fn_abi.can_unwind {
-            // The callee cannot unwind, so force the `Unreachable` unwind handling.
             match &mut cont {
                 ReturnContinuation::Stop { .. } => {}
                 ReturnContinuation::Goto { unwind, .. } => {
@@ -676,12 +678,18 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         with_caller_location: bool,
         destination: &PlaceTy<'tcx, M::Provenance>,
         target: Option<mir::BasicBlock>,
-        unwind: mir::UnwindAction,
+        mut unwind: mir::UnwindAction,
     ) -> InterpResult<'tcx> {
         let _trace =
             enter_trace_span!(M, step::init_fn_call, tracing_separate_thread = Empty, ?fn_val)
                 .or_if_tracing_disabled(|| trace!("init_fn_call: {:#?}", fn_val));
 
+        // If the signature says this cannot unwind, reflect this in the unwind destination
+        // so that we don't have to check this later.
+        if caller_fn_abi.is_some_and(|abi| !abi.can_unwind) {
+            unwind = mir::UnwindAction::Unreachable;
+        }
+
         let instance = match fn_val {
             FnVal::Instance(instance) => instance,
             FnVal::Other(extra) => {

src/tools/miri/src/shims/foreign_items.rs

@@ -317,12 +317,8 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
             name if name == this.mangle_internal_symbol(NO_ALLOC_SHIM_IS_UNSTABLE) => {
                 // This is a no-op shim that only exists to prevent making the allocator shims
                 // instantly stable.
-                let [] = this.check_shim_sig(
-                    shim_sig_nounwind!(extern "Rust" fn() -> ()),
-                    link_name,
-                    abi,
-                    args,
-                )?;
+                let [] =
+                    this.check_shim_sig(shim_sig!(extern "Rust" fn() -> ()), link_name, abi, args)?;
             }
 
             // Miri-specific extern functions

src/tools/miri/src/shims/sig.rs

@@ -12,7 +12,6 @@ pub struct ShimSig<'tcx, const ARGS: usize> {
     pub abi: ExternAbi,
     pub args: [Ty<'tcx>; ARGS],
     pub ret: Ty<'tcx>,
-    pub nounwind: bool,
 }
 
 /// Construct a `ShimSig` with convenient syntax:
@@ -33,20 +32,6 @@ macro_rules! shim_sig {
             abi: std::str::FromStr::from_str($abi).expect("incorrect abi specified"),
             args: shim_sig_args_sep!(this, [$($args)*]),
             ret: shim_sig_arg!(this, $($ret)*),
-            nounwind: false,
-        }
-    };
-}
-
-/// Same as `shim_sig!` but promises that this function will not unwind, even if the ABI allows it.
-#[macro_export]
-macro_rules! shim_sig_nounwind {
-    (extern $abi:literal fn($($args:tt)*) -> $($ret:tt)*) => {
-        |this| $crate::shims::sig::ShimSig {
-            abi: std::str::FromStr::from_str($abi).expect("incorrect abi specified"),
-            args: shim_sig_args_sep!(this, [$($args)*]),
-            ret: shim_sig_arg!(this, $($ret)*),
-            nounwind: true,
         }
     };
 }
@@ -174,7 +159,6 @@ fn check_shim_abi<'tcx>(
     this: &MiriInterpCx<'tcx>,
     link_name: Symbol,
     callee_abi: &FnAbi<'tcx, Ty<'tcx>>,
-    callee_nounwind: bool,
     caller_abi: &FnAbi<'tcx, Ty<'tcx>>,
 ) -> InterpResult<'tcx> {
     if callee_abi.conv != caller_abi.conv {
@@ -184,12 +168,9 @@ fn check_shim_abi<'tcx>(
             caller = caller_abi.conv,
         );
     }
-    // FIXME: is this needed? Or is it enough to just check this if/when an actual unwind happens?
-    if callee_abi.can_unwind && !callee_nounwind && !caller_abi.can_unwind {
-        throw_ub_format!(
-            "ABI mismatch: callee may unwind, but caller asumes that no unwinding will occur",
-        );
-    }
+    // No need to check unwinding: if the caller signature forbids unwinding, that's already
+    // reflected in the unwind destination so if an unwind occurs it will be reported as UB.
+
     if caller_abi.c_variadic && !callee_abi.c_variadic {
         throw_ub_format!(
             "ABI mismatch: `{link_name}` is a non-variadic function, but the caller is using a variadic signature"
@@ -314,7 +295,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let callee_fn_abi = this.fn_abi_of_fn_ptr(fn_sig_binder, Default::default())?;
 
         // Check everything.
-        check_shim_abi(this, link_name, callee_fn_abi, shim_sig.nounwind, caller_fn_abi)?;
+        check_shim_abi(this, link_name, callee_fn_abi, caller_fn_abi)?;
         this.check_shim_symbol_clash(link_name)?;
 
         // Return arguments.

src/tools/miri/tests/pass/function_calls/unwind_abi_mismatch.rs

@@ -0,0 +1,29 @@
+#![feature(rustc_attrs)]
+
+#[unsafe(no_mangle)]
+extern "C-unwind" fn does_not_unwind_but_could() {}
+
+fn main() {
+    // Calling a maybe-unwinding function with a non-unwinding ABI is okay if the function
+    // does not actually unwind. See `tests/fail/panic/bad_unwind.rs` for the dual test that is UB.
+    let f: extern "C-unwind" fn() = does_not_unwind_but_could;
+    let f: extern "C" fn() = unsafe { std::mem::transmute(f) };
+    f();
+
+    // The same applies when we call such a function via an extern import.
+    // This is the dual to `tests/fail/function_calls/exported_symbol_bad_unwind1.rs`.
+    extern "C" {
+        #[link_name = "does_not_unwind_but_could"]
+        fn imported();
+    }
+    unsafe { imported() };
+
+    // The same does for shims: we can invoke maybe-unwinding shims via a non-unwinding declaration.
+    // We don't have "C-unwind" shims that we could import with "C" so the closest thing we can test
+    // are "Rust" shims imported via `#[rustc_nounwind]`.
+    extern "Rust" {
+        #[rustc_nounwind]
+        pub fn miri_spin_loop();
+    }
+    unsafe { miri_spin_loop() };
+}