Apply #![deny(unsafe_op_in_unsafe_fn)] to sys/windows

Author: carbotaniuman

library/std/src/sys/windows/alloc.rs

@@ -1,3 +1,5 @@
+#![deny(unsafe_op_in_unsafe_fn)]
+
 use crate::alloc::{GlobalAlloc, Layout, System};
 use crate::sys::c;
 use crate::sys_common::alloc::{realloc_fallback, MIN_ALIGN};
@@ -6,56 +8,87 @@ use crate::sys_common::alloc::{realloc_fallback, MIN_ALIGN};
 struct Header(*mut u8);
 
 unsafe fn get_header<'a>(ptr: *mut u8) -> &'a mut Header {
-    &mut *(ptr as *mut Header).offset(-1)
+    // SAFETY: the safety contract must be upheld by the caller
+    unsafe { &mut *(ptr as *mut Header).offset(-1) }
 }
 
 unsafe fn align_ptr(ptr: *mut u8, align: usize) -> *mut u8 {
-    let aligned = ptr.add(align - (ptr as usize & (align - 1)));
-    *get_header(aligned) = Header(ptr);
-    aligned
+    // SAFETY: the safety contract must be upheld by the caller
+    unsafe {
+        let aligned = ptr.add(align - (ptr as usize & (align - 1)));
+        *get_header(aligned) = Header(ptr);
+        aligned
+    }
 }
 
 #[inline]
 unsafe fn allocate_with_flags(layout: Layout, flags: c::DWORD) -> *mut u8 {
     if layout.align() <= MIN_ALIGN {
-        return c::HeapAlloc(c::GetProcessHeap(), flags, layout.size()) as *mut u8;
+        // SAFETY: `layout.size()` comes from `Layout` and is valid.
+        return unsafe { c::HeapAlloc(c::GetProcessHeap(), flags, layout.size()) as *mut u8 };
     }
 
-    let size = layout.size() + layout.align();
-    let ptr = c::HeapAlloc(c::GetProcessHeap(), flags, size);
-    if ptr.is_null() { ptr as *mut u8 } else { align_ptr(ptr as *mut u8, layout.align()) }
+    let ptr = unsafe {
+        // SAFETY: The caller must ensure that
+        // `layout.size()` + `layout.size()` does not overflow.
+        let size = layout.size() + layout.align();
+        c::HeapAlloc(c::GetProcessHeap(), flags, size)
+    };
+
+    if ptr.is_null() {
+        ptr as *mut u8
+    } else {
+        // SAFETY: `ptr` is a valid pointer
+        // with enough allocated space to store the header.
+        unsafe { align_ptr(ptr as *mut u8, layout.align()) }
+    }
 }
 
+// SAFETY: All methods implemented follow the contract rules defined
+// in `GlobalAlloc`.
 #[stable(feature = "alloc_system_type", since = "1.28.0")]
 unsafe impl GlobalAlloc for System {
     #[inline]
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
-        allocate_with_flags(layout, 0)
+        // SAFETY: the safety contract for `allocate_with_flags` must be upheld by the caller.
+        unsafe { allocate_with_flags(layout, 0) }
     }
 
     #[inline]
     unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
-        allocate_with_flags(layout, c::HEAP_ZERO_MEMORY)
+        // SAFETY: the safety contract for `allocate_with_flags must be upheld by the caller.
+        unsafe { allocate_with_flags(layout, c::HEAP_ZERO_MEMORY) }
     }
 
     #[inline]
     unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
-        if layout.align() <= MIN_ALIGN {
-            let err = c::HeapFree(c::GetProcessHeap(), 0, ptr as c::LPVOID);
-            debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError());
-        } else {
-            let header = get_header(ptr);
-            let err = c::HeapFree(c::GetProcessHeap(), 0, header.0 as c::LPVOID);
-            debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError());
+        // SAFETY: HeapFree is safe if ptr was allocated by this allocator
+        let err = unsafe {
+            if layout.align() <= MIN_ALIGN {
+                c::HeapFree(c::GetProcessHeap(), 0, ptr as c::LPVOID);
+            } else {
+                let header = get_header(ptr);
+                c::HeapFree(c::GetProcessHeap(), 0, header.0 as c::LPVOID);
+            }
         }
+        debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError());
     }
 
     #[inline]
     unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
-        if layout.align() <= MIN_ALIGN {
-            c::HeapReAlloc(c::GetProcessHeap(), 0, ptr as c::LPVOID, new_size) as *mut u8
-        } else {
-            realloc_fallback(self, ptr, layout, new_size)
+        unsafe {
+            if layout.align() <= MIN_ALIGN {
+                // SAFETY: HeapReAlloc is safe if ptr was allocated by this allocator
+                // and new_size is not 0.
+                unsafe {
+                    c::HeapReAlloc(c::GetProcessHeap(), 0, ptr as c::LPVOID, new_size) as *mut u8
+                }
+            } else {
+                // SAFETY: The safety contract for `realloc_fallback` must be upheld by the caller
+                unsafe {
+                    realloc_fallback(self, ptr, layout, new_size)
+                }
+            }
         }
     }
 }

library/std/src/sys/windows/args.rs

@@ -1,4 +1,5 @@
 #![allow(dead_code)] // runtime init functions not used during testing
+#![deny(unsafe_op_in_unsafe_fn)]
 
 #[cfg(test)]
 mod tests;
