8277299: STACK_OVERFLOW in Java_sun_awt_shell_Win32ShellFolder2_getIconBits

[aamarsh]

Makes colorBits and maskBits dynamic variables to force allocation on the heap instead of the stack and avoid a stackoverflow. Also adds a test to confirm this stackoverflow is no longer happening.

Tier1 and the newly added test passes on Windows. Clean backport.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8277299: STACK_OVERFLOW in Java_sun_awt_shell_Win32ShellFolder2_getIconBits

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk17u-dev pull/98/head:pull/98
$ git checkout pull/98

Update a local copy of the PR:
$ git checkout pull/98
$ git pull https://git.openjdk.java.net/jdk17u-dev pull/98/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 98

View PR using the GUI difftool:
$ git pr show -t 98

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk17u-dev/pull/98.diff

[bridgekeeper]

👋 Welcome back aamarsh! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

This backport pull request has now been updated with issue from the original commit.

[openjdk]

@aamarsh This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8277299: STACK_OVERFLOW in Java_sun_awt_shell_Win32ShellFolder2_getIconBits

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 118 new commits pushed to the master branch:

• 5159ffd: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently
• 0965996: 8274750: java/io/File/GetXSpace.java failed: '/dev': 191488 != 190976
• bda9cb4: 8273387: remove some unreferenced gtk-related functions
• c36c1c2: 8279702: [macosx] ignore xcodebuild warnings on M1
• dba064a: 8277180: Intrinsify recursive ObjectMonitor locking for C2 x64 and A64
• cd73095: 8277449: compiler/vectorapi/TestLongVectorNeg.java fails with release VMs
• f723588: 8275643: C2's unaryOp vector intrinsic does not properly handle LongVector.neg
• 27cbe24: 8271056: C2: "assert(no_dead_loop) failed: dead loop detected" due to cmoving identity
• ff3eec1: 8275847: Scheduling fails with "too many D-U pinch points" on small method
• 5d54bf5: 8274944: AppCDS dump causes SEGV in VM thread while adjusting lambda proxy class info
• ... and 108 more: https://git.openjdk.java.net/jdk17u-dev/compare/e92b2bc0b48a2c04c612cfd5a2913d73ad16b3f4...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[mlbridge]

Webrevs

• 02: Full - Incremental (263b2917)
• 01: Full - Incremental (d7bbe8dd)
• 00: Full (263b2917)

[GoeLin]

Hi, we do not touch copyrights in backports. The original patch was correct.
If your backport is not marked clean by the bot, please elaborate where you did resolve files.
(Touching the copyright would break clean backports en gross.)

[aamarsh]

@GoeLin Thanks for clarifying. I reset to my first commit. The clean tag is back and the PR is ready for review!

[GoeLin]

@aamarsh, as it is clean now, you don't need a review. But you need a jdk17u-fix-yes label on the JBS Issue.
For that please see 6a) in https://wiki.openjdk.java.net/display/JDKUpdates/How+to+contribute+a+fix
Add a comment "Fix request 17u" and describe why you want to backport this and what you think about the risk etc.
Then add jdk17u-fix-request tag. Then wait until you get jdk11u-fix-yes. After this, you can integrate.

[aamarsh]

Fix request 17u: This backport avoids a stack overflow in Windows 32 bit binaries, which some external contributors release. Additionally this is a nice change for Windows 64 bit binaries as well, even though the larger stack size has avoided the problem. The risk is low, simply placing two variables on the heap instead of the stack. Additionally, Tier1 and the newly added test passes on Windows after the changes.

[GoeLin]

@aamarsh, the comment needs to go to the JBS issue. Just have a look at the other changes recently pushed to 17u-dev.

[aamarsh]

/integrate

[openjdk]

@aamarsh
Your change (at version 263b291) is now ready to be sponsored by a Committer.

[aamarsh]

@GoeLin Thanks for helping me through the backporting process! I see that the jdk17u-fix-yes tag is now on the JBS issue, so I have integrated my changes. Is there any way you could sponsor this change? Thanks again!

[GoeLin]

/sponsor

[openjdk]

Going to push as commit 2965051.
Since your change was applied there have been 120 commits pushed to the master branch:

• a5d00fb: 8281061: [s390] JFR runs into assertions while validating interpreter frames
• f391654: 8275330: C2: assert(n->is_Root() || n->is_Region() || n->is_Phi() || n->is_MachMerge() || def_block->dominates(block)) failed: uses must be dominated by definitions
• 5159ffd: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently
• 0965996: 8274750: java/io/File/GetXSpace.java failed: '/dev': 191488 != 190976
• bda9cb4: 8273387: remove some unreferenced gtk-related functions
• c36c1c2: 8279702: [macosx] ignore xcodebuild warnings on M1
• dba064a: 8277180: Intrinsify recursive ObjectMonitor locking for C2 x64 and A64
• cd73095: 8277449: compiler/vectorapi/TestLongVectorNeg.java fails with release VMs
• f723588: 8275643: C2's unaryOp vector intrinsic does not properly handle LongVector.neg
• 27cbe24: 8271056: C2: "assert(no_dead_loop) failed: dead loop detected" due to cmoving identity
• ... and 110 more: https://git.openjdk.java.net/jdk17u-dev/compare/e92b2bc0b48a2c04c612cfd5a2913d73ad16b3f4...master

Your commit was automatically rebased without conflicts.

[openjdk]

@GoeLin @aamarsh Pushed as commit 2965051.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.