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.

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

Stop dereferencing msg_send!'s first argument #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link

@madsmtm madsmtm commented Jun 4, 2021

The issue

In Objective-C, having a null pointer receiver is valid, but with msg_send! it would be converted into a reference, which is undefined behavior - yes, even though the reference is converted into a pointer immediately after, it is still undefined behaviour!

Additionally, there might be a mutable reference somewhere else in the program, which would now be aliased.

Minimal example

let ptr: *mut Object = core::ptr::null_mut();
let _: () = msg_send![ptr, someSelector];

How to fix this

This PR proposes changing msg_send! to no longer automatically dereference the receiver. This is a breaking change, but neither e.g. cocoa nor winit is affected by it - they don't use smart pointers like objc_id::Id.

objc-foundation will require a few tweaks (it uses objc_id::Id), and so will probably also some other crates, but I would say this is acceptable, the fix is very simple, see SSheldon/rust-objc-foundation#14 (and it causes a compile error, not just silent UB if you don't fix it).

Also, I think with unsafe things like msg_send! it's always better to be explicit.

In Objective-C, having a null pointer receiver is valid, but with `msg_send!` it would be converted into a reference, which is undefined behavior.

Additionally, there might be a mutable reference somewhere else in the program, which would now be aliased.

Note: The reference is converted into a pointer immediately after, but it is still UB.
madsmtm added a commit to madsmtm/rust-objc-foundation that referenced this pull request Jun 4, 2021
@SSheldon
Copy link
Owner

SSheldon commented Aug 4, 2021

I agree this isn't great. The ergonomics of using smart pointers really suffer without it, though. I was hoping I could use specialization to implement a special trait for all messageable pointer types. I haven't kept up with specialization to see if that's possible yet. I agree we should do something about this unsoundness in the next objc version with a breaking change.

@madsmtm
Copy link
Author

madsmtm commented Aug 6, 2021

I don't think the ergonomics suffer that bad, in my view this is actually better since makes it clear that msg_send isn't consuming the Id, but simply using the pointer from it.

But maybe there's a way to add something like unsafe fn as_object_pointer(&self) -> *mut Object; on Message, and then change Message to be implemented on *const Object, &Object and such (like Encode in objc-encode) instead of on Object directly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants