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

[Maurizio Cimadamore]

On Tue, 9 May 2023 11:11:36 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 16 additional commits since the last revision:
> 
>  - fixed StackCounter
>  - Merge branch 'master' into JDK-8305990-debug-info-strip-fail
>  - implemented failover stackmap generation for class version 50
>    fixed StackMapGenerator error debug print + added clone constructor to SplitConstantPool
>    adjusted and extended related tests
>  - Apply suggestions from code review
>    
>    Thanks for review.
>    
>    Co-authored-by: liach <7806504+liach at users.noreply.github.com>
>  - 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
>  - ... and 6 more: https://git.openjdk.org/jdk/compare/929ac16e...5db0ed01

Seems like GitHub UI does not let me add comments (as GitHub seems to be experiencing some issues). Here's what I added:

* StackCounter: `stack` and `local` are different in spirit. One adds new stack slots. The other ensure there's at least enough local slots available. As such I'd suggest a renaming `addStackSlot` and `ensureLocalSlot`.

* StackMapGeneration/DirectClassBuilder: the exception type seems a bit loose and we might end up catching more than is thrown by the stackmap generator.

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

PR Comment: https://git.openjdk.org/jdk/pull/13478#issuecomment-1540013928