From 4a0295c0c33776dc16f582c33ba13a937ae7b75b Mon Sep 17 00:00:00 2001 From: jhugunin Date: Tue, 17 Dec 2002 03:55:08 +0000 Subject: [PATCH] more bug fixes --- .../compiler/ast/AspectDeclaration.java | 2 +- .../compiler/batch/WorkingTestMain.java | 12 +- weaver/src/org/aspectj/weaver/World.java | 4 +- .../weaver/bcel/BcelCflowStackFieldAdder.java | 2 +- .../org/aspectj/weaver/bcel/BcelShadow.java | 105 ++++++++++++++---- .../org/aspectj/weaver/bcel/LazyClassGen.java | 19 ++++ .../aspectj/weaver/bcel/LazyMethodGen.java | 7 +- weaver/src/org/aspectj/weaver/bcel/Range.java | 2 + .../testdata/AroundArgsDynamicHelloWorld.txt | 27 +++-- .../weaver/patterns/BindingTestCase.java | 21 ++-- 10 files changed, 145 insertions(+), 56 deletions(-) diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java index 66e004d00..26bf18d8b 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java @@ -59,7 +59,7 @@ public class AspectDeclaration extends MemberTypeDeclaration { } public void resolve() { - if (binding == null || ignoreFurtherInvestigation) { + if (binding == null) { ignoreFurtherInvestigation = true; return; } 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 6b0cc3785..b70be133f 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 @@ -35,6 +35,7 @@ public class WorkingTestMain { List args = new ArrayList(); args.add("-verbose"); + //args.add("-1.3"); args.add("-d"); args.add("out"); @@ -48,8 +49,8 @@ public class WorkingTestMain { //args.add("-aspectpath"); //args.add("../weaver/testdata/megatrace.jar"); - //args.add("testdata/src1/AroundA1.java"); - args.add("../tests/new/StrictFPAdvice.java"); + args.add("testdata/src1/AroundA1.java"); + //args.add("../tests/new/AroundInnerCalls.java"); //args.add("-Xlint:error"); //args.add("testdata/src1/InterType.java"); //args.add("@" + examplesDir + "tjp/files.lst"); @@ -58,11 +59,12 @@ public class WorkingTestMain { CommandTestCase.runCompiler(args, CommandTestCase.NO_ERRORS); //CommandTestCase.runCompiler(args, new int[] {11, 14, 18, 32, 43}); - CommandTestCase.printGenerated("../out", "StrictFPAdvice"); - CommandTestCase.printGenerated("../out", "A"); +// CommandTestCase.printGenerated("../out", "AdviceOnInheritedMethod"); +// CommandTestCase.printGenerated("../out", "SuperC"); +// CommandTestCase.printGenerated("../out", "SubC"); //TestUtil.runMain("out;../bcweaver/testdata/megatrace.jar", "Privileged"); - TestUtil.runMain("out;../lib/test/testing-client.jar", "StrictFPAdvice"); + //TestUtil.runMain("out;../lib/test/testing-client.jar", "AroundInnerCalls"); } private static String examplesDir = "c:/aspectj/examples/"; diff --git a/weaver/src/org/aspectj/weaver/World.java b/weaver/src/org/aspectj/weaver/World.java index c12acf985..99f4c6c18 100644 --- a/weaver/src/org/aspectj/weaver/World.java +++ b/weaver/src/org/aspectj/weaver/World.java @@ -144,7 +144,9 @@ public abstract class World { } protected int getModifiers(Member member) { - return resolve(member).getModifiers(); + ResolvedMember r = resolve(member); + if (r == null) throw new BCException("bad resolve of " + member); + return r.getModifiers(); } protected String[] getParameterNames(Member member) { diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java b/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java index c398cec63..5e3ef4665 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java @@ -36,7 +36,7 @@ public class BcelCflowStackFieldAdder extends BcelTypeMunger { gen.getConstantPoolGen()).getField(); gen.addField(f); - LazyMethodGen clinit = gen.getStaticInitializer(); + LazyMethodGen clinit = gen.getAjcClinit(); //StaticInitializer(); InstructionList setup = new InstructionList(); InstructionFactory fact = gen.getFactory(); diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java index 432acd17b..acc8f96f1 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java @@ -309,6 +309,13 @@ public class BcelShadow extends Shadow { LazyMethodGen enclosingMethod) { InstructionList body = enclosingMethod.getBody(); + InstructionHandle ih = body.getStart(); + if (ih.getInstruction() instanceof InvokeInstruction) { + InvokeInstruction ii = (InvokeInstruction)ih.getInstruction(); + if (ii.getName(enclosingMethod.getEnclosingClass().getConstantPoolGen()).equals(NameMangler.AJC_CLINIT_NAME)) { + ih = ih.getNext(); + } + } BcelShadow s = new BcelShadow( world, @@ -319,7 +326,7 @@ public class BcelShadow extends Shadow { ShadowRange r = new ShadowRange(body); r.associateWithShadow(s); r.associateWithTargets( - Range.genStart(body), + Range.genStart(body, ih), Range.genEnd(body)); return s; } @@ -654,7 +661,7 @@ public class BcelShadow extends Shadow { //???return TypeX.forName(getEnclosingClass().getClassName()); } - public boolean hasRealTarget() { + public boolean isTargetDifferentFromThis() { return hasTarget() && isExpressionKind(); } @@ -690,9 +697,9 @@ public class BcelShadow extends Shadow { if (!hasTarget()) { throw new IllegalStateException("no target"); } - initializeTargetVar(); - return targetVar; - } + initializeTargetVar(); + return targetVar; + } public Var getArgVar(int i) { initializeArgVars(); return argVars[i]; @@ -700,8 +707,8 @@ public class BcelShadow extends Shadow { // reflective thisJoinPoint support private BcelVar thisJoinPointVar = null; - private BcelVar thisJoinPointStaticPartVar = null; //XXX should be field - private BcelVar thisEnclosingJoinPointStaticPartVar = null; //XXX should be field + private BcelVar thisJoinPointStaticPartVar = null; + private BcelVar thisEnclosingJoinPointStaticPartVar = null; public final Var getThisJoinPointVar() { return getThisJoinPointBcelVar(); @@ -800,6 +807,7 @@ public class BcelShadow extends Shadow { private void initializeThisVar() { if (thisVar != null) return; thisVar = new BcelVar(getThisType().resolve(world), 0); + thisVar.setPositionInAroundState(0); } public void initializeTargetVar() { InstructionFactory fact = getFactory(); @@ -812,14 +820,15 @@ public class BcelShadow extends Shadow { TypeX type = getTargetType(); targetVar = genTempVar(type, "ajc$target"); range.insert(targetVar.createStore(fact), Range.OutsideBefore); + targetVar.setPositionInAroundState(hasThis() ? 1 : 0); } - targetVar.setPositionInAroundState(0); } public void initializeArgVars() { if (argVars != null) return; InstructionFactory fact = getFactory(); int len = getArgCount(); argVars = new BcelVar[len]; + int positionOffset = (hasTarget() ? 0 : 1) + (hasThis() ? 0 : 1); if (getKind().argsOnStack()) { // we move backwards because we're popping off the stack @@ -828,20 +837,21 @@ public class BcelShadow extends Shadow { BcelVar tmp = genTempVar(type, "ajc$arg" + i); range.insert(tmp.createStore(getFactory()), Range.OutsideBefore); int position = i; - if (hasTarget()) position += 1; + if (hasTarget()) position += positionOffset; tmp.setPositionInAroundState(position); argVars[i] = tmp; } } else { int index = 0; if (hasThis()) index++; + for (int i = 0; i < len; i++) { TypeX type = getArgType(i); BcelVar tmp = genTempVar(type, "ajc$arg" + i); range.insert(tmp.createCopyFrom(fact, index), Range.OutsideBefore); argVars[i] = tmp; int position = i; - if (hasTarget()) position += 1; + if (hasTarget()) position += positionOffset; tmp.setPositionInAroundState(position); index += type.getSize(); } @@ -850,6 +860,7 @@ public class BcelShadow extends Shadow { public void initializeForAroundClosure() { initializeArgVars(); if (hasTarget()) initializeTargetVar(); + if (hasThis()) initializeThisVar(); } @@ -925,6 +936,10 @@ public class BcelShadow extends Shadow { } public void weaveAfterThrowing(BcelAdvice munger, TypeX catchType) { + // a good optimization would be not to generate anything here + // if the shadow is GUARANTEED empty (i.e., there's NOTHING, not even + // a shadow, inside me). + if (getRange().getStart().getNext() == getRange().getEnd()) return; InstructionFactory fact = getFactory(); InstructionList handler = new InstructionList(); BcelVar exceptionVar = genTempVar(catchType); @@ -1352,14 +1367,21 @@ public class BcelShadow extends Shadow { } } - // XXX only used for testing + // exposed for testing InstructionList makeCallToCallback(LazyMethodGen callbackMethod) { InstructionFactory fact = getFactory(); InstructionList callback = new InstructionList(); - if (targetVar != null) { + if (thisVar != null) { + callback.append(fact.ALOAD_0); + } + if (targetVar != null && targetVar != thisVar) { callback.append(BcelRenderer.renderExpr(fact, world, targetVar)); } callback.append(BcelRenderer.renderExprs(fact, world, argVars)); + // remember to render tjps + if (thisJoinPointVar != null) { + callback.append(BcelRenderer.renderExpr(fact, world, thisJoinPointVar)); + } callback.append(Utility.createInvoke(fact, callbackMethod)); return callback; } @@ -1372,13 +1394,20 @@ public class BcelShadow extends Shadow { BcelVar arrayVar = genTempVar(TypeX.OBJECTARRAY); //final Type objectArrayType = new ArrayType(Type.OBJECT, 1); final InstructionList il = new InstructionList(); - int alen = getArgCount() + (targetVar == null ? 0 : 1) + (thisJoinPointVar == null ? 0 : 1); + int alen = getArgCount() + (thisVar == null ? 0 : 1) + + ((targetVar != null && targetVar != thisVar) ? 1 : 0) + + (thisJoinPointVar == null ? 0 : 1); il.append(Utility.createConstant(fact, alen)); il.append((Instruction)fact.createNewArray(Type.OBJECT, (short)1)); arrayVar.appendStore(il, fact); int stateIndex = 0; - if (targetVar != null) { + if (thisVar != null) { + arrayVar.appendConvertableArrayStore(il, fact, stateIndex, thisVar); + thisVar.setPositionInAroundState(stateIndex); + stateIndex++; + } + if (targetVar != null && targetVar != thisVar) { arrayVar.appendConvertableArrayStore(il, fact, stateIndex, targetVar); targetVar.setPositionInAroundState(stateIndex); stateIndex++; @@ -1599,7 +1628,10 @@ public class BcelShadow extends Shadow { private IntMap makeRemap() { IntMap ret = new IntMap(5); int reti = 0; - if (targetVar != null) { + if (thisVar != null) { + ret.put(0, reti++); // thisVar guaranteed to be 0 + } + if (targetVar != null && targetVar != thisVar) { ret.put(targetVar.getSlot(), reti++); } for (int i = 0, len = argVars.length; i < len; i++) { @@ -1613,21 +1645,28 @@ public class BcelShadow extends Shadow { // aliases, which we so helpfully put into temps at the beginning of this join // point. if (! getKind().argsOnStack()) { - int index = 0; - if (hasThis()) { ret.put(0, 0); index++; } + int oldi = 0; + int newi = 0; + // if we're passing in a this and we're not argsOnStack we're always + // passing in a target too + if (hasThis()) { ret.put(0, 0); oldi++; newi+=1; } + //assert targetVar == thisVar for (int i = 0; i < getArgCount(); i++) { TypeX type = getArgType(i); - ret.put(index, index); - index += type.getSize(); + ret.put(oldi, newi); + oldi += type.getSize(); + newi += type.getSize(); } } return ret; } /** - * The new method is nonStatic iff we're from nonExpression advice - * with an enclosing non-static method. - * Otherwise, it's static + * The new method always static. + * It may take some extra arguments: this, target. + * If it's argsOnStack, then it must take both this/target + * If it's argsOnFrame, it shares this and target. + * ??? rewrite this to do less array munging, please */ private LazyMethodGen createMethodGen(String newMethodName) { Type[] parameterTypes = world.makeBcelTypes(getSignature().getParameterTypes()); @@ -1638,10 +1677,23 @@ public class BcelShadow extends Shadow { // modifiers |= Modifier.STRICT; // } modifiers |= Modifier.STATIC; - if (hasTarget()) { + if (targetVar != null && targetVar != thisVar) { TypeX targetType = getTargetType(); + ResolvedMember resolvedMember = getSignature().resolve(world); + if (resolvedMember != null && Modifier.isProtected(resolvedMember.getModifiers()) && + !samePackage(targetType.getPackageName(), getThisType().getPackageName())) + { + if (!targetType.isAssignableFrom(getThisType(), world)) { + throw new BCException("bad bytecode"); + } + targetType = getThisType(); + } parameterTypes = addType(world.makeBcelType(targetType), parameterTypes); } + if (thisVar != null) { + TypeX thisType = getThisType(); + parameterTypes = addType(world.makeBcelType(thisType), parameterTypes); + } // We always want to pass down thisJoinPoint in case we have already woven // some advice in here. If we only have a single piece of around advice on a @@ -1669,6 +1721,13 @@ public class BcelShadow extends Shadow { // TypeX.getNames(getSignature().getExceptions(world)), getEnclosingClass()); } + + private boolean samePackage(String p1, String p2) { + if (p1 == null) return p2 == null; + if (p2 == null) return false; + return p1.equals(p2); + } + private Type[] addType(Type type, Type[] types) { int len = types.length; diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java index 0a1861b47..b3b4cced8 100644 --- a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java +++ b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java @@ -318,6 +318,25 @@ public final class LazyClassGen { return clinit; } + public LazyMethodGen getAjcClinit() { + for (Iterator i = methodGens.iterator(); i.hasNext();) { + LazyMethodGen gen = (LazyMethodGen) i.next(); + if (gen.getName().equals(NameMangler.AJC_CLINIT_NAME)) return gen; + } + throw new RuntimeException("unimplemented"); +// LazyMethodGen clinit = new LazyMethodGen( +// Modifier.STATIC, +// Type.VOID, +// "", +// new Type[0], +// CollectionUtil.NO_STRINGS, +// this); +// clinit.getBody().insert(getFactory().RETURN); +// methodGens.add(clinit); +// return clinit; + } + + // reflective thisJoinPoint support Map/*BcelShadow, Field*/ tjpFields = new HashMap(); diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java index 63d105e19..35648dc5f 100644 --- a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java +++ b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java @@ -700,8 +700,11 @@ public final class LazyMethodGen { if (r instanceof ExceptionRange) { ExceptionRange er = (ExceptionRange) r; if (er.getStart() == ih) { - // order is important, insert handlers in order of start - insertHandler(er, exnList); + //System.err.println("er " + er); + if (!er.isEmpty()){ + // order is important, insert handlers in order of start + insertHandler(er, exnList); + } } } else { // we must be a shadow range or something equally useless, diff --git a/weaver/src/org/aspectj/weaver/bcel/Range.java b/weaver/src/org/aspectj/weaver/bcel/Range.java index 0d6eb21b6..49df80897 100644 --- a/weaver/src/org/aspectj/weaver/bcel/Range.java +++ b/weaver/src/org/aspectj/weaver/bcel/Range.java @@ -50,7 +50,9 @@ abstract class Range implements InstructionTargeter { boolean isEmpty() { InstructionHandle ih = start; + //System.err.println(" looking for " + end); while (ih != end) { + //System.err.println(" ih " + ih); if (! Range.isRangeHandle(ih)) return false; ih = ih.getNext(); } diff --git a/weaver/testdata/AroundArgsDynamicHelloWorld.txt b/weaver/testdata/AroundArgsDynamicHelloWorld.txt index 5ca21c9b5..0c2881392 100644 --- a/weaver/testdata/AroundArgsDynamicHelloWorld.txt +++ b/weaver/testdata/AroundArgsDynamicHelloWorld.txt @@ -68,15 +68,19 @@ public class DynamicHelloWorld extends java.lang.Object implements java.io.Seria | | IFEQ L0 | | ALOAD 4 | | CHECKCAST java.util.ArrayList - | | BIPUSH 2 + | | BIPUSH 3 | | ANEWARRAY java.lang.Object | | ASTORE 5 | | ALOAD 5 | | BIPUSH 0 - | | ALOAD 4 + | | ALOAD_0 | | AASTORE | | ALOAD 5 | | BIPUSH 1 + | | ALOAD 4 + | | AASTORE + | | ALOAD 5 + | | BIPUSH 2 | | ALOAD_3 | | AASTORE | | NEW DynamicHelloWorld$AjcClosure1 @@ -85,9 +89,10 @@ public class DynamicHelloWorld extends java.lang.Object implements java.io.Seria | | INVOKESPECIAL DynamicHelloWorld$AjcClosure1. ([Ljava/lang/Object;)V | | INVOKESTATIC Aspect.ajc_around0 (Ljava/util/ArrayList;Lorg/aspectj/runtime/internal/AroundClosure;)Z | | GOTO L1 - | | L0: ALOAD 4 + | | L0: ALOAD_0 + | | ALOAD 4 | | ALOAD_3 - | | INVOKESTATIC DynamicHelloWorld.add_aroundBody0 (Ljava/util/List;Ljava/lang/Object;)Z + | | INVOKESTATIC DynamicHelloWorld.add_aroundBody0 (LDynamicHelloWorld;Ljava/util/List;Ljava/lang/Object;)Z | | L1: NOP | method-call(boolean java.util.List.add(java.lang.Object)) | POP @@ -99,12 +104,12 @@ public class DynamicHelloWorld extends java.lang.Object implements java.io.Seria method-execution(java.lang.String DynamicHelloWorld.doit(java.lang.String, java.util.List)) end String doit(String, java.util.List) - static final boolean add_aroundBody0(java.util.List, Object): - ALOAD_0 + static final boolean add_aroundBody0(DynamicHelloWorld, java.util.List, Object): ALOAD_1 + ALOAD_2 INVOKEINTERFACE java.util.List.add (Ljava/lang/Object;)Z (line 21) IRETURN - end static final boolean add_aroundBody0(java.util.List, Object) + end static final boolean add_aroundBody0(DynamicHelloWorld, java.util.List, Object) end public class DynamicHelloWorld public class DynamicHelloWorld$AjcClosure1 extends org.aspectj.runtime.internal.AroundClosure: @@ -119,14 +124,18 @@ public class DynamicHelloWorld$AjcClosure1 extends org.aspectj.runtime.internal. ALOAD_0 GETFIELD org.aspectj.runtime.internal.AroundClosure.state [Ljava/lang/Object; ASTORE_2 + ALOAD_2 + BIPUSH 0 + AALOAD + CHECKCAST DynamicHelloWorld ALOAD_1 BIPUSH 0 AALOAD CHECKCAST java.util.List ALOAD_2 - BIPUSH 1 + BIPUSH 2 AALOAD - INVOKESTATIC DynamicHelloWorld.add_aroundBody0 (Ljava/util/List;Ljava/lang/Object;)Z + INVOKESTATIC DynamicHelloWorld.add_aroundBody0 (LDynamicHelloWorld;Ljava/util/List;Ljava/lang/Object;)Z INVOKESTATIC org.aspectj.runtime.internal.Conversions.booleanObject (Z)Ljava/lang/Object; ARETURN end public Object run(Object[]) diff --git a/weaver/testsrc/org/aspectj/weaver/patterns/BindingTestCase.java b/weaver/testsrc/org/aspectj/weaver/patterns/BindingTestCase.java index d1d2b3923..d7b598f38 100644 --- a/weaver/testsrc/org/aspectj/weaver/patterns/BindingTestCase.java +++ b/weaver/testsrc/org/aspectj/weaver/patterns/BindingTestCase.java @@ -23,20 +23,9 @@ import junit.framework.TestCase; import org.aspectj.bridge.AbortException; import org.aspectj.weaver.*; -/** - * @author hugunin - * - * To change this generated comment edit the template variable "typecomment": - * Window>Preferences>Java>Templates. - * To enable and disable the creation of type comments go to - * Window>Preferences>Java>Code Generation. - */ public class BindingTestCase extends TestCase { - /** - * Constructor for ParserTestCase. - * @param arg0 - */ + public BindingTestCase(String arg0) { super(arg0); } @@ -58,8 +47,12 @@ public class BindingTestCase extends TestCase { checkBindings("this(*)", none); checkBindings("this(a)", a); - checkBindings("args(.., a,..,b)", all); - checkBindings("args(a,..,b, ..)", all); + try {checkBindings("args(.., a,..,b)", all); + //checkBindings("args(a,..,b, ..)", all); + fail("shouldn't be implemented yet"); + } catch (AbortException ae) { + // not implemented yet + } checkBindings("args(a,..,b)", all); checkBindings("args(b)", b); -- 2.39.5