diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 473f01660bdb4..d24c6aa617e8a 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -923,7 +923,7 @@ impl Box<[T]> { #[stable(feature = "new_uninit", since = "1.82.0")] #[must_use] pub fn new_uninit_slice(len: usize) -> Box<[mem::MaybeUninit]> { - // ignore-tidy-undocumented-unsafe + // SAFETY: `len` is exactly the capacity of this `RawVec`. unsafe { RawVec::with_capacity(len).into_box(len) } } @@ -947,7 +947,7 @@ impl Box<[T]> { #[stable(feature = "new_zeroed_alloc", since = "1.92.0")] #[must_use] pub fn new_zeroed_slice(len: usize) -> Box<[mem::MaybeUninit]> { - // ignore-tidy-undocumented-unsafe + // SAFETY: `len` is exactly the capacity of this `RawVec`. unsafe { RawVec::with_capacity_zeroed(len).into_box(len) } } @@ -981,7 +981,10 @@ impl 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)) } } @@ -1016,7 +1019,10 @@ impl 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)) } } } @@ -1044,7 +1050,7 @@ impl Box<[T], A> { #[unstable(feature = "allocator_api", issue = "32838")] #[must_use] pub fn new_uninit_slice_in(len: usize, alloc: A) -> Box<[mem::MaybeUninit], A> { - // ignore-tidy-undocumented-unsafe + // SAFETY: `len` is exactly the capacity of this `RawVec`. unsafe { RawVec::with_capacity_in(len, alloc).into_box(len) } } @@ -1072,7 +1078,7 @@ impl Box<[T], A> { #[unstable(feature = "allocator_api", issue = "32838")] #[must_use] pub fn new_zeroed_slice_in(len: usize, alloc: A) -> Box<[mem::MaybeUninit], 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) } } @@ -1111,7 +1117,10 @@ impl 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)) } } @@ -1151,7 +1160,10 @@ impl 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)) } } @@ -2013,10 +2025,15 @@ unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> Drop for Box { 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); } } diff --git a/library/alloc/src/boxed/thin.rs b/library/alloc/src/boxed/thin.rs index bef24fa822e6b..7d08991659787 100644 --- a/library/alloc/src/boxed/thin.rs +++ b/library/alloc/src/boxed/thin.rs @@ -167,7 +167,7 @@ impl Drop for ThinBox { 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::(value); } @@ -249,7 +249,7 @@ impl WithHeader { 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); @@ -265,7 +265,8 @@ impl WithHeader { 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); @@ -291,7 +292,7 @@ impl WithHeader { 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); @@ -308,7 +309,8 @@ impl WithHeader { 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); @@ -368,9 +370,10 @@ impl WithHeader { 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(&self, value: *mut T) { struct DropGuard { ptr: NonNull, diff --git a/library/alloc/src/raw_vec/mod.rs b/library/alloc/src/raw_vec/mod.rs index 250c666c70827..ffc92056cf464 100644 --- a/library/alloc/src/raw_vec/mod.rs +++ b/library/alloc/src/raw_vec/mod.rs @@ -245,11 +245,17 @@ impl RawVec { ); let me = ManuallyDrop::new(self); - // ignore-tidy-undocumented-unsafe - unsafe { - let slice = me.ptr().cast::>().cast_slice(len); - Box::from_raw_in(slice, ptr::read(&me.inner.alloc)) - } + let slice = me.ptr().cast::>().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)) } } /// Reconstitutes a `RawVec` from a pointer, capacity, and allocator. @@ -438,7 +444,7 @@ const impl RawVecInner { 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)); @@ -482,7 +488,8 @@ const impl RawVecInner { // here should change to `ptr.len() / size_of::()`. 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, }) @@ -554,7 +561,7 @@ const impl RawVecInner { ) -> Result, 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()); @@ -644,7 +651,7 @@ impl RawVecInner { // 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()); @@ -678,7 +685,8 @@ impl RawVecInner { } 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); } @@ -701,7 +709,7 @@ impl RawVecInner { 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)); @@ -737,7 +745,7 @@ impl RawVecInner { 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)); @@ -838,7 +846,8 @@ impl RawVecInner { /// 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, @@ -853,17 +862,20 @@ impl RawVecInner { // 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 diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index 47ed22c156515..4741fe12ae89c 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -481,7 +481,10 @@ impl [T] { pub const fn into_vec(self: Box) -> Vec { 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) } } @@ -530,17 +533,24 @@ impl [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. + // 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::( 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; } @@ -551,7 +561,14 @@ impl [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::(