Skip to content

Soundness fixes#128

Merged
madsmtm merged 2 commits into
masterfrom
soundness-fixes
Feb 28, 2022
Merged

Soundness fixes#128
madsmtm merged 2 commits into
masterfrom
soundness-fixes

Conversation

@madsmtm
Copy link
Copy Markdown
Owner

@madsmtm madsmtm commented Feb 22, 2022

@madsmtm madsmtm added the bug Something isn't working label Feb 22, 2022
@madsmtm
Copy link
Copy Markdown
Owner Author

madsmtm commented Feb 24, 2022

Didn't really realize this before, but making type OpaqueData = UnsafeCell<...> actually means that Id<T, Shared> is much safer!

Having aliasing Id<T, Owned> and Id<T, Shared> is ofc. still UB, but if you only use Id<T, Shared> and don't implement Send and/or Sync on your type then Objective-C methods are free to mutate the object as much as they want!

(Concretely, this removes a few noalias and readonly attributes from the LLVM IR of e.g. &Object and &Class, just like using an extern type for those would)

@madsmtm
Copy link
Copy Markdown
Owner Author

madsmtm commented Feb 24, 2022

To consider: Does it make sense for Id to store an UnsafeCell? How about variance?

EDIT: Nope, it's correct as-is.

@madsmtm
Copy link
Copy Markdown
Owner Author

madsmtm commented Feb 24, 2022

Also, the pointer in Id<T, Shared> may actually have been mutated by Id::clone, so making Object = UnsafeCell is definitely better!

@madsmtm madsmtm marked this pull request as ready for review February 28, 2022 14:58
@madsmtm madsmtm force-pushed the soundness-fixes branch 2 times, most recently from 41a846b to b987506 Compare February 28, 2022 20:34
@madsmtm madsmtm merged commit a16156c into master Feb 28, 2022
@madsmtm madsmtm deleted the soundness-fixes branch February 28, 2022 23:57
@madsmtm madsmtm added this to the objc2 v0.3 milestone Apr 2, 2022
@madsmtm madsmtm added the I-unsound A soundness hole, or affecting soundness label Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working I-unsound A soundness hole, or affecting soundness

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant