fix Box potential UB and other issues, add Box::assume_init and helpers::is_aligned, centralize ALN constant type property

Author: echohumm

bench.sh

@@ -1,5 +1,5 @@
-use ::core::sync::atomic::{AtomicBool, Ordering};
 use {
+    ::core::sync::atomic::{AtomicBool, Ordering},
     memapi2::{helpers::ptr_max_align, prelude::*},
     std::{
         env,
@@ -13,7 +13,8 @@ fn main() {
 
     ctrlc::set_handler(move || {
         BREAK.store(true, Ordering::Relaxed);
-    }).expect("failed to set ctrl-c handler");
+    })
+    .expect("failed to set ctrl-c handler");
 
     let o = stdout();
     let i = stdin();
@@ -71,7 +72,9 @@ fn main() {
                     total_bytes_allocated += layout.size() as u128;
                     if layout.size() > max_success_size {
                         max_success_size = layout.size();
-                        max_success_size_align = layout.align();
+                        if layout.align() > max_success_size_align {
+                            max_success_size_align = layout.align();
+                        }
                     }
                     if layout.align() > max_success_align {
                         max_success_align = layout.align();

bin/alloc_many.rs

@@ -7,7 +7,13 @@ use {
             alloc_mut::BasicAllocMut,
             data::{
                 marker::UnsizedCopy,
-                type_props::{PtrProps, SizedProps, VarSized, varsized_nonnull_from_parts}
+                type_props::{
+                    KnownAlign,
+                    PtrProps,
+                    SizedProps,
+                    VarSized,
+                    varsized_nonnull_from_parts
+                }
             }
         }
     },
@@ -16,7 +22,7 @@ use {
         default::Default,
         fmt::{Debug, Display, Formatter, Result as FmtResult},
         marker::{PhantomData, Send, Sized, Sync},
-        mem::MaybeUninit,
+        mem::{ManuallyDrop, MaybeUninit},
         ops::{Deref, DerefMut, Drop},
         panic,
         ptr::{self, NonNull},
@@ -25,15 +31,20 @@ use {
     }
 };
 
+// TODO: box_all_unsized which adds support for trait objects and all unsized types, even those that
+// don't implement VarSized.
+
 #[inline]
-fn unwrap_fail<T: ?Sized, A: BasicAllocMut, E: Display>(r: Result<Box<T, A>, E>) -> Box<T, A> {
+fn unwrap_fail<T: ?Sized + KnownAlign, A: BasicAllocMut, E: Display>(
+    r: Result<Box<T, A>, E>
+) -> Box<T, A> {
     match r {
         Ok(b) => b,
         Err(e) => panic!("allocation for `Box` failed: {}", e)
     }
 }
 
-pub struct Box<T: ?Sized, A: BasicAllocMut = DefaultAlloc> {
+pub struct Box<T: ?Sized + KnownAlign, A: BasicAllocMut = DefaultAlloc> {
     ptr: NonNull<T>,
     alloc: A,
     _marker: PhantomData<T>
@@ -42,10 +53,12 @@ pub struct Box<T: ?Sized, A: BasicAllocMut = DefaultAlloc> {
 pub enum BoxErr<E: Debug + Display + From<Error>> {
     AllocError(E),
     NullPtr,
-    DanglingPtr(*const ())
+    DanglingPtr(usize)
 }
 
+// SAFETY: we own the `T` instance/as `Unique` puts it, "the data [this points to] is unaliased."
 unsafe impl<T: Send, A: BasicAllocMut + Send> Send for Box<T, A> {}
+// SAFETY: above
 unsafe impl<T: Sync, A: BasicAllocMut + Sync> Sync for Box<T, A> {}
 
 impl<E: Debug + Display + From<Error>> Display for BoxErr<E> {
@@ -94,12 +107,13 @@ impl<T, A: BasicAllocMut> Box<T, A> {
         data: T,
         alloc: A
     ) -> Result<Box<T, A>, BoxErr<<A as AllocDescriptor>::Error>> {
-        let mut alloc = alloc;
-        let ptr = tri!(wrap(BoxErr::AllocError) alloc.alloc_mut(T::LAYOUT)).cast();
+        let mut boxed = tri!(do Box::try_new_uninit_in(alloc));
+        // SAFETY: alloc traits always return a valid pointer.
         unsafe {
-            ptr::write(ptr.as_ptr(), data);
+            ptr::write(boxed.as_mut_ptr(), data);
         }
-        Ok(Box { ptr, alloc, _marker: PhantomData })
+        // SAFETY: we just initialized the box
+        Ok(unsafe { boxed.assume_init() })
     }
     pub fn try_new_uninit_in(
         alloc: A
@@ -110,9 +124,18 @@ impl<T, A: BasicAllocMut> Box<T, A> {
     }
 }
 
-impl<T: ?Sized> Box<T> {
+impl<T: KnownAlign, A: BasicAllocMut> Box<MaybeUninit<T>, A> {
+    // at least for now, this has no reason to be const
+    pub unsafe fn assume_init(self) -> Box<T, A> {
+        let no_drop = ManuallyDrop::new(self);
+        Box { ptr: no_drop.ptr.cast(), alloc: ptr::read(&no_drop.alloc), _marker: PhantomData }
+    }
+}
+
+impl<T: ?Sized + KnownAlign> Box<T> {
+    #[::rustversion::attr(since(1.61), const)]
     #[allow(clippy::must_use_candidate)]
-    pub const unsafe fn from_raw(ptr: NonNull<T>) -> Box<T> {
+    pub unsafe fn from_raw(ptr: NonNull<T>) -> Box<T> {
         Box::from_raw_in(ptr, DefaultAlloc)
     }
 
@@ -134,19 +157,22 @@ impl<T: ?Sized> Box<T> {
     where
         T: UnsizedCopy + VarSized
     {
+        // SAFETY: caller guarantee
         unsafe { Box::copy_from_ptr_in(p, DefaultAlloc) }
     }
 
     pub unsafe fn try_copy_from_ptr(p: *const T) -> Result<Box<T>, BoxErr<Error>>
     where
         T: UnsizedCopy + VarSized
     {
+        // SAFETY: caller guarantee
         unsafe { Box::try_copy_from_ptr_in(p, DefaultAlloc) }
     }
 }
 
-impl<T: ?Sized, A: BasicAllocMut> Box<T, A> {
-    pub const unsafe fn from_raw_in(ptr: NonNull<T>, alloc: A) -> Box<T, A> {
+impl<T: ?Sized + KnownAlign, A: BasicAllocMut> Box<T, A> {
+    #[::rustversion::attr(since(1.61), const)]
+    pub unsafe fn from_raw_in(ptr: NonNull<T>, alloc: A) -> Box<T, A> {
         Box { ptr, alloc, _marker: PhantomData }
     }
 
@@ -158,19 +184,20 @@ impl<T: ?Sized, A: BasicAllocMut> Box<T, A> {
     }
 
     pub fn try_copy_from_ref_in(
-        r: &T,
-        alloc: A
+        _r: &T,
+        _alloc: A
     ) -> Result<Box<T, A>, BoxErr<<A as AllocDescriptor>::Error>>
     where
         T: UnsizedCopy + VarSized
     {
-        unsafe { Box::try_copy_from_ptr_in(r, alloc) }
+        ::core::todo!()
     }
 
     pub unsafe fn copy_from_ptr_in(p: *const T, alloc: A) -> Box<T, A>
     where
         T: UnsizedCopy + VarSized
     {
+        // SAFETY: caller guarantee
         unwrap_fail(unsafe { Box::try_copy_from_ptr_in(p, alloc) })
     }
 
@@ -181,40 +208,61 @@ impl<T: ?Sized, A: BasicAllocMut> Box<T, A> {
     where
         T: UnsizedCopy + VarSized
     {
+        let p_addr = p.cast::<()>() as usize;
+        if p.is_null() {
+            return Err(BoxErr::NullPtr);
+        } else if p_addr == T::ALN {
+            return Err(BoxErr::DanglingPtr(p_addr));
+        } // we purposefully don't check for alignment here.
+
         let mut alloc = alloc;
 
+        // SAFETY: caller guarantee
         let sz = unsafe { p.sz() };
         let ptr = tri!(wrap(BoxErr::AllocError) alloc.alloc_mut(p.layout()));
-
+        // SAFETY: `alloc` returns a pointer to at least the `p.sz()` bytes
         unsafe {
             ptr::copy_nonoverlapping(p.cast::<u8>(), ptr.as_ptr(), sz);
         }
         Ok(Box { ptr: varsized_nonnull_from_parts(ptr, sz), alloc, _marker: PhantomData })
     }
 }
 
-impl<T: ?Sized, A: BasicAllocMut> Deref for Box<T, A> {
+impl<T: ?Sized + KnownAlign, A: BasicAllocMut> Deref for Box<T, A> {
     type Target = T;
 
     fn deref(&self) -> &T {
+        // SAFETY: `self.ptr` is always a pointer to a valid `T` unless a `from_raw` function had
+        // its safety contract broken.
         unsafe { &*self.ptr.as_ptr() }
     }
 }
 
-impl<T: ?Sized, A: BasicAllocMut> DerefMut for Box<T, A> {
+impl<T: ?Sized + KnownAlign, A: BasicAllocMut> DerefMut for Box<T, A> {
     fn deref_mut(&mut self) -> &mut T {
+        // SAFETY: `self.ptr` is always a pointer to a valid `T` unless a `from_raw` function had
+        // its safety contract broken; we own the `T`.
         unsafe { &mut *self.ptr.as_ptr() }
     }
 }
 
-impl<T: ?Sized, A: BasicAllocMut> Drop for Box<T, A> {
+impl<T: ?Sized + KnownAlign, A: BasicAllocMut> Drop for Box<T, A> {
     fn drop(&mut self) {
+        // SAFETY: `self.ptr` is always a pointer to a valid `T` unless a `from_raw` function had
+        // its safety contract broken; we own the `T`
         unsafe {
             ptr::drop_in_place(self.ptr.as_ptr());
         }
-        let layout = unsafe { self.ptr.layout() };
-        unsafe {
-            self.alloc.dealloc_mut(self.ptr.cast(), layout);
+        // TODO: we can make PtrProps functions like layout() fallible and safe with the new
+        //  KnownAlign::ALN
+        if self.ptr.as_ptr().cast::<()>() as usize != T::ALN {
+            // SAFETY: the pointer is non-null, we just validated it isn't dangling, and `alloc`
+            //  only returns aligned memory.
+            let layout = unsafe { self.ptr.layout() };
+            // SAFETY: we allocated the pointer
+            unsafe {
+                self.alloc.dealloc_mut(self.ptr.cast(), layout);
+            }
         }
     }
 }

src/data/boxed.rs

@@ -83,6 +83,17 @@ pub fn ptr_max_align(ptr: NonNull<u8>) -> usize {
     p & p.wrapping_neg()
 }
 
+/// Returns `true` if `ptr` is aligned to `align`.
+///
+/// # Safety
+///
+/// This function is safe to call, but the returned value may be incorrect if `align` is not a power
+/// of two. An underflow may also occur if `align == 0`.
+#[must_use]
+pub unsafe fn is_aligned(ptr: *const u8, align: usize) -> bool {
+    ptr as usize & (align - 1) == 0
+}
+
 /// Creates a <code>[NonNull]<\[T\]></code> from a pointer and a length.
 ///
 /// Note that this is only `const` on Rust versions 1.61 and above.

src/helpers.rs

@@ -1,3 +1,4 @@
+use crate::traits::data::type_props::KnownAlign;
 use {
     crate::{
         error::{ArithErr, ArithOp, LayoutErr},
@@ -96,7 +97,7 @@ impl Layout {
     ///
     /// # Errors
     ///
-    /// <code>Err([LayoutErr::ExceedsMax]\([T::SZ](SizedProps::SZ), [T::ALN](SizedProps::ALN),
+    /// <code>Err([LayoutErr::ExceedsMax]\([T::SZ](SizedProps::SZ), [T::ALN](KnownAlign::ALN),
     /// n\))</code> if the length of the computed array, in bytes, would
     /// exceed [`USIZE_MAX_NO_HIGH_BIT`].
     pub const fn array<T>(n: usize) -> Result<Layout, LayoutErr> {
@@ -114,7 +115,7 @@ impl Layout {
     /// # Safety
     ///
     /// The caller must ensure that <code>[T::SZ](SizedProps::SZ) * n</code> rounded up to
-    /// [`T::ALN`](SizedProps::ALN) will not exceed [`USIZE_MAX_NO_HIGH_BIT`].
+    /// [`T::ALN`](KnownAlign::ALN) will not exceed [`USIZE_MAX_NO_HIGH_BIT`].
     ///
     /// Additionally, the return value may be unexpected if <code>[T::SZ](SizedProps::SZ) * n</code>
     /// overflows.

src/layout.rs

@@ -34,14 +34,14 @@
 //!
 //! # Allocator implementations
 //! - [`DefaultAlloc`] (available unless `no_alloc` is enabled and `std` isn't)
-//! - [`System`](::std::alloc::System) when the `std` feature is enabled
+//! - `::std::alloc::System` when the `std` feature is enabled
 //! - [`CAlloc`](allocs::c_alloc::CAlloc) behind the `c_alloc` feature
 //! - [`StackAlloc`](allocs::stack_alloc::StackAlloc) behind the `stack_alloc` feature
 //!
 //! # Feature flags
 //! - `no_alloc` disables usage of the rust `alloc` crate; `std` crate is used instead if the `std`
 //!   feature is enabled
-//! - `std`: enables `std` integration (including [`System`](::std::alloc::System))
+//! - `std`: enables `std` integration (including `::std::alloc::System`)
 //! - `os_err_reporting`: best-effort OS error reporting via `errno` (requires `std`)
 //! - `alloc_temp_trait`: scoped/temporary allocation trait
 //! - `c_alloc`: C `posix_memalign`-style allocator ([`allocs::c_alloc`])
@@ -104,12 +104,11 @@ pub mod prelude {
                 BasicAllocMut,
                 DeallocMut,
                 FullAllocMut,
-
                 ReallocMut,
-                            },
+            },
             data::{
                 marker::UnsizedCopy,
-                type_props::{PtrProps, SizedProps}
+                type_props::{KnownAlign, PtrProps, SizedProps}
             },
             AllocDescriptor
         },

src/lib.rs

@@ -15,7 +15,7 @@ use {
     }
 };
 
-#[allow(unused_imports)] use crate::error::Cause;
+#[allow(unused_imports)] use crate::error::{Cause, Error};
 
 /// A memory allocation interface.
 pub trait Alloc: AllocDescriptor + AllocMut {
@@ -32,14 +32,11 @@ pub trait Alloc: AllocDescriptor + AllocMut {
     /// - <code>Err([Error::AllocFailed]\(layout, cause\))</code> if allocation fails. `cause` is
     ///   typically [`Cause::Unknown`]. If the `os_err_reporting` feature is enabled, it will be
     ///   <code>[Cause::OSErr]\(oserr\)</code>. In this case, `oserr` will be the error from
-    ///   <code>[last_os_error]\(\).[raw_os_error]\(\)</code>.
+    ///   `::std::io::Error::last_os_error().raw_os_error()`.
     /// - <code>Err([Error::Other]\(err\))</code> for allocator-specific failures. If the
     ///   `alloc_mut` feature is enabled, and using this method on a synchronization primitive
     ///   wrapping a type which implements [`AllocMut`], a generic error message will also be
     ///   returned if locking the primitive fails.
-    ///
-    /// [last_os_error]: ::std::io::Error::last_os_error
-    /// [raw_os_error]: ::std::io::Error::raw_os_error
     fn alloc(&self, layout: Layout) -> Result<NonNull<u8>, <Self as AllocDescriptor>::Error>;
 
     /// Attempts to allocate a zeroed block of memory fitting the given [`Layout`].
@@ -55,14 +52,11 @@ pub trait Alloc: AllocDescriptor + AllocMut {
     /// - <code>Err([Error::AllocFailed]\(layout, cause\))</code> if allocation fails. `cause` is
     ///   typically [`Cause::Unknown`]. If the `os_err_reporting` feature is enabled, it will be
     ///   <code>[Cause::OSErr]\(oserr\)</code>. In this case, `oserr` will be the error from
-    ///   <code>[last_os_error]\(\).[raw_os_error]\(\)</code>.
+    ///   `::std::io::Error::last_os_error().raw_os_error()`.
     /// - <code>Err([Error::Other]\(err\))</code> for allocator-specific failures. If the
     ///   `alloc_mut` feature is enabled, and using this method on a synchronization primitive
     ///   wrapping a type which implements [`AllocMut`], a generic error message will also be
     ///   returned if locking the primitive fails.
-    ///
-    /// [last_os_error]: ::std::io::Error::last_os_error
-    /// [raw_os_error]: ::std::io::Error::raw_os_error
     #[cfg_attr(miri, track_caller)]
     #[inline]
     fn zalloc(&self, layout: Layout) -> Result<NonNull<u8>, <Self as AllocDescriptor>::Error> {
@@ -168,16 +162,13 @@ pub trait Realloc: ReallocMut + Dealloc {
     /// - <code>Err([Error::AllocFailed]\(layout, cause\))</code> if allocation fails. `cause` is
     ///   typically [`Cause::Unknown`]. If the `os_err_reporting` feature is enabled, it will be
     ///   <code>[Cause::OSErr]\(oserr\)</code>. In this case, `oserr` will be the error from
-    ///   <code>[last_os_error]\(\).[raw_os_error]\(\)</code>.
+    ///   `::std::io::Error::last_os_error().raw_os_error()`.
     /// - <code>Err([Error::ReallocSmallerAlign]\(old, new\))</code> if
     ///   <code>old_layout.[align](Layout::align)() > new_layout.[align](Layout::align)()</code>.
     /// - <code>Err([Error::Other]\(err\))</code> for allocator-specific failures. If the
     ///   `alloc_mut` feature is enabled, and using this method on a synchronization primitive
     ///   wrapping a type which implements [`ReallocMut`], a generic error message will also be
     ///   returned if locking the primitive fails.
-    ///
-    /// [last_os_error]: ::std::io::Error::last_os_error
-    /// [raw_os_error]: ::std::io::Error::raw_os_error
     #[cfg_attr(miri, track_caller)]
     #[inline]
     unsafe fn realloc(
@@ -219,16 +210,13 @@ pub trait Realloc: ReallocMut + Dealloc {
     /// - <code>Err([Error::AllocFailed]\(layout, cause\))</code> if allocation fails. `cause` is
     ///   typically [`Cause::Unknown`]. If the `os_err_reporting` feature is enabled, it will be
     ///   <code>[Cause::OSErr]\(oserr\)</code>. In this case, `oserr` will be the error from
-    ///   <code>[last_os_error]\(\).[raw_os_error]\(\)</code>.
+    ///   `::std::io::Error::last_os_error().raw_os_error()`.
     /// - <code>Err([Error::ReallocSmallerAlign]\(old, new\))</code> if
     ///   <code>old_layout.[align](Layout::align)() > new_layout.[align](Layout::align)()</code>.
     /// - <code>Err([Error::Other]\(err\))</code> for allocator-specific failures. If the
     ///   `alloc_mut` feature is enabled, and using this method on a synchronization primitive
     ///   wrapping a type which implements [`ReallocMut`], a generic error message will also be
     ///   returned if locking the primitive fails.
-    ///
-    /// [last_os_error]: ::std::io::Error::last_os_error
-    /// [raw_os_error]: ::std::io::Error::raw_os_error
     #[cfg_attr(miri, track_caller)]
     #[inline]
     unsafe fn rezalloc(