Skip to content

alloc: a bunch of safety comments #162289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nia-e wants to merge 3 commits into
base: main
Choose a base branch
from
+93 −44
Open

alloc: a bunch of safety comments #162289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1a9fca
alloc: a bunch of safety comments
nia-e Sep 4, 2026
1c48bd6
review
nia-e Sep 4, 2026
e578090
typo
nia-e Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 29 additions & 12 deletions library/alloc/src/boxed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,7 @@ impl<T> Box<[T]> {
#[stable(feature = "new_uninit", since = "1.82.0")]
#[must_use]
pub fn new_uninit_slice(len: usize) -> Box<[mem::MaybeUninit<T>]> {
// ignore-tidy-undocumented-unsafe
// SAFETY: `len` is exactly the capacity of this `RawVec`.
unsafe { RawVec::with_capacity(len).into_box(len) }
}

Expand All @@ -947,7 +947,7 @@ impl<T> Box<[T]> {
#[stable(feature = "new_zeroed_alloc", since = "1.92.0")]
#[must_use]
pub fn new_zeroed_slice(len: usize) -> Box<[mem::MaybeUninit<T>]> {
// ignore-tidy-undocumented-unsafe
// SAFETY: `len` is exactly the capacity of this `RawVec`.
unsafe { RawVec::with_capacity_zeroed(len).into_box(len) }
}

Expand Down Expand Up @@ -981,7 +981,10 @@ impl<T> Box<[T]> {
};
Global.allocate(layout)?.cast()
};
// ignore-tidy-undocumented-unsafe
// SAFETY: `ptr` was just allocated with `Global` with the layout for an array of length
// `len`, and the layout creation would have failed if `len` overflowed an isize.
// `into_box` is sound to call since `len` corresponds to the length of the just-created
// `RawVec`.
unsafe { Ok(RawVec::from_raw_parts_in(ptr.as_ptr(), len, Global).into_box(len)) }
}

Expand Down Expand Up @@ -1016,7 +1019,10 @@ impl<T> Box<[T]> {
};
Global.allocate_zeroed(layout)?.cast()
};
// ignore-tidy-undocumented-unsafe
// SAFETY: `ptr` was just allocated with `Global` with the layout for an array of length
// `len`, and the layout creation would have failed if `len` overflowed an isize.
// `into_box` is sound to call since `len` corresponds to the length of the just-created
// `RawVec`.
unsafe { Ok(RawVec::from_raw_parts_in(ptr.as_ptr(), len, Global).into_box(len)) }
}
}
Expand Down Expand Up @@ -1044,7 +1050,7 @@ impl<T, A: Allocator> Box<[T], A> {
#[unstable(feature = "allocator_api", issue = "32838")]
#[must_use]
pub fn new_uninit_slice_in(len: usize, alloc: A) -> Box<[mem::MaybeUninit<T>], A> {
// ignore-tidy-undocumented-unsafe
// SAFETY: `len` is exactly the capacity of this `RawVec`.
unsafe { RawVec::with_capacity_in(len, alloc).into_box(len) }
}

Expand Down Expand Up @@ -1072,7 +1078,7 @@ impl<T, A: Allocator> Box<[T], A> {
#[unstable(feature = "allocator_api", issue = "32838")]
#[must_use]
pub fn new_zeroed_slice_in(len: usize, alloc: A) -> Box<[mem::MaybeUninit<T>], A> {
// ignore-tidy-undocumented-unsafe
// SAFETY: `len` is exactly the capacity of this `RawVec`.
unsafe { RawVec::with_capacity_zeroed_in(len, alloc).into_box(len) }
}

Expand Down Expand Up @@ -1111,7 +1117,10 @@ impl<T, A: Allocator> Box<[T], A> {
};
alloc.allocate(layout)?.cast()
};
// ignore-tidy-undocumented-unsafe
// SAFETY: `ptr` was just allocated with `alloc` with the layout for an array of length
// `len`, and the layout creation would have failed if `len` overflowed an isize.
// `into_box` is sound to call since `len` corresponds to the length of the just-created
// `RawVec`.
unsafe { Ok(RawVec::from_raw_parts_in(ptr.as_ptr(), len, alloc).into_box(len)) }
}

Expand Down Expand Up @@ -1151,7 +1160,10 @@ impl<T, A: Allocator> Box<[T], A> {
};
alloc.allocate_zeroed(layout)?.cast()
};
// ignore-tidy-undocumented-unsafe
// SAFETY: `ptr` was just allocated with `alloc` with the layout for an array of length
// `len`, and the layout creation would have failed if `len` overflowed an isize.
// `into_box` is sound to call since `len` corresponds to the length of the just-created
// `RawVec`.
unsafe { Ok(RawVec::from_raw_parts_in(ptr.as_ptr(), len, alloc).into_box(len)) }
}

