Create `windows/api.rs` for safer FFI

8 files changed, 212 insertions, 53 deletions

diff --git a/library/std/src/sys/windows/api.rs b/library/std/src/sys/windows/api.rs
new file mode 100644
index 00000000000..e9f0bbfbe2e
--- /dev/null
+++ b/library/std/src/sys/windows/api.rs
@@ -0,0 +1,157 @@
+//! # Safe(r) wrappers around Windows API functions.
+//!
+//! This module contains fairly thin wrappers around Windows API functions,
+//! aimed at centralising safety instead of having unsafe blocks spread
+//! throughout higher level code. This makes it much easier to audit FFI safety.
+//!
+//! Not all functions can be made completely safe without more context but in
+//! such cases we should still endeavour to reduce the caller's burden of safety
+//! as much as possible.
+//!
+//! ## Guidelines for wrappers
+//!
+//! Items here should be named similarly to their raw Windows API name, except
+//! that they follow Rust's case conventions. E.g. function names are
+//! lower_snake_case. The idea here is that it should be easy for a Windows
+//! C/C++ programmer to identify the underlying function that's being wrapped
+//! while not looking too out of place in Rust code.
+//!
+//! Every use of an `unsafe` block must have a related SAFETY comment, even if
+//! it's trivially safe (for example, see `get_last_error`). Public unsafe
+//! functions must document what the caller has to do to call them safely.
+//!
+//! Avoid unchecked `as` casts. For integers, either assert that the integer
+//! is in range or use `try_into` instead. For pointers, prefer to use
+//! `ptr.cast::<Type>()` when possible.
+//!
+//! This module must only depend on core and not on std types as the eventual
+//! hope is to have std depend on sys and not the other way around.
+//! However, some amount of glue code may currently be necessary so such code
+//! should go in sys/windows/mod.rs rather than here. See `IoResult` as an example.
+
+use core::ffi::c_void;
+use core::ptr::addr_of;
+
+use super::c;
+
+/// Helper method for getting the size of `T` as a u32.
+/// Errors at compile time if the size would overflow.
+///
+/// While a type larger than u32::MAX is unlikely, it is possible if only because of a bug.
+/// However, one key motivation for this function is to avoid the temptation to
+/// use frequent `as` casts. This is risky because they are too powerful.
+/// For example, the following will compile today:
+///
+/// `std::mem::size_of::<u64> as u32`
+///
+/// Note that `size_of` is never actually called, instead a function pointer is
+/// converted to a `u32`. Clippy would warn about this but, alas, it's not run
+/// on the standard library.
+const fn win32_size_of<T: Sized>() -> u32 {
+    // Const assert that the size is less than u32::MAX.
+    // Uses a trait to workaround restriction on using generic types in inner items.
+    trait Win32SizeOf: Sized {
+        const WIN32_SIZE_OF: u32 = {
+            let size = core::mem::size_of::<Self>();
+            assert!(size <= u32::MAX as usize);
+            size as u32
+        };
+    }
+    impl<T: Sized> Win32SizeOf for T {}
+
+    T::WIN32_SIZE_OF
+}
+
+/// The `SetFileInformationByHandle` function takes a generic parameter by
+/// making the user specify the type (class), a pointer to the data and its
+/// size. This trait allows attaching that information to a Rust type so that
+/// [`set_file_information_by_handle`] can be called safely.
+///
+/// This trait is designed so that it can support variable sized types.
+/// However, currently Rust's std only uses fixed sized structures.
+///
+/// # Safety
+///
+/// * `as_ptr` must return a pointer to memory that is readable up to `size` bytes.
+/// * `CLASS` must accurately reflect the type pointed to by `as_ptr`. E.g.
+/// the `FILE_BASIC_INFO` structure has the class `FileBasicInfo`.
+pub unsafe trait SetFileInformation {
+    /// The type of information to set.
+    const CLASS: i32;
+    /// A pointer to the file information to set.
+    fn as_ptr(&self) -> *const c_void;
+    /// The size of the type pointed to by `as_ptr`.
+    fn size(&self) -> u32;
+}
+/// Helper trait for implementing `SetFileInformation` for statically sized types.
+unsafe trait SizedSetFileInformation: Sized {
+    const CLASS: i32;
+}
+unsafe impl<T: SizedSetFileInformation> SetFileInformation for T {
+    const CLASS: i32 = T::CLASS;
+    fn as_ptr(&self) -> *const c_void {
+        addr_of!(*self).cast::<c_void>()
+    }
+    fn size(&self) -> u32 {
+        win32_size_of::<Self>()
+    }
+}
+
+// SAFETY: FILE_BASIC_INFO, FILE_END_OF_FILE_INFO, FILE_ALLOCATION_INFO,
+// FILE_DISPOSITION_INFO, FILE_DISPOSITION_INFO_EX and FILE_IO_PRIORITY_HINT_INFO
+// are all plain `repr(C)` structs that only contain primitive types.
+// The given information classes correctly match with the struct.
+unsafe impl SizedSetFileInformation for c::FILE_BASIC_INFO {
+    const CLASS: i32 = c::FileBasicInfo;
+}
+unsafe impl SizedSetFileInformation for c::FILE_END_OF_FILE_INFO {
+    const CLASS: i32 = c::FileEndOfFileInfo;
+}
+unsafe impl SizedSetFileInformation for c::FILE_ALLOCATION_INFO {
+    const CLASS: i32 = c::FileAllocationInfo;
+}
+unsafe impl SizedSetFileInformation for c::FILE_DISPOSITION_INFO {
+    const CLASS: i32 = c::FileDispositionInfo;
+}
+unsafe impl SizedSetFileInformation for c::FILE_DISPOSITION_INFO_EX {
+    const CLASS: i32 = c::FileDispositionInfoEx;
+}
+unsafe impl SizedSetFileInformation for c::FILE_IO_PRIORITY_HINT_INFO {
+    const CLASS: i32 = c::FileIoPriorityHintInfo;
+}
+
+#[inline]
+pub fn set_file_information_by_handle<T: SetFileInformation>(
+    handle: c::HANDLE,
+    info: &T,
+) -> Result<(), WinError> {
+    unsafe fn set_info(
+        handle: c::HANDLE,
+        class: i32,
+        info: *const c_void,
+        size: u32,
+    ) -> Result<(), WinError> {
+        let result = c::SetFileInformationByHandle(handle, class, info, size);
+        (result != 0).then_some(()).ok_or_else(|| get_last_error())
+    }
+    // SAFETY: The `SetFileInformation` trait ensures that this is safe.
+    unsafe { set_info(handle, T::CLASS, info.as_ptr(), info.size()) }
+}
+
+/// Gets the error from the last function.
+/// This must be called immediately after the function that sets the error to
+/// avoid the risk of another function overwriting it.
+pub fn get_last_error() -> WinError {
+    // SAFETY: This just returns a thread-local u32 and has no other effects.
+    unsafe { WinError { code: c::GetLastError() } }
+}
+
+/// An error code as returned by [`get_last_error`].
+///
+/// This is usually a 16-bit Win32 error code but may be a 32-bit HRESULT or NTSTATUS.
+/// Check the documentation of the Windows API function being called for expected errors.
+#[derive(Clone, Copy, PartialEq, Eq)]
+#[repr(transparent)]
+pub struct WinError {
+    pub code: u32,
+}
diff --git a/library/std/src/sys/windows/c/windows_sys.lst b/library/std/src/sys/windows/c/windows_sys.lst
index dad4a4223b2..bc5bfe80fb1 100644
--- a/library/std/src/sys/windows/c/windows_sys.lst
+++ b/library/std/src/sys/windows/c/windows_sys.lst
@@ -2224,6 +2224,7 @@ Windows.Win32.Storage.FileSystem.FILE_ACCESS_RIGHTS
 Windows.Win32.Storage.FileSystem.FILE_ADD_FILE
 Windows.Win32.Storage.FileSystem.FILE_ADD_SUBDIRECTORY
 Windows.Win32.Storage.FileSystem.FILE_ALL_ACCESS
