diff --git a/crates/error/src/backtrace.rs b/crates/error/src/backtrace.rs index 4301d3d360c7..a603be0dff28 100644 --- a/crates/error/src/backtrace.rs +++ b/crates/error/src/backtrace.rs @@ -4,7 +4,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; static ENABLED: AtomicBool = AtomicBool::new(true); fn enabled() -> bool { - ENABLED.load(Ordering::Acquire) + ENABLED.load(Ordering::Relaxed) } /// Forcibly disable capturing backtraces dynamically. @@ -15,7 +15,7 @@ fn enabled() -> bool { /// `backtrace` cargo feature. #[doc(hidden)] pub fn disable_backtrace() { - ENABLED.store(false, Ordering::Release) + ENABLED.store(false, Ordering::Relaxed) } #[track_caller] diff --git a/crates/error/src/boxed.rs b/crates/error/src/boxed.rs index cba4d90f3dfe..a3c215e8c9ff 100644 --- a/crates/error/src/boxed.rs +++ b/crates/error/src/boxed.rs @@ -1,6 +1,7 @@ use super::{OutOfMemory, Result}; use alloc::boxed::Box; use core::alloc::Layout; +use core::mem::MaybeUninit; use core::ptr::NonNull; /// Try to allocate a block of memory that fits the given layout, or return an @@ -24,27 +25,19 @@ pub(crate) unsafe fn try_alloc(layout: Layout) -> Result, OutOfMemor /// Create a `Box`, or return an `OutOfMemory` error. #[inline] -pub(crate) fn try_box(value: T) -> Result, OutOfMemory> { - let layout = alloc::alloc::Layout::new::(); +pub(crate) fn try_new_uninit_box() -> Result>, OutOfMemory> { + let layout = alloc::alloc::Layout::new::>(); if layout.size() == 0 { - // Safety: `Box` explicitly allows construction from dangling pointers - // (which are guaranteed non-null and aligned) for zero-sized types. - return Ok(unsafe { Box::from_raw(core::ptr::dangling::().cast_mut()) }); + // NB: no actual allocation takes place when boxing zero-sized types. + return Ok(Box::new(MaybeUninit::uninit())); } // Safety: layout size is non-zero. let ptr = unsafe { try_alloc(layout)? }; - let ptr = ptr.cast::(); + let ptr = ptr.cast::>(); - // Safety: The allocation succeeded, and it has `T`'s layout, so the pointer - // is valid for writing a `T`. - unsafe { - ptr.write(value); - } - - // Safety: The pointer's memory block was allocated by the global allocator, - // is valid for `T`, and is initialized. + // Safety: The pointer's memory block was allocated by the global allocator. Ok(unsafe { Box::from_raw(ptr.as_ptr()) }) } diff --git a/crates/error/src/error.rs b/crates/error/src/error.rs index 021e7e63eb24..61a544900fb7 100644 --- a/crates/error/src/error.rs +++ b/crates/error/src/error.rs @@ -1,4 +1,4 @@ -use super::boxed::try_box; +use super::boxed::try_new_uninit_box; use super::context::ContextError; use super::ptr::{MutPtr, OwnedPtr, SharedPtr}; use super::vtable::Vtable; @@ -18,8 +18,10 @@ use std::backtrace::{Backtrace, BacktraceStatus}; /// /// # Safety /// -/// Implementations must correctly report their type (or a type `T` where `*mut -/// Self` can be cast to `*mut T` and safely accessed) in `ext_is`. +/// Safety relies on `ext_is` being implemented correctly. Implementations must +/// not lie about whether they are or are not an instance of the given type id's +/// associated type (or a newtype wrapper around that type). Violations will +/// lead to unsafety. pub(crate) unsafe trait ErrorExt: core::error::Error + Send + Sync + 'static { /// Get a shared borrow of the next error in the chain. fn ext_source(&self) -> Option>; @@ -30,7 +32,8 @@ pub(crate) unsafe trait ErrorExt: core::error::Error + Send + Sync + 'static { /// Take ownership of the next error in the chain. fn ext_take_source(&mut self) -> Option; - /// Is this error an instance of `T`, where `type_id == TypeId::of::()`? + /// Is this error an instance of `T`, where `type_id == TypeId::of::()` + /// or a newtype wrapper around that type? /// /// # Safety /// @@ -137,12 +140,16 @@ impl BoxedDynError { None => crate::backtrace::capture(), }; - let error = try_box(ConcreteError { - vtable: Vtable::of::(), - #[cfg(feature = "backtrace")] - backtrace: Some(backtrace), - error, - })?; + let boxed = try_new_uninit_box()?; + let error = Box::write( + boxed, + ConcreteError { + vtable: Vtable::of::(), + #[cfg(feature = "backtrace")] + backtrace: Some(backtrace), + error, + }, + ); // We are going to pun the `ConcreteError` pointer into a `DynError` // pointer. Debug assert that their layouts are compatible first. @@ -179,7 +186,8 @@ impl BoxedDynError { // Safety: `Box::into_raw` always returns a non-null pointer. let ptr = unsafe { NonNull::new_unchecked(ptr) }; let ptr = OwnedPtr::new(ptr); - Ok(Self::from_owned_ptr(ptr)) + // Safety: points to a valid `DynError`. + Ok(unsafe { Self::from_owned_ptr(ptr) }) } fn into_owned_ptr(self) -> OwnedPtr { @@ -188,7 +196,12 @@ impl BoxedDynError { ptr } - fn from_owned_ptr(inner: OwnedPtr) -> Self { + /// # Safety + /// + /// The given pointer must be a valid `DynError` pointer: punning a + /// `ConcreteError` and is safe to drop and deallocate with its + /// `DynError::vtable` methods. + unsafe fn from_owned_ptr(inner: OwnedPtr) -> Self { BoxedDynError { inner } } } @@ -398,6 +411,7 @@ impl BoxedDynError { /// assert_eq!(error.to_string(), "oops I ate worms"); /// # } /// ``` +#[repr(transparent)] pub struct Error { pub(crate) inner: OomOrDynError, } @@ -1500,6 +1514,7 @@ impl<'a> OomOrDynErrorMut<'a> { /// Bit packed version of `enum { BoxedDynError, OutOfMemory }` that relies on /// implicit pointer tagging and `OutOfMemory` being zero-sized. +#[repr(transparent)] pub(crate) struct OomOrDynError { // Safety: this must always be the casted-to-`u8` version of either (a) // `0x1`, or (b) a valid, owned `DynError` pointer. (Note that these cases @@ -1526,7 +1541,8 @@ impl Drop for OomOrDynError { if self.is_boxed_dyn_error() { let inner = self.inner.cast::(); let inner = OwnedPtr::new(inner); - let _ = BoxedDynError::from_owned_ptr(inner); + // Safety: the pointer is a valid `DynError` pointer. + let _ = unsafe { BoxedDynError::from_owned_ptr(inner) }; } else { debug_assert!(self.is_oom()); } @@ -1640,11 +1656,9 @@ impl OomOrDynError { self, ) -> Box { let box_dyn_error_of_oom = || { - let ptr = NonNull::::dangling().as_ptr(); - // Safety: it is always safe to call `Box::::from_raw` on `T`'s - // dangling pointer if `T` is a unit type. - let boxed = unsafe { Box::from_raw(ptr) }; - boxed as _ + // NB: `Box::new` will never actually allocate for zero-sized types + // like `OutOfMemory`. + Box::new(OutOfMemory::new()) as _ }; if self.is_oom() { diff --git a/crates/error/src/vtable.rs b/crates/error/src/vtable.rs index 08a511da2d60..88fc3b39da7f 100644 --- a/crates/error/src/vtable.rs +++ b/crates/error/src/vtable.rs @@ -1,5 +1,5 @@ use super::{ConcreteError, DynError, ErrorExt, OomOrDynErrorMut, OomOrDynErrorRef, OutOfMemory}; -use crate::boxed::try_box; +use crate::boxed::try_new_uninit_box; use crate::ptr::{MutPtr, OwnedPtr, SharedPtr}; use alloc::boxed::Box; use core::{any::TypeId, fmt, ptr::NonNull}; @@ -137,7 +137,10 @@ where let error = error.cast::>(); // Safety: implied by all vtable functions' safety contract. let error = unsafe { error.into_box() }; - let error = try_box(error.error)?; + + let boxed = try_new_uninit_box()?; + let error = Box::write(boxed, error.error); + Ok(error as _) }