From a87a0d089e793903844ef40851deae48c039f8bb Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Tue, 19 Oct 2021 15:23:19 -0700 Subject: [PATCH] Add ThinBox type for 1 stack pointer sized heap allocated trait objects Relevant commit messages from squashed history in order: Add initial version of ThinBox update test to actually capture failure swap to middle ptr impl based on matthieu-m's design Fix stack overflow in debug impl The previous version would take a `&ThinBox` and deref it once, which resulted in a no-op and the same type, which it would then print causing an endless recursion. I've switched to calling `deref` by name to let method resolution handle deref the correct number of times. I've also updated the Drop impl for good measure since it seemed like it could be falling prey to the same bug, and I'll be adding some tests to verify that the drop is happening correctly. add test to verify drop is behaving add doc examples and remove unnecessary Pointee bounds ThinBox: use NonNull ThinBox: tests for size Apply suggestions from code review Co-authored-by: Alphyr <47725341+a1phyr@users.noreply.github.com> use handle_alloc_error and fix drop signature update niche and size tests add cfg for allocating APIs check null before calculating offset add test for zst and trial usage prevent optimizer induced ub in drop and cleanup metadata gathering account for arbitrary size and alignment metadata Thank you nika and thomcc! Update library/alloc/src/boxed/thin.rs Co-authored-by: Josh Triplett Update library/alloc/src/boxed/thin.rs Co-authored-by: Josh Triplett --- library/alloc/src/boxed.rs | 5 + library/alloc/src/boxed/thin.rs | 215 ++++++++++++++++++ library/alloc/src/lib.rs | 2 + library/alloc/tests/lib.rs | 2 + library/alloc/tests/thin_box.rs | 26 +++ library/std/src/error.rs | 8 + library/std/src/lib.rs | 1 + src/test/ui/box/thin_align.rs | 26 +++ src/test/ui/box/thin_drop.rs | 37 +++ src/test/ui/box/thin_new.rs | 30 +++ src/test/ui/box/thin_zst.rs | 34 +++ ...ce-issue-49593-box-never.nofallback.stderr | 4 +- 12 files changed, 388 insertions(+), 2 deletions(-) create mode 100644 library/alloc/src/boxed/thin.rs create mode 100644 library/alloc/tests/thin_box.rs create mode 100644 src/test/ui/box/thin_align.rs create mode 100644 src/test/ui/box/thin_drop.rs create mode 100644 src/test/ui/box/thin_new.rs create mode 100644 src/test/ui/box/thin_zst.rs diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index a56d4de03cd0e..e6faf1df3a810 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -163,6 +163,11 @@ use crate::str::from_boxed_utf8_unchecked; #[cfg(not(no_global_oom_handling))] use crate::vec::Vec; +#[unstable(feature = "thin_box", issue = "92791")] +pub use thin::ThinBox; + +mod thin; + /// A pointer type for heap allocation. /// /// See the [module-level documentation](../../std/boxed/index.html) for more. diff --git a/library/alloc/src/boxed/thin.rs b/library/alloc/src/boxed/thin.rs new file mode 100644 index 0000000000000..390030fa2b21c --- /dev/null +++ b/library/alloc/src/boxed/thin.rs @@ -0,0 +1,215 @@ +// Based on +// https://github.com/matthieu-m/rfc2580/blob/b58d1d3cba0d4b5e859d3617ea2d0943aaa31329/examples/thin.rs +// by matthieu-m +use crate::alloc::{self, Layout, LayoutError}; +use core::fmt::{self, Debug, Display, Formatter}; +use core::marker::{PhantomData, Unsize}; +use core::mem; +use core::ops::{Deref, DerefMut}; +use core::ptr::Pointee; +use core::ptr::{self, NonNull}; + +/// ThinBox. +/// +/// A thin pointer for heap allocation, regardless of T. +/// +/// # Examples +/// +/// ``` +/// #![feature(thin_box)] +/// use std::boxed::ThinBox; +/// +/// let five = ThinBox::new(5); +/// let thin_slice = ThinBox::<[i32]>::new_unsize([1, 2, 3, 4]); +/// +/// use std::mem::{size_of, size_of_val}; +/// let size_of_ptr = size_of::<*const ()>(); +/// assert_eq!(size_of_ptr, size_of_val(&five)); +/// assert_eq!(size_of_ptr, size_of_val(&thin_slice)); +/// ``` +#[unstable(feature = "thin_box", issue = "92791")] +pub struct ThinBox { + ptr: WithHeader<::Metadata>, + _marker: PhantomData, +} + +#[unstable(feature = "thin_box", issue = "92791")] +impl ThinBox { + /// Moves a type to the heap with its `Metadata` stored in the heap allocation instead of on + /// the stack. + /// + /// # Examples + /// + /// ``` + /// #![feature(thin_box)] + /// use std::boxed::ThinBox; + /// + /// let five = ThinBox::new(5); + /// ``` + #[cfg(not(no_global_oom_handling))] + pub fn new(value: T) -> Self { + let meta = ptr::metadata(&value); + let ptr = WithHeader::new(meta, value); + ThinBox { ptr, _marker: PhantomData } + } +} + +#[unstable(feature = "thin_box", issue = "92791")] +impl ThinBox { + /// Moves a type to the heap with its `Metadata` stored in the heap allocation instead of on + /// the stack. + /// + /// # Examples + /// + /// ``` + /// #![feature(thin_box)] + /// use std::boxed::ThinBox; + /// + /// let thin_slice = ThinBox::<[i32]>::new_unsize([1, 2, 3, 4]); + /// ``` + #[cfg(not(no_global_oom_handling))] + pub fn new_unsize(value: T) -> Self + where + T: Unsize, + { + let meta = ptr::metadata(&value as &Dyn); + let ptr = WithHeader::new(meta, value); + ThinBox { ptr, _marker: PhantomData } + } +} + +#[unstable(feature = "thin_box", issue = "92791")] +impl Debug for ThinBox { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Debug::fmt(self.deref(), f) + } +} + +#[unstable(feature = "thin_box", issue = "92791")] +impl Display for ThinBox { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Display::fmt(self.deref(), f) + } +} + +#[unstable(feature = "thin_box", issue = "92791")] +impl Deref for ThinBox { + type Target = T; + + fn deref(&self) -> &T { + let value = self.data(); + let metadata = self.meta(); + let pointer = ptr::from_raw_parts(value as *const (), metadata); + unsafe { &*pointer } + } +} + +#[unstable(feature = "thin_box", issue = "92791")] +impl DerefMut for ThinBox { + fn deref_mut(&mut self) -> &mut T { + let value = self.data(); + let metadata = self.meta(); + let pointer = ptr::from_raw_parts_mut::(value as *mut (), metadata); + unsafe { &mut *pointer } + } +} + +#[unstable(feature = "thin_box", issue = "92791")] +impl Drop for ThinBox { + fn drop(&mut self) { + unsafe { + let value = self.deref_mut(); + let value = value as *mut T; + self.ptr.drop::(value); + } + } +} + +#[unstable(feature = "thin_box", issue = "92791")] +impl ThinBox { + fn meta(&self) -> ::Metadata { + // Safety: + // - NonNull and valid. + unsafe { *self.ptr.header() } + } + + fn data(&self) -> *mut u8 { + self.ptr.value() + } +} + +/// A pointer to type-erased data, guaranteed to have a header `H` before the pointed-to location. +struct WithHeader(NonNull, PhantomData); + +impl WithHeader { + #[cfg(not(no_global_oom_handling))] + fn new(header: H, value: T) -> WithHeader { + let value_layout = Layout::new::(); + let Ok((layout, value_offset)) = Self::alloc_layout(value_layout) else { + // We pass an empty layout here because we do not know which layout caused the + // arithmetic overflow in `Layout::extend` and `handle_alloc_error` takes `Layout` as + // its argument rather than `Result`, also this function has been + // stable since 1.28 ._. + // + // On the other hand, look at this gorgeous turbofish! + alloc::handle_alloc_error(Layout::new::<()>()); + }; + + unsafe { + let ptr = alloc::alloc(layout); + + if ptr.is_null() { + alloc::handle_alloc_error(layout); + } + // Safety: + // - The size is at least `aligned_header_size`. + let ptr = ptr.add(value_offset) as *mut _; + + let ptr = NonNull::new_unchecked(ptr); + + let result = WithHeader(ptr, PhantomData); + ptr::write(result.header(), header); + ptr::write(result.value().cast(), value); + + result + } + } + + // Safety: + // - Assumes that `value` can be dereferenced. + unsafe fn drop(&self, value: *mut T) { + unsafe { + // SAFETY: Layout must have been computable if we're in drop + let (layout, value_offset) = + Self::alloc_layout(Layout::for_value_raw(value)).unwrap_unchecked(); + + ptr::drop_in_place::(value); + // We only drop the value because the Pointee trait requires that the metadata is copy + // aka trivially droppable + alloc::dealloc(self.0.as_ptr().sub(value_offset), layout); + } + } + + fn header(&self) -> *mut H { + // Safety: + // - At least `size_of::()` bytes are allocated ahead of the pointer. + // - We know that H will be aligned because the middle pointer is aligned to the greater + // of the alignment of the header and the data and the header size includes the padding + // needed to align the header. Subtracting the header size from the aligned data pointer + // will always result in an aligned header pointer, it just may not point to the + // beginning of the allocation. + unsafe { self.0.as_ptr().sub(Self::header_size()) as *mut H } + } + + fn value(&self) -> *mut u8 { + self.0.as_ptr() + } + + const fn header_size() -> usize { + mem::size_of::() + } + + fn alloc_layout(value_layout: Layout) -> Result<(Layout, usize), LayoutError> { + Layout::new::().extend(value_layout) + } +} diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 2ddb5f231b1d6..c54001dceea49 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -120,6 +120,7 @@ #![feature(nonnull_slice_from_raw_parts)] #![feature(pattern)] #![feature(ptr_internals)] +#![feature(ptr_metadata)] #![feature(receiver_trait)] #![feature(set_ptr_value)] #![feature(slice_group_by)] @@ -152,6 +153,7 @@ #![feature(fundamental)] #![cfg_attr(not(test), feature(generator_trait))] #![feature(lang_items)] +#![feature(let_else)] #![feature(min_specialization)] #![feature(negative_impls)] #![feature(never_type)] diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index 16ceb8e373dcb..16d3b36859570 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -39,6 +39,7 @@ #![feature(nonnull_slice_from_raw_parts)] #![feature(panic_update_hook)] #![feature(slice_flatten)] +#![feature(thin_box)] use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; @@ -57,6 +58,7 @@ mod rc; mod slice; mod str; mod string; +mod thin_box; mod vec; mod vec_deque; diff --git a/library/alloc/tests/thin_box.rs b/library/alloc/tests/thin_box.rs new file mode 100644 index 0000000000000..0fe6aaa4d0048 --- /dev/null +++ b/library/alloc/tests/thin_box.rs @@ -0,0 +1,26 @@ +use alloc::boxed::ThinBox; +use core::mem::size_of; + +#[test] +fn want_niche_optimization() { + fn uses_niche() -> bool { + size_of::<*const ()>() == size_of::>>() + } + + trait Tr {} + assert!(uses_niche::()); + assert!(uses_niche::<[i32]>()); + assert!(uses_niche::()); +} + +#[test] +fn want_thin() { + fn is_thin() -> bool { + size_of::<*const ()>() == size_of::>() + } + + trait Tr {} + assert!(is_thin::()); + assert!(is_thin::<[i32]>()); + assert!(is_thin::()); +} diff --git a/library/std/src/error.rs b/library/std/src/error.rs index c3cb71a5dee63..4fb94908c80fd 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -516,6 +516,14 @@ impl Error for Box { } } +#[unstable(feature = "thin_box", issue = "92791")] +impl crate::error::Error for crate::boxed::ThinBox { + fn source(&self) -> Option<&(dyn crate::error::Error + 'static)> { + use core::ops::Deref; + self.deref().source() + } +} + #[stable(feature = "error_by_ref", since = "1.51.0")] impl<'a, T: Error + ?Sized> Error for &'a T { #[allow(deprecated, deprecated_in_future)] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 5ade65ad9c629..60e7c2af8e4a5 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -290,6 +290,7 @@ #![feature(get_mut_unchecked)] #![feature(map_try_insert)] #![feature(new_uninit)] +#![feature(thin_box)] #![feature(toowned_clone_into)] #![feature(try_reserve_kind)] #![feature(vec_into_raw_parts)] diff --git a/src/test/ui/box/thin_align.rs b/src/test/ui/box/thin_align.rs new file mode 100644 index 0000000000000..3c61d0090e42b --- /dev/null +++ b/src/test/ui/box/thin_align.rs @@ -0,0 +1,26 @@ +#![feature(thin_box)] +// run-pass +use std::boxed::ThinBox; +use std::error::Error; +use std::ops::Deref; +use std::fmt; + +fn main() { + let expected = "Foo error!"; + let a: ThinBox = ThinBox::new_unsize(Foo(expected)); + let a = a.deref(); + let msg = a.to_string(); + assert_eq!(expected, msg); +} + +#[derive(Debug)] +#[repr(align(1024))] +struct Foo(&'static str); + +impl fmt::Display for Foo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Error for Foo {} diff --git a/src/test/ui/box/thin_drop.rs b/src/test/ui/box/thin_drop.rs new file mode 100644 index 0000000000000..965613c114e3a --- /dev/null +++ b/src/test/ui/box/thin_drop.rs @@ -0,0 +1,37 @@ +#![feature(thin_box)] +// run-pass +use std::boxed::ThinBox; +use std::error::Error; +use std::ops::Deref; +use std::fmt; + +fn main() { + let expected = "Foo error!"; + let mut dropped = false; + { + let foo = Foo(expected, &mut dropped); + let a: ThinBox = ThinBox::new_unsize(foo); + let a = a.deref(); + let msg = a.to_string(); + assert_eq!(expected, msg); + } + assert!(dropped); +} + +#[derive(Debug)] +#[repr(align(1024))] +struct Foo<'a>(&'static str, &'a mut bool); + +impl Drop for Foo<'_> { + fn drop(&mut self) { + *self.1 = true; + } +} + +impl fmt::Display for Foo<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl Error for Foo<'_> {} diff --git a/src/test/ui/box/thin_new.rs b/src/test/ui/box/thin_new.rs new file mode 100644 index 0000000000000..53f46478be403 --- /dev/null +++ b/src/test/ui/box/thin_new.rs @@ -0,0 +1,30 @@ +#![feature(thin_box)] +// run-pass +use std::boxed::ThinBox; +use std::error::Error; +use std::{fmt, mem}; + +fn main() { + let thin_error: ThinBox = ThinBox::new_unsize(Foo); + assert_eq!(mem::size_of::<*const i32>(), mem::size_of_val(&thin_error)); + println!("{:?}", thin_error); + + let thin = ThinBox::new(42i32); + assert_eq!(mem::size_of::<*const i32>(), mem::size_of_val(&thin)); + println!("{:?}", thin); + + let thin_slice = ThinBox::<[i32]>::new_unsize([1, 2, 3, 4]); + assert_eq!(mem::size_of::<*const i32>(), mem::size_of_val(&thin_slice)); + println!("{:?}", thin_slice); +} + +#[derive(Debug)] +struct Foo; + +impl fmt::Display for Foo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "boooo!") + } +} + +impl Error for Foo {} diff --git a/src/test/ui/box/thin_zst.rs b/src/test/ui/box/thin_zst.rs new file mode 100644 index 0000000000000..77c400d17bbe5 --- /dev/null +++ b/src/test/ui/box/thin_zst.rs @@ -0,0 +1,34 @@ +#![feature(thin_box)] +// run-pass +use std::boxed::ThinBox; +use std::error::Error; +use std::{fmt, mem}; +use std::ops::DerefMut; + +const EXPECTED: &str = "boooo!"; + +fn main() { + let thin_error: ThinBox = ThinBox::new_unsize(Foo); + assert_eq!(mem::size_of::<*const i32>(), mem::size_of_val(&thin_error)); + let msg = thin_error.to_string(); + assert_eq!(EXPECTED, msg); + + let mut thin_concrete_error: ThinBox = ThinBox::new(Foo); + assert_eq!(mem::size_of::<*const i32>(), mem::size_of_val(&thin_concrete_error)); + let msg = thin_concrete_error.to_string(); + assert_eq!(EXPECTED, msg); + let inner = thin_concrete_error.deref_mut(); + let msg = inner.to_string(); + assert_eq!(EXPECTED, msg); +} + +#[derive(Debug)] +struct Foo; + +impl fmt::Display for Foo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", EXPECTED) + } +} + +impl Error for Foo {} diff --git a/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr b/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr index a4b10a4c339f9..1b1ce67cb0c1f 100644 --- a/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr +++ b/src/test/ui/coercion/coerce-issue-49593-box-never.nofallback.stderr @@ -13,7 +13,7 @@ LL | /* *mut $0 is coerced to Box here */ Box::<_ /* ! */>::new(x BorrowError BorrowMutError Box - and 42 others + and 43 others = note: required for the cast to the object type `dyn std::error::Error` error[E0277]: the trait bound `(): std::error::Error` is not satisfied @@ -31,7 +31,7 @@ LL | /* *mut $0 is coerced to *mut Error here */ raw_ptr_box::<_ /* ! */>(x) BorrowError BorrowMutError Box - and 42 others + and 43 others = note: required for the cast to the object type `(dyn std::error::Error + 'static)` error: aborting due to 2 previous errors