diff --git a/library/std/src/sys/os_str/mod.rs b/library/std/src/sys/os_str/mod.rs index b509729475b..345e661586d 100644 --- a/library/std/src/sys/os_str/mod.rs +++ b/library/std/src/sys/os_str/mod.rs @@ -1,3 +1,5 @@ +#![forbid(unsafe_op_in_unsafe_fn)] + cfg_if::cfg_if! { if #[cfg(any( target_os = "windows", diff --git a/library/std/src/sys/os_str/wtf8.rs b/library/std/src/sys/os_str/wtf8.rs index edb923a4750..806bf033dbc 100644 --- a/library/std/src/sys/os_str/wtf8.rs +++ b/library/std/src/sys/os_str/wtf8.rs @@ -1,6 +1,5 @@ //! 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; use crate::collections::TryReserveError; use crate::fmt; @@ -71,7 +70,7 @@ pub fn into_encoded_bytes(self) -> Vec { #[inline] pub unsafe fn from_encoded_bytes_unchecked(s: Vec) -> Self { - Self { inner: Wtf8Buf::from_bytes_unchecked(s) } + unsafe { Self { inner: Wtf8Buf::from_bytes_unchecked(s) } } } pub fn with_capacity(capacity: usize) -> Buf { @@ -190,7 +189,7 @@ pub fn as_encoded_bytes(&self) -> &[u8] { #[inline] pub unsafe fn from_encoded_bytes_unchecked(s: &[u8]) -> &Slice { - mem::transmute(Wtf8::from_bytes_unchecked(s)) + unsafe { mem::transmute(Wtf8::from_bytes_unchecked(s)) } } #[track_caller] diff --git a/library/std/src/sys/pal/windows/alloc.rs b/library/std/src/sys/pal/windows/alloc.rs index 9f0194492b0..987be6b69ee 100644 --- a/library/std/src/sys/pal/windows/alloc.rs +++ b/library/std/src/sys/pal/windows/alloc.rs @@ -1,5 +1,3 @@ -#![deny(unsafe_op_in_unsafe_fn)] - use crate::alloc::{GlobalAlloc, Layout, System}; use crate::ffi::c_void; use crate::ptr; diff --git a/library/std/src/sys/pal/windows/fs.rs b/library/std/src/sys/pal/windows/fs.rs index 85fd9153d53..48c39392047 100644 --- a/library/std/src/sys/pal/windows/fs.rs +++ b/library/std/src/sys/pal/windows/fs.rs @@ -1,4 +1,3 @@ -#![allow(unsafe_op_in_unsafe_fn)] use core::ptr::addr_of; use crate::os::windows::prelude::*; @@ -795,10 +794,12 @@ fn next(&mut self) -> Option { } unsafe fn from_maybe_unaligned<'a>(p: *const u16, len: usize) -> Cow<'a, [u16]> { - if p.is_aligned() { - Cow::Borrowed(crate::slice::from_raw_parts(p, len)) - } else { - Cow::Owned((0..len).map(|i| p.add(i).read_unaligned()).collect()) + unsafe { + if p.is_aligned() { + Cow::Borrowed(crate::slice::from_raw_parts(p, len)) + } else { + Cow::Owned((0..len).map(|i| p.add(i).read_unaligned()).collect()) + } } } @@ -897,7 +898,9 @@ fn into_raw_handle(self) -> RawHandle { impl FromRawHandle for File { unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { - Self { handle: FromInner::from_inner(FromRawHandle::from_raw_handle(raw_handle)) } + unsafe { + Self { handle: FromInner::from_inner(FromRawHandle::from_raw_handle(raw_handle)) } + } } } @@ -1427,10 +1430,12 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { _hDestinationFile: c::HANDLE, lpData: *const c_void, ) -> u32 { - if dwStreamNumber == 1 { - *(lpData as *mut i64) = StreamBytesTransferred; + unsafe { + if dwStreamNumber == 1 { + *(lpData as *mut i64) = StreamBytesTransferred; + } + c::PROGRESS_CONTINUE } - c::PROGRESS_CONTINUE } let pfrom = maybe_verbatim(from)?; let pto = maybe_verbatim(to)?; diff --git a/library/std/src/sys/pal/windows/handle.rs b/library/std/src/sys/pal/windows/handle.rs index ae9ea8ff584..aaa1831dcc2 100644 --- a/library/std/src/sys/pal/windows/handle.rs +++ b/library/std/src/sys/pal/windows/handle.rs @@ -1,5 +1,4 @@ #![unstable(issue = "none", feature = "windows_handle")] -#![allow(unsafe_op_in_unsafe_fn)] #[cfg(test)] mod tests; @@ -73,7 +72,7 @@ fn into_raw_handle(self) -> RawHandle { impl FromRawHandle for Handle { unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { - Self(FromRawHandle::from_raw_handle(raw_handle)) + unsafe { Self(FromRawHandle::from_raw_handle(raw_handle)) } } } @@ -139,13 +138,23 @@ pub fn read_buf(&self, mut cursor: BorrowedCursor<'_>) -> io::Result<()> { pub unsafe fn read_overlapped( &self, - buf: &mut [u8], + buf: &mut [mem::MaybeUninit], overlapped: *mut c::OVERLAPPED, ) -> io::Result> { - let len = cmp::min(buf.len(), u32::MAX as usize) as u32; - let mut amt = 0; - let res = - cvt(c::ReadFile(self.as_raw_handle(), buf.as_mut_ptr(), len, &mut amt, overlapped)); + // SAFETY: We have exclusive access to the buffer and it's up to the caller to + // ensure the OVERLAPPED pointer is valid for the lifetime of this function. + let (res, amt) = unsafe { + let len = cmp::min(buf.len(), u32::MAX as usize) as u32; + let mut amt = 0; + let res = cvt(c::ReadFile( + self.as_raw_handle(), + buf.as_mut_ptr().cast::(), + len, + &mut amt, + overlapped, + )); + (res, amt) + }; match res { Ok(_) => Ok(Some(amt as usize)), Err(e) => { @@ -230,20 +239,24 @@ unsafe fn synchronous_read( // The length is clamped at u32::MAX. let len = cmp::min(len, u32::MAX as usize) as u32; - let status = c::NtReadFile( - self.as_handle(), - ptr::null_mut(), - None, - ptr::null_mut(), - &mut io_status, - buf, - len, - offset.map(|n| n as _).as_ref(), - None, - ); + // SAFETY: It's up to the caller to ensure `buf` is writeable up to + // the provided `len`. + let status = unsafe { + c::NtReadFile( + self.as_handle(), + ptr::null_mut(), + None, + ptr::null_mut(), + &mut io_status, + buf, + len, + offset.map(|n| n as _).as_ref(), + None, + ) + }; let status = if status == c::STATUS_PENDING { - c::WaitForSingleObject(self.as_raw_handle(), c::INFINITE); + unsafe { c::WaitForSingleObject(self.as_raw_handle(), c::INFINITE) }; io_status.status() } else { status @@ -261,7 +274,7 @@ unsafe fn synchronous_read( status if c::nt_success(status) => Ok(io_status.Information), status => { - let error = c::RtlNtStatusToDosError(status); + let error = unsafe { c::RtlNtStatusToDosError(status) }; Err(io::Error::from_raw_os_error(error as _)) } } diff --git a/library/std/src/sys/pal/windows/io.rs b/library/std/src/sys/pal/windows/io.rs index 86d457db50a..bf3dfdfdd3e 100644 --- a/library/std/src/sys/pal/windows/io.rs +++ b/library/std/src/sys/pal/windows/io.rs @@ -1,4 +1,3 @@ -#![allow(unsafe_op_in_unsafe_fn)] use crate::marker::PhantomData; use crate::mem::size_of; use crate::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle}; @@ -81,19 +80,17 @@ pub fn as_mut_slice(&mut self) -> &mut [u8] { } pub fn is_terminal(h: &impl AsHandle) -> bool { - unsafe { handle_is_console(h.as_handle()) } + handle_is_console(h.as_handle()) } -unsafe fn handle_is_console(handle: BorrowedHandle<'_>) -> bool { - let handle = handle.as_raw_handle(); - +fn handle_is_console(handle: BorrowedHandle<'_>) -> bool { // A null handle means the process has no console. - if handle.is_null() { + if handle.as_raw_handle().is_null() { return false; } let mut out = 0; - if c::GetConsoleMode(handle, &mut out) != 0 { + if unsafe { c::GetConsoleMode(handle.as_raw_handle(), &mut out) != 0 } { // False positives aren't possible. If we got a console then we definitely have a console. return true; } @@ -102,9 +99,9 @@ unsafe fn handle_is_console(handle: BorrowedHandle<'_>) -> bool { msys_tty_on(handle) } -unsafe fn msys_tty_on(handle: c::HANDLE) -> bool { +fn msys_tty_on(handle: BorrowedHandle<'_>) -> bool { // Early return if the handle is not a pipe. - if c::GetFileType(handle) != c::FILE_TYPE_PIPE { + if unsafe { c::GetFileType(handle.as_raw_handle()) != c::FILE_TYPE_PIPE } { return false; } @@ -120,12 +117,14 @@ struct FILE_NAME_INFO { } let mut name_info = FILE_NAME_INFO { FileNameLength: 0, FileName: [0; c::MAX_PATH as usize] }; // Safety: buffer length is fixed. - let res = c::GetFileInformationByHandleEx( - handle, - c::FileNameInfo, - core::ptr::addr_of_mut!(name_info) as *mut c_void, - size_of::() as u32, - ); + let res = unsafe { + c::GetFileInformationByHandleEx( + handle.as_raw_handle(), + c::FileNameInfo, + core::ptr::addr_of_mut!(name_info) as *mut c_void, + size_of::() as u32, + ) + }; if res == 0 { return false; } diff --git a/library/std/src/sys/pal/windows/os.rs b/library/std/src/sys/pal/windows/os.rs index 0a9279e50ba..f1f4d3a5d26 100644 --- a/library/std/src/sys/pal/windows/os.rs +++ b/library/std/src/sys/pal/windows/os.rs @@ -1,7 +1,6 @@ //! Implementation of `std::os` functionality for Windows. #![allow(nonstandard_style)] -#![allow(unsafe_op_in_unsafe_fn)] #[cfg(test)] mod tests; @@ -305,15 +304,21 @@ pub fn getenv(k: &OsStr) -> Option { } pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { - let k = to_u16s(k)?; - let v = to_u16s(v)?; + // SAFETY: We ensure that k and v are null-terminated wide strings. + unsafe { + let k = to_u16s(k)?; + let v = to_u16s(v)?; - cvt(c::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr())).map(drop) + cvt(c::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr())).map(drop) + } } pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> { - let v = to_u16s(n)?; - cvt(c::SetEnvironmentVariableW(v.as_ptr(), ptr::null())).map(drop) + // SAFETY: We ensure that v is a null-terminated wide strings. + unsafe { + let v = to_u16s(n)?; + cvt(c::SetEnvironmentVariableW(v.as_ptr(), ptr::null())).map(drop) + } } pub fn temp_dir() -> PathBuf { diff --git a/library/std/src/sys/pal/windows/pipe.rs b/library/std/src/sys/pal/windows/pipe.rs index d8b785f027c..7a309b9638b 100644 --- a/library/std/src/sys/pal/windows/pipe.rs +++ b/library/std/src/sys/pal/windows/pipe.rs @@ -1,4 +1,3 @@ -#![allow(unsafe_op_in_unsafe_fn)] use crate::os::windows::prelude::*; use crate::ffi::OsStr; @@ -6,7 +5,6 @@ use crate::mem; use crate::path::Path; use crate::ptr; -use crate::slice; use crate::sync::atomic::AtomicUsize; use crate::sync::atomic::Ordering::Relaxed; use crate::sys::c; @@ -325,6 +323,7 @@ pub fn is_write_vectored(&self) -> bool { /// [`ReadFileEx`]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfileex /// [`WriteFileEx`]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-writefileex /// [Asynchronous Procedure Call]: https://docs.microsoft.com/en-us/windows/win32/sync/asynchronous-procedure-calls + #[allow(unsafe_op_in_unsafe_fn)] unsafe fn alertable_io_internal( &self, io: AlertableIoFn, @@ -479,8 +478,11 @@ fn new(pipe: Handle, dst: &'a mut Vec) -> io::Result> { fn schedule_read(&mut self) -> io::Result { assert_eq!(self.state, State::NotReading); let amt = unsafe { - let slice = slice_to_end(self.dst); - self.pipe.read_overlapped(slice, &mut *self.overlapped)? + if self.dst.capacity() == self.dst.len() { + let additional = if self.dst.capacity() == 0 { 16 } else { 1 }; + self.dst.reserve(additional); + } + self.pipe.read_overlapped(self.dst.spare_capacity_mut(), &mut *self.overlapped)? }; // If this read finished immediately then our overlapped event will @@ -560,13 +562,3 @@ fn drop(&mut self) { } } } - -unsafe fn slice_to_end(v: &mut Vec) -> &mut [u8] { - if v.capacity() == 0 { - v.reserve(16); - } - if v.capacity() == v.len() { - v.reserve(1); - } - slice::from_raw_parts_mut(v.as_mut_ptr().add(v.len()), v.capacity() - v.len()) -} diff --git a/library/std/src/sys/pal/windows/stack_overflow.rs b/library/std/src/sys/pal/windows/stack_overflow.rs index ea89429cb83..467e21ab56a 100644 --- a/library/std/src/sys/pal/windows/stack_overflow.rs +++ b/library/std/src/sys/pal/windows/stack_overflow.rs @@ -1,18 +1,18 @@ #![cfg_attr(test, allow(dead_code))] -#![allow(unsafe_op_in_unsafe_fn)] use crate::sys::c; use crate::thread; /// Reserve stack space for use in stack overflow exceptions. -pub unsafe fn reserve_stack() { - let result = c::SetThreadStackGuarantee(&mut 0x5000); +pub fn reserve_stack() { + let result = unsafe { c::SetThreadStackGuarantee(&mut 0x5000) }; // Reserving stack space is not critical so we allow it to fail in the released build of libstd. // We still use debug assert here so that CI will test that we haven't made a mistake calling the function. debug_assert_ne!(result, 0, "failed to reserve stack space for exception handling"); } unsafe extern "system" fn vectored_handler(ExceptionInfo: *mut c::EXCEPTION_POINTERS) -> i32 { + // SAFETY: It's up to the caller (which in this case is the OS) to ensure that `ExceptionInfo` is valid. unsafe { let rec = &(*(*ExceptionInfo).ExceptionRecord); let code = rec.ExceptionCode; @@ -27,11 +27,14 @@ pub unsafe fn reserve_stack() { } } -pub unsafe fn init() { - let result = c::AddVectoredExceptionHandler(0, Some(vectored_handler)); - // Similar to the above, adding the stack overflow handler is allowed to fail - // but a debug assert is used so CI will still test that it normally works. - debug_assert!(!result.is_null(), "failed to install exception handler"); +pub fn init() { + // SAFETY: `vectored_handler` has the correct ABI and is safe to call during exception handling. + unsafe { + let result = c::AddVectoredExceptionHandler(0, Some(vectored_handler)); + // Similar to the above, adding the stack overflow handler is allowed to fail + // but a debug assert is used so CI will still test that it normally works. + debug_assert!(!result.is_null(), "failed to install exception handler"); + } // Set the thread stack guarantee for the main thread. reserve_stack(); } diff --git a/library/std/src/sys/pal/windows/thread.rs b/library/std/src/sys/pal/windows/thread.rs index 3648272a343..668a3c05e20 100644 --- a/library/std/src/sys/pal/windows/thread.rs +++ b/library/std/src/sys/pal/windows/thread.rs @@ -1,4 +1,3 @@ -#![allow(unsafe_op_in_unsafe_fn)] use crate::ffi::CStr; use crate::io; use crate::num::NonZero; @@ -28,21 +27,25 @@ pub unsafe fn new(stack: usize, p: Box) -> io::Result { // CreateThread rounds up values for the stack size to the nearest page size (at least 4kb). // If a value of zero is given then the default stack size is used instead. - let ret = c::CreateThread( - ptr::null_mut(), - stack, - Some(thread_start), - p as *mut _, - c::STACK_SIZE_PARAM_IS_A_RESERVATION, - ptr::null_mut(), - ); - let ret = HandleOrNull::from_raw_handle(ret); + // SAFETY: `thread_start` has the right ABI for a thread's entry point. + // `p` is simply passed through to the new thread without being touched. + let ret = unsafe { + let ret = c::CreateThread( + ptr::null_mut(), + stack, + Some(thread_start), + p as *mut _, + c::STACK_SIZE_PARAM_IS_A_RESERVATION, + ptr::null_mut(), + ); + HandleOrNull::from_raw_handle(ret) + }; return if let Ok(handle) = ret.try_into() { Ok(Thread { handle: Handle::from_inner(handle) }) } else { // 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)); + unsafe { drop(Box::from_raw(p)) }; Err(io::Error::last_os_error()) }; @@ -50,7 +53,9 @@ pub unsafe fn new(stack: usize, p: Box) -> io::Result { // Next, reserve some stack space for if we otherwise run out of stack. stack_overflow::reserve_stack(); // Finally, let's run some code. - Box::from_raw(main as *mut Box)(); + // SAFETY: We are simply recreating the box that was leaked earlier. + // It's the responsibility of the one who call `Thread::new` to ensure this is safe to call here. + unsafe { Box::from_raw(main as *mut Box)() }; 0 } } @@ -70,7 +75,7 @@ pub fn set_name(name: &CStr) { /// /// `name` must end with a zero value pub unsafe fn set_name_wide(name: &[u16]) { - c::SetThreadDescription(c::GetCurrentThread(), name.as_ptr()); + unsafe { c::SetThreadDescription(c::GetCurrentThread(), name.as_ptr()) }; } pub fn join(self) {