diff options
| author | bors <bors@rust-lang.org> | 2022-03-11 19:23:55 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-03-11 19:23:55 +0000 |
| commit | 335ffbfa547df94ac236f5c56130cecf99c8d82b (patch) | |
| tree | 01a88b422c236a89c46590a3a3eadec1935560a3 | |
| parent | c9b45e601065c3fb71a4f67481e912391d075621 (diff) | |
| parent | 2f18fa801b783fe9b7c896af40f27d1b5be8da9a (diff) | |
| download | rust-335ffbfa547df94ac236f5c56130cecf99c8d82b.tar.gz rust-335ffbfa547df94ac236f5c56130cecf99c8d82b.zip | |
Auto merge of #94472 - JmPotato:use_maybeuninit_for_vecdeque, r=m-ou-se
Use MaybeUninit in VecDeque to remove the undefined behavior of slice Signed-off-by: JmPotato <ghzpotato@gmail.com> Ref https://github.com/rust-lang/rust/issues/74189. Adjust the code to follow the [doc.rust-lang.org/reference/behavior-considered-undefined.html](https://doc.rust-lang.org/reference/behavior-considered-undefined.html). * Change the return type of `buffer_as_slice` from `&[T]` to `&[MaybeUninit<T>]`. * Add some corresponding safety comments. Benchmark results: master 8d6f527530f4ba974d922269267fe89050188789 ```rust test collections::vec_deque::tests::bench_pop_back_100 ... bench: 47 ns/iter (+/- 1) test collections::vec_deque::tests::bench_pop_front_100 ... bench: 50 ns/iter (+/- 4) test collections::vec_deque::tests::bench_push_back_100 ... bench: 69 ns/iter (+/- 10) test collections::vec_deque::tests::bench_push_front_100 ... bench: 72 ns/iter (+/- 6) test collections::vec_deque::tests::bench_retain_half_10000 ... bench: 145,891 ns/iter (+/- 7,975) test collections::vec_deque::tests::bench_retain_odd_10000 ... bench: 141,647 ns/iter (+/- 3,711) test collections::vec_deque::tests::bench_retain_whole_10000 ... bench: 120,132 ns/iter (+/- 4,078) ``` This PR ```rust test collections::vec_deque::tests::bench_pop_back_100 ... bench: 48 ns/iter (+/- 2) test collections::vec_deque::tests::bench_pop_front_100 ... bench: 51 ns/iter (+/- 3) test collections::vec_deque::tests::bench_push_back_100 ... bench: 73 ns/iter (+/- 2) test collections::vec_deque::tests::bench_push_front_100 ... bench: 73 ns/iter (+/- 2) test collections::vec_deque::tests::bench_retain_half_10000 ... bench: 131,796 ns/iter (+/- 5,440) test collections::vec_deque::tests::bench_retain_odd_10000 ... bench: 137,563 ns/iter (+/- 3,349) test collections::vec_deque::tests::bench_retain_whole_10000 ... bench: 128,815 ns/iter (+/- 3,289) ```
| -rw-r--r-- | library/alloc/src/collections/vec_deque/iter.rs | 69 | ||||
| -rw-r--r-- | library/alloc/src/collections/vec_deque/mod.rs | 56 |
2 files changed, 95 insertions, 30 deletions
diff --git a/library/alloc/src/collections/vec_deque/iter.rs b/library/alloc/src/collections/vec_deque/iter.rs index edadd666edc..e8290809276 100644 --- a/library/alloc/src/collections/vec_deque/iter.rs +++ b/library/alloc/src/collections/vec_deque/iter.rs @@ -1,5 +1,6 @@ use core::fmt; use core::iter::{FusedIterator, TrustedLen, TrustedRandomAccess, TrustedRandomAccessNoCoerce}; +use core::mem::MaybeUninit; use core::ops::Try; use super::{count, wrap_index, RingSlices}; @@ -12,7 +13,7 @@ use super::{count, wrap_index, RingSlices}; /// [`iter`]: super::VecDeque::iter #[stable(feature = "rust1", since = "1.0.0")] pub struct Iter<'a, T: 'a> { - pub(crate) ring: &'a [T], + pub(crate) ring: &'a [MaybeUninit<T>], pub(crate) tail: usize, pub(crate) head: usize, } @@ -21,7 +22,15 @@ pub struct Iter<'a, T: 'a> { impl<T: fmt::Debug> fmt::Debug for Iter<'_, T> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let (front, back) = RingSlices::ring_slices(self.ring, self.head, self.tail); - f.debug_tuple("Iter").field(&front).field(&back).finish() + // Safety: + // - `self.head` and `self.tail` in a ring buffer are always valid indices. + // - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized. + unsafe { + f.debug_tuple("Iter") + .field(&MaybeUninit::slice_assume_init_ref(front)) + .field(&MaybeUninit::slice_assume_init_ref(back)) + .finish() + } } } @@ -44,7 +53,10 @@ impl<'a, T> Iterator for Iter<'a, T> { } let tail = self.tail; self.tail = wrap_index(self.tail.wrapping_add(1), self.ring.len()); - unsafe { Some(self.ring.get_unchecked(tail)) } + // Safety: + // - `self.tail` in a ring buffer is always a valid index. + // - `self.head` and `self.tail` equality is checked above. + unsafe { Some(self.ring.get_unchecked(tail).assume_init_ref()) } } #[inline] @@ -58,8 +70,13 @@ impl<'a, T> Iterator for Iter<'a, T> { F: FnMut(Acc, Self::Item) -> Acc, { let (front, back) = RingSlices::ring_slices(self.ring, self.head, self.tail); - accum = front.iter().fold(accum, &mut f); - back.iter().fold(accum, &mut f) + // Safety: + // - `self.head` and `self.tail` in a ring buffer are always valid indices. + // - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized. + unsafe { + accum = MaybeUninit::slice_assume_init_ref(front).iter().fold(accum, &mut f); + MaybeUninit::slice_assume_init_ref(back).iter().fold(accum, &mut f) + } } fn try_fold<B, F, R>(&mut self, init: B, mut f: F) -> R @@ -70,17 +87,19 @@ impl<'a, T> Iterator for Iter<'a, T> { { let (mut iter, final_res); if self.tail <= self.head { - // single slice self.ring[self.tail..self.head] - iter = self.ring[self.tail..self.head].iter(); + // Safety: single slice self.ring[self.tail..self.head] is initialized. + iter = unsafe { MaybeUninit::slice_assume_init_ref(&self.ring[self.tail..self.head]) } + .iter(); final_res = iter.try_fold(init, &mut f); } else { - // two slices: self.ring[self.tail..], self.ring[..self.head] + // Safety: two slices: self.ring[self.tail..], self.ring[..self.head] both are initialized. let (front, back) = self.ring.split_at(self.tail); - let mut back_iter = back.iter(); + + let mut back_iter = unsafe { MaybeUninit::slice_assume_init_ref(back).iter() }; let res = back_iter.try_fold(init, &mut f); let len = self.ring.len(); self.tail = (self.ring.len() - back_iter.len()) & (len - 1); - iter = front[..self.head].iter(); + iter = unsafe { MaybeUninit::slice_assume_init_ref(&front[..self.head]).iter() }; final_res = iter.try_fold(res?, &mut f); } self.tail = self.head - iter.len(); @@ -109,7 +128,7 @@ impl<'a, T> Iterator for Iter<'a, T> { // that is in bounds. unsafe { let idx = wrap_index(self.tail.wrapping_add(idx), self.ring.len()); - self.ring.get_unchecked(idx) + self.ring.get_unchecked(idx).assume_init_ref() } } } @@ -122,7 +141,10 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> { return None; } self.head = wrap_index(self.head.wrapping_sub(1), self.ring.len()); - unsafe { Some(self.ring.get_unchecked(self.head)) } + // Safety: + // - `self.head` in a ring buffer is always a valid index. + // - `self.head` and `self.tail` equality is checked above. + unsafe { Some(self.ring.get_unchecked(self.head).assume_init_ref()) } } fn rfold<Acc, F>(self, mut accum: Acc, mut f: F) -> Acc @@ -130,8 +152,13 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> { F: FnMut(Acc, Self::Item) -> Acc, { let (front, back) = RingSlices::ring_slices(self.ring, self.head, self.tail); - accum = back.iter().rfold(accum, &mut f); - front.iter().rfold(accum, &mut f) + // Safety: + // - `self.head` and `self.tail` in a ring buffer are always valid indices. + // - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized. + unsafe { + accum = MaybeUninit::slice_assume_init_ref(back).iter().rfold(accum, &mut f); + MaybeUninit::slice_assume_init_ref(front).iter().rfold(accum, &mut f) + } } fn try_rfold<B, F, R>(&mut self, init: B, mut f: F) -> R @@ -142,16 +169,20 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> { { let (mut iter, final_res); if self.tail <= self.head { - // single slice self.ring[self.tail..self.head] - iter = self.ring[self.tail..self.head].iter(); + // Safety: single slice self.ring[self.tail..self.head] is initialized. + iter = unsafe { + MaybeUninit::slice_assume_init_ref(&self.ring[self.tail..self.head]).iter() + }; final_res = iter.try_rfold(init, &mut f); } else { - // two slices: self.ring[self.tail..], self.ring[..self.head] + // Safety: two slices: self.ring[self.tail..], self.ring[..self.head] both are initialized. let (front, back) = self.ring.split_at(self.tail); - let mut front_iter = front[..self.head].iter(); + + let mut front_iter = + unsafe { MaybeUninit::slice_assume_init_ref(&front[..self.head]).iter() }; let res = front_iter.try_rfold(init, &mut f); self.head = front_iter.len(); - iter = back.iter(); + iter = unsafe { MaybeUninit::slice_assume_init_ref(back).iter() }; final_res = iter.try_rfold(res?, &mut f); } self.head = self.tail + iter.len(); diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/library/alloc/src/collections/vec_deque/mod.rs index c3cabc754e6..63280e56332 100644 --- a/library/alloc/src/collections/vec_deque/mod.rs +++ b/library/alloc/src/collections/vec_deque/mod.rs @@ -12,7 +12,7 @@ use core::fmt; use core::hash::{Hash, Hasher}; use core::iter::{repeat_with, FromIterator}; use core::marker::PhantomData; -use core::mem::{self, ManuallyDrop}; +use core::mem::{self, ManuallyDrop, MaybeUninit}; use core::ops::{Index, IndexMut, Range, RangeBounds}; use core::ptr::{self, NonNull}; use core::slice; @@ -181,16 +181,28 @@ impl<T, A: Allocator> VecDeque<T, A> { } } - /// Turn ptr into a slice + /// Turn ptr into a slice, since the elements of the backing buffer may be uninitialized, + /// we will return a slice of [`MaybeUninit<T>`]. + /// + /// See [`MaybeUninit::zeroed`][zeroed] for examples of correct and + /// incorrect usage of this method. + /// + /// [zeroed]: mem::MaybeUninit::zeroed #[inline] - unsafe fn buffer_as_slice(&self) -> &[T] { - unsafe { slice::from_raw_parts(self.ptr(), self.cap()) } + unsafe fn buffer_as_slice(&self) -> &[MaybeUninit<T>] { + unsafe { slice::from_raw_parts(self.ptr() as *mut MaybeUninit<T>, self.cap()) } } - /// Turn ptr into a mut slice + /// Turn ptr into a mut slice, since the elements of the backing buffer may be uninitialized, + /// we will return a slice of [`MaybeUninit<T>`]. + /// + /// See [`MaybeUninit::zeroed`][zeroed] for examples of correct and + /// incorrect usage of this method. + /// + /// [zeroed]: mem::MaybeUninit::zeroed #[inline] - unsafe fn buffer_as_mut_slice(&mut self) -> &mut [T] { - unsafe { slice::from_raw_parts_mut(self.ptr(), self.cap()) } + unsafe fn buffer_as_mut_slice(&mut self) -> &mut [MaybeUninit<T>] { + unsafe { slice::from_raw_parts_mut(self.ptr() as *mut MaybeUninit<T>, self.cap()) } } /// Moves an element out of the buffer @@ -1055,9 +1067,13 @@ impl<T, A: Allocator> VecDeque<T, A> { #[inline] #[stable(feature = "deque_extras_15", since = "1.5.0")] pub fn as_slices(&self) -> (&[T], &[T]) { + // Safety: + // - `self.head` and `self.tail` in a ring buffer are always valid indices. + // - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized. unsafe { let buf = self.buffer_as_slice(); - RingSlices::ring_slices(buf, self.head, self.tail) + let (front, back) = RingSlices::ring_slices(buf, self.head, self.tail); + (MaybeUninit::slice_assume_init_ref(front), MaybeUninit::slice_assume_init_ref(back)) } } @@ -1089,11 +1105,15 @@ impl<T, A: Allocator> VecDeque<T, A> { #[inline] #[stable(feature = "deque_extras_15", since = "1.5.0")] pub fn as_mut_slices(&mut self) -> (&mut [T], &mut [T]) { + // Safety: + // - `self.head` and `self.tail` in a ring buffer are always valid indices. + // - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized. unsafe { let head = self.head; let tail = self.tail; let buf = self.buffer_as_mut_slice(); - RingSlices::ring_slices(buf, head, tail) + let (front, back) = RingSlices::ring_slices(buf, head, tail); + (MaybeUninit::slice_assume_init_mut(front), MaybeUninit::slice_assume_init_mut(back)) } } @@ -2327,7 +2347,14 @@ impl<T, A: Allocator> VecDeque<T, A> { if self.is_contiguous() { let tail = self.tail; let head = self.head; - return unsafe { RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0 }; + // Safety: + // - `self.head` and `self.tail` in a ring buffer are always valid indices. + // - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized. + return unsafe { + MaybeUninit::slice_assume_init_mut( + RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0, + ) + }; } let buf = self.buf.ptr(); @@ -2413,7 +2440,14 @@ impl<T, A: Allocator> VecDeque<T, A> { let tail = self.tail; let head = self.head; - unsafe { RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0 } + // Safety: + // - `self.head` and `self.tail` in a ring buffer are always valid indices. + // - `RingSlices::ring_slices` guarantees that the slices split according to `self.head` and `self.tail` are initialized. + unsafe { + MaybeUninit::slice_assume_init_mut( + RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0, + ) + } } /// Rotates the double-ended queue `mid` places to the left. |
