From e457b77e2a9673db5e4e83f79d8961b6e2cb454b Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 2 Apr 2024 19:37:21 +0000 Subject: [PATCH 1/2] Avoid panicking unnecessarily on startup --- .../std/src/sys/pal/windows/stack_overflow.rs | 26 +++++-------------- .../src/sys/pal/windows/stack_overflow_uwp.rs | 9 +------ library/std/src/sys/pal/windows/thread.rs | 5 ++-- 3 files changed, 10 insertions(+), 30 deletions(-) diff --git a/library/std/src/sys/pal/windows/stack_overflow.rs b/library/std/src/sys/pal/windows/stack_overflow.rs index 627763da8561b..6f2b902c71e7d 100644 --- a/library/std/src/sys/pal/windows/stack_overflow.rs +++ b/library/std/src/sys/pal/windows/stack_overflow.rs @@ -3,21 +3,10 @@ use crate::sys::c; use crate::thread; -use super::api; - -pub struct Handler; - -impl Handler { - pub unsafe fn new() -> 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 - && api::get_last_error().code != c::ERROR_CALL_NOT_IMPLEMENTED - { - panic!("failed to reserve stack space for exception handling"); - } - Handler - } +/// Reserve stack space for use in stack overflow exceptions. +pub unsafe fn reserve_stack() { + let result = c::SetThreadStackGuarantee(&mut 0x5000); + debug_assert_ne!(result, 0, "failed to reserve stack space for exception handling"); } unsafe extern "system" fn vectored_handler(ExceptionInfo: *mut c::EXCEPTION_POINTERS) -> c::LONG { @@ -36,9 +25,8 @@ unsafe extern "system" fn vectored_handler(ExceptionInfo: *mut c::EXCEPTION_POIN } pub unsafe fn init() { - if c::AddVectoredExceptionHandler(0, Some(vectored_handler)).is_null() { - panic!("failed to install exception handler"); - } + let result = c::AddVectoredExceptionHandler(0, Some(vectored_handler)); + debug_assert!(!result.is_null(), "failed to install exception handler"); // Set the thread stack guarantee for the main thread. - let _h = Handler::new(); + reserve_stack(); } diff --git a/library/std/src/sys/pal/windows/stack_overflow_uwp.rs b/library/std/src/sys/pal/windows/stack_overflow_uwp.rs index afdf7f566ae51..9e9b3efaf1b14 100644 --- a/library/std/src/sys/pal/windows/stack_overflow_uwp.rs +++ b/library/std/src/sys/pal/windows/stack_overflow_uwp.rs @@ -1,11 +1,4 @@ #![cfg_attr(test, allow(dead_code))] -pub struct Handler; - -impl Handler { - pub fn new() -> Handler { - Handler - } -} - +pub unsafe fn reserve_stack() {} pub unsafe fn init() {} diff --git a/library/std/src/sys/pal/windows/thread.rs b/library/std/src/sys/pal/windows/thread.rs index 80eee4e078db7..fe174e1e34063 100644 --- a/library/std/src/sys/pal/windows/thread.rs +++ b/library/std/src/sys/pal/windows/thread.rs @@ -48,9 +48,8 @@ impl Thread { extern "system" fn thread_start(main: *mut c_void) -> c::DWORD { unsafe { - // Next, set up our stack overflow handler which may get triggered if we run - // out of stack. - let _handler = stack_overflow::Handler::new(); + // Next, reserve some stack space for if we otherwise run out of stack. + stack_overflow::reserve_stack(); // Finally, let's run some code. Box::from_raw(main as *mut Box)(); } From 7b8f93ef4c2b01b6cc7656123301f0f5e322b603 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 4 Apr 2024 10:48:11 +0000 Subject: [PATCH 2/2] Add comments about using debug_assert --- library/std/src/sys/pal/windows/stack_overflow.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/std/src/sys/pal/windows/stack_overflow.rs b/library/std/src/sys/pal/windows/stack_overflow.rs index 6f2b902c71e7d..f93f31026f818 100644 --- a/library/std/src/sys/pal/windows/stack_overflow.rs +++ b/library/std/src/sys/pal/windows/stack_overflow.rs @@ -6,6 +6,8 @@ use crate::thread; /// Reserve stack space for use in stack overflow exceptions. pub unsafe fn reserve_stack() { let result = 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"); } @@ -26,6 +28,8 @@ 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"); // Set the thread stack guarantee for the main thread. reserve_stack();