Fix errors found during release testing

.github/workflows/ci.yml

@@ -733,7 +733,7 @@ jobs:
       run: cargo install cargo-fuzz --version=^0.11.2
 
     - name: Test Foundation
-      run: cargo test $ARGS --features=$INTERESTING_FEATURES,catch-all,Foundation
+      run: cargo test $ARGS --features=$INTERESTING_FEATURES,catch-all,Foundation_all
 
     - name: Test all frameworks
       if: ${{ env.FULL }}
@@ -742,7 +742,7 @@ jobs:
     - name: Test in release mode
       # Disabled on GNUStep 2.1 for now
       if: ${{ env.FULL && matrix.runtime != 'gnustep-2-1' }}
-      run: cargo test $ARGS --features=Foundation --release
+      run: cargo test $ARGS --features=Foundation_all --release
 
     - name: Run fuzzing
       if: ${{ matrix.fuzz }}
@@ -770,7 +770,7 @@ jobs:
 
     - name: Test with unstable features
       if: ${{ env.FULL && matrix.nightly }}
-      run: cargo test $ARGS --features=$INTERESTING_FEATURES,catch-all,Foundation,$UNSTABLE_FEATURES
+      run: cargo test $ARGS --features=$INTERESTING_FEATURES,catch-all,Foundation_all,$UNSTABLE_FEATURES
 
   test-compiler-rt:
     name: Test Compiler-RT

crates/icrate/tests/mutable_array.rs

@@ -1,6 +1,6 @@
 #![cfg(feature = "Foundation_NSMutableArray")]
-use objc2::msg_send;
 use objc2::rc::{__RcTestObject, __ThreadTestData, autoreleasepool};
+use objc2::{msg_send, ClassType};
 
 #[cfg(feature = "Foundation_NSNumber")]
 use icrate::Foundation::NSNumber;
@@ -57,6 +57,26 @@ fn test_replace() {
 
 #[test]
 #[cfg(feature = "Foundation_NSMutableString")]
+#[cfg_attr(
+    feature = "gnustep-1-7",
+    ignore = "thread safety issues regarding initialization"
+)]
+fn test_containing_mutable_objects() {
+    use Foundation::NSMutableString;
+
+    let mut array = NSMutableArray::from_vec(vec![NSMutableString::new()]);
+    let _: &mut NSMutableString = &mut array[0];
+    let _: &mut NSMutableString = array.get_mut(0).unwrap();
+    let _: &mut NSMutableString = array.first_mut().unwrap();
+    let _: &mut NSMutableString = array.last_mut().unwrap();
+}
+
+#[test]
+#[cfg(feature = "Foundation_NSMutableString")]
+#[cfg_attr(
+    feature = "gnustep-1-7",
+    ignore = "thread safety issues regarding initialization"
+)]
 fn test_allowed_mutation_while_iterating() {
     use Foundation::{NSMutableString, NSString};
 
@@ -77,6 +97,10 @@ fn test_allowed_mutation_while_iterating() {
     not(debug_assertions),
     ignore = "enumeration mutation only detected with debug assertions on"
 )]
+#[cfg_attr(
+    all(debug_assertions, feature = "gnustep-1-7"),
+    ignore = "thread safety issues regarding initialization"
+)]
 fn test_iter_mutation_detection() {
     let array = NSMutableArray::from_id_slice(&[NSObject::new(), NSObject::new()]);
 
@@ -86,6 +110,24 @@ fn test_iter_mutation_detection() {
     }
 }
 
+#[test]
+#[cfg_attr(
+    feature = "gnustep-1-7",
+    ignore = "thread safety issues regarding initialization"
+)]
+fn test_threaded() {
+    std::thread::scope(|s| {
+        s.spawn(|| {
+            let _ = NSMutableArray::from_vec(vec![NSObject::new(), NSObject::new()]);
+        });
+
+        s.spawn(|| {
+            let array = <NSMutableArray<NSObject>>::alloc();
+            assert!(array.is_some());
+        });
+    });
+}
+
 #[test]
 fn test_remove() {
     let mut array = NSMutableArray::new();

crates/objc2/src/rc/test_object.rs

@@ -41,7 +41,13 @@ impl __ThreadTestData {
             // GNUStep doesn't call `autorelease` if it's overridden
             expected.autorelease = 0;
         }
-        assert_eq!(current, expected);
+        if current != expected {
+            panic!(
+                "got differing amounts of calls:
+   current: `{current:?}`,
+  expected: `{expected:?}`"
+            )
+        }
     }
 }
 
