Description
Miri recently started checking the dereferenceable
attribute of &mut !Unpin
references by doing "fake reads" when such functions start executing. Interestingly, that makes this safe piece of code fail.
This makes sense, I think, for the same reason that we cannot do "fake reads" of &!Freeze
references: when doing such reads, we might invalidate other aliasing references. In the case of this future, what happens is that
- a reference to the field where
Delay::new(1)
is stored gets created - next time
poll
gets called, we do a "fake read" of the entire future, which invalidates the previously created reference (a noalias reference doesn't like other pointers being used for reads, that's kind of the point) - then we access the reference, causing UB
What I do not fully understand yet is why something like this is not sufficient to cause the issue.
How could we solve this? I am honestly not entirely sure, but see two avenues worth pursuing:
- Figure out whether these "fake reads" are really needed to model
dereferenceable
. I think they are, because otherwise I don't think we get the right interaction betweennoalias
anddereferenceable
, but I asked the LLVM folks about this. - Remove
dereferenceable
from&mut !Unpin
references. This is obviously correct but codegen/optimization people will probably be unhappy...
The latter point might be what we have to do, and somewhat mirrors the fact that we remove dereferenceable
from &!Freeze
references, but not really -- for shared references we thought dereferenceable_on_entry
would still be a sound attribute to add, but with this problem it looks like that might not be the case (neither for &mut !Unpin
nor for &!Freeze
), at least if the "fake read" model of dereferenceable
prevails.
The problematic code condenses to something like this:
struct NotUnpinType {
delay: usize,
delay_ref: &mut usize, // points to `delay`
}
fn poll(self: &mut NotUnpinType) {
let fake_read = *self;
self.delay_ref -= 1; // self.delay_ref is a noalias reference
// so we should be able to reorder the last two lines, but that changes the value stored in fake_read.
}