Classfile API support for JSR and RET instructions

[Brian Goetz]

Overall this is a good plan. There's a small gap regarding control of 
stack map generation.

The stack map attribute was introduced in classfile version 50; JSR and 
RET were eliminated in classfile version 51.  So there's a small window 
where stackmaps and JSR/RET can co-exist.

Our strategy with stackmaps is to generate based on an option, whose 
default is to generate.  You are right that we should refine this so 
that we never generate stack maps for classfiles < 50 because the 
stackmap attribute wasn't defined until then.  But that leaves us with a 
quandary about what to do for 51; in your proposed changes below, there 
is no way a user could get a stack map table even if they wanted one and 
didn't use JSR/RET.  Instead, I think we should:

  - Generate a stackmap if the option is set and classfile version >= 50
  - If JSR/RET is generated in classfile > 51, throw from CodeBuilder
  - If stackmap generation encounters JSR/RET in classfile 50, throw 
from StackMapGenerator



On 4/19/2023 9:29 AM, Adam Sotona wrote:
>
> Hi,
>
> Work on javap conversion from ASM to Classfile API and recent bug 
> (JDK-8305990 <https://bugs.openjdk.org/browse/JDK-8305990>) discovered 
> in StripJavaDebugAttributesPlugin brought attention to missing JSR and 
> RET instructions support in Classfile API.
>
> I’m proposing following changes to Classfile API:
>
>   * Drop Opcode.Kind.UNSUPPORTED and add Opcode.Kind.DISCONTINUED_JSR
>     and DISCONTINUED_RET
>   * Add DiscontinuedInstruction interface with inner JsrInstruction
>     and RetInstruction with standard factory methods
>   * CodeImpl will parse and stream these instructions when present in
>     the Code attribute
>   * Do not add any new conveniency methods to CodeBuilder, the only
>     way to build a code with these instructions will be
>     cob.with(DiscontinuedInstruction. JsrInstruction.of(…)) and
>     similar for RetInstruction
>   * Fix DirectCodeBuilder so it invokes StackMapGenerator only for
>     class file version >= 51.0 (BufWriterImpl must hold class version
>     for that purpose)
>   * Fix  StackMapGenerator error message when these instruction appear
>     in during generation
>   * Implement fallback to jump target inflation in CodeImpl when the
>     StackMapTable attribute is missing and class file version is < 51.0
>   * Extend tests to verify JSR and RET instructions are correctly
>     generated, transformed and reported as error (based on class file
>     version).
>
> Please review and comment this proposal under following pull request:
>
> https://github.com/openjdk/jdk/pull/13478
>
> Next step is to implement simple maxStack and maxLocals counter into 
> DirectCodeBuilder for the cases where StackMapGenerator is not 
> involved. Actual default to maxStack = maxLocals = 255 is not correct.
>
> Thanks,
>
> Adam
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230419/3cd670b3/attachment-0001.htm>