Community
Participate
Working Groups
I have a simple class: public class Swit { protected Swit() { this(3); } protected Swit(int p) { switch (p) { case 3: break; } } } And a simpl(ish) aspect: import org.aspectj.lang.JoinPoint; import org.aspectj.lang.Signature; import org.aspectj.lang.reflect.SourceLocation; public aspect FFDC { pointcut initializers( ) : staticinitialization( * ) || initialization( *.new(..) ); pointcut methodsAndConstructors( ) : execution(* *(..)) || execution(new (..) ); pointcut guarded( ) : initializers( ) || methodsAndConstructors( ); final pointcut nonStaticContext( Object o ) : this( o ); after(Object o) throwing(Throwable t) : guarded( ) && nonStaticContext( o ) { } } Compiling these two with 'ajc Swit.java FFDC.java' results in: trouble in: public class Swit extends java.lang.Object: protected void <init>(): ALOAD_0 // Swit this (line 4) ICONST_3 ISTORE_3 ASTORE_2 ALOAD_2 (line 7) INVOKESPECIAL java.lang.Object.<init> ()V initialization(void Swit.<init>()) | catch java.lang.Throwable -> E2 | | constructor-execution(void Swit.<init>(int)) | | | catch java.lang.Throwable -> E1 | | | | ILOAD_3 (line 8) | | | | TABLESWITCH | | | | 3: null | | | | default: L0 | | | | L0: GOTO L1 (line 11) | | | catch java.lang.Throwable -> E1 | | | E1: ASTORE 4 | | | INVOKESTATIC FFDC.aspectOf ()LFFDC; | | | ALOAD_2 | | | ALOAD 4 | | | INVOKEVIRTUAL FFDC.ajc$afterThrowing$FFDC$1d8 (Ljava/lang/Object;Ljava/lang/Throwable;)V | | | ALOAD 4 | | | ATHROW | | constructor-execution(void Swit.<init>(int)) | | L1: NOP | | constructor-execution(void Swit.<init>()) | | | catch java.lang.Throwable -> E0 | | | | RETURN (line 5) | | | catch java.lang.Throwable -> E0 | | | E0: ASTORE_1 | | | INVOKESTATIC FFDC.aspectOf ()LFFDC; | | | ALOAD_0 | | | ALOAD_1 | | | INVOKEVIRTUAL FFDC.ajc$afterThrowing$FFDC$1d8 (Ljava/lang/Object;Ljava/lang/Throwable;)V | | | ALOAD_1 | | | ATHROW | | constructor-execution(void Swit.<init>()) | catch java.lang.Throwable -> E2 | E2: ASTORE 6 | INVOKESTATIC FFDC.aspectOf ()LFFDC; | ALOAD_0 | ALOAD 6 | INVOKEVIRTUAL FFDC.ajc$afterThrowing$FFDC$1d8 (Ljava/lang/Object;Ljava/lang/Throwable;)V | ALOAD 6 | ATHROW initialization(void Swit.<init>()) end protected void <init>() protected void <init>(int): ALOAD_0 // Swit this (line 7) INVOKESPECIAL java.lang.Object.<init> ()V initialization(void Swit.<init>(int)) | catch java.lang.Throwable -> E1 | | constructor-execution(void Swit.<init>(int)) | | | catch java.lang.Throwable -> E0 | | | | ILOAD_1 // int arg0 (line 8) | | | | TABLESWITCH | | | | 3: null | | | | default: L0 | | | | L0: RETURN (line 11) | | | catch java.lang.Throwable -> E0 | | | E0: ASTORE_2 | | | INVOKESTATIC FFDC.aspectOf ()LFFDC; | | | ALOAD_0 | | | ALOAD_2 | | | INVOKEVIRTUAL FFDC.ajc$afterThrowing$FFDC$1d8 (Ljava/lang/Object;Ljava/lang/Throwable;)V | | | ALOAD_2 | | | ATHROW | | constructor-execution(void Swit.<init>(int)) | catch java.lang.Throwable -> E1 | E1: ASTORE_3 | INVOKESTATIC FFDC.aspectOf ()LFFDC; | ALOAD_0 | ALOAD_3 | INVOKEVIRTUAL FFDC.ajc$afterThrowing$FFDC$1d8 (Ljava/lang/Object;Ljava/lang/Throwable;)V | ALOAD_3 | ATHROW initialization(void Swit.<init>(int)) end protected void <init>(int) end public class Swit ABORT Exception thrown from AspectJ 1.1.0 This might be logged as a bug already -- find current bugs at http://bugs.eclipse.org/bugs/buglist.cgi?product=AspectJ&component=Compiler Bugs for exceptions thrown have titles File:line from the top stack, e.g., "SomeFile.java:243" If you don't find the exception below in a bug, please add a new bug at http://bugs.eclipse.org/bugs/enter_bug.cgi?product=AspectJ To make the bug a priority, please include a test program that can reproduce this exception. null java.lang.NullPointerException at org.aspectj.weaver.bcel.LazyMethodGen.remap(LazyMethodGen.java:892) at org.aspectj.weaver.bcel.LazyMethodGen.packBody (LazyMethodGen.java:800) at org.aspectj.weaver.bcel.LazyMethodGen.pack(LazyMethodGen.java:706) at org.aspectj.weaver.bcel.LazyMethodGen.getMethod (LazyMethodGen.java:284) at org.aspectj.weaver.bcel.LazyClassGen.writeBack (LazyClassGen.java:164) at org.aspectj.weaver.bcel.LazyClassGen.getJavaClass (LazyClassGen.java:169) at org.aspectj.weaver.bcel.BcelWeaver.dump(BcelWeaver.java:417) at org.aspectj.weaver.bcel.BcelWeaver.weave(BcelWeaver.java:364) at org.aspectj.weaver.bcel.BcelWeaver.weave(BcelWeaver.java:335) at org.aspectj.weaver.bcel.BcelWeaver.weave(BcelWeaver.java:272) at org.aspectj.ajdt.internal.core.builder.AjBuildManager.weaveAndGenerateClassFile s(AjBuildManager.java:256) at org.aspectj.ajdt.internal.core.builder.AjBuildManager.doBuild (AjBuildManager.java:164) at org.aspectj.ajdt.internal.core.builder.AjBuildManager.batchBuild (AjBuildManager.java:78) at org.aspectj.ajdt.ajc.AjdtCommand.doCommand(AjdtCommand.java:106) at org.aspectj.ajdt.ajc.AjdtCommand.runCommand(AjdtCommand.java:60) at org.aspectj.tools.ajc.Main.run(Main.java:217) at org.aspectj.tools.ajc.Main.runMain(Main.java:155) at org.aspectj.tools.ajc.Main.main(Main.java:72) 1 fail|abort
I should add. If I stop the first constructor from calling the second, the problem goes away.
This is an easy to reproduce bug, and I'm raising it to a P2/critical bug because it's fairly serious. My quick investigation suggests that the bug is caused by the Select instruction have two targets that are both the same InstructionHandle, and that is causing problems to the code movement in BcelClassWeaver.genInlineInstructions. I'm cc'ing Erik since this is his code and he has the most experience with bcel's instruction target code. A test is in bugs/NewSwitch.java and ajcTestsFailing.xml. As a work-around/improvement, I believe that you shouldn't include initialization join points in your FFDC code in 1.1. In 1.1, all constructor- execution join points include any non-static initializer code since there's no way to separate that code from the actual constructor body in a bytecode form. initialization join points are very expensive to implement because they require inlining of constructor bodies. Since you don't appear to need them here, you'd be better off without them.
After some investigation ... this problem is due to the fact that cloning Select instructions is not working as expected. Because Java clone() is being used, the InstructionHandle[] array field 'targets' within the Select instruction is not copied, only the reference is copied. This means the code in genInlineInstructions() which inlines the constructor code from one constructor to the other breaks because when it patches up the 'targets' for any Select instructions that get inlined - when it modifies any of the 'targets' entries, it is breaking the original version of the Select statement. The remap() function was blowing up when processing the select statement as it was attempting to lookup an instructionhandle that only existed in the method where the same statement had just been inlined. Am I making sense ? Anyway... its a BCEL bug to do with Select() not implementing clone/copy quite right. The patch in aspectj to get around it is to programmatically build a copy of the select statement (Using the SWITCH class) and inline the copy. I've run all the tests (I believe) and it works, and the NewSwitch test now works too. Here is the patch for BcelClassWeaver: --------------------8<------------------------ Index: BcelClassWeaver.java =================================================================== RCS file: /home/technology/org.aspectj/modules/weaver/src/org/aspectj/weaver/bcel/Bc elClassWeaver.java,v retrieving revision 1.13 diff -u -r1.13 BcelClassWeaver.java --- BcelClassWeaver.java 2 May 2003 06:28:16 -0000 1.13 +++ BcelClassWeaver.java 15 Jul 2003 13:56:06 -0000 @@ -44,6 +44,7 @@ import org.apache.bcel.generic.PUTSTATIC; import org.apache.bcel.generic.RET; import org.apache.bcel.generic.ReturnInstruction; +import org.apache.bcel.generic.SWITCH; import org.apache.bcel.generic.Select; import org.apache.bcel.generic.Type; import org.aspectj.bridge.IMessage; @@ -462,7 +463,28 @@ dest = ret.append (fact.createBranchInstruction(Constants.GOTO, end)); } } else if (fresh instanceof BranchInstruction) { - dest = ret.append((BranchInstruction) fresh); + if (fresh instanceof Select) { + // Bugzilla #39479 + // Need to manually copy Select instructions - if we rely on the the 'fresh' object + // created by copy() above, the InstructionHandle array 'targets' inside the Select + // object will not have been deep copied, so modifying targets in fresh will modify + // the original Select - not what we want ! (It is a bug in BCEL to do with cloning + // Select objects). + Select freshSelect = (Select)fresh; + + // Create a new targets array that looks just like the existing one + InstructionHandle[] targets = new InstructionHandle[freshSelect.getTargets().length]; + for (int i = 0; i < targets.length; i++) { + targets[i] = freshSelect.getTargets() [i]; + } + + // Create a new select statement with the new targets array + SWITCH switchStatement = new SWITCH (freshSelect.getMatchs(),targets,freshSelect.getTarget()); + Select sel = (Select) switchStatement.getInstruction(); + dest = ret.append((BranchInstruction)sel); + } else { + dest = ret.append((BranchInstruction) fresh); + } } else if ( fresh instanceof LocalVariableInstruction || fresh instanceof RET) { IndexedInstruction indexed = (IndexedInstruction) fresh; --------------------8<------------------------ Is that fix ok?
Created attachment 5462 [details] Patch for BCELClassWeaver Sorry about embedding the patch in the previous comment - I thought it would work. it doesnt, you cant cut/paste it from the bug report into Eclipse. Hopefully attaching the patch like this will be better.
*** Bug 40109 has been marked as a duplicate of this bug. ***
Fix implemented based on excellent patch contributed by Andy Clement. One improvement was made to generalize the patch with a single method org.aspectj.weaver.bcel.Utility.copyInstruction that works-around the bug in Select.copy(). Changed all calls to Instruction.copy() to use this new method, would be nice to add the rule: * declare error: * call(* Instruction.copy()) && within(org.aspectj.weaver) * && !withincode(* Utility.copyInstruction(Instruction)): * "use Utility.copyInstruction to work-around bug in Select.copy ()";
updated target milestone field to 1.1.1
