Skip to content

Commit

Permalink
Auto merge of #127851 - tgross35:rollup-i39um74, r=tgross35
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Jul 17, 2024
2 parents 3de0a7c + f99f61b commit 08cdc2f
Show file tree
Hide file tree
Showing 14 changed files with 121 additions and 97 deletions.
2 changes: 2 additions & 0 deletions library/std/src/sys/os_str/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![forbid(unsafe_op_in_unsafe_fn)]

cfg_if::cfg_if! {
if #[cfg(any(
target_os = "windows",
Expand Down
5 changes: 2 additions & 3 deletions library/std/src/sys/os_str/wtf8.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -71,7 +70,7 @@ impl Buf {

#[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 {
Expand Down Expand Up @@ -190,7 +189,7 @@ impl Slice {

#[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]
Expand Down
2 changes: 0 additions & 2 deletions library/std/src/sys/pal/windows/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![deny(unsafe_op_in_unsafe_fn)]

use crate::alloc::{GlobalAlloc, Layout, System};
use crate::ffi::c_void;
use crate::ptr;
Expand Down
23 changes: 14 additions & 9 deletions library/std/src/sys/pal/windows/fs.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![allow(unsafe_op_in_unsafe_fn)]
use core::ptr::addr_of;

use crate::os::windows::prelude::*;
Expand Down Expand Up @@ -795,10 +794,12 @@ impl<'a> Iterator for DirBuffIter<'a> {
}

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

Expand Down Expand Up @@ -897,7 +898,9 @@ impl IntoRawHandle for File {

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

Expand Down Expand Up @@ -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)?;
Expand Down
11 changes: 7 additions & 4 deletions library/std/src/sys/pal/windows/futex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub type SmallAtomic = AtomicU8;
/// Must be the underlying type of SmallAtomic
pub type SmallPrimitive = u8;

pub unsafe trait Futex {}
pub unsafe trait Waitable {
type Atomic;
}
Expand All @@ -24,6 +25,7 @@ macro_rules! unsafe_waitable_int {
unsafe impl Waitable for $int {
type Atomic = $atomic;
}
unsafe impl Futex for $atomic {}
)*
};
}
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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)
}
53 changes: 33 additions & 20 deletions library/std/src/sys/pal/windows/handle.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![unstable(issue = "none", feature = "windows_handle")]
#![allow(unsafe_op_in_unsafe_fn)]

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -73,7 +72,7 @@ impl IntoRawHandle for Handle {

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

Expand Down Expand Up @@ -139,13 +138,23 @@ impl Handle {

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) => {
Expand Down Expand Up @@ -230,20 +239,24 @@ impl Handle {

// 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
Expand All @@ -261,7 +274,7 @@ impl Handle {
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 _))
}
}
Expand Down
29 changes: 14 additions & 15 deletions library/std/src/sys/pal/windows/io.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -81,19 +80,17 @@ impl<'a> IoSliceMut<'a> {
}

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;
}
Expand All @@ -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;
}

Expand All @@ -120,12 +117,14 @@ unsafe fn msys_tty_on(handle: c::HANDLE) -> bool {
}
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;
}
Expand Down
17 changes: 11 additions & 6 deletions library/std/src/sys/pal/windows/os.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Implementation of `std::os` functionality for Windows.

#![allow(nonstandard_style)]
#![allow(unsafe_op_in_unsafe_fn)]

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 6 additions & 14 deletions library/std/src/sys/pal/windows/pipe.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
#![allow(unsafe_op_in_unsafe_fn)]
use crate::os::windows::prelude::*;

use crate::ffi::OsStr;
use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, Read};
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;
Expand Down Expand Up @@ -325,6 +323,7 @@ impl AnonPipe {
/// [`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,
Expand Down Expand Up @@ -479,8 +478,11 @@ impl<'a> 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
Expand Down Expand Up @@ -560,13 +562,3 @@ impl<'a> Drop for AsyncPipe<'a> {
}
}
}

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())
}
Loading

0 comments on commit 08cdc2f

Please sign in to comment.