Fix for Bugzilla Bug 72528

	  	around advice throws java.lang.VerifyError at runtime

5 files changed, 159 insertions, 2 deletions

diff --git a/tests/bugs/ArrayCloning.java b/tests/bugs/ArrayCloning.java
new file mode 100644
index 000000000..43884833f
--- /dev/null
+++ b/tests/bugs/ArrayCloning.java
@@ -0,0 +1,75 @@
+
+public class ArrayCloning {
+
+    public static void main(String[] args) {
+        ArrayCloning ArrayCloning = new ArrayCloning();
+        Integer[] clonedStaticField = ArrayCloning.clone1();
+        checkIdentical(clonedStaticField,ArrayCloning.staticField);
+    
+        Integer[] clonedField = ArrayCloning.clone2();
+        checkIdentical(clonedField,ArrayCloning.nonStaticField);
+
+        Integer[] clown = null;
+
+        clown = ArrayCloning.clone3();
+        clown = ArrayCloning.clone4();
+        Integer[][] ArrayCloningArrayCloning = ArrayCloning.clone5();
+    }
+
+    public static void checkIdentical(Integer[] one, Integer[] two) {
+      if (one[0]!=two[0]) throw new RuntimeException("Not the same (a)");
+      if (one[1]!=two[1]) throw new RuntimeException("Not the same (b)");
+    }
+
+    private static Integer[] staticField = new Integer[2];
+
+    private Integer[] nonStaticField = new Integer[2];
+
+    public ArrayCloning() {
+      nonStaticField[0] = new Integer(32);
+      nonStaticField[1] = new Integer(64);
+    }
+
+    static {
+      staticField[0] = new Integer(1);
+      staticField[1] = new Integer(2);
+    }
+    
+    public Integer[] clone1() {
+      System.err.println("Clone call on a static field");
+      return (Integer[])staticField.clone();
+    }
+
+    public Integer[] clone2() {
+       System.err.println("Clone call on a non-static field");
+       return (Integer[])nonStaticField.clone();
+    }
+
+    public Integer[] clone3() {
+       System.err.println("Clone call on a local variable");
+       Integer[] ArrayCloningArrayCloning = staticField;
+       return (Integer[])ArrayCloningArrayCloning.clone();
+    }
+
+
+    // Clone call on anonymous 'thing' !
+    public Integer[] clone4() {
+      System.err.println("Clone call on a 1 dimensional anonymous integer array");
+      return (Integer[])new Integer[5].clone();
+    }
+
+    // Sick
+    public Integer[][] clone5() {
+      System.err.println("Clone call on a 2 dimensional anonymous integer array");
+      return (Integer[][])new Integer[5][3].clone();
+    }
+
+
+}
+
+aspect HelloWorldAspect {
+    Object[] around(): call(* java.lang.Object.clone()) && within(ArrayCloning) {
+  	Object[] ret = proceed(); 
+	return (Object[])ret.clone();
+    }
+}
diff --git a/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java b/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java
index bb9f64266..be7192266 100644
--- a/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java
@@ -301,6 +301,10 @@ public class Ajc121Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   public void test055_cnfe() {
     runTest("passing null to array arguments confuzes static join point signature. (2)");
   }
+  
+  public void test056_arrayCloning() {
+    runTest("around advice throws java.lang.VerifyError at runtime");
+  }
    
 }
 
diff --git a/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml b/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml
index 43dce6610..ddb48afb5 100644
--- a/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml
+++ b/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml
@@ -443,3 +443,9 @@
 	  <compile files="Main2.java,MainAspect.java"/>
 	  <run class="dk.infimum.aspectjtest.Main2"/>
 	</ajc-test>
+	
+    <ajc-test dir="bugs" pr="72528"
+		title="around advice throws java.lang.VerifyError at runtime">
+		<compile files="ArrayCloning.java"/>
+		<run class="ArrayCloning"/>
+	</ajc-test>
diff --git a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java
index 67a96e072..fb81c5a7f 100644
--- a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java
+++ b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java
@@ -724,7 +724,11 @@ public abstract class ResolvedTypeX extends TypeX {
         }
         public final ResolvedMember[] getDeclaredMethods() {
             // ??? should this return clone?  Probably not...
-            return ResolvedMember.NONE;
+            // If it ever does, here is the code:
+            //  ResolvedMember cloneMethod =
+            //    new ResolvedMember(Member.METHOD,this,Modifier.PUBLIC,TypeX.OBJECT,"clone",new TypeX[]{});
+            //  return new ResolvedMember[]{cloneMethod};
+        	return ResolvedMember.NONE;
         }
         public final ResolvedTypeX[] getDeclaredInterfaces() {
             return
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
index 288b9fd74..2d15f0989 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
@@ -21,6 +21,7 @@ import java.util.List;
 import org.aspectj.apache.bcel.Constants;
 import org.aspectj.apache.bcel.classfile.Field;
 import org.aspectj.apache.bcel.generic.ACONST_NULL;
+import org.aspectj.apache.bcel.generic.ANEWARRAY;
 import org.aspectj.apache.bcel.generic.ArrayType;
 import org.aspectj.apache.bcel.generic.BranchInstruction;
 import org.aspectj.apache.bcel.generic.ConstantPoolGen;
@@ -36,6 +37,8 @@ import org.aspectj.apache.bcel.generic.InstructionHandle;
 import org.aspectj.apache.bcel.generic.InstructionList;
 import org.aspectj.apache.bcel.generic.InstructionTargeter;
 import org.aspectj.apache.bcel.generic.InvokeInstruction;
+import org.aspectj.apache.bcel.generic.LoadInstruction;
+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.ReturnInstruction;
@@ -1110,11 +1113,73 @@ public class BcelShadow extends Shadow {
         } else {
             initializeArgVars(); // gotta pop off the args before we find the target
             TypeX type = getTargetType();
+            type = ensureTargetTypeIsCorrect(type);
             targetVar = genTempVar(type, "ajc$target");
             range.insert(targetVar.createStore(fact), Range.OutsideBefore); 
 	        targetVar.setPositionInAroundState(hasThis() ? 1 : 0);            
         }
     }
