Re: NegativeArraySizeException merging frames during replaceAsmInstructions
Eirik Bjørsnøs (via asm Mailing List) <[email protected]> Thu, 17 Mar 2022 12:50:52 +0100
| Newsgroups | gmane.comp.java.objectweb.asm |
|---|---|
| Message-ID | <CA+pBWhse_jxoD1p-4kOXUUzhkAVMNuqoLmFCtsu25YzaTnvM9A@mail.gmail.com> |
This is a multi-part message in MIME format... ------------=_1647517867-25320-7 Content-Type: multipart/alternative; boundary="00000000000007f42c05da68a5c1" --00000000000007f42c05da68a5c1 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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. 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 =3D new Label(); > end =3D 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=C3=B8rsn=C3=B8s a =C3=A9crit : > > 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(ReplaceAsmInstruction= sNASE.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=3Dhelp > > OW2 mailing lists service home page: http://www.ow2.org/wws > --00000000000007f42c05da68a5c1 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><br><div>Eric,</div><div><br></div><div>When I add a frame= before the exception handler, the NASE magically no longer occurs.</div><d= iv><br></div><div>My "real" instrumentation uses a custom AdviceA= dapter which does not extend=C2=A0LocalVariablesSorter and thus does not re= quire expanded frames.</div><div><br></div><div>This will make it a bit har= der to make a simple reproducer that=C2=A0shows the=C2=A0real=C2=A0problem,= but I'll give it a go :-)</div><div><br></div><div>Eirik.</div></div><= br><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">On Thu,= Mar 17, 2022 at 11:40 AM <<a href=3D"mailto:[email protected]">ebruneto= [email protected]</a>> wrote:<br></div><blockquote class=3D"gmail_quote" style= =3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding= -left:1ex">Hi,<br> <br> The issue is that your instrumentation changes the control flow of a <br> method, with the addition of a try catch block, but the ClassWriter only <b= r> has a COMPUTE_MAXS option. As a result, frames are not recomputed, and <br> there is a missing frame just before the exception handler. To fix this <br> you can either use COMPUTE_FRAMES instead of COMPUTE_MAXS, or manually <br> compute and insert the missing frame before the ATHROW with a <br> vistFrame(...).<br> <br> Eric<br> <br> PS: in your TryCatchWrapper, it is better to visit the try catch block <br> before its labels, as follows (and as documented in MethodVisitor):<br> <br> =C2=A0 =C2=A0 =C2=A0private static class TryCatchWrapper extends AdviceAdap= ter {<br> <br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0private Label start;<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0private Label end;<br> <br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public TryCatchWrapper(MethodVisitor mv, = int access, String <br> name, String descriptor) {<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0super(ASM9, mv, access, nam= e, descriptor);<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> <br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0@Override<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0protected void onMethodEnter() {<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0start =3D new Label();<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0end =3D new Label();<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0visitTryCatchBlock(start, e= nd, end, null);<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mv.visitLabel(start);<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> <br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0@Override<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public void visitMaxs(int maxStack, int m= axLocals) {<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0visitLabel(end);<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0visitInsn(ATHROW);<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0super.visitMaxs(maxStack, m= axLocals);<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> =C2=A0 =C2=A0 =C2=A0}<br> <br> <br> Le 16/03/2022 20:59, Eirik Bj=C3=B8rsn=C3=B8s a =C3=A9crit=C2=A0:<br> > Hi,<br> > <br> > I'm observing a NASE during toByteArray/replaceAsmInstructions whi= le<br> > instrumenting byte code in a Java agent I'm working on.<br> > <br> > I have been able to create a standalone reproducer [1] which provokes<= br> > the following conditions:<br> > <br> > 1: One of the methods in the class should incude a Label which returns= <br> > false from the Label.resolve method. This is because the label is a<br> > forward reference with a relative offset larger than Short.MAX_VALUE,<= br> > hence ASMs hasAsmInstructions is set to true. This seems necessary to<= br> > force a call to replaceAsmInstructions which triggers frame<br> > processing.<br> > <br> > 2: Another method of the class is instrumented with a trycatchblock<br> > which wraps the body of the method This catch block handler simply<br> > ATHROWs the Throwable. This seems necessary to create the conditions<b= r> > where the frame merge throws NASE.<br> > <br> > Here's the stack trace cased by the minimal reproducer:<br> > <br> > Exception in thread "main" java.lang.NegativeArraySizeExcept= ion: -1<br> > at org.objectweb.asm.Frame.merge(Frame.java:1222)<br> > at org.objectweb.asm.CurrentFrame.execute(CurrentFrame.java:53)<br> > at org.objectweb.asm.MethodWriter.visitInsn(MethodWriter.java:868)<br> > at org.objectweb.asm.ClassReader.readCode(ClassReader.java:2213)<br> > at org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1514)<br> > at org.objectweb.asm.ClassReader.accept(ClassReader.java:744)<br> > at<br> > org.objectweb.asm.ClassWriter.replaceAsmInstructions(ClassWriter.java:= 755)<br> > at org.objectweb.asm.ClassWriter.toByteArray(ClassWriter.java:718)<br> > at<br> > com.example.asmnase.ReplaceAsmInstructionsNASE.main(ReplaceAsmInstruct= ionsNASE.java:37)<br> > <br> > The following Gist contains the reproducer:<br> > <br> > <a href=3D"https://gist.github.com/eirbjo/3eb8ca755f2a399ac8e5def6f434= 39e5" rel=3D"noreferrer" target=3D"_blank">https://gist.github.com/eirbjo/3= eb8ca755f2a399ac8e5def6f43439e5</a><br> > <br> > Note that the instrumentation produces valid class files which are<br> > verified and loaded by the JVM. It's only in the abnormal case of<= br> > large relative forward references that ASM throws this exception. So I= <br> > think the produced byte code is valid and there might be a bug in ASM<= br> > here.<br> > <br> > What do you think?<br> > <br> > Cheers,<br> > Eirik.<br> > <br> > --<br> > You receive this message as a subscriber of the <a href=3D"mailto:asm@= ow2.org" target=3D"_blank">[email protected]</a> mailing <br> > list.<br> > To unsubscribe: mailto:<a href=3D"mailto:[email protected]" targ= et=3D"_blank">[email protected]</a><br> > For general help: mailto:<a href=3D"mailto:[email protected]" target=3D"_b= lank">[email protected]</a>?subject=3Dhelp<br> > OW2 mailing lists service home page: <a href=3D"http://www.ow2.org/wws= " rel=3D"noreferrer" target=3D"_blank">http://www.ow2.org/wws</a><br> </blockquote></div> --00000000000007f42c05da68a5c1-- ------------=_1647517867-25320-7 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 ------------=_1647517867-25320-7--