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

Avoid Box invalidation to be compatible with tag-raw-pointers #1166

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

saethlin
Copy link
Contributor

It looks like the only outstanding issue aliasing issue in this repo
(with raw pointer tagging) was this one move of a Box after saving a
pointer to the allocation that the Box guards. This is UB according to
the Stacked Borrows with raw pointer tagging in combination with the way
rustc applies noalias to Box. As is often the case, the resolution here
is to convert the Box down to a raw pointer before a pointer into the
allocation is created.

It is unclear to me if the spec will change and make this code valid without this PR. In rust-lang/unsafe-code-guidelines#326 I've advocated for removing this special property from Box altogether. But by the same token, it's entirely possible that this code is UB today because it does aliased access through a noalias pointer. I don't know if the access to entries.len() is problematic aliasing with the group_up pointer.

It looks like the only outstanding issue aliasing issue in this repo
(with raw pointer tagging) was this one move of a Box after saving a
pointer to the allocation that the Box guards. This is UB according to
the Stacked Borrows with raw pointer tagging in combination with the way
rustc applies noalias to Box. As is often the case, the resolution here
is to convert the Box down to a raw pointer before a pointer into the
allocation is created.
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

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