Use extern "C-unwind"

C-unwind was added in Rust 1.71, and allows panicking/unwinding /exceptions across foreign function interfaces. Additionally, Rust decided to let handling of foreign unwinds be implementation defined behavior (instead of undefined), so we can now mark `throw` as safe, see rust-lang/rust#128321. This has a cost in that we now have landing pads on every message send; this is strictly the correct choice, though, so we will have to bear with it. Fixes #539.
madsmtm · Sep 12, 2024 · 087f8c1 · 087f8c1
1 parent dfb7562
commit 087f8c1
Show file tree

Hide file tree

Showing 66 changed files with 4,212 additions and 1,140 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -492,7 +492,7 @@ jobs:
 
     - name: Test Foundation with unstable features
       if: ${{ matrix.nightly }}
-      run: cargo test $ARGS $PUBLIC_CRATES -ptests $INTERESTING_FEATURES -pobjc2-foundation --features=catch-all,unstable-autoreleasesafe,unstable-c-unwind ${{ matrix.sdk != '10.12' && '--features=unstable-simd' || '' }}
+      run: cargo test $ARGS $PUBLIC_CRATES -ptests $INTERESTING_FEATURES -pobjc2-foundation --features=catch-all,unstable-autoreleasesafe ${{ matrix.sdk != '10.12' && '--features=unstable-simd' || '' }}
 
     # TODO: Re-enable this on all of Foundation once we do some form of
     # availability checking.

diff --git a/Cargo.toml b/Cargo.toml
@@ -41,9 +41,6 @@ ptr_as_ptr = "warn"
 inherits = "release"
 # Enable LTO to allow testing the `unstable-static-sel-inlined` feature
 lto = true
-# Don't emit unwind info; while important to get right, the control flow is
-# very hard to glean from assembly output.
-panic = "abort"
 
 # Release data for framework crates
 [workspace.metadata.release]

diff --git a/crates/block2/CHANGELOG.md b/crates/block2/CHANGELOG.md
@@ -73,6 +73,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * **BREAKING**: Fixed `GlobalBlock` not having the correct variance. This may
   break if you were using lifetimes in your parameters, as those are now a bit
   too restrictive.
+* **BREAKING**: Converted function signatures into using `extern "C-unwind"`.
+  This allows unwinding through blocks.
 
 
 ## 0.4.0 - 2023-12-03

diff --git a/crates/block2/Cargo.toml b/crates/block2/Cargo.toml
@@ -41,14 +41,6 @@ unstable-winobjc = ["gnustep-1-8"]
 # Link to ObjFW.
 unstable-objfw = []
 
-# Uses `extern "C-unwind"` on relevant function declarations.
-#
-# This raises MSRV to `1.71`.
-#
-# Warning: Enabling this is a breaking change for consumer crates, as it
-# changes the signature of functions.
-unstable-c-unwind = []
-
 # Expose private ffi functions and statics.
 unstable-private = []
 

diff --git a/crates/block2/src/abi.rs b/crates/block2/src/abi.rs
@@ -199,7 +199,7 @@ pub struct BlockHeader {
     /// If the BLOCK_USE_SRET & BLOCK_HAS_SIGNATURE flag is set, there is an
     /// additional hidden parameter, which is a pointer to the space on the
     /// stack allocated to hold the return value.
-    pub invoke: Option<crate::__c_unwind!(unsafe extern "C" fn())>,
+    pub invoke: Option<unsafe extern "C-unwind" fn()>,
     /// The block's descriptor.
     pub(crate) descriptor: BlockDescriptorPtr,
 }
@@ -257,13 +257,12 @@ pub(crate) struct BlockDescriptorCopyDispose {
     ///
     /// This may be NULL since macOS 11.0.1 in Apple's runtime, but this
     /// should not be relied on.
-    pub(crate) copy:
-        Option<crate::__c_unwind!(unsafe extern "C" fn(dst: *mut c_void, src: *const c_void))>,
+    pub(crate) copy: Option<unsafe extern "C-unwind" fn(dst: *mut c_void, src: *const c_void)>,
     /// Helper to destroy the block after being copied.
     ///
     /// This may be NULL since macOS 11.0.1 in Apple's runtime, but this
     /// should not be relied on.
-    pub(crate) dispose: Option<crate::__c_unwind!(unsafe extern "C" fn(src: *mut c_void))>,
+    pub(crate) dispose: Option<unsafe extern "C-unwind" fn(src: *mut c_void)>,
 }
 
 /// Block descriptor that has an encoding / a signature.