Expand Down Expand Up @@ -2013,10 +2025,15 @@ unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> Drop for Box<T, A> {

let ptr = self.0;

// ignore-tidy-undocumented-unsafe
unsafe {
let layout = Layout::for_value_raw(ptr.as_ptr());
if layout.size() != 0 {
// SAFETY: The construction site of the unsized box had ensured for us that the
// allocation was made with a valid layout (the size does not overflow an isize,
// possibly because the size of the type is 0).
let layout = unsafe { Layout::for_value_raw(ptr.as_ptr()) };
if layout.size() != 0 {
// SAFETY: Any nonzero allocation would have been created with the allocator
// of this box and `layout` would fit that allocation. We also are the only ones
// responsible for doing this deallocation and know that the pointer must be valid.
unsafe {
self.1.deallocate(From::from(ptr.cast()), layout);
}
}
Expand Down
19 changes: 11 additions & 8 deletions library/alloc/src/boxed/thin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl<T: ?Sized> Drop for ThinBox<T> {
fn drop(&mut self) {
let value = self.deref_mut();
let value = value as *mut T;
// ignore-tidy-undocumented-unsafe
// SAFETY: `value` is valid for reads and writes for our `T`.
unsafe {
self.with_header().drop::<T>(value);
}
Expand Down Expand Up @@ -249,7 +249,7 @@ impl<H> WithHeader<H> {
debug_assert!(value_offset == 0 && T::IS_ZST && H::IS_ZST);
layout.dangling_ptr()
} else {
// ignore-tidy-undocumented-unsafe
// SAFETY: We check above that the layout size is nonzero.
let ptr = unsafe { alloc::alloc(layout) };
if ptr.is_null() {
alloc::handle_alloc_error(layout);
Expand All @@ -265,7 +265,8 @@ impl<H> WithHeader<H> {

let result = WithHeader(ptr, PhantomData);

// ignore-tidy-undocumented-unsafe
// SAFETY: `result.header()` promises to give us a valid place for writing
// the header, and `result.value()` promises the same for the value.
unsafe {
ptr::write(result.header(), header);
ptr::write(result.value().cast(), value);
Expand All @@ -291,7 +292,7 @@ impl<H> WithHeader<H> {
debug_assert!(value_offset == 0 && T::IS_ZST && H::IS_ZST);
layout.dangling_ptr()
} else {
// ignore-tidy-undocumented-unsafe
// SAFETY: We check above that the layout size is nonzero.
let ptr = unsafe { alloc::alloc(layout) };
if ptr.is_null() {
return Err(core::alloc::AllocError);
Expand All @@ -308,7 +309,8 @@ impl<H> WithHeader<H> {

let result = WithHeader(ptr, PhantomData);

// ignore-tidy-undocumented-unsafe
// SAFETY: `result.header()` promises to give us a valid place for writing
// the header, and `result.value()` promises the same for the value.
unsafe {
ptr::write(result.header(), header);
ptr::write(result.value().cast(), value);
Expand Down Expand Up @@ -368,9 +370,10 @@ impl<H> WithHeader<H> {
WithHeader(NonNull::new(value_ptr.cast()).unwrap(), PhantomData)
}

// Safety:
// - Assumes that either `value` can be dereferenced, or is the
// `NonNull::dangling()` we use when both `T` and `H` are ZSTs.
/// # Safety
///
/// `value` must point to an undropped owned `T`, and `self` must not be
/// accessed again after this is called.
unsafe fn drop<T: ?Sized>(&self, value: *mut T) {
struct DropGuard<H> {
ptr: NonNull<u8>,
Expand Down
48 changes: 30 additions & 18 deletions library/alloc/src/raw_vec/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,17 @@ impl<T, A: Allocator> RawVec<T, A> {
);

let me = ManuallyDrop::new(self);
// ignore-tidy-undocumented-unsafe
unsafe {
let slice = me.ptr().cast::<MaybeUninit<T>>().cast_slice(len);
Box::from_raw_in(slice, ptr::read(&me.inner.alloc))
}
let slice = me.ptr().cast::<MaybeUninit<T>>().cast_slice(len);
// SAFETY: `slice` is a valid pointer for `len` `T`s, and the
// above `ManuallyDrop` ensures that the destructor of `me` which
// would free the allocation is never run. The caller upholds that
// `len` meets or exceeds the last requested capacity, ensuring that
// the layout generated when dropping the resulting `Box` fits the
// allocation the `RawVec` created.
//
// Moving the allocator out of `me.inner` is also sound since it is
// never accessed after this point.
unsafe { Box::from_raw_in(slice, ptr::read(&me.inner.alloc)) }
Comment on lines +249 to +258

@Darksonn Darksonn Sep 4, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should address why this creates a valid Box. In particular, if I call this and then drop the resulting Box, why does the destructor of Box not violate the safety requirements of dealloc, in particular the requirements about the length being the same as what it was allocated with?

View changes since the review

}

/// Reconstitutes a `RawVec` from a pointer, capacity, and allocator.
Expand Down Expand Up @@ -438,7 +444,7 @@ const impl<A: [const] Allocator + [const] Destruct> RawVecInner<A> {
fn with_capacity_in(capacity: usize, alloc: A, elem_layout: Layout) -> Self {
match Self::try_allocate_in(capacity, AllocInit::Uninitialized, alloc, elem_layout) {
Ok(this) => {
// ignore-tidy-undocumented-unsafe
// SAFETY: We already allocated at least `capacity`.
unsafe {
// Make it more obvious that a subsequent Vec::reserve(capacity) will not allocate.
hint::assert_unchecked(!this.needs_to_grow(0, capacity, elem_layout));
Expand Down Expand Up @@ -482,7 +488,8 @@ const impl<A: [const] Allocator + [const] Destruct> RawVecInner<A> {
// here should change to `ptr.len() / size_of::<T>()`.
Ok(Self {
ptr: Unique::from(ptr.cast()),
// ignore-tidy-undocumented-unsafe
// SAFETY: We return early if `T` is a ZST, and if `capacity` would
// overflow an isize layout creation would have returned early as well.
cap: unsafe { Cap::new_unchecked(capacity) },
alloc,
})
Expand Down Expand Up @@ -554,7 +561,7 @@ const impl<A: [const] Allocator + [const] Destruct> RawVecInner<A> {
) -> Result<NonNull<[u8]>, TryReserveError> {
let new_layout = layout_array(cap, elem_layout)?;

// ignore-tidy-undocumented-unsafe
// SAFETY: Upheld by caller.
let memory = if let Some((ptr, old_layout)) = unsafe { self.current_memory(elem_layout) } {
// FIXME(const-hack): switch to `debug_assert_eq`
debug_assert!(old_layout.align() == new_layout.align());
Expand Down Expand Up @@ -644,7 +651,7 @@ impl<A: Allocator> RawVecInner<A> {
// and could hypothetically handle differences between stride and size, but this memory
// has already been allocated so we know it can't overflow and currently Rust does not
// support such types. So we can do better by skipping some checks and avoid an unwrap.
// ignore-tidy-undocumented-unsafe
// SAFETY: Upheld by caller, unless the element size is 0 which is checked against.
unsafe {
let alloc_size = elem_layout.size().unchecked_mul(self.cap.as_inner());
let layout = Layout::from_size_align_unchecked(alloc_size, elem_layout.align());
Expand Down Expand Up @@ -678,7 +685,8 @@ impl<A: Allocator> RawVecInner<A> {
}

if self.needs_to_grow(len, additional, elem_layout) {
// ignore-tidy-undocumented-unsafe
// SAFETY: `needs_to_grow` ensures that `len + additional` is greater than
// the current capacity, with the other preconditions upheld by our caller.
unsafe {
do_reserve_and_handle(self, len, additional, elem_layout);
}
Expand All @@ -701,7 +709,7 @@ impl<A: Allocator> RawVecInner<A> {
self.grow_amortized(len, additional, elem_layout)?;
}
}
// ignore-tidy-undocumented-unsafe
// SAFETY: If we've already grown, we will not need to again immediately after.
unsafe {
// Inform the optimizer that the reservation has succeeded or wasn't needed
hint::assert_unchecked(!self.needs_to_grow(len, additional, elem_layout));
Expand Down Expand Up @@ -737,7 +745,7 @@ impl<A: Allocator> RawVecInner<A> {
self.grow_exact(len, additional, elem_layout)?;
}
}
// ignore-tidy-undocumented-unsafe
// SAFETY: If we've already grown, we will not need to again immediately after.
unsafe {
// Inform the optimizer that the reservation has succeeded or wasn't needed
hint::assert_unchecked(!self.needs_to_grow(len, additional, elem_layout));
Expand Down Expand Up @@ -838,7 +846,8 @@ impl<A: Allocator> RawVecInner<A> {
/// big for LLVM to be willing to inline.
///
/// # Safety
/// `cap <= self.capacity()`
/// - `cap <= self.capacity()`
/// - `elem_layout` must be valid for `self`.
unsafe fn shrink_unchecked(
&mut self,
cap: usize,
Expand All @@ -853,17 +862,20 @@ impl<A: Allocator> RawVecInner<A> {
// for the T::IS_ZST case since current_memory() will have returned
// None.
if cap == 0 {
// ignore-tidy-undocumented-unsafe
// SAFETY: T isn't a ZST if we're here and `ptr` is our pointer that `current_memory`
// ensures was allocated with `layout`.
unsafe { self.alloc.deallocate(ptr, layout) };
self.ptr =
// ignore-tidy-undocumented-unsafe
// SAFETY: Alignment is guaranteed to be nonzero.
unsafe { Unique::new_unchecked(ptr::without_provenance_mut(elem_layout.align())) };
self.cap = ZERO_CAP;
} else {
// ignore-tidy-undocumented-unsafe
// SAFETY: `cap` is less than the previous capacity, which must have fit in an
// isize already for the non-ZST case. `shrink` is also sound to call since
// `current_memory` ensures `ptr` and `layout` are correct for the old allocation,
// while `new_layout` is computed with a smaller size than the old one per the
// requirement we instate on our callers.
let ptr = unsafe {
// Layout cannot overflow here because it would have
// overflowed earlier when capacity was larger.
let new_size = elem_layout.size().unchecked_mul(cap);
let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
self.alloc
Expand Down
29 changes: 23 additions & 6 deletions library/alloc/src/slice.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,10 @@ impl<T> [T] {
pub const fn into_vec<A: Allocator>(self: Box<Self, A>) -> Vec<T, A> {
let len = self.len();
let (b, alloc) = Box::into_raw_with_allocator(self);
// ignore-tidy-undocumented-unsafe
// SAFETY: `b` is currently allocated with `alloc` and was allocated with the
// matching layout for an array of `T * len`, the length is equal to the capacity,
// and the existence of a `Box<[T]>` is proof that the first `len` elements are
// valid `T`s.
unsafe { Vec::from_raw_parts_in(b as *mut T, len, len, alloc) }
}

Expand Down Expand Up @@ -530,17 +533,24 @@ impl<T> [T] {
// If `m > 0`, there are remaining bits up to the leftmost '1'.
while m > 0 {
// `buf.extend(buf)`:
// ignore-tidy-undocumented-unsafe
// SAFETY: We're copying `len` elements after offsetting by `len`,
// with the previous call to `extend` ensuring that the first `len`
// elements are valid `T`s and the call to `with_capacity` ensuring
// we have `len * n` space to write the new elements.
Comment on lines +536 to +539

@Darksonn Darksonn Sep 4, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must argue that it's in-bounds of the allocation and that there's no overlap.

It's clear that there is no overlap, but this doesn't really explain why 2*buf.len() <= capacity.

No need to explain that the elements are valid because copy_nonoverlapping performs an untyped copy, so validity is not required. Explaining it on the set_len call is sufficient.

View changes since the review

// Each iteration of this loop doubles the number of initialised elements,
// which is tracked via `m` - when `m == 0`, we've written `most_significant_bit(n)`
// elements to the buffer.
unsafe {
ptr::copy_nonoverlapping::<T>(
buf.as_ptr(),
(buf.as_mut_ptr()).add(buf.len()),
buf.len(),
);
// `buf` has capacity of `self.len() * n`.
let buf_len = buf.len();
buf.set_len(buf_len * 2);
}
// `buf` has capacity of `self.len() * n`.
let buf_len = buf.len();
// SAFETY: We initialised another `buf_len` elements above.
unsafe { buf.set_len(buf_len * 2) };

m >>= 1;
}
Expand All @@ -551,7 +561,14 @@ impl<T> [T] {
let rem_len = capacity - buf.len(); // `self.len() * rem`
if rem_len > 0 {
// `buf.extend(buf[0 .. rem_len])`:
// ignore-tidy-undocumented-unsafe
// SAFETY: We're copying `rem_len` elements after offsetting by `len`. The previous
// looping `copy_nonoverlapping` always doubled the number of instantiated elements,
// and so if `rem_len` was greater than `len` it would have allowed for another such
// doubling, until such time that `rem_len < len`. Thus, the space for these remaining
// `rem_len` elements must be preceded by more than `rem_len` previously-copied
// elements.
// Setting the length is correct since we've initialised the whole `capacity`-length
// space with copies of the previous `len` elements.
unsafe {
// This is non-overlapping since `2^expn > rem`.
ptr::copy_nonoverlapping::<T>(
Expand Down
Loading