RFR: Classfile API stack maps processing

[Brian Goetz]

> I thought PROCESS_STACK_MAP switch is clear the maps will pass to the 
> CodeBuilder.
>
> It seems to me similar to PROCESS_DEBUG, PROCESS_LINE_NUMBERS and 
> PROCESS_UNKNOWN_ATTRIBUTES.
>
> In all the above cases the relevant attributes appear or disappear 
> from processing.
>

It is structurally similar, but (a) slightly different and (b) the 
danger level is much higher.

For PROCESS_{DEBUG,LINE_NUMBERS}, we explode the appropriate attribute 
into finer-grained elements, and reconstruct them into an attribute at 
the end.  (This reminds me that we need to make sure that we're not 
doing *both* reconstructed and pass-through processing in these cases.)  
So this is opting into a more active mode of control over these 
elements.  And secondarily, messing up the line number tables or LVT 
will not result in unloadable classfiles.

It seem just too easy for a user to do the following:

  - request PROCESS_STACK_MAP because there is something useful they 
want to see in it, and just passing it through to the builder because 
that's a reasonable default for an element you don't plan to manipulate;
  - adapt the instruction stream (say, to add logging), but without 
rewriting the stack map.

Now, we will write a classfile with a wrong stack map.  This seems too 
easy and too dangerous.  Hence, asking users to _turn off_ generation as 
well as turning on "give me stack maps" seems a reasonable precaution.

> I still “slightly” think that if user drops something specific to 
> CodeBuilder – it should override even the Generator.
>
> However I’m ready to put Generator priority over the content dropped 
> to CodeBuilder.
>

We faced a similar situation with BootstrapMethods attribute. THere, we 
concluded there is no case we want to let the user generate a BMA; if 
they want to generate BMA _entries_, they can do so with 
ConstantPoolBuilder.  There are various places where we filter out BMA 
in various places, such as attributes written to the ClassBuilder.  
Stack maps feel about 90% along on the spectrum of [ line numbers ... 
bootstrap methods ] to me.

> What about another option to keep the status quo, drop 
> PROCESS_STACK_MAPswitch and do not stream StackMapAttribute at all?
>
> As users can still access it from 
> CodeModel::findAttribute(Attributes.STACK_MAP_TABLE) and manually pass 
> it to CodeBuilder, which will then override even Generator.
>

This is more in line with how we handle BootstrapMethod attribute, and I 
think it is fine.  I think it is better, actually, because there is  no 
confusion about the interaction between "process" and "generate".  Here, 
"generate" indicates clear control over what to do with user-provided 
stack maps -- if we are in generate mode, ignore the user one.  No 
confusion.  So, +1 to this idea.

> *From: *Brian Goetz <brian.goetz at oracle.com>
> *Date: *Wednesday, 17 August 2022 18:58
> *To: *Adam Sotona <adam.sotona at oracle.com>, 
> classfile-api-dev at openjdk.org <classfile-api-dev at openjdk.org>
> *Subject: *Re: RFR: Classfile API stack maps processing
>
>
>
>     1.Processing stack maps is extremely advanced feature and 99.9% of
>     users don’t want to touch it, so why PROCESS_STACK_MAPis off by
>     default and GENERATE_STACK_MAPS is on by default
>
>
> Agree with this.
>
>
>     1.When the 0.1% user needs to process the stack maps, he needs
>     full control over it. So when user enables PROCESS_STACK_MAP, full
>     control of what drops to code builder is up-to user.
>
>
> OK, I don't mind two options, but I'd like to be clear on what they 
> mean.  I don't think that merely turning on PROCESS_STACK_MAP will 
> clue the user in to the fact that they have disengaged all the safety 
> guards.  Some users may just want to _read_ the stack map.  So I'm OK 
> with having both PROCESS and GENERATE options, but we should be clear 
> what they mean, and be conservative in their meanings.  Here's what 
> makes sense to me:
>
> GENERATE: the stack map is *always* generated by the library. Any 
> stack maps sent downstream are still ignored.
> PROCESS: send the stack map to the user.
> PROCESS & !GENERATE: send the stack map to the user, write any stack 
> map the user supplied to the classfile.
>
> So users who want full control select PROCESS and !GENERATE; users who 
> want to see the stackmap select PROCESS only; users who don't care 
> about stack maps (which is most of them) select neither.
>
> I think what this exposes is we need a way to override some options on 
> a per-method basis, which is a separate problem.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20220817/a5e2665e/attachment.htm>