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

Stacked Borrows: don't read from memory during retagging #931

Merged
merged 2 commits into from Aug 29, 2019

Conversation

@RalfJung
Copy link
Member

commented Aug 28, 2019

Currently, retagging of a shared reference tries real hard to find the UnsafeCell and mark everything else as frozen. We even read enum discriminants to figure out the variant and determine if there is an UnsafeCell in there or not.

Unfortunately, that leads to some very hard to analyze behavior: during retagging, we do read accesses, which are subject to the rules of Stacked Borrows and the existing tags! My head hurts when I try to think about this. It's just too recursive.

This PR simplifies the semantics by treating enums like unions: if any variant has an UnsafeCell, the entire thing behaves like an UnsafeCell. This means retagging no longer has to read from memory, the way it affects the stack is entirely determined by the type.

@RalfJung

This comment has been minimized.

Copy link
Member Author

commented Aug 29, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2019

📌 Commit f3ff100 has been approved by RalfJung

bors added a commit that referenced this pull request Aug 29, 2019
Auto merge of #931 - RalfJung:retag-makes-my-head-hurt, r=RalfJung
Stacked Borrows: don't read from memory during retagging

Currently, retagging of a shared reference tries real hard to find the `UnsafeCell` and mark everything else as frozen. We even read enum discriminants to figure out the variant and determine if there is an `UnsafeCell` in there or not.

Unfortunately, that leads to some very hard to analyze behavior: during retagging, we do read accesses, which are subject to the rules of Stacked Borrows and the existing tags! My head hurts when I try to think about this. It's just too recursive.

This PR simplifies the semantics by treating enums like unions: if any variant has an `UnsafeCell`, the entire thing behaves like an `UnsafeCell`. This means retagging no longer has to read from memory, the way it affects the stack is entirely determined by the type.
@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2019

⌛️ Testing commit f3ff100 with merge e3b87f6...

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing e3b87f6 to master...

@bors bors merged commit f3ff100 into rust-lang:master Aug 29, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details

@RalfJung RalfJung deleted the RalfJung:retag-makes-my-head-hurt branch Aug 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.