+Windows.Win32.Storage.FileSystem.FILE_ALLOCATION_INFO
 Windows.Win32.Storage.FileSystem.FILE_APPEND_DATA
 Windows.Win32.Storage.FileSystem.FILE_ATTRIBUTE_ARCHIVE
 Windows.Win32.Storage.FileSystem.FILE_ATTRIBUTE_COMPRESSED
@@ -2284,6 +2285,7 @@ Windows.Win32.Storage.FileSystem.FILE_GENERIC_READ
 Windows.Win32.Storage.FileSystem.FILE_GENERIC_WRITE
 Windows.Win32.Storage.FileSystem.FILE_ID_BOTH_DIR_INFO
 Windows.Win32.Storage.FileSystem.FILE_INFO_BY_HANDLE_CLASS
+Windows.Win32.Storage.FileSystem.FILE_IO_PRIORITY_HINT_INFO
 Windows.Win32.Storage.FileSystem.FILE_LIST_DIRECTORY
 Windows.Win32.Storage.FileSystem.FILE_NAME_NORMALIZED
 Windows.Win32.Storage.FileSystem.FILE_NAME_OPENED
diff --git a/library/std/src/sys/windows/c/windows_sys.rs b/library/std/src/sys/windows/c/windows_sys.rs
index 20b44966a39..d66dfda6b8e 100644
--- a/library/std/src/sys/windows/c/windows_sys.rs
+++ b/library/std/src/sys/windows/c/windows_sys.rs
@@ -3107,6 +3107,16 @@ impl ::core::clone::Clone for FILETIME {
 pub type FILE_ACCESS_RIGHTS = u32;
 pub const FILE_ADD_FILE: FILE_ACCESS_RIGHTS = 2u32;
 pub const FILE_ADD_SUBDIRECTORY: FILE_ACCESS_RIGHTS = 4u32;
+#[repr(C)]
+pub struct FILE_ALLOCATION_INFO {
+    pub AllocationSize: i64,
+}
+impl ::core::marker::Copy for FILE_ALLOCATION_INFO {}
+impl ::core::clone::Clone for FILE_ALLOCATION_INFO {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
 pub const FILE_ALL_ACCESS: FILE_ACCESS_RIGHTS = 2032127u32;
 pub const FILE_APPEND_DATA: FILE_ACCESS_RIGHTS = 4u32;
 pub const FILE_ATTRIBUTE_ARCHIVE: FILE_FLAGS_AND_ATTRIBUTES = 32u32;
@@ -3248,6 +3258,16 @@ impl ::core::clone::Clone for FILE_ID_BOTH_DIR_INFO {
     }
 }
 pub type FILE_INFO_BY_HANDLE_CLASS = i32;
+#[repr(C)]
+pub struct FILE_IO_PRIORITY_HINT_INFO {
+    pub PriorityHint: PRIORITY_HINT,
+}
+impl ::core::marker::Copy for FILE_IO_PRIORITY_HINT_INFO {}
+impl ::core::clone::Clone for FILE_IO_PRIORITY_HINT_INFO {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
 pub const FILE_LIST_DIRECTORY: FILE_ACCESS_RIGHTS = 1u32;
 pub const FILE_NAME_NORMALIZED: GETFINALPATHNAMEBYHANDLE_FLAGS = 0u32;
 pub const FILE_NAME_OPENED: GETFINALPATHNAMEBYHANDLE_FLAGS = 8u32;
@@ -3753,6 +3773,7 @@ pub const PIPE_SERVER_END: NAMED_PIPE_MODE = 1u32;
 pub const PIPE_TYPE_BYTE: NAMED_PIPE_MODE = 0u32;
 pub const PIPE_TYPE_MESSAGE: NAMED_PIPE_MODE = 4u32;
 pub const PIPE_WAIT: NAMED_PIPE_MODE = 0u32;
+pub type PRIORITY_HINT = i32;
 pub type PROCESSOR_ARCHITECTURE = u16;
 pub type PROCESS_CREATION_FLAGS = u32;
 #[repr(C)]
diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs
index 6ded683aade..3bd8a30969c 100644
--- a/library/std/src/sys/windows/fs.rs
+++ b/library/std/src/sys/windows/fs.rs
@@ -19,7 +19,7 @@ use crate::thread;
 use core::ffi::c_void;
 
 use super::path::maybe_verbatim;
-use super::to_u16s;
+use super::{api, to_u16s, IoResult};
 
 pub struct File {
     handle: Handle,
@@ -123,7 +123,7 @@ impl Iterator for ReadDir {
             let mut wfd = mem::zeroed();
             loop {
                 if c::FindNextFileW(self.handle.0, &mut wfd) == 0 {
-                    if c::GetLastError() == c::ERROR_NO_MORE_FILES {
+                    if api::get_last_error().code == c::ERROR_NO_MORE_FILES {
                         return None;
                     } else {
                         return Some(Err(Error::last_os_error()));
@@ -318,17 +318,8 @@ impl File {
     }
 
     pub fn truncate(&self, size: u64) -> io::Result<()> {
-        let mut info = c::FILE_END_OF_FILE_INFO { EndOfFile: size as c::LARGE_INTEGER };
-        let size = mem::size_of_val(&info);
-        cvt(unsafe {
-            c::SetFileInformationByHandle(
-                self.handle.as_raw_handle(),
-                c::FileEndOfFileInfo,
-                &mut info as *mut _ as *mut _,
-                size as c::DWORD,
-            )
-        })?;
-        Ok(())
+        let info = c::FILE_END_OF_FILE_INFO { EndOfFile: size as i64 };
+        api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
     }
 
     #[cfg(not(target_vendor = "uwp"))]
@@ -565,23 +556,14 @@ impl File {
     }
 
     pub fn set_permissions(&self, perm: FilePermissions) -> io::Result<()> {
-        let mut info = c::FILE_BASIC_INFO {
+        let info = c::FILE_BASIC_INFO {
             CreationTime: 0,
             LastAccessTime: 0,
             LastWriteTime: 0,
             ChangeTime: 0,
             FileAttributes: perm.attrs,
         };
-        let size = mem::size_of_val(&info);
-        cvt(unsafe {
-            c::SetFileInformationByHandle(
-                self.handle.as_raw_handle(),
-                c::FileBasicInfo,
-                &mut info as *mut _ as *mut _,
-                size as c::DWORD,
-            )
-        })?;
-        Ok(())
+        api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
     }
 
     pub fn set_times(&self, times: FileTimes) -> io::Result<()> {
@@ -641,38 +623,20 @@ impl File {
     /// If the operation is not supported for this filesystem or OS version
     /// then errors will be `ERROR_NOT_SUPPORTED` or `ERROR_INVALID_PARAMETER`.
     fn posix_delete(&self) -> io::Result<()> {
-        let mut info = c::FILE_DISPOSITION_INFO_EX {
+        let info = c::FILE_DISPOSITION_INFO_EX {
             Flags: c::FILE_DISPOSITION_FLAG_DELETE
                 | c::FILE_DISPOSITION_FLAG_POSIX_SEMANTICS
                 | c::FILE_DISPOSITION_FLAG_IGNORE_READONLY_ATTRIBUTE,
         };
-        let size = mem::size_of_val(&info);
-        cvt(unsafe {
-            c::SetFileInformationByHandle(
-                self.handle.as_raw_handle(),
-                c::FileDispositionInfoEx,
-                &mut info as *mut _ as *mut _,
-                size as c::DWORD,
-            )
-        })?;
-        Ok(())
+        api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
     }
 
     /// Delete a file using win32 semantics. The file won't actually be deleted
     /// until all file handles are closed. However, marking a file for deletion
     /// will prevent anyone from opening a new handle to the file.
     fn win32_delete(&self) -> io::Result<()> {
-        let mut info = c::FILE_DISPOSITION_INFO { DeleteFile: c::TRUE as _ };
-        let size = mem::size_of_val(&info);
-        cvt(unsafe {
-            c::SetFileInformationByHandle(
-                self.handle.as_raw_handle(),
-                c::FileDispositionInfo,
-                &mut info as *mut _ as *mut _,
-                size as c::DWORD,
-            )
-        })?;
-        Ok(())
+        let info = c::FILE_DISPOSITION_INFO { DeleteFile: c::TRUE as _ };
+        api::set_file_information_by_handle(self.handle.as_raw_handle(), &info).io_result()
     }
 
     /// Fill the given buffer with as many directory entries as will fit.
diff --git a/library/std/src/sys/windows/mod.rs b/library/std/src/sys/windows/mod.rs
index b609ad2472c..c4e56e13be3 100644
--- a/library/std/src/sys/windows/mod.rs
+++ b/library/std/src/sys/windows/mod.rs
@@ -44,6 +44,18 @@ cfg_if::cfg_if! {
     }
 }
 
+mod api;
+
+/// Map a Result<T, WinError> to io::Result<T>.
+trait IoResult<T> {
+    fn io_result(self) -> crate::io::Result<T>;
+}
+impl<T> IoResult<T> for Result<T, api::WinError> {
+    fn io_result(self) -> crate::io::Result<T> {
+        self.map_err(|e| crate::io::Error::from_raw_os_error(e.code as i32))
+    }
+}
+
 // SAFETY: must be called only once during runtime initialization.
 // NOTE: this is not guaranteed to run, for example when Rust code is called externally.
 pub unsafe fn init(_argc: isize, _argv: *const *const u8, _sigpipe: u8) {
@@ -241,11 +253,11 @@ where
             // not an actual error.
             c::SetLastError(0);
             let k = match f1(buf.as_mut_ptr().cast::<u16>(), n as c::DWORD) {
-                0 if c::GetLastError() == 0 => 0,
+                0 if api::get_last_error().code == 0 => 0,
                 0 => return Err(crate::io::Error::last_os_error()),
                 n => n,
             } as usize;
-            if k == n && c::GetLastError() == c::ERROR_INSUFFICIENT_BUFFER {
+            if k == n && api::get_last_error().code == c::ERROR_INSUFFICIENT_BUFFER {
                 n = n.saturating_mul(2).min(c::DWORD::MAX as usize);
             } else if k > n {
                 n = k;
diff --git a/library/std/src/sys/windows/os.rs b/library/std/src/sys/windows/os.rs
index 58afca088ef..8cc905101de 100644
--- a/library/std/src/sys/windows/os.rs
+++ b/library/std/src/sys/windows/os.rs
@@ -17,10 +17,10 @@ use crate::ptr;
 use crate::slice;
 use crate::sys::{c, cvt};
 
-use super::to_u16s;
+use super::{api, to_u16s};
 
 pub fn errno() -> i32 {
-    unsafe { c::GetLastError() as i32 }
+    api::get_last_error().code as i32
 }
 
 /// Gets a detailed string description for the given error number.
@@ -336,7 +336,7 @@ fn home_dir_crt() -> Option<PathBuf> {
         super::fill_utf16_buf(
             |buf, mut sz| {
                 match c::GetUserProfileDirectoryW(token, buf, &mut sz) {
-                    0 if c::GetLastError() != c::ERROR_INSUFFICIENT_BUFFER => 0,
+                    0 if api::get_last_error().code != c::ERROR_INSUFFICIENT_BUFFER => 0,
                     0 => sz,
                     _ => sz - 1, // sz includes the null terminator
                 }
diff --git a/library/std/src/sys/windows/stack_overflow.rs b/library/std/src/sys/windows/stack_overflow.rs
index 0caf0a317a4..627763da856 100644
--- a/library/std/src/sys/windows/stack_overflow.rs
+++ b/library/std/src/sys/windows/stack_overflow.rs
@@ -3,6 +3,8 @@
 use crate::sys::c;
 use crate::thread;
 
+use super::api;
+
 pub struct Handler;
 
 impl Handler {
@@ -10,7 +12,7 @@ impl Handler {
         // This API isn't available on XP, so don't panic in that case and just
         // pray it works out ok.
         if c::SetThreadStackGuarantee(&mut 0x5000) == 0
-            && c::GetLastError() as u32 != c::ERROR_CALL_NOT_IMPLEMENTED as u32
+            && api::get_last_error().code != c::ERROR_CALL_NOT_IMPLEMENTED
         {
             panic!("failed to reserve stack space for exception handling");
         }
diff --git a/library/std/src/sys/windows/stdio.rs b/library/std/src/sys/windows/stdio.rs
index 3fcaaa508e3..a9ff909aa67 100644
--- a/library/std/src/sys/windows/stdio.rs
+++ b/library/std/src/sys/windows/stdio.rs
@@ -9,6 +9,7 @@ use crate::str;
 use crate::sys::c;
 use crate::sys::cvt;
 use crate::sys::handle::Handle;
+use crate::sys::windows::api;
 use core::str::utf8_char_width;
 
 #[cfg(test)]
@@ -369,7 +370,7 @@ fn read_u16s(handle: c::HANDLE, buf: &mut [MaybeUninit<u16>]) -> io::Result<usiz
 
         // ReadConsoleW returns success with ERROR_OPERATION_ABORTED for Ctrl-C or Ctrl-Break.
         // Explicitly check for that case here and try again.
-        if amount == 0 && unsafe { c::GetLastError() } == c::ERROR_OPERATION_ABORTED {
+        if amount == 0 && api::get_last_error().code == c::ERROR_OPERATION_ABORTED {
             continue;
         }
         break;