From c1665e77768e459daa25dfb75b103b044ffd28e7 Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Sun, 13 Sep 2020 11:19:49 -0500 Subject: [PATCH 01/11] Apply `#![deny(unsafe_op_in_unsafe_fn)]` to sys/windows --- library/std/src/sys/windows/alloc.rs | 75 +++++++---- library/std/src/sys/windows/args.rs | 14 ++- library/std/src/sys/windows/c.rs | 1 + library/std/src/sys/windows/cmath.rs | 1 + library/std/src/sys/windows/compat.rs | 6 +- library/std/src/sys/windows/condvar.rs | 28 +++-- library/std/src/sys/windows/env.rs | 2 + library/std/src/sys/windows/ext/ffi.rs | 1 + library/std/src/sys/windows/ext/fs.rs | 1 + library/std/src/sys/windows/ext/io.rs | 1 + library/std/src/sys/windows/ext/mod.rs | 1 + library/std/src/sys/windows/ext/process.rs | 1 + library/std/src/sys/windows/ext/raw.rs | 1 + library/std/src/sys/windows/ext/thread.rs | 1 + library/std/src/sys/windows/fs.rs | 5 +- library/std/src/sys/windows/handle.rs | 6 +- library/std/src/sys/windows/io.rs | 2 + library/std/src/sys/windows/memchr.rs | 1 + library/std/src/sys/windows/mod.rs | 1 + library/std/src/sys/windows/mutex.rs | 118 +++++++++++------- library/std/src/sys/windows/net.rs | 1 + library/std/src/sys/windows/os.rs | 2 +- library/std/src/sys/windows/os_str.rs | 2 + library/std/src/sys/windows/path.rs | 5 +- library/std/src/sys/windows/pipe.rs | 5 +- library/std/src/sys/windows/process.rs | 1 + library/std/src/sys/windows/rand.rs | 2 + library/std/src/sys/windows/rwlock.rs | 14 ++- library/std/src/sys/windows/stack_overflow.rs | 21 ++-- .../std/src/sys/windows/stack_overflow_uwp.rs | 1 + library/std/src/sys/windows/stdio.rs | 1 + library/std/src/sys/windows/stdio_uwp.rs | 1 + library/std/src/sys/windows/thread.rs | 29 +++-- .../std/src/sys/windows/thread_local_dtor.rs | 1 + .../std/src/sys/windows/thread_local_key.rs | 56 ++++++--- library/std/src/sys/windows/time.rs | 2 + 36 files changed, 283 insertions(+), 128 deletions(-) diff --git a/library/std/src/sys/windows/alloc.rs b/library/std/src/sys/windows/alloc.rs index 99b4d6c72a0e3..f5cfeb7dfcd91 100644 --- a/library/std/src/sys/windows/alloc.rs +++ b/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) + } + } } } } diff --git a/library/std/src/sys/windows/args.rs b/library/std/src/sys/windows/args.rs index bcc2ea9ae00f0..6b869307ad2e8 100644 --- a/library/std/src/sys/windows/args.rs +++ b/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 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; diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index 657421e3fa4cc..2e3fe02ea07c0 100644 --- a/library/std/src/sys/windows/c.rs +++ b/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")] diff --git a/library/std/src/sys/windows/cmath.rs b/library/std/src/sys/windows/cmath.rs index 1a5421facd0c1..c261d35667561 100644 --- a/library/std/src/sys/windows/cmath.rs +++ b/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}; diff --git a/library/std/src/sys/windows/compat.rs b/library/std/src/sys/windows/compat.rs index 3f25f05e1b9a7..f69da83c4c4a8 100644 --- a/library/std/src/sys/windows/compat.rs +++ b/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::(addr())($($argname),*) + unsafe { + mem::transmute::(addr())($($argname),*) + } } } diff --git a/library/std/src/sys/windows/condvar.rs b/library/std/src/sys/windows/condvar.rs index 44547a5c51a34..c22d4688dc338 100644 --- a/library/std/src/sys/windows/condvar.rs +++ b/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) { diff --git a/library/std/src/sys/windows/env.rs b/library/std/src/sys/windows/env.rs index f0a99d6200cac..9338718c11fca 100644 --- a/library/std/src/sys/windows/env.rs +++ b/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"; diff --git a/library/std/src/sys/windows/ext/ffi.rs b/library/std/src/sys/windows/ext/ffi.rs index 1df2a0df143b3..3b8a201cfcb28 100644 --- a/library/std/src/sys/windows/ext/ffi.rs +++ b/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}; diff --git a/library/std/src/sys/windows/ext/fs.rs b/library/std/src/sys/windows/ext/fs.rs index e0615f2d33431..172906e0e9594 100644 --- a/library/std/src/sys/windows/ext/fs.rs +++ b/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}; diff --git a/library/std/src/sys/windows/ext/io.rs b/library/std/src/sys/windows/ext/io.rs index e75f9a4bfd5e3..8d32e9e77a4eb 100644 --- a/library/std/src/sys/windows/ext/io.rs +++ b/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; diff --git a/library/std/src/sys/windows/ext/mod.rs b/library/std/src/sys/windows/ext/mod.rs index 613d3dc189a43..689b33930af89 100644 --- a/library/std/src/sys/windows/ext/mod.rs +++ b/library/std/src/sys/windows/ext/mod.rs @@ -6,6 +6,7 @@ //! `std` types and idioms with Windows in a way that the normal //! platform-agnostic idioms would not normally support. +#![deny(unsafe_op_in_unsafe_fn)] #![stable(feature = "rust1", since = "1.0.0")] #![doc(cfg(windows))] #![allow(missing_docs)] diff --git a/library/std/src/sys/windows/ext/process.rs b/library/std/src/sys/windows/ext/process.rs index 61e4c6a1d1718..cfb77bb2953a5 100644 --- a/library/std/src/sys/windows/ext/process.rs +++ b/library/std/src/sys/windows/ext/process.rs @@ -1,5 +1,6 @@ //! Extensions to `std::process` for Windows. +#![deny(unsafe_op_in_unsafe_fn)] #![stable(feature = "process_extensions", since = "1.2.0")] use crate::os::windows::io::{AsRawHandle, FromRawHandle, IntoRawHandle, RawHandle}; diff --git a/library/std/src/sys/windows/ext/raw.rs b/library/std/src/sys/windows/ext/raw.rs index 5014e008eb599..ff0dcd7e54927 100644 --- a/library/std/src/sys/windows/ext/raw.rs +++ b/library/std/src/sys/windows/ext/raw.rs @@ -1,5 +1,6 @@ //! Windows-specific primitives. +#![deny(unsafe_op_in_unsafe_fn)] #![stable(feature = "raw_ext", since = "1.1.0")] use crate::os::raw::c_void; diff --git a/library/std/src/sys/windows/ext/thread.rs b/library/std/src/sys/windows/ext/thread.rs index 41c29f5b950ef..c8165f47fb251 100644 --- a/library/std/src/sys/windows/ext/thread.rs +++ b/library/std/src/sys/windows/ext/thread.rs @@ -1,5 +1,6 @@ //! Extensions to `std::thread` for Windows. +#![deny(unsafe_op_in_unsafe_fn)] #![stable(feature = "thread_extensions", since = "1.9.0")] use crate::os::windows::io::{AsRawHandle, IntoRawHandle, RawHandle}; diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index cdbfac267b9a1..96820b83ac761 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::os::windows::prelude::*; use crate::ffi::OsString; @@ -862,7 +864,8 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { lpData: c::LPVOID, ) -> c::DWORD { if dwStreamNumber == 1 { - *(lpData as *mut i64) = StreamBytesTransferred; + //SAFETY: We pass a *mut i64 as lpData to CopyFileExW, so we must get it back + unsafe { *(lpData as *mut i64) = StreamBytesTransferred }; } c::PROGRESS_CONTINUE } diff --git a/library/std/src/sys/windows/handle.rs b/library/std/src/sys/windows/handle.rs index 0d4baa3b340df..b66deee994f06 100644 --- a/library/std/src/sys/windows/handle.rs +++ b/library/std/src/sys/windows/handle.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![unstable(issue = "none", feature = "windows_handle")] use crate::cmp; @@ -120,7 +121,10 @@ impl RawHandle { ) -> io::Result> { let len = cmp::min(buf.len(), ::MAX as usize) as c::DWORD; let mut amt = 0; - let res = cvt(c::ReadFile(self.0, buf.as_ptr() as c::LPVOID, len, &mut amt, overlapped)); + // SAFETY: the safety contract must be upheld by the caller + let res = unsafe { + cvt(c::ReadFile(self.0, buf.as_ptr() as c::LPVOID, len, &mut amt, overlapped)) + }; match res { Ok(_) => Ok(Some(amt as usize)), Err(e) => { diff --git a/library/std/src/sys/windows/io.rs b/library/std/src/sys/windows/io.rs index fb06df1f80cda..c6798b6bccb3e 100644 --- a/library/std/src/sys/windows/io.rs +++ b/library/std/src/sys/windows/io.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::marker::PhantomData; use crate::slice; use crate::sys::c; diff --git a/library/std/src/sys/windows/memchr.rs b/library/std/src/sys/windows/memchr.rs index b9e5bcc1b4bbd..afa69b947912e 100644 --- a/library/std/src/sys/windows/memchr.rs +++ b/library/std/src/sys/windows/memchr.rs @@ -2,4 +2,5 @@ // Copyright 2015 Andrew Gallant, bluss and Nicolas Koch // Fallback memchr is fastest on Windows. +#![deny(unsafe_op_in_unsafe_fn)] pub use core::slice::memchr::{memchr, memrchr}; diff --git a/library/std/src/sys/windows/mod.rs b/library/std/src/sys/windows/mod.rs index 8c19cc78b09cd..4489ee880fc70 100644 --- a/library/std/src/sys/windows/mod.rs +++ b/library/std/src/sys/windows/mod.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![allow(missing_docs, nonstandard_style)] use crate::ffi::{OsStr, OsString}; diff --git a/library/std/src/sys/windows/mutex.rs b/library/std/src/sys/windows/mutex.rs index fa51b006c346f..c944528dcf0e8 100644 --- a/library/std/src/sys/windows/mutex.rs +++ b/library/std/src/sys/windows/mutex.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + //! System Mutexes //! //! The Windows implementation of mutexes is a little odd and it may not be @@ -65,53 +67,61 @@ impl Mutex { #[inline] pub unsafe fn init(&mut self) {} pub unsafe fn lock(&self) { - match kind() { - Kind::SRWLock => c::AcquireSRWLockExclusive(raw(self)), - Kind::CriticalSection => { - let inner = &*self.inner(); - inner.remutex.lock(); - if inner.held.replace(true) { - // It was already locked, so we got a recursive lock which we do not want. - inner.remutex.unlock(); - panic!("cannot recursively lock a mutex"); + unsafe { + match kind() { + Kind::SRWLock => c::AcquireSRWLockExclusive(raw(self)), + Kind::CriticalSection => { + let inner = &*self.inner(); + inner.remutex.lock(); + if inner.held.replace(true) { + // It was already locked, so we got a recursive lock which we do not want. + inner.remutex.unlock(); + panic!("cannot recursively lock a mutex"); + } } } } } pub unsafe fn try_lock(&self) -> bool { - match kind() { - Kind::SRWLock => c::TryAcquireSRWLockExclusive(raw(self)) != 0, - Kind::CriticalSection => { - let inner = &*self.inner(); - if !inner.remutex.try_lock() { - false - } else if inner.held.replace(true) { - // It was already locked, so we got a recursive lock which we do not want. - inner.remutex.unlock(); - false - } else { - true + unsafe { + match kind() { + Kind::SRWLock => c::TryAcquireSRWLockExclusive(raw(self)) != 0, + Kind::CriticalSection => { + let inner = &*self.inner(); + if !inner.remutex.try_lock() { + false + } else if inner.held.replace(true) { + // It was already locked, so we got a recursive lock which we do not want. + inner.remutex.unlock(); + false + } else { + true + } } } } } pub unsafe fn unlock(&self) { - match kind() { - Kind::SRWLock => c::ReleaseSRWLockExclusive(raw(self)), - Kind::CriticalSection => { - let inner = &*(self.lock.load(Ordering::SeqCst) as *const Inner); - inner.held.set(false); - inner.remutex.unlock(); + unsafe { + match kind() { + Kind::SRWLock => c::ReleaseSRWLockExclusive(raw(self)), + Kind::CriticalSection => { + let inner = &*(self.lock.load(Ordering::SeqCst) as *const Inner); + inner.held.set(false); + inner.remutex.unlock(); + } } } } pub unsafe fn destroy(&self) { - match kind() { - Kind::SRWLock => {} - Kind::CriticalSection => match self.lock.load(Ordering::SeqCst) { - 0 => {} - n => Box::from_raw(n as *mut Inner).remutex.destroy(), - }, + unsafe { + match kind() { + Kind::SRWLock => {} + Kind::CriticalSection => match self.lock.load(Ordering::SeqCst) { + 0 => {} + n => Box::from_raw(n as *mut Inner).remutex.destroy(), + }, + } } } @@ -121,13 +131,18 @@ impl Mutex { n => return n as *const _, } let inner = box Inner { remutex: ReentrantMutex::uninitialized(), held: Cell::new(false) }; - inner.remutex.init(); + unsafe { + inner.remutex.init(); + } let inner = Box::into_raw(inner); - match self.lock.compare_and_swap(0, inner as usize, Ordering::SeqCst) { - 0 => inner, - n => { - Box::from_raw(inner).remutex.destroy(); - n as *const _ + + unsafe { + match self.lock.compare_and_swap(0, inner as usize, Ordering::SeqCst) { + 0 => inner, + n => { + Box::from_raw(inner).remutex.destroy(); + n as *const _ + } } } } @@ -150,23 +165,38 @@ impl ReentrantMutex { } pub unsafe fn init(&self) { - c::InitializeCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())); + // SAFETY: The caller must ensure that the mutex is not moved or copied + unsafe { + c::InitializeCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())); + } } pub unsafe fn lock(&self) { - c::EnterCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())); + // SAFETY: The caller must ensure that the mutex is not moved or copied + unsafe { + c::EnterCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())); + } } #[inline] pub unsafe fn try_lock(&self) -> bool { - c::TryEnterCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())) != 0 + // SAFETY: The caller must ensure that the mutex is not moved or copied + unsafe { + c::TryEnterCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())) != 0 + } } pub unsafe fn unlock(&self) { - c::LeaveCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())); + // SAFETY: The caller must ensure that the mutex is not moved or copied + unsafe { + c::LeaveCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())); + } } pub unsafe fn destroy(&self) { - c::DeleteCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())); + // SAFETY: The caller must ensure that the mutex is not moved or copied + unsafe { + c::DeleteCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())); + } } } diff --git a/library/std/src/sys/windows/net.rs b/library/std/src/sys/windows/net.rs index 9e74454bc2335..2adee24bd1725 100644 --- a/library/std/src/sys/windows/net.rs +++ b/library/std/src/sys/windows/net.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![unstable(issue = "none", feature = "windows_net")] use crate::cmp; diff --git a/library/std/src/sys/windows/os.rs b/library/std/src/sys/windows/os.rs index 77c378a66afd7..649526ec893f1 100644 --- a/library/std/src/sys/windows/os.rs +++ b/library/std/src/sys/windows/os.rs @@ -1,5 +1,5 @@ //! Implementation of `std::os` functionality for Windows. - +#![deny(unsafe_op_in_unsafe_fn)] #![allow(nonstandard_style)] #[cfg(test)] diff --git a/library/std/src/sys/windows/os_str.rs b/library/std/src/sys/windows/os_str.rs index 7e09a4fd56130..fb8aa0945706b 100644 --- a/library/std/src/sys/windows/os_str.rs +++ b/library/std/src/sys/windows/os_str.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + /// The underlying OsString/OsStr implementation on Windows is a /// wrapper around the "WTF-8" encoding; see the `wtf8` module for more. use crate::borrow::Cow; diff --git a/library/std/src/sys/windows/path.rs b/library/std/src/sys/windows/path.rs index dda3ed68cfc95..8da654d2b86f6 100644 --- a/library/std/src/sys/windows/path.rs +++ b/library/std/src/sys/windows/path.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::ffi::OsStr; use crate::mem; use crate::path::Prefix; @@ -16,7 +18,8 @@ fn os_str_as_u8_slice(s: &OsStr) -> &[u8] { unsafe { mem::transmute(s) } } unsafe fn u8_slice_as_os_str(s: &[u8]) -> &OsStr { - mem::transmute(s) + // SAFETY: the safety contract must be upheld by the caller + unsafe { mem::transmute(s) } } #[inline] diff --git a/library/std/src/sys/windows/pipe.rs b/library/std/src/sys/windows/pipe.rs index 104a8db46596e..75e838a7b1cba 100644 --- a/library/std/src/sys/windows/pipe.rs +++ b/library/std/src/sys/windows/pipe.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::os::windows::prelude::*; use crate::ffi::OsStr; @@ -364,5 +366,6 @@ unsafe fn slice_to_end(v: &mut Vec) -> &mut [u8] { if v.capacity() == v.len() { v.reserve(1); } - slice::from_raw_parts_mut(v.as_mut_ptr().add(v.len()), v.capacity() - v.len()) + //SAFETY: `v` is a valid slice, and we reserve enough space + unsafe { slice::from_raw_parts_mut(v.as_mut_ptr().add(v.len()), v.capacity() - v.len()) } } diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index 243065b94b125..3900e4f78ae21 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![unstable(feature = "process_internals", issue = "none")] #[cfg(test)] diff --git a/library/std/src/sys/windows/rand.rs b/library/std/src/sys/windows/rand.rs index 87ea416bf675a..4648b2b444f17 100644 --- a/library/std/src/sys/windows/rand.rs +++ b/library/std/src/sys/windows/rand.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::io; use crate::mem; use crate::sys::c; diff --git a/library/std/src/sys/windows/rwlock.rs b/library/std/src/sys/windows/rwlock.rs index a769326352c40..00ac562c148cd 100644 --- a/library/std/src/sys/windows/rwlock.rs +++ b/library/std/src/sys/windows/rwlock.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::cell::UnsafeCell; use crate::sys::c; @@ -14,27 +16,27 @@ impl RWLock { } #[inline] pub unsafe fn read(&self) { - c::AcquireSRWLockShared(self.inner.get()) + unsafe { c::AcquireSRWLockShared(self.inner.get()) } } #[inline] pub unsafe fn try_read(&self) -> bool { - c::TryAcquireSRWLockShared(self.inner.get()) != 0 + unsafe { c::TryAcquireSRWLockShared(self.inner.get()) != 0 } } #[inline] pub unsafe fn write(&self) { - c::AcquireSRWLockExclusive(self.inner.get()) + unsafe { c::AcquireSRWLockExclusive(self.inner.get()) } } #[inline] pub unsafe fn try_write(&self) -> bool { - c::TryAcquireSRWLockExclusive(self.inner.get()) != 0 + unsafe { c::TryAcquireSRWLockExclusive(self.inner.get()) != 0 } } #[inline] pub unsafe fn read_unlock(&self) { - c::ReleaseSRWLockShared(self.inner.get()) + unsafe { c::ReleaseSRWLockShared(self.inner.get()) } } #[inline] pub unsafe fn write_unlock(&self) { - c::ReleaseSRWLockExclusive(self.inner.get()) + unsafe { c::ReleaseSRWLockExclusive(self.inner.get()) } } #[inline] diff --git a/library/std/src/sys/windows/stack_overflow.rs b/library/std/src/sys/windows/stack_overflow.rs index 187ad4e66c3ef..c5719c3cefee4 100644 --- a/library/std/src/sys/windows/stack_overflow.rs +++ b/library/std/src/sys/windows/stack_overflow.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![cfg_attr(test, allow(dead_code))] use crate::sys::c; @@ -9,12 +10,14 @@ impl Handler { pub unsafe fn new() -> Handler { // This API isn't available on XP, so don't panic in that case and just // pray it works out ok. - if c::SetThreadStackGuarantee(&mut 0x5000) == 0 { - if c::GetLastError() as u32 != c::ERROR_CALL_NOT_IMPLEMENTED as u32 { - panic!("failed to reserve stack space for exception handling"); + unsafe { + if c::SetThreadStackGuarantee(&mut 0x5000) == 0 { + if c::GetLastError() as u32 != c::ERROR_CALL_NOT_IMPLEMENTED as u32 { + panic!("failed to reserve stack space for exception handling"); + } } + Handler } - Handler } } @@ -31,11 +34,13 @@ extern "system" fn vectored_handler(ExceptionInfo: *mut c::EXCEPTION_POINTERS) - } pub unsafe fn init() { - if c::AddVectoredExceptionHandler(0, vectored_handler).is_null() { - panic!("failed to install exception handler"); + unsafe { + if c::AddVectoredExceptionHandler(0, vectored_handler).is_null() { + panic!("failed to install exception handler"); + } + // Set the thread stack guarantee for the main thread. + let _h = Handler::new(); } - // Set the thread stack guarantee for the main thread. - let _h = Handler::new(); } pub unsafe fn cleanup() {} diff --git a/library/std/src/sys/windows/stack_overflow_uwp.rs b/library/std/src/sys/windows/stack_overflow_uwp.rs index e7236cf359cd5..6954bdf89b1bb 100644 --- a/library/std/src/sys/windows/stack_overflow_uwp.rs +++ b/library/std/src/sys/windows/stack_overflow_uwp.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![cfg_attr(test, allow(dead_code))] pub struct Handler; diff --git a/library/std/src/sys/windows/stdio.rs b/library/std/src/sys/windows/stdio.rs index ff214497166be..90aa0ae42a2f4 100644 --- a/library/std/src/sys/windows/stdio.rs +++ b/library/std/src/sys/windows/stdio.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![unstable(issue = "none", feature = "windows_stdio")] use crate::char::decode_utf16; diff --git a/library/std/src/sys/windows/stdio_uwp.rs b/library/std/src/sys/windows/stdio_uwp.rs index 872511af862a7..3a032e3a55ffb 100644 --- a/library/std/src/sys/windows/stdio_uwp.rs +++ b/library/std/src/sys/windows/stdio_uwp.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![unstable(issue = "none", feature = "windows_stdio")] use crate::io; diff --git a/library/std/src/sys/windows/thread.rs b/library/std/src/sys/windows/thread.rs index 38839ea5e90ed..422f018f7e2c8 100644 --- a/library/std/src/sys/windows/thread.rs +++ b/library/std/src/sys/windows/thread.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::ffi::CStr; use crate::io; use crate::ptr; @@ -29,19 +31,24 @@ impl Thread { // Round up to the next 64 kB because that's what the NT kernel does, // might as well make it explicit. let stack_size = (stack + 0xfffe) & (!0xfffe); - let ret = c::CreateThread( - ptr::null_mut(), - stack_size, - thread_start, - p as *mut _, - c::STACK_SIZE_PARAM_IS_A_RESERVATION, - ptr::null_mut(), - ); + // SAFETY: the safety contract must be upheld by the caller + let ret = unsafe { + c::CreateThread( + ptr::null_mut(), + stack_size, + thread_start, + p as *mut _, + c::STACK_SIZE_PARAM_IS_A_RESERVATION, + ptr::null_mut(), + ) + }; return if ret as usize == 0 { - // The thread failed to start and as a result p was not consumed. Therefore, it is - // safe to reconstruct the box so that it gets deallocated. - drop(Box::from_raw(p)); + // SAFETY: The thread failed to start and as a result p was not consumed. + // Therefore, it is safe to reconstruct the box so that it gets deallocated. + unsafe { + drop(Box::from_raw(p)); + }; Err(io::Error::last_os_error()) } else { Ok(Thread { handle: Handle::new(ret) }) diff --git a/library/std/src/sys/windows/thread_local_dtor.rs b/library/std/src/sys/windows/thread_local_dtor.rs index 7be13bc4b2bc7..fc967239ec6fc 100644 --- a/library/std/src/sys/windows/thread_local_dtor.rs +++ b/library/std/src/sys/windows/thread_local_dtor.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![unstable(feature = "thread_local_internals", issue = "none")] #![cfg(target_thread_local)] diff --git a/library/std/src/sys/windows/thread_local_key.rs b/library/std/src/sys/windows/thread_local_key.rs index 82901871e78ae..9156965f1acff 100644 --- a/library/std/src/sys/windows/thread_local_key.rs +++ b/library/std/src/sys/windows/thread_local_key.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::mem; use crate::ptr; use crate::sync::atomic::AtomicPtr; @@ -47,23 +49,29 @@ pub type Dtor = unsafe extern "C" fn(*mut u8); #[inline] pub unsafe fn create(dtor: Option) -> Key { - let key = c::TlsAlloc(); + // SAFETY: `c::TlsAlloc()` is guranteed to be safe + let key = unsafe { c::TlsAlloc() }; assert!(key != c::TLS_OUT_OF_INDEXES); if let Some(f) = dtor { - register_dtor(key, f); + // SAFETY: the safety contract must be upheld by the caller + unsafe { + register_dtor(key, f); + } } key } #[inline] pub unsafe fn set(key: Key, value: *mut u8) { - let r = c::TlsSetValue(key, value as c::LPVOID); - debug_assert!(r != 0); + unsafe { + let r = c::TlsSetValue(key, value as c::LPVOID); + debug_assert!(r != 0); + } } #[inline] pub unsafe fn get(key: Key) -> *mut u8 { - c::TlsGetValue(key) as *mut u8 + unsafe { c::TlsGetValue(key) as *mut u8 } } #[inline] @@ -211,18 +219,25 @@ pub static p_thread_callback: unsafe extern "system" fn(c::LPVOID, c::DWORD, c:: #[allow(dead_code, unused_variables)] unsafe extern "system" fn on_tls_callback(h: c::LPVOID, dwReason: c::DWORD, pv: c::LPVOID) { if dwReason == c::DLL_THREAD_DETACH || dwReason == c::DLL_PROCESS_DETACH { - run_dtors(); + unsafe { + run_dtors(); + } } - // See comments above for what this is doing. Note that we don't need this - // trickery on GNU windows, just on MSVC. - reference_tls_used(); + // SAFETY: See comments above for what this is doing. + // Note that we don't need this trickery on GNU windows, just on MSVC. + unsafe { + reference_tls_used(); + } #[cfg(target_env = "msvc")] unsafe fn reference_tls_used() { extern "C" { static _tls_used: u8; } - crate::intrinsics::volatile_load(&_tls_used); + // SAFETY: This just inhibits linker removal as described above. + unsafe { + crate::intrinsics::volatile_load(&_tls_used); + } } #[cfg(not(target_env = "msvc"))] unsafe fn reference_tls_used() {} @@ -237,16 +252,21 @@ unsafe fn run_dtors() { } any_run = false; let mut cur = DTORS.load(SeqCst); - while !cur.is_null() { - let ptr = c::TlsGetValue((*cur).key); - if !ptr.is_null() { - c::TlsSetValue((*cur).key, ptr::null_mut()); - ((*cur).dtor)(ptr as *mut _); - any_run = true; - } + // SAFETY: If `cur` is non-null, it must be a + // valid pointer added by `register_dtor` + unsafe { + while !cur.is_null() { + let ptr = c::TlsGetValue((*cur).key); - cur = (*cur).next; + if !ptr.is_null() { + c::TlsSetValue((*cur).key, ptr::null_mut()); + ((*cur).dtor)(ptr as *mut _); + any_run = true; + } + + cur = (*cur).next; + } } } } diff --git a/library/std/src/sys/windows/time.rs b/library/std/src/sys/windows/time.rs index 91e4f7654840d..beb31a919ecac 100644 --- a/library/std/src/sys/windows/time.rs +++ b/library/std/src/sys/windows/time.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::cmp::Ordering; use crate::convert::TryInto; use crate::fmt; From fe6652f4f4a1497a5743b7ff0ddf0481f3ad2f10 Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Sun, 13 Sep 2020 11:45:44 -0500 Subject: [PATCH 02/11] Add SAFETY comments for mutex.rs --- library/std/src/sys/windows/mutex.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/windows/mutex.rs b/library/std/src/sys/windows/mutex.rs index c944528dcf0e8..5c32e179622ed 100644 --- a/library/std/src/sys/windows/mutex.rs +++ b/library/std/src/sys/windows/mutex.rs @@ -67,6 +67,7 @@ impl Mutex { #[inline] pub unsafe fn init(&mut self) {} pub unsafe fn lock(&self) { + // SAFETY: The caller must ensure that the mutex is not moved or copied unsafe { match kind() { Kind::SRWLock => c::AcquireSRWLockExclusive(raw(self)), @@ -83,6 +84,7 @@ impl Mutex { } } pub unsafe fn try_lock(&self) -> bool { + // SAFETY: The caller must ensure that the mutex is not moved or copied unsafe { match kind() { Kind::SRWLock => c::TryAcquireSRWLockExclusive(raw(self)) != 0, @@ -102,6 +104,7 @@ impl Mutex { } } pub unsafe fn unlock(&self) { + // SAFETY: The caller must ensure that the mutex is not moved or copied unsafe { match kind() { Kind::SRWLock => c::ReleaseSRWLockExclusive(raw(self)), @@ -114,6 +117,7 @@ impl Mutex { } } pub unsafe fn destroy(&self) { + // SAFETY: The caller must ensure that the mutex is not moved or copied unsafe { match kind() { Kind::SRWLock => {} @@ -134,8 +138,10 @@ impl Mutex { unsafe { inner.remutex.init(); } - let inner = Box::into_raw(inner); - + } + + unsafe fn flag_locked(&self) -> bool { + // SAFETY: The caller must ensure that the mutex is not moved or copied unsafe { match self.lock.compare_and_swap(0, inner as usize, Ordering::SeqCst) { 0 => inner, From b38c16487f66211734f624a5ca610e13e9edcb96 Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Sun, 13 Sep 2020 12:17:38 -0500 Subject: [PATCH 03/11] Add semicolon --- library/std/src/sys/windows/alloc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/windows/alloc.rs b/library/std/src/sys/windows/alloc.rs index f5cfeb7dfcd91..543c7f313ad7d 100644 --- a/library/std/src/sys/windows/alloc.rs +++ b/library/std/src/sys/windows/alloc.rs @@ -70,7 +70,7 @@ unsafe impl GlobalAlloc for System { 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()); } From 210635156940e385424b56086f8e60edc31e263c Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Sun, 13 Sep 2020 12:30:42 -0500 Subject: [PATCH 04/11] Delete semicolons --- library/std/src/sys/windows/alloc.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/windows/alloc.rs b/library/std/src/sys/windows/alloc.rs index 543c7f313ad7d..41f30fc20bfc5 100644 --- a/library/std/src/sys/windows/alloc.rs +++ b/library/std/src/sys/windows/alloc.rs @@ -65,10 +65,10 @@ unsafe impl GlobalAlloc for System { // 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); + 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); + c::HeapFree(c::GetProcessHeap(), 0, header.0 as c::LPVOID) } }; debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError()); From 475df91e2fcf3d9cbd226030df611702173010b7 Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Sun, 13 Sep 2020 13:28:59 -0500 Subject: [PATCH 05/11] Remove unsafe block --- library/std/src/sys/windows/alloc.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/library/std/src/sys/windows/alloc.rs b/library/std/src/sys/windows/alloc.rs index 41f30fc20bfc5..95efb6c222844 100644 --- a/library/std/src/sys/windows/alloc.rs +++ b/library/std/src/sys/windows/alloc.rs @@ -71,23 +71,21 @@ unsafe impl GlobalAlloc for System { c::HeapFree(c::GetProcessHeap(), 0, header.0 as c::LPVOID) } }; - debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError()); + debug_assert!(err != 0, "Failed to free heap memory: {}", unsafe {c::GetLastError()}); } #[inline] unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { - 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) - } + 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) } } } From c5304ff3fda10e8347e0ae188fc71874fe748cd4 Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Sun, 13 Sep 2020 13:43:38 -0500 Subject: [PATCH 06/11] Format --- library/std/src/sys/windows/alloc.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/library/std/src/sys/windows/alloc.rs b/library/std/src/sys/windows/alloc.rs index 95efb6c222844..38040d9976110 100644 --- a/library/std/src/sys/windows/alloc.rs +++ b/library/std/src/sys/windows/alloc.rs @@ -71,7 +71,7 @@ unsafe impl GlobalAlloc for System { c::HeapFree(c::GetProcessHeap(), 0, header.0 as c::LPVOID) } }; - debug_assert!(err != 0, "Failed to free heap memory: {}", unsafe {c::GetLastError()}); + debug_assert!(err != 0, "Failed to free heap memory: {}", unsafe { c::GetLastError() }); } #[inline] @@ -79,14 +79,10 @@ unsafe impl GlobalAlloc for System { 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 - } + 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) - } + unsafe { realloc_fallback(self, ptr, layout, new_size) } } } } From a6bb6d7ab4cfbc7af5cb234fa1a9a8d7c1b6f4fd Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Sun, 13 Sep 2020 15:25:47 -0500 Subject: [PATCH 07/11] Address review comments --- library/std/src/sys/windows/alloc.rs | 15 +++++++++++---- library/std/src/sys/windows/args.rs | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/windows/alloc.rs b/library/std/src/sys/windows/alloc.rs index 38040d9976110..9df2330f7b307 100644 --- a/library/std/src/sys/windows/alloc.rs +++ b/library/std/src/sys/windows/alloc.rs @@ -7,11 +7,17 @@ use crate::sys_common::alloc::{realloc_fallback, MIN_ALIGN}; #[repr(C)] struct Header(*mut u8); +/// # Safety +/// +/// There must be a `Header` at `ptr.offset(-1)`. unsafe fn get_header<'a>(ptr: *mut u8) -> &'a mut Header { // SAFETY: the safety contract must be upheld by the caller unsafe { &mut *(ptr as *mut Header).offset(-1) } } +/// # Safety +/// +/// `ptr`, once aligned, must have space for a Header at `ptr.offset(-1)`. unsafe fn align_ptr(ptr: *mut u8, align: usize) -> *mut u8 { // SAFETY: the safety contract must be upheld by the caller unsafe { @@ -30,7 +36,7 @@ unsafe fn allocate_with_flags(layout: Layout, flags: c::DWORD) -> *mut u8 { let ptr = unsafe { // SAFETY: The caller must ensure that - // `layout.size()` + `layout.size()` does not overflow. + // `layout.size()` + `layout.align()` does not overflow. let size = layout.size() + layout.align(); c::HeapAlloc(c::GetProcessHeap(), flags, size) }; @@ -71,17 +77,18 @@ unsafe impl GlobalAlloc for System { c::HeapFree(c::GetProcessHeap(), 0, header.0 as c::LPVOID) } }; + // SAFETY: `c::GetLastError()` cannot fail debug_assert!(err != 0, "Failed to free heap memory: {}", unsafe { c::GetLastError() }); } #[inline] unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + // SAFETY: HeapReAlloc/realloc_fallback is safe if ptr was allocated by this allocator + // and new_size is not 0. + debug_assert_ne!(new_size, 0); 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) } } } diff --git a/library/std/src/sys/windows/args.rs b/library/std/src/sys/windows/args.rs index 6b869307ad2e8..012356f036f26 100644 --- a/library/std/src/sys/windows/args.rs +++ b/library/std/src/sys/windows/args.rs @@ -54,7 +54,7 @@ unsafe fn parse_lp_cmd_line OsString>( const SPACE: u16 = ' ' as u16; let mut ret_val = Vec::new(); - //SAFETY: the caller must supply a valid pointer + // SAFETY: the caller must supply a pointer that is valid to dereference let mut cmd_line = unsafe { if lp_cmd_line.is_null() || *lp_cmd_line == 0 { ret_val.push(exe_name()); From 7c4738e8847e91c45afefda460c4b9d266b7c064 Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Fri, 18 Sep 2020 09:08:10 -0500 Subject: [PATCH 08/11] Fix merge issue --- library/std/src/sys/windows/mutex.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/library/std/src/sys/windows/mutex.rs b/library/std/src/sys/windows/mutex.rs index 5c32e179622ed..34f579671dfb9 100644 --- a/library/std/src/sys/windows/mutex.rs +++ b/library/std/src/sys/windows/mutex.rs @@ -134,15 +134,10 @@ impl Mutex { 0 => {} n => return n as *const _, } - let inner = box Inner { remutex: ReentrantMutex::uninitialized(), held: Cell::new(false) }; unsafe { + let inner = box Inner { remutex: ReentrantMutex::uninitialized(), held: Cell::new(false) }; inner.remutex.init(); - } - } - - unsafe fn flag_locked(&self) -> bool { - // SAFETY: The caller must ensure that the mutex is not moved or copied - unsafe { + let inner = Box::into_raw(inner); match self.lock.compare_and_swap(0, inner as usize, Ordering::SeqCst) { 0 => inner, n => { From ef8baea8558e5d64f37182eae83be1b0339c0814 Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Fri, 18 Sep 2020 09:32:42 -0500 Subject: [PATCH 09/11] "format" --- library/std/src/sys/windows/mutex.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/windows/mutex.rs b/library/std/src/sys/windows/mutex.rs index 34f579671dfb9..7787fed466ae1 100644 --- a/library/std/src/sys/windows/mutex.rs +++ b/library/std/src/sys/windows/mutex.rs @@ -135,7 +135,8 @@ impl Mutex { n => return n as *const _, } unsafe { - let inner = box Inner { remutex: ReentrantMutex::uninitialized(), held: Cell::new(false) }; + let inner = + box Inner { remutex: ReentrantMutex::uninitialized(), held: Cell::new(false) }; inner.remutex.init(); let inner = Box::into_raw(inner); match self.lock.compare_and_swap(0, inner as usize, Ordering::SeqCst) { @@ -182,9 +183,7 @@ impl ReentrantMutex { #[inline] pub unsafe fn try_lock(&self) -> bool { // SAFETY: The caller must ensure that the mutex is not moved or copied - unsafe { - c::TryEnterCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())) != 0 - } + unsafe { c::TryEnterCriticalSection(UnsafeCell::raw_get(self.inner.as_ptr())) != 0 } } pub unsafe fn unlock(&self) { From ff27e01c79fa1afb95d6476ee4285afad66edb6f Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Tue, 29 Sep 2020 11:59:23 -0500 Subject: [PATCH 10/11] Change `get_header` to use raw pointers --- library/std/src/sys/windows/alloc.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/windows/alloc.rs b/library/std/src/sys/windows/alloc.rs index 9df2330f7b307..a09d1687a0561 100644 --- a/library/std/src/sys/windows/alloc.rs +++ b/library/std/src/sys/windows/alloc.rs @@ -1,6 +1,7 @@ #![deny(unsafe_op_in_unsafe_fn)] use crate::alloc::{GlobalAlloc, Layout, System}; +use crate::ptr; use crate::sys::c; use crate::sys_common::alloc::{realloc_fallback, MIN_ALIGN}; @@ -10,7 +11,7 @@ struct Header(*mut u8); /// # Safety /// /// There must be a `Header` at `ptr.offset(-1)`. -unsafe fn get_header<'a>(ptr: *mut u8) -> &'a mut Header { +unsafe fn get_header<'a>(ptr: *mut u8) -> *mut Header { // SAFETY: the safety contract must be upheld by the caller unsafe { &mut *(ptr as *mut Header).offset(-1) } } @@ -22,7 +23,7 @@ unsafe fn align_ptr(ptr: *mut u8, align: usize) -> *mut u8 { // 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); + ptr::write(get_header(aligned), Header(ptr)); aligned } } @@ -74,7 +75,7 @@ unsafe impl GlobalAlloc for System { 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) + c::HeapFree(c::GetProcessHeap(), 0, (*header).0 as c::LPVOID) } }; // SAFETY: `c::GetLastError()` cannot fail From 9057f5e87f50e8173e0cecdaf59d02df852703d4 Mon Sep 17 00:00:00 2001 From: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com> Date: Sun, 18 Oct 2020 17:03:24 -0500 Subject: [PATCH 11/11] "fix merge issue" --- library/std/src/sys/windows/compat.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/windows/compat.rs b/library/std/src/sys/windows/compat.rs index f69da83c4c4a8..2505cc8e6396d 100644 --- a/library/std/src/sys/windows/compat.rs +++ b/library/std/src/sys/windows/compat.rs @@ -45,7 +45,12 @@ macro_rules! compat_fn { static PTR: AtomicUsize = AtomicUsize::new(0); #[allow(unused_variables)] - unsafe extern "system" fn fallback($($argname: $argtype),*) -> $rettype $body + unsafe extern "system" fn fallback($($argname: $argtype),*) -> $rettype { + #[allow(unused_unsafe)] + unsafe { + $body + } + } /// This address is stored in `PTR` to incidate an unavailable API. ///