From a83ccecf515b5273e4dd0306c1b9f6dcb7daf963 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Sun, 8 Sep 2019 01:24:45 -0700 Subject: [PATCH 1/2] Use MaybeUninit for storage of inline items. This includes two breaking changes, in addition to the fact that it will require a MSRV bump: 1. The functions on the `Array` trait `ptr` and `ptr_mut` have been removed. Because these took a `&self`/`&mut self` argument, there's no way for us to call them when we only have a `MaybeUninit`. Now, we just use the memory of the object directly. This limits the flexibility of custom implementations of `Array`, (they can no longer return pointers to values other than themselves) but I imagine this is very rare and was probably broken somehow to begin with. Anybody who does this will get a compile error. 2. `from_buf_and_len_unchecked` now takes a MaybeUninit, so that callers have the option of only partially initializing the array. --- lib.rs | 101 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 49 insertions(+), 52 deletions(-) diff --git a/lib.rs b/lib.rs index 843eef2..583b046 100644 --- a/lib.rs +++ b/lib.rs @@ -63,7 +63,7 @@ use std::iter::{repeat, FromIterator, IntoIterator}; #[cfg(feature = "serde")] use std::marker::PhantomData; use std::mem; -use std::mem::ManuallyDrop; +use std::mem::MaybeUninit; use std::ops; use std::ptr; use std::slice; @@ -280,29 +280,27 @@ impl<'a, T: 'a> Drop for Drain<'a, T> { #[cfg(feature = "union")] union SmallVecData { - inline: ManuallyDrop, + inline: MaybeUninit, heap: (*mut A::Item, usize), } #[cfg(feature = "union")] impl SmallVecData { #[inline] - unsafe fn inline(&self) -> &A { - &self.inline + unsafe fn inline(&self) -> *const A::Item { + self.inline.as_ptr() as *const A::Item } #[inline] - unsafe fn inline_mut(&mut self) -> &mut A { - &mut self.inline + unsafe fn inline_mut(&mut self) -> *mut A::Item { + self.inline.as_mut_ptr() as *mut A::Item } #[inline] - fn from_inline(inline: A) -> SmallVecData { - SmallVecData { - inline: ManuallyDrop::new(inline), - } + fn from_inline(inline: MaybeUninit) -> SmallVecData { + SmallVecData { inline } } #[inline] - unsafe fn into_inline(self) -> A { - ManuallyDrop::into_inner(self.inline) + unsafe fn into_inline(self) -> MaybeUninit { + self.inline } #[inline] unsafe fn heap(&self) -> (*mut A::Item, usize) { @@ -320,34 +318,34 @@ impl SmallVecData { #[cfg(not(feature = "union"))] enum SmallVecData { - Inline(ManuallyDrop), + Inline(MaybeUninit), Heap((*mut A::Item, usize)), } #[cfg(not(feature = "union"))] impl SmallVecData { #[inline] - unsafe fn inline(&self) -> &A { + unsafe fn inline(&self) -> *const A::Item { match *self { - SmallVecData::Inline(ref a) => a, + SmallVecData::Inline(ref a) => a.as_ptr() as *const A::Item, _ => debug_unreachable!(), } } #[inline] - unsafe fn inline_mut(&mut self) -> &mut A { + unsafe fn inline_mut(&mut self) -> *mut A::Item { match *self { - SmallVecData::Inline(ref mut a) => a, + SmallVecData::Inline(ref mut a) => a.as_mut_ptr() as *mut A::Item, _ => debug_unreachable!(), } } #[inline] - fn from_inline(inline: A) -> SmallVecData { - SmallVecData::Inline(ManuallyDrop::new(inline)) + fn from_inline(inline: MaybeUninit) -> SmallVecData { + SmallVecData::Inline(inline) } #[inline] - unsafe fn into_inline(self) -> A { + unsafe fn into_inline(self) -> MaybeUninit { match self { - SmallVecData::Inline(a) => ManuallyDrop::into_inner(a), + SmallVecData::Inline(a) => a, _ => debug_unreachable!(), } } @@ -412,11 +410,15 @@ impl SmallVec { /// Construct an empty vector #[inline] pub fn new() -> SmallVec { - unsafe { - SmallVec { - capacity: 0, - data: SmallVecData::from_inline(mem::uninitialized()), - } + // Try to detect invalid custom implementations of `Array`. Hopefuly, + // this check should be optimized away entirely for valid ones. + assert!( + mem::size_of::() == A::size() * mem::size_of::() + && mem::align_of::() >= mem::align_of::() + ); + SmallVec { + capacity: 0, + data: SmallVecData::from_inline(MaybeUninit::uninit()), } } @@ -456,10 +458,10 @@ impl SmallVec { pub fn from_vec(mut vec: Vec) -> SmallVec { if vec.capacity() <= A::size() { unsafe { - let mut data = SmallVecData::::from_inline(mem::uninitialized()); + let mut data = SmallVecData::::from_inline(MaybeUninit::uninit()); let len = vec.len(); vec.set_len(0); - ptr::copy_nonoverlapping(vec.as_ptr(), data.inline_mut().ptr_mut(), len); + ptr::copy_nonoverlapping(vec.as_ptr(), data.inline_mut(), len); SmallVec { capacity: len, @@ -492,7 +494,7 @@ impl SmallVec { pub fn from_buf(buf: A) -> SmallVec { SmallVec { capacity: A::size(), - data: SmallVecData::from_inline(buf), + data: SmallVecData::from_inline(MaybeUninit::new(buf)), } } @@ -511,7 +513,7 @@ impl SmallVec { #[inline] pub fn from_buf_and_len(buf: A, len: usize) -> SmallVec { assert!(len <= A::size()); - unsafe { SmallVec::from_buf_and_len_unchecked(buf, len) } + unsafe { SmallVec::from_buf_and_len_unchecked(MaybeUninit::new(buf), len) } } /// Constructs a new `SmallVec` on the stack from an `A` without @@ -520,16 +522,17 @@ impl SmallVec { /// /// ```rust /// use smallvec::SmallVec; + /// use std::mem::MaybeUninit; /// /// let buf = [1, 2, 3, 4, 5, 0, 0, 0]; /// let small_vec: SmallVec<_> = unsafe { - /// SmallVec::from_buf_and_len_unchecked(buf, 5) + /// SmallVec::from_buf_and_len_unchecked(MaybeUninit::new(buf), 5) /// }; /// /// assert_eq!(&*small_vec, &[1, 2, 3, 4, 5]); /// ``` #[inline] - pub unsafe fn from_buf_and_len_unchecked(buf: A, len: usize) -> SmallVec { + pub unsafe fn from_buf_and_len_unchecked(buf: MaybeUninit, len: usize) -> SmallVec { SmallVec { capacity: len, data: SmallVecData::from_inline(buf), @@ -579,7 +582,7 @@ impl SmallVec { let (ptr, len) = self.data.heap(); (ptr, len, self.capacity) } else { - (self.data.inline().ptr(), self.capacity, A::size()) + (self.data.inline(), self.capacity, A::size()) } } } @@ -592,11 +595,7 @@ impl SmallVec { let &mut (ptr, ref mut len_ptr) = self.data.heap_mut(); (ptr, len_ptr, self.capacity) } else { - ( - self.data.inline_mut().ptr_mut(), - &mut self.capacity, - A::size(), - ) + (self.data.inline_mut(), &mut self.capacity, A::size()) } } } @@ -663,8 +662,8 @@ impl SmallVec { if unspilled { return; } - self.data = SmallVecData::from_inline(mem::uninitialized()); - ptr::copy_nonoverlapping(ptr, self.data.inline_mut().ptr_mut(), len); + self.data = SmallVecData::from_inline(MaybeUninit::uninit()); + ptr::copy_nonoverlapping(ptr, self.data.inline_mut(), len); self.capacity = len; } else if new_cap != cap { let mut vec = Vec::with_capacity(new_cap); @@ -730,8 +729,8 @@ impl SmallVec { if self.inline_size() >= len { unsafe { let (ptr, len) = self.data.heap(); - self.data = SmallVecData::from_inline(mem::uninitialized()); - ptr::copy_nonoverlapping(ptr, self.data.inline_mut().ptr_mut(), len); + self.data = SmallVecData::from_inline(MaybeUninit::uninit()); + ptr::copy_nonoverlapping(ptr, self.data.inline_mut(), len); deallocate(ptr, self.capacity); self.capacity = len; } @@ -900,7 +899,7 @@ impl SmallVec { unsafe { let data = ptr::read(&self.data); mem::forget(self); - Ok(data.into_inline()) + Ok(data.into_inline().assume_init()) } } } @@ -1062,8 +1061,12 @@ where SmallVec { capacity: len, data: SmallVecData::from_inline(unsafe { - let mut data: A = mem::uninitialized(); - ptr::copy_nonoverlapping(slice.as_ptr(), data.ptr_mut(), len); + let mut data: MaybeUninit = MaybeUninit::uninit(); + ptr::copy_nonoverlapping( + slice.as_ptr(), + data.as_mut_ptr() as *mut A::Item, + len, + ); data }), } @@ -1603,10 +1606,6 @@ pub unsafe trait Array { type Item; /// Returns the number of items the array can hold. fn size() -> usize; - /// Returns a pointer to the first element of the array. - fn ptr(&self) -> *const Self::Item; - /// Returns a mutable pointer to the first element of the array. - fn ptr_mut(&mut self) -> *mut Self::Item; } /// Set the length of the vec when the `SetLenOnDrop` value goes out of scope. @@ -1650,8 +1649,6 @@ macro_rules! impl_array( unsafe impl Array for [T; $size] { type Item = T; fn size() -> usize { $size } - fn ptr(&self) -> *const T { self.as_ptr() } - fn ptr_mut(&mut self) -> *mut T { self.as_mut_ptr() } } )+ } @@ -1985,7 +1982,7 @@ mod tests { ); } - #[cfg(feature = "std")] + #[cfg(all(feature = "std", not(miri)))] // Miri currently does not support unwinding #[test] // https://github.com/servo/rust-smallvec/issues/96 fn test_insert_many_panic() { From 28c09eb854cd8349889dc2118f73e9dc227183bc Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Sun, 8 Sep 2019 01:47:13 -0700 Subject: [PATCH 2/2] Run miri in CI, and bump MSRV --- .travis.yml | 5 +++-- scripts/run_miri.sh | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 scripts/run_miri.sh diff --git a/.travis.yml b/.travis.yml index ee744e1..2e54a81 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: rust rust: - - 1.20.0 + - 1.36.0 - nightly - beta - stable @@ -11,4 +11,5 @@ script: | ([ $TRAVIS_RUST_VERSION != nightly ] || cargo check --verbose --no-default-features) && ([ $TRAVIS_RUST_VERSION != nightly ] || cargo test --verbose --features union) && ([ $TRAVIS_RUST_VERSION != nightly ] || cargo test --verbose --all-features) && - ([ $TRAVIS_RUST_VERSION != nightly ] || cargo bench --verbose bench) + ([ $TRAVIS_RUST_VERSION != nightly ] || cargo bench --verbose bench) && + ([ $TRAVIS_RUST_VERSION != nightly ] || bash ./scripts/run_miri.sh) diff --git a/scripts/run_miri.sh b/scripts/run_miri.sh new file mode 100644 index 0000000..42f2884 --- /dev/null +++ b/scripts/run_miri.sh @@ -0,0 +1,21 @@ +#!/usr/bin/bash + +set -ex + +# Clean out our target dir, which may have artifacts compiled by a version of +# rust different from the one we're about to download. +cargo clean + +# Install and run the latest version of nightly where miri built successfully. +# Taken from: https://github.com/rust-lang/miri#running-miri-on-ci + +MIRI_NIGHTLY=nightly-$(curl -s https://rust-lang.github.io/rustup-components-history/x86_64-unknown-linux-gnu/miri) +echo "Installing latest nightly with Miri: $MIRI_NIGHTLY" +rustup default "$MIRI_NIGHTLY" + +rustup component add miri +cargo miri setup + +cargo miri test --verbose -- -- -Zunstable-options --exclude-should-panic +cargo miri test --verbose --features union -- -- -Zunstable-options --exclude-should-panic +cargo miri test --verbose --all-features -- -- -Zunstable-options --exclude-should-panic