Classfile API support for JSR and RET instructions

[forax at univ-mlv.fr]

----- Original Message -----
> From: "-" <liangchenblue at gmail.com>
> To: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "Brian Goetz" <brian.goetz at oracle.com>, "Adam Sotona" <adam.sotona at oracle.com>, "classfile-api-dev"
> <classfile-api-dev at openjdk.org>
> Sent: Thursday, April 20, 2023 5:53:56 AM
> Subject: Re: Classfile API support for JSR and RET instructions

> Sorry, but I fail to see scenarios where the classfile itself has to
> be kept at an older version with a stackmap; why can't a newer
> classfile be generated at once?

There are several reasons to use an agent instead of transforming the bytecode ahead of time. Usually it's because the agent needs runtime information, not available when the classfile is generated. Sometimes, it's also because of the way companies are organized (the ops team and the dev team only talking to each other through 200-page word documents).

> 
> Chen

regards,
Rémi

> 
> On Wed, Apr 19, 2023 at 10:44 AM Remi Forax <forax at univ-mlv.fr> wrote:
>>
>>
>>
>> ________________________________
>>
>> From: "Brian Goetz" <brian.goetz at oracle.com>
>> To: "Adam Sotona" <adam.sotona at oracle.com>, "classfile-api-dev"
>> <classfile-api-dev at openjdk.org>
>> Sent: Wednesday, April 19, 2023 5:21:52 PM
>> Subject: Re: Classfile API support for JSR and RET instructions
>>
>> 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.
>>
>>
>> Why ? StackMapTable is an attribute, it can be present in class file version <
>> 50 and it will be ignored by the VM.
>> Generating stack map is quite costly so generating them beforehand even if the
>> class file version is < 50 is a trick to make agents that upgrade the class
>> file version at runtime faster (because for most transformations, patching the
>> stack maps is linear).
>>
>> Rémi
>>
>>
>>
>>
>> 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)
>> 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
>>
>>