From c1f3ab0fa0647bc89366765edd5506a0cd4702ea Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 6 Jan 2024 13:53:00 +0100 Subject: [PATCH 1/9] Remove byref helper structure It is unused, and will likely need to be changed before it is useful. --- crates/block2/src/abi.rs | 53 ---------------------------------------- crates/block2/src/lib.rs | 9 +++++++ 2 files changed, 9 insertions(+), 53 deletions(-) diff --git a/crates/block2/src/abi.rs b/crates/block2/src/abi.rs index fe5ab024e..82b72d5da 100644 --- a/crates/block2/src/abi.rs +++ b/crates/block2/src/abi.rs @@ -257,56 +257,3 @@ pub struct Block_descriptor_with_signature { // pub Block_dispose: Option, // } // Example usage: https://github.com/apple-oss-distributions/libdispatch/blob/libdispatch-84.5/src/once.c - -/// Structure used for on-stack variables that are referenced by blocks. -#[repr(C)] -#[doc(alias = "Block_byref_1")] -#[doc(alias = "_block_byref")] -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: *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 - /// heap version. Otherwise it should point to itself. - pub forwarding: *mut Block_byref_header, - /// Flags and reference count. - /// - /// TODO: Volatile! - pub flags: block_flags, - #[cfg(feature = "apple")] - /// Size of this structure. - pub size: u32, - #[cfg(not(feature = "apple"))] - /// Size of this structure. - pub size: i32, -} - -/// Structure used for on-stack variables that are referenced by blocks. -/// -/// requires BLOCK_BYREF_HAS_COPY_DISPOSE -#[repr(C)] -#[doc(alias = "Block_byref_2")] -pub struct Block_byref { - pub header: Block_byref_header, - /// Copy function. - pub keep: Option, - /// Dispose function. - pub destroy: Option, -} - -#[cfg(any(doc, feature = "apple"))] -/// Structure used for on-stack variables that are referenced by blocks. -/// -/// requires BLOCK_BYREF_LAYOUT_EXTENDED -#[repr(C)] -#[doc(alias = "Block_byref_3")] -pub struct Block_byref_extended { - pub header: Block_byref_header, - /// Same as [`Block_byref::keep`]. - pub keep: Option, - /// Same as [`Block_byref::destroy`]. - pub destroy: Option, - pub layout: *const c_char, -} diff --git a/crates/block2/src/lib.rs b/crates/block2/src/lib.rs index 94c6c8b55..02dc50763 100644 --- a/crates/block2/src/lib.rs +++ b/crates/block2/src/lib.rs @@ -248,3 +248,12 @@ pub use block::{Block, BlockArguments}; pub use concrete_block::{ConcreteBlock, IntoConcreteBlock}; pub use global::GlobalBlock; pub use rc_block::RcBlock; + +// Note: We could use `_Block_object_assign` and `_Block_object_dispose` to +// implement a `ByRef` wrapper, which would behave like `__block` marked +// variables and have roughly the same memory management scheme as blocks. +// +// But I've yet to see the value in such a wrapper in Rust code compared to +// just using `Box`, `Rc` or `Arc`, and since `__block` variables are +// basically never exposed as part of a (public) function's API, we won't +// implement such a thing yet. From 1199ca856360e10a5a35f6827d4fb855cf5c3768 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 6 Jan 2024 13:55:44 +0100 Subject: [PATCH 2/9] Fix type of flags to be `c_int` --- crates/block2/src/abi.rs | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/crates/block2/src/abi.rs b/crates/block2/src/abi.rs index 82b72d5da..660d7a1dc 100644 --- a/crates/block2/src/abi.rs +++ b/crates/block2/src/abi.rs @@ -11,14 +11,14 @@ )] use core::ffi::c_void; -use std::os::raw::{c_char, c_ulong}; +use std::os::raw::{c_char, c_int, c_ulong}; use crate::ffi::Class; /// Block descriptor flags. /// Values for Block_layout->flags to describe block objects #[allow(non_camel_case_types)] -pub type block_flags = i32; +pub type block_flags = c_int; #[cfg(any(doc, feature = "apple"))] pub const BLOCK_DEALLOCATING: block_flags = 0x0001; @@ -79,7 +79,8 @@ pub const BLOCK_IS_GLOBAL: block_flags = 1 << 28; /// /// See #[doc(alias = "BLOCK_USE_SRET")] -#[doc(alias = "BLOCK_HAS_DESCRIPTOR")] // compiler-rt || macOS 10.6 +#[doc(alias = "BLOCK_HAS_DESCRIPTOR")] +// compiler-rt || macOS 10.6 pub const BLOCK_USE_STRET: block_flags = 1 << 29; /// Block has an Objective-C type encoding. @@ -90,51 +91,35 @@ pub const BLOCK_HAS_SIGNATURE: block_flags = 1 << 30; /// compiler pub const BLOCK_HAS_EXTENDED_LAYOUT: block_flags = 1 << 31; -/// Flags used in the final argument to _Block_object_assign() and -/// _Block_object_dispose(). These indicate the type of copy or dispose to -/// perform. -/// Values for _Block_object_assign() and _Block_object_dispose() parameters -/// -/// This is a helper type, in the sources this type does not have a name! -#[allow(non_camel_case_types)] -pub type block_assign_dispose_flags = i32; - /// The value is of some id-like type, and should be copied as an Objective-C /// object: i.e. by sending -retain or via the GC assign functions in GC mode /// (not yet supported). /// /// id, NSObject, __attribute__((NSObject)), block, ... -pub const BLOCK_FIELD_IS_OBJECT: block_assign_dispose_flags = 3; +pub const BLOCK_FIELD_IS_OBJECT: c_int = 3; /// The field is a block. This must be copied by the block copy functions. /// /// a block variable -pub const BLOCK_FIELD_IS_BLOCK: block_assign_dispose_flags = 7; +pub const BLOCK_FIELD_IS_BLOCK: c_int = 7; /// The field is an indirect reference to a variable declared with the __block /// storage qualifier. /// /// the on stack structure holding the __block variable -pub const BLOCK_FIELD_IS_BYREF: block_assign_dispose_flags = 8; +pub const BLOCK_FIELD_IS_BYREF: c_int = 8; /// The field is an indirect reference to a variable declared with the __block /// storage qualifier. /// /// declared __weak, only used in byref copy helpers -pub const BLOCK_FIELD_IS_WEAK: block_assign_dispose_flags = 16; +pub const BLOCK_FIELD_IS_WEAK: c_int = 16; /// The field is an indirect reference to a variable declared with the __block /// storage qualifier. /// /// called from __block (byref) copy/dispose support routines. -pub const BLOCK_BYREF_CALLER: block_assign_dispose_flags = 128; - -#[cfg(any(doc, feature = "apple"))] -pub const BLOCK_ALL_COPY_DISPOSE_FLAGS: block_assign_dispose_flags = BLOCK_FIELD_IS_OBJECT - | BLOCK_FIELD_IS_BLOCK - | BLOCK_FIELD_IS_BYREF - | BLOCK_FIELD_IS_WEAK - | BLOCK_BYREF_CALLER; +pub const BLOCK_BYREF_CALLER: c_int = 128; // TODO: BLOCK_LAYOUT_X From bb84596a0ed7dd990aca703375fdc2cca659d178 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 7 Jan 2024 10:13:25 +0100 Subject: [PATCH 3/9] Refactor block ABI to be more Rusty --- crates/block2/src/abi.rs | 235 +++++++++++++--------------- crates/block2/src/block.rs | 10 +- crates/block2/src/concrete_block.rs | 20 ++- crates/block2/src/debug.rs | 16 +- crates/block2/src/global.rs | 26 +-- crates/block2/src/lib.rs | 2 +- 6 files changed, 147 insertions(+), 162 deletions(-) diff --git a/crates/block2/src/abi.rs b/crates/block2/src/abi.rs index 660d7a1dc..74b2c0679 100644 --- a/crates/block2/src/abi.rs +++ b/crates/block2/src/abi.rs @@ -3,27 +3,22 @@ //! be looking! //! //! [ABI]: https://clang.llvm.org/docs/Block-ABI-Apple.html -#![allow( - unreachable_pub, - unused, - non_camel_case_types, - missing_debug_implementations -)] - -use core::ffi::c_void; +#![allow(unused)] + +use core::{ffi::c_void, mem::MaybeUninit}; use std::os::raw::{c_char, c_int, c_ulong}; use crate::ffi::Class; /// Block descriptor flags. -/// Values for Block_layout->flags to describe block objects #[allow(non_camel_case_types)] -pub type block_flags = c_int; +pub(crate) type BlockFlags = c_int; -#[cfg(any(doc, feature = "apple"))] -pub const BLOCK_DEALLOCATING: block_flags = 0x0001; +/// Note: Not public ABI. +pub(crate) const BLOCK_DEALLOCATING: BlockFlags = 0x0001; -pub const BLOCK_REFCOUNT_MASK: block_flags = if cfg!(feature = "gnustep-1-7") { +/// Note: Not public ABI. +pub(crate) const BLOCK_REFCOUNT_MASK: BlockFlags = if cfg!(feature = "gnustep-1-7") { // Mask for the reference count in byref structure's flags field. The low // 3 bytes are reserved for the reference count, the top byte for the flags. 0x00ffffff @@ -35,37 +30,29 @@ pub const BLOCK_REFCOUNT_MASK: block_flags = if cfg!(feature = "gnustep-1-7") { 0 }; -#[cfg(any(doc, feature = "apple"))] -/// compiler -pub const BLOCK_INLINE_LAYOUT_STRING: block_flags = 1 << 21; +/// Note: Not public ABI. +pub(crate) const BLOCK_INLINE_LAYOUT_STRING: BlockFlags = 1 << 21; -#[cfg(any(doc, feature = "apple"))] -/// compiler -pub const BLOCK_SMALL_DESCRIPTOR: block_flags = 1 << 22; +/// Note: Not public ABI. +pub(crate) const BLOCK_SMALL_DESCRIPTOR: BlockFlags = 1 << 22; -#[cfg(any(doc, feature = "apple"))] // Part of ABI? -/// compiler -pub const BLOCK_IS_NOESCAPE: block_flags = 1 << 23; +pub(crate) const BLOCK_IS_NOESCAPE: BlockFlags = 1 << 23; -#[cfg(any(doc, feature = "apple"))] -/// runtime -pub const BLOCK_NEEDS_FREE: block_flags = 1 << 24; +/// Note: Not public ABI. +pub(crate) const BLOCK_NEEDS_FREE: BlockFlags = 1 << 24; /// The block descriptor contains copy and dispose helpers. -/// compiler -pub const BLOCK_HAS_COPY_DISPOSE: block_flags = 1 << 25; +pub(crate) const BLOCK_HAS_COPY_DISPOSE: BlockFlags = 1 << 25; -/// The helpers have C++ code. -/// compiler: helpers have C++ code -pub const BLOCK_HAS_CTOR: block_flags = 1 << 26; +/// Helpers have C++ code. +#[doc(alias = "BLOCK_HAS_CXX_OBJ")] +pub(crate) const BLOCK_HAS_CTOR: BlockFlags = 1 << 26; -#[cfg(any(doc, feature = "apple"))] -/// compiler -pub const BLOCK_IS_GC: block_flags = 1 << 27; +/// Note: Not public ABI. +pub(crate) const BLOCK_IS_GC: BlockFlags = 1 << 27; /// Block is stored in global memory and does not need to be copied. -/// compiler -pub const BLOCK_IS_GLOBAL: block_flags = 1 << 28; +pub(crate) const BLOCK_IS_GLOBAL: BlockFlags = 1 << 28; /// Block function uses a calling convention that returns a structure via a /// pointer passed in by the caller. @@ -80,165 +67,165 @@ pub const BLOCK_IS_GLOBAL: block_flags = 1 << 28; /// See #[doc(alias = "BLOCK_USE_SRET")] #[doc(alias = "BLOCK_HAS_DESCRIPTOR")] -// compiler-rt || macOS 10.6 -pub const BLOCK_USE_STRET: block_flags = 1 << 29; +pub(crate) const BLOCK_USE_STRET: BlockFlags = 1 << 29; /// Block has an Objective-C type encoding. -/// compiler -pub const BLOCK_HAS_SIGNATURE: block_flags = 1 << 30; +pub(crate) const BLOCK_HAS_SIGNATURE: BlockFlags = 1 << 30; -#[cfg(any(doc, feature = "apple"))] -/// compiler -pub const BLOCK_HAS_EXTENDED_LAYOUT: block_flags = 1 << 31; +/// Note: Not public ABI. +pub(crate) const BLOCK_HAS_EXTENDED_LAYOUT: BlockFlags = 1 << 31; /// The value is of some id-like type, and should be copied as an Objective-C /// object: i.e. by sending -retain or via the GC assign functions in GC mode /// (not yet supported). /// /// id, NSObject, __attribute__((NSObject)), block, ... -pub const BLOCK_FIELD_IS_OBJECT: c_int = 3; +pub(crate) const BLOCK_FIELD_IS_OBJECT: c_int = 3; /// The field is a block. This must be copied by the block copy functions. /// /// a block variable -pub const BLOCK_FIELD_IS_BLOCK: c_int = 7; +pub(crate) const BLOCK_FIELD_IS_BLOCK: c_int = 7; /// The field is an indirect reference to a variable declared with the __block /// storage qualifier. /// /// the on stack structure holding the __block variable -pub const BLOCK_FIELD_IS_BYREF: c_int = 8; +pub(crate) const BLOCK_FIELD_IS_BYREF: c_int = 8; /// The field is an indirect reference to a variable declared with the __block /// storage qualifier. /// /// declared __weak, only used in byref copy helpers -pub const BLOCK_FIELD_IS_WEAK: c_int = 16; +pub(crate) const BLOCK_FIELD_IS_WEAK: c_int = 16; /// The field is an indirect reference to a variable declared with the __block /// storage qualifier. /// /// called from __block (byref) copy/dispose support routines. -pub const BLOCK_BYREF_CALLER: c_int = 128; - -// TODO: BLOCK_LAYOUT_X +pub(crate) const BLOCK_BYREF_CALLER: c_int = 128; /// The expected layout of every block. #[repr(C)] #[doc(alias = "__block_literal")] -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. +#[doc(alias = "Block_layout")] +#[doc(alias = "Block_basic")] +#[allow(missing_debug_implementations)] +pub struct BlockLayout { + /// 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: *const Class, /// Flags. - /// See the `block_flags` enumerated type for possible values. - /// Contains ref count in Apple and ObjFW. - pub flags: block_flags, - /// Reserved - always initialised to 0 by the compiler (but this is not - /// said in the specification). /// - /// Used for the reference count in GNUStep and WinObjC. - pub reserved: i32, - /// The function that implements the block. The first argument is this - /// structure, the subsequent arguments are the block's explicit - /// 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. + /// See the `BlockFlags` enumerated type for possible values. + /// + /// Contains reference count in Apple's and ObjFW's runtime. + #[doc(alias = "Block_flags")] + pub(crate) flags: BlockFlags, + /// Reserved. + /// + /// Initialized to 0 by the compiler, but is said to be uninitialized in + /// the specification. + /// + /// Used for the reference count in GNUStep's and WinObjC's runtime. + #[doc(alias = "Block_size")] + pub(crate) reserved: MaybeUninit, + /// The function that implements the block. + /// + /// The first argument is a pointer to this structure, the subsequent + /// arguments are the block's explicit 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, /// The block's descriptor. The actual type of this is: /// ```pseudo-code /// match (BLOCK_HAS_COPY_DISPOSE, BLOCK_HAS_SIGNATURE) { - /// (false, false) => Block_descriptor_header, - /// (true, false) => Block_descriptor, - /// (false, true) => Block_descriptor_basic, - /// (true, true) => Block_descriptor_with_signature, + /// (false, false) => BlockDescriptor, + /// (true, false) => BlockDescriptorCopyDispose, + /// (false, true) => BlockDescriptorSignature, + /// (true, true) => BlockDescriptorCopyDisposeSignature, /// } /// ``` /// - /// But since all of these start with `Block_descriptor_header`, it is - /// always safe to reinterpret this pointer as that. - // Note: Important to use `*const c_void` until we know which type it is! - pub descriptor: *const c_void, + /// Since all of these start with `BlockDescriptor`, it is always safe to + /// reinterpret this pointer as that. + /// + /// Note: We don't use a `union` here, since that would be forced to have + /// a greater size than is actually required. + pub(crate) descriptor: *const c_void, } +/// Basic block descriptor. #[repr(C)] -#[allow(missing_copy_implementations)] #[doc(alias = "__block_descriptor")] #[doc(alias = "Block_descriptor_1")] -pub struct Block_descriptor_header { +pub(crate) struct BlockDescriptor { /// Reserved for future use. Currently always 0. - pub reserved: c_ulong, // usize + pub(crate) reserved: c_ulong, /// Size of the block. - pub size: c_ulong, // usize + pub(crate) size: c_ulong, } /// Block descriptor that contains copy and dispose operations. /// -/// Requires BLOCK_HAS_COPY_DISPOSE +/// Requires BLOCK_HAS_COPY_DISPOSE. #[repr(C)] +#[doc(alias = "__block_descriptor")] #[doc(alias = "Block_descriptor_2")] -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, - /// Dispose function, generated by the compiler to help copy the block if - /// it contains nontrivial destructors. - pub dispose: Option, +pub(crate) struct BlockDescriptorCopyDispose { + /// Reserved for future use. Currently always 0. + pub(crate) reserved: c_ulong, + /// Size of the block. + pub(crate) size: c_ulong, + + /// Helper to copy the block if it contains nontrivial copy operations. + pub(crate) copy: Option, + /// Helper to destroy the block after being copied. + pub(crate) dispose: Option, } -/// Extended block descriptor that does not contain copy and dispose helper -/// functions. +/// Block descriptor that has an encoding / a signature. /// -/// Requires BLOCK_HAS_SIGNATURE +/// Requires BLOCK_HAS_SIGNATURE. #[repr(C)] +#[doc(alias = "__block_descriptor")] #[doc(alias = "Block_descriptor_3")] -pub struct Block_descriptor_basic { - pub header: Block_descriptor_header, +pub(crate) struct BlockDescriptorSignature { + /// Reserved for future use. Currently always 0. + pub(crate) reserved: c_ulong, + /// Size of the block. + pub(crate) size: c_ulong, /// Objective-C type encoding of the block. #[doc(alias = "signature")] - pub encoding: *const c_char, + pub(crate) encoding: *const c_char, } -/// Requires BLOCK_HAS_COPY_DISPOSE and BLOCK_HAS_SIGNATURE +/// Block descriptor that contains copy and dispose operations, and which +/// has an encoding / a signature. +/// +/// Requires BLOCK_HAS_COPY_DISPOSE and BLOCK_HAS_SIGNATURE. #[repr(C)] +#[doc(alias = "__block_descriptor")] #[doc(alias = "Block_descriptor_2")] #[doc(alias = "Block_descriptor_3")] -pub struct Block_descriptor_with_signature { - pub header: Block_descriptor_header, +pub(crate) struct BlockDescriptorCopyDisposeSignature { + /// Reserved for future use. Currently always 0. + pub(crate) reserved: c_ulong, + /// Size of the block. + pub(crate) size: c_ulong, - /// Same as [`Block_descriptor::copy`]. - pub copy: Option, - /// Same as [`Block_descriptor::dispose`]. - pub dispose: Option, + /// Helper to copy the block if it contains nontrivial copy operations. + pub(crate) copy: Option, + /// Helper to destroy the block if required. + pub(crate) dispose: Option, /// Objective-C type encoding of the block. #[doc(alias = "signature")] - pub encoding: *const c_char, + pub(crate) encoding: *const c_char, } - -// #[cfg(any(doc, feature = "apple"))] -// pub layout: *const c_char, - -// #[repr(C)] -// pub struct Block_descriptor_small { -// pub size: u32, -// pub signature: i32, -// pub layout: i32, -// pub copy: i32, -// pub dispose: i32, -// } - -// #[repr(C)] -// pub struct Block_basic { -// pub isa: *mut Class, -// pub Block_flags: i32, -// pub Block_size: i32, -// pub Block_invoke: Option, -// pub Block_copy: Option, -// pub Block_dispose: Option, -// } -// Example usage: https://github.com/apple-oss-distributions/libdispatch/blob/libdispatch-84.5/src/once.c diff --git a/crates/block2/src/block.rs b/crates/block2/src/block.rs index 075f55d22..349bb1df8 100644 --- a/crates/block2/src/block.rs +++ b/crates/block2/src/block.rs @@ -89,10 +89,10 @@ block_args_impl!( #[repr(C)] pub struct Block { _inner: [u8; 0], - // We store `Block_layout` + a bit more, but `Block` has to remain an - // empty type otherwise the compiler thinks we only have provenance over - // `Block_layout`. - _layout: PhantomData, + // We store `BlockLayout` + the closure captures, but `Block` has to + // remain an empty type otherwise the compiler thinks we only have + // provenance over `BlockLayout`. + _layout: PhantomData, // To get correct variance on args and return types _p: PhantomData R>, } @@ -113,7 +113,7 @@ impl Block { /// caller must ensure that calling it will not cause a data race. pub unsafe fn call(&self, args: A) -> R { let ptr: *const Self = self; - let layout = unsafe { ptr.cast::().as_ref().unwrap_unchecked() }; + let layout = unsafe { ptr.cast::().as_ref().unwrap_unchecked() }; // TODO: Is `invoke` actually ever null? let invoke = layout.invoke.unwrap_or_else(|| unreachable!()); diff --git a/crates/block2/src/concrete_block.rs b/crates/block2/src/concrete_block.rs index 87dc7e11f..e518e7109 100644 --- a/crates/block2/src/concrete_block.rs +++ b/crates/block2/src/concrete_block.rs @@ -1,6 +1,6 @@ use core::ffi::c_void; use core::marker::PhantomData; -use core::mem::{self, ManuallyDrop}; +use core::mem::{self, ManuallyDrop, MaybeUninit}; use core::ops::Deref; use core::ptr; use std::os::raw::c_ulong; @@ -163,7 +163,7 @@ concrete_block_impl!( #[repr(C)] pub struct ConcreteBlock { p: PhantomData>, - pub(crate) layout: abi::Block_layout, + pub(crate) layout: abi::BlockLayout, pub(crate) closure: F, } @@ -187,17 +187,15 @@ where impl ConcreteBlock { // TODO: Use new ABI with BLOCK_HAS_SIGNATURE - const FLAGS: abi::block_flags = if mem::needs_drop::() { + const FLAGS: abi::BlockFlags = if mem::needs_drop::() { abi::BLOCK_HAS_COPY_DISPOSE } else { 0 }; - const DESCRIPTOR: abi::Block_descriptor = abi::Block_descriptor { - header: abi::Block_descriptor_header { - reserved: 0, - size: mem::size_of::() as c_ulong, - }, + const DESCRIPTOR: abi::BlockDescriptorCopyDispose = abi::BlockDescriptorCopyDispose { + reserved: 0, + size: mem::size_of::() as c_ulong, copy: if mem::needs_drop::() { Some(block_context_copy::) } else { @@ -214,12 +212,12 @@ impl ConcreteBlock { /// Unsafe because the caller must ensure the invoke function takes the /// correct arguments. unsafe fn with_invoke(invoke: unsafe extern "C" fn(), closure: F) -> Self { - let layout = abi::Block_layout { + let layout = abi::BlockLayout { isa: unsafe { ptr::addr_of!(ffi::_NSConcreteStackBlock) }, flags: Self::FLAGS, - reserved: 0, + reserved: MaybeUninit::new(0), invoke: Some(invoke), - descriptor: &Self::DESCRIPTOR as *const abi::Block_descriptor as *mut c_void, + descriptor: &Self::DESCRIPTOR as *const abi::BlockDescriptorCopyDispose as *mut c_void, }; Self { p: PhantomData, diff --git a/crates/block2/src/debug.rs b/crates/block2/src/debug.rs index 212dc5c46..1a3624fa8 100644 --- a/crates/block2/src/debug.rs +++ b/crates/block2/src/debug.rs @@ -34,7 +34,7 @@ impl Debug for Isa { } } -fn debug_block_layout(layout: &abi::Block_layout, f: &mut DebugStruct<'_, '_>) { +fn debug_block_layout(layout: &abi::BlockLayout, f: &mut DebugStruct<'_, '_>) { f.field("isa", &Isa(layout.isa)); f.field("flags", &BlockFlags(layout.flags)); f.field("reserved", &layout.reserved); @@ -53,7 +53,7 @@ impl Debug for Block { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { let mut f = f.debug_struct("Block"); let ptr: *const Self = self; - let layout = unsafe { ptr.cast::().as_ref().unwrap() }; + let layout = unsafe { ptr.cast::().as_ref().unwrap() }; debug_block_layout(layout, &mut f); f.finish_non_exhaustive() } @@ -62,7 +62,7 @@ impl Debug for Block { impl Debug for RcBlock { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { let mut f = f.debug_struct("RcBlock"); - let layout = unsafe { self.ptr.cast::().as_ref().unwrap() }; + let layout = unsafe { self.ptr.cast::().as_ref().unwrap() }; debug_block_layout(layout, &mut f); f.finish_non_exhaustive() } @@ -86,7 +86,7 @@ impl Debug for GlobalBlock { } #[derive(Clone, Copy, PartialEq, Eq)] -struct BlockFlags(abi::block_flags); +struct BlockFlags(abi::BlockFlags); impl Debug for BlockFlags { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { @@ -153,7 +153,7 @@ impl Debug for BlockDescriptor { let header = unsafe { self.descriptor - .cast::() + .cast::() .as_ref() .unwrap() }; @@ -166,7 +166,7 @@ impl Debug for BlockDescriptor { (true, false) => { let descriptor = unsafe { self.descriptor - .cast::() + .cast::() .as_ref() .unwrap() }; @@ -176,7 +176,7 @@ impl Debug for BlockDescriptor { (false, true) => { let descriptor = unsafe { self.descriptor - .cast::() + .cast::() .as_ref() .unwrap() }; @@ -192,7 +192,7 @@ impl Debug for BlockDescriptor { (true, true) => { let descriptor = unsafe { self.descriptor - .cast::() + .cast::() .as_ref() .unwrap() }; diff --git a/crates/block2/src/global.rs b/crates/block2/src/global.rs index adca770d4..a47ff173c 100644 --- a/crates/block2/src/global.rs +++ b/crates/block2/src/global.rs @@ -1,8 +1,8 @@ -use core::ffi::c_void; use core::marker::PhantomData; use core::mem; use core::ops::Deref; use core::ptr; +use core::{ffi::c_void, mem::MaybeUninit}; use std::os::raw::c_ulong; use objc2::encode::EncodeReturn; @@ -11,9 +11,9 @@ use super::Block; use crate::{abi, BlockArguments}; // TODO: Should this be a static to help the compiler deduplicating them? -const GLOBAL_DESCRIPTOR: abi::Block_descriptor_header = abi::Block_descriptor_header { +const GLOBAL_DESCRIPTOR: abi::BlockDescriptor = abi::BlockDescriptor { reserved: 0, - size: mem::size_of::() as c_ulong, + size: mem::size_of::() as c_ulong, }; /// An Objective-C block that does not capture its environment. @@ -28,7 +28,7 @@ const GLOBAL_DESCRIPTOR: abi::Block_descriptor_header = abi::Block_descriptor_he /// [`global_block!`]: crate::global_block #[repr(C)] pub struct GlobalBlock { - pub(crate) layout: abi::Block_layout, + pub(crate) layout: abi::BlockLayout, p: PhantomData<(A, R)>, } @@ -52,22 +52,22 @@ where // triggers an error. impl GlobalBlock { // TODO: Use new ABI with BLOCK_HAS_SIGNATURE - const FLAGS: abi::block_flags = abi::BLOCK_IS_GLOBAL | abi::BLOCK_USE_STRET; + const FLAGS: abi::BlockFlags = abi::BLOCK_IS_GLOBAL | abi::BLOCK_USE_STRET; #[doc(hidden)] - pub const __DEFAULT_LAYOUT: abi::Block_layout = abi::Block_layout { + pub const __DEFAULT_LAYOUT: abi::BlockLayout = abi::BlockLayout { // Populated in `global_block!` isa: ptr::null_mut(), flags: Self::FLAGS, - reserved: 0, + reserved: MaybeUninit::new(0), // Populated in `global_block!` invoke: None, - descriptor: &GLOBAL_DESCRIPTOR as *const abi::Block_descriptor_header as *mut c_void, + descriptor: &GLOBAL_DESCRIPTOR as *const abi::BlockDescriptor as *mut c_void, }; /// Use the [`global_block`] macro instead. #[doc(hidden)] - pub const unsafe fn from_layout(layout: abi::Block_layout) -> Self { + pub const unsafe fn from_layout(layout: abi::BlockLayout) -> Self { Self { layout, p: PhantomData, @@ -173,10 +173,10 @@ macro_rules! global_block { let mut layout = $crate::GlobalBlock::<($($t,)*) $(, $r)?>::__DEFAULT_LAYOUT; layout.isa = ::core::ptr::addr_of!($crate::ffi::_NSConcreteGlobalBlock); layout.invoke = ::core::option::Option::Some({ - unsafe extern "C" fn inner(_: *mut $crate::__Block_layout, $($a: $t),*) $(-> $r)? { + unsafe extern "C" fn inner(_: *mut $crate::__BlockLayout, $($a: $t),*) $(-> $r)? { $body } - let inner: unsafe extern "C" fn(*mut $crate::__Block_layout, $($a: $t),*) $(-> $r)? = inner; + let inner: unsafe extern "C" fn(*mut $crate::__BlockLayout, $($a: $t),*) $(-> $r)? = inner; // TODO: SAFETY ::core::mem::transmute(inner) @@ -253,12 +253,12 @@ mod tests { #[test] fn test_debug() { let invoke = NOOP_BLOCK.layout.invoke.unwrap(); - let size = mem::size_of::(); + let size = mem::size_of::(); let expected = format!( "GlobalBlock {{ isa: _NSConcreteGlobalBlock, flags: {DEBUG_BLOCKFLAGS}, - reserved: 0, + reserved: core::mem::maybe_uninit::MaybeUninit, invoke: Some( {invoke:#?}, ), diff --git a/crates/block2/src/lib.rs b/crates/block2/src/lib.rs index 02dc50763..b3b1ebf6d 100644 --- a/crates/block2/src/lib.rs +++ b/crates/block2/src/lib.rs @@ -243,7 +243,7 @@ mod global; mod rc_block; #[doc(hidden)] -pub use abi::Block_layout as __Block_layout; +pub use abi::BlockLayout as __BlockLayout; pub use block::{Block, BlockArguments}; pub use concrete_block::{ConcreteBlock, IntoConcreteBlock}; pub use global::GlobalBlock; From c3bd0c243bdca8ea099aa2227f88065bdb8ed537 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 6 Jan 2024 17:11:48 +0100 Subject: [PATCH 4/9] Further refactor block ABI --- crates/block2/src/abi.rs | 157 +++++++++++++++++++--------- crates/block2/src/block.rs | 6 +- crates/block2/src/concrete_block.rs | 17 +-- crates/block2/src/debug.rs | 89 ++++------------ crates/block2/src/global.rs | 20 ++-- 5 files changed, 148 insertions(+), 141 deletions(-) diff --git a/crates/block2/src/abi.rs b/crates/block2/src/abi.rs index 74b2c0679..f456156e7 100644 --- a/crates/block2/src/abi.rs +++ b/crates/block2/src/abi.rs @@ -5,75 +5,132 @@ //! [ABI]: https://clang.llvm.org/docs/Block-ABI-Apple.html #![allow(unused)] +use core::fmt; use core::{ffi::c_void, mem::MaybeUninit}; use std::os::raw::{c_char, c_int, c_ulong}; +use alloc::format; + use crate::ffi::Class; /// Block descriptor flags. -#[allow(non_camel_case_types)] -pub(crate) type BlockFlags = c_int; +#[repr(transparent)] +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) struct BlockFlags(pub(crate) c_int); -/// Note: Not public ABI. -pub(crate) const BLOCK_DEALLOCATING: BlockFlags = 0x0001; +impl BlockFlags { + pub(crate) const EMPTY: Self = Self(0); -/// Note: Not public ABI. -pub(crate) const BLOCK_REFCOUNT_MASK: BlockFlags = if cfg!(feature = "gnustep-1-7") { - // Mask for the reference count in byref structure's flags field. The low - // 3 bytes are reserved for the reference count, the top byte for the flags. - 0x00ffffff -} else if cfg!(any(feature = "compiler-rt", feature = "objfw")) { - 0xffff -} else if cfg!(feature = "apple") { - 0xfffe // runtime -} else { - 0 -}; + /// Note: Not public ABI. + const BLOCK_DEALLOCATING: Self = Self(0x0001); -/// Note: Not public ABI. -pub(crate) const BLOCK_INLINE_LAYOUT_STRING: BlockFlags = 1 << 21; + /// Note: Not public ABI. + const BLOCK_REFCOUNT_MASK: Self = Self(if cfg!(feature = "gnustep-1-7") { + // Mask for the reference count in byref structure's flags field. The low + // 3 bytes are reserved for the reference count, the top byte for the flags. + 0x00ffffff + } else if cfg!(any(feature = "compiler-rt", feature = "objfw")) { + 0xffff + } else if cfg!(feature = "apple") { + 0xfffe // runtime + } else { + 0 + }); -/// Note: Not public ABI. -pub(crate) const BLOCK_SMALL_DESCRIPTOR: BlockFlags = 1 << 22; + /// Note: Not public ABI. + const BLOCK_INLINE_LAYOUT_STRING: Self = Self(1 << 21); -pub(crate) const BLOCK_IS_NOESCAPE: BlockFlags = 1 << 23; + /// Note: Not public ABI. + const BLOCK_SMALL_DESCRIPTOR: Self = Self(1 << 22); -/// Note: Not public ABI. -pub(crate) const BLOCK_NEEDS_FREE: BlockFlags = 1 << 24; + pub(crate) const BLOCK_IS_NOESCAPE: Self = Self(1 << 23); -/// The block descriptor contains copy and dispose helpers. -pub(crate) const BLOCK_HAS_COPY_DISPOSE: BlockFlags = 1 << 25; + /// Note: Not public ABI. + const BLOCK_NEEDS_FREE: Self = Self(1 << 24); -/// Helpers have C++ code. -#[doc(alias = "BLOCK_HAS_CXX_OBJ")] -pub(crate) const BLOCK_HAS_CTOR: BlockFlags = 1 << 26; + /// The block descriptor contains copy and dispose helpers. + pub(crate) const BLOCK_HAS_COPY_DISPOSE: Self = Self(1 << 25); -/// Note: Not public ABI. -pub(crate) const BLOCK_IS_GC: BlockFlags = 1 << 27; + /// Helpers have C++ code. + #[doc(alias = "BLOCK_HAS_CXX_OBJ")] + pub(crate) const BLOCK_HAS_CTOR: Self = Self(1 << 26); -/// Block is stored in global memory and does not need to be copied. -pub(crate) const BLOCK_IS_GLOBAL: BlockFlags = 1 << 28; + /// Note: Not public ABI. + const BLOCK_IS_GC: Self = Self(1 << 27); -/// Block function uses a calling convention that returns a structure via a -/// pointer passed in by the caller. -/// -/// match (BLOCK_USE_STRET, BLOCK_HAS_SIGNATURE) { -/// (false, false) => 10.6.ABI, no signature field available -/// (true, false) => 10.6.ABI, no signature field available -/// (false, true) => ABI.2010.3.16, regular calling convention, presence of signature field -/// (true, true) => ABI.2010.3.16, stret calling convention, presence of signature field, -/// } -/// -/// See -#[doc(alias = "BLOCK_USE_SRET")] -#[doc(alias = "BLOCK_HAS_DESCRIPTOR")] -pub(crate) const BLOCK_USE_STRET: BlockFlags = 1 << 29; + /// Block is stored in global memory and does not need to be copied. + pub(crate) const BLOCK_IS_GLOBAL: Self = Self(1 << 28); + + /// Block function uses a calling convention that returns a structure via a + /// pointer passed in by the caller. + /// + /// match (BLOCK_USE_STRET, BLOCK_HAS_SIGNATURE) { + /// (false, false) => 10.6.ABI, no signature field available + /// (true, false) => 10.6.ABI, no signature field available + /// (false, true) => ABI.2010.3.16, regular calling convention, presence of signature field + /// (true, true) => ABI.2010.3.16, stret calling convention, presence of signature field, + /// } + /// + /// See + #[doc(alias = "BLOCK_USE_SRET")] + #[doc(alias = "BLOCK_HAS_DESCRIPTOR")] + pub(crate) const BLOCK_USE_STRET: Self = Self(1 << 29); -/// Block has an Objective-C type encoding. -pub(crate) const BLOCK_HAS_SIGNATURE: BlockFlags = 1 << 30; + /// Block has an Objective-C type encoding. + pub(crate) const BLOCK_HAS_SIGNATURE: Self = Self(1 << 30); -/// Note: Not public ABI. -pub(crate) const BLOCK_HAS_EXTENDED_LAYOUT: BlockFlags = 1 << 31; + /// Note: Not public ABI. + const BLOCK_HAS_EXTENDED_LAYOUT: Self = Self(1 << 31); +} + +impl fmt::Debug for BlockFlags { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut f = f.debug_struct("BlockFlags"); + f.field("value", &format!("{:032b}", self.0)); + + macro_rules! test_flags { + {$( + $(#[$m:meta])? + $name:ident: $flag:ident + );* $(;)?} => ($( + $(#[$m])? + f.field(stringify!($name), &(self.0 & Self::$flag.0 != 0)); + )*) + } + test_flags! { + #[cfg(feature = "apple")] + deallocating: BLOCK_DEALLOCATING; + #[cfg(feature = "apple")] + inline_layout_string: BLOCK_INLINE_LAYOUT_STRING; + #[cfg(feature = "apple")] + small_descriptor: BLOCK_SMALL_DESCRIPTOR; + #[cfg(feature = "apple")] + is_noescape: BLOCK_IS_NOESCAPE; + #[cfg(feature = "apple")] + needs_free: BLOCK_NEEDS_FREE; + has_copy_dispose: BLOCK_HAS_COPY_DISPOSE; + has_ctor: BLOCK_HAS_CTOR; + #[cfg(feature = "apple")] + is_gc: BLOCK_IS_GC; + is_global: BLOCK_IS_GLOBAL; + use_stret: BLOCK_USE_STRET; + has_signature: BLOCK_HAS_SIGNATURE; + #[cfg(feature = "apple")] + has_extended_layout: BLOCK_HAS_EXTENDED_LAYOUT; + } + + f.field( + "over_referenced", + &(self.0 & Self::BLOCK_REFCOUNT_MASK.0 == Self::BLOCK_REFCOUNT_MASK.0), + ); + f.field( + "reference_count", + &((self.0 & Self::BLOCK_REFCOUNT_MASK.0) >> 1), + ); + + f.finish_non_exhaustive() + } +} /// The value is of some id-like type, and should be copied as an Objective-C /// object: i.e. by sending -retain or via the GC assign functions in GC mode diff --git a/crates/block2/src/block.rs b/crates/block2/src/block.rs index 349bb1df8..cf327cc48 100644 --- a/crates/block2/src/block.rs +++ b/crates/block2/src/block.rs @@ -3,7 +3,7 @@ use core::mem; use objc2::encode::{EncodeArgument, EncodeReturn, Encoding, RefEncode}; -use crate::abi; +use crate::abi::BlockLayout; /// Types that may be used as the arguments of an Objective-C block. /// @@ -92,7 +92,7 @@ pub struct Block { // We store `BlockLayout` + the closure captures, but `Block` has to // remain an empty type otherwise the compiler thinks we only have // provenance over `BlockLayout`. - _layout: PhantomData, + _layout: PhantomData, // To get correct variance on args and return types _p: PhantomData R>, } @@ -113,7 +113,7 @@ impl Block { /// caller must ensure that calling it will not cause a data race. pub unsafe fn call(&self, args: A) -> R { let ptr: *const Self = self; - let layout = unsafe { ptr.cast::().as_ref().unwrap_unchecked() }; + let layout = unsafe { ptr.cast::().as_ref().unwrap_unchecked() }; // TODO: Is `invoke` actually ever null? let invoke = layout.invoke.unwrap_or_else(|| unreachable!()); diff --git a/crates/block2/src/concrete_block.rs b/crates/block2/src/concrete_block.rs index e518e7109..1fdadeff3 100644 --- a/crates/block2/src/concrete_block.rs +++ b/crates/block2/src/concrete_block.rs @@ -7,7 +7,8 @@ use std::os::raw::c_ulong; use objc2::encode::{EncodeArgument, EncodeReturn, Encoding, RefEncode}; -use crate::{abi, ffi, Block, BlockArguments, RcBlock}; +use crate::abi::{BlockDescriptorCopyDispose, BlockFlags, BlockLayout}; +use crate::{ffi, Block, BlockArguments, RcBlock}; mod private { pub trait Sealed {} @@ -163,7 +164,7 @@ concrete_block_impl!( #[repr(C)] pub struct ConcreteBlock { p: PhantomData>, - pub(crate) layout: abi::BlockLayout, + pub(crate) layout: BlockLayout, pub(crate) closure: F, } @@ -187,13 +188,13 @@ where impl ConcreteBlock { // TODO: Use new ABI with BLOCK_HAS_SIGNATURE - const FLAGS: abi::BlockFlags = if mem::needs_drop::() { - abi::BLOCK_HAS_COPY_DISPOSE + const FLAGS: BlockFlags = if mem::needs_drop::() { + BlockFlags::BLOCK_HAS_COPY_DISPOSE } else { - 0 + BlockFlags::EMPTY }; - const DESCRIPTOR: abi::BlockDescriptorCopyDispose = abi::BlockDescriptorCopyDispose { + const DESCRIPTOR: BlockDescriptorCopyDispose = BlockDescriptorCopyDispose { reserved: 0, size: mem::size_of::() as c_ulong, copy: if mem::needs_drop::() { @@ -212,12 +213,12 @@ impl ConcreteBlock { /// Unsafe because the caller must ensure the invoke function takes the /// correct arguments. unsafe fn with_invoke(invoke: unsafe extern "C" fn(), closure: F) -> Self { - let layout = abi::BlockLayout { + let layout = BlockLayout { isa: unsafe { ptr::addr_of!(ffi::_NSConcreteStackBlock) }, flags: Self::FLAGS, reserved: MaybeUninit::new(0), invoke: Some(invoke), - descriptor: &Self::DESCRIPTOR as *const abi::BlockDescriptorCopyDispose as *mut c_void, + descriptor: &Self::DESCRIPTOR as *const BlockDescriptorCopyDispose as *mut c_void, }; Self { p: PhantomData, diff --git a/crates/block2/src/debug.rs b/crates/block2/src/debug.rs index 1a3624fa8..58d8e1a89 100644 --- a/crates/block2/src/debug.rs +++ b/crates/block2/src/debug.rs @@ -1,10 +1,13 @@ -use alloc::format; use core::ffi::c_void; use core::fmt::{Debug, DebugStruct, Error, Formatter}; use core::ptr; use std::ffi::CStr; -use crate::{abi, ffi, Block, ConcreteBlock, GlobalBlock, RcBlock}; +use crate::abi::{ + BlockDescriptor, BlockDescriptorCopyDispose, BlockDescriptorCopyDisposeSignature, + BlockDescriptorSignature, BlockFlags, BlockLayout, +}; +use crate::{ffi, Block, ConcreteBlock, GlobalBlock, RcBlock}; #[derive(Clone, Copy, PartialEq, Eq)] struct Isa(*const ffi::Class); @@ -34,16 +37,16 @@ impl Debug for Isa { } } -fn debug_block_layout(layout: &abi::BlockLayout, f: &mut DebugStruct<'_, '_>) { +fn debug_block_layout(layout: &BlockLayout, f: &mut DebugStruct<'_, '_>) { f.field("isa", &Isa(layout.isa)); - f.field("flags", &BlockFlags(layout.flags)); + f.field("flags", &layout.flags); f.field("reserved", &layout.reserved); f.field("invoke", &layout.invoke); f.field( "descriptor", - &BlockDescriptor { - has_copy_dispose: layout.flags & abi::BLOCK_HAS_COPY_DISPOSE != 0, - has_signature: layout.flags & abi::BLOCK_HAS_SIGNATURE != 0, + &BlockDescriptorHelper { + has_copy_dispose: layout.flags.0 & BlockFlags::BLOCK_HAS_COPY_DISPOSE.0 != 0, + has_signature: layout.flags.0 & BlockFlags::BLOCK_HAS_SIGNATURE.0 != 0, descriptor: layout.descriptor, }, ); @@ -53,7 +56,7 @@ impl Debug for Block { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { let mut f = f.debug_struct("Block"); let ptr: *const Self = self; - let layout = unsafe { ptr.cast::().as_ref().unwrap() }; + let layout = unsafe { ptr.cast::().as_ref().unwrap() }; debug_block_layout(layout, &mut f); f.finish_non_exhaustive() } @@ -62,7 +65,7 @@ impl Debug for Block { impl Debug for RcBlock { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { let mut f = f.debug_struct("RcBlock"); - let layout = unsafe { self.ptr.cast::().as_ref().unwrap() }; + let layout = unsafe { self.ptr.cast::().as_ref().unwrap() }; debug_block_layout(layout, &mut f); f.finish_non_exhaustive() } @@ -86,64 +89,13 @@ impl Debug for GlobalBlock { } #[derive(Clone, Copy, PartialEq, Eq)] -struct BlockFlags(abi::BlockFlags); - -impl Debug for BlockFlags { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - let mut f = f.debug_struct("BlockFlags"); - f.field("value", &format!("{:032b}", self.0)); - - macro_rules! test_flags { - {$( - $(#[$m:meta])? - $name:ident: $flag:ident - );* $(;)?} => ($( - $(#[$m])? - f.field(stringify!($name), &(self.0 & abi::$flag != 0)); - )*) - } - test_flags! { - #[cfg(feature = "apple")] - deallocating: BLOCK_DEALLOCATING; - #[cfg(feature = "apple")] - inline_layout_string: BLOCK_INLINE_LAYOUT_STRING; - #[cfg(feature = "apple")] - small_descriptor: BLOCK_SMALL_DESCRIPTOR; - #[cfg(feature = "apple")] - is_noescape: BLOCK_IS_NOESCAPE; - #[cfg(feature = "apple")] - needs_free: BLOCK_NEEDS_FREE; - has_copy_dispose: BLOCK_HAS_COPY_DISPOSE; - has_ctor: BLOCK_HAS_CTOR; - #[cfg(feature = "apple")] - is_gc: BLOCK_IS_GC; - is_global: BLOCK_IS_GLOBAL; - use_stret: BLOCK_USE_STRET; - has_signature: BLOCK_HAS_SIGNATURE; - #[cfg(feature = "apple")] - has_extended_layout: BLOCK_HAS_EXTENDED_LAYOUT; - } - - f.field( - "over_referenced", - &(self.0 & abi::BLOCK_REFCOUNT_MASK == abi::BLOCK_REFCOUNT_MASK), - ); - f.field( - "reference_count", - &((self.0 & abi::BLOCK_REFCOUNT_MASK) >> 1), - ); - f.finish_non_exhaustive() - } -} - -#[derive(Clone, Copy, PartialEq, Eq)] -struct BlockDescriptor { +struct BlockDescriptorHelper { has_copy_dispose: bool, has_signature: bool, descriptor: *const c_void, } -impl Debug for BlockDescriptor { +impl Debug for BlockDescriptorHelper { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { if self.descriptor.is_null() { return f.write_str("(null)"); @@ -151,12 +103,7 @@ impl Debug for BlockDescriptor { let mut f = f.debug_struct("BlockDescriptor"); - let header = unsafe { - self.descriptor - .cast::() - .as_ref() - .unwrap() - }; + let header = unsafe { self.descriptor.cast::().as_ref().unwrap() }; f.field("reserved", &header.reserved); f.field("size", &header.size); @@ -166,7 +113,7 @@ impl Debug for BlockDescriptor { (true, false) => { let descriptor = unsafe { self.descriptor - .cast::() + .cast::() .as_ref() .unwrap() }; @@ -176,7 +123,7 @@ impl Debug for BlockDescriptor { (false, true) => { let descriptor = unsafe { self.descriptor - .cast::() + .cast::() .as_ref() .unwrap() }; @@ -192,7 +139,7 @@ impl Debug for BlockDescriptor { (true, true) => { let descriptor = unsafe { self.descriptor - .cast::() + .cast::() .as_ref() .unwrap() }; diff --git a/crates/block2/src/global.rs b/crates/block2/src/global.rs index a47ff173c..74dce2012 100644 --- a/crates/block2/src/global.rs +++ b/crates/block2/src/global.rs @@ -8,12 +8,13 @@ use std::os::raw::c_ulong; use objc2::encode::EncodeReturn; use super::Block; -use crate::{abi, BlockArguments}; +use crate::abi::{BlockDescriptor, BlockFlags, BlockLayout}; +use crate::BlockArguments; // TODO: Should this be a static to help the compiler deduplicating them? -const GLOBAL_DESCRIPTOR: abi::BlockDescriptor = abi::BlockDescriptor { +const GLOBAL_DESCRIPTOR: BlockDescriptor = BlockDescriptor { reserved: 0, - size: mem::size_of::() as c_ulong, + size: mem::size_of::() as c_ulong, }; /// An Objective-C block that does not capture its environment. @@ -28,7 +29,7 @@ const GLOBAL_DESCRIPTOR: abi::BlockDescriptor = abi::BlockDescriptor { /// [`global_block!`]: crate::global_block #[repr(C)] pub struct GlobalBlock { - pub(crate) layout: abi::BlockLayout, + pub(crate) layout: BlockLayout, p: PhantomData<(A, R)>, } @@ -52,22 +53,23 @@ where // triggers an error. impl GlobalBlock { // TODO: Use new ABI with BLOCK_HAS_SIGNATURE - const FLAGS: abi::BlockFlags = abi::BLOCK_IS_GLOBAL | abi::BLOCK_USE_STRET; + const FLAGS: BlockFlags = + BlockFlags(BlockFlags::BLOCK_IS_GLOBAL.0 | BlockFlags::BLOCK_USE_STRET.0); #[doc(hidden)] - pub const __DEFAULT_LAYOUT: abi::BlockLayout = abi::BlockLayout { + pub const __DEFAULT_LAYOUT: BlockLayout = BlockLayout { // Populated in `global_block!` isa: ptr::null_mut(), flags: Self::FLAGS, reserved: MaybeUninit::new(0), // Populated in `global_block!` invoke: None, - descriptor: &GLOBAL_DESCRIPTOR as *const abi::BlockDescriptor as *mut c_void, + descriptor: &GLOBAL_DESCRIPTOR as *const BlockDescriptor as *mut c_void, }; /// Use the [`global_block`] macro instead. #[doc(hidden)] - pub const unsafe fn from_layout(layout: abi::BlockLayout) -> Self { + pub const unsafe fn from_layout(layout: BlockLayout) -> Self { Self { layout, p: PhantomData, @@ -253,7 +255,7 @@ mod tests { #[test] fn test_debug() { let invoke = NOOP_BLOCK.layout.invoke.unwrap(); - let size = mem::size_of::(); + let size = mem::size_of::(); let expected = format!( "GlobalBlock {{ isa: _NSConcreteGlobalBlock, From 157667e7ac7d546eb36f68a443b6c223801a179f Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 7 Jan 2024 08:13:24 +0100 Subject: [PATCH 5/9] Refactor debug impls --- crates/block2/src/block.rs | 12 +++++++++ crates/block2/src/concrete_block.rs | 11 ++++++++ crates/block2/src/debug.rs | 40 ++--------------------------- crates/block2/src/global.rs | 13 ++++++++-- crates/block2/src/rc_block.rs | 12 +++++++++ 5 files changed, 48 insertions(+), 40 deletions(-) diff --git a/crates/block2/src/block.rs b/crates/block2/src/block.rs index cf327cc48..d575cd37d 100644 --- a/crates/block2/src/block.rs +++ b/crates/block2/src/block.rs @@ -1,9 +1,11 @@ +use core::fmt; use core::marker::PhantomData; use core::mem; use objc2::encode::{EncodeArgument, EncodeReturn, Encoding, RefEncode}; use crate::abi::BlockLayout; +use crate::debug::debug_block_layout; /// Types that may be used as the arguments of an Objective-C block. /// @@ -120,3 +122,13 @@ impl Block { unsafe { A::__call_block(invoke, ptr as *mut Self, args) } } } + +impl fmt::Debug for Block { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut f = f.debug_struct("Block"); + let ptr: *const Self = self; + let layout = unsafe { ptr.cast::().as_ref().unwrap() }; + debug_block_layout(layout, &mut f); + f.finish_non_exhaustive() + } +} diff --git a/crates/block2/src/concrete_block.rs b/crates/block2/src/concrete_block.rs index 1fdadeff3..8038041d0 100644 --- a/crates/block2/src/concrete_block.rs +++ b/crates/block2/src/concrete_block.rs @@ -1,4 +1,5 @@ use core::ffi::c_void; +use core::fmt; use core::marker::PhantomData; use core::mem::{self, ManuallyDrop, MaybeUninit}; use core::ops::Deref; @@ -8,6 +9,7 @@ use std::os::raw::c_ulong; use objc2::encode::{EncodeArgument, EncodeReturn, Encoding, RefEncode}; use crate::abi::{BlockDescriptorCopyDispose, BlockFlags, BlockLayout}; +use crate::debug::debug_block_layout; use crate::{ffi, Block, BlockArguments, RcBlock}; mod private { @@ -264,3 +266,12 @@ unsafe extern "C" fn block_context_dispose(block: *mut c_void) { unsafe extern "C" fn block_context_copy(_dst: *mut c_void, _src: *mut c_void) { // The runtime memmoves the src block into the dst block, nothing to do } + +impl fmt::Debug for ConcreteBlock { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut f = f.debug_struct("ConcreteBlock"); + debug_block_layout(&self.layout, &mut f); + f.field("closure", &self.closure); + f.finish() + } +} diff --git a/crates/block2/src/debug.rs b/crates/block2/src/debug.rs index 58d8e1a89..3e6fa9476 100644 --- a/crates/block2/src/debug.rs +++ b/crates/block2/src/debug.rs @@ -7,7 +7,7 @@ use crate::abi::{ BlockDescriptor, BlockDescriptorCopyDispose, BlockDescriptorCopyDisposeSignature, BlockDescriptorSignature, BlockFlags, BlockLayout, }; -use crate::{ffi, Block, ConcreteBlock, GlobalBlock, RcBlock}; +use crate::ffi; #[derive(Clone, Copy, PartialEq, Eq)] struct Isa(*const ffi::Class); @@ -37,7 +37,7 @@ impl Debug for Isa { } } -fn debug_block_layout(layout: &BlockLayout, f: &mut DebugStruct<'_, '_>) { +pub(crate) fn debug_block_layout(layout: &BlockLayout, f: &mut DebugStruct<'_, '_>) { f.field("isa", &Isa(layout.isa)); f.field("flags", &layout.flags); f.field("reserved", &layout.reserved); @@ -52,42 +52,6 @@ fn debug_block_layout(layout: &BlockLayout, f: &mut DebugStruct<'_, '_>) { ); } -impl Debug for Block { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - let mut f = f.debug_struct("Block"); - let ptr: *const Self = self; - let layout = unsafe { ptr.cast::().as_ref().unwrap() }; - debug_block_layout(layout, &mut f); - f.finish_non_exhaustive() - } -} - -impl Debug for RcBlock { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - let mut f = f.debug_struct("RcBlock"); - let layout = unsafe { self.ptr.cast::().as_ref().unwrap() }; - debug_block_layout(layout, &mut f); - f.finish_non_exhaustive() - } -} - -impl Debug for ConcreteBlock { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - let mut f = f.debug_struct("ConcreteBlock"); - debug_block_layout(&self.layout, &mut f); - f.field("closure", &self.closure); - f.finish() - } -} - -impl Debug for GlobalBlock { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - let mut f = f.debug_struct("GlobalBlock"); - debug_block_layout(&self.layout, &mut f); - f.finish_non_exhaustive() - } -} - #[derive(Clone, Copy, PartialEq, Eq)] struct BlockDescriptorHelper { has_copy_dispose: bool, diff --git a/crates/block2/src/global.rs b/crates/block2/src/global.rs index 74dce2012..e365f984f 100644 --- a/crates/block2/src/global.rs +++ b/crates/block2/src/global.rs @@ -1,3 +1,4 @@ +use core::fmt; use core::marker::PhantomData; use core::mem; use core::ops::Deref; @@ -7,9 +8,9 @@ use std::os::raw::c_ulong; use objc2::encode::EncodeReturn; -use super::Block; use crate::abi::{BlockDescriptor, BlockFlags, BlockLayout}; -use crate::BlockArguments; +use crate::debug::debug_block_layout; +use crate::{Block, BlockArguments}; // TODO: Should this be a static to help the compiler deduplicating them? const GLOBAL_DESCRIPTOR: BlockDescriptor = BlockDescriptor { @@ -92,6 +93,14 @@ where } } +impl fmt::Debug for GlobalBlock { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut f = f.debug_struct("GlobalBlock"); + debug_block_layout(&self.layout, &mut f); + f.finish_non_exhaustive() + } +} + /// Construct a static [`GlobalBlock`]. /// /// The syntax is similar to a static closure (except that all types have to diff --git a/crates/block2/src/rc_block.rs b/crates/block2/src/rc_block.rs index ba5b8471d..1291d3ba2 100644 --- a/crates/block2/src/rc_block.rs +++ b/crates/block2/src/rc_block.rs @@ -1,5 +1,8 @@ +use core::fmt; use core::ops::Deref; +use crate::abi::BlockLayout; +use crate::debug::debug_block_layout; use crate::{ffi, Block}; /// A reference-counted Objective-C block. @@ -57,3 +60,12 @@ impl Drop for RcBlock { unsafe { ffi::_Block_release(self.ptr.cast()) }; } } + +impl fmt::Debug for RcBlock { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut f = f.debug_struct("RcBlock"); + let layout = unsafe { self.ptr.cast::().as_ref().unwrap() }; + debug_block_layout(layout, &mut f); + f.finish_non_exhaustive() + } +} From eb85fbf1998dd24945fc2e095888c0afa2903f73 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 7 Jan 2024 09:25:38 +0100 Subject: [PATCH 6/9] Use a union to access the block descriptor --- crates/block2/src/abi.rs | 42 ++++++++++++++++++----------- crates/block2/src/concrete_block.rs | 6 +++-- crates/block2/src/debug.rs | 30 ++++++--------------- crates/block2/src/global.rs | 8 +++--- 4 files changed, 43 insertions(+), 43 deletions(-) diff --git a/crates/block2/src/abi.rs b/crates/block2/src/abi.rs index f456156e7..cbdd89c84 100644 --- a/crates/block2/src/abi.rs +++ b/crates/block2/src/abi.rs @@ -199,22 +199,32 @@ pub struct BlockLayout { /// additional hidden argument, which is a pointer to the space on the /// stack allocated to hold the return value. pub invoke: Option, - /// The block's descriptor. The actual type of this is: - /// ```pseudo-code - /// match (BLOCK_HAS_COPY_DISPOSE, BLOCK_HAS_SIGNATURE) { - /// (false, false) => BlockDescriptor, - /// (true, false) => BlockDescriptorCopyDispose, - /// (false, true) => BlockDescriptorSignature, - /// (true, true) => BlockDescriptorCopyDisposeSignature, - /// } - /// ``` - /// - /// Since all of these start with `BlockDescriptor`, it is always safe to - /// reinterpret this pointer as that. - /// - /// Note: We don't use a `union` here, since that would be forced to have - /// a greater size than is actually required. - pub(crate) descriptor: *const c_void, + /// The block's descriptor. + pub(crate) descriptor: BlockDescriptorPtr, +} + +/// The type of this is: +/// ```pseudo-code +/// match (BLOCK_HAS_COPY_DISPOSE, BLOCK_HAS_SIGNATURE) { +/// (false, false) => BlockDescriptor, +/// (true, false) => BlockDescriptorCopyDispose, +/// (false, true) => BlockDescriptorSignature, +/// (true, true) => BlockDescriptorCopyDisposeSignature, +/// } +/// ``` +/// +/// Since all of these start with `BlockDescriptor`, it is always safe to +/// use the `basic` field. +// +// Note: We use an union on top of the pointer, since otherwise the descriptor +// would be forced to have a greater size than is actually required. +#[repr(C)] +#[derive(Clone, Copy)] +pub(crate) union BlockDescriptorPtr { + pub(crate) basic: *const BlockDescriptor, + pub(crate) with_copy_dispose: *const BlockDescriptorCopyDispose, + pub(crate) with_signature: *const BlockDescriptorSignature, + pub(crate) with_copy_dispose_signature: *const BlockDescriptorCopyDisposeSignature, } /// Basic block descriptor. diff --git a/crates/block2/src/concrete_block.rs b/crates/block2/src/concrete_block.rs index 8038041d0..d1ede14e7 100644 --- a/crates/block2/src/concrete_block.rs +++ b/crates/block2/src/concrete_block.rs @@ -8,7 +8,7 @@ use std::os::raw::c_ulong; use objc2::encode::{EncodeArgument, EncodeReturn, Encoding, RefEncode}; -use crate::abi::{BlockDescriptorCopyDispose, BlockFlags, BlockLayout}; +use crate::abi::{BlockDescriptorCopyDispose, BlockDescriptorPtr, BlockFlags, BlockLayout}; use crate::debug::debug_block_layout; use crate::{ffi, Block, BlockArguments, RcBlock}; @@ -220,7 +220,9 @@ impl ConcreteBlock { flags: Self::FLAGS, reserved: MaybeUninit::new(0), invoke: Some(invoke), - descriptor: &Self::DESCRIPTOR as *const BlockDescriptorCopyDispose as *mut c_void, + descriptor: BlockDescriptorPtr { + with_copy_dispose: &Self::DESCRIPTOR, + }, }; Self { p: PhantomData, diff --git a/crates/block2/src/debug.rs b/crates/block2/src/debug.rs index 3e6fa9476..3cd579b2a 100644 --- a/crates/block2/src/debug.rs +++ b/crates/block2/src/debug.rs @@ -1,12 +1,8 @@ -use core::ffi::c_void; use core::fmt::{Debug, DebugStruct, Error, Formatter}; use core::ptr; use std::ffi::CStr; -use crate::abi::{ - BlockDescriptor, BlockDescriptorCopyDispose, BlockDescriptorCopyDisposeSignature, - BlockDescriptorSignature, BlockFlags, BlockLayout, -}; +use crate::abi::{BlockDescriptorPtr, BlockFlags, BlockLayout}; use crate::ffi; #[derive(Clone, Copy, PartialEq, Eq)] @@ -52,22 +48,22 @@ pub(crate) fn debug_block_layout(layout: &BlockLayout, f: &mut DebugStruct<'_, ' ); } -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy)] struct BlockDescriptorHelper { has_copy_dispose: bool, has_signature: bool, - descriptor: *const c_void, + descriptor: BlockDescriptorPtr, } impl Debug for BlockDescriptorHelper { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - if self.descriptor.is_null() { + if unsafe { self.descriptor.basic }.is_null() { return f.write_str("(null)"); } let mut f = f.debug_struct("BlockDescriptor"); - let header = unsafe { self.descriptor.cast::().as_ref().unwrap() }; + let header = unsafe { self.descriptor.basic.as_ref().unwrap() }; f.field("reserved", &header.reserved); f.field("size", &header.size); @@ -75,22 +71,12 @@ impl Debug for BlockDescriptorHelper { match (self.has_copy_dispose, self.has_signature) { (false, false) => {} (true, false) => { - let descriptor = unsafe { - self.descriptor - .cast::() - .as_ref() - .unwrap() - }; + let descriptor = unsafe { self.descriptor.with_copy_dispose.as_ref().unwrap() }; f.field("copy", &descriptor.copy); f.field("dispose", &descriptor.dispose); } (false, true) => { - let descriptor = unsafe { - self.descriptor - .cast::() - .as_ref() - .unwrap() - }; + let descriptor = unsafe { self.descriptor.with_signature.as_ref().unwrap() }; f.field( "encoding", &if descriptor.encoding.is_null() { @@ -103,7 +89,7 @@ impl Debug for BlockDescriptorHelper { (true, true) => { let descriptor = unsafe { self.descriptor - .cast::() + .with_copy_dispose_signature .as_ref() .unwrap() }; diff --git a/crates/block2/src/global.rs b/crates/block2/src/global.rs index e365f984f..3c3de49e4 100644 --- a/crates/block2/src/global.rs +++ b/crates/block2/src/global.rs @@ -1,14 +1,14 @@ use core::fmt; use core::marker::PhantomData; use core::mem; +use core::mem::MaybeUninit; use core::ops::Deref; use core::ptr; -use core::{ffi::c_void, mem::MaybeUninit}; use std::os::raw::c_ulong; use objc2::encode::EncodeReturn; -use crate::abi::{BlockDescriptor, BlockFlags, BlockLayout}; +use crate::abi::{BlockDescriptor, BlockDescriptorPtr, BlockFlags, BlockLayout}; use crate::debug::debug_block_layout; use crate::{Block, BlockArguments}; @@ -65,7 +65,9 @@ impl GlobalBlock { reserved: MaybeUninit::new(0), // Populated in `global_block!` invoke: None, - descriptor: &GLOBAL_DESCRIPTOR as *const BlockDescriptor as *mut c_void, + descriptor: BlockDescriptorPtr { + basic: &GLOBAL_DESCRIPTOR, + }, }; /// Use the [`global_block`] macro instead. From d730c13da04cbfa608b872c590b5d4f25e9ae1a2 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 7 Jan 2024 10:48:19 +0100 Subject: [PATCH 7/9] Remove hidden __BlockLayout --- crates/block2/src/global.rs | 8 +++++--- crates/block2/src/lib.rs | 2 -- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/block2/src/global.rs b/crates/block2/src/global.rs index 3c3de49e4..84f9a3212 100644 --- a/crates/block2/src/global.rs +++ b/crates/block2/src/global.rs @@ -186,13 +186,15 @@ macro_rules! global_block { let mut layout = $crate::GlobalBlock::<($($t,)*) $(, $r)?>::__DEFAULT_LAYOUT; layout.isa = ::core::ptr::addr_of!($crate::ffi::_NSConcreteGlobalBlock); layout.invoke = ::core::option::Option::Some({ - unsafe extern "C" fn inner(_: *mut $crate::__BlockLayout, $($a: $t),*) $(-> $r)? { + unsafe extern "C" fn inner(_: *mut $crate::GlobalBlock<($($t,)*) $(, $r)?>, $($a: $t),*) $(-> $r)? { $body } - let inner: unsafe extern "C" fn(*mut $crate::__BlockLayout, $($a: $t),*) $(-> $r)? = inner; // TODO: SAFETY - ::core::mem::transmute(inner) + ::core::mem::transmute::< + unsafe extern "C" fn(*mut $crate::GlobalBlock<($($t,)*) $(, $r)?>, $($a: $t),*) $(-> $r)?, + unsafe extern "C" fn(), + >(inner) }); $crate::GlobalBlock::from_layout(layout) }; diff --git a/crates/block2/src/lib.rs b/crates/block2/src/lib.rs index b3b1ebf6d..1daf78174 100644 --- a/crates/block2/src/lib.rs +++ b/crates/block2/src/lib.rs @@ -242,8 +242,6 @@ pub mod ffi; mod global; mod rc_block; -#[doc(hidden)] -pub use abi::BlockLayout as __BlockLayout; pub use block::{Block, BlockArguments}; pub use concrete_block::{ConcreteBlock, IntoConcreteBlock}; pub use global::GlobalBlock; From 7e51d60e09cddafb65dea8285feed8ad9d1ff92b Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 7 Jan 2024 10:52:08 +0100 Subject: [PATCH 8/9] Rename block layout to block header --- crates/block2/src/abi.rs | 4 ++-- crates/block2/src/block.rs | 18 +++++++++--------- crates/block2/src/concrete_block.rs | 14 +++++++------- crates/block2/src/debug.rs | 18 +++++++++--------- crates/block2/src/global.rs | 28 ++++++++++++++-------------- crates/block2/src/rc_block.rs | 8 ++++---- crates/tests/src/ffi.rs | 4 ++-- 7 files changed, 47 insertions(+), 47 deletions(-) diff --git a/crates/block2/src/abi.rs b/crates/block2/src/abi.rs index cbdd89c84..87bd0e147 100644 --- a/crates/block2/src/abi.rs +++ b/crates/block2/src/abi.rs @@ -162,13 +162,13 @@ pub(crate) const BLOCK_FIELD_IS_WEAK: c_int = 16; /// called from __block (byref) copy/dispose support routines. pub(crate) const BLOCK_BYREF_CALLER: c_int = 128; -/// The expected layout of every block. +/// The expected header of every block. #[repr(C)] #[doc(alias = "__block_literal")] #[doc(alias = "Block_layout")] #[doc(alias = "Block_basic")] #[allow(missing_debug_implementations)] -pub struct BlockLayout { +pub struct BlockHeader { /// Class pointer. /// /// Always initialised to &_NSConcreteStackBlock for blocks that are diff --git a/crates/block2/src/block.rs b/crates/block2/src/block.rs index d575cd37d..79f07cc10 100644 --- a/crates/block2/src/block.rs +++ b/crates/block2/src/block.rs @@ -4,8 +4,8 @@ use core::mem; use objc2::encode::{EncodeArgument, EncodeReturn, Encoding, RefEncode}; -use crate::abi::BlockLayout; -use crate::debug::debug_block_layout; +use crate::abi::BlockHeader; +use crate::debug::debug_block_header; /// Types that may be used as the arguments of an Objective-C block. /// @@ -91,10 +91,10 @@ block_args_impl!( #[repr(C)] pub struct Block { _inner: [u8; 0], - // We store `BlockLayout` + the closure captures, but `Block` has to + // We store `BlockHeader` + the closure captures, but `Block` has to // remain an empty type otherwise the compiler thinks we only have - // provenance over `BlockLayout`. - _layout: PhantomData, + // provenance over `BlockHeader`. + _header: PhantomData, // To get correct variance on args and return types _p: PhantomData R>, } @@ -115,9 +115,9 @@ impl Block { /// caller must ensure that calling it will not cause a data race. pub unsafe fn call(&self, args: A) -> R { let ptr: *const Self = self; - let layout = unsafe { ptr.cast::().as_ref().unwrap_unchecked() }; + let header = unsafe { ptr.cast::().as_ref().unwrap_unchecked() }; // TODO: Is `invoke` actually ever null? - let invoke = layout.invoke.unwrap_or_else(|| unreachable!()); + let invoke = header.invoke.unwrap_or_else(|| unreachable!()); unsafe { A::__call_block(invoke, ptr as *mut Self, args) } } @@ -127,8 +127,8 @@ impl fmt::Debug for Block { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("Block"); let ptr: *const Self = self; - let layout = unsafe { ptr.cast::().as_ref().unwrap() }; - debug_block_layout(layout, &mut f); + let header = unsafe { ptr.cast::().as_ref().unwrap() }; + debug_block_header(header, &mut f); f.finish_non_exhaustive() } } diff --git a/crates/block2/src/concrete_block.rs b/crates/block2/src/concrete_block.rs index d1ede14e7..c648a0aeb 100644 --- a/crates/block2/src/concrete_block.rs +++ b/crates/block2/src/concrete_block.rs @@ -8,8 +8,8 @@ use std::os::raw::c_ulong; use objc2::encode::{EncodeArgument, EncodeReturn, Encoding, RefEncode}; -use crate::abi::{BlockDescriptorCopyDispose, BlockDescriptorPtr, BlockFlags, BlockLayout}; -use crate::debug::debug_block_layout; +use crate::abi::{BlockDescriptorCopyDispose, BlockDescriptorPtr, BlockFlags, BlockHeader}; +use crate::debug::debug_block_header; use crate::{ffi, Block, BlockArguments, RcBlock}; mod private { @@ -166,7 +166,7 @@ concrete_block_impl!( #[repr(C)] pub struct ConcreteBlock { p: PhantomData>, - pub(crate) layout: BlockLayout, + pub(crate) header: BlockHeader, pub(crate) closure: F, } @@ -215,7 +215,7 @@ impl ConcreteBlock { /// Unsafe because the caller must ensure the invoke function takes the /// correct arguments. unsafe fn with_invoke(invoke: unsafe extern "C" fn(), closure: F) -> Self { - let layout = BlockLayout { + let header = BlockHeader { isa: unsafe { ptr::addr_of!(ffi::_NSConcreteStackBlock) }, flags: Self::FLAGS, reserved: MaybeUninit::new(0), @@ -226,7 +226,7 @@ impl ConcreteBlock { }; Self { p: PhantomData, - layout, + header, closure, } } @@ -246,7 +246,7 @@ impl ConcreteBlock { impl Clone for ConcreteBlock { fn clone(&self) -> Self { - unsafe { Self::with_invoke(self.layout.invoke.unwrap(), self.closure.clone()) } + unsafe { Self::with_invoke(self.header.invoke.unwrap(), self.closure.clone()) } } } @@ -272,7 +272,7 @@ unsafe extern "C" fn block_context_copy(_dst: *mut c_void, _src: *mut c_void) impl fmt::Debug for ConcreteBlock { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("ConcreteBlock"); - debug_block_layout(&self.layout, &mut f); + debug_block_header(&self.header, &mut f); f.field("closure", &self.closure); f.finish() } diff --git a/crates/block2/src/debug.rs b/crates/block2/src/debug.rs index 3cd579b2a..503db5ef3 100644 --- a/crates/block2/src/debug.rs +++ b/crates/block2/src/debug.rs @@ -2,7 +2,7 @@ use core::fmt::{Debug, DebugStruct, Error, Formatter}; use core::ptr; use std::ffi::CStr; -use crate::abi::{BlockDescriptorPtr, BlockFlags, BlockLayout}; +use crate::abi::{BlockDescriptorPtr, BlockFlags, BlockHeader}; use crate::ffi; #[derive(Clone, Copy, PartialEq, Eq)] @@ -33,17 +33,17 @@ impl Debug for Isa { } } -pub(crate) fn debug_block_layout(layout: &BlockLayout, f: &mut DebugStruct<'_, '_>) { - f.field("isa", &Isa(layout.isa)); - f.field("flags", &layout.flags); - f.field("reserved", &layout.reserved); - f.field("invoke", &layout.invoke); +pub(crate) fn debug_block_header(header: &BlockHeader, f: &mut DebugStruct<'_, '_>) { + f.field("isa", &Isa(header.isa)); + f.field("flags", &header.flags); + f.field("reserved", &header.reserved); + f.field("invoke", &header.invoke); f.field( "descriptor", &BlockDescriptorHelper { - has_copy_dispose: layout.flags.0 & BlockFlags::BLOCK_HAS_COPY_DISPOSE.0 != 0, - has_signature: layout.flags.0 & BlockFlags::BLOCK_HAS_SIGNATURE.0 != 0, - descriptor: layout.descriptor, + has_copy_dispose: header.flags.0 & BlockFlags::BLOCK_HAS_COPY_DISPOSE.0 != 0, + has_signature: header.flags.0 & BlockFlags::BLOCK_HAS_SIGNATURE.0 != 0, + descriptor: header.descriptor, }, ); } diff --git a/crates/block2/src/global.rs b/crates/block2/src/global.rs index 84f9a3212..2ef393a1b 100644 --- a/crates/block2/src/global.rs +++ b/crates/block2/src/global.rs @@ -8,14 +8,14 @@ use std::os::raw::c_ulong; use objc2::encode::EncodeReturn; -use crate::abi::{BlockDescriptor, BlockDescriptorPtr, BlockFlags, BlockLayout}; -use crate::debug::debug_block_layout; +use crate::abi::{BlockDescriptor, BlockDescriptorPtr, BlockFlags, BlockHeader}; +use crate::debug::debug_block_header; use crate::{Block, BlockArguments}; // TODO: Should this be a static to help the compiler deduplicating them? const GLOBAL_DESCRIPTOR: BlockDescriptor = BlockDescriptor { reserved: 0, - size: mem::size_of::() as c_ulong, + size: mem::size_of::() as c_ulong, }; /// An Objective-C block that does not capture its environment. @@ -30,7 +30,7 @@ const GLOBAL_DESCRIPTOR: BlockDescriptor = BlockDescriptor { /// [`global_block!`]: crate::global_block #[repr(C)] pub struct GlobalBlock { - pub(crate) layout: BlockLayout, + pub(crate) header: BlockHeader, p: PhantomData<(A, R)>, } @@ -58,7 +58,7 @@ impl GlobalBlock { BlockFlags(BlockFlags::BLOCK_IS_GLOBAL.0 | BlockFlags::BLOCK_USE_STRET.0); #[doc(hidden)] - pub const __DEFAULT_LAYOUT: BlockLayout = BlockLayout { + pub const __DEFAULT_HEADER: BlockHeader = BlockHeader { // Populated in `global_block!` isa: ptr::null_mut(), flags: Self::FLAGS, @@ -72,9 +72,9 @@ impl GlobalBlock { /// Use the [`global_block`] macro instead. #[doc(hidden)] - pub const unsafe fn from_layout(layout: BlockLayout) -> Self { + pub const unsafe fn from_header(header: BlockHeader) -> Self { Self { - layout, + header, p: PhantomData, } } @@ -98,7 +98,7 @@ where impl fmt::Debug for GlobalBlock { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("GlobalBlock"); - debug_block_layout(&self.layout, &mut f); + debug_block_header(&self.header, &mut f); f.finish_non_exhaustive() } } @@ -183,9 +183,9 @@ macro_rules! global_block { $(#[$m])* #[allow(unused_unsafe)] $vis static $name: $crate::GlobalBlock<($($t,)*) $(, $r)?> = unsafe { - let mut layout = $crate::GlobalBlock::<($($t,)*) $(, $r)?>::__DEFAULT_LAYOUT; - layout.isa = ::core::ptr::addr_of!($crate::ffi::_NSConcreteGlobalBlock); - layout.invoke = ::core::option::Option::Some({ + let mut header = $crate::GlobalBlock::<($($t,)*) $(, $r)?>::__DEFAULT_HEADER; + header.isa = ::core::ptr::addr_of!($crate::ffi::_NSConcreteGlobalBlock); + header.invoke = ::core::option::Option::Some({ unsafe extern "C" fn inner(_: *mut $crate::GlobalBlock<($($t,)*) $(, $r)?>, $($a: $t),*) $(-> $r)? { $body } @@ -196,7 +196,7 @@ macro_rules! global_block { unsafe extern "C" fn(), >(inner) }); - $crate::GlobalBlock::from_layout(layout) + $crate::GlobalBlock::from_header(header) }; }; } @@ -267,8 +267,8 @@ mod tests { #[test] fn test_debug() { - let invoke = NOOP_BLOCK.layout.invoke.unwrap(); - let size = mem::size_of::(); + let invoke = NOOP_BLOCK.header.invoke.unwrap(); + let size = mem::size_of::(); let expected = format!( "GlobalBlock {{ isa: _NSConcreteGlobalBlock, diff --git a/crates/block2/src/rc_block.rs b/crates/block2/src/rc_block.rs index 1291d3ba2..d7ce12de5 100644 --- a/crates/block2/src/rc_block.rs +++ b/crates/block2/src/rc_block.rs @@ -1,8 +1,8 @@ use core::fmt; use core::ops::Deref; -use crate::abi::BlockLayout; -use crate::debug::debug_block_layout; +use crate::abi::BlockHeader; +use crate::debug::debug_block_header; use crate::{ffi, Block}; /// A reference-counted Objective-C block. @@ -64,8 +64,8 @@ impl Drop for RcBlock { impl fmt::Debug for RcBlock { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("RcBlock"); - let layout = unsafe { self.ptr.cast::().as_ref().unwrap() }; - debug_block_layout(layout, &mut f); + let header = unsafe { self.ptr.cast::().as_ref().unwrap() }; + debug_block_header(header, &mut f); f.finish_non_exhaustive() } } diff --git a/crates/tests/src/ffi.rs b/crates/tests/src/ffi.rs index 517aa6806..def91c157 100644 --- a/crates/tests/src/ffi.rs +++ b/crates/tests/src/ffi.rs @@ -69,8 +69,8 @@ extern "C" { } #[no_mangle] -extern "C" fn debug_block(layout: *mut c_void) { - let block: &Block<(), ()> = unsafe { &*(layout as *const Block<(), ()>) }; +extern "C" fn debug_block(block: *mut c_void) { + let block: &Block<(), ()> = unsafe { &*(block as *const Block<(), ()>) }; std::println!("{block:#?}"); } From 4ecabc61b15d944438d404fb755609705def249f Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 7 Jan 2024 10:56:43 +0100 Subject: [PATCH 9/9] Ensure no block ABI types have trailing padding --- crates/block2/src/abi.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/block2/src/abi.rs b/crates/block2/src/abi.rs index 87bd0e147..0ea8549c5 100644 --- a/crates/block2/src/abi.rs +++ b/crates/block2/src/abi.rs @@ -296,3 +296,26 @@ pub(crate) struct BlockDescriptorCopyDisposeSignature { #[doc(alias = "signature")] pub(crate) encoding: *const c_char, } + +#[cfg(test)] +mod tests { + use super::*; + + fn assert_no_trailing_padding() { + struct AddU8 { + t: T, + extra: u8, + } + assert_ne!(core::mem::size_of::(), core::mem::size_of::>()); + } + + #[test] + fn no_types_have_padding() { + assert_no_trailing_padding::(); + assert_no_trailing_padding::(); + assert_no_trailing_padding::(); + assert_no_trailing_padding::(); + assert_no_trailing_padding::(); + assert_no_trailing_padding::(); + } +}