<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;
        mso-ligatures:standardcontextual;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:#0563C1;
        text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
        {mso-style-priority:34;
        margin-top:0cm;
        margin-right:0cm;
        margin-bottom:0cm;
        margin-left:36.0pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;
        mso-ligatures:standardcontextual;}
span.EmailStyle20
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
        {page:WordSection1;}
/* List Definitions */
@list l0
        {mso-list-id:484706898;
        mso-list-template-ids:750950862;}
@list l0:level1
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7 ;
        mso-level-tab-stop:36.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level2
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7 ;
        mso-level-tab-stop:72.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level3
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7 ;
        mso-level-tab-stop:108.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level4
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7 ;
        mso-level-tab-stop:144.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level5
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7 ;
        mso-level-tab-stop:180.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level6
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7 ;
        mso-level-tab-stop:216.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level7
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7 ;
        mso-level-tab-stop:252.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level8
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7 ;
        mso-level-tab-stop:288.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level9
        {mso-level-number-format:bullet;
        mso-level-text:\F0B7 ;
        mso-level-tab-stop:324.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l1
        {mso-list-id:931090071;
        mso-list-template-ids:1481132352;}
@list l1:level1
        {mso-level-tab-stop:36.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;}
@list l1:level2
        {mso-level-tab-stop:72.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;}
@list l1:level3
        {mso-level-tab-stop:108.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;}
@list l1:level4
        {mso-level-tab-stop:144.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;}
@list l1:level5
        {mso-level-tab-stop:180.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;}
@list l1:level6
        {mso-level-tab-stop:216.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;}
@list l1:level7
        {mso-level-tab-stop:252.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;}
@list l1:level8
        {mso-level-tab-stop:288.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;}
@list l1:level9
        {mso-level-tab-stop:324.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;}
ol
        {margin-bottom:0cm;}
ul
        {margin-bottom:0cm;}
--></style>
</head>
<body lang="en-CZ" link="#0563C1" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<div>
<p class="MsoNormal" style="margin-left:36.0pt">On 19.04.2023 17:21, "Brian Goetz" <brian.goetz@oracle.com> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal" style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:36.0pt">
<br>
<span style="font-size:13.5pt;font-family:"Courier New"">Overall this is a good plan.  There's a small gap regarding control of stack map generation. 
<br>
<br>
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.
<br>
<br>
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:<br>
<br>
 - Generate a stackmap if the option is set and classfile version >= 50<br>
 - If JSR/RET is generated in classfile > 51, throw from CodeBuilder<br>
 - If stackmap generation encounters JSR/RET in classfile 50, throw from </span><span lang="EN-US" style="font-size:13.5pt;font-family:"Courier New"">StackMapGenerator<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span lang="EN-US">The last condition unfortunately does not allow to agnostically process classes of unknown versions (the use case of jlink
</span><span lang="EN-US">StripJavaDebugAttributesPlugin)</span><span lang="EN-US">. If a classfile version 50 with JSR/RET instructions appears, there is no way how to turn off the stackmap generation for transformation of already parsed classfile. We have
 only a global boolean option for stackmap generation and it must be set before we know the class version. We may alternatively swallow the exception for classfile version 50 or call generator based on detection of JSR/RET for classfile version 50.
<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span lang="EN-US">There are more cases asking for different behavior of stack map generation. For example to fix missing stackmaps or generate them always.<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span lang="EN-US">We may solve it by a multi-state global option to generate stack maps (never, when mandatory, always).<o:p></o:p></span></p>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span lang="EN-US">Or we may remove the global option and move the decision point down to the CodeBuilder. Default behavior would be “when mandatory” and user can prevent or enforce stackmaps generation for
 individual methods as a part of transformation or building.<o:p></o:p></span></p>
<p class="MsoNormal" style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:36.0pt">
<span style="font-size:13.5pt;font-family:"Courier New""><br>
<br>
<br>
</span><o:p></o:p></p>
<div>
<p class="MsoNormal" style="margin-left:36.0pt">On 4/19/2023 9:29 AM, Adam Sotona wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US">Hi,</span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US">Work on javap conversion from ASM to Classfile API and recent bug (<a href="https://bugs.openjdk.org/browse/JDK-8305990">JDK-8305990</a>) discovered in StripJavaDebugAttributesPlugin brought
 attention to missing JSR and RET instructions support in Classfile API.</span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US"> </span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US">I’m proposing following changes to Classfile API:</span><o:p></o:p></p>
<p class="MsoListParagraph" style="margin-left:72.0pt;text-indent:-18.0pt;mso-list:l1 level1 lfo3">
<![if !supportLists]><span style="mso-list:Ignore">1.<span style="font:7.0pt "Times New Roman"">      
</span></span><![endif]><span lang="EN-US">Drop Opcode.Kind.UNSUPPORTED and add Opcode.Kind.DISCONTINUED_JSR and DISCONTINUED_RET</span><o:p></o:p></p>
<p class="MsoListParagraph" style="margin-left:72.0pt;text-indent:-18.0pt;mso-list:l1 level1 lfo3">
<![if !supportLists]><span style="mso-list:Ignore">2.<span style="font:7.0pt "Times New Roman"">      
</span></span><![endif]><span lang="EN-US">Add DiscontinuedInstruction interface with inner JsrInstruction and RetInstruction with standard factory methods</span><o:p></o:p></p>
<p class="MsoListParagraph" style="margin-left:72.0pt;text-indent:-18.0pt;mso-list:l1 level1 lfo3">
<![if !supportLists]><span style="mso-list:Ignore">3.<span style="font:7.0pt "Times New Roman"">      
</span></span><![endif]><span lang="EN-US">CodeImpl will parse and stream these instructions when present in the Code attribute</span><o:p></o:p></p>
<p class="MsoListParagraph" style="margin-left:72.0pt;text-indent:-18.0pt;mso-list:l1 level1 lfo3">
<![if !supportLists]><span style="mso-list:Ignore">4.<span style="font:7.0pt "Times New Roman"">      
</span></span><![endif]><span lang="EN-US">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</span><o:p></o:p></p>
<p class="MsoListParagraph" style="margin-left:72.0pt;text-indent:-18.0pt;mso-list:l1 level1 lfo3">
<![if !supportLists]><span style="mso-list:Ignore">5.<span style="font:7.0pt "Times New Roman"">      
</span></span><![endif]><span lang="EN-US">Fix DirectCodeBuilder so it invokes StackMapGenerator only for class file version >= 51.0 (BufWriterImpl must hold class version for that purpose)</span><o:p></o:p></p>
<p class="MsoListParagraph" style="margin-left:72.0pt;text-indent:-18.0pt;mso-list:l1 level1 lfo3">
<![if !supportLists]><span style="mso-list:Ignore">6.<span style="font:7.0pt "Times New Roman"">      
</span></span><![endif]><span lang="EN-US">Fix  StackMapGenerator error message when these instruction appear in during generation</span><o:p></o:p></p>
<p class="MsoListParagraph" style="margin-left:72.0pt;text-indent:-18.0pt;mso-list:l1 level1 lfo3">
<![if !supportLists]><span style="mso-list:Ignore">7.<span style="font:7.0pt "Times New Roman"">      
</span></span><![endif]><span lang="EN-US">Implement fallback to jump target inflation in CodeImpl when the StackMapTable attribute is missing and class file version is < 51.0</span><o:p></o:p></p>
<p class="MsoListParagraph" style="margin-left:72.0pt;text-indent:-18.0pt;mso-list:l1 level1 lfo3">
<![if !supportLists]><span style="mso-list:Ignore">8.<span style="font:7.0pt "Times New Roman"">      
</span></span><![endif]><span lang="EN-US">Extend tests to verify JSR and RET instructions are correctly generated, transformed and reported as error (based on class file version).</span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US"> </span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US">Please review and comment this proposal under following pull request:</span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US"><a href="https://github.com/openjdk/jdk/pull/13478">https://github.com/openjdk/jdk/pull/13478</a></span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US"> </span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US">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.</span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US"> </span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US">Thanks,</span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:36.0pt"><span lang="EN-US">Adam</span><o:p></o:p></p>
</blockquote>
<p class="MsoNormal" style="margin-left:36.0pt"><span style="mso-ligatures:none"><o:p> </o:p></span></p>
</div>
</body>
</html>