From 026edbb4ef85ef3d38876548a771b7c72c87803f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Nov 2021 13:36:39 +1100 Subject: [PATCH 1/3] Use unchecked construction in `Layout::pad_to_align`. Other, similar methods for `Layout` do likewise, and there's already an `unwrap()` around the result demonstrating the safety. --- library/core/src/alloc/layout.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/core/src/alloc/layout.rs b/library/core/src/alloc/layout.rs index cc32d5223b49f..a9abb8d2d59c0 100644 --- a/library/core/src/alloc/layout.rs +++ b/library/core/src/alloc/layout.rs @@ -281,7 +281,9 @@ impl Layout { // > `usize::MAX`) let new_size = self.size() + pad; - Layout::from_size_align(new_size, self.align()).unwrap() + // SAFETY: self.align is already known to be valid and new_size has been + // padded already. + unsafe { Layout::from_size_align_unchecked(new_size, self.align()) } } /// Creates a layout describing the record for `n` instances of From f3bda74d363a060ade5e5caeb654ba59bfed51a4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Nov 2021 13:39:05 +1100 Subject: [PATCH 2/3] Optimize `Layout::array`. The current implementation is much more conservative than it needs to be, because it's dealing with the size and alignment of a given `T`, which are more restricted than an arbitrary `Layout`. For example, imagine a struct with a `u32` and a `u4`. You can safely create a `Layout { size_: 5, align_: 4 }` by hand, but `Layout::new::` will give `Layout { size_: 8, align_: 4}`, where the size already has padding that accounts for the alignment. (And the existing `debug_assert_eq!` in `Layout::array` already demonstrates that no additional padding is required.) --- library/core/src/alloc/layout.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/library/core/src/alloc/layout.rs b/library/core/src/alloc/layout.rs index a9abb8d2d59c0..9df0b5c551915 100644 --- a/library/core/src/alloc/layout.rs +++ b/library/core/src/alloc/layout.rs @@ -405,9 +405,17 @@ impl Layout { #[stable(feature = "alloc_layout_manipulation", since = "1.44.0")] #[inline] pub fn array(n: usize) -> Result { - let (layout, offset) = Layout::new::().repeat(n)?; - debug_assert_eq!(offset, mem::size_of::()); - Ok(layout.pad_to_align()) + let array_size = mem::size_of::().checked_mul(n).ok_or(LayoutError)?; + + // SAFETY: + // - Size: `array_size` cannot be too big because `size_of::()` must + // be a multiple of `align_of::()`. Therefore, `array_size` + // rounded up to the nearest multiple of `align_of::()` is just + // `array_size`. And `array_size` cannot be too big because it was + // just checked by the `checked_mul()`. + // - Alignment: `align_of::()` will always give an acceptable + // (non-zero, power of two) alignment. + Ok(unsafe { Layout::from_size_align_unchecked(array_size, mem::align_of::()) }) } } From dbfb91385f7e4c27a6a48ecc3f683e3295af8b49 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 25 Nov 2021 19:29:10 +1100 Subject: [PATCH 3/3] Add a unit test for zero-sized types in `RawVec`. Because there's some subtle behaviour specific to zero-sized types and it's currently not well tested. --- library/alloc/src/raw_vec/tests.rs | 84 ++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/library/alloc/src/raw_vec/tests.rs b/library/alloc/src/raw_vec/tests.rs index 8c15a24409bab..ff322f0da97c6 100644 --- a/library/alloc/src/raw_vec/tests.rs +++ b/library/alloc/src/raw_vec/tests.rs @@ -77,3 +77,87 @@ fn reserve_does_not_overallocate() { assert!(v.capacity() >= 12 + 12 / 2); } } + +struct ZST; + +// A `RawVec` holding zero-sized elements should always look like this. +fn zst_sanity(v: &RawVec) { + assert_eq!(v.capacity(), usize::MAX); + assert_eq!(v.ptr(), core::ptr::Unique::::dangling().as_ptr()); + assert_eq!(v.current_memory(), None); +} + +#[test] +fn zst() { + let cap_err = Err(crate::collections::TryReserveErrorKind::CapacityOverflow.into()); + + assert_eq!(std::mem::size_of::(), 0); + + // All these different ways of creating the RawVec produce the same thing. + + let v: RawVec = RawVec::new(); + zst_sanity(&v); + + let v: RawVec = RawVec::with_capacity_in(100, Global); + zst_sanity(&v); + + let v: RawVec = RawVec::with_capacity_in(100, Global); + zst_sanity(&v); + + let v: RawVec = RawVec::allocate_in(0, AllocInit::Uninitialized, Global); + zst_sanity(&v); + + let v: RawVec = RawVec::allocate_in(100, AllocInit::Uninitialized, Global); + zst_sanity(&v); + + let mut v: RawVec = RawVec::allocate_in(usize::MAX, AllocInit::Uninitialized, Global); + zst_sanity(&v); + + // Check all these operations work as expected with zero-sized elements. + + assert!(!v.needs_to_grow(100, usize::MAX - 100)); + assert!(v.needs_to_grow(101, usize::MAX - 100)); + zst_sanity(&v); + + v.reserve(100, usize::MAX - 100); + //v.reserve(101, usize::MAX - 100); // panics, in `zst_reserve_panic` below + zst_sanity(&v); + + v.reserve_exact(100, usize::MAX - 100); + //v.reserve_exact(101, usize::MAX - 100); // panics, in `zst_reserve_exact_panic` below + zst_sanity(&v); + + assert_eq!(v.try_reserve(100, usize::MAX - 100), Ok(())); + assert_eq!(v.try_reserve(101, usize::MAX - 100), cap_err); + zst_sanity(&v); + + assert_eq!(v.try_reserve_exact(100, usize::MAX - 100), Ok(())); + assert_eq!(v.try_reserve_exact(101, usize::MAX - 100), cap_err); + zst_sanity(&v); + + assert_eq!(v.grow_amortized(100, usize::MAX - 100), cap_err); + assert_eq!(v.grow_amortized(101, usize::MAX - 100), cap_err); + zst_sanity(&v); + + assert_eq!(v.grow_exact(100, usize::MAX - 100), cap_err); + assert_eq!(v.grow_exact(101, usize::MAX - 100), cap_err); + zst_sanity(&v); +} + +#[test] +#[should_panic(expected = "capacity overflow")] +fn zst_reserve_panic() { + let mut v: RawVec = RawVec::new(); + zst_sanity(&v); + + v.reserve(101, usize::MAX - 100); +} + +#[test] +#[should_panic(expected = "capacity overflow")] +fn zst_reserve_exact_panic() { + let mut v: RawVec = RawVec::new(); + zst_sanity(&v); + + v.reserve_exact(101, usize::MAX - 100); +}