Skip to content

Commit 8caa27d

Browse files
committed
Do not allocate for ZST ThinBox attempt 2 (using const_allocate)
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.
1 parent 688c30d commit 8caa27d

File tree

2 files changed

+90
-13
lines changed

2 files changed

+90
-13
lines changed

library/alloc/src/boxed/thin.rs

+88-13
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44
use crate::alloc::{self, Layout, LayoutError};
55
use core::error::Error;
66
use core::fmt::{self, Debug, Display, Formatter};
7+
#[cfg(not(no_global_oom_handling))]
8+
use core::intrinsics::const_allocate;
79
use core::marker::PhantomData;
810
#[cfg(not(no_global_oom_handling))]
911
use core::marker::Unsize;
10-
use core::mem::{self, SizedTypeProperties};
12+
use core::mem;
13+
#[cfg(not(no_global_oom_handling))]
14+
use core::mem::SizedTypeProperties;
1115
use core::ops::{Deref, DerefMut};
1216
use core::ptr::Pointee;
1317
use core::ptr::{self, NonNull};
@@ -91,6 +95,74 @@ impl<T> ThinBox<T> {
9195

9296
#[unstable(feature = "thin_box", issue = "92791")]
9397
impl<Dyn: ?Sized> ThinBox<Dyn> {
98+
#[cfg(not(no_global_oom_handling))]
99+
fn new_unsize_zst<T>(value: T) -> Self
100+
where
101+
T: Unsize<Dyn>,
102+
{
103+
assert!(mem::size_of::<T>() == 0);
104+
105+
// Utility to statically allocate metadata followed by ZST value.
106+
struct DynZstAlloc<T, Dyn: ?Sized>(PhantomData<(T, Dyn)>);
107+
108+
impl<'a, T: 'a, Dyn: ?Sized + 'a> DynZstAlloc<T, Dyn>
109+
where
110+
T: Unsize<Dyn>,
111+
{
112+
// We are returning `[T; 0]` here instead of `T` because
113+
// compiler instantiates this code even for non-ZST,
114+
// and reports a pointer going beyond the allocation.
115+
const ALLOC: &'a [T; 0] = {
116+
const fn max(a: usize, b: usize) -> usize {
117+
if a > b { a } else { b }
118+
}
119+
120+
// ZST T pointer must be aligned to both the value and the metadata.
121+
// Metadata must be at the address `&T - size_of::<Metadata>`.
122+
// This is how we need to allocate the memory:
123+
// [ padding | metadata | ZST T value ]
124+
125+
let alloc_align =
126+
max(mem::align_of::<T>(), mem::align_of::<<Dyn as Pointee>::Metadata>());
127+
128+
let alloc_size =
129+
max(mem::align_of::<T>(), mem::size_of::<<Dyn as Pointee>::Metadata>());
130+
131+
unsafe {
132+
// SAFETY: align is power of two because it is the maximum of two alignments.
133+
let alloc: *mut u8 = const_allocate(alloc_size, alloc_align);
134+
135+
// Zerofill to be safe.
136+
// SAFETY: `alloc_size` is the size of the allocation.
137+
ptr::write_bytes(alloc, 0, alloc_size);
138+
139+
// SAFETY: adding offset within the allocation.
140+
let metadata_ptr = alloc.add(
141+
alloc_size
142+
.checked_sub(mem::size_of::<<Dyn as Pointee>::Metadata>())
143+
.unwrap(),
144+
);
145+
// SAFETY: `*metadata_ptr` is within the allocation.
146+
ptr::write(
147+
metadata_ptr as *mut <Dyn as Pointee>::Metadata,
148+
ptr::metadata::<Dyn>(ptr::dangling::<T>() as *const Dyn),
149+
);
150+
151+
// SAFETY: `alloc_size` is the size of the allocation
152+
// and the pointer we create is zero-sized.
153+
&*(alloc.add(alloc_size) as *const [T; 0])
154+
}
155+
};
156+
}
157+
158+
let ptr = WithOpaqueHeader(
159+
NonNull::new(DynZstAlloc::<T, Dyn>::ALLOC as *const _ as *mut u8).unwrap(),
160+
);
161+
// Forget the value to avoid double drop.
162+
mem::forget(value);
163+
ThinBox::<Dyn> { ptr, _marker: PhantomData }
164+
}
165+
94166
/// Moves a type to the heap with its [`Metadata`] stored in the heap allocation instead of on
95167
/// the stack.
96168
///
@@ -109,9 +181,13 @@ impl<Dyn: ?Sized> ThinBox<Dyn> {
109181
where
110182
T: Unsize<Dyn>,
111183
{
112-
let meta = ptr::metadata(&value as &Dyn);
113-
let ptr = WithOpaqueHeader::new(meta, value);
114-
ThinBox { ptr, _marker: PhantomData }
184+
if mem::size_of::<T>() == 0 {
185+
Self::new_unsize_zst(value)
186+
} else {
187+
let meta = ptr::metadata(&value as &Dyn);
188+
let ptr = WithOpaqueHeader::new(meta, value);
189+
ThinBox { ptr, _marker: PhantomData }
190+
}
115191
}
116192
}
117193

@@ -300,20 +376,19 @@ impl<H> WithHeader<H> {
300376

301377
impl<H> Drop for DropGuard<H> {
302378
fn drop(&mut self) {
379+
// All ZST are allocated statically.
380+
if self.value_layout.size() == 0 {
381+
return;
382+
}
383+
303384
unsafe {
304385
// SAFETY: Layout must have been computable if we're in drop
305386
let (layout, value_offset) =
306387
WithHeader::<H>::alloc_layout(self.value_layout).unwrap_unchecked();
307388

308-
// Note: Don't deallocate if the layout size is zero, because the pointer
309-
// didn't come from the allocator.
310-
if layout.size() != 0 {
311-
alloc::dealloc(self.ptr.as_ptr().sub(value_offset), layout);
312-
} else {
313-
debug_assert!(
314-
value_offset == 0 && H::IS_ZST && self.value_layout.size() == 0
315-
);
316-
}
389+
// Since we only allocate for non-ZSTs, the layout size cannot be zero.
390+
debug_assert!(layout.size() != 0);
391+
alloc::dealloc(self.ptr.as_ptr().sub(value_offset), layout);
317392
}
318393
}
319394
}

library/alloc/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,10 @@
114114
#![feature(const_box)]
115115
#![feature(const_cow_is_borrowed)]
116116
#![feature(const_eval_select)]
117+
#![feature(const_heap)]
117118
#![feature(const_maybe_uninit_as_mut_ptr)]
118119
#![feature(const_maybe_uninit_write)]
120+
#![feature(const_option)]
119121
#![feature(const_pin)]
120122
#![feature(const_refs_to_cell)]
121123
#![feature(const_size_of_val)]

0 commit comments

Comments
 (0)