Merge pull request #162 from madsmtm/block-fixes

Smaller `block2` soundness fixes

Author: madsmtm

block-sys/CHANGELOG.md

@@ -12,6 +12,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   scripts becomes `DEP_BLOCK_0_0_CC_ARGS`).
 * **BREAKING**: Apple's runtime is now always the default.
 
+### Fixed
+* **BREAKING**: Tweak the types of a lot of fields and arguments.
+
 
 ## 0.0.3 - 2022-01-03
 

block-sys/src/lib.rs

@@ -9,6 +9,9 @@
 //!
 //! [ABI]: https://clang.llvm.org/docs/Block-ABI-Apple.html
 
+// TODO: Replace `extern "C"` with `extern "C-unwind"` where applicable.
+// See https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html.
+
 #![no_std]
 // Update in Cargo.toml as well.
 #![doc(html_root_url = "https://docs.rs/block-sys/0.0.3")]
@@ -244,7 +247,7 @@ pub struct Block_layout {
     /// Class pointer. Always initialised to &_NSConcreteStackBlock for blocks
     /// that are created on the stack or &_NSConcreteGlobalBlock for blocks
     /// that are created in global storage.
-    pub isa: *mut Class,
+    pub isa: *const Class,
     /// Flags.
     /// See the `block_flags` enumerated type for possible values.
     /// Contains ref count in Apple and ObjFW.
@@ -259,7 +262,7 @@ pub struct Block_layout {
     /// parameters. If the BLOCK_USE_SRET & BLOCK_HAS_SIGNATURE flag is set,
     /// there is an additional hidden argument, which is a pointer to the
     /// space on the stack allocated to hold the return value.
-    pub invoke: Option<unsafe extern "C" fn(block: *mut Block_layout, ...)>,
+    pub invoke: Option<unsafe extern "C" fn()>,
     /// The block's descriptor. The actual type of this is:
     /// ```pseudo-code
     /// match (BLOCK_HAS_COPY_DISPOSE, BLOCK_HAS_SIGNATURE) {
@@ -271,7 +274,8 @@ pub struct Block_layout {
     /// ```
     ///
     /// But it is safe to access this through just `Block_descriptor_header`.
-    pub descriptor: *mut Block_descriptor_header,
+    // Note: Important to use `*const c_void` until we know which type it is!
+    pub descriptor: *const c_void,
 }
 
 #[repr(C)]
@@ -288,12 +292,13 @@ pub struct Block_descriptor_header {
 #[repr(C)]
 pub struct Block_descriptor {
     pub header: Block_descriptor_header,
+
     /// Copy function, generated by the compiler to help copy the block if it
     /// contains nontrivial copy operations.
-    pub copy: Option<unsafe extern "C" fn(dst: *mut Block_byref, src: *mut Block_byref)>,
+    pub copy: Option<unsafe extern "C" fn(dst: *mut c_void, src: *mut c_void)>,
     /// Dispose function, generated by the compiler to help copy the block if
     /// it contains nontrivial destructors.
-    pub dispose: Option<unsafe extern "C" fn(src: *mut Block_byref)>,
+    pub dispose: Option<unsafe extern "C" fn(src: *mut c_void)>,
 }
 
 /// Extended block descriptor that does not contain copy and dispose helper
@@ -312,7 +317,12 @@ pub struct Block_descriptor_basic {
 /// Requires BLOCK_HAS_COPY_DISPOSE and BLOCK_HAS_SIGNATURE
 #[repr(C)]
 pub struct Block_descriptor_with_signature {
-    pub inner: Block_descriptor,
+    pub header: Block_descriptor_header,
+
+    /// Same as [`Block_descriptor::copy`].
+    pub copy: Option<unsafe extern "C" fn(dst: *mut c_void, src: *mut c_void)>,
+    /// Same as [`Block_descriptor::dispose`].
+    pub dispose: Option<unsafe extern "C" fn(src: *mut c_void)>,
 
     /// Objective-C type encoding of the block.
     #[doc(alias = "signature")]
@@ -336,7 +346,7 @@ pub struct Block_descriptor_with_signature {
 //     pub isa: *mut c_void,
 //     pub Block_flags: i32,
 //     pub Block_size: i32,
-//     pub Block_invoke: Option<unsafe extern "C" fn(block: *mut c_void)>,
+//     pub Block_invoke: Option<unsafe extern "C" fn()>,
 //     pub Block_copy: Option<unsafe extern "C" fn(dst: *mut c_void, src: *mut c_void)>,
 //     pub Block_dispose: Option<unsafe extern "C" fn(block: *mut c_void)>,
 // }
@@ -348,7 +358,7 @@ pub struct Block_descriptor_with_signature {
 pub struct Block_byref_header {
     /// Class pointer. Currently unused on GNUStep and always NULL. Could be
     /// used in the future to support introspection.
-    pub isa: *mut c_void,
+    pub isa: *const Class,
     /// The pointer to the structure that contains the real version of the
     /// data. All accesses go via this pointer. If an on-stack byref structure
     /// is copied to the heap, then its forwarding pointer should point to the
@@ -374,10 +384,9 @@ pub struct Block_byref_header {
 pub struct Block_byref {
     pub header: Block_byref_header,
     /// Copy function.
-    pub keep:
-        Option<unsafe extern "C" fn(dst: *mut Block_byref_header, src: *mut Block_byref_header)>,
+    pub keep: Option<unsafe extern "C" fn(dst: *mut c_void, src: *mut c_void)>,
     /// Dispose function.
-    pub destroy: Option<unsafe extern "C" fn(src: *mut Block_byref_header)>,
+    pub destroy: Option<unsafe extern "C" fn(src: *mut c_void)>,
 }
 
 #[cfg(feature = "apple")]
@@ -387,7 +396,11 @@ pub struct Block_byref {
 #[repr(C)]
 #[doc(alias = "Block_byref_3")]
 pub struct Block_byref_extended {
-    pub inner: Block_byref,
+    pub header: Block_byref_header,
+    /// Same as [`Block_byref::keep`].
+    pub keep: Option<unsafe extern "C" fn(dst: *mut c_void, src: *mut c_void)>,
+    /// Same as [`Block_byref::destroy`].
+    pub destroy: Option<unsafe extern "C" fn(src: *mut c_void)>,
     pub layout: *const c_char,
 }
 
@@ -399,41 +412,27 @@ mod tests {
 
     #[test]
     fn smoke() {
-        assert_eq!(unsafe { _Block_copy(ptr::null()) }, ptr::null_mut());
+        assert_eq!(unsafe { _Block_copy(ptr::null_mut()) }, ptr::null_mut());
     }
 
     #[test]
     fn test_linkable() {
         println!("{:p}", unsafe { &_NSConcreteGlobalBlock });
         println!("{:p}", unsafe { &_NSConcreteStackBlock });
         println!("{:p}", unsafe { &_NSConcreteMallocBlock });
+        println!("{:p}", _Block_copy as unsafe extern "C" fn(_) -> _);
         println!(
             "{:p}",
-            _Block_copy as unsafe extern "C" fn(*const c_void) -> *mut c_void
-        );
-        println!(
-            "{:p}",
-            _Block_object_assign
-                as unsafe extern "C" fn(*mut c_void, *const c_void, block_assign_dispose_flags)
-        );
-        println!(
-            "{:p}",
-            _Block_object_dispose
-                as unsafe extern "C" fn(*const c_void, block_assign_dispose_flags)
-        );
-        println!(
-            "{:p}",
-            _Block_release as unsafe extern "C" fn(*const c_void)
+            _Block_object_assign as unsafe extern "C" fn(_, _, _)
         );
+        println!("{:p}", _Block_object_dispose as unsafe extern "C" fn(_, _));
+        println!("{:p}", _Block_release as unsafe extern "C" fn(_));
         #[cfg(any(feature = "apple", feature = "compiler-rt"))]
         {
             println!("{:p}", unsafe { &_NSConcreteAutoBlock });
             println!("{:p}", unsafe { &_NSConcreteFinalizingBlock });
             println!("{:p}", unsafe { &_NSConcreteWeakBlockVariable });
-            println!(
-                "{:p}",
-                Block_size as unsafe extern "C" fn(*mut c_void) -> c_ulong
-            );
+            println!("{:p}", Block_size as unsafe extern "C" fn(_) -> _);
         }
     }
 }

block2/CHANGELOG.md

@@ -14,6 +14,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * Improved documentation.
 * **BREAKING**: Updated `ffi` module to `block-sys v0.0.3`
 
+### Removed
+* **BREAKING**: Removed `DerefMut` implementation for `ConcreteBlock`.
+
 
 ## 0.2.0-alpha.2 - 2021-12-22
 

block2/src/global.rs

@@ -1,3 +1,4 @@
+use core::ffi::c_void;
 use core::marker::PhantomData;
 use core::mem;
 use core::ops::Deref;
@@ -61,7 +62,7 @@ impl<A, R> GlobalBlock<A, R> {
         reserved: 0,
         // Populated in `global_block!`
         invoke: None,
-        descriptor: &GLOBAL_DESCRIPTOR as *const _ as *mut _,
+        descriptor: &GLOBAL_DESCRIPTOR as *const ffi::Block_descriptor_header as *mut c_void,
     };
 
     /// Use the [`global_block`] macro instead.
@@ -81,7 +82,7 @@ where
 {
     type Target = Block<A, R>;
 
-    fn deref(&self) -> &Block<A, R> {
+    fn deref(&self) -> &Self::Target {
         // TODO: SAFETY
         unsafe { &*(self as *const Self as *const Block<A, R>) }
     }
@@ -168,7 +169,7 @@ macro_rules! global_block {
         #[allow(unused_unsafe)]
         $vis static $name: $crate::GlobalBlock<($($t,)*) $(, $r)?> = unsafe {
             let mut layout = $crate::GlobalBlock::<($($t,)*) $(, $r)?>::__DEFAULT_LAYOUT;
-            layout.isa = &$crate::ffi::_NSConcreteGlobalBlock as *const _ as *mut _;
+            layout.isa = &$crate::ffi::_NSConcreteGlobalBlock;
             layout.invoke = ::core::option::Option::Some({
                 unsafe extern "C" fn inner(_: *mut $crate::ffi::Block_layout, $($a: $t),*) $(-> $r)? {
                     $body