FromZeros boxed slice method supports slice DSTs

Change methods that allocate to return a new error, `AllocError`, on
allocation failure rather than panicking or aborting.

Makes progress on #29

Author: joshlf

src/error.rs

@@ -570,6 +570,20 @@ impl<Src, Dst: ?Sized + TryFromBytes> TryReadError<Src, Dst> {
     }
 }
 
+/// The error type of a failed allocation.
+///
+/// This type is intended to be deprecated in favor of the standard library's
+/// [`AllocError`] type once it is stabilized. When that happens, this type will
+/// be replaced by a type alias to the standard library type. We do not intend
+/// to treat this as a breaking change; users who wish to avoid breakage should
+/// avoid writing code which assumes that this is *not* such an alias. For
+/// example, implementing the same trait for both types will result in an impl
+/// conflict once this type is an alias.
+///
+/// [`AllocError`]: https://doc.rust-lang.org/alloc/alloc/struct.AllocError.html
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+pub struct AllocError;
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/impls.rs

@@ -1020,7 +1020,7 @@ mod tests {
         impl<T: FromBytes> TryFromBytesTestable for T {
             fn with_passing_test_cases<F: Fn(Box<Self>)>(f: F) {
                 // Test with a zeroed value.
-                f(Self::new_box_zeroed());
+                f(Self::new_box_zeroed().unwrap());
 
                 let ffs = {
                     let mut t = Self::new_zeroed();

src/lib.rs

@@ -2113,14 +2113,15 @@ pub unsafe trait FromZeros: TryFromBytes {
     /// Note that `Box<Self>` can be converted to `Arc<Self>` and other
     /// container types without reallocation.
     ///
-    /// # Panics
+    /// # Errors
     ///
-    /// Panics if allocation of `size_of::<Self>()` bytes fails.
+    /// Returns an error on allocation failure. Allocation failure is guaranteed
+    /// never to cause a panic or an abort.
     #[must_use = "has no side effects (other than allocation)"]
     #[cfg(any(feature = "alloc", test))]
     #[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
     #[inline]
-    fn new_box_zeroed() -> Box<Self>
+    fn new_box_zeroed() -> Result<Box<Self>, AllocError>
     where
         Self: Sized,
     {
@@ -2146,22 +2147,18 @@ pub unsafe trait FromZeros: TryFromBytes {
             // [2] Per https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.dangling:
             //
             //   Creates a new `NonNull` that is dangling, but well-aligned.
-            unsafe {
-                return Box::from_raw(NonNull::dangling().as_ptr());
-            }
+            return Ok(unsafe { Box::from_raw(NonNull::dangling().as_ptr()) });
         }
 
         // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
         #[allow(clippy::undocumented_unsafe_blocks)]
         let ptr = unsafe { alloc::alloc::alloc_zeroed(layout).cast::<Self>() };
         if ptr.is_null() {
-            alloc::alloc::handle_alloc_error(layout);
+            return Err(AllocError);
         }
         // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
         #[allow(clippy::undocumented_unsafe_blocks)]
-        unsafe {
-            Box::from_raw(ptr)
-        }
+        Ok(unsafe { Box::from_raw(ptr) })
     }
 
     /// Creates a `Box<[Self]>` (a boxed slice) from zeroed bytes.
@@ -2181,22 +2178,24 @@ pub unsafe trait FromZeros: TryFromBytes {
     /// actual information, but its `len()` property will report the correct
     /// value.
     ///
-    /// # Panics
+    /// # Errors
     ///
-    /// * Panics if `size_of::<Self>() * len` overflows.
-    /// * Panics if allocation of `size_of::<Self>() * len` bytes fails.
+    /// Returns an error on allocation failure. Allocation failure is
+    /// guaranteed never to cause a panic or an abort.
     #[must_use = "has no side effects (other than allocation)"]
     #[cfg(feature = "alloc")]
     #[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
     #[inline]
-    fn new_box_slice_zeroed(len: usize) -> Box<[Self]>
+    fn new_box_zeroed_with_elems(count: usize) -> Result<Box<Self>, AllocError>
     where
-        Self: Sized,
+        Self: KnownLayout<PointerMetadata = usize>,
     {
-        let size = mem::size_of::<Self>()
-            .checked_mul(len)
-            .expect("mem::size_of::<Self>() * len overflows `usize`");
-        let align = mem::align_of::<Self>();
+        let size = match count.size_for_metadata(Self::LAYOUT) {
+            Some(size) => size,
+            None => return Err(AllocError),
+        };
+
+        let align = Self::LAYOUT.align.get();
         // On stable Rust versions <= 1.64.0, `Layout::from_size_align` has a
         // bug in which sufficiently-large allocations (those which, when
         // rounded up to the alignment, overflow `isize`) are not rejected,
@@ -2205,32 +2204,58 @@ pub unsafe trait FromZeros: TryFromBytes {
         // TODO(#67): Once our MSRV is > 1.64.0, remove this assertion.
         #[allow(clippy::as_conversions)]
         let max_alloc = (isize::MAX as usize).saturating_sub(align);
-        assert!(size <= max_alloc);
+        if size > max_alloc {
+            return Err(AllocError);
+        }
+
         // TODO(https://github.com/rust-lang/rust/issues/55724): Use
         // `Layout::repeat` once it's stabilized.
-        let layout =
-            Layout::from_size_align(size, align).expect("total allocation size overflows `isize`");
+        let layout = Layout::from_size_align(size, align).or(Err(AllocError))?;
 
         let ptr = if layout.size() != 0 {
             // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
             #[allow(clippy::undocumented_unsafe_blocks)]
-            let ptr = unsafe { alloc::alloc::alloc_zeroed(layout).cast::<Self>() };
-            if ptr.is_null() {
-                alloc::alloc::handle_alloc_error(layout);
+            let ptr = unsafe { alloc::alloc::alloc_zeroed(layout) };
+            match NonNull::new(ptr) {
+                Some(ptr) => ptr,
+                None => return Err(AllocError),
             }
-            ptr
         } else {
+            // SAFETY: `Self::LAYOUT.align` is a `NonZeroUsize`, which is
+            // guaranteed to be non-zero.
+            //
             // `Box<[T]>` does not allocate when `T` is zero-sized or when `len`
             // is zero, but it does require a non-null dangling pointer for its
             // allocation.
-            NonNull::<Self>::dangling().as_ptr()
+            //
+            // TODO(...): Use `without_provenance` once it's stable. That may
+            // optimize better. As written, Rust may assume that this consumes
+            // "exposed" provenance, and thus Rust may have to assume that this
+            // may consume provenance from any pointer whose provenance has been
+            // exposed.
+            unsafe { NonNull::new_unchecked(Self::LAYOUT.align.get() as *mut u8) }
         };
 
-        // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
+        let ptr = Self::raw_from_ptr_len(ptr, count);
+
+        // TODO(#429): Add a "SAFETY" comment and remove this `allow`. Make sure
+        // to include a justification that `ptr.as_ptr()` is validly-aligned in
+        // the ZST case (in which we manually construct a dangling pointer).
         #[allow(clippy::undocumented_unsafe_blocks)]
-        unsafe {
-            Box::from_raw(slice::from_raw_parts_mut(ptr, len))
-        }
+        Ok(unsafe { Box::from_raw(ptr.as_ptr()) })
+    }
+
+    #[deprecated(since = "0.8.0", note = "renamed to `FromZeros::new_box_zeroed_with_elems`")]
+    #[doc(hidden)]
+    #[cfg(feature = "alloc")]
+    #[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
+    #[must_use = "has no side effects (other than allocation)"]
+    #[inline(always)]
+    fn new_box_slice_zeroed(len: usize) -> Result<Box<[Self]>, AllocError>
+    where
+        Self: Sized,
+    {
+        <[Self]>::new_box_zeroed_with_elems(len)
     }
 
     /// Creates a `Vec<Self>` from zeroed bytes.
@@ -2249,19 +2274,19 @@ pub unsafe trait FromZeros: TryFromBytes {
     /// actual information, but its `len()` property will report the correct
     /// value.
     ///
-    /// # Panics
+    /// # Errors
     ///
-    /// * Panics if `size_of::<Self>() * len` overflows.
-    /// * Panics if allocation of `size_of::<Self>() * len` bytes fails.
+    /// Returns an error on allocation failure. Allocation failure is
+    /// guaranteed never to cause a panic or an abort.
     #[must_use = "has no side effects (other than allocation)"]
     #[cfg(feature = "alloc")]
     #[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
     #[inline(always)]
-    fn new_vec_zeroed(len: usize) -> Vec<Self>
+    fn new_vec_zeroed(len: usize) -> Result<Vec<Self>, AllocError>
     where
         Self: Sized,
     {
-        Self::new_box_slice_zeroed(len).into()
+        <[Self]>::new_box_zeroed_with_elems(len).map(Into::into)
     }
 }
 
@@ -4972,7 +4997,7 @@ mod alloc_support {
 
         #[test]
         fn test_new_box_zeroed() {
-            assert_eq!(*u64::new_box_zeroed(), 0);
+            assert_eq!(u64::new_box_zeroed(), Ok(Box::new(0)));
         }
 
         #[test]
@@ -4986,13 +5011,13 @@ mod alloc_support {
             // when running under Miri.
             #[allow(clippy::unit_cmp)]
             {
-                assert_eq!(*<()>::new_box_zeroed(), ());
+                assert_eq!(<()>::new_box_zeroed(), Ok(Box::new(())));
             }
         }
 
         #[test]
         fn test_new_box_slice_zeroed() {
-            let mut s: Box<[u64]> = u64::new_box_slice_zeroed(3);
+            let mut s: Box<[u64]> = <[u64]>::new_box_zeroed_with_elems(3).unwrap();
             assert_eq!(s.len(), 3);
             assert_eq!(&*s, &[0, 0, 0]);
             s[1] = 3;
@@ -5001,13 +5026,13 @@ mod alloc_support {
 
         #[test]
         fn test_new_box_slice_zeroed_empty() {
-            let s: Box<[u64]> = u64::new_box_slice_zeroed(0);
+            let s: Box<[u64]> = <[u64]>::new_box_zeroed_with_elems(0).unwrap();
             assert_eq!(s.len(), 0);
         }
 
         #[test]
         fn test_new_box_slice_zeroed_zst() {
-            let mut s: Box<[()]> = <()>::new_box_slice_zeroed(3);
+            let mut s: Box<[()]> = <[()]>::new_box_zeroed_with_elems(3).unwrap();
             assert_eq!(s.len(), 3);
             assert!(s.get(10).is_none());
             // This test exists in order to exercise unsafe code, especially
@@ -5021,21 +5046,19 @@ mod alloc_support {
 
         #[test]
         fn test_new_box_slice_zeroed_zst_empty() {
-            let s: Box<[()]> = <()>::new_box_slice_zeroed(0);
+            let s: Box<[()]> = <[()]>::new_box_zeroed_with_elems(0).unwrap();
             assert_eq!(s.len(), 0);
         }
 
         #[test]
-        #[should_panic(expected = "mem::size_of::<Self>() * len overflows `usize`")]
-        fn test_new_box_slice_zeroed_panics_mul_overflow() {
-            let _ = u16::new_box_slice_zeroed(usize::MAX);
-        }
+        fn test_new_box_slice_zeroed_errors() {
+            assert_eq!(<[u16]>::new_box_zeroed_with_elems(usize::MAX), Err(AllocError));
 
-        #[test]
-        #[should_panic(expected = "assertion failed: size <= max_alloc")]
-        fn test_new_box_slice_zeroed_panics_isize_overflow() {
             let max = usize::try_from(isize::MAX).unwrap();
-            let _ = u16::new_box_slice_zeroed((max / mem::size_of::<u16>()) + 1);
+            assert_eq!(
+                <[u16]>::new_box_zeroed_with_elems((max / mem::size_of::<u16>()) + 1),
+                Err(AllocError)
+            );
         }
     }
 }
@@ -5525,14 +5548,20 @@ mod tests {
 
         #[cfg(feature = "alloc")]
         {
-            assert_eq!(bool::new_box_zeroed(), Box::new(false));
-            assert_eq!(char::new_box_zeroed(), Box::new('\0'));
+            assert_eq!(bool::new_box_zeroed(), Ok(Box::new(false)));
+            assert_eq!(char::new_box_zeroed(), Ok(Box::new('\0')));
 
-            assert_eq!(bool::new_box_slice_zeroed(3).as_ref(), [false, false, false]);
-            assert_eq!(char::new_box_slice_zeroed(3).as_ref(), ['\0', '\0', '\0']);
+            assert_eq!(
+                <[bool]>::new_box_zeroed_with_elems(3).unwrap().as_ref(),
+                [false, false, false]
+            );
+            assert_eq!(
+                <[char]>::new_box_zeroed_with_elems(3).unwrap().as_ref(),
+                ['\0', '\0', '\0']
+            );
 
-            assert_eq!(bool::new_vec_zeroed(3).as_ref(), [false, false, false]);
-            assert_eq!(char::new_vec_zeroed(3).as_ref(), ['\0', '\0', '\0']);
+            assert_eq!(bool::new_vec_zeroed(3).unwrap().as_ref(), [false, false, false]);
+            assert_eq!(char::new_vec_zeroed(3).unwrap().as_ref(), ['\0', '\0', '\0']);
         }
 
         let mut string = "hello".to_string();