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

Fix HSTRING to conform to Rust's aliasing rules #2057

Merged
merged 1 commit into from Sep 22, 2022

Conversation

ChrisDenton
Copy link
Collaborator

Pointers derived from &mut references must still uphold the uniqueness guarantee. Fortunately the fix is simple here: we just use a shared reference when duplicating HSTRINGs. Casting between pointer types is fine, it's effectively just a lint.

Fixes: #2056

Pointers derived from `&mut` references must still uphold the uniqueness guarantee. Therefore we should be using a shared reference when duplicating `HSTRING`s.
Copy link
Collaborator

@kennykerr kennykerr left a comment

Choose a reason for hiding this comment

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

Thanks!

@kennykerr kennykerr changed the title Fix subtle UB Fix HSTRING to conform to Rust's aliasing rules Sep 22, 2022
@kennykerr kennykerr merged commit b13b328 into microsoft:master Sep 22, 2022
@Muon
Copy link

Muon commented Oct 18, 2022

This is wrong. You can have as many *mut T pointing to the same T as you like, you just can't have multiple &mut T at once (otherwise this entirely safe example should not compile). Furthermore, this PR potentially introduces UB, since you cannot legally mutate through a *mut T that was obtained from an &T. Here's a playground example you can run with MIRI to see that it picks up UB: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=11397c0ce7a6705b02c7d78518d90383

@ChrisDenton
Copy link
Collaborator Author

The first safe playground example you linked is safe because the pointers are never used. It's always safe to not use pointers.

Indeed, mutating a pointer obtained through an &T would not be sound. However, in this case it's being used in a HSTRING which is immutable.

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.

hstring::Header::duplicate does not uphold aliasing rules
3 participants