Skip to content

Commit

Permalink
Merge pull request #128 from madsmtm/soundness-fixes
Browse files Browse the repository at this point in the history 
Soundness fixes
  • Loading branch information
@madsmtm
madsmtm authored Feb 28, 2022
2 parents a10fec8 + b987506 commit a16156c
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 113 deletions.
2 changes: 1 addition & 1 deletion block-sys/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub struct Class {
_priv: [u8; 0],

/// See objc_sys::OpaqueData
_opaque: PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>,
_opaque: UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>,
}

/// Block descriptor flags.
Expand Down
2 changes: 1 addition & 1 deletion objc-sys/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ pub use various::*;
/// (It's also less of a breaking change on our part if we re-add these).
///
/// TODO: Replace this with `extern type` to also mark it as `!Sized`.
type OpaqueData = PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>;
type OpaqueData = UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>;

#[cfg(test)]
mod tests {
Expand Down
13 changes: 2 additions & 11 deletions objc2-foundation/src/array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,10 @@ unsafe impl<T: Send> Send for NSArray<T, Owned> {}

object! {
// TODO: Ensure that this deref to NSArray is safe!
unsafe pub struct NSMutableArray<T, O: Ownership>: NSArray<T, O> {
item: PhantomData<Id<T, O>>,
}
// This "inherits" NSArray, and has the same `Send`/`Sync` impls as that.
unsafe pub struct NSMutableArray<T, O: Ownership>: NSArray<T, O> {}
}

// SAFETY: Same as NSArray.
//
// TODO: Properly verify this
unsafe impl<T: Sync + Send> Sync for NSMutableArray<T, Shared> {}
unsafe impl<T: Sync + Send> Send for NSMutableArray<T, Shared> {}
unsafe impl<T: Sync> Sync for NSMutableArray<T, Owned> {}
unsafe impl<T: Send> Send for NSMutableArray<T, Owned> {}

unsafe fn from_refs<T: Message + ?Sized>(cls: &Class, refs: &[&T]) -> NonNull<Object> {
let obj: *mut Object = unsafe { msg_send![cls, alloc] };
let obj: *mut Object = unsafe {
Expand Down
8 changes: 7 additions & 1 deletion objc2/src/rc/id.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use core::fmt;
use core::marker::PhantomData;
use core::mem::ManuallyDrop;
use core::ops::{Deref, DerefMut};
use core::panic::{RefUnwindSafe, UnwindSafe};
use core::ptr::NonNull;
use std::panic::{RefUnwindSafe, UnwindSafe};

use super::AutoreleasePool;
use super::{Owned, Ownership, Shared};
Expand Down Expand Up @@ -117,6 +117,11 @@ pub struct Id<T: ?Sized, O: Ownership> {
item: PhantomData<T>,
/// To prevent warnings about unused type parameters.
own: PhantomData<O>,
/// Marks the type as !UnwindSafe. Later on we'll re-enable this.
///
/// See <https://github.com/rust-lang/rust/issues/93367> for why this is
/// required.
notunwindsafe: PhantomData<&'static mut ()>,
}

impl<T: Message + ?Sized, O: Ownership> Id<T, O> {
Expand Down Expand Up @@ -171,6 +176,7 @@ impl<T: Message + ?Sized, O: Ownership> Id<T, O> {
ptr,
item: PhantomData,
own: PhantomData,
notunwindsafe: PhantomData,
}
}

Expand Down
62 changes: 12 additions & 50 deletions tests/ui/nsarray_bound_not_send_sync.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,30 +1,10 @@
error[E0277]: `UnsafeCell<()>` cannot be shared between threads safely
error[E0277]: `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>` cannot be shared between threads safely
--> ui/nsarray_bound_not_send_sync.rs:9:5
|
9 | needs_sync::<NSArray<Object, Shared>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `UnsafeCell<()>` cannot be shared between threads safely
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>` cannot be shared between threads safely
|
= help: within `objc2::runtime::Object`, the trait `Sync` is not implemented for `UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
= note: required because of the requirements on the impl of `Sync` for `NSArray<objc2::runtime::Object, Shared>`
note: required by a bound in `needs_sync`
--> ui/nsarray_bound_not_send_sync.rs:5:27
|
5 | fn needs_sync<T: ?Sized + Sync>() {}
| ^^^^ required by this bound in `needs_sync`

error[E0277]: `*const UnsafeCell<()>` cannot be shared between threads safely
--> ui/nsarray_bound_not_send_sync.rs:9:5
|
9 | needs_sync::<NSArray<Object, Shared>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `*const UnsafeCell<()>` cannot be shared between threads safely
|
= help: within `objc2::runtime::Object`, the trait `Sync` is not implemented for `*const UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= help: within `objc2::runtime::Object`, the trait `Sync` is not implemented for `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
= note: required because of the requirements on the impl of `Sync` for `NSArray<objc2::runtime::Object, Shared>`
Expand All @@ -41,8 +21,9 @@ error[E0277]: `*const UnsafeCell<()>` cannot be sent between threads safely
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `*const UnsafeCell<()>` cannot be sent between threads safely
|
= help: within `objc2::runtime::Object`, the trait `Send` is not implemented for `*const UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `(*const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(*const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
= note: required because of the requirements on the impl of `Sync` for `NSArray<objc2::runtime::Object, Shared>`
Expand All @@ -52,33 +33,13 @@ note: required by a bound in `needs_sync`
5 | fn needs_sync<T: ?Sized + Sync>() {}
| ^^^^ required by this bound in `needs_sync`

error[E0277]: `UnsafeCell<()>` cannot be shared between threads safely
--> ui/nsarray_bound_not_send_sync.rs:10:5
|
10 | needs_send::<NSArray<Object, Shared>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `UnsafeCell<()>` cannot be shared between threads safely
|
= help: within `objc2::runtime::Object`, the trait `Sync` is not implemented for `UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
= note: required because of the requirements on the impl of `Send` for `NSArray<objc2::runtime::Object, Shared>`
note: required by a bound in `needs_send`
--> ui/nsarray_bound_not_send_sync.rs:6:27
|
6 | fn needs_send<T: ?Sized + Send>() {}
| ^^^^ required by this bound in `needs_send`

error[E0277]: `*const UnsafeCell<()>` cannot be shared between threads safely
error[E0277]: `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>` cannot be shared between threads safely
--> ui/nsarray_bound_not_send_sync.rs:10:5
|
10 | needs_send::<NSArray<Object, Shared>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `*const UnsafeCell<()>` cannot be shared between threads safely
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>` cannot be shared between threads safely
|
= help: within `objc2::runtime::Object`, the trait `Sync` is not implemented for `*const UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= help: within `objc2::runtime::Object`, the trait `Sync` is not implemented for `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
= note: required because of the requirements on the impl of `Send` for `NSArray<objc2::runtime::Object, Shared>`
Expand All @@ -95,8 +56,9 @@ error[E0277]: `*const UnsafeCell<()>` cannot be sent between threads safely
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `*const UnsafeCell<()>` cannot be sent between threads safely
|
= help: within `objc2::runtime::Object`, the trait `Send` is not implemented for `*const UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `(*const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(*const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
= note: required because of the requirements on the impl of `Send` for `NSArray<objc2::runtime::Object, Shared>`
Expand Down
61 changes: 12 additions & 49 deletions tests/ui/object_not_send_sync.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,29 +1,10 @@
error[E0277]: `UnsafeCell<()>` cannot be shared between threads safely
error[E0277]: `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>` cannot be shared between threads safely
--> ui/object_not_send_sync.rs:11:5
|
11 | needs_sync::<Object>();
| ^^^^^^^^^^^^^^^^^^^^ `UnsafeCell<()>` cannot be shared between threads safely
| ^^^^^^^^^^^^^^^^^^^^ `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>` cannot be shared between threads safely
|
= help: within `objc2::runtime::Object`, the trait `Sync` is not implemented for `UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
note: required by a bound in `needs_sync`
--> ui/object_not_send_sync.rs:7:27
|
7 | fn needs_sync<T: ?Sized + Sync>() {}
| ^^^^ required by this bound in `needs_sync`

error[E0277]: `*const UnsafeCell<()>` cannot be shared between threads safely
--> ui/object_not_send_sync.rs:11:5
|
11 | needs_sync::<Object>();
| ^^^^^^^^^^^^^^^^^^^^ `*const UnsafeCell<()>` cannot be shared between threads safely
|
= help: within `objc2::runtime::Object`, the trait `Sync` is not implemented for `*const UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= help: within `objc2::runtime::Object`, the trait `Sync` is not implemented for `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
note: required by a bound in `needs_sync`
Expand All @@ -39,8 +20,9 @@ error[E0277]: `*const UnsafeCell<()>` cannot be sent between threads safely
| ^^^^^^^^^^^^^^^^^^^^ `*const UnsafeCell<()>` cannot be sent between threads safely
|
= help: within `objc2::runtime::Object`, the trait `Send` is not implemented for `*const UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `(*const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(*const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
note: required by a bound in `needs_send`
Expand All @@ -49,33 +31,13 @@ note: required by a bound in `needs_send`
8 | fn needs_send<T: ?Sized + Send>() {}
| ^^^^ required by this bound in `needs_send`

error[E0277]: `UnsafeCell<()>` cannot be shared between threads safely
--> ui/object_not_send_sync.rs:13:5
|
13 | needs_sync::<NSObject>();
| ^^^^^^^^^^^^^^^^^^^^^^ `UnsafeCell<()>` cannot be shared between threads safely
|
= help: within `NSObject`, the trait `Sync` is not implemented for `UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
= note: required because it appears within the type `NSObject`
note: required by a bound in `needs_sync`
--> ui/object_not_send_sync.rs:7:27
|
7 | fn needs_sync<T: ?Sized + Sync>() {}
| ^^^^ required by this bound in `needs_sync`

error[E0277]: `*const UnsafeCell<()>` cannot be shared between threads safely
error[E0277]: `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>` cannot be shared between threads safely
--> ui/object_not_send_sync.rs:13:5
|
13 | needs_sync::<NSObject>();
| ^^^^^^^^^^^^^^^^^^^^^^ `*const UnsafeCell<()>` cannot be shared between threads safely
| ^^^^^^^^^^^^^^^^^^^^^^ `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>` cannot be shared between threads safely
|
= help: within `NSObject`, the trait `Sync` is not implemented for `*const UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= help: within `NSObject`, the trait `Sync` is not implemented for `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
= note: required because it appears within the type `NSObject`
Expand All @@ -92,8 +54,9 @@ error[E0277]: `*const UnsafeCell<()>` cannot be sent between threads safely
| ^^^^^^^^^^^^^^^^^^^^^^ `*const UnsafeCell<()>` cannot be sent between threads safely
|
= help: within `NSObject`, the trait `Send` is not implemented for `*const UnsafeCell<()>`
= note: required because it appears within the type `(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `(*const UnsafeCell<()>, PhantomPinned)`
= note: required because it appears within the type `PhantomData<(*const UnsafeCell<()>, PhantomPinned)>`
= note: required because it appears within the type `UnsafeCell<PhantomData<(*const UnsafeCell<()>, PhantomPinned)>>`
= note: required because it appears within the type `objc_object`
= note: required because it appears within the type `objc2::runtime::Object`
= note: required because it appears within the type `NSObject`
Expand Down

0 comments on commit a16156c

Please sign in to comment.