<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Rafael, first for your annotation output, how did you produce those outputs? I don't think ClassFile API performs any check to ensure an annotation's structure matches that of its annotation interface.</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<ol start="1" data-editing-info="{"orderedStyleType":1,"unorderedStyleType":1}" data-listchain="__List_Chain_57" style="margin-top: 0px; margin-bottom: 0px; list-style-type: decimal;">
<li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<div class="elementToProof">Thanks for the reproducer. It is indeed a bug with BytecodeHelpers, which should use lookupDescriptor instead of invocationType. Filed
<a href="https://bugs.openjdk.org/browse/JDK-8338406">https://bugs.openjdk.org/browse/JDK-8338406</a> which I will fix.</div>
</li><li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<div class="elementToProof">I am thinking of exposing the label to bci conversion to attribute writers for Code: that way, we can create a custom LineNumberTable attribute that only performs writing, and in writing it converts its labels to bci, which should
fit our need here. What do you think?</div>
</li><li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<div class="elementToProof">These instructions are supported: You need to call `codeBuilder.with(DiscontinuedInstruction.JsrInstruction.of(...))` to create them instead of through convenience factories. And our stack maps generation automatically fails and
falls back to stack counter in default configuration if it encounters a Java 6 class file with jsr/ret.</div>
</li></ol>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
For the annotation thing, if you can provide the source code, I can help you diagnose. I am currently working on both core reflection and classfile API areas.</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Chen</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Rafael Winterhalter <rafael.wth@gmail.com><br>
<b>Sent:</b> Wednesday, August 14, 2024 6:05 AM<br>
<b>To:</b> Chen Liang <chen.l.liang@oracle.com><br>
<b>Cc:</b> classfile-api-dev <classfile-api-dev@openjdk.org><br>
<b>Subject:</b> Re: [External] : Re: ASM to OpenJDK ClassReader/ClassWriter bridge: experiences and questions</font>
<div> </div>
</div>
<div>
<div dir="ltr">
<div>Quick finding: my tests that check for incompatible change in annotation values compared to their class file. There was a change in Java 21 (if I recall correctly) to display the actual value. This seems to have regressed. Since Byte Buddy emulates the
JDKs behavior, these tests are failing now compared to the mainline. For example, if an annotation changes its property from int to array int[], previously compiled annotations are toString-ed on mainline JDK as:</div>
<div><br>
</div>
<div>incompatibleValueArray=/* Warning type mismatch! "java.lang.Integer[42]" */</div>
<div><br>
</div>
<div>The ClassFile API branch toString-s the annotation as it was done prior to Java 21:<br>
</div>
<div><br>
</div>
<div>incompatibleValueArray=/* Warning type mismatch! "Array with component tag: I" */</div>
<div><br>
</div>
<div>I suggest to use the mainline expression as this makes debugging things much easier.</div>
<div>Thanks, Rafael<br>
</div>
</div>
<br>
<div class="x_gmail_quote">
<div dir="ltr" class="x_gmail_attr">Am Mi., 14. Aug. 2024 um 12:36 Uhr schrieb Rafael Winterhalter <<a href="mailto:rafael.wth@gmail.com">rafael.wth@gmail.com</a>>:<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div dir="ltr">
<div>Hello,</div>
<div><br>
</div>
<div>after some further fixes, I am down to 30 failed tests of 10436 tests in total now, only using the ClassFile API for reading and writing classes with ASM as an intermediate. I have not got the stack map frame generation to work yet, but will tackle this
next. The remaining <br>
</div>
<div><br>
</div>
1. I will try to create a reproducer, but in short:
<div>
<div>
<div>
<div><br>
DirectMethodHandleDesc.Kind kind = DirectMethodHandleDesc.Kind.CONSTRUCTOR;<br>
ClassDesc owner = ClassDesc.of("sample.DynamicConstantBootstrap");<br>
String name = "<init>";<br>
String lookupDescriptor = "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;)V";<br>
DirectMethodHandleDesc methodHandle = MethodHandleDesc.of(kind, owner, name, lookupDescriptor);<br>
System.out.println(methodHandle.invocationType().returnType());</div>
<div><br>
</div>
<div>The printed return type will be the owner type. If I change the kind to STATIC, it will be void as expected. When adding this to a LCD instruction, it will be converted to a constant pool entry where the return type is validated against it being a constructor
returning void. The conversion happens in BytecodeHelpers.handleConstantDescToHandleInfo where the patched return type is used for creating the symbol what fails validation.<br>
</div>
<div><br>
</div>
<div>2. The line number thing causes another row of issues. If, as suggested previously, the line number attribute could accept a label rather than a BCI, this could be fixed easily.</div>
<div><br>
</div>
<div>3. Some tests fail due to the impossibility of handling JSR and RET instructions. I fully understand that those are old, but especially JDBC drivers are often compiled to rather old Java versions. Java agents often instrument those, and it would be a pity
if those would not be supported therefore. Is there a plan to add support for these instructions? Fortunately, stack map frame generation does not need to consider them as these old class files do not require stack map frames. Adding them is not the worst
for this reason.</div>
<div><br>
</div>
<div>With these three things in place, I think I can offer full support for the new API starting the day this becomes non-experimental.<br>
</div>
<div><br>
</div>
<div>Best regards, Rafael<br>
</div>
</div>
</div>
</div>
</div>
<br>
<div class="x_gmail_quote">
<div dir="ltr" class="x_gmail_attr">Am Mi., 14. Aug. 2024 um 00:43 Uhr schrieb Chen Liang <<a href="mailto:chen.l.liang@oracle.com" target="_blank">chen.l.liang@oracle.com</a>>:<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div>
<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, thanks for the quick followup!</div>
<div style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
Indeed, the line number label handling enhancement would be helpful. (and that actually applies to all user attribute encoding of Labels, as currently user attributes cannot convert labels to bcis too)</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)">
For the following stacktrace, I don't think JDK's constants API is wrong: you should use (Lookup, String, Class) -> void type for the bootstrap method's method handle type. I checked bytebuddy's JavaConstant.Dynamic and JavaConstantDynamicTest and still have
no clue how that happens, or how it passes elsewhere. It would be great if you can provide a simple generated class file or its javap output, especially if this class can run on java command line so we can check the VM behavior.</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)">
And congratulations on the progress! The "dead code" warning shouldn't happen usually, as ClassFile API has PATCH_DEAD_CODE by default for DeadCodeOption. These errors should go away if you restore the option.</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)">
Thanks again for trying out!</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="x_m_-2108534409459503927m_-2539508962093348512appendonsend"></div>
<hr style="display:inline-block; width:98%">
<div id="x_m_-2108534409459503927m_-2539508962093348512divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Rafael Winterhalter <<a href="mailto:rafael.wth@gmail.com" target="_blank">rafael.wth@gmail.com</a>><br>
<b>Sent:</b> Tuesday, August 13, 2024 5:08 PM<br>
<b>To:</b> Chen Liang <<a href="mailto:chen.l.liang@oracle.com" target="_blank">chen.l.liang@oracle.com</a>><br>
<b>Cc:</b> classfile-api-dev <<a href="mailto:classfile-api-dev@openjdk.org" target="_blank">classfile-api-dev@openjdk.org</a>><br>
<b>Subject:</b> [External] : Re: ASM to OpenJDK ClassReader/ClassWriter bridge: experiences and questions</font>
<div> </div>
</div>
<div>
<div dir="ltr">
<div>Thanks, I will try the manual stack map frame translation! As for line numbers: I also wanted to create the attribute manually. Unfortunately, LineNumberInfo only accepts a bci instead of a Label. If that can be adjusted (I think this is sensible as this
is the only location where bci seems to be exposed), I could solve the problem that way.</div>
<div><br>
</div>
<div>I also found a possible bug with the following stacktrace:</div>
<div><br>
</div>
<div>java.lang.IllegalArgumentException: Expected type of (T*)V for constructor, found MethodTypeDesc[(MethodHandles$Lookup,String,Class)DynamicConstantBootstrap]<br>
at java.base/java.lang.constant.DirectMethodHandleDescImpl.validateConstructor(DirectMethodHandleDescImpl.java:107)<br>
at java.base/java.lang.constant.DirectMethodHandleDescImpl.<init>(DirectMethodHandleDescImpl.java:75)<br>
at java.base/java.lang.constant.MethodHandleDesc.of(MethodHandleDesc.java:89)<br>
at java.base/jdk.internal.classfile.impl.AbstractPoolEntry$MethodHandleEntryImpl.asSymbol(AbstractPoolEntry.java:919)<br>
at java.base/java.lang.classfile.constantpool.ConstantDynamicEntry.asSymbol(ConstantDynamicEntry.java:65)<br>
at java.base/jdk.internal.classfile.impl.StackMapGenerator.processLdc(StackMapGenerator.java:705)</div>
<div><br>
</div>
<div>This happens when using a constructor as the bootstrap for a dynamic constant. The check does not seem to consider that constructors are valid bootstraps similar to static methods.</div>
<div><br>
</div>
<div>Other than that, I ran Byte Buddy's entire test suite with about 10.000 tests using that bridge now and only about 200 tests seem to fail. Most commonly this is due to "Unable to generate stack map frame for dead code at bytecode offset 36 of method foobar()",
but this I might be able to solve with your suggestion. The reminder are mostly about edge cases that I will investigate further.</div>
<div><br>
</div>
<div>Well done! If these two issues can be resolved, I plan to offer an adapter into Byte Buddy once this is public API.</div>
<div>Rafael<br>
</div>
</div>
<br>
<div>
<div dir="ltr">Am Di., 13. Aug. 2024 um 23:29 Uhr schrieb Chen Liang <<a href="mailto:chen.l.liang@oracle.com" target="_blank">chen.l.liang@oracle.com</a>>:<br>
</div>
<blockquote style="margin:0px 0px 0px 0.8ex; border-left:1px solid rgb(204,204,204); padding-left:1ex">
<div>
<div dir="ltr">
<div style="direction:ltr; font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
Hi Rafael, thanks for your adoption!</div>
<ol start="1" style="direction:ltr; margin-top:0px; margin-bottom:0px; list-style-type:decimal">
<li style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<div style="direction:ltr">In fact, ClassFile API has quite a few non-standard (JVMS) attributes: CharacterRangeTable, CompilationID, ModuleHashes, ModuleResolution, ModuleTarget, SourceDebugExtension, SourceID, and seems there's little information available
about these attributes. I think you can treat them as if they are unknown attributes in your translations to ASM.</div>
</li><li style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<div style="direction:ltr">Line numbers are naturally streamed just like labels. If an ASM user is streaming line numbers late, an alternative way is to buffer the other elements into an ASM tree first, then inject the line number tokens on the second round.
(If LineNumberTableAttribute supports using Labels, then we can probably just write that attribute at last, too)</div>
</li><li style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<div style="direction:ltr">Unknown attributes are troubling the ClassFile API too. We have a stability() in AttributeMapper to indicate if we should retain or drop attributes. You can always copy any attribute over as-is by calling XxxBuilder::with(attribute).
You should always handle the unknown attributes with a default branch in a switch if you wish to be forward compatible, or an exhaustive switch if you wish to fail fast on newer known attributes.</div>
</li><li style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<div style="direction:ltr">For stack map frames, you can use StackMapsOption.DROP_STACK_MAPS and pass a StackMapsTableAttribute in CodeBuilder.with; this is already how ProxyGenerator does it. However we don't allow users to specify max stacks and locals, which
can be hurting performance sensitive use cases.</div>
</li></ol>
<div style="direction:ltr; font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<div style="direction:ltr; font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
Regards,</div>
<div style="direction:ltr; font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
Chen Liang</div>
<div id="x_m_-2108534409459503927m_-2539508962093348512x_m_1123692594695973572x_appendonsend">
</div>
<hr style="direction:ltr; display:inline-block; width:98%">
<div id="x_m_-2108534409459503927m_-2539508962093348512x_m_1123692594695973572x_divRplyFwdMsg" dir="ltr">
<span style="font-family:Calibri,sans-serif; font-size:11pt; color:rgb(0,0,0)"><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> Tuesday, August 13, 2024 4:51 AM<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> ASM to OpenJDK ClassReader/ClassWriter bridge: experiences and questions</span>
<div> </div>
</div>
<div style="direction:ltr">Hello!</div>
<div style="direction:ltr"><br>
</div>
<div style="direction:ltr">It's been a while since I last tried, but I now managed to implement both a JDK-based ClassReader and ClassWriter for ASM. This seems to work really well for a range of tests that I did as it allows me to avoid major rewrites of ASM-based
code while still having forward-compatibility as long as there are no new language constructs as the JDK bundles parser and serializer.</div>
<div style="direction:ltr"><br>
</div>
<div style="direction:ltr">The code can be found here: <a href="https://urldefense.com/v3/__https://github.com/raphw/asm-jdk-bridge__;!!ACWV5N9M2RV99hQ!KAQFTclWgq_IwySyN468Wdy3QUCwqLln_sFAi0SSvWoBHHyKMExsxZbmfhPm0axz2oGf-EbHxWkP391GDU6uYQ$" id="x_m_-2108534409459503927m_-2539508962093348512x_m_1123692594695973572OWA2740785e-b87c-54ec-3710-3f68e8ade305" target="_blank">
https://github.com/raphw/asm-jdk-bridge</a></div>
<div style="direction:ltr"><br>
</div>
<div style="direction:ltr">A few notes and questions I have:</div>
<div style="direction:ltr"><br>
</div>
<div style="direction:ltr">1. The OpenJDK API supports CharacterRangeTable which is a non-standard attribute that is not described in the specification. By introducing it to the official API, isn't the attribute in a way formalized? Is there a plan to standardize
the attribute?</div>
<div style="direction:ltr"><br>
</div>
<div style="direction:ltr">2. One thing that cannot be mapped directly from OpenJDK to ASM are line numbers. In ASM, they can be added later using a Label. In OpenJDK they have to be visited at "the right time". Using labels is otherwise common for other attributes,
both in ASM and OpenJDK, but line numbers are the exception. Is there a reason for this?</div>
<div style="direction:ltr"><br>
</div>
<div style="direction:ltr">3. It would be nice if there was a way to flag what attributes should be treated as UnknownAttributes. Right now, I retain UnknownAttributes as they are. But if a future release of the OpenJDK promotes an UnknownAttribute to a known
one, I might miss it when copying them over as raw arrays.</div>
<div style="direction:ltr"><br>
</div>
<div style="direction:ltr">4. I take it so that there are no plans to add support for manually defining StackMapFrame and method sizes? Sometimes, the OpenJDK erases information, for example if a local variable is never assigned a value, it will simply be treated
as "N" value. This is not a big issue, but it can make slight transformations that can have very subtle implications when for example writing Java agents, so I would still hope for an option for this to be used by advanced users.</div>
<div style="direction:ltr"><br>
</div>
<div style="direction:ltr">Thanks! Rafael</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
</div>
</body>
</html>