Skip to content

Commit bd7b0e1

Browse files
authored
Unrolled build for rust-lang#116816
Rollup merge of rust-lang#116816 - ChrisDenton:api.rs, r=workingjubilee Create `windows/api.rs` for safer FFI FFI is inherently unsafe. For memory safety we need to assert that some contract is being upheld on both sides of the FFI, though of course we can only ever check our side. In Rust, `unsafe` blocks are used to assert safety and `// SAFETY` comments describing why it is safe. Currently in sys/windows we have a lot of this unsafety spread all over the place, with variations on the same unsafe patterns repeated. And because of the repitition and frequency, we're a bit lax with the safety comments. This PR aims to fix this and to make FFI safety more auditable by creating an `api` module with the goal of centralising and consolidating this unsafety. It contains thin wrappers around the Windows API that make most functions safe to call or, if that's not possible, then at least safer. Note that its goal is *only* to address safety. It does not stray far from the Windows API and intentionally does not attempt to make higher lever wrappers around, for example, file handles. This is better left to the existing modules. The windows/api.rs file has a top level comment to help future contributors understand the intent of the module and the design decisions made. I chose two functions as a first tentative step towards the above goal: - [`GetLastError`](https://learn.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror) is trivially safe. There's no reason to wrap it in an `unsafe` block every time. So I simply created a safe `get_last_error` wrapper. - [`SetFileInformationByHandle`](https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfileinformationbyhandle) is more complex. It essentially takes a generic type but over a C API which necessitates some amount of ceremony. Rather than implementing similar unsafe patterns in multiple places, I provide a safe `set_file_information_by_handle` that takes a Rusty generic type and handles converting that to the form required by the C FFI. r? libs
2 parents 17659c7 + 3733316 commit bd7b0e1

File tree

8 files changed

+212
-53
lines changed

8 files changed

+212
-53
lines changed

library/std/src/sys/windows/api.rs

