On Thu, 14 Nov 2024 21:35:52 GMT, Coleen Phillimore <cole...@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
