Skip to content

Commit 6e29f87

Browse files
committed
Do not allocate for ZST ThinBox attempt 2 (using const_allocate)
There's PR #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 5da1a1b commit 6e29f87

File tree

2 files changed

+95
-13
lines changed

2 files changed

+95
-13
lines changed

library/alloc/src/boxed/thin.rs

+93-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::{MaybeUninit, SizedTypeProperties};
1115
use core::ops::{Deref, DerefMut};
1216
use core::ptr::Pointee;
1317
use core::ptr::{self, NonNull};
@@ -91,6 +95,79 @@ 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+
// This is a helper struct to allocate the header with proper alignment:
106+
// Header **end** must be aligned to the value.
107+
// We insert the padding not after the `header` field, but before the `header` field.
108+
// Allocation layout is be:
109+
// ```
110+
// [ padding | metadata | ZST value ]
111+
// ```
112+
#[repr(C)]
113+
struct DynZstAlloc<T, Dyn: ?Sized> {
114+
header: MaybeUninit<<Dyn as Pointee>::Metadata>,
115+
value: MaybeUninit<T>,
116+
}
117+
118+
impl<'a, T: 'a, Dyn: ?Sized + 'a> DynZstAlloc<T, Dyn>
119+
where
120+
T: Unsize<Dyn>,
121+
{
122+
const fn value_offset() -> usize {
123+
mem::offset_of!(DynZstAlloc<T, Dyn>, value)
124+
}
125+
126+
const ALLOC: &'a DynZstAlloc<T, Dyn> = {
127+
// SAFETY: this is compile time evaluation.
128+
unsafe {
129+
let size = mem::size_of::<DynZstAlloc<T, Dyn>>();
130+
let alloc: *mut u8 =
131+
const_allocate(size, mem::align_of::<DynZstAlloc<T, Dyn>>());
132+
133+
// Zerofill to be safe.
134+
ptr::write_bytes(alloc, 0, size);
135+
136+
// Here we pick proper placement of the metadata,
137+
// which is not where the metadata field is declared.
138+
let metadata_ptr = alloc.add(
139+
Self::value_offset()
140+
.checked_sub(mem::size_of::<<Dyn as Pointee>::Metadata>())
141+
.unwrap(),
142+
);
143+
ptr::write(
144+
metadata_ptr as *mut <Dyn as Pointee>::Metadata,
145+
ptr::metadata::<Dyn>(ptr::dangling::<T>() as *const Dyn),
146+
);
147+
148+
&*(alloc as *const DynZstAlloc<T, Dyn>)
149+
}
150+
};
151+
}
152+
153+
let alloc: &DynZstAlloc<T, Dyn> = DynZstAlloc::<T, Dyn>::ALLOC;
154+
155+
let value_offset = DynZstAlloc::<T, Dyn>::value_offset();
156+
157+
let ptr = WithOpaqueHeader(
158+
NonNull::new(
159+
// SAFETY: there's no overflow here because we add field offset.
160+
unsafe {
161+
(alloc as *const DynZstAlloc<T, Dyn> as *mut u8).add(value_offset) as *mut _
162+
},
163+
)
164+
.unwrap(),
165+
);
166+
// Forget the value to avoid double drop.
167+
mem::forget(value);
168+
ThinBox::<Dyn> { ptr, _marker: PhantomData }
169+
}
170+
94171
/// Moves a type to the heap with its [`Metadata`] stored in the heap allocation instead of on
95172
/// the stack.
96173
///
@@ -109,9 +186,13 @@ impl<Dyn: ?Sized> ThinBox<Dyn> {
109186
where
110187
T: Unsize<Dyn>,
111188
{
112-
let meta = ptr::metadata(&value as &Dyn);
113-
let ptr = WithOpaqueHeader::new(meta, value);
114-
ThinBox { ptr, _marker: PhantomData }
189+
if mem::size_of::<T>() == 0 {
190+
Self::new_unsize_zst(value)
191+
} else {
192+
let meta = ptr::metadata(&value as &Dyn);
193+
let ptr = WithOpaqueHeader::new(meta, value);
194+
ThinBox { ptr, _marker: PhantomData }
195+
}
115196
}
116197
}
117198

@@ -300,20 +381,19 @@ impl<H> WithHeader<H> {
300381

301382
impl<H> Drop for DropGuard<H> {
302383
fn drop(&mut self) {
384+
// All ZST are allocated statically.
385+
if self.value_layout.size() == 0 {
386+
return;
387+
}
388+
303389
unsafe {
304390
// SAFETY: Layout must have been computable if we're in drop
305391
let (layout, value_offset) =
306392
WithHeader::<H>::alloc_layout(self.value_layout).unwrap_unchecked();
307393

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-
}
394+
// Since we only allocate for non-ZSTs, the layout size cannot be zero.
395+
debug_assert!(layout.size() != 0);
396+
alloc::dealloc(self.ptr.as_ptr().sub(value_offset), layout);
317397
}
318398
}
319399
}

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)