Skip to content

Commit

Permalink
Rollup merge of #82564 - WaffleLapkin:revert_spare_mut, r=RalfJung
Browse files Browse the repository at this point in the history
Revert `Vec::spare_capacity_mut` impl to prevent pointers invalidation

The implementation was changed in #79015.

Later it was [pointed out](#81944 (comment)) that the implementation invalidates pointers to the buffer (initialized elements) by creating a unique reference to the buffer. This PR reverts the implementation.

r? `@RalfJung`
  • Loading branch information
Dylan-DPC authored Mar 4, 2021
2 parents ae2482e + 950f121 commit b0b63ec
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
24 changes: 14 additions & 10 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1877,7 +1877,15 @@ impl<T, A: Allocator> Vec<T, A> {
#[unstable(feature = "vec_spare_capacity", issue = "75017")]
#[inline]
pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] {
self.split_at_spare_mut().1
// Note:
// This method is not implemented in terms of `split_at_spare_mut`,
// to prevent invalidation of pointers to the buffer.
unsafe {
slice::from_raw_parts_mut(
self.as_mut_ptr().add(self.len) as *mut MaybeUninit<T>,
self.buf.capacity() - self.len,
)
}
}

/// Returns vector content as a slice of `T`, along with the remaining spare
Expand Down Expand Up @@ -1934,20 +1942,16 @@ impl<T, A: Allocator> Vec<T, A> {
#[unstable(feature = "vec_split_at_spare", issue = "81944")]
#[inline]
pub fn split_at_spare_mut(&mut self) -> (&mut [T], &mut [MaybeUninit<T>]) {
let ptr = self.as_mut_ptr();

// SAFETY:
// - `ptr` is guaranteed to be in bounds for `capacity` elements
// - `len` is guaranteed to less or equal to `capacity`
// - `MaybeUninit<T>` has the same layout as `T`
let spare_ptr = unsafe { ptr.cast::<MaybeUninit<T>>().add(self.len) };
let Range { start: ptr, end: spare_ptr } = self.as_mut_ptr_range();
let spare_ptr = spare_ptr.cast::<MaybeUninit<T>>();
let spare_len = self.buf.capacity() - self.len;

// SAFETY:
// - `ptr` is guaranteed to be valid for `len` elements
// - `spare_ptr` is offseted from `ptr` by `len`, so it doesn't overlap `initialized` slice
// - `spare_ptr` is pointing one element past the buffer, so it doesn't overlap with `initialized`
unsafe {
let initialized = slice::from_raw_parts_mut(ptr, self.len);
let spare = slice::from_raw_parts_mut(spare_ptr, self.buf.capacity() - self.len);
let spare = slice::from_raw_parts_mut(spare_ptr, spare_len);

(initialized, spare)
}
Expand Down
1 change: 1 addition & 0 deletions library/alloc/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#![feature(vecdeque_binary_search)]
#![feature(slice_group_by)]
#![feature(vec_extend_from_within)]
#![feature(vec_spare_capacity)]

use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};
Expand Down
4 changes: 4 additions & 0 deletions library/alloc/tests/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1691,6 +1691,10 @@ fn test_stable_pointers() {
next_then_drop(v.splice(5..6, vec![1; 10].into_iter().filter(|_| true))); // lower bound not exact
assert_eq!(*v0, 13);

// spare_capacity_mut
v.spare_capacity_mut();
assert_eq!(*v0, 13);

// Smoke test that would fire even outside Miri if an actual relocation happened.
*v0 -= 13;
assert_eq!(v[0], 0);
Expand Down

0 comments on commit b0b63ec

Please sign in to comment.