[External] : Re: ASM to OpenJDK ClassReader/ClassWriter bridge: experiences and questions
Rafael Winterhalter
rafael.wth at gmail.com
Wed Aug 14 16:34:51 UTC 2024
Brilliant, I am now down to:
(1) Waiting for a fix for the bug relating to BytecodeHelpers.
(2) issues with inability to define line numbers by labels, I think your
idea to add a translation method from labels to BCI (and having an
equivalent way of reading a BCI in the reader) is the right thing to do,
this will generally be useful for custom attributes.
(3) thanks, this resolves some more failing tests.
I now also got the manual computation of stack map frames to work. I would
however love if I could control the exact frames that are issued. Ideally
agents change as little as possible to stay compatible with other byte code
manipulation tools. This could be offered as a "write only" API similar to
what you suggest for line numbers. What do you think?
As for the annotations, I am still trying to find out what has changed. If
I understand it better, I can file a separate bug report, in case that it
is a regression.
Thanks, Rafael
Am Mi., 14. Aug. 2024 um 17:29 Uhr schrieb Chen Liang <
chen.l.liang at oracle.com>:
> 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.
>
>
> 1. Thanks for the reproducer. It is indeed a bug with BytecodeHelpers,
> which should use lookupDescriptor instead of invocationType. Filed
> https://bugs.openjdk.org/browse/JDK-8338406 which I will fix.
> 2. 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?
> 3. 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.
>
> 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.
>
> Chen
> ------------------------------
> *From:* Rafael Winterhalter <rafael.wth at gmail.com>
> *Sent:* Wednesday, August 14, 2024 6:05 AM
> *To:* Chen Liang <chen.l.liang at oracle.com>
> *Cc:* classfile-api-dev <classfile-api-dev at openjdk.org>
> *Subject:* Re: [External] : Re: ASM to OpenJDK ClassReader/ClassWriter
> bridge: experiences and questions
>
> 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:
>
> incompatibleValueArray=/* Warning type mismatch! "java.lang.Integer[42]" */
>
> The ClassFile API branch toString-s the annotation as it was done prior to
> Java 21:
>
> incompatibleValueArray=/* Warning type mismatch! "Array with component
> tag: I" */
>
> I suggest to use the mainline expression as this makes debugging things
> much easier.
> Thanks, Rafael
>
> Am Mi., 14. Aug. 2024 um 12:36 Uhr schrieb Rafael Winterhalter <
> rafael.wth at gmail.com>:
>
> Hello,
>
> 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
>
> 1. I will try to create a reproducer, but in short:
>
> DirectMethodHandleDesc.Kind kind =
> DirectMethodHandleDesc.Kind.CONSTRUCTOR;
> ClassDesc owner = ClassDesc.of("sample.DynamicConstantBootstrap");
> String name = "<init>";
> String lookupDescriptor =
> "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;)V";
> DirectMethodHandleDesc methodHandle = MethodHandleDesc.of(kind,
> owner, name, lookupDescriptor);
> System.out.println(methodHandle.invocationType().returnType());
>
> 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.
>
> 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.
>
> 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.
>
> With these three things in place, I think I can offer full support for the
> new API starting the day this becomes non-experimental.
>
> Best regards, Rafael
>
> Am Mi., 14. Aug. 2024 um 00:43 Uhr schrieb Chen Liang <
> chen.l.liang at oracle.com>:
>
> Hi Rafael, thanks for the quick followup!
> 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)
>
> 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.
>
> 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.
>
> Thanks again for trying out!
> Chen Liang
> ------------------------------
> *From:* Rafael Winterhalter <rafael.wth at gmail.com>
> *Sent:* Tuesday, August 13, 2024 5:08 PM
> *To:* Chen Liang <chen.l.liang at oracle.com>
> *Cc:* classfile-api-dev <classfile-api-dev at openjdk.org>
> *Subject:* [External] : Re: ASM to OpenJDK ClassReader/ClassWriter
> bridge: experiences and questions
>
> 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.
>
> I also found a possible bug with the following stacktrace:
>
> java.lang.IllegalArgumentException: Expected type of (T*)V for
> constructor, found
> MethodTypeDesc[(MethodHandles$Lookup,String,Class)DynamicConstantBootstrap]
> at
> java.base/java.lang.constant.DirectMethodHandleDescImpl.validateConstructor(DirectMethodHandleDescImpl.java:107)
> at
> java.base/java.lang.constant.DirectMethodHandleDescImpl.<init>(DirectMethodHandleDescImpl.java:75)
> at
> java.base/java.lang.constant.MethodHandleDesc.of(MethodHandleDesc.java:89)
> at
> java.base/jdk.internal.classfile.impl.AbstractPoolEntry$MethodHandleEntryImpl.asSymbol(AbstractPoolEntry.java:919)
> at
> java.base/java.lang.classfile.constantpool.ConstantDynamicEntry.asSymbol(ConstantDynamicEntry.java:65)
> at
> java.base/jdk.internal.classfile.impl.StackMapGenerator.processLdc(StackMapGenerator.java:705)
>
> 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.
>
> 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.
>
> Well done! If these two issues can be resolved, I plan to offer an adapter
> into Byte Buddy once this is public API.
> Rafael
>
> Am Di., 13. Aug. 2024 um 23:29 Uhr schrieb Chen Liang <
> chen.l.liang at oracle.com>:
>
> Hi Rafael, thanks for your adoption!
>
> 1. 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.
> 2. 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)
> 3. 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.
> 4. 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.
>
>
> Regards,
> Chen Liang
> ------------------------------
> *From:* classfile-api-dev <classfile-api-dev-retn at openjdk.org> on behalf
> of Rafael Winterhalter <rafael.wth at gmail.com>
> *Sent:* Tuesday, August 13, 2024 4:51 AM
> *To:* classfile-api-dev <classfile-api-dev at openjdk.org>
> *Subject:* ASM to OpenJDK ClassReader/ClassWriter bridge: experiences and
> questions
>
> Hello!
>
> 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.
>
> The code can be found here: https://github.com/raphw/asm-jdk-bridge
> <https://urldefense.com/v3/__https://github.com/raphw/asm-jdk-bridge__;!!ACWV5N9M2RV99hQ!KAQFTclWgq_IwySyN468Wdy3QUCwqLln_sFAi0SSvWoBHHyKMExsxZbmfhPm0axz2oGf-EbHxWkP391GDU6uYQ$>
>
> A few notes and questions I have:
>
> 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?
>
> 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?
>
> 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.
>
> 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.
>
> Thanks! Rafael
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20240814/a3b83101/attachment-0001.htm>
More information about the classfile-api-dev
mailing list