On Mon, 24 Aug 2026 11:40:47 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: > > Temporarily bump `DEFAULT_STACK_SHADOW_PAGES` > > This is just to confirm whether setting `DEFAULT_STACK_SHADOW_PAGES` on > Windows/x64 to be the same as other x64 OSes resolves the problem in CI > (since I can't reproduce the problem locally). Coming back to this after being away for a few weeks due to some travels. My apologies for the long message, but it records all of the subtleties that I have leart so far. I've refactored and updated the handling of `EXCEPTION_STACK_OVERFLOW` and `EXCEPTION_ACCESS_VIOLATION` to, what I believe, is now a simpler and more intuitive implementation. In total, here is the list of changes: 1. Dean, you're right that we should be using `PAGE_NOACCESS` for the reserved + yellow + red zone, so `os::guard_memory()` calls `VirtualProtect()` with `PAGE_NOACCESS` instead of `PAGE_READWRITE | PAGE_GUARD`. 2. The logical conclusion from using `PAGE_NOACCESS` is that accesses to any of the reserved or yellow or red zones should raise `EXCEPTION_ACCESS_VIOLATION`. If the faulting address is in the yellow or reserved zone, the code for `EXCEPTION_ACCESS_VIOLATION` handles recoverable exceptions. If the address is in the red zone, the code handles unrecoverable exceptions. All other cases, like access violations from safepoint polling or nulls, are handled as before. 3. The tricky part is the `EXCEPTION_STACK_OVERFLOW`, which might seem unnecessary since `PAGE_NOACCESS` protects access to the reserved + yellow + red zones. Initially, I had the code for `EXCEPTION_STACK_OVERFLOW` unconditionally stop the JVM with a fatal error, but that's incorrect due to a very narrow case. When all of the stack pages are committed, there is one page at the high-address boundary of the reserved zone (if the reserved zone is non-empty, otherwise, at the boundary of the yellow zone) that has both the `PAGE_GUARD` attribute (because all preceeding pages have been committed) and the `PAGE_NOACCESS` attribute (because this page is part of the reserved or yellow zone). When this special page is accessed by the stack banging code, the Windows stack-growth machinery kicks in prior to validating the `PAGE_NOACCESS` attribute, and because the stack can't grow any further, Windows raises `EXCEPTION_STACK_OVERFLOW`. Here's some ASCII art to show the key point. <top of stack> usable stack reserved/yellow page #0 <-- accessing this raises `EXCEPTION_STACK_OVERFLOW` reserved/yellow page #1 <-- accessing this raises `EXCEPTION_ACCESS_VIOLATION` reserved/yellow page #2 <-- accessing this raises `EXCEPTION_ACCESS_VIOLATION` ... red page <-- accessing this raises `EXCEPTION_ACCESS_VIOLATION` <bottom of stack> There's an additional subtlety here, which is that the different stack banging sequences (page-by-page accesses versus large jumps) may land in either the reserved zone or yellow zone, so asserting that the accessed page is the highest reserved page is incorrect. Consequently, I updated the code so that an `EXCEPTION_STACK_OVERFLOW` in _any_ yellow or reserved page is treated as a recoverable stack overflow. Effectively, this makes the handling of `EXCEPTION_STACK_OVERFLOW` and `EXCEPTION_ACCESS_VIOLATION` identical except for the fatal error case. I've separated the recoverable versus unrecoverable stack overflow handling into separate functions that hopefully makes the code easier to read. In the process, I discovered a mismatch between the POSIX and the Windows code for handling reserved zone accesses. Side note: I think this sets us up for enabling the reserved stack on Windows, but I'd prefer to set `DEFAULT_STACK_RESERVED_PAGES` to 1 for Windows in a separate change. On Windows/x64, both ReservedStackTest.java and ReservedStackTestCompiler.java tests pass after setting `DEFAULT_STACK_RESERVED_PAGES` to 1, but the same change on Windows/ARM64 causes ReservedStackTest.java to fail. With the updated handlers for `EXCEPTION_{STACK_OVERFLOW|ACCESS_VIOLATION}` and `DEFAULT_STACK_RESERVED_PAGES` still set to 0 on Windows, I ran tier 1 through 3 tests from hotspot, jdk, langtools, and lib-test, and they all pass. ------------- PR Comment: https://git.openjdk.org/jdk/pull/32365#issuecomment-5715606597