+    
+    /* PR 72528
+     * This method double checks the target type under certain conditions.  The Java 1.4
+     * compilers seem to take calls to clone methods on array types and create bytecode that
+     * looks like clone is being called on Object.  If we advise a clone call with around
+     * advice we extract the call into a helper method which we can then refer to.  Because the
+     * type in the bytecode for the call to clone is Object we create a helper method with
+     * an Object parameter - this is not correct as we have lost the fact that the actual
+     * type is an array type.  If we don't do the check below we will create code that fails
+     * java verification.  This method checks for the peculiar set of conditions and if they
+     * are true, it has a sneak peek at the code before the call to see what is on the stack.
+     */
+    public TypeX ensureTargetTypeIsCorrect(TypeX tx) {
+    	if (tx.equals(ResolvedTypeX.OBJECT) && getKind() == MethodCall && 
+    	    getSignature().getReturnType().equals(ResolvedTypeX.OBJECT) && 
+			getSignature().getArity()==0 && 
+			getSignature().getName().charAt(0) == 'c' &&
+			getSignature().getName().equals("clone")) {
+    		
+    		// Lets go back through the code from the start of the shadow
+            InstructionHandle searchPtr = range.getStart().getPrev();
+            while (Range.isRangeHandle(searchPtr) || 
+            	   searchPtr.getInstruction() instanceof StoreInstruction) { // ignore this instruction - it doesnt give us the info we want
+            	searchPtr = searchPtr.getPrev();  
+            }
+            
+            // A load instruction may tell us the real type of what the clone() call is on
+            if (searchPtr.getInstruction() instanceof LoadInstruction) {
+            	LoadInstruction li = (LoadInstruction)searchPtr.getInstruction();
+            	li.getIndex();
+            	LocalVariableTag lvt = LazyMethodGen.getLocalVariableTag(searchPtr,li.getIndex());
+            	return lvt.getType();
+            }
+            // A field access instruction may tell us the real type of what the clone() call is on
+            if (searchPtr.getInstruction() instanceof FieldInstruction) {
+            	FieldInstruction si = (FieldInstruction)searchPtr.getInstruction();
+            	Type t = si.getFieldType(getEnclosingClass().getConstantPoolGen());
+            	return BcelWorld.fromBcel(t);
+            } 
+            // A new array instruction obviously tells us it is an array type !
+            if (searchPtr.getInstruction() instanceof ANEWARRAY) {
+            	//ANEWARRAY ana = (ANEWARRAY)searchPoint.getInstruction();
+            	//Type t = ana.getType(getEnclosingClass().getConstantPoolGen());
+            	// Just use a standard java.lang.object array - that will work fine
+            	return BcelWorld.fromBcel(new ArrayType(Type.OBJECT,1));
+            }
+            // A multi new array instruction obviously tells us it is an array type !
+            if (searchPtr.getInstruction() instanceof MULTIANEWARRAY) {
+            	MULTIANEWARRAY ana = (MULTIANEWARRAY)searchPtr.getInstruction();
+                // Type t = ana.getType(getEnclosingClass().getConstantPoolGen());
+            	// t = new ArrayType(t,ana.getDimensions());
+            	// Just use a standard java.lang.object array - that will work fine
+            	return BcelWorld.fromBcel(new ArrayType(Type.OBJECT,ana.getDimensions()));
+            }
+            throw new BCException("Can't determine real target of clone() when processing instruction "+
+              searchPtr.getInstruction());
+    	}
+    	return tx;
+    }
+    
+    
     public void initializeArgVars() {
         if (argVars != null) return;
     	InstructionFactory fact = getFactory();
@@ -2208,9 +2273,12 @@ public class BcelShadow extends Shadow {
         modifiers |= Modifier.STATIC;
         if (targetVar != null && targetVar != thisVar) {
             TypeX targetType = getTargetType();
+            targetType = ensureTargetTypeIsCorrect(targetType);
             ResolvedMember resolvedMember = getSignature().resolve(world);
+            
             if (resolvedMember != null && Modifier.isProtected(resolvedMember.getModifiers()) && 
-            	!samePackage(targetType.getPackageName(), getEnclosingType().getPackageName()))
+            	!samePackage(targetType.getPackageName(), getEnclosingType().getPackageName()) &&
+				!resolvedMember.getName().equals("clone"))
             {
             	if (!targetType.isAssignableFrom(getThisType(), world)) {
             		throw new BCException("bad bytecode");