Rollup merge of rust-lang#73622 - LeSeulArtichaut:unsafe-libcore, r=nikomatsakis

Deny unsafe ops in unsafe fns in libcore

After `liballoc`, It's time for `libcore` :D

I planned to do this bit by bit to avoid having a big chunk of diffs, so to make reviews easier, and to make the unsafe blocks narrower and take the time to document them properly.

r? @nikomatsakis cc @RalfJung

Author: Manishearth

src/libcore/alloc/global.rs

@@ -127,9 +127,12 @@ pub unsafe trait GlobalAlloc {
     #[stable(feature = "global_alloc", since = "1.28.0")]
     unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
         let size = layout.size();
-        let ptr = self.alloc(layout);
+        // SAFETY: the safety contract for `alloc` must be upheld by the caller.
+        let ptr = unsafe { self.alloc(layout) };
         if !ptr.is_null() {
-            ptr::write_bytes(ptr, 0, size);
+            // SAFETY: as allocation succeeded, the region from `ptr`
+            // of size `size` is guaranteed to be valid for writes.
+            unsafe { ptr::write_bytes(ptr, 0, size) };
         }
         ptr
     }
@@ -187,11 +190,18 @@ pub unsafe trait GlobalAlloc {
     /// [`handle_alloc_error`]: ../../alloc/alloc/fn.handle_alloc_error.html
     #[stable(feature = "global_alloc", since = "1.28.0")]
     unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
-        let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
-        let new_ptr = self.alloc(new_layout);
+        // SAFETY: the caller must ensure that the `new_size` does not overflow.
+        // `layout.align()` comes from a `Layout` and is thus guaranteed to be valid.
+        let new_layout = unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
+        // SAFETY: the caller must ensure that `new_layout` is greater than zero.
+        let new_ptr = unsafe { self.alloc(new_layout) };
         if !new_ptr.is_null() {
-            ptr::copy_nonoverlapping(ptr, new_ptr, cmp::min(layout.size(), new_size));
-            self.dealloc(ptr, layout);
+            // SAFETY: the previously allocated block cannot overlap the newly allocated block.
+            // The safety contract for `dealloc` must be upheld by the caller.
+            unsafe {
+                ptr::copy_nonoverlapping(ptr, new_ptr, cmp::min(layout.size(), new_size));
+                self.dealloc(ptr, layout);
+            }
         }
         new_ptr
     }

src/libcore/alloc/layout.rs

@@ -90,7 +90,8 @@ impl Layout {
     #[rustc_const_stable(feature = "alloc_layout", since = "1.28.0")]
     #[inline]
     pub const unsafe fn from_size_align_unchecked(size: usize, align: usize) -> Self {
-        Layout { size_: size, align_: NonZeroUsize::new_unchecked(align) }
+        // SAFETY: the caller must ensure that `align` is greater than zero.
+        Layout { size_: size, align_: unsafe { NonZeroUsize::new_unchecked(align) } }
     }
 
     /// The minimum size in bytes for a memory block of this layout.

src/libcore/alloc/mod.rs

@@ -54,7 +54,9 @@ impl AllocInit {
     #[inline]
     #[unstable(feature = "allocator_api", issue = "32838")]
     pub unsafe fn init(self, memory: MemoryBlock) {
-        self.init_offset(memory, 0)
+        // SAFETY: the safety contract for `init_offset` must be
+        // upheld by the caller.
+        unsafe { self.init_offset(memory, 0) }
     }
 
     /// Initialize the memory block like specified by `init` at the specified `offset`.
@@ -78,7 +80,10 @@ impl AllocInit {
         match self {
             AllocInit::Uninitialized => (),
             AllocInit::Zeroed => {
-                memory.ptr.as_ptr().add(offset).write_bytes(0, memory.size - offset)
+                // SAFETY: the caller must guarantee that `offset` is smaller than or equal to `memory.size`,
+                // so the memory from `memory.ptr + offset` of length `memory.size - offset`
+                // is guaranteed to be contaned in `memory` and thus valid for writes.
+                unsafe { memory.ptr.as_ptr().add(offset).write_bytes(0, memory.size - offset) }
             }
         }
     }
@@ -281,11 +286,23 @@ pub unsafe trait AllocRef {
                     return Ok(MemoryBlock { ptr, size });
                 }
 
-                let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
+                let new_layout =
+                    // SAFETY: the caller must ensure that the `new_size` does not overflow.
+                    // `layout.align()` comes from a `Layout` and is thus guaranteed to be valid for a Layout.
+                    // The caller must ensure that `new_size` is greater than zero.
+                    unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
                 let new_memory = self.alloc(new_layout, init)?;
-                ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), size);
-                self.dealloc(ptr, layout);
-                Ok(new_memory)
+
+                // SAFETY: because `new_size` must be greater than or equal to `size`, both the old and new
+                // memory allocation are valid for reads and writes for `size` bytes. Also, because the old
+                // allocation wasn't yet deallocated, it cannot overlap `new_memory`. Thus, the call to
+                // `copy_nonoverlapping` is safe.
+                // The safety contract for `dealloc` must be upheld by the caller.
+                unsafe {
+                    ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), size);
+                    self.dealloc(ptr, layout);
+                    Ok(new_memory)
+                }
             }
         }
     }
