From 94b381d6fc6a225316bd5b60c6e48151ac868e35 Mon Sep 17 00:00:00 2001
From: Chris Denton <chris@chrisdenton.dev>
Date: Mon, 15 Jul 2024 13:39:05 +0000
Subject: [PATCH] Some Windows functions are safe

---
 std/src/sys/pal/windows/io.rs             | 29 +++++++++++------------
 std/src/sys/pal/windows/stack_overflow.rs | 19 ++++++++-------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/std/src/sys/pal/windows/io.rs b/std/src/sys/pal/windows/io.rs
index 86d457db50a38..bf3dfdfdd3e7d 100644
--- a/std/src/sys/pal/windows/io.rs
+++ b/std/src/sys/pal/windows/io.rs
@@ -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 @@ 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;
     }
@@ -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 @@ 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;
     }
diff --git a/std/src/sys/pal/windows/stack_overflow.rs b/std/src/sys/pal/windows/stack_overflow.rs
index ea89429cb83cc..467e21ab56a28 100644
--- a/std/src/sys/pal/windows/stack_overflow.rs
+++ b/std/src/sys/pal/windows/stack_overflow.rs
@@ -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 @@ 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");
+    }
     // Set the thread stack guarantee for the main thread.
     reserve_stack();
 }