From 27d204cf9f57a0cc06f4d7c5800c93eb55edc02a Mon Sep 17 00:00:00 2001 From: aclement Date: Fri, 3 Sep 2004 13:45:46 +0000 Subject: [PATCH] Fix for Bugzilla Bug 72528 around advice throws java.lang.VerifyError at runtime --- tests/bugs/ArrayCloning.java | 75 +++++++++++++++++++ .../systemtest/ajc121/Ajc121Tests.java | 4 + .../systemtest/ajc121/ajc121-tests.xml | 6 ++ .../src/org/aspectj/weaver/ResolvedTypeX.java | 6 +- .../org/aspectj/weaver/bcel/BcelShadow.java | 70 ++++++++++++++++- 5 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 tests/bugs/ArrayCloning.java 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 @@ + + + + + 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"); -- 2.39.5