jdk
jdk copied to clipboard
openjdk
8335977: Deoptimization fails with assert "object should be reallocated already"
Please, review this patch to fix an issue that may occur when serializing debug information related to reduce allocation merges. The problem happens when there are more than one JVMS in a uncommon_trap and a younger JVMS doesn't have the RAM inputs as a local/expression/monitor but an older JVMS does. In that situation the loop at line 1173 of output.cpp will set the is_root property of the ObjectValue to false when processing the younger JVMS even though it may have been set to true when visiting the older JVMS.
Tested on:
- Win, Mac & Linux tier1-4 on x64 & Aarch64.
- CTW with some thousands of jars.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8335977: Deoptimization fails with assert "object should be reallocated already" (Bug - P2)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21624/head:pull/21624
$ git checkout pull/21624
Update a local copy of the PR:
$ git checkout pull/21624
$ git pull https://git.openjdk.org/jdk.git pull/21624/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 21624
View PR using the GUI difftool:
$ git pr show -t 21624
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21624.diff
Webrev
:wave: Welcome back cslucas! 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.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@JohnTortugo 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:
8335977: Deoptimization fails with assert "object should be reallocated already"
Co-authored-by: Christian Hagedorn <[email protected]>
Reviewed-by: thartmann, kvn, vlivanov
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 242 new commits pushed to the master branch:
- b2694934b567c896b715c420584ce0989d2decc9: 8338981: Access to private classes should be permitted inside the permits clause of the enclosing top-level class
- 3ccd2f757d069c16147b331a90b1590c8ad4ae24: 8342458: More consistent constant instruction handling
- 29ae26517f9c1fd5e641b3a560aa93e370920506: 8341861: GHA: Use only retention mechanism to remove bundles
- e5bbad059d5dc293288ac415c9f7d0fa89c64b74: 8334999: RISC-V: implement AES single block encryption/decryption intrinsics
- c40bb7621c0e49581dac587b6900b6d281572813: 8343296: IGV: Show pre/main/post at CountedLoopNodes
- dee0982c603b389148a2e615c10c1276c3c589ae: 8343132: Remove temporary transitions from Virtual thread implementation
- 2f1ba5ef09441ed9e505fe2e17c57ccb1771f777: 8343307: Throw ZipException instead of IAE in ZipFile.Source::initCEN
- 388d44fbf0126f253860edc88c2efd57f86e5a2b: 8342156: C2: Compilation failure with fewer arguments after JDK-8329032
- 688e92e7f5febddd2935cb7f500dd3f10fbd9401: 8342642: Class loading failure due to archived map issue in ModuleLoaderMap.Mapper
- f340ab2d360b5dfd375095ac053e9f4fc22a98de: 8342181: Update tests to use stronger Key and Salt size
- ... and 232 more: https://git.openjdk.org/jdk/compare/bd6264420b9f248999dd8387c25c549b08bd193a...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 (@TobiHartmann, @iwanowww, @vnkozlov) 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).
@JohnTortugo 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.
Does your test also reproduce the NPE example that @chhagedorn triggered with his Test3.java? If not, I think that one should be added as well.
The test I added in this PR is based on @chhagedorn Test3.java. I was able to reproduce the issue on my end fairly easily.
Now that the -XX:+StressUnstableIfTraps option was added with JDK-8335334, could you please also add Test.java that I used originally to reproduce the issue?
I wasn't able to reproduce the issue using Test.java, how often does it reproduce for you with the flags that you listed at the top of Test.java ?
Thanks for testing @TobiHartmann !
The test I added in this PR is based on @chhagedorn Test3.java. I was able to reproduce the issue on my end fairly easily.
Ah right, all good then. Please add @chhagedorn as co-contributor since he extracted that test.
I wasn't able to reproduce the issue using Test.java, how often does it reproduce for you with the flags that you listed at the top of Test.java ?
Right, I just tried and it does not seem to reproduce anymore. Too bad but let's leave it out then.
@JohnTortugo This pull request has not yet been marked as ready for integration.
@JohnTortugo Your change (at version 818b09e6d2c74a0b08981a5bb92a36792f8ab743) is now ready to be sponsored by a Committer.
Going to push as commit 7d8bd21eb0187647ec574abf4fac4f99c435c60b.
Since your change was applied there have been 242 commits pushed to the master branch:
- b2694934b567c896b715c420584ce0989d2decc9: 8338981: Access to private classes should be permitted inside the permits clause of the enclosing top-level class
- 3ccd2f757d069c16147b331a90b1590c8ad4ae24: 8342458: More consistent constant instruction handling
- 29ae26517f9c1fd5e641b3a560aa93e370920506: 8341861: GHA: Use only retention mechanism to remove bundles
- e5bbad059d5dc293288ac415c9f7d0fa89c64b74: 8334999: RISC-V: implement AES single block encryption/decryption intrinsics
- c40bb7621c0e49581dac587b6900b6d281572813: 8343296: IGV: Show pre/main/post at CountedLoopNodes
- dee0982c603b389148a2e615c10c1276c3c589ae: 8343132: Remove temporary transitions from Virtual thread implementation
- 2f1ba5ef09441ed9e505fe2e17c57ccb1771f777: 8343307: Throw ZipException instead of IAE in ZipFile.Source::initCEN
- 388d44fbf0126f253860edc88c2efd57f86e5a2b: 8342156: C2: Compilation failure with fewer arguments after JDK-8329032
- 688e92e7f5febddd2935cb7f500dd3f10fbd9401: 8342642: Class loading failure due to archived map issue in ModuleLoaderMap.Mapper
- f340ab2d360b5dfd375095ac053e9f4fc22a98de: 8342181: Update tests to use stronger Key and Salt size
- ... and 232 more: https://git.openjdk.org/jdk/compare/bd6264420b9f248999dd8387c25c549b08bd193a...master
Your commit was automatically rebased without conflicts.
@vnkozlov @JohnTortugo Pushed as commit 7d8bd21eb0187647ec574abf4fac4f99c435c60b.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.