Fix issues from review and unsoundness of RawVec::into_box

Author: TimDiekmann

src/liballoc/alloc.rs

@@ -4,7 +4,7 @@
 
 use core::intrinsics::{self, min_align_of_val, size_of_val};
 use core::ptr::{NonNull, Unique};
-use core::usize;
+use core::{mem, usize};
 
 #[stable(feature = "alloc_module", since = "1.28.0")]
 #[doc(inline)]
@@ -165,102 +165,96 @@ pub unsafe fn alloc_zeroed(layout: Layout) -> *mut u8 {
 #[unstable(feature = "allocator_api", issue = "32838")]
 unsafe impl AllocRef for Global {
     #[inline]
-    fn alloc(&mut self, layout: Layout, init: AllocInit) -> Result<(NonNull<u8>, usize), AllocErr> {
-        let new_size = layout.size();
-        if new_size == 0 {
-            Ok((layout.dangling(), 0))
-        } else {
-            unsafe {
+    fn alloc(&mut self, layout: Layout, init: AllocInit) -> Result<MemoryBlock, AllocErr> {
+        unsafe {
+            if layout.size() == 0 {
+                Ok(MemoryBlock::new(layout.dangling(), layout))
+            } else {
                 let raw_ptr = match init {
                     AllocInit::Uninitialized => alloc(layout),
                     AllocInit::Zeroed => alloc_zeroed(layout),
                 };
                 let ptr = NonNull::new(raw_ptr).ok_or(AllocErr)?;
-                Ok((ptr, new_size))
+                Ok(MemoryBlock::new(ptr, layout))
             }
         }
     }
 
     #[inline]
-    unsafe fn dealloc(&mut self, ptr: NonNull<u8>, layout: Layout) {
-        if layout.size() != 0 {
-            dealloc(ptr.as_ptr(), layout)
+    unsafe fn dealloc(&mut self, memory: MemoryBlock) {
+        if memory.size() != 0 {
+            dealloc(memory.ptr().as_ptr(), memory.layout())
         }
     }
 
     #[inline]
     unsafe fn grow(
         &mut self,
-        ptr: NonNull<u8>,
-        layout: Layout,
+        memory: &mut MemoryBlock,
         new_size: usize,
         placement: ReallocPlacement,
         init: AllocInit,
-    ) -> Result<(NonNull<u8>, usize), AllocErr> {
-        let old_size = layout.size();
+    ) -> Result<(), AllocErr> {
+        let old_size = memory.size();
         debug_assert!(
             new_size >= old_size,
-            "`new_size` must be greater than or equal to `layout.size()`"
+            "`new_size` must be greater than or equal to `memory.size()`"
         );
 
         if old_size == new_size {
-            return Ok((ptr, new_size));
+            return Ok(());
         }
 
+        let new_layout = Layout::from_size_align_unchecked(new_size, memory.align());
         match placement {
+            ReallocPlacement::InPlace => return Err(AllocErr),
+            ReallocPlacement::MayMove if memory.size() == 0 => {
+                *memory = self.alloc(new_layout, init)?
+            }
             ReallocPlacement::MayMove => {
-                if old_size == 0 {
-                    self.alloc(Layout::from_size_align_unchecked(new_size, layout.align()), init)
-                } else {
-                    // `realloc` probably checks for `new_size > old_size` or something similar.
-                    // `new_size` must be greater than or equal to `old_size` due to the safety constraint,
-                    // and `new_size` == `old_size` was caught before
-                    intrinsics::assume(new_size > old_size);
-                    let ptr =
-                        NonNull::new(realloc(ptr.as_ptr(), layout, new_size)).ok_or(AllocErr)?;
-                    let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
-                    init.initialize_offset(ptr, new_layout, old_size);
-                    Ok((ptr, new_size))
-                }
+                // `realloc` probably checks for `new_size > old_size` or something similar.
+                intrinsics::assume(new_size > old_size);
+                let ptr = realloc(memory.ptr().as_ptr(), memory.layout(), new_size);
+                *memory = MemoryBlock::new(NonNull::new(ptr).ok_or(AllocErr)?, new_layout);
+                memory.init_offset(init, old_size);
             }
-            ReallocPlacement::InPlace => Err(AllocErr),
         }
+        Ok(())
     }
 
     #[inline]
     unsafe fn shrink(
         &mut self,
-        ptr: NonNull<u8>,
-        layout: Layout,
+        memory: &mut MemoryBlock,
         new_size: usize,
         placement: ReallocPlacement,
-    ) -> Result<(NonNull<u8>, usize), AllocErr> {
-        let old_size = layout.size();
+    ) -> Result<(), AllocErr> {
+        let old_size = memory.size();
         debug_assert!(
             new_size <= old_size,
-            "`new_size` must be smaller than or equal to `layout.size()`"
+            "`new_size` must be smaller than or equal to `memory.size()`"
         );
 
         if old_size == new_size {
-            return Ok((ptr, new_size));
+            return Ok(());
         }
 
+        let new_layout = Layout::from_size_align_unchecked(new_size, memory.align());
         match placement {
+            ReallocPlacement::InPlace => return Err(AllocErr),
+            ReallocPlacement::MayMove if new_size == 0 => {
+                let new_memory = MemoryBlock::new(new_layout.dangling(), new_layout);
+                let old_memory = mem::replace(memory, new_memory);
+                self.dealloc(old_memory)
+            }
             ReallocPlacement::MayMove => {
-                let ptr = if new_size == 0 {
-                    self.dealloc(ptr, layout);
-                    layout.dangling()
-                } else {
-                    // `realloc` probably checks for `new_size > old_size` or something similar.
-                    // `new_size` must be smaller than or equal to `old_size` due to the safety constraint,
-                    // and `new_size` == `old_size` was caught before
-                    intrinsics::assume(new_size < old_size);
-                    NonNull::new(realloc(ptr.as_ptr(), layout, new_size)).ok_or(AllocErr)?
-                };
-                Ok((ptr, new_size))
+                // `realloc` probably checks for `new_size < old_size` or something similar.
+                intrinsics::assume(new_size < old_size);
+                let ptr = realloc(memory.ptr().as_ptr(), memory.layout(), new_size);
+                *memory = MemoryBlock::new(NonNull::new(ptr).ok_or(AllocErr)?, new_layout);
             }
-            ReallocPlacement::InPlace => Err(AllocErr),
         }
+        Ok(())
     }
 }
 
@@ -272,7 +266,7 @@ unsafe impl AllocRef for Global {
 unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
     let layout = Layout::from_size_align_unchecked(size, align);
     match Global.alloc(layout, AllocInit::Uninitialized) {
-        Ok((ptr, _)) => ptr.as_ptr(),
+        Ok(memory) => memory.ptr().as_ptr(),
         Err(_) => handle_alloc_error(layout),
     }
 }
@@ -288,7 +282,7 @@ pub(crate) unsafe fn box_free<T: ?Sized>(ptr: Unique<T>) {
     let size = size_of_val(ptr.as_ref());
     let align = min_align_of_val(ptr.as_ref());
     let layout = Layout::from_size_align_unchecked(size, align);
-    Global.dealloc(ptr.cast().into(), layout)
+    Global.dealloc(MemoryBlock::new(ptr.cast().into(), layout))
 }
 
 /// Abort on memory allocation error or failure.

src/liballoc/alloc/tests.rs

@@ -8,17 +8,17 @@ use test::Bencher;
 fn allocate_zeroed() {
     unsafe {
         let layout = Layout::from_size_align(1024, 1).unwrap();
-        let (ptr, _) = Global
+        let memory = Global
             .alloc(layout.clone(), AllocInit::Zeroed)
             .unwrap_or_else(|_| handle_alloc_error(layout));
 
-        let mut i = ptr.cast::<u8>().as_ptr();
+        let mut i = memory.ptr().cast::<u8>().as_ptr();
         let end = i.add(layout.size());
         while i < end {
             assert_eq!(*i, 0);
             i = i.offset(1);
         }
-        Global.dealloc(ptr, layout);
+        Global.dealloc(memory);
     }
 }
 

src/liballoc/boxed.rs

@@ -143,7 +143,6 @@ use core::ops::{
 };
 use core::pin::Pin;
 use core::ptr::{self, NonNull, Unique};
-use core::slice;
 use core::task::{Context, Poll};
 
 use crate::alloc::{self, AllocInit, AllocRef, Global};
@@ -199,7 +198,7 @@ impl<T> Box<T> {
         let ptr = Global
             .alloc(layout, AllocInit::Uninitialized)
             .unwrap_or_else(|_| alloc::handle_alloc_error(layout))
-            .0
+            .ptr()
             .cast();
         unsafe { Box::from_raw(ptr.as_ptr()) }
     }
@@ -228,7 +227,7 @@ impl<T> Box<T> {
         let ptr = Global
             .alloc(layout, AllocInit::Zeroed)
             .unwrap_or_else(|_| alloc::handle_alloc_error(layout))
-            .0
+            .ptr()
             .cast();
         unsafe { Box::from_raw(ptr.as_ptr()) }
     }
@@ -265,13 +264,7 @@ impl<T> Box<[T]> {
     /// ```
     #[unstable(feature = "new_uninit", issue = "63291")]
     pub fn new_uninit_slice(len: usize) -> Box<[mem::MaybeUninit<T>]> {
-        let layout = alloc::Layout::array::<mem::MaybeUninit<T>>(len).unwrap();
-        let ptr = Global
-            .alloc(layout, AllocInit::Uninitialized)
-            .unwrap_or_else(|_| alloc::handle_alloc_error(layout))
-            .0
-            .cast();
-        unsafe { Box::from_raw(slice::from_raw_parts_mut(ptr.as_ptr(), len)) }
+        unsafe { RawVec::with_capacity(len).into_box(len) }
     }
 }
 
@@ -776,7 +769,7 @@ impl<T: Copy> From<&[T]> for Box<[T]> {
         let buf = RawVec::with_capacity(len);
         unsafe {
             ptr::copy_nonoverlapping(slice.as_ptr(), buf.ptr(), len);
-            buf.into_box().assume_init()
+            buf.into_box(slice.len()).assume_init()
         }
     }
 }

src/liballoc/collections/btree/node.rs

@@ -31,6 +31,7 @@
 // - A node of length `n` has `n` keys, `n` values, and (in an internal node) `n + 1` edges.
 //   This implies that even an empty internal node has at least one edge.
 
+use core::alloc::MemoryBlock;
 use core::cmp::Ordering;
 use core::marker::PhantomData;
 use core::mem::{self, MaybeUninit};
@@ -227,7 +228,10 @@ impl<K, V> Root<K, V> {
         }
 
         unsafe {
-            Global.dealloc(NonNull::from(top).cast(), Layout::new::<InternalNode<K, V>>());
+            Global.dealloc(MemoryBlock::new(
+                NonNull::from(top).cast(),
+                Layout::new::<InternalNode<K, V>>(),
+            ));
         }
     }
 }
@@ -392,14 +396,14 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::LeafOrInternal> {
         let height = self.height;
         let node = self.node;
         let ret = self.ascend().ok();
-        Global.dealloc(
+        Global.dealloc(MemoryBlock::new(
             node.cast(),
             if height > 0 {
                 Layout::new::<InternalNode<K, V>>()
             } else {
                 Layout::new::<LeafNode<K, V>>()
             },
-        );
+        ));
         ret
     }
 }
@@ -1142,7 +1146,7 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::
 
             (*left_node.as_leaf_mut()).len += right_len as u16 + 1;
 
-            if self.node.height > 1 {
+            let layout = if self.node.height > 1 {
                 ptr::copy_nonoverlapping(
                     right_node.cast_unchecked().as_internal().edges.as_ptr(),
                     left_node
@@ -1159,10 +1163,11 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::
                         .correct_parent_link();
                 }
 
-                Global.dealloc(right_node.node.cast(), Layout::new::<InternalNode<K, V>>());
+                Layout::new::<InternalNode<K, V>>()
             } else {
-                Global.dealloc(right_node.node.cast(), Layout::new::<LeafNode<K, V>>());
-            }
+                Layout::new::<LeafNode<K, V>>()
+            };
+            Global.dealloc(MemoryBlock::new(right_node.node.cast(), layout));
 
             Handle::new_edge(self.node, self.idx)
         }

src/liballoc/lib.rs

@@ -100,6 +100,7 @@
 #![feature(lang_items)]
 #![feature(libc)]
 #![cfg_attr(not(bootstrap), feature(negative_impls))]
+#![feature(new_uninit)]
 #![feature(nll)]
 #![feature(optin_builtin_traits)]
 #![feature(pattern)]