+157
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
//! # Safe(r) wrappers around Windows API functions.
2+
//!
3+
//! This module contains fairly thin wrappers around Windows API functions,
4+
//! aimed at centralising safety instead of having unsafe blocks spread
5+
//! throughout higher level code. This makes it much easier to audit FFI safety.
6+
//!
7+
//! Not all functions can be made completely safe without more context but in
8+
//! such cases we should still endeavour to reduce the caller's burden of safety
9+
//! as much as possible.
10+
//!
11+
//! ## Guidelines for wrappers
12+
//!
13+
//! Items here should be named similarly to their raw Windows API name, except
14+
//! that they follow Rust's case conventions. E.g. function names are
15+
//! lower_snake_case. The idea here is that it should be easy for a Windows
16+
//! C/C++ programmer to identify the underlying function that's being wrapped
17+
//! while not looking too out of place in Rust code.
18+
//!
19+
//! Every use of an `unsafe` block must have a related SAFETY comment, even if
20+
//! it's trivially safe (for example, see `get_last_error`). Public unsafe
21+
//! functions must document what the caller has to do to call them safely.
22+
//!
23+
//! Avoid unchecked `as` casts. For integers, either assert that the integer
24+
//! is in range or use `try_into` instead. For pointers, prefer to use
25+
//! `ptr.cast::<Type>()` when possible.
26+
//!
27+
//! This module must only depend on core and not on std types as the eventual
28+
//! hope is to have std depend on sys and not the other way around.
29+
//! However, some amount of glue code may currently be necessary so such code
30+
//! should go in sys/windows/mod.rs rather than here. See `IoResult` as an example.
31+
32+
use core::ffi::c_void;
33+
use core::ptr::addr_of;
34+
35+
use super::c;
36+
37+
/// Helper method for getting the size of `T` as a u32.
38+
/// Errors at compile time if the size would overflow.
39+
///
40+
/// While a type larger than u32::MAX is unlikely, it is possible if only because of a bug.
41+
/// However, one key motivation for this function is to avoid the temptation to
42+
/// use frequent `as` casts. This is risky because they are too powerful.
43+
/// For example, the following will compile today:
44+
///
45+
/// `std::mem::size_of::<u64> as u32`
46+
///
47+
/// Note that `size_of` is never actually called, instead a function pointer is
48+
/// converted to a `u32`. Clippy would warn about this but, alas, it's not run
49+
/// on the standard library.
50+
const fn win32_size_of<T: Sized>() -> u32 {
51+
// Const assert that the size is less than u32::MAX.
52+
// Uses a trait to workaround restriction on using generic types in inner items.
53+
trait Win32SizeOf: Sized {
54+
const WIN32_SIZE_OF: u32 = {
55+
let size = core::mem::size_of::<Self>();
56+
assert!(size <= u32::MAX as usize);
57+
size as u32
58+
};
59+
}
60+
impl<T: Sized> Win32SizeOf for T {}
61+
62+
T::WIN32_SIZE_OF
63+
}
64+
65+
/// The `SetFileInformationByHandle` function takes a generic parameter by
66+
/// making the user specify the type (class), a pointer to the data and its
67+
/// size. This trait allows attaching that information to a Rust type so that
68+
/// [`set_file_information_by_handle`] can be called safely.
69+
///
70+
/// This trait is designed so that it can support variable sized types.
71+
/// However, currently Rust's std only uses fixed sized structures.
72+
///
73+
/// # Safety
74+
///
75+
/// * `as_ptr` must return a pointer to memory that is readable up to `size` bytes.
76+
/// * `CLASS` must accurately reflect the type pointed to by `as_ptr`. E.g.
77+
/// the `FILE_BASIC_INFO` structure has the class `FileBasicInfo`.
78+
pub unsafe trait SetFileInformation {
79+
/// The type of information to set.
80+
const CLASS: i32;
81+
/// A pointer to the file information to set.
82+
fn as_ptr(&self) -> *const c_void;
83+
/// The size of the type pointed to by `as_ptr`.
84+
fn size(&self) -> u32;
85+
}
86+
/// Helper trait for implementing `SetFileInformation` for statically sized types.
87+
unsafe trait SizedSetFileInformation: Sized {
88+
const CLASS: i32;
89+
}
90+
unsafe impl<T: SizedSetFileInformation> SetFileInformation for T {
91+
const CLASS: i32 = T::CLASS;
92+
fn as_ptr(&self) -> *const c_void {
93+
addr_of!(*self).cast::<c_void>()
94+
}
95+
fn size(&self) -> u32 {
96+
win32_size_of::<Self>()
97+
}
98+
}
99+
100+
// SAFETY: FILE_BASIC_INFO, FILE_END_OF_FILE_INFO, FILE_ALLOCATION_INFO,
101+
// FILE_DISPOSITION_INFO, FILE_DISPOSITION_INFO_EX and FILE_IO_PRIORITY_HINT_INFO
102+
// are all plain `repr(C)` structs that only contain primitive types.
103+
// The given information classes correctly match with the struct.
104+
unsafe impl SizedSetFileInformation for c::FILE_BASIC_INFO {
105+
const CLASS: i32 = c::FileBasicInfo;
106+
}
107+
unsafe impl SizedSetFileInformation for c::FILE_END_OF_FILE_INFO {
108+
const CLASS: i32 = c::FileEndOfFileInfo;
109+
}
110+
unsafe impl SizedSetFileInformation for c::FILE_ALLOCATION_INFO {
111+
const CLASS: i32 = c::FileAllocationInfo;
112+
}
113+
unsafe impl SizedSetFileInformation for c::FILE_DISPOSITION_INFO {
114+
const CLASS: i32 = c::FileDispositionInfo;
115+
}
116+
unsafe impl SizedSetFileInformation for c::FILE_DISPOSITION_INFO_EX {
117+
const CLASS: i32 = c::FileDispositionInfoEx;
118+
}
119+
unsafe impl SizedSetFileInformation for c::FILE_IO_PRIORITY_HINT_INFO {
120+
const CLASS: i32 = c::FileIoPriorityHintInfo;
121+
}
122+
123+
#[inline]
124+
pub fn set_file_information_by_handle<T: SetFileInformation>(
125+
handle: c::HANDLE,
126+
info: &T,
127+
) -> Result<(), WinError> {
128+
unsafe fn set_info(
129+
handle: c::HANDLE,
130+
class: i32,
131+
info: *const c_void,
132+
size: u32,
133+
) -> Result<(), WinError> {
134+
let result = c::SetFileInformationByHandle(handle, class, info, size);
135+
(result != 0).then_some(()).ok_or_else(|| get_last_error())
136+
}
137+
// SAFETY: The `SetFileInformation` trait ensures that this is safe.
138+
unsafe { set_info(handle, T::CLASS, info.as_ptr(), info.size()) }
139+
}
140+
141+
/// Gets the error from the last function.
142+
/// This must be called immediately after the function that sets the error to
143+
/// avoid the risk of another function overwriting it.
144+
pub fn get_last_error() -> WinError {
145+
// SAFETY: This just returns a thread-local u32 and has no other effects.
146+
unsafe { WinError { code: c::GetLastError() } }
147+
}
148+
149+
/// An error code as returned by [`get_last_error`].
150+
///
151+
/// This is usually a 16-bit Win32 error code but may be a 32-bit HRESULT or NTSTATUS.
152+
/// Check the documentation of the Windows API function being called for expected errors.
153+
#[derive(Clone, Copy, PartialEq, Eq)]
154+
#[repr(transparent)]
155+
pub struct WinError {
156+
pub code: u32,
157+
}