@@ -52,11 +53,14 @@ unsafe fn parse_lp_cmd_line<F: Fn() -> OsString>(
     const TAB: u16 = '\t' as u16;
     const SPACE: u16 = ' ' as u16;
     let mut ret_val = Vec::new();
-    if lp_cmd_line.is_null() || *lp_cmd_line == 0 {
-        ret_val.push(exe_name());
-        return ret_val;
-    }
-    let mut cmd_line = {
+
+    //SAFETY: the caller must supply a valid pointer
+    let mut cmd_line = unsafe {
+        if lp_cmd_line.is_null() || *lp_cmd_line == 0 {
+            ret_val.push(exe_name());
+            return ret_val;
+        }
+
         let mut end = 0;
         while *lp_cmd_line.offset(end) != 0 {
             end += 1;

library/std/src/sys/windows/c.rs

@@ -1,6 +1,7 @@
 //! C definitions used by libnative that don't belong in liblibc
 
 #![allow(nonstandard_style)]
+#![deny(unsafe_op_in_unsafe_fn)]
 #![cfg_attr(test, allow(dead_code))]
 #![unstable(issue = "none", feature = "windows_c")]
 

library/std/src/sys/windows/cmath.rs

@@ -1,3 +1,4 @@
+#![deny(unsafe_op_in_unsafe_fn)]
 #![cfg(not(test))]
 
 use libc::{c_double, c_float};

library/std/src/sys/windows/compat.rs

@@ -11,6 +11,8 @@
 //! manner we pay a semi-large one-time cost up front for detecting whether a
 //! function is available but afterwards it's just a load and a jump.
 
+#![deny(unsafe_op_in_unsafe_fn)]
+
 use crate::ffi::CString;
 use crate::sys::c;
 
@@ -86,7 +88,9 @@ macro_rules! compat_fn {
             }
 
             pub unsafe fn call($($argname: $argtype),*) -> $rettype {
-                mem::transmute::<usize, F>(addr())($($argname),*)
+                unsafe {
+                    mem::transmute::<usize, F>(addr())($($argname),*)
+                }
             }
         }
 

library/std/src/sys/windows/condvar.rs

@@ -1,3 +1,5 @@
+#![deny(unsafe_op_in_unsafe_fn)]
+
 use crate::cell::UnsafeCell;
 use crate::sys::c;
 use crate::sys::mutex::{self, Mutex};
@@ -23,17 +25,23 @@ impl Condvar {
 
     #[inline]
     pub unsafe fn wait(&self, mutex: &Mutex) {
-        let r = c::SleepConditionVariableSRW(self.inner.get(), mutex::raw(mutex), c::INFINITE, 0);
+        // SAFETY: The caller must ensure that the condvar is not moved or copied
+        let r = unsafe {
+            c::SleepConditionVariableSRW(self.inner.get(), mutex::raw(mutex), c::INFINITE, 0)
+        };
         debug_assert!(r != 0);
     }
 
     pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
-        let r = c::SleepConditionVariableSRW(
-            self.inner.get(),
-            mutex::raw(mutex),
-            super::dur2timeout(dur),
-            0,
-        );
+        // SAFETY: The caller must ensure that the condvar is not moved or copied
+        let r = unsafe {
+            c::SleepConditionVariableSRW(
+                self.inner.get(),
+                mutex::raw(mutex),
+                super::dur2timeout(dur),
+                0,
+            )
+        };
         if r == 0 {
             debug_assert_eq!(os::errno() as usize, c::ERROR_TIMEOUT as usize);
             false
@@ -44,12 +52,14 @@ impl Condvar {
 
     #[inline]
     pub unsafe fn notify_one(&self) {
-        c::WakeConditionVariable(self.inner.get())
+        // SAFETY: The caller must ensure that the condvar is not moved or copied
+        unsafe { c::WakeConditionVariable(self.inner.get()) }
     }
 
     #[inline]
     pub unsafe fn notify_all(&self) {
-        c::WakeAllConditionVariable(self.inner.get())
+        // SAFETY: The caller must ensure that the condvar is not moved or copied
+        unsafe { c::WakeAllConditionVariable(self.inner.get()) }
     }
 
     pub unsafe fn destroy(&self) {

library/std/src/sys/windows/env.rs

@@ -1,3 +1,5 @@
+#![deny(unsafe_op_in_unsafe_fn)]
+
 pub mod os {
     pub const FAMILY: &str = "windows";
     pub const OS: &str = "windows";

library/std/src/sys/windows/ext/ffi.rs

@@ -50,6 +50,7 @@
 //! [`collect`]: crate::iter::Iterator::collect
 //! [U+FFFD]: crate::char::REPLACEMENT_CHARACTER
 
+#![deny(unsafe_op_in_unsafe_fn)]
 #![stable(feature = "rust1", since = "1.0.0")]
 
 use crate::ffi::{OsStr, OsString};

library/std/src/sys/windows/ext/fs.rs

@@ -1,5 +1,6 @@
 //! Windows-specific extensions for the primitives in the `std::fs` module.
 
+#![deny(unsafe_op_in_unsafe_fn)]
 #![stable(feature = "rust1", since = "1.0.0")]
 
 use crate::fs::{self, Metadata, OpenOptions};

library/std/src/sys/windows/ext/io.rs

@@ -1,5 +1,6 @@
 //! Windows-specific extensions to general I/O primitives.
 
+#![deny(unsafe_op_in_unsafe_fn)]
 #![stable(feature = "rust1", since = "1.0.0")]
 
 use crate::fs;