Re: NegativeArraySizeException merging frames during replaceAsmInstructions
Eirik Bjørsnøs (via asm Mailing List) <[email protected]> Thu, 17 Mar 2022 16:15:09 +0100
| Newsgroups | gmane.comp.java.objectweb.asm |
|---|---|
| Message-ID | <CA+pBWhsKAteiWYQLnQYHDmGWg6FTe2hsWTyHLqqfo+pU813L4A@mail.gmail.com> |
This is a multi-part message in MIME format... ------------=_1647530125-25320-11 Content-Type: multipart/alternative; boundary="00000000000093517005da6b7f0a" --00000000000093517005da6b7f0a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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. Eirik. On Thu, Mar 17, 2022 at 2:32 PM <[email protected]> wrote: > Le 17/03/2022 12:50, Eirik Bj=C3=B8rsn=C3=B8s a =C3=A9crit : > > 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 =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 > --00000000000093517005da6b7f0a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><br><div>Eric,</div><div><br></div><div>Thanks a lot for y= our patience :-)<br></div><div><br></div><div>I finally figured out that I = did indeed have one branch target which was missing a frame. The reason I t= hought I didn't was that the particular class being instrumented was a = Java 6 class, so frames are not strictly required by the JVM.</div><div><br= ></div><div>Eirik.=C2=A0</div></div><br><div class=3D"gmail_quote"><div dir= =3D"ltr" class=3D"gmail_attr">On Thu, Mar 17, 2022 at 2:32 PM <<a href= =3D"mailto:[email protected]">[email protected]</a>> wrote:<br></div><bl= ockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-lef= t:1px solid rgb(204,204,204);padding-left:1ex">Le 17/03/2022 12:50, Eirik B= j=C3=B8rsn=C3=B8s a =C3=A9crit=C2=A0:<br> > Eric,<br> > <br> > When I add a frame before the exception handler, the NASE magically no= <br> > longer occurs.<br> > <br> > My "real" instrumentation uses a custom AdviceAdapter which = does not<br> > extend LocalVariablesSorter and thus does not require expanded frames.= <br> <br> Even if you don't need EXPAND_FRAMES, if your instrumentation changes <= br> the control flow (e.g. with new if or goto statements, or with try catch <b= r> blocks) you still need to update or insert new frames accordingly <br> (either manually or with COMPUTE_FRAMES).<br> <br> > This will make it a bit harder to make a simple reproducer that shows<= br> > the real problem, but I'll give it a go :-)<br> > <br> > Eirik.<br> > <br> > On Thu, Mar 17, 2022 at 11:40 AM <<a href=3D"mailto:ebruneton@free.= fr" target=3D"_blank">[email protected]</a>> wrote:<br> > <br> >> Hi,<br> >> <br> >> The issue is that your instrumentation changes the control flow of= a<br> >> <br> >> method, with the addition of a try catch block, but the ClassWrite= r<br> >> only<br> >> has a COMPUTE_MAXS option. As a result, frames are not recomputed,= <br> >> and<br> >> there is a missing frame just before the exception handler. To fix= <br> >> this<br> >> you can either use COMPUTE_FRAMES instead of COMPUTE_MAXS, or<br> >> 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<b= r> >> block<br> >> before its labels, as follows (and as documented in MethodVisitor)= :<br> >> <br> >> private static class TryCatchWrapper extends AdviceAdapter {<br> >> <br> >> private Label start;<br> >> private Label end;<br> >> <br> >> public TryCatchWrapper(MethodVisitor mv, int access, String<br> >> <br> >> name, String descriptor) {<br> >> super(ASM9, mv, access, name, descriptor);<br> >> }<br> >> <br> >> @Override<br> >> protected void onMethodEnter() {<br> >> start =3D new Label();<br> >> end =3D new Label();<br> >> visitTryCatchBlock(start, end, end, null);<br> >> mv.visitLabel(start);<br> >> }<br> >> <br> >> @Override<br> >> public void visitMaxs(int maxStack, int maxLocals) {<br> >> visitLabel(end);<br> >> visitInsn(ATHROW);<br> >> super.visitMaxs(maxStack, maxLocals);<br> >> }<br> >> }<br> >> <br> >> Le 16/03/2022 20:59, Eirik Bj=C3=B8rsn=C3=B8s a =C3=A9crit :<br> >>> Hi,<br> >>> <br> >>> I'm observing a NASE during toByteArray/replaceAsmInstruct= ions<br> >> while<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<b= r> >> provokes<br> >>> the following conditions:<br> >>> <br> >>> 1: One of the methods in the class should incude a Label which= <br> >> returns<br> >>> false from the Label.resolve method. This is because the label= is<br> >> a<br> >>> forward reference with a relative offset larger than<br> >> Short.MAX_VALUE,<br> >>> hence ASMs hasAsmInstructions is set to true. This seems neces= sary<br> >> to<br> >>> force a call to replaceAsmInstructions which triggers frame<br> >>> processing.<br> >>> <br> >>> 2: Another method of the class is instrumented with a<br> >> trycatchblock<br> >>> which wraps the body of the method This catch block handler si= mply<br> >>> ATHROWs the Throwable. This seems necessary to create the<br> >> conditions<br> >>> 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.NegativeArraySi= zeException:<br> >> -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:221= 3)<br> >>> at org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1= 514)<br> >>> at org.objectweb.asm.ClassReader.accept(ClassReader.java:744)<= br> >>> at<br> >>> <br> >> <br> > org.objectweb.asm.ClassWriter.replaceAsmInstructions(ClassWriter.java:= 755)<br> >>> at org.objectweb.asm.ClassWriter.toByteArray(ClassWriter.java:= 718)<br> >>> at<br> >>> <br> >> <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/3eb8ca755f2a399ac8e5= def6f43439e5" rel=3D"noreferrer" target=3D"_blank">https://gist.github.com/= eirbjo/3eb8ca755f2a399ac8e5def6f43439e5</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 excepti= on.<br> >> So I<br> >>> think the produced byte code is valid and there might be a bug= in<br> >> 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"mai= lto:[email protected]" target=3D"_blank">[email protected]</a><br> >> mailing<br> >>> list.<br> >>> To unsubscribe: mailto:<a href=3D"mailto:[email protected]= rg" target=3D"_blank">[email protected]</a><br> >>> For general help: mailto:<a href=3D"mailto:[email protected]" targ= et=3D"_blank">[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><b= r> </blockquote></div> --00000000000093517005da6b7f0a-- ------------=_1647530125-25320-11 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 ------------=_1647530125-25320-11--