From 9be47cc3a9ead4208d6247fcc5da7216c231278a Mon Sep 17 00:00:00 2001
From: Andy Clement <aclement@pivotal.io>
Date: Wed, 20 Jan 2016 12:58:09 -0800
Subject: [PATCH] Fixes for compile time overweaving

In writing a testcase for 352389 I discovered overweaving
just wasn't working for a compile time series of test steps.
This was due to a guard preventing secondary calls to
addOrReplaceAspect. Without the secondary call the crosscutting
collector for the aspect had recorded no mungers from the
original aspect because it was still using an EclipseSourceType
delegate. Later when it was using the binary BcelObjectType
delegate and mungers were available, they weren't collected
because of that missing addOrReplaceAspect call.
---
 tests/bugs189/352389/A.java                   |  3 ++
 tests/bugs189/352389/B.java                   |  3 ++
 tests/bugs189/352389/C.java                   |  3 ++
 tests/bugs189/352389/Code.java                |  7 +++
 .../systemtest/ajc189/Ajc189Tests.java        |  4 ++
 .../org/aspectj/systemtest/ajc189/ajc189.xml  | 53 +++++++++++++++++++
 .../org/aspectj/weaver/bcel/BcelWeaver.java   |  6 ++-
 7 files changed, 78 insertions(+), 1 deletion(-)
 create mode 100644 tests/bugs189/352389/A.java
 create mode 100644 tests/bugs189/352389/B.java
 create mode 100644 tests/bugs189/352389/C.java
 create mode 100644 tests/bugs189/352389/Code.java

diff --git a/tests/bugs189/352389/A.java b/tests/bugs189/352389/A.java
new file mode 100644
index 000000000..d82598fa1
--- /dev/null
+++ b/tests/bugs189/352389/A.java
@@ -0,0 +1,3 @@
+public aspect A {
+before(): execution(* m(..)) { System.out.println("a"); }
+}
diff --git a/tests/bugs189/352389/B.java b/tests/bugs189/352389/B.java
new file mode 100644
index 000000000..950dfae0a
--- /dev/null
+++ b/tests/bugs189/352389/B.java
@@ -0,0 +1,3 @@
+public aspect B {
+before(): execution(* m(..)) { System.out.println("b"); }
+}
diff --git a/tests/bugs189/352389/C.java b/tests/bugs189/352389/C.java
new file mode 100644
index 000000000..f86644d3b
--- /dev/null
+++ b/tests/bugs189/352389/C.java
@@ -0,0 +1,3 @@
+public aspect C {
+before(): execution(* m(..)) { System.out.println("c"); }
+}
diff --git a/tests/bugs189/352389/Code.java b/tests/bugs189/352389/Code.java
new file mode 100644
index 000000000..c7c4e64ff
--- /dev/null
+++ b/tests/bugs189/352389/Code.java
@@ -0,0 +1,7 @@
+public class Code {
+public static void main(String []argv) {
+  new Code().m();
+}
+
+public void m() {}
+}
diff --git a/tests/src/org/aspectj/systemtest/ajc189/Ajc189Tests.java b/tests/src/org/aspectj/systemtest/ajc189/Ajc189Tests.java
index ac3ceb984..9d5206c4b 100644
--- a/tests/src/org/aspectj/systemtest/ajc189/Ajc189Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc189/Ajc189Tests.java
@@ -25,6 +25,10 @@ public class Ajc189Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
 		runTest("while npe");
 	}
 	
