8328085: C2: Use after free in PhaseChaitin::Register_Allocate()

[reinrich]

Hi all,

This pull request contains a backport of commit a21862ab from the openjdk/jdk21u-dev repository.

The commit being backported was authored by Richard Reingruber on 17 Dec 2024 and had no reviewers.

I'd consider the risk medium. There could be scenarios with higher memory usage in c2 register allocation.
I've done some testing with which did not reveal higher higher memory usage.
(the measuring code was part of the original pull request)

Max. ResourceArea size in KB after C2 PhaseChaitin::merge_multidefs

DaCapo Benchmark        Basline        Pull Request

avrora                  2273           2259
batik                   3456           3179
biojava                 3372           3541
cassandra               563            595
eclipse                 4044           4090
fop                     3986           3986
graphchi                3024           3024
h2                      3826           3712
h2o                     5750           5962
jme                     2209           2147
jython                  9734           9774
kafka                   3115           3493
luindex                 3380           3447
lusearch                3866           3381
pmd                     6497           5779
spring                  4771           4944
sunflow                 3088           3088
tomcat                  3375           3467
tradebeans              3348           3672
tradesoap               3480           3353
xalan                   3476           3106
zxing                   3741           4766

zxing results are volatile. I've made 3 additional runs:

Max. ResourceArea size in KB after C2 PhaseChaitin::merge_multidefs running DaCapo::zxing

Baseline:     3844 6146 5448
Pull Request: 6013 3615 4472

The fix passed our CI testing: JTReg tests: tier1-4 of hotspot and jdk. All of Langtools and jaxp. JCK, SPECjvm2008, SPECjbb2015, Renaissance Suite, and SAP specific tests.
Testing was done with fastdebug builds on the main platforms and also on Linux/PPC64le and AIX.

Thanks, Richard.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• JDK-8328085 needs maintainer approval

Issue

• JDK-8328085: C2: Use after free in PhaseChaitin::Register_Allocate() (Bug - P3 - Approved)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/3197/head:pull/3197
$ git checkout pull/3197

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3197

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/3197.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back rrich! 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]

@reinrich 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:

8328085: C2: Use after free in PhaseChaitin::Register_Allocate()

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 14 new commits pushed to the master branch:

• 59f98c1: 8226933: [TEST_BUG]GTK L&F: There is no swatches or RGB tab in JColorChooser
• 7c825dd: 8314225: SIGSEGV in JavaThread::is_lock_owned
• 06c8242: 8198237: [macos] Test java/awt/Frame/ExceptionOnSetExtendedStateTest/ExceptionOnSetExtendedStateTest.java fails
• b0286b3: 8327505: Test com/sun/jmx/remote/NotificationMarshalVersions/TestSerializationMismatch.java fails
• 6a1576d: 8317636: Improve heap walking API tests to verify correctness of field indexes
• 0228f7c: 8208565: [TEST_BUG] javax\swing\PopupFactory\6276087\NonOpaquePopupMenuTest.java throws NPE
• c93489b: 8314610: hotspot can't compile with the latest of gtest because of
• 36481dd: 8328368: Convert java/awt/image/multiresolution/MultiDisplayTest/MultiDisplayTest.java applet test to main
• 4dab8ba: 8311306: Test com/sun/management/ThreadMXBean/ThreadCpuTimeArray.java failed: out of expected range
• fe3e985: 8292704: sun/security/tools/jarsigner/compatibility/Compatibility.java use wrong key size for EC
• ... and 4 more: https://git.openjdk.org/jdk17u-dev/compare/96100ad2f4c71d924468c1feed37f323eebf91dc...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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

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

[openjdk]

⚠️ @reinrich This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

[mlbridge]

Webrevs

• 00: Full (b451ebf3)

[reinrich]

/integrate

[openjdk]

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

• 59f98c1: 8226933: [TEST_BUG]GTK L&F: There is no swatches or RGB tab in JColorChooser
• 7c825dd: 8314225: SIGSEGV in JavaThread::is_lock_owned
• 06c8242: 8198237: [macos] Test java/awt/Frame/ExceptionOnSetExtendedStateTest/ExceptionOnSetExtendedStateTest.java fails
• b0286b3: 8327505: Test com/sun/jmx/remote/NotificationMarshalVersions/TestSerializationMismatch.java fails
• 6a1576d: 8317636: Improve heap walking API tests to verify correctness of field indexes
• 0228f7c: 8208565: [TEST_BUG] javax\swing\PopupFactory\6276087\NonOpaquePopupMenuTest.java throws NPE
• c93489b: 8314610: hotspot can't compile with the latest of gtest because of
• 36481dd: 8328368: Convert java/awt/image/multiresolution/MultiDisplayTest/MultiDisplayTest.java applet test to main
• 4dab8ba: 8311306: Test com/sun/management/ThreadMXBean/ThreadCpuTimeArray.java failed: out of expected range
• fe3e985: 8292704: sun/security/tools/jarsigner/compatibility/Compatibility.java use wrong key size for EC
• ... and 4 more: https://git.openjdk.org/jdk17u-dev/compare/96100ad2f4c71d924468c1feed37f323eebf91dc...master

Your commit was automatically rebased without conflicts.

[openjdk]

@reinrich Pushed as commit 5491436.

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