fix for

Bugzilla Bug 37152  
   java.lang.VerifyError: 

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

3 files changed, 185 insertions, 90 deletions

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
index a2d3aedc6..0a4a9d318 100644
--- 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 30f5b263f..04b785dc8 100644
--- 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 19fa930a5..2b46566c5 100644
--- 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