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

std: Use mem::replace in TLS initialization #30267

Merged
merged 1 commit into from
Dec 10, 2015

Conversation

alexcrichton
Copy link
Member

Due to #30228 it's not currently sound to do *ptr = Some(value), so instead
use mem::replace which fixes the soundness hole for now.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

r? @aturon

cc @eddyb

@rust-highfive rust-highfive assigned aturon and unassigned nikomatsakis Dec 8, 2015
@arielb1
Copy link
Contributor

arielb1 commented Dec 8, 2015

While there is indeed a codegen/soundness bug involving panicking destructors, the root cause of the issue here is that you hold a &mut reference while calling a destructor that can access that reference. This problem is indeed fixed by the mem::replace, but you should comment that.

Due to rust-lang#30228 it's not currently sound to do `*ptr = Some(value)`, so instead
use `mem::replace` which fixes the soundness hole for now.
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 10, 2015

📌 Commit 9e0ff77 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 10, 2015

⌛ Testing commit 9e0ff77 with merge ce13275...

bors added a commit that referenced this pull request Dec 10, 2015
Due to #30228 it's not currently sound to do `*ptr = Some(value)`, so instead
use `mem::replace` which fixes the soundness hole for now.
@bors bors merged commit 9e0ff77 into rust-lang:master Dec 10, 2015
@alexcrichton alexcrichton deleted the tls-init-oh-my branch December 10, 2015 23:40
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.

6 participants