+	public void testOverweaving_352389() throws Exception {
+		runTest("overweaving");
+	}
+	
 	// ---
 
 	public static Test suite() {
diff --git a/tests/src/org/aspectj/systemtest/ajc189/ajc189.xml b/tests/src/org/aspectj/systemtest/ajc189/ajc189.xml
index f64438a25..d9b2b4226 100644
--- a/tests/src/org/aspectj/systemtest/ajc189/ajc189.xml
+++ b/tests/src/org/aspectj/systemtest/ajc189/ajc189.xml
@@ -6,4 +6,57 @@
 <compile files="While.java" options="-1.8"/>
 </ajc-test>
 
+<ajc-test dir="bugs189/352389" title="overweaving">
+
+	<compile files="A.java Code.java" outjar="one.jar" options="-1.8 -showWeaveInfo">
+		<message kind="weave" text="Join point 'method-execution(void Code.m())' in Type 'Code' (Code.java:6) advised by before advice from 'A' (A.java:2)"/>
+	</compile>
+	<run class="Code" classpath="$sandbox/one.jar">
+		<stdout>
+		<line text="a"/>
+		</stdout>
+	</run>
+
+	<!-- The non overweaving case would look like this -->
+	<compile files="B.java" inpath="one.jar" options="-1.8 -showWeaveInfo" outjar="two.jar">
+		<message kind="weave" text="Join point 'method-execution(void Code.m())' in Type 'Code' (Code.java:6) advised by before advice from 'B' (B.java:2)"/>
+		<message kind="weave" text="Join point 'method-execution(void Code.m())' in Type 'Code' (Code.java:6) advised by before advice from 'A' (one.jar!A.class:2(from A.java))"/>
+	</compile>
+	
+	<!-- With overweaving it looks like the advice from A isn't applying but it just isn't applying during this weave step, it
+	     remains in the bytecode applied from before. -->
+	<compile files="B.java" inpath="one.jar" options="-1.8 -Xset:pipelineCompilation=false{overWeaving=true -showWeaveInfo" outjar="two.jar">
+		<message kind="warning" text="advice defined in A has not been applied [Xlint:adviceDidNotMatch]"/>
+		<message kind="weave" text="Join point 'method-execution(void Code.m())' in Type 'Code' (Code.java:6) advised by before advice from 'B' (B.java:2)"/>
+	</compile>
+	<!-- Note: with overweaving output is b>a  without overweaving it is a>b -->
+	<run class="Code" classpath="$sandbox/two.jar">
+		<stdout>
+		<line text="b"/>
+		<line text="a"/>
+		</stdout>
+	</run>
+
+	<compile files="C.java" inpath="two.jar" options="-1.8 -Xset:overWeaving=true -showWeaveInfo" outjar="three.jar">
+		<message kind="warning" text="advice defined in A has not been applied [Xlint:adviceDidNotMatch]"/>
+		<message kind="warning" text="advice defined in B has not been applied [Xlint:adviceDidNotMatch]"/>
+		<message kind="weave" text="Join point 'method-execution(void Code.m())' in Type 'Code' (Code.java:6) advised by before advice from 'C' (C.java:2)"/>
+	</compile>
+	<!-- 
+	<compile files="C.java" inpath="two.jar" options="-1.8 -showWeaveInfo" outjar="three.jar">
+		<message kind="weave" text="Join point 'method-execution(void Code.m())' in Type 'Code' (Code.java:6) advised by before advice from 'C' (C.java:2)"/>
+		<message kind="weave" text="Join point 'method-execution(void Code.m())' in Type 'Code' (Code.java:6) advised by before advice from 'B' (two.jar!B.class:2(from B.java))"/>
+		<message kind="weave" text="Join point 'method-execution(void Code.m())' in Type 'Code' (Code.java:6) advised by before advice from 'A' (two.jar!A.class:2(from A.java))"/>
+	</compile>
+	-->
+	<!-- Note: with overweaving c>b>a  but without overweaving a>b>c -->
+	<run class="Code" classpath="$sandbox/three.jar">
+		<stdout>
+		<line text="c"/>
+		<line text="b"/>
+		<line text="a"/>
+		</stdout>
+	</run>
+</ajc-test>
+
 </suite>
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index 815becde0..5e524549e 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -503,7 +503,11 @@ public class BcelWeaver {
 			UnwovenClassFile jc = i.next();
 			String name = jc.getClassName();
 			ResolvedType type = world.resolve(name);
-			if (type.isAspect() && !world.isOverWeaving()) {
+			// No overweaving guard. If you have one then when overweaving is on the
+			// addOrReplaceAspect will not be called when the aspect delegate changes from
+			// EclipseSourceType to BcelObjectType. This will mean the mungers
+			// are not picked up.
+			if (type.isAspect()) {
 				needToReweaveWorld |= xcutSet.addOrReplaceAspect(type);
 			}
 		}
-- 
2.39.5