Skip to content

Commit

Permalink
Auto merge of #56161 - RalfJung:vecdeque-stacked-borrows, r=SimonSapin
Browse files Browse the repository at this point in the history
VecDeque: fix for stacked borrows

`VecDeque` violates a version of stacked borrows where creating a shared reference is not enough to make a location *mutably accessible* from raw pointers (and I think that is the version we want).  There are two problems:

* Creating a `NonNull<T>` from `&mut T` goes through `&T` (inferred for a `_`), then `*const T`, then `NonNull<T>`. That means in this stricter version of Stacked Borrows, we cannot actually write to such a `NonNull` because it was created from a shared reference! This PR fixes that by going from `&mut T` to `*mut T` to `*const T`.
* `VecDeque::drain` creates the `Drain` struct by *first* creating a `NonNull` from `self` (which is an `&mut VecDeque`), and *then* calling `self.buffer_as_mut_slice()`. The latter reborrows `self`, asserting that `self` is currently the unique pointer to access this `VecDeque`, and hence invalidating the `NonNull` that was created earlier. This PR fixes that by instead using `self.buffer_as_slice()`, which only performs read accesses and creates only shared references, meaning the raw pointer (`NonNull`) remains valid.

It is possible that other methods on `VecDeque` do something similar, miri's test coverage of `VecDeque` is sparse to say the least.

Cc @nikomatsakis @gankro
  • Loading branch information
bors committed Dec 13, 2018
2 parents ced7cc5 + feb775c commit 9fe5cb5
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
5 changes: 4 additions & 1 deletion src/liballoc/collections/vec_deque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1026,7 +1026,10 @@ impl<T> VecDeque<T> {
iter: Iter {
tail: drain_tail,
head: drain_head,
ring: unsafe { self.buffer_as_mut_slice() },
// Crucially, we only create shared references from `self` here and read from
// it. We do not write to `self` nor reborrow to a mutable reference.
// Hence the raw pointer we created above, for `deque`, remains valid.
ring: unsafe { self.buffer_as_slice() },
},
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/libcore/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2848,14 +2848,14 @@ impl<T: ?Sized> fmt::Pointer for Unique<T> {
#[unstable(feature = "ptr_internals", issue = "0")]
impl<'a, T: ?Sized> From<&'a mut T> for Unique<T> {
fn from(reference: &'a mut T) -> Self {
Unique { pointer: unsafe { NonZero(reference as _) }, _marker: PhantomData }
Unique { pointer: unsafe { NonZero(reference as *mut T) }, _marker: PhantomData }
}
}

#[unstable(feature = "ptr_internals", issue = "0")]
impl<'a, T: ?Sized> From<&'a T> for Unique<T> {
fn from(reference: &'a T) -> Self {
Unique { pointer: unsafe { NonZero(reference as _) }, _marker: PhantomData }
Unique { pointer: unsafe { NonZero(reference as *const T) }, _marker: PhantomData }
}
}

Expand Down Expand Up @@ -3058,14 +3058,14 @@ impl<T: ?Sized> From<Unique<T>> for NonNull<T> {
impl<'a, T: ?Sized> From<&'a mut T> for NonNull<T> {
#[inline]
fn from(reference: &'a mut T) -> Self {
NonNull { pointer: unsafe { NonZero(reference as _) } }
NonNull { pointer: unsafe { NonZero(reference as *mut T) } }
}
}

#[stable(feature = "nonnull", since = "1.25.0")]
impl<'a, T: ?Sized> From<&'a T> for NonNull<T> {
#[inline]
fn from(reference: &'a T) -> Self {
NonNull { pointer: unsafe { NonZero(reference as _) } }
NonNull { pointer: unsafe { NonZero(reference as *const T) } }
}
}

0 comments on commit 9fe5cb5

Please sign in to comment.