Don't drop uninit memory when MapWindows::clone panics#156588
Don't drop uninit memory when MapWindows::clone panics#156588Jules-Bertholet wants to merge 3 commits into
MapWindows::clone panics#156588Conversation
|
The Miri subtree was changed cc @rust-lang/miri |
rustbot
added
S-waiting-on-review
|
r? @scottmcm rustbot has assigned @scottmcm. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| // | ||
| // FIXME make these unsafe fields once that feature is ready |
There was a problem hiding this comment.
| // | |
| // FIXME make these unsafe fields once that feature is ready |
I don't think it's worth adding this FIXME to very single type with an invariant.
Co-authored-by: Ralf Jung <post@ralfj.de>
|
I'm leaving libs reviews. |
|
☔ The latest upstream changes (presumably #156728) made this pull request unmergeable. Please resolve the merge conflicts. |
| // `clone()` could panic; `ManuallyDrop` guards against that. | ||
| // (We could instead just construct `self.as_array_ref().clone()` | ||
| // as a local on the stack before creating the buffer. | ||
| // That would avoid the leak amplification, |
There was a problem hiding this comment.
But this doesn't do leak amplification, right? If the clone panics, all the elements that have already been cloned are dropped – the array clone implementation takes care of that.
|
|
||
| assert!(result.is_err()); | ||
|
|
||
| // current impl does leak amplification |
There was a problem hiding this comment.
I don't think it does? It's just that nothing has been cloned successfully yet, and all the existing elements will be dropped after this point.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
6 participants
Fixes #156501, using the approach suggested in @bjorn3's comment #156517 (comment)