@@ -356,11 +373,23 @@ pub unsafe trait AllocRef {
                     return Ok(MemoryBlock { ptr, size });
                 }
 
-                let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
+                let new_layout =
+                // SAFETY: the caller must ensure that the `new_size` does not overflow.
+                // `layout.align()` comes from a `Layout` and is thus guaranteed to be valid for a Layout.
+                // The caller must ensure that `new_size` is greater than zero.
+                    unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
                 let new_memory = self.alloc(new_layout, AllocInit::Uninitialized)?;
-                ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), new_size);
-                self.dealloc(ptr, layout);
-                Ok(new_memory)
+
+                // SAFETY: because `new_size` must be lower than or equal to `size`, both the old and new
+                // memory allocation are valid for reads and writes for `new_size` bytes. Also, because the
+                // old allocation wasn't yet deallocated, it cannot overlap `new_memory`. Thus, the call to
+                // `copy_nonoverlapping` is safe.
+                // The safety contract for `dealloc` must be upheld by the caller.
+                unsafe {
+                    ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), new_size);
+                    self.dealloc(ptr, layout);
+                    Ok(new_memory)
+                }
             }
         }
     }
@@ -386,7 +415,8 @@ where
 
     #[inline]
     unsafe fn dealloc(&mut self, ptr: NonNull<u8>, layout: Layout) {
-        (**self).dealloc(ptr, layout)
+        // SAFETY: the safety contract must be upheld by the caller
+        unsafe { (**self).dealloc(ptr, layout) }
     }
 
     #[inline]
@@ -398,7 +428,8 @@ where
         placement: ReallocPlacement,
         init: AllocInit,
     ) -> Result<MemoryBlock, AllocErr> {
-        (**self).grow(ptr, layout, new_size, placement, init)
+        // SAFETY: the safety contract must be upheld by the caller
+        unsafe { (**self).grow(ptr, layout, new_size, placement, init) }
     }
 
     #[inline]
@@ -409,6 +440,7 @@ where
         new_size: usize,
         placement: ReallocPlacement,
     ) -> Result<MemoryBlock, AllocErr> {
-        (**self).shrink(ptr, layout, new_size, placement)
+        // SAFETY: the safety contract must be upheld by the caller
+        unsafe { (**self).shrink(ptr, layout, new_size, placement) }
     }
 }

src/libcore/cell.rs

