RFR: 8330606: Redefinition doesn't but should verify the new klass [v2]

[David Holmes]

On Thu, 14 Nov 2024 21:35:52 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change adds a couple of comments, removes some ancient bootstrapping code, and adds an old test that we call the verifier for redefining a class, even one in the bootclass path.
>> Tested with tier1-4.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Updated the test with some comments.

Sorry but I don't see how these changes deal with "redefinition should but doesn't verify the new klass" - the only change here is to not skip Object/Class/String/Throwable. ??

src/hotspot/share/classfile/verifier.cpp line 280:

> 278: 
> 279:   return (should_verify_class &&
> 280:     // return if the class is a bootstrapping class

This method should have a comment explaining what the eligibility criteria is and how/why `should_verify_class` should be set in different conditions.

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineVerifyError.java line 68:

> 66:         classWriter.visit(52, ACC_SUPER | ACC_PUBLIC, "java/lang/VerifyError", null, "java/lang/LinkageError", null);
> 67:         {
> 68:             fieldVisitor = classWriter.visitField(ACC_PRIVATE | ACC_FINAL | ACC_STATIC, "serialVersionUID", "J", null, new Long(7001962396098498785L));

Why do we need a serialVersionUID in this test class?

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineVerifyError.java line 71:

> 69:             fieldVisitor.visitEnd();
> 70:         }
> 71:         {

Please add a comment showing the Java code this asm is implementing.

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineVerifyError.java line 90:

> 88:         }
> 89: 
> 90:         {   // broken method

Please add a comment showing the Java code this asm is implementing.

-------------

PR Review: https://git.openjdk.org/jdk/pull/22116#pullrequestreview-2437674162
PR Review Comment: https://git.openjdk.org/jdk/pull/22116#discussion_r1843201864
PR Review Comment: https://git.openjdk.org/jdk/pull/22116#discussion_r1843203365
PR Review Comment: https://git.openjdk.org/jdk/pull/22116#discussion_r1843203716
PR Review Comment: https://git.openjdk.org/jdk/pull/22116#discussion_r1843203836