-
Notifications
You must be signed in to change notification settings - Fork 9
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
Illegal aliasing of mutable references #8
Comments
Oh wow, I've been unable to use miri with proptest for the longest time, I'm so happy it apparently works now. |
I spoke too soon. You've just run into a new failing test that runs before the proptest tests run. Miri still remains unable to cope with proptest. 😞 |
I can look into what it takes to make proptest work. Miri is slow though, so fuzzing doesn't work very well. |
Proptest on miri isn't a huge priority for me, tbh, I'd rather get rid of proptest altogether and come up with a more stable solution. It's just a bit of a work in progress. |
Anyway, I fixed the issue, your suggestion was pretty much spot on. Only difference in the impl is that I went with just a straightforward |
Thanks! It's also a good motivator for me to implement enough shims to get proptest to work. ;) |
To make sure that there are no more issues like #3 in this crate, I decided to run its test suite in Miri. Unfortunately, that fails:
Miri backtrace
My interpretation of what happens is that the problem is here:
sized-chunks/src/ring_buffer/iter.rs
Line 84 in d71d0ea
This creates a fresh
&mut RingBuffer
that is passed toRingBuffer::mut_ptr
. Any time a mutable reference is created it must be the unique pointer pointing to that memory -- all old aliases become invalid. We are still experimenting with the exact ruels for this; for further details, see this document.The issue here is that this new mutably reference that got created here overaps with the mutable references that were handed out by the iterator previously. Thus those old references become invalid (their "lifetime ends"), but the caller still uses them later, leading to the error. This is a problem because Rust would like to perform optimizations based on the assumption that mutable references are truly unique -- code like this is incompatible with such optimizations.
Given that you are handling aliased pointers here (the
buffer
pointer and the ones returned bynext
overlap) and want to perform mutation, I think the only solution is to use raw pointers throughout. That way, the Rust compiler knows that nothing is unique, and does not do any optimizations. I think you can do that here by replacing thebuffer
field with a raw pointer to theMaybeUninit<N::SizedType>
that lives inside the buffer. Then you never have to create a reference except for the ones returned bynext
, and those do not overlap because they all point to different elements. Does that make sense?Also Cc rust-lang/unsafe-code-guidelines#133: this crate would benefit from mutable references being "less eager" in asserting uniqueness. Unfortunately, it is not clear if that is possible and what the trade-offs are.
The text was updated successfully, but these errors were encountered: