<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<font size="4"><font face="monospace">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 StackMapGenerator<br>
<br>
<br>
</font></font><br>
<div class="moz-cite-prefix">On 4/19/2023 9:29 AM, Adam Sotona
wrote:<br>
</div>
<blockquote type="cite" cite="mid:CY4PR1001MB2150AC33FDE9291AE3121B628C629@CY4PR1001MB2150.namprd10.prod.outlook.com">
<meta name="Generator" content="Microsoft Word 15 (filtered
medium)">
<style>@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}@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;}p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
mso-ligatures:standardcontextual;
mso-fareast-language:EN-US;}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;
mso-fareast-language:EN-US;}span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;
mso-fareast-language:EN-US;}div.WordSection1
{page:WordSection1;}ol
{margin-bottom:0cm;}ul
{margin-bottom:0cm;}</style>
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US">Hi,<o:p></o:p></span></p>
<p class="MsoNormal"><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" moz-do-not-send="true">JDK-8305990</a>) discovered in
StripJavaDebugAttributesPlugin brought attention to missing
JSR and RET instructions support in Classfile API.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">I’m proposing following
changes to Classfile API:<o:p></o:p></span></p>
<ul style="margin-top:0cm" type="disc">
<li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo2"><span lang="EN-US">Drop Opcode.Kind.UNSUPPORTED and add
Opcode.Kind.DISCONTINUED_JSR and DISCONTINUED_RET<o:p></o:p></span></li>
<li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo2"><span lang="EN-US">Add DiscontinuedInstruction interface with
inner JsrInstruction and RetInstruction with standard
factory methods<o:p></o:p></span></li>
<li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo2"><span lang="EN-US">CodeImpl will parse and stream these
instructions when present in the Code attribute<o:p></o:p></span></li>
<li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo2"><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<o:p></o:p></span></li>
<li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo2"><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)<o:p></o:p></span></li>
<li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo2"><span lang="EN-US">Fix StackMapGenerator error message when
these instruction appear in during generation<o:p></o:p></span></li>
<li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo2"><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<o:p></o:p></span></li>
<li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo2"><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).<o:p></o:p></span></li>
</ul>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Please review and
comment this proposal under following pull request:<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><a href="https://github.com/openjdk/jdk/pull/13478" moz-do-not-send="true" class="moz-txt-link-freetext">https://github.com/openjdk/jdk/pull/13478</a><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><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.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Adam<o:p></o:p></span></p>
</div>
</blockquote>
<br>
</body>
</html>