389678: Better support for overweaving

More testcases for overweaving and better handling of WeaverStateInfo
to avoid the dreaded problems deserialized the 'special key' used
to store diffs.

With these changes once a class is woven via overweaving we switch
the diff we store in the weaverstateinfo to 0 byte array (indicating
overweaving happened for later weavers that see it). We also stop
writing the special 'key' into the attribute and avoid looking-for
and attempting to replace it at the end of weaving.

3 files changed, 38 insertions, 22 deletions

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
index b4eaa3dd4..83919ed50 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
@@ -538,8 +538,12 @@ class BcelClassWeaver implements IClassWeaver {
 		if (inReweavableMode) {
 			WeaverStateInfo wsi = clazz.getOrCreateWeaverStateInfo(true);
 			wsi.addAspectsAffectingType(aspectsAffectingType);
-			wsi.setUnwovenClassFileData(ty.getJavaClass().getBytes());
-			wsi.setReweavable(true);
+			if (!world.isOverWeaving()) {
+				wsi.setUnwovenClassFileData(ty.getJavaClass().getBytes());
+				wsi.setReweavable(true);
+			} else {
+				wsi.markOverweavingInUse();
+			}
 		} else {
 			clazz.getOrCreateWeaverStateInfo(false).setReweavable(false);
 		}
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index 4306602e7..f83a79379 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -1364,15 +1364,12 @@ public class BcelWeaver {
 				if (!alreadyConfirmedReweavableState.contains(requiredTypeSignature)) {
 					ResolvedType rtx = world.resolve(UnresolvedType.forSignature(requiredTypeSignature), true);
 					boolean exists = !rtx.isMissing();
-					if (!exists) {
-						world.getLint().missingAspectForReweaving.signal(new String[] { rtx.getName(), className },
+					if (!world.isOverWeaving()) {
+						if (!exists) {
+							world.getLint().missingAspectForReweaving.signal(new String[] { rtx.getName(), className },
 								classType.getSourceLocation(), null);
-						// world.showMessage(IMessage.ERROR, WeaverMessages.format(WeaverMessages.MISSING_REWEAVABLE_TYPE,
-						// requiredTypeName, className), classType.getSourceLocation(), null);
-					} else {
-						if (world.isOverWeaving()) {
-							// System.out.println(">> Removing " + requiredTypeName + " from weaving process: "
-							// + xcutSet.deleteAspect(rtx));
+							// world.showMessage(IMessage.ERROR, WeaverMessages.format(WeaverMessages.MISSING_REWEAVABLE_TYPE,
+							// requiredTypeName, className), classType.getSourceLocation(), null);
 						} else {
 							// weaved in aspect that are not declared in aop.xml
 							// trigger an error for now
@@ -1386,8 +1383,8 @@ public class BcelWeaver {
 								world.showMessage(IMessage.INFO, WeaverMessages.format(WeaverMessages.VERIFIED_REWEAVABLE_TYPE,
 										rtx.getName(), rtx.getSourceLocation().getSourceFile()), null, null);
 							}
+							alreadyConfirmedReweavableState.add(requiredTypeSignature);
 						}
-						alreadyConfirmedReweavableState.add(requiredTypeSignature);
 					}
 				}
 			}
@@ -1396,11 +1393,21 @@ public class BcelWeaver {
 			// ().getFileName(), wsi.getUnwovenClassFileData()));
 			// new: reweavable default with clever diff
 			if (!world.isOverWeaving()) {
-				byte[] bytes = wsi.getUnwovenClassFileData(classType.getJavaClass().getBytes());
-				WeaverVersionInfo wvi = classType.getWeaverVersionAttribute();
-				JavaClass newJavaClass = Utility.makeJavaClass(classType.getJavaClass().getFileName(), bytes);
-				classType.setJavaClass(newJavaClass, true);
-				classType.getResolvedTypeX().ensureConsistent();
+				byte[] ucfd = wsi.getUnwovenClassFileData();
+				if (ucfd.length == 0) {
+					// Size 0 indicates the class was previously overwoven, so you need to be overweaving now!
+					world.getMessageHandler().handleMessage(
+							MessageUtil.error(
+							WeaverMessages.format(WeaverMessages.MUST_KEEP_OVERWEAVING_ONCE_START,
+									className)));
+//									onType.getName(), annoX.getTypeName(), annoX.getValidTargets()),
+//							decA.getSourceLocation()));					
+				} else {
+					byte[] bytes = wsi.getUnwovenClassFileData(classType.getJavaClass().getBytes());
+					JavaClass newJavaClass = Utility.makeJavaClass(classType.getJavaClass().getFileName(), bytes);
+					classType.setJavaClass(newJavaClass, true);
+					classType.getResolvedTypeX().ensureConsistent();
+				}
 			}
 			// } else {
 			// classType.resetState();
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
index 95ef524b5..83189e1d4 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
@@ -533,9 +533,14 @@ public final class LazyClassGen {
 			myGen.addAttribute(Utility.bcelAttribute(new AjAttribute.WeaverVersionInfo(), getConstantPool()));
 		}
 
-		// 352389: don't add another one (there will already be one there and this new one won't deserialize correctly)
-		if (!world.isOverWeaving() || !myGen.hasAttribute(WeaverState.AttributeName)) {
-			if (myType != null && myType.getWeaverState() != null) {
+		// see 389678: TODO more finessing possible here?
+		if (world.isOverWeaving()) {
+			if (myGen.hasAttribute(WeaverState.AttributeName) && myType!=null && myType.getWeaverState() != null) {
+				myGen.removeAttribute(myGen.getAttribute(WeaverState.AttributeName));
+				myGen.addAttribute(Utility.bcelAttribute(new AjAttribute.WeaverState(myType.getWeaverState()), getConstantPool()));
+			}
+		} else {
+			if (!myGen.hasAttribute(WeaverState.AttributeName) && myType != null && myType.getWeaverState() != null) {
 				myGen.addAttribute(Utility.bcelAttribute(new AjAttribute.WeaverState(myType.getWeaverState()), getConstantPool()));
 			}
 		}
@@ -750,8 +755,8 @@ public final class LazyClassGen {
 	public byte[] getJavaClassBytesIncludingReweavable(BcelWorld world) {
 		writeBack(world);
 		byte[] wovenClassFileData = myGen.getJavaClass().getBytes();
-		// At 1.6 stackmaps are optional
-		// At 1.7 or later stackmaps are required (if not turning off the verifier)
+		// At 1.6 stackmaps are optional, whilst at 1.7 and later they
+		// are required (unless turning off the verifier)
 		if ((myGen.getMajor() == Constants.MAJOR_1_6 && world.shouldGenerateStackMaps()) || myGen.getMajor() > Constants.MAJOR_1_6) {
 			if (!AsmDetector.isAsmAround) {
 				throw new BCException("Unable to find Asm for stackmap generation (Looking for 'aj.org.objectweb.asm.ClassReader'). Stackmap generation for woven code is required to avoid verify errors on a Java 1.7 or higher runtime");
@@ -760,7 +765,7 @@ public final class LazyClassGen {
 		}
 
 		WeaverStateInfo wsi = myType.getWeaverState();// getOrCreateWeaverStateInfo();
-		if (wsi != null && wsi.isReweavable()) { // && !reweavableDataInserted
+		if (wsi != null && wsi.isReweavable() && !world.isOverWeaving()) { // && !reweavableDataInserted
 			// reweavableDataInserted = true;
 			return wsi.replaceKeyWithDiff(wovenClassFileData);
 		} else {