From 2f04a793ae0b9331f6d853a971a670c8ea99b0b7 Mon Sep 17 00:00:00 2001 From: Waffle Date: Sat, 27 Feb 2021 00:20:46 +0300 Subject: [PATCH 1/4] Revert `Vec::spare_capacity_mut` impl to prevent pointers invalidation --- library/alloc/src/vec/mod.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index b1b2619428366..e5545d2a77ff7 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1879,7 +1879,15 @@ impl Vec { #[unstable(feature = "vec_spare_capacity", issue = "75017")] #[inline] pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit] { - 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, + self.buf.capacity() - self.len, + ) + } } /// Returns vector content as a slice of `T`, along with the remaining spare From 9c4e3af39d5e93b2976afa8062ee5c705ba589c5 Mon Sep 17 00:00:00 2001 From: Waffle Date: Wed, 3 Mar 2021 01:00:59 +0300 Subject: [PATCH 2/4] Add test that Vec::spare_capacity_mut doesn't invalidate pointers --- library/alloc/tests/lib.rs | 1 + library/alloc/tests/vec.rs | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index dd98f806451d8..d40496bfaaba7 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -21,6 +21,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}; diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 2969da58d4268..fab450285854d 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -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); From a1835bcb01fc345653fd9e9ce02b0d103032c58d Mon Sep 17 00:00:00 2001 From: Waffle Date: Wed, 3 Mar 2021 01:04:20 +0300 Subject: [PATCH 3/4] Make Vec::split_at_spare_mut impl safer & simplier --- library/alloc/src/vec/mod.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index e5545d2a77ff7..ff401b33832ce 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1944,20 +1944,16 @@ impl Vec { #[unstable(feature = "vec_split_at_spare", issue = "81944")] #[inline] pub fn split_at_spare_mut(&mut self) -> (&mut [T], &mut [MaybeUninit]) { - 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` has the same layout as `T` - let spare_ptr = unsafe { ptr.cast::>().add(self.len) }; + let Range { start: ptr, end: spare_ptr } = self.as_mut_ptr_range(); + let spare_ptr = spare_ptr.cast::>(); + 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` slice 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) } From 950f12119ef724156a9a4e17e1b375eb28c6af11 Mon Sep 17 00:00:00 2001 From: Waffle Lapkin Date: Wed, 3 Mar 2021 20:04:20 +0300 Subject: [PATCH 4/4] Update library/alloc/src/vec/mod.rs Co-authored-by: Ralf Jung --- library/alloc/src/vec/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index ff401b33832ce..4b992d1756bef 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1950,7 +1950,7 @@ impl Vec { // SAFETY: // - `ptr` is guaranteed to be valid for `len` elements - // - `spare_ptr` is pointing one element past the buffer, so it doesn't overlap with `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, spare_len);