Auto merge of #127851 - tgross35:rollup-i39um74, r=tgross35

Rollup of 4 pull requests

Successful merges:

 - #127763 (Make more Windows functions `#![deny(unsafe_op_in_unsafe_fn)]`)
 - #127813 (Prevent double reference in generic futex)
 - #127847 (Reviewer on vacation)
 - #127850 (bootstrap: open `llvm-config` as r+w)

r? `@ghost`
`@rustbot` modify labels: rollup
This commit is contained in:
bors 2024-07-17 13:20:54 +00:00
commit 08cdc2fa1a
14 changed files with 121 additions and 97 deletions

View File

@ -1,3 +1,5 @@
#![forbid(unsafe_op_in_unsafe_fn)]
cfg_if::cfg_if! {
if #[cfg(any(
target_os = "windows",

View File

@ -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<u8> {
#[inline]
pub unsafe fn from_encoded_bytes_unchecked(s: Vec<u8>) -> 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]

View File

@ -1,5 +1,3 @@
#![deny(unsafe_op_in_unsafe_fn)]
use crate::alloc::{GlobalAlloc, Layout, System};
use crate::ffi::c_void;
use crate::ptr;

View File

@ -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<Self::Item> {
}
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<u64> {
_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)?;

View File

@ -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<T> Waitable for *const T {
unsafe impl<T> Waitable for *mut T {
type Atomic = AtomicPtr<T>;
}
unsafe impl<T> Futex for AtomicPtr<T> {}
pub fn wait_on_address<W: Waitable>(
address: &W::Atomic,
@ -61,14 +64,14 @@ pub fn wait_on_address<W: Waitable>(
}
}
pub fn wake_by_address_single<T>(address: &T) {
pub fn wake_by_address_single<T: Futex>(address: &T) {
unsafe {
let addr = ptr::from_ref(address).cast::<c_void>();
c::WakeByAddressSingle(addr);
}
}
pub fn wake_by_address_all<T>(address: &T) {
pub fn wake_by_address_all<T: Futex>(address: &T) {
unsafe {
let addr = ptr::from_ref(address).cast::<c_void>();
c::WakeByAddressAll(addr);
@ -80,11 +83,11 @@ pub fn futex_wait<W: Waitable>(futex: &W::Atomic, expected: W, timeout: Option<D
wait_on_address(futex, expected, timeout) || api::get_last_error() != WinError::TIMEOUT
}
pub fn futex_wake<T>(futex: &T) -> bool {
pub fn futex_wake<T: Futex>(futex: &T) -> bool {
wake_by_address_single(futex);
false
}
pub fn futex_wake_all<T>(futex: &T) {
pub fn futex_wake_all<T: Futex>(futex: &T) {
wake_by_address_all(futex)
}

View File

@ -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<u8>],
overlapped: *mut c::OVERLAPPED,
) -> io::Result<Option<usize>> {
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::<u8>(),
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 _))
}
}

View File

@ -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::<FILE_NAME_INFO>() 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::<FILE_NAME_INFO>() as u32,
)
};
if res == 0 {
return false;
}

View File

@ -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<OsString> {
}
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 {

View File

@ -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<u8>) -> io::Result<AsyncPipe<'a>> {
fn schedule_read(&mut self) -> io::Result<bool> {
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<u8>) -> &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())
}

View File

@ -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();
}

View File

@ -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<dyn FnOnce()>) -> io::Result<Thread> {
// 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<dyn FnOnce()>) -> io::Result<Thread> {
// 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<dyn FnOnce()>)();
// 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<dyn FnOnce()>)() };
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) {

View File

@ -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);
}
}
}

View File

@ -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));

View File

@ -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 = [