Deny unsafe ops in unsafe fns, part 1

Author: LeSeulArtichaut

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

@@ -1,6 +1,7 @@
 //! Memory allocation APIs
 
 #![stable(feature = "alloc_module", since = "1.28.0")]
+#![deny(unsafe_op_in_unsafe_fn)]
 
 mod global;
 mod layout;
@@ -54,7 +55,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 +81,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 +287,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 +374,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 +416,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 +429,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 +441,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

@@ -187,6 +187,7 @@
 //!
 
 #![stable(feature = "rust1", since = "1.0.0")]
+#![deny(unsafe_op_in_unsafe_fn)]
 
 use crate::cmp::Ordering;
 use crate::fmt::{self, Debug, Display};
@@ -1005,7 +1006,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

@@ -1,5 +1,7 @@
 //! Character conversions.
 
+#![deny(unsafe_op_in_unsafe_fn)]
+
 use crate::convert::TryFrom;
 use crate::fmt;
 use crate::mem::transmute;
@@ -99,7 +101,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

@@ -1,5 +1,7 @@
 //! impl char {}
 
+#![deny(unsafe_op_in_unsafe_fn)]
+
 use crate::slice;
 use crate::str::from_utf8_unchecked_mut;
 use crate::unicode::printable::is_printable;
@@ -183,7 +185,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

@@ -1,3 +1,5 @@
+#![deny(unsafe_op_in_unsafe_fn)]
+
 use super::{From, TryFrom};
 use crate::num::TryFromIntError;
 
@@ -28,7 +30,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/hint.rs

@@ -2,6 +2,8 @@
 
 //! Hints to compiler that affects how code should be emitted or optimized.
 
+#![deny(unsafe_op_in_unsafe_fn)]
+
 use crate::intrinsics;
 
 /// Informs the compiler that this point in the code is not reachable, enabling
@@ -46,7 +48,9 @@ use crate::intrinsics;
 #[inline]
 #[stable(feature = "unreachable", since = "1.27.0")]
 pub unsafe fn unreachable_unchecked() -> ! {
-    intrinsics::unreachable()
+    // SAFETY: the safety contract for `intrinsics::unreachable` must
+    // be upheld by the caller.
+    unsafe { intrinsics::unreachable() }
 }
 
 /// Emits a machine instruction hinting to the processor that it is running in busy-wait

src/libcore/intrinsics.rs

@@ -53,6 +53,7 @@
     issue = "none"
 )]
 #![allow(missing_docs)]
+#![deny(unsafe_op_in_unsafe_fn)]
 
 use crate::marker::DiscriminantKind;
 use crate::mem;
@@ -2097,7 +2098,10 @@ pub unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize) {
         // Not panicking to keep codegen impact smaller.
         abort();
     }
-    copy_nonoverlapping(src, dst, count)
+
+    // SAFETY: the safety contract for `copy_nonoverlapping` must be
+    // upheld by the caller.
+    unsafe { copy_nonoverlapping(src, dst, count) }
 }
 
 /// Copies `count * size_of::<T>()` bytes from `src` to `dst`. The source
@@ -2163,7 +2167,9 @@ pub unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {
         // Not panicking to keep codegen impact smaller.
         abort();
     }
-    copy(src, dst, count)
+
+    // SAFETY: the safety contract for `copy` must be upheld by the caller.
+    unsafe { copy(src, dst, count) }
 }
 
 /// Sets `count * size_of::<T>()` bytes of memory starting at `dst` to
@@ -2246,5 +2252,7 @@ pub unsafe fn write_bytes<T>(dst: *mut T, val: u8, count: usize) {
     }
 
     debug_assert!(is_aligned_and_not_null(dst), "attempt to write to unaligned or null pointer");
-    write_bytes(dst, val, count)
+
+    // SAFETY: the safety contract for `write_bytes` must be upheld by the caller.
+    unsafe { write_bytes(dst, val, count) }
 }

src/libcore/lib.rs

@@ -148,6 +148,7 @@
 #![feature(const_type_id)]
 #![feature(const_caller_location)]
 #![feature(no_niche)] // rust-lang/rust#68303
+#![feature(unsafe_block_in_unsafe_fn)]
 
 #[prelude_import]
 #[allow(unused)]

src/libcore/pin.rs

@@ -375,6 +375,7 @@
 //! [`i32`]: ../../std/primitive.i32.html
 
 #![stable(feature = "pin", since = "1.33.0")]
+#![deny(unsafe_op_in_unsafe_fn)]
 
 use crate::cmp::{self, PartialEq, PartialOrd};
 use crate::fmt;
@@ -679,7 +680,10 @@ impl<'a, T: ?Sized> Pin<&'a T> {
     {
         let pointer = &*self.pointer;
         let new_pointer = func(pointer);
-        Pin::new_unchecked(new_pointer)
+
+        // SAFETY: the safety contract for `new_unchecked` must be
+        // upheld by the caller.
+        unsafe { Pin::new_unchecked(new_pointer) }
     }
 
     /// Gets a shared reference out of a pin.
@@ -769,9 +773,13 @@ impl<'a, T: ?Sized> Pin<&'a mut T> {
         U: ?Sized,
         F: FnOnce(&mut T) -> &mut U,
     {
-        let pointer = Pin::get_unchecked_mut(self);
+        // SAFETY: the caller is responsible for not moving the
+        // value out of this reference.
+        let pointer = unsafe { Pin::get_unchecked_mut(self) };
         let new_pointer = func(pointer);
-        Pin::new_unchecked(new_pointer)
+        // SAFETY: as the value of `this` is guaranteed to not have
+        // been moved out, this call to `new_unchecked` is safe.
+        unsafe { Pin::new_unchecked(new_pointer) }
     }
 }