RFR: 8305990: Stripping debug info of ASM 9.5 fails [v10]

[Chen Liang]

On Tue, 2 May 2023 14:15:27 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> Classfile API didn't handle transformations of class files version 50 and below correctly. 
>> 
>> Proposed fix have two parts: 
>> 1. Inflation of branch targets does not depend on StackMapTable attribute presence for class file version 50 and below. Alternative fallback implementation is provided. 
>> 2. StackMapTable attribute is not generated for class file versions below 50.
>> 
>> StackMapsTest is also extended to test this patch.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona 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 12 additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8305990-debug-info-strip-fail
>  - Update StackCounter.java
>  - added comments to StackCounter about maxStack upper bounds calculation for JSR/RET instructions
>  - fixed stack counting of JSR instructions
>  - implemented StackCounter
>  - Making some BufWriter fields final
>  - Update src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java
>    
>    Co-authored-by: liach <7806504+liach at users.noreply.github.com>
>  - Update src/java.base/share/classes/jdk/internal/classfile/Opcode.java
>    
>    Co-authored-by: ExE Boss <3889017+ExE-Boss at users.noreply.github.com>
>  - fixed ClassPrinterImpl
>  - DiscontinuedInstruction implementation + test
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/995f6608...f132f737

Also, has brian reviewed the DiscontinuedInstruction API changes?

src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java line 109:

> 107:         maxLocals = isStatic ? 0 : 1;
> 108:         for (var cd : methodDesc.parameterList()) {
> 109:             maxLocals += TypeKind.fromDescriptor(cd.descriptorString()).slotSize();

Suggestion:

            maxLocals += TypeKind.from(cd).slotSize();

src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java line 310:

> 308:                         var mDesc = MethodTypeDesc.ofDescriptor(nameAndType.type().stringValue());
> 309:                         for (var arg : mDesc.parameterList()) {
> 310:                             stack(-TypeKind.fromDescriptor(arg.descriptorString()).slotSize());

Suggestion:

                            stack(-TypeKind.from(arg).slotSize());

src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java line 315:

> 313:                             stack(-1);
> 314:                         }
> 315:                         stack(TypeKind.fromDescriptor(mDesc.returnType().descriptorString()).slotSize());

Suggestion:

                        stack(TypeKind.from(mDesc.returnType()).slotSize());

src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java line 378:

> 376:                 bcs.bci,
> 377:                 methodName,
> 378:                 methodDesc.parameterList().stream().map(ClassDesc::displayName).collect(Collectors.joining(","))));

Suggestion:

                methodDesc.displayDescriptor());

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

PR Review: https://git.openjdk.org/jdk/pull/13478#pullrequestreview-1413358082
PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1185172984
PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1185174342
PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1185174639
PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1185176343