On Wed, 4 Nov 2026 17:33:21 GMT, Ashay Rane <[email protected]> wrote:
> Without this patch, HotSpot uses `VirtualAlloc()` with `MEM_COMMIT` to > make Windows convert stack pages from "reserved" to "committed", but > doing so does not update the `StackLimit` field in the Thread > Environment Block (TEB) tracked by Windows. Subsequent use of the stack > by HotSpot can cause the stack pointer to be _below_ `StackLimit`, which > breaks the invariant expected by the Win32 API functions like > `__chkstk()`. > > To fix this problem, this patch touches pages one by one, starting from > `StackLimit` down to the target address. This has the intended effect > of commiting the pages, but it also correctly updates `StackLimit`. > > The included test reliably fails without this patch, and it passes with > this patch. I validated this patch using Tier 1 through 3 tests in the > JDK, HotSpot, and LangTools test groups. > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). src/hotspot/os/windows/os_windows.cpp line 2696: > 2694: // and installs a new moving guard. Re-read StackLimit after every > touch > 2695: // because Windows may grow the stack by more than one page at a time. > 2696: while (target < (address)tib->StackLimit) { So this depends on StackLimit changing, otherwise we get into an infitnite loop? Is the compiler guaranteed to treat StackLimit as volatile? Maybe we should force it to volatile like we do for "probe", or use an atomic read. src/hotspot/os/windows/os_windows.cpp line 2704: > 2702: *probe = 0; > 2703: > 2704: if ((address)tib->StackLimit >= old_limit) { Is StackLimit > old_limit possible? Maybe we could asset that it never happens. test/hotspot/jtreg/runtime/StackGuardPages/libStackLimit.cpp line 31: > 29: > 30: // Compute an address that's comfortably below the existing > `StackLimit`. > 31: const size_t offset = 256 * 1024; Codex thinks this could cause a crash if someone runs the test witn -Xss256k. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33729#discussion_r4498272356 PR Review Comment: https://git.openjdk.org/jdk/pull/33729#discussion_r4498259823 PR Review Comment: https://git.openjdk.org/jdk/pull/33729#discussion_r4498283312
