Skip to content

Commit

Permalink
Fix tests in old runtime / on macOS 10.12 / with panic=abort
Browse files Browse the repository at this point in the history
  • Loading branch information
madsmtm committed Jan 22, 2025
1 parent 060b671 commit 5e6f1af
Show file tree
Hide file tree
Showing 12 changed files with 121 additions and 93 deletions.
93 changes: 61 additions & 32 deletions crates/objc2/src/__macro_helpers/msg_send_retained.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ mod tests {
use super::*;

use crate::rc::{autoreleasepool, Allocated, PartialInit, RcTestObject, ThreadTestData};
use crate::runtime::{AnyObject, NSObject, NSZone};
use crate::runtime::{AnyObject, NSObject, NSObjectProtocol, NSZone};
use crate::{class, define_class, extern_methods, msg_send, test_utils, AllocAnyThread};

#[test]
Expand Down Expand Up @@ -787,34 +787,29 @@ mod tests {
let _obj: Retained<AnyObject> = unsafe { msg_send![obj, description] };
}

// This is imperfect, but will do for now.
// See also `tests/id_retain_autoreleased.rs`.
//
// Work around https://github.com/rust-lang/rust-clippy/issues/9737:
#[allow(clippy::if_same_then_else)]
const IF_AUTORELEASE_NOT_SKIPPED: usize = if cfg!(feature = "gnustep-1-7") {
1
} else if cfg!(target_arch = "x86") {
// x86 autorelease_return is not currently tail-called, so the
// optimization doesn't work on define_class! functions.
2
} else if cfg!(target_arch = "aarch64") {
// Currently doesn't work
2
} else if cfg!(any(debug_assertions, feature = "catch-all")) {
2
} else {
1
} - 1;

// 32-bit ARM unwinding sometimes interferes with the optimization
const IF_AUTORELEASE_NOT_SKIPPED_ARM_HACK: usize = {
if cfg!(all(target_arch = "arm", panic = "unwind")) {
1
/// This is imperfect, but will do for now.
const fn autorelease_skipped(self_declared: bool) -> bool {
if cfg!(feature = "gnustep-1-7") {
// GNUStep does the optimization a different way, so it isn't
// optimization-dependent.
true
} else if cfg!(all(target_arch = "arm", panic = "unwind")) {
// 32-bit ARM unwinding sometimes interferes with the optimization
false
} else if self_declared {
// FIXME: Autorelease_return is not currently tail-called, so the
// optimization doesn't work on define_class! functions.
false
} else if cfg!(feature = "catch-all") {
// FIXME: `catch-all` is inserted before we get a chance to retain.
false
} else if cfg!(debug_assertions) {
// `debug_assertions` ~proxy for if optimizations are off.
false
} else {
IF_AUTORELEASE_NOT_SKIPPED
true
}
};
}

macro_rules! test_error_retained {
($expected:expr, $if_autorelease_not_skipped:expr, $sel:ident, $($obj:tt)*) => {
Expand Down Expand Up @@ -868,7 +863,7 @@ mod tests {
let cls = RcTestObject::class();
test_error_retained!(
expected,
IF_AUTORELEASE_NOT_SKIPPED_ARM_HACK,
if autorelease_skipped(true) { 0 } else { 1 },
idAndShouldError,
cls
);
Expand All @@ -879,7 +874,7 @@ mod tests {
expected.init += 1;
test_error_retained!(
expected,
IF_AUTORELEASE_NOT_SKIPPED_ARM_HACK,
if autorelease_skipped(true) { 0 } else { 1 },
idAndShouldError,
&obj
);
Expand Down Expand Up @@ -913,15 +908,49 @@ mod tests {
assert!(res.is_some());
expected.alloc += 1;
expected.init += 1;
expected.autorelease += IF_AUTORELEASE_NOT_SKIPPED_ARM_HACK;
expected.retain += IF_AUTORELEASE_NOT_SKIPPED_ARM_HACK;
expected.autorelease += if autorelease_skipped(true) { 0 } else { 1 };
expected.retain += if autorelease_skipped(true) { 0 } else { 1 };
expected.assert_current();
res
});
expected.release += IF_AUTORELEASE_NOT_SKIPPED_ARM_HACK;
expected.release += if autorelease_skipped(true) { 0 } else { 1 };
expected.assert_current();
}

fn create_obj() -> Retained<NSObject> {
let obj = ManuallyDrop::new(NSObject::new());
unsafe {
let obj: *mut NSObject = msg_send![&*obj, autorelease];
// All code between the `msg_send!` and the `retain_autoreleased`
// must be able to be optimized away for this to work.
Retained::retain_autoreleased(obj).unwrap()
}
}

#[test]
fn test_retain_autoreleased() {
autoreleasepool(|_| {
// Run once to allow DYLD to resolve the symbol stubs.
// Required for making `retain_autoreleased` work on x86_64.
let _data = create_obj();

// When compiled in release mode / with optimizations enabled,
// subsequent usage of `retain_autoreleased` will succeed in
// retaining the autoreleased value!
let expected = if autorelease_skipped(false) { 1 } else { 2 };

let data = create_obj();
assert_eq!(data.retainCount(), expected);

let data = create_obj();
assert_eq!(data.retainCount(), expected);

// Here we manually clean up the autorelease, so it will always be 1.
let data = autoreleasepool(|_| create_obj());
assert_eq!(data.retainCount(), 1);
});
}

#[test]
fn msg_send_class() {
let cls = NSObject::class();
Expand Down
2 changes: 1 addition & 1 deletion crates/objc2/src/exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ mod tests {

#[test]
#[cfg_attr(
all(target_vendor = "apple", target_os = "macos", target_arch = "x86"),
all(target_os = "macos", target_arch = "x86"),
ignore = "`NULL` exceptions are invalid on 32-bit / w. fragile runtime"
)]
fn test_catch_null() {
Expand Down
5 changes: 3 additions & 2 deletions crates/objc2/src/rc/autorelease.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Pool {
/// `drop` when possible, so that is what we are going to do.
///
/// [clang documentation]: https://clang.llvm.org/docs/AutomaticReferenceCounting.html#autoreleasepool
/// [revision `551.1`]: https://github.com/apple-oss-distributions/objc4/blob/objc4-551.1/runtime/objc-exception.mm#L516
/// [revision `551.1`]: https://github.com/apple-oss-distributions/objc4/blob/objc4-551.1/runtime/objc-exception.mm#L540
#[inline]
unsafe fn drain(self) {
#[cfg(all(target_os = "macos", target_arch = "x86"))]
Expand Down Expand Up @@ -582,8 +582,9 @@ mod tests {
}

#[test]
#[cfg_attr(panic = "abort", ignore = "requires `catch_unwind`")]
#[cfg_attr(
all(target_os = "macos", target_arch = "x86"),
all(target_os = "macos", target_arch = "x86", not(panic = "abort")),
ignore = "unwinding through an auto release pool on macOS 32 bit won't pop the pool"
)]
fn test_unwind_still_autoreleases() {
Expand Down
11 changes: 9 additions & 2 deletions crates/objc2/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1886,8 +1886,15 @@ mod tests {
assert_eq!(&*retained, protocol);

// Protocols don't implement isKindOfClass: on GNUStep.
if !cfg!(feature = "gnustep-1-7") {
// Protocols are NSObject subclasses.
if cfg!(feature = "gnustep-1-7") {
return;
}

// In the old runtime, NSObjectProtocol are not NSObject subclasses.
if cfg!(all(target_os = "macos", target_arch = "x86")) {
let _ = retained.downcast::<NSObject>().unwrap_err();
} else {
// But elsewhere they are.
let obj = retained.downcast::<NSObject>().unwrap();
// Test that we can call NSObject methods on protocols.
assert_eq!(obj, obj);
Expand Down
4 changes: 4 additions & 0 deletions crates/objc2/tests/protocol_shared_retain_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ use objc2::{Message, ProtocolType};
feature = "gnustep-1-7",
ignore = "Protocols don't implement isKindOfClass: on GNUStep"
)]
#[cfg_attr(
all(target_os = "macos", target_arch = "x86"),
ignore = "protocols are not NSObject subclasses in the old runtime"
)]
fn protocol_has_shared_retain_count() {
let obj: &AnyObject = <dyn NSObjectProtocol>::protocol().unwrap().as_ref();
let obj = obj.downcast_ref::<NSObject>().unwrap();
Expand Down
49 changes: 0 additions & 49 deletions crates/objc2/tests/retain_autoreleased.rs

This file was deleted.

1 change: 1 addition & 0 deletions crates/objc2/tests/track_caller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ impl Drop for PanicChecker {
}

#[test]
#[cfg_attr(panic = "abort", ignore = "requires `catch_unwind`")]
fn test_track_caller() {
let checker = PanicChecker::new();

Expand Down
20 changes: 15 additions & 5 deletions crates/tests/src/exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,19 @@ fn throw_catch_raise_catch() {
let exc = res.unwrap_err().unwrap();
let exc = NSException::from_exception(exc).unwrap();

assert_eq!(exc.retainCount(), 1);
if cfg!(all(target_os = "macos", target_arch = "x86")) {
assert_eq!(exc.retainCount(), 2);
} else {
assert_eq!(exc.retainCount(), 1);
}
exc
});

assert_eq!(exc.retainCount(), 1);
if cfg!(all(target_os = "macos", target_arch = "x86")) {
assert_eq!(exc.retainCount(), 2);
} else {
assert_eq!(exc.retainCount(), 1);
}

let exc = autoreleasepool(|_| {
let res = catch(|| {
Expand All @@ -42,9 +50,11 @@ fn throw_catch_raise_catch() {

let exc = NSException::from_exception(res.unwrap_err().unwrap()).unwrap();

// Undesired: The inner pool _should_ have been drained on unwind, but
// it isn't, see `rc::Pool::drain`.
assert_eq!(exc.retainCount(), 2);
if cfg!(all(target_os = "macos", target_arch = "x86")) {
assert_eq!(exc.retainCount(), 3);
} else {
assert_eq!(exc.retainCount(), 1);
}
exc
});

Expand Down
7 changes: 6 additions & 1 deletion framework-crates/objc2-core-foundation/src/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,12 @@ mod tests {
#[test]
fn to_from_number() {
let n = CFNumber::new_i32(442);
assert_eq!(n.as_i8(), Some(442i32 as i8));
if cfg!(all(target_os = "macos", target_arch = "x86")) {
// Seems to return `None` in the old runtime.
assert_eq!(n.as_i8(), None);
} else {
assert_eq!(n.as_i8(), Some(442i32 as i8));
}
assert_eq!(n.as_i16(), Some(442));
assert_eq!(n.as_i32(), Some(442));
assert_eq!(n.as_i64(), Some(442));
Expand Down
8 changes: 8 additions & 0 deletions framework-crates/objc2-foundation/src/tests/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ fn test_from_base_class(cls: &AnyClass) {
feature = "gnustep-1-7",
ignore = "GNUStep stack overflows here for some reason?"
)]
#[cfg_attr(
all(target_os = "macos", target_arch = "x86"),
ignore = "the old runtime seems to have been compiled with -fno-exceptions?"
)]
fn no_copy() {
let mut builder = base_class_builder("NoCopy").unwrap();

Expand Down Expand Up @@ -190,6 +194,10 @@ fn no_copy() {
feature = "gnustep-1-7",
ignore = "GNUStep stack overflows here for some reason?"
)]
#[cfg_attr(
all(target_os = "macos", target_arch = "x86"),
ignore = "the old runtime seems to have been compiled with -fno-exceptions?"
)]
fn no_is_equal_hash() {
let mut builder = base_class_builder("NoIsEqualHash").unwrap();

Expand Down
9 changes: 8 additions & 1 deletion framework-crates/objc2-foundation/src/tests/mutable_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,14 @@ fn test_sort() {

#[test]
#[cfg(feature = "NSValue")]
#[should_panic = "`itemsPtr` was NULL, likely due to mutation during iteration"]
#[cfg_attr(
all(target_os = "macos", target_arch = "x86"),
should_panic = "mutation detected during enumeration"
)]
#[cfg_attr(
not(all(target_os = "macos", target_arch = "x86")),
should_panic = "`itemsPtr` was NULL, likely due to mutation during iteration"
)]
#[cfg_attr(feature = "gnustep-1-7", ignore = "errors differently on GNUStep")]
fn null_itemsptr() {
use crate::NSNumber;
Expand Down
5 changes: 5 additions & 0 deletions framework-crates/objc2-metal/tests/null_error_handling.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Regression test for https://github.com/madsmtm/objc2/issues/653.
#![cfg(all(feature = "MTLBinaryArchive", feature = "MTLDevice"))]
use objc2::available;
use objc2_foundation::{ns_string, NSURL};
use objc2_metal::{
MTLBinaryArchive, MTLBinaryArchiveDescriptor, MTLCreateSystemDefaultDevice, MTLDevice,
Expand All @@ -14,6 +15,10 @@ fn test() {
// Ignore, this won't work in CI.
return;
};
// `MTLBinaryArchiveDescriptor` is unavailable before these.
if !available!(ios = 14.0, macos = 11.0, tvos = 14.0, visionos = 1.0) {
return;
}
let binary_archive = device
.newBinaryArchiveWithDescriptor_error(&MTLBinaryArchiveDescriptor::new())
.unwrap();
Expand Down

0 comments on commit 5e6f1af

Please sign in to comment.