Deny unsafe ops in unsafe fns in std::sys_common

Author: LeSeulArtichaut

library/std/src/sys_common/alloc.rs

@@ -33,14 +33,25 @@ pub unsafe fn realloc_fallback(
     old_layout: Layout,
     new_size: usize,
 ) -> *mut u8 {
-    // Docs for GlobalAlloc::realloc require this to be valid:
-    let new_layout = Layout::from_size_align_unchecked(new_size, old_layout.align());
+    // SAFETY: as stated in docs for GlobalAlloc::realloc, the caller
+    // must guarantee that `new_size` is valid for a `Layout`.
+    // The `old_layout.align()` is guaranteed to be valid as it comes
+    // from a `Layout`.
+    let new_layout = unsafe { Layout::from_size_align_unchecked(new_size, old_layout.align()) };
 
-    let new_ptr = GlobalAlloc::alloc(alloc, new_layout);
+    // SAFETY: as stated in docs for GlobalAlloc::realloc, the caller
+    // must guarantee that `new_size` is greater than zero.
+    let new_ptr = unsafe { GlobalAlloc::alloc(alloc, new_layout) };
     if !new_ptr.is_null() {
         let size = cmp::min(old_layout.size(), new_size);
-        ptr::copy_nonoverlapping(ptr, new_ptr, size);
-        GlobalAlloc::dealloc(alloc, ptr, old_layout);
+        // SAFETY: the newly allocated memory cannot overlap the previously
+        // allocated memory. Also, the call to `dealloc` is safe since
+        // the caller must guarantee that `ptr` is allocated via this allocator
+        // and layout is the same layout that was used to allocate `ptr`.
+        unsafe {
+            ptr::copy_nonoverlapping(ptr, new_ptr, size);
+            GlobalAlloc::dealloc(alloc, ptr, old_layout);
+        }
     }
     new_ptr
 }

library/std/src/sys_common/at_exit_imp.rs

