Stream: t-lang/wg-unsafe-code-guidelines

Topic: stacked borrows violation in VecDeque


RalfJ (Oct 30 2018 at 15:35, on Zulip):

The one remaining stacked borrows violation in the test suite is rather interesting. It appears when using VecDeque::drain, but the actual problem is NonNull::from(&mut T). That looks as follows:

impl<'a, T: ?Sized> From<&'a mut T> for NonNull<T> {
    fn from(reference: &'a mut T) -> Self {
        NonNull { pointer: NonZero(reference as _) }
    }
}

Looks innocent, doesn't it? What actually happens in MIR, however, is...

_6 = &(*_1)
_5 = move _6 as *const T (Misc)
...

IOW, we turn the mutable ref into a shared one, and then turn that into a raw (const) ptr and go from there. WAY later, we end up writing through this reference, and my model says "nope, you cannot write through this because you got it from a shared ref".

RalfJ (Oct 30 2018 at 15:36, on Zulip):

Funny enough, the distinction between *mut and *const -- which was meant to help -- actually turns out to be harmful here.

nikomatsakis (Oct 30 2018 at 17:17, on Zulip):

:)

nikomatsakis (Oct 30 2018 at 17:18, on Zulip):

I suppose there is no particular reason for the cast to work that way

RalfJ (Oct 30 2018 at 17:21, on Zulip):

true

RalfJ (Oct 30 2018 at 17:21, on Zulip):

but it also shows the dangers of making implicit operations like coercions carry semantic meaning

RalfJ (Oct 30 2018 at 17:21, on Zulip):

yet, I am fairly sure we want the mut-to-shared-ref cast to have semantic meaning

RalfJ (Oct 30 2018 at 17:22, on Zulip):

we should probably "just" change inference to avoid introducing such casts unless it absolutely has to...

RalfJ (Oct 30 2018 at 17:28, on Zulip):

hm, changing just that cast is not enough to make the test pass... seems like I will have to investigate some more

nikomatsakis (Oct 30 2018 at 17:34, on Zulip):

but it also shows the dangers of making implicit operations like coercions carry semantic meaning

confirm

RalfJ (Oct 30 2018 at 17:51, on Zulip):

uh yeah I see what is going on... after creating the raw ptr, we call self.buffer_as_mut_slice()

RalfJ (Oct 30 2018 at 17:51, on Zulip):

so we use self again

RalfJ (Oct 30 2018 at 17:51, on Zulip):

which kills the reborrows, including the raw one

RalfJ (Oct 30 2018 at 18:33, on Zulip):

and this is not easy to fix, either... buffer_as_mut_slice returns a reference so we cannot create the NonNull after creating the slice, either. hm.

RalfJ (Nov 05 2018 at 15:06, on Zulip):

I re-did a large chunk of the model, though the core stayed the same, fixing "partially interior mutable data" at https://github.com/RalfJung/miri/tree/new-interior-mut. Interestingly that also fixed VecDeque. I still have to understand how and why.

RalfJ (Nov 07 2018 at 18:44, on Zulip):

This now gets allowed because when we turn &mut to &, we first push a Shr ("shared" -> accessible by raw ptrs) and then freeze

RalfJ (Nov 07 2018 at 18:45, on Zulip):

then NonNull::from creates a raw ptr, and when we use that the freezing is dropped but the raw ptr remains usable

RalfJ (Nov 07 2018 at 18:45, on Zulip):

I didn't intend to it this way but I have tos ay this seems to work quite well so I am inclined to leave it this way :D

Nicole Mazzuca (Nov 07 2018 at 19:07, on Zulip):

huh, cool

Last update: Nov 19 2019 at 19:00UTC