Skip to content

Commit

Permalink
Do not allocate for ZST ThinBox attempt 2 (using const_allocate)
Browse files Browse the repository at this point in the history
There's PR rust-lang#123184
which avoids allocation for ZST ThinBox.

That PR has an issue with unsoundness with misuse of `MaybeUninit`
(see comments in that PR).

This PR is much simpler implementation which does not have this
problem, but it uses `const_allocate` feature.
  • Loading branch information
stepancheg committed Mar 31, 2024
1 parent 5da1a1b commit 34e0d9a
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 13 deletions.
105 changes: 92 additions & 13 deletions library/alloc/src/boxed/thin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
use crate::alloc::{self, Layout, LayoutError};
use core::error::Error;
use core::fmt::{self, Debug, Display, Formatter};
use core::intrinsics::const_allocate;
use core::marker::PhantomData;
#[cfg(not(no_global_oom_handling))]
use core::marker::Unsize;
use core::mem::{self, SizedTypeProperties};
use core::mem;
#[cfg(not(no_global_oom_handling))]
use core::mem::{MaybeUninit, SizedTypeProperties};
use core::ops::{Deref, DerefMut};
use core::ptr::Pointee;
use core::ptr::{self, NonNull};
Expand Down Expand Up @@ -91,6 +94,79 @@ impl<T> ThinBox<T> {

#[unstable(feature = "thin_box", issue = "92791")]
impl<Dyn: ?Sized> ThinBox<Dyn> {
#[cfg(not(no_global_oom_handling))]
fn new_unsize_zst<T>(value: T) -> Self
where
T: Unsize<Dyn>,
{
assert!(mem::size_of::<T>() == 0);

// This is a helper struct to allocate the header with proper alignment:
// Header **end** must be aligned to the value.
// We insert the padding not after the `header` field, but before the `header` field.
// Allocation layout is be:
// ```
// [ padding | metadata | ZST value ]
// ```
#[repr(C)]
struct DynZstAlloc<T, Dyn: ?Sized> {
header: MaybeUninit<<Dyn as Pointee>::Metadata>,
value: MaybeUninit<T>,
}

impl<'a, T: 'a, Dyn: ?Sized + 'a> DynZstAlloc<T, Dyn>
where
T: Unsize<Dyn>,
{
const fn value_offset() -> usize {
mem::offset_of!(DynZstAlloc<T, Dyn>, value)
}

const ALLOC: &'a DynZstAlloc<T, Dyn> = {
// SAFETY: this is compile time evaluation.
unsafe {
let size = mem::size_of::<DynZstAlloc<T, Dyn>>();
let alloc: *mut u8 =
const_allocate(size, mem::align_of::<DynZstAlloc<T, Dyn>>());

// Zerofill to be safe.
ptr::write_bytes(alloc, 0, size);

// Here we pick proper placement of the metadata,
// which is not where the metadata field is declared.
let metadata_ptr = alloc.add(
Self::value_offset()
.checked_sub(mem::size_of::<<Dyn as Pointee>::Metadata>())
.unwrap(),
);
ptr::write(
metadata_ptr as *mut <Dyn as Pointee>::Metadata,
ptr::metadata::<Dyn>(ptr::dangling::<T>() as *const Dyn),
);

&*(alloc as *const DynZstAlloc<T, Dyn>)
}
};
}

let alloc: &DynZstAlloc<T, Dyn> = DynZstAlloc::<T, Dyn>::ALLOC;

let value_offset = DynZstAlloc::<T, Dyn>::value_offset();

let ptr = WithOpaqueHeader(
NonNull::new(
// SAFETY: there's no overflow here because we add field offset.
unsafe {
(alloc as *const DynZstAlloc<T, Dyn> as *mut u8).add(value_offset) as *mut _
},
)
.unwrap(),
);
// Forget the value to avoid double drop.
mem::forget(value);
ThinBox::<Dyn> { ptr, _marker: PhantomData }
}

/// Moves a type to the heap with its [`Metadata`] stored in the heap allocation instead of on
/// the stack.
///
Expand All @@ -109,9 +185,13 @@ impl<Dyn: ?Sized> ThinBox<Dyn> {
where
T: Unsize<Dyn>,
{
let meta = ptr::metadata(&value as &Dyn);
let ptr = WithOpaqueHeader::new(meta, value);
ThinBox { ptr, _marker: PhantomData }
if mem::size_of::<T>() == 0 {
Self::new_unsize_zst(value)
} else {
let meta = ptr::metadata(&value as &Dyn);
let ptr = WithOpaqueHeader::new(meta, value);
ThinBox { ptr, _marker: PhantomData }
}
}
}

Expand Down Expand Up @@ -300,20 +380,19 @@ impl<H> WithHeader<H> {

impl<H> Drop for DropGuard<H> {
fn drop(&mut self) {
// All ZST are allocated statically.
if self.value_layout.size() == 0 {
return;
}

unsafe {
// SAFETY: Layout must have been computable if we're in drop
let (layout, value_offset) =
WithHeader::<H>::alloc_layout(self.value_layout).unwrap_unchecked();

// Note: Don't deallocate if the layout size is zero, because the pointer
// didn't come from the allocator.
if layout.size() != 0 {
alloc::dealloc(self.ptr.as_ptr().sub(value_offset), layout);
} else {
debug_assert!(
value_offset == 0 && H::IS_ZST && self.value_layout.size() == 0
);
}
// Since we only allocate for non-ZSTs, the layout size cannot be zero.
debug_assert!(layout.size() != 0);
alloc::dealloc(self.ptr.as_ptr().sub(value_offset), layout);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,10 @@
#![feature(const_box)]
#![feature(const_cow_is_borrowed)]
#![feature(const_eval_select)]
#![feature(const_heap)]
#![feature(const_maybe_uninit_as_mut_ptr)]
#![feature(const_maybe_uninit_write)]
#![feature(const_option)]
#![feature(const_pin)]
#![feature(const_refs_to_cell)]
#![feature(const_size_of_val)]
Expand Down

0 comments on commit 34e0d9a

Please sign in to comment.