Re: NegativeArraySizeException merging frames during replaceAsmInstructions

This is a multi-part message in MIME format...

------------=_1647523951-25320-9
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit

Le 17/03/2022 12:50, Eirik Bjørsnøs a écrit :
> Eric,
> 
> When I add a frame before the exception handler, the NASE magically no
> longer occurs.
> 
> My "real" instrumentation uses a custom AdviceAdapter which does not
> extend LocalVariablesSorter and thus does not require expanded frames.

Even if you don't need EXPAND_FRAMES, if your instrumentation changes 
the control flow (e.g. with new if or goto statements, or with try catch 
blocks) you still need to update or insert new frames accordingly 
(either manually or with COMPUTE_FRAMES).

> This will make it a bit harder to make a simple reproducer that shows
> the real problem, but I'll give it a go :-)
> 
> Eirik.
> 
> On Thu, Mar 17, 2022 at 11:40 AM <[email protected]> wrote:
> 
>> Hi,
>> 
>> The issue is that your instrumentation changes the control flow of a
>> 
>> method, with the addition of a try catch block, but the ClassWriter
>> only
>> has a COMPUTE_MAXS option. As a result, frames are not recomputed,
>> and
>> there is a missing frame just before the exception handler. To fix
>> this
>> you can either use COMPUTE_FRAMES instead of COMPUTE_MAXS, or
>> manually
>> compute and insert the missing frame before the ATHROW with a
>> vistFrame(...).
>> 
>> Eric
>> 
>> PS: in your TryCatchWrapper, it is better to visit the try catch
>> block
>> before its labels, as follows (and as documented in MethodVisitor):
>> 
>> private static class TryCatchWrapper extends AdviceAdapter {
>> 
>> private Label start;
>> private Label end;
>> 
>> public TryCatchWrapper(MethodVisitor mv, int access, String
>> 
>> name, String descriptor) {
>> super(ASM9, mv, access, name, descriptor);
>> }
>> 
>> @Override
>> protected void onMethodEnter() {
>> start = new Label();
>> end = new Label();
>> visitTryCatchBlock(start, end, end, null);
>> mv.visitLabel(start);
>> }
>> 
>> @Override
>> public void visitMaxs(int maxStack, int maxLocals) {
>> visitLabel(end);
>> visitInsn(ATHROW);
>> super.visitMaxs(maxStack, maxLocals);
>> }
>> }
>> 
>> Le 16/03/2022 20:59, Eirik Bjørsnøs a écrit :
>>> Hi,
>>> 
>>> I'm observing a NASE during toByteArray/replaceAsmInstructions
>> while
>>> instrumenting byte code in a Java agent I'm working on.
>>> 
>>> I have been able to create a standalone reproducer [1] which
>> provokes
>>> the following conditions:
>>> 
>>> 1: One of the methods in the class should incude a Label which
>> returns
>>> false from the Label.resolve method. This is because the label is
>> a
>>> forward reference with a relative offset larger than
>> Short.MAX_VALUE,
>>> hence ASMs hasAsmInstructions is set to true. This seems necessary
>> to
>>> force a call to replaceAsmInstructions which triggers frame
>>> processing.
>>> 
>>> 2: Another method of the class is instrumented with a
>> trycatchblock
>>> which wraps the body of the method This catch block handler simply
>>> ATHROWs the Throwable. This seems necessary to create the
>> conditions
>>> where the frame merge throws NASE.
>>> 
>>> Here's the stack trace cased by the minimal reproducer:
>>> 
>>> Exception in thread "main" java.lang.NegativeArraySizeException:
>> -1
>>> at org.objectweb.asm.Frame.merge(Frame.java:1222)
>>> at org.objectweb.asm.CurrentFrame.execute(CurrentFrame.java:53)
>>> at org.objectweb.asm.MethodWriter.visitInsn(MethodWriter.java:868)
>>> at org.objectweb.asm.ClassReader.readCode(ClassReader.java:2213)
>>> at org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1514)
>>> at org.objectweb.asm.ClassReader.accept(ClassReader.java:744)
>>> at
>>> 
>> 
> org.objectweb.asm.ClassWriter.replaceAsmInstructions(ClassWriter.java:755)
>>> at org.objectweb.asm.ClassWriter.toByteArray(ClassWriter.java:718)
>>> at
>>> 
>> 
> com.example.asmnase.ReplaceAsmInstructionsNASE.main(ReplaceAsmInstructionsNASE.java:37)
>>> 
>>> The following Gist contains the reproducer:
>>> 
>>> https://gist.github.com/eirbjo/3eb8ca755f2a399ac8e5def6f43439e5
>>> 
>>> Note that the instrumentation produces valid class files which are
>>> verified and loaded by the JVM. It's only in the abnormal case of
>>> large relative forward references that ASM throws this exception.
>> So I
>>> think the produced byte code is valid and there might be a bug in
>> ASM
>>> here.
>>> 
>>> What do you think?
>>> 
>>> Cheers,
>>> Eirik.
>>> 
>>> --
>>> You receive this message as a subscriber of the [email protected]
>> mailing
>>> list.
>>> To unsubscribe: mailto:[email protected]
>>> For general help: mailto:[email protected]?subject=help
>>> OW2 mailing lists service home page: http://www.ow2.org/wws

------------=_1647523951-25320-9
Content-Type: text/plain; charset="UTF-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit


-- 
You receive this message as a subscriber of the [email protected] mailing list.
To unsubscribe: mailto:[email protected]
For general help: mailto:[email protected]?subject=help
OW2 mailing lists service home page: http://www.ow2.org/wws

------------=_1647523951-25320-9--