Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Soundness fixes #128

Merged
merged 2 commits into from
Feb 28, 2022
Merged

Soundness fixes #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0211c2
Fix UnwindSafe/Send/Sync impls for upcoming changes to the compiler
madsmtm Feb 28, 2022
b987506
Properly mark objects as mutable behind shared references
madsmtm Feb 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion block-sys/src/lib.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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