Rollup merge of #145067 - btj:patch-3, r=tgross35

RawVecInner: add missing `unsafe` to unsafe fns
Some (module-private) functions in `library/alloc/src/raw_vec/mod.rs` are unsafe (i.e. may cause UB when called from safe code) but are not marked `unsafe`. Specifically:
- `RawVecInner::grow_exact` causes UB if called with `len` and `additional` arguments such that `len + additional` is less than the current capacity. Indeed, in that case it calls [Allocator::grow](https://doc.rust-lang.org/std/alloc/trait.Allocator.html#method.grow) with a `new_layout` that is smaller than `old_layout`, which violates a safety precondition.
- The RawVecInner methods for resizing the buffer cause UB if called with an `elem_layout` different from the one used to initially allocate the buffer, because in that case `Allocator::grow` or `Allocator::shrink` are called with an `old_layout` that does not *fit* the allocated block, which violates a safety precondition.
- `RawVecInner::current_memory` might cause UB if called with an `elem_layout` different from the one used to initially allocate the buffer, because the `unchecked_mul` might overflow.
- Furthermore, these methods cause UB if called with an `elem_layout` where the size is not a multiple of the alignment. This is because `Layout::repeat` is used (in `layout_array`) to compute the allocation's layout when allocating, which includes padding to ensure alignment of array elements, but simple multiplication is used (in `current_memory`) to compute the old allocation's layout when resizing or deallocating, which would cause the layout used to resize or deallocate to not *fit* the allocated block, which violates a safety precondition.
I discovered these issues while performing formal verification of `library/alloc/src/raw_vec/mod.rs` per [Challenge 19](https://model-checking.github.io/verify-rust-std/challenges/0019-rawvec.html) of the [AWS Rust Standard Library Verification Contest](https://aws.amazon.com/blogs/opensource/verify-the-safety-of-the-rust-standard-library/).

Author: Zalathar

library/alloc/src/raw_vec/mod.rs

@@ -177,6 +177,8 @@ impl<T, A: Allocator> RawVec<T, A> {
 /// the returned `RawVec`.
 #[inline]
 pub(crate)constfnnew_in(alloc:A) -> Self{
+// Check assumption made in `current_memory`
+const{assert!(T::LAYOUT.size() % T::LAYOUT.align() == 0)};
 Self{inner:RawVecInner::new_in(alloc,Alignment::of::<T>()),_marker:PhantomData}
 }
 
@@ -328,7 +330,8 @@ impl<T, A: Allocator> RawVec<T, A> {
 #[inline]
 #[track_caller]
 pub(crate)fnreserve(&mutself,len:usize,additional:usize){
-self.inner.reserve(len, additional,T::LAYOUT)
+// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
+unsafe{self.inner.reserve(len, additional,T::LAYOUT)}
 }
 
 /// A specialized version of `self.reserve(len, 1)` which requires the
@@ -337,7 +340,8 @@ impl<T, A: Allocator> RawVec<T, A> {
 #[inline(never)]
 #[track_caller]
 pub(crate)fngrow_one(&mutself){
-self.inner.grow_one(T::LAYOUT)
+// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
+unsafe{self.inner.grow_one(T::LAYOUT)}
 }
 
 /// The same as `reserve`, but returns on errors instead of panicking or aborting.
@@ -346,7 +350,8 @@ impl<T, A: Allocator> RawVec<T, A> {
 len:usize,
 additional:usize,
 ) -> Result<(),TryReserveError>{
-self.inner.try_reserve(len, additional,T::LAYOUT)
+// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
+unsafe{self.inner.try_reserve(len, additional,T::LAYOUT)}
 }
 
 /// Ensures that the buffer contains at least enough space to hold `len +
@@ -369,7 +374,8 @@ impl<T, A: Allocator> RawVec<T, A> {
 #[cfg(not(no_global_oom_handling))]
 #[track_caller]
 pub(crate)fnreserve_exact(&mutself,len:usize,additional:usize){
-self.inner.reserve_exact(len, additional,T::LAYOUT)
+// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
+unsafe{self.inner.reserve_exact(len, additional,T::LAYOUT)}
 }
 
 /// The same as `reserve_exact`, but returns on errors instead of panicking or aborting.
@@ -378,7 +384,8 @@ impl<T, A: Allocator> RawVec<T, A> {
 len:usize,
 additional:usize,
 ) -> Result<(),TryReserveError>{
-self.inner.try_reserve_exact(len, additional,T::LAYOUT)
+// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
+unsafe{self.inner.try_reserve_exact(len, additional,T::LAYOUT)}
 }
 
 /// Shrinks the buffer down to the specified capacity. If the given amount
@@ -395,7 +402,8 @@ impl<T, A: Allocator> RawVec<T, A> {
 #[track_caller]
 #[inline]
 pub(crate)fnshrink_to_fit(&mutself,cap:usize){
-self.inner.shrink_to_fit(cap,T::LAYOUT)
+// SAFETY: All calls on self.inner pass T::LAYOUT as the elem_layout
+unsafe{self.inner.shrink_to_fit(cap,T::LAYOUT)}
 }
 }
 
@@ -518,8 +526,12 @@ impl<A: Allocator> RawVecInner<A> {
 &self.alloc
 }
 
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
 #[inline]
-fncurrent_memory(&self,elem_layout:Layout) -> Option<(NonNull<u8>,Layout)>{
+unsafefncurrent_memory(&self,elem_layout:Layout) -> Option<(NonNull<u8>,Layout)>{
 if elem_layout.size() == 0 || self.cap.as_inner() == 0{
 None
 }else{
@@ -535,48 +547,67 @@ impl<A: Allocator> RawVecInner<A> {
 }
 }
 
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
 #[cfg(not(no_global_oom_handling))]
 #[inline]
 #[track_caller]
-fnreserve(&mutself,len:usize,additional:usize,elem_layout:Layout){
+unsafefnreserve(&mutself,len:usize,additional:usize,elem_layout:Layout){
 // Callers expect this function to be very cheap when there is already sufficient capacity.
 // Therefore, we move all the resizing and error-handling logic from grow_amortized and
 // handle_reserve behind a call, while making sure that this function is likely to be
 // inlined as just a comparison and a call if the comparison fails.
 #[cold]
-fndo_reserve_and_handle<A:Allocator>(
+unsafefndo_reserve_and_handle<A:Allocator>(
 slf:&mutRawVecInner<A>,
 len:usize,
 additional:usize,
 elem_layout:Layout,
 ){
-ifletErr(err) = slf.grow_amortized(len, additional, elem_layout){
+// SAFETY: Precondition passed to caller
+ifletErr(err) = unsafe{ slf.grow_amortized(len, additional, elem_layout)}{
 handle_error(err);
 }
 }
 
 ifself.needs_to_grow(len, additional, elem_layout){
-do_reserve_and_handle(self, len, additional, elem_layout);
+unsafe{
+do_reserve_and_handle(self, len, additional, elem_layout);
+}
 }
 }
 
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
 #[cfg(not(no_global_oom_handling))]
 #[inline]
 #[track_caller]
-fngrow_one(&mutself,elem_layout:Layout){
-ifletErr(err) = self.grow_amortized(self.cap.as_inner(),1, elem_layout){
+unsafefngrow_one(&mutself,elem_layout:Layout){
+// SAFETY: Precondition passed to caller
+ifletErr(err) = unsafe{self.grow_amortized(self.cap.as_inner(),1, elem_layout)}{
 handle_error(err);
 }
 }
 
-fntry_reserve(
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
+unsafefntry_reserve(
 &mutself,
 len:usize,
 additional:usize,
 elem_layout:Layout,
 ) -> Result<(),TryReserveError>{
 ifself.needs_to_grow(len, additional, elem_layout){
-self.grow_amortized(len, additional, elem_layout)?;
+// SAFETY: Precondition passed to caller
+unsafe{
+self.grow_amortized(len, additional, elem_layout)?;
+}
 }
 unsafe{
 // Inform the optimizer that the reservation has succeeded or wasn't needed
@@ -585,22 +616,34 @@ impl<A: Allocator> RawVecInner<A> {
 Ok(())
 }
 
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
 #[cfg(not(no_global_oom_handling))]
 #[track_caller]
-fnreserve_exact(&mutself,len:usize,additional:usize,elem_layout:Layout){
-ifletErr(err) = self.try_reserve_exact(len, additional, elem_layout){
+unsafefnreserve_exact(&mutself,len:usize,additional:usize,elem_layout:Layout){
+// SAFETY: Precondition passed to caller
+ifletErr(err) = unsafe{self.try_reserve_exact(len, additional, elem_layout)}{
 handle_error(err);
 }
 }
 
-fntry_reserve_exact(
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
+unsafefntry_reserve_exact(
 &mutself,
 len:usize,
 additional:usize,
 elem_layout:Layout,
 ) -> Result<(),TryReserveError>{
 ifself.needs_to_grow(len, additional, elem_layout){
-self.grow_exact(len, additional, elem_layout)?;
+// SAFETY: Precondition passed to caller
+unsafe{
+self.grow_exact(len, additional, elem_layout)?;
+}
 }
 unsafe{
 // Inform the optimizer that the reservation has succeeded or wasn't needed
@@ -609,11 +652,16 @@ impl<A: Allocator> RawVecInner<A> {
 Ok(())
 }
 
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
+/// - `cap` must be less than or equal to `self.capacity(elem_layout.size())`
 #[cfg(not(no_global_oom_handling))]
 #[inline]
 #[track_caller]
-fnshrink_to_fit(&mutself,cap:usize,elem_layout:Layout){
-ifletErr(err) = self.shrink(cap, elem_layout){
+unsafefnshrink_to_fit(&mutself,cap:usize,elem_layout:Layout){
+ifletErr(err) = unsafe{self.shrink(cap, elem_layout)}{
 handle_error(err);
 }
 }
@@ -632,7 +680,13 @@ impl<A: Allocator> RawVecInner<A> {
 self.cap = unsafe{Cap::new_unchecked(cap)};
 }
 
-fngrow_amortized(
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
+/// - The sum of `len` and `additional` must be greater than or equal to
+/// `self.capacity(elem_layout.size())`
+unsafefngrow_amortized(
 &mutself,
 len:usize,
 additional:usize,
@@ -657,14 +711,25 @@ impl<A: Allocator> RawVecInner<A> {
 
 let new_layout = layout_array(cap, elem_layout)?;
 
-let ptr = finish_grow(new_layout,self.current_memory(elem_layout),&mutself.alloc)?;
-// SAFETY: layout_array would have resulted in a capacity overflow if we tried to allocate more than `isize::MAX` items
+// SAFETY:
+// - For the `current_memory` call: Precondition passed to caller
+// - For the `finish_grow` call: Precondition passed to caller
+// + `current_memory` does the right thing
+let ptr =
+unsafe{finish_grow(new_layout,self.current_memory(elem_layout),&mutself.alloc)? };
 
+// SAFETY: layout_array would have resulted in a capacity overflow if we tried to allocate more than `isize::MAX` items
 unsafe{self.set_ptr_and_cap(ptr, cap)};
 Ok(())
 }
 
-fngrow_exact(
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
+/// - The sum of `len` and `additional` must be greater than or equal to
+/// `self.capacity(elem_layout.size())`
+unsafefngrow_exact(
 &mutself,
 len:usize,
 additional:usize,
@@ -679,17 +744,27 @@ impl<A: Allocator> RawVecInner<A> {
 let cap = len.checked_add(additional).ok_or(CapacityOverflow)?;
 let new_layout = layout_array(cap, elem_layout)?;
 
-let ptr = finish_grow(new_layout,self.current_memory(elem_layout),&mutself.alloc)?;
+// SAFETY:
+// - For the `current_memory` call: Precondition passed to caller
+// - For the `finish_grow` call: Precondition passed to caller
+// + `current_memory` does the right thing
+let ptr =
+unsafe{finish_grow(new_layout,self.current_memory(elem_layout),&mutself.alloc)? };
 // SAFETY: layout_array would have resulted in a capacity overflow if we tried to allocate more than `isize::MAX` items
 unsafe{
 self.set_ptr_and_cap(ptr, cap);
 }
 Ok(())
 }
 
+/// # Safety
+/// - `elem_layout` must be valid for `self`, i.e. it must be the same `elem_layout` used to
+/// initially construct `self`
+/// - `elem_layout`'s size must be a multiple of its alignment
+/// - `cap` must be less than or equal to `self.capacity(elem_layout.size())`
 #[cfg(not(no_global_oom_handling))]
 #[inline]
-fnshrink(&mutself,cap:usize,elem_layout:Layout) -> Result<(),TryReserveError>{
+unsafefnshrink(&mutself,cap:usize,elem_layout:Layout) -> Result<(),TryReserveError>{
 assert!(cap <= self.capacity(elem_layout.size()),"Tried to shrink to a larger capacity");
 // SAFETY: Just checked this isn't trying to grow
 unsafe{self.shrink_unchecked(cap, elem_layout)}
@@ -711,8 +786,12 @@ impl<A: Allocator> RawVecInner<A> {
 cap:usize,
 elem_layout:Layout,
 ) -> Result<(),TryReserveError>{
-let(ptr, layout) =
-ifletSome(mem) = self.current_memory(elem_layout){ mem }else{returnOk(())};
+// SAFETY: Precondition passed to caller
+let(ptr, layout) = ifletSome(mem) = unsafe{self.current_memory(elem_layout)}{
+ mem
+}else{
+returnOk(());
+};
 
 // If shrinking to 0, deallocate the buffer. We don't reach this point
 // for the T::IS_ZST case since current_memory() will have returned
@@ -748,18 +827,26 @@ impl<A: Allocator> RawVecInner<A> {
 /// Ideally this function would take `self` by move, but it cannot because it exists to be
 /// called from a `Drop` impl.
 unsafefndeallocate(&mutself,elem_layout:Layout){
-ifletSome((ptr, layout)) = self.current_memory(elem_layout){
+// SAFETY: Precondition passed to caller
+ifletSome((ptr, layout)) = unsafe{self.current_memory(elem_layout)}{
 unsafe{
 self.alloc.deallocate(ptr, layout);
 }
 }
 }
 }
 
+/// # Safety
+/// If `current_memory` matches `Some((ptr, old_layout))`:
+/// - `ptr` must denote a block of memory *currently allocated* via `alloc`
+/// - `old_layout` must *fit* that block of memory
+/// - `new_layout` must have the same alignment as `old_layout`
+/// - `new_layout.size()` must be greater than or equal to `old_layout.size()`
+/// If `current_memory` is `None`, this function is safe.
 // not marked inline(never) since we want optimizers to be able to observe the specifics of this
 // function, see tests/codegen-llvm/vec-reserve-extend.rs.
 #[cold]
-fnfinish_grow<A>(
+unsafefnfinish_grow<A>(
 new_layout:Layout,
 current_memory:Option<(NonNull<u8>,Layout)>,
 alloc:&mutA,

library/alloc/src/raw_vec/tests.rs

@@ -85,7 +85,7 @@ struct ZST;
 fnzst_sanity<T>(v:&RawVec<T>){
 assert_eq!(v.capacity(),usize::MAX);
 assert_eq!(v.ptr(), core::ptr::Unique::<T>::dangling().as_ptr());
-assert_eq!(v.inner.current_memory(T::LAYOUT),None);
+assert_eq!(unsafe{v.inner.current_memory(T::LAYOUT)},None);
 }
 
 #[test]
@@ -126,12 +126,12 @@ fn zst() {
 assert_eq!(v.try_reserve_exact(101,usize::MAX - 100), cap_err);
 zst_sanity(&v);
 
-assert_eq!(v.inner.grow_amortized(100,usize::MAX - 100,ZST::LAYOUT), cap_err);
-assert_eq!(v.inner.grow_amortized(101,usize::MAX - 100,ZST::LAYOUT), cap_err);
+assert_eq!(unsafe{v.inner.grow_amortized(100,usize::MAX - 100,ZST::LAYOUT)}, cap_err);
+assert_eq!(unsafe{v.inner.grow_amortized(101,usize::MAX - 100,ZST::LAYOUT)}, cap_err);
 zst_sanity(&v);
 
-assert_eq!(v.inner.grow_exact(100,usize::MAX - 100,ZST::LAYOUT), cap_err);
-assert_eq!(v.inner.grow_exact(101,usize::MAX - 100,ZST::LAYOUT), cap_err);
+assert_eq!(unsafe{v.inner.grow_exact(100,usize::MAX - 100,ZST::LAYOUT)}, cap_err);
+assert_eq!(unsafe{v.inner.grow_exact(101,usize::MAX - 100,ZST::LAYOUT)}, cap_err);
 zst_sanity(&v);
 }