library/std/src/sys/windows/c/windows_sys.lst

+2
Original file line numberDiff line numberDiff line change
@@ -2224,6 +2224,7 @@ Windows.Win32.Storage.FileSystem.FILE_ACCESS_RIGHTS
22242224
Windows.Win32.Storage.FileSystem.FILE_ADD_FILE
22252225
Windows.Win32.Storage.FileSystem.FILE_ADD_SUBDIRECTORY
22262226
Windows.Win32.Storage.FileSystem.FILE_ALL_ACCESS
2227+
Windows.Win32.Storage.FileSystem.FILE_ALLOCATION_INFO
22272228
Windows.Win32.Storage.FileSystem.FILE_APPEND_DATA
22282229
Windows.Win32.Storage.FileSystem.FILE_ATTRIBUTE_ARCHIVE
22292230
Windows.Win32.Storage.FileSystem.FILE_ATTRIBUTE_COMPRESSED
@@ -2284,6 +2285,7 @@ Windows.Win32.Storage.FileSystem.FILE_GENERIC_READ
22842285
Windows.Win32.Storage.FileSystem.FILE_GENERIC_WRITE
22852286
Windows.Win32.Storage.FileSystem.FILE_ID_BOTH_DIR_INFO
22862287
Windows.Win32.Storage.FileSystem.FILE_INFO_BY_HANDLE_CLASS
2288+
Windows.Win32.Storage.FileSystem.FILE_IO_PRIORITY_HINT_INFO
22872289
Windows.Win32.Storage.FileSystem.FILE_LIST_DIRECTORY
22882290
Windows.Win32.Storage.FileSystem.FILE_NAME_NORMALIZED
22892291
Windows.Win32.Storage.FileSystem.FILE_NAME_OPENED

library/std/src/sys/windows/c/windows_sys.rs

+21
Original file line numberDiff line numberDiff line change
@@ -3129,6 +3129,16 @@ impl ::core::clone::Clone for FILETIME {
31293129
pub type FILE_ACCESS_RIGHTS = u32;
31303130
pub const FILE_ADD_FILE: FILE_ACCESS_RIGHTS = 2u32;
31313131
pub const FILE_ADD_SUBDIRECTORY: FILE_ACCESS_RIGHTS = 4u32;
3132+
#[repr(C)]
3133+
pub struct FILE_ALLOCATION_INFO {
3134+
pub AllocationSize: i64,
3135+
}
3136+
impl ::core::marker::Copy for FILE_ALLOCATION_INFO {}
3137+
impl ::core::clone::Clone for FILE_ALLOCATION_INFO {
3138+
fn clone(&self) -> Self {
3139+
*self
3140+
}
3141+
}
31323142
pub const FILE_ALL_ACCESS: FILE_ACCESS_RIGHTS = 2032127u32;
31333143
pub const FILE_APPEND_DATA: FILE_ACCESS_RIGHTS = 4u32;
31343144
pub const FILE_ATTRIBUTE_ARCHIVE: FILE_FLAGS_AND_ATTRIBUTES = 32u32;
@@ -3270,6 +3280,16 @@ impl ::core::clone::Clone for FILE_ID_BOTH_DIR_INFO {
32703280
}
32713281
}
32723282
pub type FILE_INFO_BY_HANDLE_CLASS = i32;
3283+
#[repr(C)]
3284+
pub struct FILE_IO_PRIORITY_HINT_INFO {
3285+
pub PriorityHint: PRIORITY_HINT,
3286+
}
3287+
impl ::core::marker::Copy for FILE_IO_PRIORITY_HINT_INFO {}
3288+
impl ::core::clone::Clone for FILE_IO_PRIORITY_HINT_INFO {
3289+
fn clone(&self) -> Self {
3290+
*self
3291+
}
3292+
}
32733293
pub const FILE_LIST_DIRECTORY: FILE_ACCESS_RIGHTS = 1u32;
32743294
pub const FILE_NAME_NORMALIZED: GETFINALPATHNAMEBYHANDLE_FLAGS = 0u32;
32753295
pub const FILE_NAME_OPENED: GETFINALPATHNAMEBYHANDLE_FLAGS = 8u32;
@@ -3775,6 +3795,7 @@ pub const PIPE_SERVER_END: NAMED_PIPE_MODE = 1u32;
37753795
pub const PIPE_TYPE_BYTE: NAMED_PIPE_MODE = 0u32;
37763796
pub const PIPE_TYPE_MESSAGE: NAMED_PIPE_MODE = 4u32;
37773797
pub const PIPE_WAIT: NAMED_PIPE_MODE = 0u32;
3798+
pub type PRIORITY_HINT = i32;
37783799
pub type PROCESSOR_ARCHITECTURE = u16;
37793800
pub type PROCESS_CREATION_FLAGS = u32;
37803801
#[repr(C)]