@@ -303,13 +302,12 @@ pub(crate) struct BlockDescriptorCopyDisposeSignature {
     ///
     /// This may be NULL since macOS 11.0.1 in Apple's runtime, but this
     /// should not be relied on.
-    pub(crate) copy:
-        Option<crate::__c_unwind!(unsafe extern "C" fn(dst: *mut c_void, src: *const c_void))>,
+    pub(crate) copy: Option<unsafe extern "C-unwind" fn(dst: *mut c_void, src: *const c_void)>,
     /// Helper to destroy the block after being copied.
     ///
     /// This may be NULL since macOS 11.0.1 in Apple's runtime, but this
     /// should not be relied on.
-    pub(crate) dispose: Option<crate::__c_unwind!(unsafe extern "C" fn(src: *mut c_void))>,
+    pub(crate) dispose: Option<unsafe extern "C-unwind" fn(src: *mut c_void)>,
 
     /// Objective-C type encoding of the block.
     #[doc(alias = "signature")]

diff --git a/crates/block2/src/ffi.rs b/crates/block2/src/ffi.rs
@@ -5,30 +5,6 @@ use core::ffi::c_int;
 use core::ffi::c_void;
 use core::marker::{PhantomData, PhantomPinned};
 
-#[cfg(not(feature = "unstable-c-unwind"))]
-#[doc(hidden)]
-#[macro_export]
-macro_rules! __c_unwind {
-    (unsafe extern "C" $($t:tt)*) => {
-        unsafe extern "C" $($t)*
-    };
-    (extern "C" $($t:tt)*) => {
-        extern "C" $($t)*
-    };
-}
-
-#[cfg(feature = "unstable-c-unwind")]
-#[doc(hidden)]
-#[macro_export]
-macro_rules! __c_unwind {
-    (unsafe extern "C" $($t:tt)*) => {
-        unsafe extern "C-unwind" $($t)*
-    };
-    (extern "C" $($t:tt)*) => {
-        extern "C-unwind" $($t)*
-    };
-}
-
 /// Type for block class ISAs.
 ///
 /// This will likely become an extern type in the future.
@@ -55,26 +31,8 @@ pub struct Class {
     _opaque: UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>,
 }
 
-#[cfg(not(feature = "unstable-c-unwind"))]
-macro_rules! extern_c_unwind {
-    ($($t:tt)*) => {
-        extern "C" {
-            $($t)*
-        }
-    };
-}
-
-#[cfg(feature = "unstable-c-unwind")]
-macro_rules! extern_c_unwind {
-    ($($t:tt)*) => {
-        extern "C-unwind" {
-            $($t)*
-        }
-    };
-}
-
 // Use `extern "C-unwind"`, runtime functions may call external routines.
-extern_c_unwind! {
+extern "C-unwind" {
     /// Class ISA used for global blocks.
     pub static _NSConcreteGlobalBlock: Class;
 
@@ -126,7 +84,7 @@ pub mod private {
     #[cfg(any(doc, target_vendor = "apple", feature = "compiler-rt"))]
     use core::ffi::c_ulong;
 
-    extern_c_unwind! {
+    extern "C-unwind" {
         pub static _NSConcreteMallocBlock: Class;
         #[cfg(any(doc, target_vendor = "apple", feature = "compiler-rt"))]
         pub static _NSConcreteAutoBlock: Class;
@@ -203,21 +161,15 @@ mod tests {
         println!("{:?}", unsafe {
             ptr::addr_of!(private::_NSConcreteMallocBlock)
         });
+        println!("{:p}", _Block_copy as unsafe extern "C-unwind" fn(_) -> _);
         println!(
             "{:p}",
-            _Block_copy as __c_unwind!(unsafe extern "C" fn(_) -> _)
-        );
-        println!(
-            "{:p}",
-            _Block_object_assign as __c_unwind!(unsafe extern "C" fn(_, _, _))
-        );
-        println!(
-            "{:p}",
-            _Block_object_dispose as __c_unwind!(unsafe extern "C" fn(_, _))
+            _Block_object_assign as unsafe extern "C-unwind" fn(_, _, _)
         );
         println!(
             "{:p}",
-            _Block_release as __c_unwind!(unsafe extern "C" fn(_))
+            _Block_object_dispose as unsafe extern "C-unwind" fn(_, _)
         );
+        println!("{:p}", _Block_release as unsafe extern "C-unwind" fn(_));
     }
 }
diff --git a/crates/block2/src/global.rs b/crates/block2/src/global.rs
@@ -187,17 +187,17 @@ macro_rules! global_block {
             let mut header = $crate::GlobalBlock::<dyn Fn($($t),*) $(-> $r)? + 'static>::__DEFAULT_HEADER;
             header.isa = ::core::ptr::addr_of!($crate::ffi::_NSConcreteGlobalBlock);
             header.invoke = ::core::option::Option::Some({
-                $crate::__c_unwind!(unsafe extern "C" fn inner(
+                unsafe extern "C-unwind" fn inner(
                     _: *mut $crate::GlobalBlock<dyn Fn($($t),*) $(-> $r)? + 'static>,
                     $($a: $t),*
                 ) $(-> $r)? {
                     $body
-                });
+                };
 
                 // TODO: SAFETY
                 ::core::mem::transmute::<
-                    $crate::__c_unwind!(unsafe extern "C" fn(*mut $crate::GlobalBlock<dyn Fn($($t),*) $(-> $r)? + 'static>, $($a: $t),*) $(-> $r)?),
-                    $crate::__c_unwind!(unsafe extern "C" fn()),
+                    unsafe extern "C-unwind" fn(*mut $crate::GlobalBlock<dyn Fn($($t),*) $(-> $r)? + 'static>, $($a: $t),*) $(-> $r)?,
+                    unsafe extern "C-unwind" fn(),
                 >(inner)
             });
             $crate::GlobalBlock::from_header(header)

diff --git a/crates/block2/src/stack.rs b/crates/block2/src/stack.rs
@@ -77,7 +77,7 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> {
     const SIZE: c_ulong = mem::size_of::<Self>() as _;
 
     // Drop the closure that this block contains.
-    crate::__c_unwind! {unsafe extern "C" fn drop_closure(block: *mut c_void) {
+    unsafe extern "C-unwind" fn drop_closure(block: *mut c_void) {
         let block: *mut Self = block.cast();
         // When this function is called, the block no longer lives on the
         // stack, it has been moved to the heap as part of some `_Block_copy`
@@ -97,7 +97,7 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> {
         // part of some `_Block_copy` operation, and as such it is valid to
         // drop here.
         unsafe { ptr::drop_in_place(closure) };
-    }}
+    }
 
     const DESCRIPTOR_BASIC: BlockDescriptor = BlockDescriptor {
         reserved: 0,
@@ -108,7 +108,7 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> {
 // `StackBlock::new`
 impl<'f, A, R, Closure: Clone> StackBlock<'f, A, R, Closure> {
     // Clone the closure from one block to another.
-    crate::__c_unwind! {unsafe extern "C" fn clone_closure(dst: *mut c_void, src: *const c_void) {
+    unsafe extern "C-unwind" fn clone_closure(dst: *mut c_void, src: *const c_void) {
         let dst: *mut Self = dst.cast();
         let src: *const Self = src.cast();
         // When this function is called as part of some `_Block_copy`
@@ -132,7 +132,7 @@ impl<'f, A, R, Closure: Clone> StackBlock<'f, A, R, Closure> {
         // already `memmove`d data once more, which is unnecessary for closure
         // captures that implement `Copy`.
         unsafe { ptr::write(dst_closure, src_closure.clone()) };
-    }}
+    }
 
     const DESCRIPTOR_WITH_CLONE: BlockDescriptorCopyDispose = BlockDescriptorCopyDispose {
         reserved: 0,
@@ -198,10 +198,10 @@ impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> {
 
 // `RcBlock::new`
 impl<'f, A, R, Closure> StackBlock<'f, A, R, Closure> {
-    crate::__c_unwind! {unsafe extern "C" fn empty_clone_closure(_dst: *mut c_void, _src: *const c_void) {
+    unsafe extern "C-unwind" fn empty_clone_closure(_dst: *mut c_void, _src: *const c_void) {
         // We do nothing, the closure has been `memmove`'d already, and
         // ownership will be passed in `RcBlock::new`.
-    }}
+    }
 
     const DESCRIPTOR_WITH_DROP: BlockDescriptorCopyDispose = BlockDescriptorCopyDispose {
         reserved: 0,

diff --git a/crates/block2/src/traits.rs b/crates/block2/src/traits.rs
@@ -34,7 +34,7 @@ pub unsafe trait BlockFn: private::Sealed<Self::Args, Self::Output> {
     /// Calls the given invoke function with the block and arguments.
     #[doc(hidden)]
     unsafe fn __call_block(
-        invoke: crate::__c_unwind!(unsafe extern "C" fn()),
+        invoke: unsafe extern "C-unwind" fn(),
         block: *mut Block<Self>,
         args: Self::Args,
     ) -> Self::Output;
@@ -60,7 +60,7 @@ where
     type Dyn: ?Sized + BlockFn<Args = A, Output = R>;
 
     #[doc(hidden)]
-    fn __get_invoke_stack_block() -> crate::__c_unwind!(unsafe extern "C" fn());
+    fn __get_invoke_stack_block() -> unsafe extern "C-unwind" fn();
 }
 
 macro_rules! impl_traits {
@@ -77,12 +77,12 @@ macro_rules! impl_traits {
 
             #[inline]
             unsafe fn __call_block(
-                invoke: crate::__c_unwind!(unsafe extern "C" fn()),
+                invoke: unsafe extern "C-unwind" fn(),
                 block: *mut Block<Self>,
                 ($($a,)*): Self::Args,
             ) -> Self::Output {
                 // Very similar to `MessageArguments::__invoke`
-                let invoke: unsafe extern "C" fn(*mut Block<Self> $(, $t)*) -> R = unsafe {
+                let invoke: unsafe extern "C-unwind" fn(*mut Block<Self> $(, $t)*) -> R = unsafe {
                     mem::transmute(invoke)
                 };
 
@@ -99,8 +99,8 @@ macro_rules! impl_traits {
             type Dyn = dyn Fn($($t),*) -> R + 'f;
 
             #[inline]
-            fn __get_invoke_stack_block() -> crate::__c_unwind!(unsafe extern "C" fn()) {
-                crate::__c_unwind!(unsafe extern "C" fn invoke<'f, $($t,)* R, Closure>(
+            fn __get_invoke_stack_block() -> unsafe extern "C-unwind" fn() {
+                unsafe extern "C-unwind" fn invoke<'f, $($t,)* R, Closure>(
                     block: *mut StackBlock<'f, ($($t,)*), R, Closure>,
                     $($a: $t,)*
                 ) -> R
@@ -109,12 +109,12 @@ macro_rules! impl_traits {
                 {
                     let closure = unsafe { &*ptr::addr_of!((*block).closure) };
                     (closure)($($a),*)
-                });
+                }
 
                 unsafe {
                     mem::transmute::<
-                        crate::__c_unwind!(unsafe extern "C" fn(*mut StackBlock<'f, ($($t,)*), R, Closure>, $($t,)*) -> R),
-                        crate::__c_unwind!(unsafe extern "C" fn()),
+                        unsafe extern "C-unwind" fn(*mut StackBlock<'f, ($($t,)*), R, Closure>, $($t,)*) -> R,
+                        unsafe extern "C-unwind" fn(),
                     >(invoke)
                 }
             }

diff --git a/crates/header-translator/src/rust_type.rs b/crates/header-translator/src/rust_type.rs
@@ -1279,7 +1279,11 @@ impl Ty {
                         if *nullability != Nullability::NonNull {
                             write!(f, "Option<")?;
                         }
-                        write!(f, "unsafe extern \"C\" fn(")?;
+                        // Allow pointers that the user provides to unwind.
+                        //
+                        // This is not _necessarily_ safe, though in practice
+                        // it will be for all of Apple's frameworks.
+                        write!(f, "unsafe extern \"C-unwind\" fn(")?;
                         for arg in arguments {
                             write!(f, "{},", arg.plain())?;
                         }

diff --git a/crates/header-translator/src/stmt.rs b/crates/header-translator/src/stmt.rs
@@ -2370,7 +2370,9 @@ impl Stmt {
                     body: None,
                     safe: false,
                 } => {
-                    writeln!(f, "extern \"C\" {{")?;
+                    // Functions are always C-unwind, since we don't know
+                    // anything about them.
+                    writeln!(f, "extern \"C-unwind\" {{")?;
 
                     write!(f, "    {}", self.cfg_gate_ln(config))?;
                     write!(f, "    {availability}")?;
@@ -2395,14 +2397,14 @@ impl Stmt {
                     write!(f, "{}", self.cfg_gate_ln(config))?;
                     write!(f, "{availability}")?;
                     writeln!(f, "#[inline]")?;
-                    write!(f, "pub extern \"C\" fn {}(", id.name)?;
+                    write!(f, "pub extern \"C-unwind\" fn {}(", id.name)?;
                     for (param, arg_ty) in arguments {
                         let param = handle_reserved(&crate::to_snake_case(param));
                         write!(f, "{param}: {},", arg_ty.fn_argument())?;
                     }
                     writeln!(f, "){} {{", result_type.fn_return())?;
 
-                    writeln!(f, "    extern \"C\" {{")?;
+                    writeln!(f, "    extern \"C-unwind\" {{")?;
 
                     write!(f, "        fn {}(", id.name)?;
                     for (param, arg_ty) in arguments {

diff --git a/crates/objc2-exception-helper/Cargo.toml b/crates/objc2-exception-helper/Cargo.toml
@@ -38,14 +38,6 @@ gnustep-1-9 = ["gnustep-1-8"]
 gnustep-2-0 = ["gnustep-1-9"]
 gnustep-2-1 = ["gnustep-2-0"]
 
-# Uses `extern "C-unwind"` on relevant function declarations.
-#
-# This raises MSRV to `1.71`.
-#
-# Warning: Enabling this is a breaking change for consumer crates, as it
-# changes the signature of functions.
-unstable-c-unwind = []
-
 [build-dependencies]
 cc = "1.0.80"
 

diff --git a/crates/objc2-exception-helper/src/lib.rs b/crates/objc2-exception-helper/src/lib.rs
@@ -18,9 +18,6 @@ extern crate std;
 
 use core::ffi::c_void;
 
-#[cfg(not(feature = "unstable-c-unwind"))]
-type TryCatchClosure = extern "C" fn(*mut c_void);
-#[cfg(feature = "unstable-c-unwind")]
 type TryCatchClosure = extern "C-unwind" fn(*mut c_void);
 
 // `try_catch` is deliberately `extern "C"`, we just prevented the unwind.
@@ -59,11 +56,6 @@ mod tests {
 
         static VALUE: SyncPtr = SyncPtr(&VALUE.0 as *const *mut c_void as *mut c_void);
 
-        #[cfg(not(feature = "unstable-c-unwind"))]
-        extern "C" fn check_value(value: *mut c_void) {
-            assert_eq!(VALUE.0, value);
-        }
-        #[cfg(feature = "unstable-c-unwind")]
         extern "C-unwind" fn check_value(value: *mut c_void) {
             assert_eq!(VALUE.0, value);
         }

diff --git a/crates/objc2/CHANGELOG.md b/crates/objc2/CHANGELOG.md
@@ -120,6 +120,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Fixed
 * Remove an incorrect assertion when adding protocols to classes in an unexpected
   order.
+* **BREAKING**: Converted function signatures into using `extern "C-unwind"`
+  where applicable. This allows Rust and Objective-C unwinding to interoperate.
 
 
 ## 0.5.2 - 2024-05-21