> 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 with a new target base due to a merge 
or a rebase. The incremental webrev excludes the unrelated changes brought in 
by the merge/rebase. The pull request contains seven additional commits since 
the last revision:

 - Merge branch 'master' into JDK-8067946-setthreadstackguarantee
 - Simplify handling of stack overflow and access violation exceptions
 - 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).
 - Mark stack pages using `PAGE_NOACCESS` instead of `PAGE_GUARD`
 - Update comment block to be precise about `SetThreadStackGuarantee()`
 - Skip compilation of libTestWindowsStackPages.c on non-Windows platforms
   
   Also fix typo in variable name (`uload_requested` -> `ulong_requested`).
 - Use yellow page count to set minimum stack size
   
   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.

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/32365/files
  - new: https://git.openjdk.org/jdk/pull/32365/files/f2cded95..3e8118ae

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=32365&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=32365&range=04-05

  Stats: 268943 lines in 2854 files changed: 146748 ins; 105546 del; 16649 mod
  Patch: https://git.openjdk.org/jdk/pull/32365.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/32365/head:pull/32365

PR: https://git.openjdk.org/jdk/pull/32365

Reply via email to