RFR: 8356548: Avoid using ASM to parse latest class files in tests [v6]

[Ioi Lam]

On Fri, 22 Aug 2025 15:06:46 GMT, Chen Liang <liach at openjdk.org> wrote:

>> For early eval; test by changing the ClassReader max accepted version of test ASM to 24 instead of 25
>
> Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
> 
>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/asm-test-upgrade
>  - Merge branch 'fix/asm-test-upgrade' of github.com:liachmodded/jdk into fix/asm-test-upgrade
>  - Update test/hotspot/jtreg/compiler/calls/common/InvokeDynamicPatcher.java
>    
>    Co-authored-by: Andrew Haley <aph-open at littlepinkcloud.com>
>  - Variable name improvements
>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/asm-test-upgrade
>  - Move other tier 4,5, etc tests to not use ClassReader
>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/asm-test-upgrade
>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/asm-test-upgrade
>  - Use classfile api instead of javac setting version
>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/asm-test-upgrade
>  - ... and 1 more: https://git.openjdk.org/jdk/compare/e0d5ae0b...a659f538

I looked at the diff with whitespace off and looked at all change. They seem reasonable to me. The bytecode changes seems to be 1-1 matching with the old code. I just have a few style nits.

Since the changes are subtle, I think before integration, you should do a final run up to tier 6 just to be safe.

test/hotspot/jtreg/compiler/jvmci/common/CTVMUtilities.java line 80:

> 78:             throw new Error("TEST BUG: cannot read " + binaryName, e);
> 79:         }
> 80:         ClassModel classModel = ClassFile.of().parse(fileBytes);

Can you put these in a separate function, like `ClassModel getClassModel(Class<?> class)`?

test/hotspot/jtreg/compiler/jvmci/common/CTVMUtilities.java line 93:

> 91:         for (var methodModel : classModel.methods()) {
> 92:             if (methodModel.methodName().equalsString(methodName)
> 93:                     && methodModel.methodType().isMethodType(methodType)) {

For readability, I think we should put the above in a separate function, and then:


var methodModel = findMethodInClass(classModel, m);


Then we can potentially use findMethodInClass() in other test cases when we have a need to do so.

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/MissedStackMapFrames/MissedStackMapFrames.java line 64:

> 62:             var foundStackMapTable = method.code().flatMap(code -> code.findAttribute(Attributes.stackMapTable()));
> 63:             if (foundStackMapTable.isPresent()) {
> 64:                 count += foundStackMapTable.get().entries().size();

The old logging should be preserved:

log(" method " + name + " - " + methodFrames + " frames");

test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineAnnotations.java line 104:

> 102:                 public void atEnd(ClassBuilder builder) {
> 103:                     // Re-add dummy fields
> 104:                     dummyFields.forEach(builder);

Comment: `// Re-add dummy fields at the end of the class`

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

Marked as reviewed by iklam (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25124#pullrequestreview-3182050159
PR Review Comment: https://git.openjdk.org/jdk/pull/25124#discussion_r2319959194
PR Review Comment: https://git.openjdk.org/jdk/pull/25124#discussion_r2319956360
PR Review Comment: https://git.openjdk.org/jdk/pull/25124#discussion_r2319921409
PR Review Comment: https://git.openjdk.org/jdk/pull/25124#discussion_r2319969688