avoid mem-forget-in-disguise #458
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Darksonn
approved these changes
Dec 30, 2020
src/bytes.rs
Outdated
| let ptr = slice.as_ptr(); | ||
| drop(Box::into_raw(slice)); |
There was a problem hiding this comment.
How does this compare to just doing this?
let ptr = Box::into_raw(slice) as *const _;There was a problem hiding this comment.
Where the as here is doing the wide-to-thin ptr conversion?
Yeah something like that should also work. The important part is not using slice any more after ptr got created.
There was a problem hiding this comment.
I'm mainly worried about the as_ptr creating the pointer through a &self instead of &mut self, but that may not be a problem since this is for Bytes where we can't modify it anyway.
There was a problem hiding this comment.
That's fair. I updated my patch.
RalfJung
changed the title
Merged
taiki-e
added a commit
to rust-lang/futures-rs
that referenced
this pull request
Jan 12, 2022
This fixes stacked borrows violation violations. See tokio-rs/bytes#458 for more.
taiki-e
added a commit
to rust-lang/futures-rs
that referenced
this pull request
Jan 12, 2022
This fixes stacked borrows violation violations. See tokio-rs/bytes#458 for more.
taiki-e
added a commit
to rust-lang/futures-rs
that referenced
this pull request
Feb 6, 2022
This fixes stacked borrows violation violations. See tokio-rs/bytes#458 for more.
taiki-e
added a commit
to rust-lang/futures-rs
that referenced
this pull request
Feb 6, 2022
This fixes stacked borrows violation violations. See tokio-rs/bytes#458 for more.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Box::into_raw(slice)(which is used just as a way to encodemem::forgethere, from what I can tell) uses theBoxand thus asserts its uniqueness as a pointer, which is not compatible with what the code here does. That's why the docs recommendManuallyDropovermem::forget. This adjusts the code accordingly.I found this with Miri using the
-Zmiri-track-raw-pointersflag. Unfortunately, the test suite cannot pass with that flag sincebytesuses int-to-ptr casts, which are incompatible with-Zmiri-track-raw-pointers. But this fix seems worth landing nevertheless.