@@ -385,6 +391,10 @@ mod tests {
     } else if cfg!(all(target_arch = "arm", panic = "unwind")) {
         // 32-bit ARM unwinding interferes with the optimization
         2
+    } else if cfg!(target_arch = "x86") {
+        // x86 autorelease_return is not currently tail-called, so the
+        // optimization doesn't work on declare_class! functions.
+        2
     } else if cfg!(any(debug_assertions, feature = "exception")) {
         2
     } else {

crates/test-assembly/crates/test_autorelease_return/Cargo.toml

@@ -0,0 +1,25 @@
+[package]
+name = "test_autorelease_return"
+version = "0.1.0"
+edition = "2021"
+publish = false
+
+[lib]
+path = "lib.rs"
+
+[dependencies]
+objc2 = { path = "../../../objc2", default-features = false }
+
+[features]
+default = ["apple", "std"]
+std = ["objc2/std"]
+# Runtime
+apple = ["objc2/apple"]
+gnustep-1-7 = ["objc2/gnustep-1-7"]
+gnustep-1-8 = ["gnustep-1-7", "objc2/gnustep-1-8"]
+gnustep-1-9 = ["gnustep-1-8", "objc2/gnustep-1-9"]
+gnustep-2-0 = ["gnustep-1-9", "objc2/gnustep-2-0"]
+gnustep-2-1 = ["gnustep-2-0", "objc2/gnustep-2-1"]
+
+# Hack
+assembly-features = []

crates/test-assembly/crates/test_autorelease_return/expected/apple-aarch64.s

@@ -0,0 +1,16 @@
+	.section	__TEXT,__text,regular,pure_instructions
+	.globl	_simple
+	.p2align	2
+_simple:
+	b	_objc_autoreleaseReturnValue
+
+	.globl	_with_body
+	.p2align	2
+_with_body:
+	stp	x29, x30, [sp, #-16]!
+	mov	x29, sp
+	bl	_objc_msgSend
+	ldp	x29, x30, [sp], #16
+	b	_objc_autoreleaseReturnValue
+
+.subsections_via_symbols

crates/test-assembly/crates/test_autorelease_return/expected/apple-armv7.s

@@ -0,0 +1,19 @@
+	.section	__TEXT,__text,regular,pure_instructions
+	.syntax unified
+	.globl	_simple
+	.p2align	2
+	.code	32
+_simple:
+	b	_objc_autoreleaseReturnValue
+
+	.globl	_with_body
+	.p2align	2
+	.code	32
+_with_body:
+	push	{r7, lr}
+	mov	r7, sp
+	bl	_objc_msgSend
+	pop	{r7, lr}
+	b	_objc_autoreleaseReturnValue
+
+.subsections_via_symbols

crates/test-assembly/crates/test_autorelease_return/expected/apple-armv7s.s

@@ -0,0 +1,22 @@
+	.section	__TEXT,__text,regular,pure_instructions
+	.syntax unified
+	.globl	_simple
+	.p2align	2
+	.code	32
+_simple:
+	push	{r7, lr}
+	mov	r7, sp
+	bl	_objc_autoreleaseReturnValue
+	pop	{r7, pc}
+
+	.globl	_with_body
+	.p2align	2
+	.code	32
+_with_body:
+	push	{r7, lr}
+	mov	r7, sp
+	bl	_objc_msgSend
+	bl	_objc_autoreleaseReturnValue
+	pop	{r7, pc}
+
+.subsections_via_symbols

crates/test-assembly/crates/test_autorelease_return/expected/apple-x86.s

@@ -0,0 +1,28 @@
+	.section	__TEXT,__text,regular,pure_instructions
+	.intel_syntax noprefix
+	.globl	_simple
+	.p2align	4, 0x90
+_simple:
+	push	ebp
+	mov	ebp, esp
+	pop	ebp
+	jmp	_objc_autoreleaseReturnValue
+
+	.globl	_with_body
+	.p2align	4, 0x90
+_with_body:
+	push	ebp
+	mov	ebp, esp
+	sub	esp, 8
+	mov	eax, dword ptr [ebp + 8]
+	mov	ecx, dword ptr [ebp + 12]
+	mov	dword ptr [esp + 4], ecx
+	mov	dword ptr [esp], eax
+	call	_objc_msgSend
+	mov	dword ptr [esp], eax
+	call	_objc_autoreleaseReturnValue
+	add	esp, 8
+	pop	ebp
+	ret
+
+.subsections_via_symbols

crates/test-assembly/crates/test_autorelease_return/expected/apple-x86_64.s

@@ -0,0 +1,21 @@
+	.section	__TEXT,__text,regular,pure_instructions
+	.intel_syntax noprefix
+	.globl	_simple
+	.p2align	4, 0x90
+_simple:
+	push	rbp
+	mov	rbp, rsp
+	pop	rbp
+	jmp	_objc_autoreleaseReturnValue
+
+	.globl	_with_body
+	.p2align	4, 0x90
+_with_body:
+	push	rbp
+	mov	rbp, rsp
+	call	_objc_msgSend
+	mov	rdi, rax
+	pop	rbp
+	jmp	_objc_autoreleaseReturnValue
+
+.subsections_via_symbols

crates/test-assembly/crates/test_autorelease_return/expected/gnustep-x86.s

@@ -0,0 +1,56 @@
+	.text
+	.intel_syntax noprefix
+	.section	.text.simple,"ax",@progbits
+	.globl	simple
+	.p2align	4, 0x90
+	.type	simple,@function
+simple:
+	push	ebx
+	sub	esp, 8
+	mov	eax, dword ptr [esp + 16]
+	call	.L0$pb
+.L0$pb:
+	pop	ebx
+.Ltmp0:
+	add	ebx, offset _GLOBAL_OFFSET_TABLE_+(.Ltmp0-.L0$pb)
+	mov	dword ptr [esp], eax
+	call	objc_autoreleaseReturnValue@PLT
+	add	esp, 8
+	pop	ebx
+	ret
+.Lfunc_end0:
+	.size	simple, .Lfunc_end0-simple
+
+	.section	.text.with_body,"ax",@progbits
+	.globl	with_body
+	.p2align	4, 0x90
+	.type	with_body,@function
+with_body:
+	push	ebx
+	push	edi
+	push	esi
+	sub	esp, 16
+	mov	esi, dword ptr [esp + 32]
+	mov	edi, dword ptr [esp + 36]
+	call	.L1$pb
+.L1$pb:
+	pop	ebx
+.Ltmp1:
+	add	ebx, offset _GLOBAL_OFFSET_TABLE_+(.Ltmp1-.L1$pb)
+	mov	dword ptr [esp + 4], edi
+	mov	dword ptr [esp], esi
+	call	objc_msg_lookup@PLT
+	mov	dword ptr [esp + 4], edi
+	mov	dword ptr [esp], esi
+	call	eax
+	mov	dword ptr [esp], eax
+	call	objc_autoreleaseReturnValue@PLT
+	add	esp, 16
+	pop	esi
+	pop	edi
+	pop	ebx
+	ret
+.Lfunc_end1:
+	.size	with_body, .Lfunc_end1-with_body
+
+	.section	".note.GNU-stack","",@progbits

crates/test-assembly/crates/test_autorelease_return/expected/gnustep-x86_64.s

@@ -0,0 +1,34 @@
+	.text
+	.intel_syntax noprefix
+	.section	.text.simple,"ax",@progbits
+	.globl	simple
+	.p2align	4, 0x90
+	.type	simple,@function
+simple:
+	jmp	qword ptr [rip + objc_autoreleaseReturnValue@GOTPCREL]
+.Lfunc_end0:
+	.size	simple, .Lfunc_end0-simple
+
+	.section	.text.with_body,"ax",@progbits
+	.globl	with_body
+	.p2align	4, 0x90
+	.type	with_body,@function
+with_body:
+	push	r14
+	push	rbx
+	push	rax
+	mov	rbx, rsi
+	mov	r14, rdi
+	call	qword ptr [rip + objc_msg_lookup@GOTPCREL]
+	mov	rdi, r14
+	mov	rsi, rbx
+	call	rax
+	mov	rdi, rax
+	add	rsp, 8
+	pop	rbx
+	pop	r14
+	jmp	qword ptr [rip + objc_autoreleaseReturnValue@GOTPCREL]
+.Lfunc_end1:
+	.size	with_body, .Lfunc_end1-with_body
+
+	.section	".note.GNU-stack","",@progbits

crates/test-assembly/crates/test_autorelease_return/lib.rs

@@ -0,0 +1,16 @@
+//! Test that `Id::autorelease_return` is tail-called properly.
+
+use objc2::__macro_helpers::{MsgSendId, New};
+use objc2::rc::Id;
+use objc2::runtime::{Class, Object, Sel};
+
+#[no_mangle]
+fn simple(obj: Id<Object>) -> *mut Object {
+    Id::autorelease_return(obj)
+}
+
+#[no_mangle]
+unsafe fn with_body(cls: &Class, sel: Sel) -> *mut Object {
+    let obj: Option<Id<Object>> = New::send_message_id(cls, sel, ());
+    Id::autorelease_return(obj.unwrap_unchecked())
+}