@@ -1005,7 +1005,12 @@ impl<T: ?Sized> RefCell<T> {
     #[inline]
     pub unsafe fn try_borrow_unguarded(&self) -> Result<&T, BorrowError> {
         if !is_writing(self.borrow.get()) {
-            Ok(&*self.value.get())
+            // SAFETY: We check that nobody is actively writing now, but it is
+            // the caller's responsibility to ensure that nobody writes until
+            // the returned reference is no longer in use.
+            // Also, `self.value.get()` refers to the value owned by `self`
+            // and is thus guaranteed to be valid for the lifetime of `self`.
+            Ok(unsafe { &*self.value.get() })
         } else {
             Err(BorrowError { _private: () })
         }

src/libcore/char/convert.rs

@@ -99,7 +99,8 @@ pub fn from_u32(i: u32) -> Option<char> {
 #[inline]
 #[stable(feature = "char_from_unchecked", since = "1.5.0")]
 pub unsafe fn from_u32_unchecked(i: u32) -> char {
-    if cfg!(debug_assertions) { char::from_u32(i).unwrap() } else { transmute(i) }
+    // SAFETY: the caller must guarantee that `i` is a valid char value.
+    if cfg!(debug_assertions) { char::from_u32(i).unwrap() } else { unsafe { transmute(i) } }
 }
 
 #[stable(feature = "char_convert", since = "1.13.0")]

src/libcore/char/methods.rs

@@ -183,7 +183,8 @@ impl char {
     #[unstable(feature = "assoc_char_funcs", reason = "recently added", issue = "71763")]
     #[inline]
     pub unsafe fn from_u32_unchecked(i: u32) -> char {
-        super::convert::from_u32_unchecked(i)
+        // SAFETY: the safety contract must be upheld by the caller.
+        unsafe { super::convert::from_u32_unchecked(i) }
     }
 
     /// Converts a digit in the given radix to a `char`.

src/libcore/convert/num.rs

@@ -28,7 +28,8 @@ macro_rules! impl_float_to_int {
                 #[doc(hidden)]
                 #[inline]
                 unsafe fn to_int_unchecked(self) -> $Int {
-                    crate::intrinsics::float_to_int_unchecked(self)
+                    // SAFETY: the safety contract must be upheld by the caller.
+                    unsafe { crate::intrinsics::float_to_int_unchecked(self) }
                 }
             }
         )+

src/libcore/ffi.rs

@@ -333,7 +333,8 @@ impl<'f> VaListImpl<'f> {
     /// Advance to the next arg.
     #[inline]
     pub unsafe fn arg<T: sealed_trait::VaArgSafe>(&mut self) -> T {
-        va_arg(self)
+        // SAFETY: the caller must uphold the safety contract for `va_arg`.
+        unsafe { va_arg(self) }
     }
 
     /// Copies the `va_list` at the current location.
@@ -343,7 +344,10 @@ impl<'f> VaListImpl<'f> {
     {
         let mut ap = self.clone();
         let ret = f(ap.as_va_list());
-        va_end(&mut ap);
+        // SAFETY: the caller must uphold the safety contract for `va_end`.
+        unsafe {
+            va_end(&mut ap);
+        }
         ret
     }
 }

src/libcore/future/mod.rs

@@ -85,5 +85,7 @@ where
 #[unstable(feature = "gen_future", issue = "50547")]
 #[inline]
 pub unsafe fn get_context<'a, 'b>(cx: ResumeTy) -> &'a mut Context<'b> {
-    &mut *cx.0.as_ptr().cast()
+    // SAFETY: the caller must guarantee that `cx.0` is a valid pointer
+    // that fulfills all the requirements for a mutable reference.
+    unsafe { &mut *cx.0.as_ptr().cast() }
 }

src/libcore/hash/sip.rs

@@ -130,15 +130,19 @@ unsafe fn u8to64_le(buf: &[u8], start: usize, len: usize) -> u64 {
     let mut i = 0; // current byte index (from LSB) in the output u64
     let mut out = 0;
     if i + 3 < len {
-        out = load_int_le!(buf, start + i, u32) as u64;
+        // SAFETY: `i` cannot be greater than `len`, and the caller must guarantee
+        // that the index start..start+len is in bounds.
+        out = unsafe { load_int_le!(buf, start + i, u32) } as u64;
         i += 4;
     }
     if i + 1 < len {
-        out |= (load_int_le!(buf, start + i, u16) as u64) << (i * 8);
+        // SAFETY: same as above.
+        out |= (unsafe { load_int_le!(buf, start + i, u16) } as u64) << (i * 8);
         i += 2
     }
     if i < len {
-        out |= (*buf.get_unchecked(start + i) as u64) << (i * 8);
+        // SAFETY: same as above.
+        out |= (unsafe { *buf.get_unchecked(start + i) } as u64) << (i * 8);
         i += 1;
     }
     debug_assert_eq!(i, len);