fix for

Bugzilla Bug 37152
   java.lang.VerifyError:

This fix needs a little clean-up and testing before closing out

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
index a2d3aedc65736a1c363cad36c5a5d5a633c427e0..0a4a9d3185ba5046f65477c88b1b1ae23a66598a 100644 (file)
--- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
@@ -400,7 +400,7 @@ class BcelClassWeaver implements IClassWeaver {
                        genArgumentStores(donor, recipient, frameEnv, fact);
 
                InstructionList inlineInstructions = 
-                       genInlineInstructions(donor, recipient, frameEnv, fact);
+                       genInlineInstructions(donor, recipient, frameEnv, fact, false);
 
                inlineInstructions.insert(argumentStores);
                
@@ -418,11 +418,12 @@ class BcelClassWeaver implements IClassWeaver {
         *                      initially populated with argument locations.
         * @param fact an instruction factory for recipient
         */
-       private static InstructionList genInlineInstructions(
+       static InstructionList genInlineInstructions(
                LazyMethodGen donor,
                LazyMethodGen recipient,
                IntMap frameEnv,
-               InstructionFactory fact) 
+               InstructionFactory fact,
+               boolean keepReturns) 
        {
                InstructionList footer = new InstructionList();
                InstructionHandle end = footer.append(fact.NOP);
@@ -455,7 +456,11 @@ class BcelClassWeaver implements IClassWeaver {
                        if (src.getInstruction() == Range.RANGEINSTRUCTION) {
                                dest = ret.append(Range.RANGEINSTRUCTION);
                        } else if (fresh instanceof ReturnInstruction) {
-                               dest = ret.append(fact.createBranchInstruction(Constants.GOTO, end));
+                               if (keepReturns) {
+                                       dest = ret.append(fresh);
+                               } else {
+                                       dest = ret.append(fact.createBranchInstruction(Constants.GOTO, end));
+                               }
                        } else if (fresh instanceof BranchInstruction) {
                                dest = ret.append((BranchInstruction) fresh);
                        } else if (
@@ -560,7 +565,7 @@ class BcelClassWeaver implements IClassWeaver {
                    }
                }                       
                }
-               ret.append(footer);
+               if (!keepReturns) ret.append(footer);
                return ret;
        }
 

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
index 30f5b263f38f4221bc3832341ad10b35627800aa..04b785dc81df44cc7fc5c1dcd1210e07486d91d9 100644 (file)
--- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
@@ -1201,11 +1201,7 @@ public class BcelShadow extends Shadow {
         BcelObjectType ot = BcelWorld.getBcelObjectType(declaringType); 
         
                LazyMethodGen adviceMethod = ot.getLazyClassGen().getLazyMethodGen(mungerSig);
-               //TODO handle the second part of this if by creating a method for the around
-               //     advice rather than going to a full-on closure
-               if (!adviceMethod.getCanInline() ||
-                   isFallsThrough() && adviceMethod.hasExceptionHandlers() // fix for Bug 29665
-                       )
+               if (!adviceMethod.getCanInline())
                {
                        weaveAroundClosure(munger, hasDynamicTest);
                        return;
@@ -1216,7 +1212,6 @@ public class BcelShadow extends Shadow {
                //??? should consider optimizations to recognize simple cases that don't require body extraction
                enclosingMethod.setCanInline(false);
                
-               
                // start by exposing various useful things into the frame
                final InstructionFactory fact = getFactory();
                
@@ -1226,6 +1221,88 @@ public class BcelShadow extends Shadow {
                        NameMangler.aroundCallbackMethodName(
                                getSignature(),
                                getEnclosingClass()));
+                               
+                               
+        // now extract the advice into its own method
+        String adviceMethodName =
+                       NameMangler.aroundCallbackMethodName(
+                                                       getSignature(),
+                                                       getEnclosingClass()) + "$advice";
+        
+               List paramTypeList = new ArrayList();
+               List argVarList = new ArrayList();
+               List proceedVarList = new ArrayList();
+               int extraParamOffset = 0;
+               
+               //TODO paramTypeList not needed any more
+               
+               // start w/ stuff
+               if (thisVar != null) {
+                       paramTypeList.add(thisVar.getType());
+                       argVarList.add(thisVar);
+                       proceedVarList.add(new BcelVar(thisVar.getType(), extraParamOffset));
+                       extraParamOffset += thisVar.getType().getSize();
+               }
+               
+               if (targetVar != null && targetVar != thisVar) {
+                       paramTypeList.add(targetVar.getType());
+                       argVarList.add(targetVar);
+                       proceedVarList.add(new BcelVar(targetVar.getType(), extraParamOffset));
+                       extraParamOffset += targetVar.getType().getSize();
+               }
+               for (int i = 0, len = getArgCount(); i < len; i++) {
+                       paramTypeList.add(argVars[i].getType());
+                       argVarList.add(argVars[i]);
+                       proceedVarList.add(new BcelVar(argVars[i].getType(), extraParamOffset));
+                       extraParamOffset += argVars[i].getType().getSize();
+               }
+               if (thisJoinPointVar != null) {
+                       paramTypeList.add(thisJoinPointVar.getType());
+                       argVarList.add(thisJoinPointVar);
+                       proceedVarList.add(new BcelVar(thisJoinPointVar.getType(), extraParamOffset));
+                       extraParamOffset += thisJoinPointVar.getType().getSize();
+               }
+        
+        Type[] adviceParameterTypes = adviceMethod.getArgumentTypes();
+        Type[] extractedMethodParameterTypes = extractedMethod.getArgumentTypes();
+               Type[] parameterTypes = new Type[extractedMethodParameterTypes.length + adviceParameterTypes.length + 1];
+               int parameterIndex = 0;
+               System.arraycopy(extractedMethodParameterTypes, 0, parameterTypes, parameterIndex, extractedMethodParameterTypes.length);
+               parameterIndex += extractedMethodParameterTypes.length;
+
+//        for (Iterator i = paramTypeList.iterator(); i.hasNext(); ) {
+//             ResolvedTypeX t = (ResolvedTypeX)i.next();
+//             parameterTypes[parameterIndex++] = BcelWorld.makeBcelType(t);
+//        }
+        parameterTypes[parameterIndex++] =
+               BcelWorld.makeBcelType(adviceMethod.getEnclosingClass().getType());
+               System.arraycopy(adviceParameterTypes, 0, parameterTypes, parameterIndex, adviceParameterTypes.length);
+
+        LazyMethodGen localAdviceMethod =
+                                       new LazyMethodGen(
+                                               Modifier.FINAL | Modifier.STATIC, 
+                                               adviceMethod.getReturnType(), 
+                                               adviceMethodName,
+                                               parameterTypes,
+                                               new String[0],
+                                               getEnclosingClass());
+    
+               getEnclosingClass().addMethodGen(localAdviceMethod);
+               
+               int nVars = adviceMethod.getMaxLocals() + extraParamOffset;
+               IntMap varMap = IntMap.idMap(nVars);
+               for (int i=extraParamOffset; i < nVars; i++) {
+                       varMap.put(i-extraParamOffset, i);
+               }
+               
+               localAdviceMethod.getBody().insert(
+                       BcelClassWeaver.genInlineInstructions(adviceMethod, 
+                                       localAdviceMethod, varMap, fact, true));
+                                       
+               localAdviceMethod.setMaxLocals(nVars);
+                                       
+               //System.err.println(localAdviceMethod);
+               
     
        // the shadow is now empty.  First, create a correct call
        // to the around advice.  This includes both the call (which may involve 
@@ -1236,11 +1313,15 @@ public class BcelShadow extends Shadow {
        InstructionList advice = new InstructionList();
         InstructionHandle adviceMethodInvocation;
         {
+                       for (Iterator i = argVarList.iterator(); i.hasNext(); ) {
+                               BcelVar var = (BcelVar)i.next();
+                               var.appendLoad(advice, fact);
+                       }               
                // ??? we don't actually need to push NULL for the closure if we take care
                    advice.append(munger.getAdviceArgSetup(this, null, new InstructionList(fact.ACONST_NULL)));
                    adviceMethodInvocation =
                        advice.append(
-                               Utility.createInvoke(fact, getWorld(), munger.getSignature()));
+                               Utility.createInvoke(fact, localAdviceMethod)); //(fact, getWorld(), munger.getSignature()));
                        advice.append(
                        Utility.createConversion(
                            getFactory(), 
@@ -1273,7 +1354,7 @@ public class BcelShadow extends Shadow {
         // now the range contains everything we need.  We now inline the advice method.
 
                                
-        BcelClassWeaver.inlineMethod(adviceMethod, enclosingMethod, adviceMethodInvocation);
+        //BcelClassWeaver.inlineMethod(adviceMethod, enclosingMethod, adviceMethodInvocation);
 
         // now search through the advice, looking for a call to PROCEED.  
         // Then we replace the call to proceed with some argument setup, and a 
@@ -1281,9 +1362,9 @@ public class BcelShadow extends Shadow {
         String proceedName = 
                NameMangler.proceedMethodName(munger.getSignature().getName());
 
-        InstructionHandle curr = getRange().getStart();
-        InstructionHandle end = getRange().getEnd();
-        ConstantPoolGen cpg = extractedMethod.getEnclosingClass().getConstantPoolGen();
+        InstructionHandle curr = localAdviceMethod.getBody().getStart();
+        InstructionHandle end = localAdviceMethod.getBody().getEnd();
+        ConstantPoolGen cpg = localAdviceMethod.getEnclosingClass().getConstantPoolGen();
         while (curr != end) {
                        InstructionHandle next = curr.getNext();
                        Instruction inst = curr.getInstruction();
@@ -1291,8 +1372,8 @@ public class BcelShadow extends Shadow {
                                        && proceedName.equals(((INVOKESTATIC) inst).getMethodName(cpg))) {
 
                                        
-                               enclosingMethod.getBody().append(curr, getRedoneProceedCall(fact, extractedMethod, munger));
-                               Utility.deleteInstruction(curr, enclosingMethod);
+                               localAdviceMethod.getBody().append(curr, getRedoneProceedCall(fact, extractedMethod, munger, localAdviceMethod, proceedVarList));
+                               Utility.deleteInstruction(curr, localAdviceMethod);
                        }
                        curr = next;
         }
@@ -1302,28 +1383,30 @@ public class BcelShadow extends Shadow {
        private InstructionList getRedoneProceedCall(
                InstructionFactory fact,
                LazyMethodGen callbackMethod,
-               BcelAdvice munger) 
+               BcelAdvice munger,
+               LazyMethodGen localAdviceMethod,
+               List argVarList) 
        {
                InstructionList ret = new InstructionList();
                // we have on stack all the arguments for the ADVICE call.
                // we have in frame somewhere all the arguments for the non-advice call.
                
-               List argVarList = new ArrayList();
-               
-               // start w/ stuff
-               if (thisVar != null) {
-                       argVarList.add(thisVar);
-               }
-               
-        if (targetVar != null && targetVar != thisVar) {
-            argVarList.add(targetVar);
-        }
-               for (int i = 0, len = getArgCount(); i < len; i++) {
-                       argVarList.add(argVars[i]);
-               }
-               if (thisJoinPointVar != null) {
-                       argVarList.add(thisJoinPointVar);
-               }
+//             List argVarList = new ArrayList();
+//             
+//             // start w/ stuff
+//             if (thisVar != null) {
+//                     argVarList.add(thisVar);
+//             }
+//             
+//        if (targetVar != null && targetVar != thisVar) {
+//            argVarList.add(targetVar);
+//        }
+//             for (int i = 0, len = getArgCount(); i < len; i++) {
+//                     argVarList.add(argVars[i]);
+//             }
+//             if (thisJoinPointVar != null) {
+//                     argVarList.add(thisJoinPointVar);
+//             }
                
                BcelVar[] adviceVars = munger.getExposedStateAsBcelVars();              
                //??? this is too easy
@@ -1349,7 +1432,7 @@ public class BcelShadow extends Shadow {
                
                //System.out.println("stateTypes: " + Arrays.asList(stateTypes));
                BcelVar[] proceedVars = 
-                       Utility.pushAndReturnArrayOfVars(proceedParamTypes, ret, fact, enclosingMethod);
+                       Utility.pushAndReturnArrayOfVars(proceedParamTypes, ret, fact, localAdviceMethod);
                
 
                Type[] stateTypes = callbackMethod.getArgumentTypes();

diff --git a/weaver/testdata/TraceJarHello.txt b/weaver/testdata/TraceJarHello.txt
index 19fa930a59337f6f92d2394cac49088207d70a3c..2b46566c54d5eae853650e8d552514915e3e845c 100644 (file)
--- a/weaver/testdata/TraceJarHello.txt
+++ b/weaver/testdata/TraceJarHello.txt
@@ -65,47 +65,19 @@ public class DynamicHelloWorld extends java.lang.Object implements java.io.Seria
     |               INVOKESTATIC MyTrace.aspectOf ()LMyTrace;
     |               ALOAD_0
     |               INVOKEVIRTUAL Trace.ajc$before$Trace$51 (Ljava/lang/Object;)V
-    |               INVOKESTATIC MyTrace.aspectOf ()LMyTrace;
-    |               ACONST_NULL
-    |               ASTORE 13
-    |               ASTORE 12
-    |               LDC "Hi"   (line 9)
-    |               ASTORE 14
-    |               GETSTATIC java.lang.System.out Ljava/io/PrintStream;   (line 10)
-    |               NEW java.lang.StringBuffer
-    |               DUP
-    |               LDC "start around: "
-    |               INVOKESPECIAL java.lang.StringBuffer.<init> (Ljava/lang/String;)V
-    |               ALOAD 14
-    |               INVOKEVIRTUAL java.lang.StringBuffer.append (Ljava/lang/Object;)Ljava/lang/StringBuffer;
-    |               INVOKEVIRTUAL java.lang.StringBuffer.toString ()Ljava/lang/String;
-    |               INVOKEVIRTUAL java.io.PrintStream.println (Ljava/lang/String;)V
-    |               ALOAD 13   (line 11)
-    |               ASTORE 18
     |               ALOAD_0
     |               ALOAD_3
     |               ALOAD 4
-    |               INVOKESTATIC DynamicHelloWorld.doit_aroundBody1 (LDynamicHelloWorld;Ljava/lang/String;Ljava/util/List;)Ljava/lang/String;
-    |               ASTORE 16
-    |               GETSTATIC java.lang.System.out Ljava/io/PrintStream;   (line 12)
-    |               NEW java.lang.StringBuffer
-    |               DUP
-    |               LDC "exiting around with: "
-    |               INVOKESPECIAL java.lang.StringBuffer.<init> (Ljava/lang/String;)V
-    |               ALOAD 16
-    |               INVOKEVIRTUAL java.lang.StringBuffer.append (Ljava/lang/Object;)Ljava/lang/StringBuffer;
-    |               INVOKEVIRTUAL java.lang.StringBuffer.toString ()Ljava/lang/String;
-    |               INVOKEVIRTUAL java.io.PrintStream.println (Ljava/lang/String;)V
-    |               ALOAD 16   (line 13)
-    |               GOTO L0
-    |           L0: NOP
+    |               INVOKESTATIC MyTrace.aspectOf ()LMyTrace;
+    |               ACONST_NULL
+    |               INVOKESTATIC DynamicHelloWorld.doit_aroundBody3$advice (LDynamicHelloWorld;Ljava/lang/String;Ljava/util/List;LTrace;Lorg/aspectj/runtime/internal/AroundClosure;)Ljava/lang/Object;
     |               CHECKCAST java.lang.String
-    |               GOTO L1
-    |           L1: DUP
-    |               ASTORE 19
+    |               GOTO L0
+    |           L0: DUP
+    |               ASTORE 5
     |               INVOKESTATIC MyTrace.aspectOf ()LMyTrace;
     |               ALOAD_0
-    |               ALOAD 19
+    |               ALOAD 5
     |               INVOKEVIRTUAL MyTrace.ajc$afterReturning$MyTrace$6e (Ljava/lang/Object;Ljava/lang/Object;)V
     |               ARETURN
     method-execution(java.lang.String DynamicHelloWorld.doit(java.lang.String, java.util.List))
@@ -125,45 +97,80 @@ public class DynamicHelloWorld extends java.lang.Object implements java.io.Seria
                     ARETURN
   end static final String doit_aroundBody0(DynamicHelloWorld, String, java.util.List)
 
-  static final String doit_aroundBody1(DynamicHelloWorld, String, java.util.List):
-                    INVOKESTATIC MyTrace.aspectOf ()LMyTrace;
-                    ALOAD_0
-                    ACONST_NULL
-                    ASTORE_3
-                    ASTORE 5
-                    ASTORE 7
+  static final Object doit_aroundBody1$advice(DynamicHelloWorld, String, java.util.List, Trace, Object, org.aspectj.runtime.internal.AroundClosure):
                     GETSTATIC java.lang.System.out Ljava/io/PrintStream;   (line 17)
                     NEW java.lang.StringBuffer
                     DUP
                     LDC "start around(2): "
                     INVOKESPECIAL java.lang.StringBuffer.<init> (Ljava/lang/String;)V
-                    ALOAD 5
+                    ALOAD 4
                     INVOKEVIRTUAL java.lang.StringBuffer.append (Ljava/lang/Object;)Ljava/lang/StringBuffer;
                     INVOKEVIRTUAL java.lang.StringBuffer.toString ()Ljava/lang/String;
                     INVOKEVIRTUAL java.io.PrintStream.println (Ljava/lang/String;)V
-                    ALOAD 5   (line 18)
-                    ALOAD_3
-                    ASTORE 9
-                    ASTORE 11
-                    ALOAD 11
+                    ALOAD 4   (line 18)
+                    ALOAD 5
+                    ASTORE 7
+                    ASTORE 8
+                    ALOAD 8
                     CHECKCAST DynamicHelloWorld
                     ALOAD_1
                     ALOAD_2
                     INVOKESTATIC DynamicHelloWorld.doit_aroundBody0 (LDynamicHelloWorld;Ljava/lang/String;Ljava/util/List;)Ljava/lang/String;
-                    ASTORE 13
+                    ASTORE 6
                     GETSTATIC java.lang.System.out Ljava/io/PrintStream;   (line 19)
                     NEW java.lang.StringBuffer
                     DUP
                     LDC "exiting around with(2): "
                     INVOKESPECIAL java.lang.StringBuffer.<init> (Ljava/lang/String;)V
-                    ALOAD 13
+                    ALOAD 6
                     INVOKEVIRTUAL java.lang.StringBuffer.append (Ljava/lang/Object;)Ljava/lang/StringBuffer;
                     INVOKEVIRTUAL java.lang.StringBuffer.toString ()Ljava/lang/String;
                     INVOKEVIRTUAL java.io.PrintStream.println (Ljava/lang/String;)V
-                    ALOAD 13   (line 20)
-                    GOTO L0
-                L0: NOP
+                    ALOAD 6   (line 20)
+                    ARETURN
+  end static final Object doit_aroundBody1$advice(DynamicHelloWorld, String, java.util.List, Trace, Object, org.aspectj.runtime.internal.AroundClosure)
+
+  static final String doit_aroundBody2(DynamicHelloWorld, String, java.util.List):
+                    ALOAD_0
+                    ALOAD_1
+                    ALOAD_2
+                    INVOKESTATIC MyTrace.aspectOf ()LMyTrace;
+                    ALOAD_0
+                    ACONST_NULL
+                    INVOKESTATIC DynamicHelloWorld.doit_aroundBody1$advice (LDynamicHelloWorld;Ljava/lang/String;Ljava/util/List;LTrace;Ljava/lang/Object;Lorg/aspectj/runtime/internal/AroundClosure;)Ljava/lang/Object;
                     CHECKCAST java.lang.String
                     ARETURN
-  end static final String doit_aroundBody1(DynamicHelloWorld, String, java.util.List)
+  end static final String doit_aroundBody2(DynamicHelloWorld, String, java.util.List)
+
+  static final Object doit_aroundBody3$advice(DynamicHelloWorld, String, java.util.List, Trace, org.aspectj.runtime.internal.AroundClosure):
+                    LDC "Hi"   (line 9)
+                    ASTORE 5
+                    GETSTATIC java.lang.System.out Ljava/io/PrintStream;   (line 10)
+                    NEW java.lang.StringBuffer
+                    DUP
+                    LDC "start around: "
+                    INVOKESPECIAL java.lang.StringBuffer.<init> (Ljava/lang/String;)V
+                    ALOAD 5
+                    INVOKEVIRTUAL java.lang.StringBuffer.append (Ljava/lang/Object;)Ljava/lang/StringBuffer;
+                    INVOKEVIRTUAL java.lang.StringBuffer.toString ()Ljava/lang/String;
+                    INVOKEVIRTUAL java.io.PrintStream.println (Ljava/lang/String;)V
+                    ALOAD 4   (line 11)
+                    ASTORE 7
+                    ALOAD_0
+                    ALOAD_1
+                    ALOAD_2
+                    INVOKESTATIC DynamicHelloWorld.doit_aroundBody2 (LDynamicHelloWorld;Ljava/lang/String;Ljava/util/List;)Ljava/lang/String;
+                    ASTORE 6
+                    GETSTATIC java.lang.System.out Ljava/io/PrintStream;   (line 12)
+                    NEW java.lang.StringBuffer
+                    DUP
+                    LDC "exiting around with: "
+                    INVOKESPECIAL java.lang.StringBuffer.<init> (Ljava/lang/String;)V
+                    ALOAD 6
+                    INVOKEVIRTUAL java.lang.StringBuffer.append (Ljava/lang/Object;)Ljava/lang/StringBuffer;
+                    INVOKEVIRTUAL java.lang.StringBuffer.toString ()Ljava/lang/String;
+                    INVOKEVIRTUAL java.io.PrintStream.println (Ljava/lang/String;)V
+                    ALOAD 6   (line 13)
+                    ARETURN
+  end static final Object doit_aroundBody3$advice(DynamicHelloWorld, String, java.util.List, Trace, org.aspectj.runtime.internal.AroundClosure)
 end public class DynamicHelloWorld