8316653: Large NMethodSizeLimit triggers assert during C1 code buffer allocation

[dlunde]

This changeset fixes an overflow issue for the develop flag -XX:NMethodSizeLimit.

In summary, java -XX:NMethodSizeLimit=2147483647 triggers an assert in CodeCache::allocate:

#  assert(size > 0) failed: Code cache allocation request must be > 0 but is -1932735136

Why?

1. The function code_buffer_size computes NMethodSizeLimit + NMethodSizeLimit/10 as a signed integer (may overflow).
2. The BufferBlob::create function takes code_buffer_size() as argument and uses it (among other things, such as sizeof(BufferBlob)) to compute an unsigned size.
3. BufferBlob::operator new treats the unsigned size as signed (may overflow).

Changes:

• Change all data types in the above chain to uint and leave the flag range for NMethodSizeLimit untouched (range(0, max_jint)). This ensures no overflow.
• Add a new regression test.

Testing

Platforms: windows-x64, linux-x64, linux-aarch64, macosx-x64, macosx-aarch64.

• tier1
• HotSpot parts of tier2 and tier3

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8316653: Large NMethodSizeLimit triggers assert during C1 code buffer allocation (Bug - P4)

Reviewers

• Vladimir Kozlov (@vnkozlov - Reviewer) ⚠️ Review applies to e673ac1c
• Roberto Castañeda Lozano (@robcasloz - Reviewer)
• Tobias Hartmann (@TobiHartmann - Reviewer) ⚠️ Review applies to e673ac1c

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/16656/head:pull/16656
$ git checkout pull/16656

Update a local copy of the PR:
$ git checkout pull/16656
$ git pull https://git.openjdk.org/jdk.git pull/16656/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16656

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/16656.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

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

@dlunde The following label will be automatically applied to this pull request:

• hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (f52ef32f)
• 00: Full (e673ac1c)

[vnkozlov]

Looks good.

[robcasloz]

Looks good!

[openjdk]

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

8316653: Large NMethodSizeLimit triggers assert during C1 code buffer allocation

Reviewed-by: kvn, rcastanedalo, thartmann

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

• bad6999: 8313672: C2: PhaseCCP does not correctly track analysis dependencies involving shift, convert, and mask
• fbe1937: 8319955: Improve dependencies removal during class unloading
• 4c1540b: 8287284: C2: loop optimization performs split_thru_phi infinitely many times
• 70f0c01: 8320054: Remove unused _count from NMT walker classes
• e7486e8: 8315986: [macos14] javax/swing/JMenuItem/4654927/bug4654927.java: component must be showing on the screen to determine its location
• a6343c0: 8319999: Refactor MetaspaceShared::use_full_module_graph()
• d9a89c5: 8319572: Test jdk/incubator/vector/LoadJsvmlTest.java ignores VM flags
• 1e76ba0: 8319439: Move BufferNode from PtrQueue files to new files
• d5abe49: 8319628: DateFormat does not mention IllegalArgumentException for invalid style args
• d725b73: 8301310: The SendRawSysexMessage test may cause a JVM crash
• ... and 17 more: https://git.openjdk.org/jdk/compare/7df73a23d3b459b3358c460466ebfe0fd942c99d...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. Possible candidates are the reviewers of this PR (@vnkozlov, @robcasloz, @TobiHartmann) but any other Committer may sponsor as well.

➡️ 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).

[TobiHartmann]

Looks good to me.

[dlunde]

/integrate

[openjdk]

@dlunde
Your change (at version f52ef32) is now ready to be sponsored by a Committer.

[robcasloz]

/sponsor

[openjdk]

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

• 2db9ea9: 8317723: C2: CountedLoopEndNodes and Zero Trip Guards are wrongly treated as Runtime Predicate
• eaa4417: 8319301: Static analysis warnings after JDK-8318016
• 3452210: 8283140: Remove unused encoding classes/operands from x86_64.ad
• f0b7eb5: 8304020: Speed up test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java and clarify its purpose
• ffa35d8: 8301997: Move method resolution information out of the cpCache
• 891d8cf: 8319986: Invalid/inconsistent description and example for DateFormat
• 536b1ce: 8320130: Problemlist 2 vmTestbase/nsk/jdi/StepRequest/addClassFilter_rt tests with Xcomp
• 0bda467: 8319187: Add three eMudhra emSign roots
• 3c6ffca: 8318219: RISC-V: C2 ExpandBits
• 77a6966: 8320053: GHA: Cross-compile gtest code
• ... and 30 more: https://git.openjdk.org/jdk/compare/7df73a23d3b459b3358c460466ebfe0fd942c99d...master

Your commit was automatically rebased without conflicts.

[openjdk]

@robcasloz @dlunde Pushed as commit c36ec2c.

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