From bcdbd68f76629692a5e780702086bff96cdc4c7c Mon Sep 17 00:00:00 2001 From: jhugunin Date: Fri, 2 May 2003 20:36:06 +0000 Subject: [PATCH] fix and better tests for Bugzilla Bug 37152 java.lang.VerifyError: --- .../compiler/ast/AccessForInlineVisitor.java | 15 +++++ .../compiler/batch/WorkingTestMain.java | 10 ++-- tests/ajcTests.xml | 11 +++- tests/bugs/inlineAround/aspect1/Base.java | 30 ++++++++++ tests/bugs/inlineAround/aspect2/Concrete.java | 7 +++ tests/bugs/inlineAround/p1/Main.java | 26 ++++++++ tests/jimTests.xml | 14 +---- .../org/aspectj/weaver/bcel/BcelAdvice.java | 9 +++ .../org/aspectj/weaver/bcel/BcelShadow.java | 59 ++++++------------- 9 files changed, 119 insertions(+), 62 deletions(-) create mode 100644 tests/bugs/inlineAround/aspect1/Base.java create mode 100644 tests/bugs/inlineAround/aspect2/Concrete.java create mode 100644 tests/bugs/inlineAround/p1/Main.java diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java index aa195135e..76acd7306 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java @@ -13,6 +13,8 @@ package org.aspectj.ajdt.internal.compiler.ast; +import java.util.Arrays; + import org.aspectj.ajdt.internal.compiler.lookup.EclipseFactory; import org.aspectj.ajdt.internal.compiler.lookup.InlineAccessFieldBinding; import org.aspectj.ajdt.internal.compiler.lookup.InterTypeFieldBinding; @@ -77,9 +79,18 @@ public class AccessForInlineVisitor extends AbstractSyntaxTreeVisitorAdapter { } public void endVisit(QualifiedNameReference ref, BlockScope scope) { + //System.err.println("qref: " + ref + ", " + ref.binding.getClass().getName() + ", " + ref.codegenBinding.getClass().getName()); + //System.err.println(" others: " + Arrays.asList(ref.otherBindings)); if (ref.binding instanceof FieldBinding) { ref.binding = getAccessibleField((FieldBinding)ref.binding); } + if (ref.otherBindings != null) { + for (int i=0, len=ref.otherBindings.length; i < len; i++) { + if (ref.otherBindings[i] instanceof FieldBinding) { + ref.otherBindings[i] = getAccessibleField((FieldBinding)ref.otherBindings[i]); + } + } + } } public void endVisit(FieldReference ref, BlockScope scope) { @@ -121,6 +132,7 @@ public class AccessForInlineVisitor extends AbstractSyntaxTreeVisitorAdapter { } private FieldBinding getAccessibleField(FieldBinding binding) { + //System.err.println("checking field: " + binding); if (!binding.isValidBinding()) return binding; makePublic(binding.declaringClass); @@ -135,6 +147,9 @@ public class AccessForInlineVisitor extends AbstractSyntaxTreeVisitorAdapter { ResolvedMember m = EclipseFactory.makeResolvedMember(binding); if (inAspect.accessForInline.containsKey(m)) return (FieldBinding)inAspect.accessForInline.get(m); FieldBinding ret = new InlineAccessFieldBinding(inAspect, binding); + + //System.err.println(" made accessor: " + ret); + inAspect.accessForInline.put(m, ret); return ret; } diff --git a/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java b/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java index c86d1c50f..d63418ff6 100644 --- a/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java +++ b/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java @@ -49,8 +49,8 @@ public class WorkingTestMain { //args.add("-aspectpath"); //args.add("../weaver/testdata/megatrace.jar"); - args.add("testdata/src1/AroundA1.java"); - args.add("-XnoInline"); + args.add("c:/aspectj/scratch/arno/*.java"); + //args.add("-XnoInline"); //args.add("../tests/new/Counting1.java"); //args.add("-Xlint:error"); //args.add("testdata/src1/InterType.java"); @@ -60,11 +60,11 @@ public class WorkingTestMain { CommandTestCase.runCompiler(args, CommandTestCase.NO_ERRORS); //CommandTestCase.runCompiler(args, new int[] {11, 14, 18, 32, 43}); - CommandTestCase.printGenerated("../out", "AroundA1"); + //CommandTestCase.printGenerated("../out", "AroundA1"); // CommandTestCase.printGenerated("../out", "SuperC"); -// CommandTestCase.printGenerated("../out", "SubC"); + CommandTestCase.printGenerated("../out", "org.schmidmeier.unittests.cache.TimeCacheTestsWorking"); - TestUtil.runMain("out;../lib/test/testing-client.jar", "AroundA1"); + TestUtil.runMain("out;../lib/test/testing-client.jar", "org.schmidmeier.unittests.cache.AllTimeCacheTests"); } private static String examplesDir = "../docs/dist/doc/examples/"; diff --git a/tests/ajcTests.xml b/tests/ajcTests.xml index 6a164462d..732f9b4bb 100644 --- a/tests/ajcTests.xml +++ b/tests/ajcTests.xml @@ -13,7 +13,6 @@ so there may only be one copy marked "messages-vary" here. new-messages-vary like messages-vary, except need to make ajcTest10 variant - incremental-test incremental test. All tests using inc-compile. fail-{...} test fails in some configuration fail-unimplmented eajc throwing "unimplemented" exception fail-commandLine fails in ajc on command line (move to ajcTestsBroken.xml) @@ -5745,7 +5744,7 @@ + + + + + + + diff --git a/tests/bugs/inlineAround/aspect1/Base.java b/tests/bugs/inlineAround/aspect1/Base.java new file mode 100644 index 000000000..e61f34648 --- /dev/null +++ b/tests/bugs/inlineAround/aspect1/Base.java @@ -0,0 +1,30 @@ +package aspect1; + +public abstract aspect Base { + private Helper h = new Helper(); + { + h.h1 = new Helper(); + h.h1.h1 = new Helper(); + } + + private class Inner { + String data = "inner"; + } + + protected abstract pointcut where(); + + Object around(double d, int i): where() && args(i, d) { + String s = h.data + h.h1.data + h.h1.h1.data + d + i; + System.err.println(s); + return proceed(d, i); + } +} + + +class Helper { + String data = "helper"; + Helper h1; + String getData() { + return data; + } +} \ No newline at end of file diff --git a/tests/bugs/inlineAround/aspect2/Concrete.java b/tests/bugs/inlineAround/aspect2/Concrete.java new file mode 100644 index 000000000..f065fa9f1 --- /dev/null +++ b/tests/bugs/inlineAround/aspect2/Concrete.java @@ -0,0 +1,7 @@ +package aspect2; + +import aspect1.Base; + +aspect Concrete extends Base perthis(where()) { + protected pointcut where(): call(* *..C.*(..)); +} \ No newline at end of file diff --git a/tests/bugs/inlineAround/p1/Main.java b/tests/bugs/inlineAround/p1/Main.java new file mode 100644 index 000000000..bf44127c7 --- /dev/null +++ b/tests/bugs/inlineAround/p1/Main.java @@ -0,0 +1,26 @@ +package p1; + +public class Main { + public static void main(String[] args) { + new Main().doit(); + } + + private void doit() { + long l = 2l + testit(3.2, new C().doit(23, 3.14), 5l); + System.err.println(l); + } + + private long testit(double d, long l1, long l2) { + return (long)(d + l1 + l2); + } +} + +class C { + long doit(int i, double d) { + return (long)(d + i + f1(d, i, i)); + } + + int f1(double d1, long l1, int i1) { + return (int)(d1 + l1 + i1); + } +} \ No newline at end of file diff --git a/tests/jimTests.xml b/tests/jimTests.xml index c071015d0..80a883ac8 100644 --- a/tests/jimTests.xml +++ b/tests/jimTests.xml @@ -1,16 +1,4 @@ - - - - - - - - - + \ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java index a62bbd927..8666df8f5 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java @@ -77,6 +77,15 @@ public class BcelAdvice extends Advice { } pointcutTest = getPointcut().findResidue(shadow, exposedState); + // these initializations won't be performed by findResidue, but need to be + // so that the joinpoint is primed for weaving + if (getKind() == AdviceKind.PerThisEntry) { + shadow.getThisVar(); + } else if (getKind() == AdviceKind.PerTargetEntry) { + shadow.getTargetVar(); + } + + // make sure thisJoinPoint parameters are initialized if ((getExtraParameterFlags() & ThisJoinPointStaticPart) != 0) { ((BcelShadow)shadow).getThisJoinPointStaticPartVar(); diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java index 04b785dc8..c91731e0e 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java @@ -104,6 +104,9 @@ import org.aspectj.weaver.ast.Var; * do have a target of sorts, just one that needs to be pushed on the stack, * dupped, and not touched otherwise until the constructor runs. * + * @author Jim Hugunin + * @author Erik Hilsdale + * */ public class BcelShadow extends Shadow { @@ -1176,8 +1179,8 @@ public class BcelShadow extends Shadow { * * It extracts the instructions of the original shadow into a method. * - * Then it inlines the instructions of the advice in its place, taking care - * to treat the closure argument specially (it doesn't exist). + * Then it extracts the instructions of the advice into a new method defined on + * this enclosing class. This new method can then be specialized as below. * * Then it searches in the instructions of the advice for any call to the * proceed method. @@ -1192,6 +1195,11 @@ public class BcelShadow extends Shadow { * * If only one call to proceed is made, we can re-inline the original shadow. * We are not doing that presently. + * + * If the body of the advice can be determined to not alter the stack, or if + * this shadow doesn't care about the stack, i.e. method-execution, then the + * new method for the advice can also be re-lined. We are not doing that + * presently. */ // !!! THIS BLOCK OF CODE SHOULD BE IN A METHOD CALLED weaveAround(...); @@ -1229,35 +1237,30 @@ public class BcelShadow extends Shadow { getSignature(), getEnclosingClass()) + "$advice"; - List paramTypeList = new ArrayList(); List argVarList = new ArrayList(); List proceedVarList = new ArrayList(); int extraParamOffset = 0; - //TODO paramTypeList not needed any more - - // start w/ stuff + // Create the extra parameters that are needed for passing to proceed + // This code is very similar to that found in makeCallToCallback and should + // be rationalized in the future if (thisVar != null) { - paramTypeList.add(thisVar.getType()); argVarList.add(thisVar); proceedVarList.add(new BcelVar(thisVar.getType(), extraParamOffset)); extraParamOffset += thisVar.getType().getSize(); } if (targetVar != null && targetVar != thisVar) { - paramTypeList.add(targetVar.getType()); argVarList.add(targetVar); proceedVarList.add(new BcelVar(targetVar.getType(), extraParamOffset)); extraParamOffset += targetVar.getType().getSize(); } for (int i = 0, len = getArgCount(); i < len; i++) { - paramTypeList.add(argVars[i].getType()); argVarList.add(argVars[i]); proceedVarList.add(new BcelVar(argVars[i].getType(), extraParamOffset)); extraParamOffset += argVars[i].getType().getSize(); } if (thisJoinPointVar != null) { - paramTypeList.add(thisJoinPointVar.getType()); argVarList.add(thisJoinPointVar); proceedVarList.add(new BcelVar(thisJoinPointVar.getType(), extraParamOffset)); extraParamOffset += thisJoinPointVar.getType().getSize(); @@ -1270,10 +1273,6 @@ public class BcelShadow extends Shadow { System.arraycopy(extractedMethodParameterTypes, 0, parameterTypes, parameterIndex, extractedMethodParameterTypes.length); parameterIndex += extractedMethodParameterTypes.length; -// for (Iterator i = paramTypeList.iterator(); i.hasNext(); ) { -// ResolvedTypeX t = (ResolvedTypeX)i.next(); -// parameterTypes[parameterIndex++] = BcelWorld.makeBcelType(t); -// } parameterTypes[parameterIndex++] = BcelWorld.makeBcelType(adviceMethod.getEnclosingClass().getType()); System.arraycopy(adviceParameterTypes, 0, parameterTypes, parameterIndex, adviceParameterTypes.length); @@ -1289,6 +1288,10 @@ public class BcelShadow extends Shadow { getEnclosingClass().addMethodGen(localAdviceMethod); + + // create a map that will move all slots in advice method forward by extraParamOffset + // in order to make room for the new proceed-required arguments that are added at + // the beginning of the parameter list int nVars = adviceMethod.getMaxLocals() + extraParamOffset; IntMap varMap = IntMap.idMap(nVars); for (int i=extraParamOffset; i < nVars; i++) { @@ -1350,11 +1353,7 @@ public class BcelShadow extends Shadow { range.append(callback); range.append(afterThingie); } - - // now the range contains everything we need. We now inline the advice method. - - //BcelClassWeaver.inlineMethod(adviceMethod, enclosingMethod, adviceMethodInvocation); // now search through the advice, looking for a call to PROCEED. // Then we replace the call to proceed with some argument setup, and a @@ -1391,31 +1390,7 @@ public class BcelShadow extends Shadow { // we have on stack all the arguments for the ADVICE call. // we have in frame somewhere all the arguments for the non-advice call. -// List argVarList = new ArrayList(); -// -// // start w/ stuff -// if (thisVar != null) { -// argVarList.add(thisVar); -// } -// -// if (targetVar != null && targetVar != thisVar) { -// argVarList.add(targetVar); -// } -// for (int i = 0, len = getArgCount(); i < len; i++) { -// argVarList.add(argVars[i]); -// } -// if (thisJoinPointVar != null) { -// argVarList.add(thisJoinPointVar); -// } - BcelVar[] adviceVars = munger.getExposedStateAsBcelVars(); - //??? this is too easy -// for (int i=0; i < adviceVars.length; i++) { -// if (adviceVars[i] != null) -// adviceVars[i].setPositionInAroundState(i); -// } - - IntMap proceedMap = makeProceedArgumentMap(adviceVars); // System.out.println(proceedMap + " for " + this); -- 2.39.5