fix and testcode update for: 151673: after advice sometimes woven incorrectly into strangely formed input bytecode

10 files changed, 21 insertions, 16 deletions

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
index bfa5c23ef..f1694cbb0 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
@@ -49,6 +49,7 @@ import org.aspectj.apache.bcel.generic.MULTIANEWARRAY;
 import org.aspectj.apache.bcel.generic.NEW;
 import org.aspectj.apache.bcel.generic.ObjectType;
 import org.aspectj.apache.bcel.generic.PUSH;
+import org.aspectj.apache.bcel.generic.RETURN;
 import org.aspectj.apache.bcel.generic.ReturnInstruction;
 import org.aspectj.apache.bcel.generic.SWAP;
 import org.aspectj.apache.bcel.generic.StoreInstruction;
@@ -1792,6 +1793,12 @@ public class BcelShadow extends Shadow {
 	 * variable to hold the return value, and load the value from this var before
 	 * returning (see pr148007 for why we do this - it works around a JRockit bug,
 	 * and is also closer to what javac generates)
+	 * 
+	 * Sometimes the 'last return' isnt the right one - some rogue code can 
+	 * include the real return from the body of a subroutine that exists at the end
+	 * of the method. In this case the last return is RETURN but that may not be
+	 * correct for a method with a non-void return type... pr151673
+	 * 
 	 * @param returns list of all the return instructions in the shadow
 	 * @param returnInstructions instruction list into which the return instructions should
 	 * be generated
@@ -1799,15 +1806,26 @@ public class BcelShadow extends Shadow {
 	 */
 	private BcelVar generateReturnInstructions(List returns, InstructionList returnInstructions) {
 		BcelVar returnValueVar = null;
-    	InstructionHandle lastReturnHandle = (InstructionHandle)returns.get(returns.size() - 1);
-    	Instruction newReturnInstruction = Utility.copyInstruction(lastReturnHandle.getInstruction());
     	if (this.hasANonVoidReturnType()) {
+    		// Find the last *correct* return - this is a method with a non-void return type
+    		// so ignore RETURN
+    		Instruction  newReturnInstruction = null;
+    		int i=returns.size()-1;
+    		while (newReturnInstruction == null && i>=0) {
+    			InstructionHandle ih = (InstructionHandle)returns.get(i);
+    			if (!(ih.getInstruction() instanceof RETURN)) {
+    				newReturnInstruction = Utility.copyInstruction(ih.getInstruction());
+    			}
+    			i--;
+    		}
         	returnValueVar = genTempVar(this.getReturnType());
             returnValueVar.appendLoad(returnInstructions,getFactory());
+        	returnInstructions.append(newReturnInstruction);
     	} else {
+        	InstructionHandle lastReturnHandle = (InstructionHandle)returns.get(returns.size() - 1);
+        	Instruction newReturnInstruction = Utility.copyInstruction(lastReturnHandle.getInstruction());
     		returnInstructions.append(newReturnInstruction);
     	}
-    	returnInstructions.append(newReturnInstruction);
     	return returnValueVar;
 	}
     
diff --git a/weaver/testdata/AfterFancyHelloWorld.txt b/weaver/testdata/AfterFancyHelloWorld.txt
index bb35ec38c..a00b8e78a 100644
--- a/weaver/testdata/AfterFancyHelloWorld.txt
+++ b/weaver/testdata/AfterFancyHelloWorld.txt
@@ -12,7 +12,6 @@ public abstract class FancyHelloWorld extends java.lang.Object:
     |               ATHROW
     |           L0: INVOKESTATIC Aspect.ajc_after_constructor_execution ()V
     |               RETURN
-    |               RETURN
     constructor-execution(void FancyHelloWorld.<init>())
   end public void <init>()
 
@@ -101,7 +100,6 @@ public abstract class FancyHelloWorld extends java.lang.Object:
     |               ATHROW
     |           L7: INVOKESTATIC Aspect.ajc_after_method_execution ()V
     |               RETURN
-    |               RETURN
     method-execution(void FancyHelloWorld.main(java.lang.String[]))
   end public static void main(String[])
 
diff --git a/weaver/testdata/AfterHelloWorld.txt b/weaver/testdata/AfterHelloWorld.txt
index 2521c5f3c..982112b96 100644
--- a/weaver/testdata/AfterHelloWorld.txt
+++ b/weaver/testdata/AfterHelloWorld.txt
@@ -12,7 +12,6 @@ public class HelloWorld extends java.lang.Object:
     |               ATHROW
     |           L0: INVOKESTATIC Aspect.ajc_after_constructor_execution ()V
     |               RETURN
-    |               RETURN
     constructor-execution(void HelloWorld.<init>())
   end public void <init>()
 
@@ -54,7 +53,6 @@ public class HelloWorld extends java.lang.Object:
     |               ATHROW
     |           L2: INVOKESTATIC Aspect.ajc_after_method_execution ()V
     |               RETURN
-    |               RETURN
     method-execution(void HelloWorld.main(java.lang.String[]))
   end public static void main(String[])
 
diff --git a/weaver/testdata/AfterReturningFancyHelloWorld.txt b/weaver/testdata/AfterReturningFancyHelloWorld.txt
index 8429607db..1132efee3 100644
--- a/weaver/testdata/AfterReturningFancyHelloWorld.txt
+++ b/weaver/testdata/AfterReturningFancyHelloWorld.txt
@@ -6,7 +6,6 @@ public abstract class FancyHelloWorld extends java.lang.Object:
     |               NOP
     |               INVOKESTATIC Aspect.ajc_afterReturning_constructor_execution ()V
     |               RETURN
-    |               RETURN
     constructor-execution(void FancyHelloWorld.<init>())
   end public void <init>()
 
@@ -57,7 +56,6 @@ public abstract class FancyHelloWorld extends java.lang.Object:
     |           L2: NOP   (line 17)
     |               INVOKESTATIC Aspect.ajc_afterReturning_method_execution ()V
     |               RETURN
-    |               RETURN
     method-execution(void FancyHelloWorld.main(java.lang.String[]))
   end public static void main(String[])
 
diff --git a/weaver/testdata/AfterReturningHelloWorld.txt b/weaver/testdata/AfterReturningHelloWorld.txt
index abc748eb5..28f8980af 100644
--- a/weaver/testdata/AfterReturningHelloWorld.txt
+++ b/weaver/testdata/AfterReturningHelloWorld.txt
@@ -6,7 +6,6 @@ public class HelloWorld extends java.lang.Object:
     |               NOP
     |               INVOKESTATIC Aspect.ajc_afterReturning_constructor_execution ()V
     |               RETURN
-    |               RETURN
     constructor-execution(void HelloWorld.<init>())
   end public void <init>()
 
@@ -26,7 +25,6 @@ public class HelloWorld extends java.lang.Object:
     |               NOP   (line 11)
     |               INVOKESTATIC Aspect.ajc_afterReturning_method_execution ()V
     |               RETURN
-    |               RETURN
     method-execution(void HelloWorld.main(java.lang.String[]))
   end public static void main(String[])
 
diff --git a/weaver/testdata/ArgsAfterReturningFancyHelloWorld.txt b/weaver/testdata/ArgsAfterReturningFancyHelloWorld.txt
index 6016b6d4b..eeeb8db46 100644
--- a/weaver/testdata/ArgsAfterReturningFancyHelloWorld.txt
+++ b/weaver/testdata/ArgsAfterReturningFancyHelloWorld.txt
@@ -62,7 +62,6 @@ public abstract class FancyHelloWorld extends java.lang.Object:
     |               ALOAD 8
     |               INVOKESTATIC Aspect.ajc_afterReturning_method_execution (Ljava/lang/Object;)V
     |               RETURN
-    |               RETURN
     method-execution(void FancyHelloWorld.main(java.lang.String[]))
   end public static void main(String[])
 
diff --git a/weaver/testdata/ArgsAfterReturningHelloWorld.txt b/weaver/testdata/ArgsAfterReturningHelloWorld.txt
index d1de34129..373a66bfc 100644
--- a/weaver/testdata/ArgsAfterReturningHelloWorld.txt
+++ b/weaver/testdata/ArgsAfterReturningHelloWorld.txt
@@ -25,7 +25,6 @@ public class HelloWorld extends java.lang.Object:
     |               ALOAD_2
     |               INVOKESTATIC Aspect.ajc_afterReturning_method_execution (Ljava/lang/Object;)V
     |               RETURN
-    |               RETURN
     method-execution(void HelloWorld.main(java.lang.String[]))
   end public static void main(String[])
 
diff --git a/weaver/testdata/ArgsBeforeAfterHelloWorld.txt b/weaver/testdata/ArgsBeforeAfterHelloWorld.txt
index e7abc1f57..2da3bfdfc 100644
--- a/weaver/testdata/ArgsBeforeAfterHelloWorld.txt
+++ b/weaver/testdata/ArgsBeforeAfterHelloWorld.txt
@@ -45,7 +45,6 @@ public class HelloWorld extends java.lang.Object:
     |           L1: ALOAD_3
     |               INVOKESTATIC Aspect.ajc_afterReturning_method_execution (Ljava/lang/Object;)V
     |               RETURN
-    |               RETURN
     method-execution(void HelloWorld.main(java.lang.String[]))
   end public static void main(String[])
 
diff --git a/weaver/testdata/CflowNonStaticBeforeFancyHelloWorld.txt b/weaver/testdata/CflowNonStaticBeforeFancyHelloWorld.txt
index c6c066ae1..d52ce2b79 100644
--- a/weaver/testdata/CflowNonStaticBeforeFancyHelloWorld.txt
+++ b/weaver/testdata/CflowNonStaticBeforeFancyHelloWorld.txt
@@ -53,7 +53,6 @@ public abstract class FancyHelloWorld extends java.lang.Object:
     |           L4: GETSTATIC Aspect.ajc$perCflowStack Lorg/aspectj/runtime/internal/CFlowStack;
     |               INVOKEVIRTUAL org.aspectj.runtime.internal.CFlowStack.pop ()V
     |               RETURN
-    |               RETURN
     method-execution(void FancyHelloWorld.main(java.lang.String[]))
   end public static void main(String[])
 
diff --git a/weaver/testdata/CflowNonStaticBeforeHelloWorld.txt b/weaver/testdata/CflowNonStaticBeforeHelloWorld.txt
index 9970f974d..b5c9e7c7a 100644
--- a/weaver/testdata/CflowNonStaticBeforeHelloWorld.txt
+++ b/weaver/testdata/CflowNonStaticBeforeHelloWorld.txt
@@ -30,7 +30,6 @@ public class HelloWorld extends java.lang.Object:
     |           L1: GETSTATIC Aspect.ajc$perCflowStack Lorg/aspectj/runtime/internal/CFlowStack;
     |               INVOKEVIRTUAL org.aspectj.runtime.internal.CFlowStack.pop ()V
     |               RETURN
-    |               RETURN
     method-execution(void HelloWorld.main(java.lang.String[]))
   end public static void main(String[])