On Thu, 20 Aug 2026 08:40:46 GMT, Ashay Rane <[email protected]> wrote:
>> Prior to this patch, the `StackYellowPages` and `StackRedPages` >> influenced the total reserved stack memory, but they didn't change the >> minimum stack size specified as required by HotSpot to Windows. This >> patch adds the call to `SetThreadStackGuarantee()` to set the minimum >> stack size. >> >> The key change here is that the argument to `SetThreadStackGuarantee()` >> is set to one less than the total number of yellow pages. The one less >> page is because Windows uses a guard page (access to which tells Windows >> to commit more stack pages); in the terminal case, we want the guard >> page to coincide with the top-most yellow stack page, thus leaving all >> except one yellow page available as the stack size. >> >> Importantly, however, we do not use the red page count as the argument >> to `SetThreadStackGurantee()`, since the red stack pages are unavailable >> for handling recoverable overflows. The red page count impacts the >> _total reserved_ stack size, just not the _minimum_ stack size. Still, >> forcing the red stack page count to be included into the computation of >> the argument to `SetThreadStackGurantee()` causes HotSpot to fail with >> the assertion `assert(!in_vm) failed: Undersized StackShadowPages`, >> since Windows is unable to commit more stack pages due to the minimum >> stack size now being larger than just the yellow page count minus one. >> >> The accompanying test passes zero to `SetThreadStackGuarantee()` to >> probe the current minimum stack size, which we then compare against the >> expected size based on the yellow page count. The same test fails >> without this patch on both Windows/x64 and on Windows/ARM64. >> >> Validated this patch by running through all tier 1, 2, and 3 HotSpot >> jtreg tests on Windows/x64 and Windows/ARM64 in FastDebug config. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Ashay Rane has updated the pull request incrementally with one additional > commit since the last revision: > > Update comment block to be precise about `SetThreadStackGuarantee()` Dean and David, thanks so much for working with me on this change. Here's the core patch that I have so far: diff --git a/src/hotspot/os/windows/os_windows.cpp b/src/hotspot/os/windows/os_windows.cpp index 1f16a7ece4b..2364deed456 100644 --- a/src/hotspot/os/windows/os_windows.cpp +++ b/src/hotspot/os/windows/os_windows.cpp @@ -2746,17 +2746,24 @@ LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo) { exceptionInfo->ContextRecord); } } else if (exception_code == EXCEPTION_ACCESS_VIOLATION) { + // Decide the next steps based on the address that caused the exception. + address addr = (address) exception_record->ExceptionInformation[1]; + StackOverflow* overflow_state = thread->stack_overflow_state(); + if (overflow_state->in_stack_yellow_reserved_zone(addr)) { + assert(!in_vm, "Undersized StackShadowPages"); + overflow_state->disable_stack_yellow_reserved_zone(); + return in_java + ? Handle_Exception(exceptionInfo, SharedRuntime::continuation_for_implicit_exception(thread, pc, SharedRuntime::STACK_OVERFLOW)) + : EXCEPTION_CONTINUE_EXECUTION; + } else if (overflow_state->in_stack_red_zone(addr)) { + overflow_state->disable_stack_red_zone(); + tty->print_raw_cr("An unrecoverable stack overflow has occurred."); + VMError::report_and_die(t, exception_code, pc, exception_record, + exceptionInfo->ContextRecord); + } + if (in_java) { // Either stack overflow or null pointer exception. - address addr = (address) exception_record->ExceptionInformation[1]; - address stack_end = thread->stack_end(); - if (addr < stack_end && addr >= stack_end - os::vm_page_size()) { - // Stack overflow. - assert(!os::uses_stack_guard_pages(), - "should be caught by red zone code above."); - return Handle_Exception(exceptionInfo, - SharedRuntime::continuation_for_implicit_exception(thread, pc, SharedRuntime::STACK_OVERFLOW)); - } // Check for safepoint polling and implicit null // We only expect null pointers in the stubs (vtable) // the rest are checked explicitly now. @@ -3966,26 +3973,6 @@ bool os::pd_release_memory(char* addr, size_t bytes) { } bool os::pd_create_stack_guard_pages(char* addr, size_t size) { - // `SetThreadStackGuarantee()` specifies the minimum amount of stack that - // remains available when Windows raises `EXCEPTION_STACK_OVERFLOW`. HotSpot - // uses the yellow and reserved zones to handle recoverable stack overflows, - // so their combined size decides the argument to `SetThreadStackGuarantee(). - // The red zone is used only for unrecoverable overflows and is therefore - // excluded, although it remains part of the total stack guard-zone size. - // - // One page of the yellow and reserved zones is the guard page whose access - // triggers the stack overflow exception. That page is not part of the stack - // that is available to handle the exception, so we request one page less than - // the combined yellow and reserved zone size. - const size_t requested = StackOverflow::stack_yellow_reserved_zone_size() - - os::vm_page_size(); - ULONG ulong_requested = checked_cast<ULONG>(requested); - - if (SetThreadStackGuarantee(&ulong_requested) == 0) { - log_warning(os, thread)("Failed to set thread stack guarantee to %zu bytes " - "(error %lu)", requested, GetLastError()); - } - return os::commit_memory(addr, size, !ExecMem); } @@ -4072,7 +4059,7 @@ bool os::protect_memory(char* addr, size_t bytes, ProtType prot, bool os::guard_memory(char* addr, size_t bytes) { DWORD old_status; - return VirtualProtect(addr, bytes, PAGE_READWRITE | PAGE_GUARD, &old_status) != 0; + return VirtualProtect(addr, bytes, PAGE_NOACCESS, &old_status) != 0; } bool os::unguard_memory(char* addr, size_t bytes) { This makes the two key tests (HashOverflowTest and TestStackOverflowDuringInit) pass. Sadly, I am still not able to drop the 3-versus-2 default yellow page initialization despite this change. I am running the tier 1-3 HotSpot jtreg tests and if those pass, I'll push the change along with updated tests. Of course, let me know if you spot any flaws. Thanks again! ------------- PR Comment: https://git.openjdk.org/jdk/pull/32365#issuecomment-5379128302
