jdk icon indicating copy to clipboard operation
jdk copied to clipboard

Published 20 hours ago • verified publisher icon openjdk

8342303: Segfault in update_inherited_vtable: AppCDS, old bytecode, and redefineClasses

Open calvinccheung opened this issue 1 year ago • 3 comments

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-8342303: Segfault in update_inherited_vtable: AppCDS, old bytecode, and redefineClasses (Bug - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21667

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

Using diff file

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

calvinccheung avatar Oct 23 '24 17:10 calvinccheung

:wave: Welcome back ccheung! 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 Oct 23 '24 17:10 bridgekeeper[bot]

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

8342303: Segfault in update_inherited_vtable: AppCDS, old bytecode, and redefineClasses

Reviewed-by: dholmes, coleenp, iklam

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

  • 0e1c1b793d43064aabe9571057284899c9580f30: 8343452: Incorrect WINDOWS build variable is used in macroAssembler_x86.cpp
  • 619b4d596634ee84b9bf5884b97a69eb01661657: 8334107: Specification for MemorySegment::get/setString could use some clarification
  • a6c85daa1c5e685ab64cbf9860a022aaa4a0d7f8: 8342945: Replace predicate walking code in get_assertion_predicates() used for Loop Unswitching and cleaning useless Template Assertion Predicates with a predicate visitor
  • 97b681e93a9469d8d16122dc10bbf2f5b5fe2266: 8340586: JdkJfrEvent::get_all_klasses stores non-strong oops in JNI handles
  • f2316f6829c9b671e992401ee39d7a1a1805857e: 8343505: Problemlist java/lang/Thread/jni/AttachCurrentThread/AttachTest.java
  • 9e31e78e39a4b573c158ef31af3ab4e9a1e229de: 8342647: [macosx] Clean up the NSInvocation based call to NSProcessInfo.operatingSystemVersion

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch. 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[bot] avatar Oct 23 '24 17:10 openjdk[bot]

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

  • hotspot

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.

openjdk[bot] avatar Oct 23 '24 17:10 openjdk[bot]

/label remove hotspot

calvinccheung avatar Oct 28 '24 21:10 calvinccheung

/label add hotspot-runtime

calvinccheung avatar Oct 28 '24 21:10 calvinccheung

@calvinccheung The hotspot label was successfully removed.

openjdk[bot] avatar Oct 28 '24 21:10 openjdk[bot]

@calvinccheung The hotspot-runtime label was successfully added.

openjdk[bot] avatar Oct 28 '24 21:10 openjdk[bot]

Webrevs

mlbridge[bot] avatar Oct 28 '24 21:10 mlbridge[bot]

I've added two tests:

  • redefine a super class loaded by the boot class loader;
  • redefine an old super class loaded by the AppClassLoader.

Also simplified the OldClassAndRedefineClass.java test.

calvinccheung avatar Oct 29 '24 21:10 calvinccheung

There is a RedefineHelper class that hides all this instrumentation and agent code, and makes redefinition tests really simple. I can't tell if you can use that from all the code in these tests. Examples are in the serviceability/jvmti/RedefineClasses tests.

In fact, some CDS tests are also using RedefineClassHelper. It did simply the test code. I've updated the tests.

calvinccheung avatar Nov 01 '24 17:11 calvinccheung

@iklam, @coleenp, @dholmes-ora, I've noticed some failures from tier3 testing due to the jdk/test/lib/compiler/InMemoryJavaCompiler class not found. So I added the class to the "@build" command of the affected tests. Please re-review. Probably one re-review is enough. Thanks!

calvinccheung avatar Nov 07 '24 05:11 calvinccheung

Thanks for the reviews!

/integrate

calvinccheung avatar Nov 07 '24 18:11 calvinccheung

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

  • 4f5a241ec11981bcb5f8ab0936b0e821ec5a03ce: 8343491: javax/management/remote/mandatory/connection/DeadLockTest.java failing with NoSuchObjectException: no such object in table
  • 56c588b4e88d779cd5c5f67e7bfb4e1641eb9c25: 8343417: (fs) BasicFileAttributeView.setTimes uses microsecond precision with NOFOLLOW_LINKS
  • d3c042f9a0cd91e7cdf7f45cb0ea387f7ba9785b: 8343770: Build fails due to use of sun.misc.Unsafe in LoopOverRandom
  • 1d117f65f06456ae571aecc146542c2f79d402cf: 8343394: Make MemorySessionImpl.state a stable field
  • d2b681d4557109158fbbce9db995f4146c344c97: 8343730: JMX cleanups
  • ac82a8f89c7066fb1d379b12bcfd68053cb39ba4: 8343610: InOutPathTest jpackage test produces invalid app image on macOS
  • f0b251d76078e8d5b47e967b0449c4cbdcb5a005: 8343531: Improve print_location for invalid heap pointers
  • 4244682309e7ae1be892280dfd6a6f70ccecc760: 8339190: Parameter arrays that are capped during annotation processing report incorrect length
  • 7620b129888d57514d9ef588e0681f1d43377236: 8323803: ConstantOopReadValue::print_on should print 'null' instead of 'nullptr'
  • 592a48b163ed582872b686e7a606cf8b96fcbcbc: 8321997: Increase upper limit of LoopOptsCount flag
  • ... and 7 more: https://git.openjdk.org/jdk/compare/e33dc13567a4f0d9a6c1ae63fa0424ca27d52584...master

Your commit was automatically rebased without conflicts.

openjdk[bot] avatar Nov 07 '24 18:11 openjdk[bot]

@calvinccheung Pushed as commit ccda8159f65be39d6fab9be87b1a79465aae5452.

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

openjdk[bot] avatar Nov 07 '24 18:11 openjdk[bot]