library/std/src/sys/windows/fs.rs

+10-46
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::thread;
1919
use core::ffi::c_void;
2020

2121
use super::path::maybe_verbatim;
22-
use super::to_u16s;
22+
use super::{api, to_u16s, IoResult};
2323

2424
pub struct File {
2525
handle: Handle,
@@ -123,7 +123,7 @@ impl Iterator for ReadDir {
123123
let mut wfd = mem::zeroed();
124124
loop {
125125
if c::FindNextFileW(self.handle.0, &mut wfd) == 0 {
126-
if c::GetLastError() == c::ERROR_NO_MORE_FILES {
126+
if api::get_last_error().code == c::ERROR_NO_MORE_FILES {
127127
return None;
128128
} else {
129129
return Some(Err(Error::last_os_error()));
@@ -318,17 +318,8 @@ impl File {
318318
}
319319

320320
pub fn truncate(&self, size: u64) -> io::Result<()> {
321-
let mut info = c::FILE_END_OF_FILE_INFO { EndOfFile: size as c::LARGE_INTEGER };
322-
let size = mem::size_of_val(&info);
323-
cvt(unsafe {
324-
c::SetFileInformationByHandle(
325-
self.handle.as_raw_handle(),
326-
c::FileEndOfFileInfo,
327-
&mut info as *mut _ as *mut _,
328-
size as c::DWORD,
329-
)
330-
})?;
331-
Ok(())
321+
let info = c::FILE_END_OF_FILE_INFO { EndOfFile: size as i64 };
322+
api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
332323
}
333324

334325
#[cfg(not(target_vendor = "uwp"))]
@@ -565,23 +556,14 @@ impl File {
565556
}
566557

567558
pub fn set_permissions(&self, perm: FilePermissions) -> io::Result<()> {
568-
let mut info = c::FILE_BASIC_INFO {
559+
let info = c::FILE_BASIC_INFO {
569560
CreationTime: 0,
570561
LastAccessTime: 0,
571562
LastWriteTime: 0,
572563
ChangeTime: 0,
573564
FileAttributes: perm.attrs,
574565
};
575-
let size = mem::size_of_val(&info);
576-
cvt(unsafe {
577-
c::SetFileInformationByHandle(
578-
self.handle.as_raw_handle(),
579-
c::FileBasicInfo,
580-
&mut info as *mut _ as *mut _,
581-
size as c::DWORD,
582-
)
583-
})?;
584-
Ok(())
566+
api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
585567
}
586568

587569
pub fn set_times(&self, times: FileTimes) -> io::Result<()> {
@@ -641,38 +623,20 @@ impl File {
641623
/// If the operation is not supported for this filesystem or OS version
642624
/// then errors will be `ERROR_NOT_SUPPORTED` or `ERROR_INVALID_PARAMETER`.
643625
fn posix_delete(&self) -> io::Result<()> {
644-
let mut info = c::FILE_DISPOSITION_INFO_EX {
626+
let info = c::FILE_DISPOSITION_INFO_EX {
645627
Flags: c::FILE_DISPOSITION_FLAG_DELETE
646628
| c::FILE_DISPOSITION_FLAG_POSIX_SEMANTICS
647629
| c::FILE_DISPOSITION_FLAG_IGNORE_READONLY_ATTRIBUTE,
648630
};
649-
let size = mem::size_of_val(&info);
650-
cvt(unsafe {
651-
c::SetFileInformationByHandle(
652-
self.handle.as_raw_handle(),
653-
c::FileDispositionInfoEx,
654-
&mut info as *mut _ as *mut _,
655-
size as c::DWORD,
656-
)
657-
})?;
658-
Ok(())
631+
api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
659632
}
660633

661634
/// Delete a file using win32 semantics. The file won't actually be deleted
662635
/// until all file handles are closed. However, marking a file for deletion
663636
/// will prevent anyone from opening a new handle to the file.
664637
fn win32_delete(&self) -> io::Result<()> {
665-
let mut info = c::FILE_DISPOSITION_INFO { DeleteFile: c::TRUE as _ };
666-
let size = mem::size_of_val(&info);
667-
cvt(unsafe {
668-
c::SetFileInformationByHandle(
669-
self.handle.as_raw_handle(),
670-
c::FileDispositionInfo,
671-
&mut info as *mut _ as *mut _,
672-
size as c::DWORD,
673-
)
674-
})?;
675-
Ok(())
638+
let info = c::FILE_DISPOSITION_INFO { DeleteFile: c::TRUE as _ };
639+
api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
676640
}
677641

678642
/// Fill the given buffer with as many directory entries as will fit.

library/std/src/sys/windows/mod.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,18 @@ cfg_if::cfg_if! {
4444
}
4545
}
4646

47+
mod api;
48+
49+
/// Map a Result<T, WinError> to io::Result<T>.
50+
trait IoResult<T> {
51+
fn io_result(self) -> crate::io::Result<T>;
52+
}
53+
impl<T> IoResult<T> for Result<T, api::WinError> {
54+
fn io_result(self) -> crate::io::Result<T> {
55+
self.map_err(|e| crate::io::Error::from_raw_os_error(e.code as i32))
56+
}
57+
}
58+
4759
// SAFETY: must be called only once during runtime initialization.
4860
// NOTE: this is not guaranteed to run, for example when Rust code is called externally.
4961
pub unsafe fn init(_argc: isize, _argv: *const *const u8, _sigpipe: u8) {
@@ -241,11 +253,11 @@ where
241253
// not an actual error.
242254
c::SetLastError(0);
243255
let k = match f1(buf.as_mut_ptr().cast::<u16>(), n as c::DWORD) {
244-
0 if c::GetLastError() == 0 => 0,
256+
0 if api::get_last_error().code == 0 => 0,
245257
0 => return Err(crate::io::Error::last_os_error()),
246258
n => n,
247259
} as usize;
248-
if k == n && c::GetLastError() == c::ERROR_INSUFFICIENT_BUFFER {
260+
if k == n && api::get_last_error().code == c::ERROR_INSUFFICIENT_BUFFER {
249261
n = n.saturating_mul(2).min(c::DWORD::MAX as usize);
250262
} else if k > n {
251263
n = k;

library/std/src/sys/windows/os.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ use crate::ptr;
1717
use crate::slice;
1818
use crate::sys::{c, cvt};
1919

20-
use super::to_u16s;
20+
use super::{api, to_u16s};
2121

2222
pub fn errno() -> i32 {
23-
unsafe { c::GetLastError() as i32 }
23+
api::get_last_error().code as i32
2424
}
2525

2626
/// Gets a detailed string description for the given error number.
@@ -336,7 +336,7 @@ fn home_dir_crt() -> Option<PathBuf> {
336336
super::fill_utf16_buf(
337337
|buf, mut sz| {
338338
match c::GetUserProfileDirectoryW(token, buf, &mut sz) {
339-
0 if c::GetLastError() != c::ERROR_INSUFFICIENT_BUFFER => 0,
339+
0 if api::get_last_error().code != c::ERROR_INSUFFICIENT_BUFFER => 0,
340340
0 => sz,
341341
_ => sz - 1, // sz includes the null terminator
342342
}

library/std/src/sys/windows/stack_overflow.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,16 @@
33
use crate::sys::c;
44
use crate::thread;
55

6+
use super::api;
7+
68
pub struct Handler;
79

810
impl Handler {
911
pub unsafe fn new() -> Handler {
1012
// This API isn't available on XP, so don't panic in that case and just
1113
// pray it works out ok.
1214
if c::SetThreadStackGuarantee(&mut 0x5000) == 0
13-
&& c::GetLastError() as u32 != c::ERROR_CALL_NOT_IMPLEMENTED as u32
15+
&& api::get_last_error().code != c::ERROR_CALL_NOT_IMPLEMENTED
1416
{
1517
panic!("failed to reserve stack space for exception handling");
1618
}

0 commit comments

Comments
 (0)