From d1a3c1daeb5514993873b8e018224a3ea531cbf3 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Mon, 15 Jul 2024 13:38:23 +0000 Subject: [PATCH 01/10] Deny more windows unsafe_op_in_unsafe_fn --- library/std/src/sys/os_str/wtf8.rs | 5 +- library/std/src/sys/pal/windows/alloc.rs | 2 - library/std/src/sys/pal/windows/fs.rs | 23 +++++---- library/std/src/sys/pal/windows/handle.rs | 63 +++++++++++++---------- library/std/src/sys/pal/windows/os.rs | 17 +++--- 5 files changed, 63 insertions(+), 47 deletions(-) diff --git a/library/std/src/sys/os_str/wtf8.rs b/library/std/src/sys/os_str/wtf8.rs index edb923a4750..cc00fcaf13d 100644 --- a/library/std/src/sys/os_str/wtf8.rs +++ b/library/std/src/sys/os_str/wtf8.rs @@ -1,5 +1,6 @@ //! The underlying OsString/OsStr implementation on Windows is a //! wrapper around the "WTF-8" encoding; see the `wtf8` module for more. +#![deny(unsafe_op_in_unsafe_fn)] use crate::borrow::Cow; use crate::collections::TryReserveError; @@ -71,7 +72,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 +191,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..e63f5c9dec2 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)) } } } @@ -142,19 +141,23 @@ pub unsafe fn read_overlapped( buf: &mut [u8], 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)); - match res { - Ok(_) => Ok(Some(amt as usize)), - Err(e) => { - if e.raw_os_error() == Some(c::ERROR_IO_PENDING as i32) { - Ok(None) - } else if e.raw_os_error() == Some(c::ERROR_BROKEN_PIPE as i32) { - Ok(Some(0)) - } else { - Err(e) + // 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. + 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(), len, &mut amt, overlapped)); + match res { + Ok(_) => Ok(Some(amt as usize)), + Err(e) => { + if e.raw_os_error() == Some(c::ERROR_IO_PENDING as i32) { + Ok(None) + } else if e.raw_os_error() == Some(c::ERROR_BROKEN_PIPE as i32) { + Ok(Some(0)) + } else { + Err(e) + } } } } @@ -230,20 +233,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 +268,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/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 { From 37295e6268c7d6dd4290899d40b06b11284b2aab Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Mon, 15 Jul 2024 13:39:05 +0000 Subject: [PATCH 02/10] Some Windows functions are safe --- library/std/src/sys/pal/windows/io.rs | 29 +++++++++---------- .../std/src/sys/pal/windows/stack_overflow.rs | 19 +++++++----- 2 files changed, 25 insertions(+), 23 deletions(-) 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/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(); } From 59222346549ff66230d26a489705a27725e526b5 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Mon, 15 Jul 2024 13:50:58 +0000 Subject: [PATCH 03/10] allow(unsafe_op_in_unsafe_fn) on some functions These need to get their safety story straight --- library/std/src/sys/pal/windows/pipe.rs | 4 +++- library/std/src/sys/pal/windows/thread.rs | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/pal/windows/pipe.rs b/library/std/src/sys/pal/windows/pipe.rs index d8b785f027c..19364617e74 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; @@ -325,6 +324,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, @@ -561,6 +561,7 @@ fn drop(&mut self) { } } +#[allow(unsafe_op_in_unsafe_fn)] unsafe fn slice_to_end(v: &mut Vec) -> &mut [u8] { if v.capacity() == 0 { v.reserve(16); @@ -568,5 +569,6 @@ unsafe fn slice_to_end(v: &mut Vec) -> &mut [u8] { if v.capacity() == v.len() { v.reserve(1); } + // FIXME: Isn't this just spare_capacity_mut but worse? 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/thread.rs b/library/std/src/sys/pal/windows/thread.rs index 3648272a343..2bf6ccc6eef 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; @@ -23,6 +22,8 @@ pub struct Thread { impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements + #[allow(unsafe_op_in_unsafe_fn)] + // FIXME: check the internal safety pub unsafe fn new(stack: usize, p: Box) -> io::Result { let p = Box::into_raw(Box::new(p)); @@ -70,7 +71,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) { From 55c84e39cc22ab70c7e7e44f7ec7d6cadb1abff6 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 16 Jul 2024 20:24:57 +0000 Subject: [PATCH 04/10] Remove `slice_to_end` --- library/std/src/sys/pal/windows/handle.rs | 11 ++++++++--- library/std/src/sys/pal/windows/pipe.rs | 20 +++++--------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/library/std/src/sys/pal/windows/handle.rs b/library/std/src/sys/pal/windows/handle.rs index e63f5c9dec2..4553e3a232c 100644 --- a/library/std/src/sys/pal/windows/handle.rs +++ b/library/std/src/sys/pal/windows/handle.rs @@ -138,7 +138,7 @@ 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> { // SAFETY: We have exclusive access to the buffer and it's up to the caller to @@ -146,8 +146,13 @@ pub unsafe fn read_overlapped( 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(), len, &mut amt, overlapped)); + let res = cvt(c::ReadFile( + self.as_raw_handle(), + buf.as_mut_ptr().cast::(), + len, + &mut amt, + overlapped, + )); match res { Ok(_) => Ok(Some(amt as usize)), Err(e) => { diff --git a/library/std/src/sys/pal/windows/pipe.rs b/library/std/src/sys/pal/windows/pipe.rs index 19364617e74..7a309b9638b 100644 --- a/library/std/src/sys/pal/windows/pipe.rs +++ b/library/std/src/sys/pal/windows/pipe.rs @@ -5,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; @@ -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,15 +562,3 @@ fn drop(&mut self) { } } } - -#[allow(unsafe_op_in_unsafe_fn)] -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); - } - // FIXME: Isn't this just spare_capacity_mut but worse? - slice::from_raw_parts_mut(v.as_mut_ptr().add(v.len()), v.capacity() - v.len()) -} From 10b845cbc8c3ff699e66d10e7b5da00aef495ddd Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 16 Jul 2024 20:48:39 +0000 Subject: [PATCH 05/10] Add unsafe blocks in unsafe Thread::new --- library/std/src/sys/pal/windows/thread.rs | 30 +++++++++++++---------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/library/std/src/sys/pal/windows/thread.rs b/library/std/src/sys/pal/windows/thread.rs index 2bf6ccc6eef..668a3c05e20 100644 --- a/library/std/src/sys/pal/windows/thread.rs +++ b/library/std/src/sys/pal/windows/thread.rs @@ -22,28 +22,30 @@ pub struct Thread { impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements - #[allow(unsafe_op_in_unsafe_fn)] - // FIXME: check the internal safety pub unsafe fn new(stack: usize, p: Box) -> io::Result { let p = Box::into_raw(Box::new(p)); // 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()) }; @@ -51,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 } } From a33abbba9836944b467b7f65f090192d292d6080 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Wed, 17 Jul 2024 05:52:38 +0000 Subject: [PATCH 06/10] forbid(unsafe_op_in_unsafe_fn) in sys/os_str --- library/std/src/sys/os_str/mod.rs | 2 ++ library/std/src/sys/os_str/wtf8.rs | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) 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 cc00fcaf13d..806bf033dbc 100644 --- a/library/std/src/sys/os_str/wtf8.rs +++ b/library/std/src/sys/os_str/wtf8.rs @@ -1,7 +1,5 @@ //! The underlying OsString/OsStr implementation on Windows is a //! wrapper around the "WTF-8" encoding; see the `wtf8` module for more. -#![deny(unsafe_op_in_unsafe_fn)] - use crate::borrow::Cow; use crate::collections::TryReserveError; use crate::fmt; From 2043de12a364a8f6196bb58726af71b93abee888 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Wed, 17 Jul 2024 05:53:20 +0000 Subject: [PATCH 07/10] Narrow the scope of the ReadFile unsafe block --- library/std/src/sys/pal/windows/handle.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/library/std/src/sys/pal/windows/handle.rs b/library/std/src/sys/pal/windows/handle.rs index 4553e3a232c..aaa1831dcc2 100644 --- a/library/std/src/sys/pal/windows/handle.rs +++ b/library/std/src/sys/pal/windows/handle.rs @@ -143,7 +143,7 @@ pub unsafe fn read_overlapped( ) -> io::Result> { // 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. - unsafe { + 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( @@ -153,16 +153,17 @@ pub unsafe fn read_overlapped( &mut amt, overlapped, )); - match res { - Ok(_) => Ok(Some(amt as usize)), - Err(e) => { - if e.raw_os_error() == Some(c::ERROR_IO_PENDING as i32) { - Ok(None) - } else if e.raw_os_error() == Some(c::ERROR_BROKEN_PIPE as i32) { - Ok(Some(0)) - } else { - Err(e) - } + (res, amt) + }; + match res { + Ok(_) => Ok(Some(amt as usize)), + Err(e) => { + if e.raw_os_error() == Some(c::ERROR_IO_PENDING as i32) { + Ok(None) + } else if e.raw_os_error() == Some(c::ERROR_BROKEN_PIPE as i32) { + Ok(Some(0)) + } else { + Err(e) } } } From 0585c4a23ed7489ab21aa586eec4700b567cee03 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 16 Jul 2024 14:56:18 +0000 Subject: [PATCH 08/10] Prevent double reference in generic futex --- library/std/src/sys/pal/windows/futex.rs | 11 +++++++---- library/std/src/sys/sync/once/futex.rs | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/pal/windows/futex.rs b/library/std/src/sys/pal/windows/futex.rs index cb802fdd9c9..c54810e06cd 100644 --- a/library/std/src/sys/pal/windows/futex.rs +++ b/library/std/src/sys/pal/windows/futex.rs @@ -15,6 +15,7 @@ /// Must be the underlying type of SmallAtomic pub type SmallPrimitive = u8; +pub unsafe trait Futex {} pub unsafe trait Waitable { type Atomic; } @@ -24,6 +25,7 @@ macro_rules! unsafe_waitable_int { unsafe impl Waitable for $int { type Atomic = $atomic; } + unsafe impl Futex for $atomic {} )* }; } @@ -46,6 +48,7 @@ unsafe impl Waitable for *const T { unsafe impl Waitable for *mut T { type Atomic = AtomicPtr; } +unsafe impl Futex for AtomicPtr {} pub fn wait_on_address( address: &W::Atomic, @@ -61,14 +64,14 @@ pub fn wait_on_address( } } -pub fn wake_by_address_single(address: &T) { +pub fn wake_by_address_single(address: &T) { unsafe { let addr = ptr::from_ref(address).cast::(); c::WakeByAddressSingle(addr); } } -pub fn wake_by_address_all(address: &T) { +pub fn wake_by_address_all(address: &T) { unsafe { let addr = ptr::from_ref(address).cast::(); c::WakeByAddressAll(addr); @@ -80,11 +83,11 @@ pub fn futex_wait(futex: &W::Atomic, expected: W, timeout: Option(futex: &T) -> bool { +pub fn futex_wake(futex: &T) -> bool { wake_by_address_single(futex); false } -pub fn futex_wake_all(futex: &T) { +pub fn futex_wake_all(futex: &T) { wake_by_address_all(futex) } diff --git a/library/std/src/sys/sync/once/futex.rs b/library/std/src/sys/sync/once/futex.rs index 609085dcd47..8a231e65ad1 100644 --- a/library/std/src/sys/sync/once/futex.rs +++ b/library/std/src/sys/sync/once/futex.rs @@ -57,7 +57,7 @@ fn drop(&mut self) { // up on the Once. `futex_wake_all` does its own synchronization, hence // we do not need `AcqRel`. if self.state.swap(self.set_state_on_drop_to, Release) == QUEUED { - futex_wake_all(&self.state); + futex_wake_all(self.state); } } } From a216a34ef3e22568d45d854e7b7239427f9ad9b7 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Wed, 17 Jul 2024 07:53:02 +0000 Subject: [PATCH 09/10] jhpratt on vacation --- triagebot.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/triagebot.toml b/triagebot.toml index d47d54d45c0..763a5206c3c 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -905,7 +905,7 @@ cc = ["@kobzol"] [assign] warn_non_default_branch = true contributing_url = "https://rustc-dev-guide.rust-lang.org/getting-started.html" -users_on_vacation = ["jyn514"] +users_on_vacation = ["jyn514", "jhpratt"] [assign.adhoc_groups] compiler-team = [ From 3bee50736d833c5016068fa0b3a1b07f9b36f441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 17 Jul 2024 16:45:42 +0800 Subject: [PATCH 10/10] bootstrap: open llvm_config as r+w This previously failed on Windows because the `llvm_config` file was open as read-only. --- src/bootstrap/src/core/download.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index d01a910e815..56a8528d0a1 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -706,7 +706,7 @@ pub(crate) fn maybe_download_ci_llvm(&self) { let file_times = fs::FileTimes::new().set_accessed(now).set_modified(now); let llvm_config = llvm_root.join("bin").join(exe("llvm-config", self.build)); - let llvm_config_file = t!(File::open(llvm_config)); + let llvm_config_file = t!(File::options().write(true).open(llvm_config)); t!(llvm_config_file.set_times(file_times));