Re: NegativeArraySizeException merging frames during replaceAsmInstructions

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

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

Le 17/03/2022 16:15, Eirik Bjørsnøs a écrit :
> Eric,
> 
> Thanks a lot for your patience :-)
> 
> I finally figured out that I did indeed have one branch target which
> was missing a frame. The reason I thought I didn't was that the
> particular class being instrumented was a Java 6 class, so frames are
> not strictly required by the JVM.

If your class is a Java 6 class but contains frames before 
instrumentation, another solution might be to remove all the frames as 
part of the instrumentation (and to not add any for your new branch 
targets).

> Eirik.
> 
> On Thu, Mar 17, 2022 at 2:32 PM <[email protected]> wrote:
> 
>> 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

------------=_1647531237-25320-12
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

------------=_1647531237-25320-12--