<div dir="ltr"><div>Hei Chen,</div><div><br></div><div>I think I understand the problem then. With ASM, type annotations on elements are processed just after the element was encountered. From the javadoc of ASM: "Visits an annotation on an instruction. This method must be called just after the annotated instruction." In contrast, the Class File API requires a label to be inserted "just before" the instruction is added. <br></div><div><br></div><div>Ideally, CodeBuilder would allow me to call something like "newBoundLabelOnPreviousInstruction", but I understand if you do not want to add something like it. If the Class File API allowed to add labels both prior or after encountering an instruction, one could work with both approaches of inserting type annotations prior or after an instruction is added. This would make migrating from ASM more straight-forward, too, as I assume that this is the prevalent model today with ASM being so dominant.<br></div><div><br></div><div>Otherwise, I will need to work around and delay any ASM invocation for my bridge, to consider if a subsequent call represents a type annotation and apply a reordering this way. This is not a big deal, but of course it would be much easier solved if I could bind a label with a BCI offset further up the stack. Possibly, I could even add a label before any annotatable instruction, just in case. Would this be overly expensive to do?</div><div><br></div><div>Cheers, Rafael<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Am Mo., 11. Nov. 2024 um 00:12 Uhr schrieb Chen Liang <<a href="mailto:chen.l.liang@oracle.com">chen.l.liang@oracle.com</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg8361609369171322493">
<div dir="ltr">
<div style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
Hi Rafael,</div>
<div style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
I believe this is due to a misunderstanding with how ClassFile API labels work. A ClassFile API label models an insertion point represented by a BCI; so in your example, the bound label should be located right before the new instruction, so that the next immediately
executed instruction would be the new instruction. This is like how the labels for control flows work.</div>
<div style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<br>
</div>
<div style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
Unfortunately, I understand this is distinct from the ASM model - ASM visitor model treats this as if this is a tiny tree structure and considers a type annotation as a subsidiary of an instruction, and this brings challenges to porting the implementations.
Here's some context: we ClassFile API developers long believed that type annotations interact poorly with class file transformations because of hard-to-track arbitrary references such as interface indices, and thus we did not integrate it as a part of the
ClassFile API code streaming model, and we drops type annotation attributes in transformations by the attribute stability settings.</div>
<div style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<br>
</div>
<div style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
Regards,</div>
<div style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
Chen Liang</div>
<div id="m_8361609369171322493appendonsend"></div>
<hr style="display:inline-block;width:98%">
<div id="m_8361609369171322493divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> classfile-api-dev <<a href="mailto:classfile-api-dev-retn@openjdk.org" target="_blank">classfile-api-dev-retn@openjdk.org</a>> on behalf of Rafael Winterhalter <<a href="mailto:rafael.wth@gmail.com" target="_blank">rafael.wth@gmail.com</a>><br>
<b>Sent:</b> Saturday, November 9, 2024 3:09 PM<br>
<b>To:</b> classfile-api-dev <<a href="mailto:classfile-api-dev@openjdk.org" target="_blank">classfile-api-dev@openjdk.org</a>><br>
<b>Subject:</b> Offset error when writing type annotations on NEW instructions</font>
<div> </div>
</div>
<div>
<div dir="ltr">
<div>With the most recent build of JDK 24, I discover one test failure in my test suite when comparing with ASM (<a href="https://github.com/raphw/asm-jdk-bridge" target="_blank">https://github.com/raphw/asm-jdk-bridge</a>): Any type annotation that is added to a "new" instruction
is added to the subsequent instruction instead. This might irritate parsers as the label will point to an instruction that cannot normally be annotated, as NEW is normally followed by DUP.</div>
<div><br>
</div>
<div>I created a reproducers that compares a javac compiled class and one that is created with the class file API. The error is likely in the attribute mapper's write method as I cannot reproduce it when testing a class file reader using the class file API
compared to ASM. The reproducer is as follows and is visualized in the first line of each of the two representations where the correct byte code offset is 0, not 3 as the annotation is added to the first instruction in the method:</div>
<div><br>
</div>
<div>
<div style="background-color:rgb(30,31,34);color:rgb(188,190,196)">
<pre style="font-family:"JetBrains Mono",monospace;font-size:9.8pt"><br><span style="color:rgb(207,142,109)">import </span>java.io.InputStream;<br><span style="color:rgb(207,142,109)">import </span>java.lang.annotation.ElementType;<br><span style="color:rgb(207,142,109)">import </span>java.lang.annotation.<span style="color:rgb(179,174,96)">Retention</span>;<br><span style="color:rgb(207,142,109)">import </span>java.lang.annotation.RetentionPolicy;<br><span style="color:rgb(207,142,109)">import </span>java.lang.annotation.<span style="color:rgb(179,174,96)">Target</span>;<br><span style="color:rgb(207,142,109)">import </span>java.lang.classfile.*;<br><span style="color:rgb(207,142,109)">import </span>java.lang.classfile.attribute.RuntimeVisibleTypeAnnotationsAttribute;<br><span style="color:rgb(207,142,109)">import </span>java.lang.classfile.instruction.LineNumber;<br><span style="color:rgb(207,142,109)">import </span>java.lang.classfile.instruction.LocalVariable;<br><span style="color:rgb(207,142,109)">import </span>java.lang.constant.ClassDesc;<br><span style="color:rgb(207,142,109)">import </span>java.lang.constant.MethodTypeDesc;<br><span style="color:rgb(207,142,109)">import </span>java.util.List;<br><br><span style="color:rgb(207,142,109)">public class </span>TypeAnnotationSample {<br><br> <span style="color:rgb(207,142,109)">void </span><span style="color:rgb(86,168,245)">m</span>() {<br> <span style="color:rgb(207,142,109)">new </span><span style="color:rgb(179,174,96)">@A </span>Object();<br> }<br><br> <span style="color:rgb(207,142,109)">public static void </span><span style="color:rgb(86,168,245)">main</span>(String[] args) <span style="color:rgb(207,142,109)">throws </span>Exception {<br> <span style="color:rgb(207,142,109)">byte</span>[] compiled;<br> <span style="color:rgb(207,142,109)">try </span>(InputStream inputStream = TypeAnnotationSample.<span style="color:rgb(207,142,109)">class</span>.getClassLoader().getResourceAsStream(<br> TypeAnnotationSample.<span style="color:rgb(207,142,109)">class</span>.getName().replace(<span style="color:rgb(106,171,115)">'.'</span>, <span style="color:rgb(106,171,115)">'/'</span>) + <span style="color:rgb(106,171,115)">".class"<br></span><span style="color:rgb(106,171,115)"> </span>)) {<br> compiled = inputStream.readAllBytes();<br> }<br> <span style="font-style:italic">print</span>(compiled);<br> System.<span style="color:rgb(199,125,187);font-style:italic">out</span>.println(<span style="color:rgb(106,171,115)">"-----"</span>);<br> <span style="color:rgb(207,142,109)">byte</span>[] created = ClassFile.<span style="font-style:italic">of</span>().build(ClassDesc.<span style="font-style:italic">of</span>(<span style="color:rgb(106,171,115)">"Synthetic"</span>), classBuilder -> {<br> classBuilder.withMethod(<span style="color:rgb(106,171,115)">"m"</span>, MethodTypeDesc.<span style="font-style:italic">of</span>(<span style="color:rgb(207,142,109)">void</span>.<span style="color:rgb(207,142,109)">class</span>.describeConstable().orElseThrow()), <span style="color:rgb(42,172,184)">0</span>, methodBuilder -> {<br> methodBuilder.withCode(codeBuilder -> {<br> codeBuilder.new_(Object.<span style="color:rgb(207,142,109)">class</span>.describeConstable().orElseThrow());<br> Label label = codeBuilder.newBoundLabel();<br> codeBuilder.with(RuntimeVisibleTypeAnnotationsAttribute.<span style="font-style:italic">of</span>(TypeAnnotation.<span style="font-style:italic">of</span>(<br> TypeAnnotation.TargetInfo.<span style="font-style:italic">ofNewExpr</span>(label),<br> List.<span style="font-style:italic">of</span>(),<br> Annotation.<span style="font-style:italic">of</span>(<span style="color:rgb(179,174,96)">A</span>.<span style="color:rgb(207,142,109)">class</span>.describeConstable().orElseThrow())<br> )));<br> codeBuilder.dup()<br> .invokespecial(Object.<span style="color:rgb(207,142,109)">class</span>.describeConstable().orElseThrow(), <span style="color:rgb(106,171,115)">"<init>"</span>, MethodTypeDesc.<span style="font-style:italic">of</span>(<span style="color:rgb(207,142,109)">void</span>.<span style="color:rgb(207,142,109)">class</span>.describeConstable().orElseThrow()))<br> .pop()<br> .return_();<br> });<br> });<br> });<br> <span style="font-style:italic">print</span>(created);<br> }<br><br> <span style="color:rgb(207,142,109)">private static void </span><span style="color:rgb(86,168,245)">print</span>(<span style="color:rgb(207,142,109)">byte</span>[] classFile) {<br> ClassModel classModel = ClassFile.<span style="font-style:italic">of</span>().parse(classFile);<br> MethodModel methodModel = classModel.methods().stream()<br> .filter(element -> element.methodName().equalsString(<span style="color:rgb(106,171,115)">"m"</span>))<br> .findFirst()<br> .orElseThrow();<br> CodeModel codeModel = methodModel.code().orElseThrow();<br> codeModel.elementStream().forEach(element -> {<br> <span style="color:rgb(207,142,109)">switch </span>(element) {<br> <span style="color:rgb(207,142,109)">case </span>LocalVariable _ -> { }<br> <span style="color:rgb(207,142,109)">case </span>LineNumber _ -> { }<br> <span style="color:rgb(207,142,109)">case </span>RuntimeVisibleTypeAnnotationsAttribute a -> System.<span style="color:rgb(199,125,187);font-style:italic">out</span>.println(a.annotations().stream()<br> .map(x -> x.annotation()<br> + <span style="color:rgb(106,171,115)">"@" </span>+ x.targetPath()<br> + <span style="color:rgb(106,171,115)">"/" </span>+ ((TypeAnnotation.OffsetTarget) x.targetInfo()).target())<br> .toList());<br> <span style="color:rgb(207,142,109)">default </span>-> System.<span style="color:rgb(199,125,187);font-style:italic">out</span>.println(element);<br> }<br> });<br> }<br><br> <span style="color:rgb(179,174,96)">@Retention</span>(RetentionPolicy.<span style="color:rgb(199,125,187);font-style:italic">RUNTIME</span>)<br> <span style="color:rgb(179,174,96)">@Target</span>(ElementType.<span style="color:rgb(199,125,187);font-style:italic">TYPE_USE</span>)<br> @<span style="color:rgb(207,142,109)">interface </span><span style="color:rgb(179,174,96)">A </span>{<br> }<br>}</pre>
</div>
</div>
<div><br>
</div>
<div>I think that the attribute mapper must subtract the bytes of the instruction as it is currently adding the bytes at the end of the instruction, not at its beginning. ASM and javac set the annotation offset to the beginning of the statement, so I understand
that this should be the right approach.</div>
<div><br>
</div>
<div>Thanks, Rafael<br>
</div>
</div>
</div>
</div>
</div></blockquote></div>