8282241: Invalid generic signature for redefined classes

[alexmenkov]

JDK-8238048 (fixed in jdk15) moved major_version, minor_version, generic_signature_index and source_file_name_index from InstanceKlass to ConstantPool.
We still have some incorrect code in CP merge during class redefinition.

rewrite_cp_refs(scratch_class) updates generic_signature_index and source_file_name_index in the scratch_cp, so we need to copy the attributes (merge_cp->copy_fields(scratch_cp())) after rewrite_cp_refs.

In redefine_single_class we don't need to copy source_file_name_index because it's a CP property and we swap CPs. So this copying actually sets the value from old class.

tested:

• test/jdk/java/lang/instrument
• test/hotspot/jtreg/serviceability/jvmti/RedefineClasses
• test/hotspot/jtreg/vmTestbase/nsk/jvmti/RedefineClasses
• test/hotspot/jtreg/vmTestbase/nsk/jvmti/RetransformClasses

---

Progress

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

Issue

• JDK-8282241: Invalid generic signature for redefined classes

Reviewers

• Coleen Phillimore (@coleenp - Reviewer)
• Serguei Spitsyn (@sspitsyn - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7676/head:pull/7676
$ git checkout pull/7676

Update a local copy of the PR:
$ git checkout pull/7676
$ git pull https://git.openjdk.java.net/jdk pull/7676/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7676

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7676.diff

[bridgekeeper]

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

@alexmenkov The following labels will be automatically applied to this pull request:

• core-libs
• hotspot
• serviceability

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

[mlbridge]

Webrevs

• 01: Full - Incremental (c51b82d7)
• 00: Full (8c6d55c5)

[AlanBateman]

/label remove core-libs

[openjdk]

@AlanBateman
The core-libs label was successfully removed.

[coleenp]

Thank you for fixing this. It took me a while to figure this out again and it looks correct. Can you use the new redefine class test framework for the test instead?

[coleenp]

Can you write this test in the framework where the newer RedefineClasses test are in test/hotspot/jtreg/serviceability/jvmti/RedefineClasses ? You can just write the new class as a string that the inMemory compiler compiles for you. It's a lot simpler and doesn't use a shell script at all.

[alexmenkov]

Reworked the test:

• used in-memory compirer and asm to prepare new version of the class;
• used redefineClass instead of retransformClasses (ClassFileTransformer is not needed anymore);
• used RedefineClassHelper agent instead of the agent from ATransformerManagementTestCase framework;
• moved the test to test/hotspot/jtreg/serviceability/jvmti/RedefineClasses as actually it tests JVMTI functionality.

[coleenp]

Thanks, the test is much more understandable now.

[openjdk]

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

8282241: Invalid generic signature for redefined classes

Reviewed-by: coleenp, sspitsyn

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

• 3e39304: 8281631: HashMap copy constructor and putAll can over-allocate table
• 0cf291b: 8283234: Improve @jls usage in java.base
• 9b8afce: 8283260: gcc is not supported on mac
• 08cadb4: 8271195: Use largest available large page size smaller than LargePageSizeInBytes when available
• de4f04c: 8253495: CDS generates non-deterministic output
• 4df24c5: 8283230: Improve @jls usage in ElementType
• 27fe3d7: 8240756: [macos] SwingSet2:TableDemo:Printed Japanese characters were garbled
• bacfaa3: 8282414: x86: Enhance the assembler to generate more compact instructions
• 1465ea9: 8282355: compiler/arguments/TestCodeEntryAlignment.java failed "guarantee(sect->end() <= tend) failed: sanity"
• ac06bdb: 8282507: Add a separate license file for hsdis
• ... and 400 more: https://git.openjdk.java.net/jdk/compare/46c6c6f308b5ec0ec3b762df4b76de555287474c...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.

[alexmenkov]

Ping. Need 2nd reviewer.

[sspitsyn]

Alex,
The fix looks good. Thank you for taking care about it!
Thanks,
Serguei

[alexmenkov]

/integrate

[openjdk]

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

• 3e73a0b: 8283237: CallSite should be a sealed class
• a771600: 8283279: [Testbug] Improve TestGetSwapSpaceSize
• 2b291d8: 8282536: java.net.InetAddress should be a sealed class
• dc45b0a: 8283513: Parallel: Skip the card marking in PSRootsClosure
• 78ef2fd: 8283562: JDK-8282306 breaks gtests on zero
• 61d7d86: 8283254: Remove redundant class jdk/internal/agent/spi/AgentProvider
• 6ed0ba2: 8283543: indentation error at com.sun.tools.javac.comp.Enter::visitTopLevel
• 91fab6a: 8283411: InflaterInputStream holds on to a temporary byte array of 512 bytes
• d8c5572: 8282204: Use lea instructions for arithmetic operations on x86_64
• 026b853: 8283298: Make CodeCacheSegmentSize a product flag
• ... and 474 more: https://git.openjdk.java.net/jdk/compare/46c6c6f308b5ec0ec3b762df4b76de555287474c...master

Your commit was automatically rebased without conflicts.

[openjdk]

@alexmenkov Pushed as commit f017739.

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