On Wed, 19 Apr 2023 13:44:47 GMT, Adam Sotona <asot...@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 incrementally with one additional 
> commit since the last revision:
> 
>   fixed ClassPrinterImpl

src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java
 line 724:

> 722:         public int branchByteOffset() {
> 723:             return size == 3
> 724:                    ? (int) (short) code.classReader.readU2(pos + 1)

Suggestion:

                   ? code.classReader.readS2(pos + 1)

the assembly of this offset is the same as the assembly of the increment amount 
in wide iinc.

src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java line 
86:

> 84:     }
> 85: 
> 86:     public void setMajorVersion(int majorVersion) {

We should ensure the version is not changed once writing has already happened, 
and the constant pool builder should have access to the major version as well 
to prevent writing of invalid entries (like condy before java 11)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1171622180
PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1171630285

Reply via email to