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

Eliminate mut reference UB in Drop impl for Rc<T> #76530

Merged
merged 4 commits into from
Sep 12, 2020

Conversation

carbotaniuman
Copy link
Contributor

@carbotaniuman carbotaniuman commented Sep 9, 2020

This changes self.ptr.as_mut() with get_mut_unchecked which
does not use an intermediate reference. Arc already handled this
case properly.

Fixes #76509

This changes `self.ptr.as_mut()` with `get_mut_unchecked` which
does not use an intermediate reference.  Arc<T> already handled this
case properly.
@rust-highfive
Copy link
Collaborator

r? @sfackler

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 9, 2020
@carbotaniuman
Copy link
Contributor Author

carbotaniuman commented Sep 9, 2020

Hmm still fails Miri - it looks like I need to change a lot more code.

This avoids overlapping a reference covering the data field,
which may be changed due in concurrent conditions. This fully
fixed the UB mainfested with `new_cyclic`.
library/alloc/src/rc.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

r? @RalfJung

@rust-highfive rust-highfive assigned RalfJung and unassigned sfackler Sep 11, 2020
@RalfJung
Copy link
Member

Thanks a lot, this is great. :) I just have some minor nits.

library/alloc/src/rc.rs Outdated Show resolved Hide resolved
@RalfJung RalfJung added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 11, 2020
@RalfJung
Copy link
Member

Looking good, thanks again. :)
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 11, 2020

📌 Commit b729368 has been approved by RalfJung

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 11, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 12, 2020
Rollup of 7 pull requests

Successful merges:

 - rust-lang#76114 (Add saturating methods for `Duration`)
 - rust-lang#76297 (rustdoc: fix min_const_generics with ty::Param)
 - rust-lang#76484 (Add MaybeUninit::assume_init_drop.)
 - rust-lang#76530 (Eliminate mut reference UB in Drop impl for Rc<T>)
 - rust-lang#76583 (Update `std::os` module documentation.)
 - rust-lang#76599 (Finish off revisions for const generics UI tests.)
 - rust-lang#76615 (Add missing examples on binary core traits)

Failed merges:

r? `@ghost`
@bors bors merged commit a49451c into rust-lang:master Sep 12, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 12, 2020
@carbotaniuman carbotaniuman deleted the fix-rc branch April 20, 2022 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"test_rc_cyclic_with_one_ref" fails in Miri
6 participants