Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make more Windows functions #![deny(unsafe_op_in_unsafe_fn)] #127763

Merged
merged 7 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions library/std/src/sys/os_str/wtf8.rs
Original file line number Diff line number Diff line change
@@ -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)]
ChrisDenton marked this conversation as resolved.
Show resolved Hide resolved

use crate::borrow::Cow;
use crate::collections::TryReserveError;
Expand Down Expand Up @@ -71,7 +72,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 +191,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
70 changes: 41 additions & 29 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,22 +138,31 @@ 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));
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().cast::<u8>(),
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)
}
}
}
ChrisDenton marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -230,20 +238,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 +273,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())
}
19 changes: 11 additions & 8 deletions library/std/src/sys/pal/windows/stack_overflow.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -27,11 +27,14 @@ unsafe extern "system" fn vectored_handler(ExceptionInfo: *mut c::EXCEPTION_POIN
}
}

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");
}
tgross35 marked this conversation as resolved.
Show resolved Hide resolved
// Set the thread stack guarantee for the main thread.
reserve_stack();
}
Loading
Loading