@@ -26,10 +26,14 @@ const DONE: *mut Queue = 1_usize as *mut _;
 const ITERS: usize = 10;
 
 unsafe fn init() -> bool {
-    if QUEUE.is_null() {
+    // SAFETY: the caller must ensure that `QUEUE` is not in use anywhere else,
+    // which `push` below does by locking `LOCK`.
+    if unsafe { QUEUE.is_null() } {
         let state: Box<Queue> = box Vec::new();
-        QUEUE = Box::into_raw(state);
-    } else if QUEUE == DONE {
+        unsafe {
+            QUEUE = Box::into_raw(state);
+        }
+    } else if unsafe { QUEUE == DONE } {
         // can't re-init after a cleanup
         return false;
     }

library/std/src/sys_common/backtrace.rs

@@ -76,44 +76,46 @@ unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt::
     let mut res = Ok(());
     // Start immediately if we're not using a short backtrace.
     let mut start = print_fmt != PrintFmt::Short;
-    backtrace_rs::trace_unsynchronized(|frame| {
-        if print_fmt == PrintFmt::Short && idx > MAX_NB_FRAMES {
-            return false;
-        }
+    unsafe {
+        backtrace_rs::trace_unsynchronized(|frame| {
+            if print_fmt == PrintFmt::Short && idx > MAX_NB_FRAMES {
+                return false;
+            }
 
-        let mut hit = false;
-        let mut stop = false;
-        backtrace_rs::resolve_frame_unsynchronized(frame, |symbol| {
-            hit = true;
-            if print_fmt == PrintFmt::Short {
-                if let Some(sym) = symbol.name().and_then(|s| s.as_str()) {
-                    if sym.contains("__rust_begin_short_backtrace") {
-                        stop = true;
-                        return;
-                    }
-                    if sym.contains("__rust_end_short_backtrace") {
-                        start = true;
-                        return;
+            let mut hit = false;
+            let mut stop = false;
+            backtrace_rs::resolve_frame_unsynchronized(frame, |symbol| {
+                hit = true;
+                if print_fmt == PrintFmt::Short {
+                    if let Some(sym) = symbol.name().and_then(|s| s.as_str()) {
+                        if sym.contains("__rust_begin_short_backtrace") {
+                            stop = true;
+                            return;
+                        }
+                        if sym.contains("__rust_end_short_backtrace") {
+                            start = true;
+                            return;
+                        }
                     }
                 }
-            }
 
-            if start {
-                res = bt_fmt.frame().symbol(frame, symbol);
+                if start {
+                    res = bt_fmt.frame().symbol(frame, symbol);
+                }
+            });
+            if stop {
+                return false;
             }
-        });
-        if stop {
-            return false;
-        }
-        if !hit {
-            if start {
-                res = bt_fmt.frame().print_raw(frame.ip(), None, None, None);
+            if !hit {
+                if start {
+                    res = bt_fmt.frame().print_raw(frame.ip(), None, None, None);
+                }
             }
-        }
 
-        idx += 1;
-        res.is_ok()
-    });
+            idx += 1;
+            res.is_ok()
+        });
+    }
     res?;
     bt_fmt.finish()?;
     if print_fmt == PrintFmt::Short {

library/std/src/sys_common/condvar.rs

@@ -25,19 +25,22 @@ impl Condvar {
     /// address.
     #[inline]
     pub unsafe fn init(&mut self) {
-        self.0.init()
+        // SAFETY: the caller must uphold the safety contract for `init`.
+        unsafe { self.0.init() }
     }
 
     /// Signals one waiter on this condition variable to wake up.
     #[inline]
     pub unsafe fn notify_one(&self) {
-        self.0.notify_one()
+        // SAFETY: the caller must uphold the safety contract for `notify_one`.
+        unsafe { self.0.notify_one() }
     }
 
     /// Awakens all current waiters on this condition variable.
     #[inline]
     pub unsafe fn notify_all(&self) {
-        self.0.notify_all()
+        // SAFETY: the caller must uphold the safety contract for `notify_all`.
+        unsafe { self.0.notify_all() }
     }
 
     /// Waits for a signal on the specified mutex.
@@ -47,7 +50,8 @@ impl Condvar {
     /// on this condition variable.
     #[inline]
     pub unsafe fn wait(&self, mutex: &Mutex) {
-        self.0.wait(mutex::raw(mutex))
+        // SAFETY: the caller must uphold the safety contract for `wait`.
+        unsafe { self.0.wait(mutex::raw(mutex)) }
     }
 
     /// Waits for a signal on the specified mutex with a timeout duration
@@ -58,7 +62,8 @@ impl Condvar {
     /// on this condition variable.
     #[inline]
     pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
-        self.0.wait_timeout(mutex::raw(mutex), dur)
+        // SAFETY: the caller must uphold the safety contract for `wait_timeout`.
+        unsafe { self.0.wait_timeout(mutex::raw(mutex), dur) }
     }
 
     /// Deallocates all resources associated with this condition variable.
@@ -67,6 +72,7 @@ impl Condvar {
     /// this condition variable.
     #[inline]
     pub unsafe fn destroy(&self) {
-        self.0.destroy()
+        // SAFETY: the caller must uphold the safety contract for `destroy`.
+        unsafe { self.0.destroy() }
     }
 }

library/std/src/sys_common/mod.rs

@@ -14,6 +14,7 @@
 
 #![allow(missing_docs)]
 #![allow(missing_debug_implementations)]
+#![deny(unsafe_op_in_unsafe_fn)]
 
 #[cfg(test)]
 mod tests;

library/std/src/sys_common/mutex.rs

@@ -30,7 +30,8 @@ impl Mutex {
     /// `init()`) is undefined behavior.
     #[inline]
     pub unsafe fn init(&mut self) {
-        self.0.init()
+        // SAFETY: the caller must uphold the safety contract for `init`.
+        unsafe { self.0.init() }
     }
 
     /// Locks the mutex blocking the current thread until it is available.
@@ -39,14 +40,18 @@ impl Mutex {
     /// previous function call.
     #[inline]
     pub unsafe fn raw_lock(&self) {
-        self.0.lock()
+        // SAFETY: the caller must uphold the safety contract for `lock`.
+        unsafe { self.0.lock() }
     }
 
     /// Calls raw_lock() and then returns an RAII guard to guarantee the mutex
     /// will be unlocked.
     #[inline]
     pub unsafe fn lock(&self) -> MutexGuard<'_> {
-        self.raw_lock();
+        // SAFETY: the caller must uphold the safety contract for `raw_lock`.
+        unsafe {
+            self.raw_lock();
+        }
         MutexGuard(&self.0)
     }
 
@@ -57,7 +62,8 @@ impl Mutex {
     /// previous function call.
     #[inline]
     pub unsafe fn try_lock(&self) -> bool {
-        self.0.try_lock()
+        // SAFETY: the caller must uphold the safety contract for `try_lock`.
+        unsafe { self.0.try_lock() }
     }
 
     /// Unlocks the mutex.
@@ -69,7 +75,8 @@ impl Mutex {
     /// lock() whenever possible.
     #[inline]
     pub unsafe fn raw_unlock(&self) {
-        self.0.unlock()
+        // SAFETY: the caller must uphold the safety contract for `unlock`.
+        unsafe { self.0.unlock() }
     }
 
     /// Deallocates all resources associated with this mutex.
@@ -78,7 +85,8 @@ impl Mutex {
     /// this mutex.
     #[inline]
     pub unsafe fn destroy(&self) {
-        self.0.destroy()
+        // SAFETY: the caller must uphold the safety contract for `destroy`.
+        unsafe { self.0.destroy() }
     }
 }
 

library/std/src/sys_common/remutex.rs

@@ -53,7 +53,12 @@ impl<T> ReentrantMutex<T> {
     /// once this mutex is in its final resting place, and only then are the
     /// lock/unlock methods safe.
     pub const unsafe fn new(t: T) -> ReentrantMutex<T> {
-        ReentrantMutex { inner: sys::ReentrantMutex::uninitialized(), data: t }
+        // `ReentrantMutex::uninitialized()` is not unsafe on all platforms
+        #[allow(unused_unsafe)]
+        // SAFETY: the caller must guarantee that `init` is called.
+        unsafe {
+            ReentrantMutex { inner: sys::ReentrantMutex::uninitialized(), data: t }
+        }
     }
 
     /// Initializes this mutex so it's ready for use.
@@ -63,7 +68,8 @@ impl<T> ReentrantMutex<T> {
     /// Unsafe to call more than once, and must be called after this will no
     /// longer move in memory.
     pub unsafe fn init(&self) {
-        self.inner.init();
+        // SAFETY: the caller must uphold the safety contract for `init`.
+        unsafe { self.inner.init() };
     }
 
     /// Acquires a mutex, blocking the current thread until it is able to do so.
@@ -79,6 +85,7 @@ impl<T> ReentrantMutex<T> {
     /// this call will return failure if the mutex would otherwise be
     /// acquired.
     pub fn lock(&self) -> ReentrantMutexGuard<'_, T> {
+        // SAFETY: the caller must uphold the safety contract for `lock`.
         unsafe { self.inner.lock() }
         ReentrantMutexGuard::new(&self)
     }

library/std/src/sys_common/rwlock.rs

@@ -23,7 +23,8 @@ impl RWLock {
     /// previous method call.
     #[inline]
     pub unsafe fn read(&self) {
-        self.0.read()
+        // SAFETY: the caller must uphold the safety contract for `read`.
+        unsafe { self.0.read() }
     }
 
     /// Attempts to acquire shared access to this lock, returning whether it
@@ -35,7 +36,8 @@ impl RWLock {
     /// previous method call.
     #[inline]
     pub unsafe fn try_read(&self) -> bool {
-        self.0.try_read()
+        // SAFETY: the caller must uphold the safety contract for `try_read`.
+        unsafe { self.0.try_read() }
     }
 
     /// Acquires write access to the underlying lock, blocking the current thread
@@ -45,7 +47,8 @@ impl RWLock {
     /// previous method call.
     #[inline]
     pub unsafe fn write(&self) {
-        self.0.write()
+        // SAFETY: the caller must uphold the safety contract for `write`.
+        unsafe { self.0.write() }
     }
 
     /// Attempts to acquire exclusive access to this lock, returning whether it
@@ -57,15 +60,17 @@ impl RWLock {
     /// previous method call.
     #[inline]
     pub unsafe fn try_write(&self) -> bool {
-        self.0.try_write()
+        // SAFETY: the caller must uphold the safety contract for `try_write`.
+        unsafe { self.0.try_write() }
     }
 
     /// Unlocks previously acquired shared access to this lock.
     ///
     /// Behavior is undefined if the current thread does not have shared access.
     #[inline]
     pub unsafe fn read_unlock(&self) {
-        self.0.read_unlock()
+        // SAFETY: the caller must uphold the safety contract for `read_unlock`.
+        unsafe { self.0.read_unlock() }
     }
 
     /// Unlocks previously acquired exclusive access to this lock.
@@ -74,7 +79,8 @@ impl RWLock {
     /// exclusive access.
     #[inline]
     pub unsafe fn write_unlock(&self) {
-        self.0.write_unlock()
+        // SAFETY: the caller must uphold the safety contract for `write_unlock`.
+        unsafe { self.0.write_unlock() }
     }
 
     /// Destroys OS-related resources with this RWLock.
@@ -83,6 +89,7 @@ impl RWLock {
     /// lock.
     #[inline]
     pub unsafe fn destroy(&self) {
-        self.0.destroy()
+        // SAFETY: the caller must uphold the safety contract for `destroy`.
+        unsafe { self.0.destroy() }
     }
 }

library/std/src/sys_common/thread_info.rs

@@ -1,4 +1,8 @@
-#![allow(dead_code)] // stack_guard isn't used right now on all platforms
+// stack_guard isn't used right now on all platforms
+#![allow(dead_code)]
+// FIXME: this should be removed once `thread_local!` denies unsafe ops in unsafe
+// fns. This cannot be done yet because the macro is also used in libproc_macro.
+#![allow(unsafe_op_in_unsafe_fn)]
 
 use crate::cell::RefCell;
 use crate::sys::thread::guard::Guard;

library/std/src/sys_common/thread_local_dtor.rs

@@ -29,21 +29,25 @@ pub unsafe fn register_dtor_fallback(t: *mut u8, dtor: unsafe extern "C" fn(*mut
 
     static DTORS: StaticKey = StaticKey::new(Some(run_dtors));
     type List = Vec<(*mut u8, unsafe extern "C" fn(*mut u8))>;
-    if DTORS.get().is_null() {
+    if unsafe { DTORS.get().is_null() } {
         let v: Box<List> = box Vec::new();
-        DTORS.set(Box::into_raw(v) as *mut u8);
+        unsafe { DTORS.set(Box::into_raw(v) as *mut u8) }
     }
-    let list: &mut List = &mut *(DTORS.get() as *mut List);
+    let list: &mut List = unsafe { &mut *(DTORS.get() as *mut List) };
     list.push((t, dtor));
 
     unsafe extern "C" fn run_dtors(mut ptr: *mut u8) {
         while !ptr.is_null() {
-            let list: Box<List> = Box::from_raw(ptr as *mut List);
+            let list: Box<List> = unsafe { Box::from_raw(ptr as *mut List) };
             for (ptr, dtor) in list.into_iter() {
-                dtor(ptr);
+                unsafe {
+                    dtor(ptr);
+                }
+            }
+            unsafe {
+                ptr = DTORS.get();
+                DTORS.set(ptr::null_mut());
             }
-            ptr = DTORS.get();
-            DTORS.set(ptr::null_mut());
         }
     }
 }