diff options
6 files changed, 189 insertions, 70 deletions
diff --git a/tests/bugs195/333274/Code.java b/tests/bugs195/333274/Code.java new file mode 100644 index 000000000..d456c0a05 --- /dev/null +++ b/tests/bugs195/333274/Code.java @@ -0,0 +1,39 @@ +import org.aspectj.lang.annotation.*; +import org.aspectj.lang.*; + +public class Code { + public static void main(String []argv) { +try { + foo(); +} catch (Throwable t) { + System.out.println("Caught "+t); +} + } + + public static void foo() { + print1("abc"); + print2("def"); + print1("ghi"); + } + + public static void print1(String s) { + System.out.println(s); + } + + public static void print2(String s) { + System.out.println(s); + } +} + +@Aspect +class Azpect { + @Around("call(* print2(..))") + public Object one(ProceedingJoinPoint pjp) { + return pjp.proceed(); + } + @Around("call(* print2(..))") + public Object two(ProceedingJoinPoint pjp) { + //return pjp.proceed(); + throw new IllegalStateException(""); + } +} diff --git a/tests/src/test/java/org/aspectj/systemtest/AllTests19.java b/tests/src/test/java/org/aspectj/systemtest/AllTests19.java index c2c798c32..eeff00b31 100644 --- a/tests/src/test/java/org/aspectj/systemtest/AllTests19.java +++ b/tests/src/test/java/org/aspectj/systemtest/AllTests19.java @@ -1,12 +1,9 @@ /******************************************************************************* - * Copyright (c) 2013, 2014 Contributors + * Copyright (c) 2013, 2019 Contributors * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html - * - * Contributors: - * Andy Clement - initial API and implementation *******************************************************************************/ package org.aspectj.systemtest; @@ -14,19 +11,25 @@ import org.aspectj.systemtest.ajc190.AllTestsAspectJ190; import org.aspectj.systemtest.ajc191.AllTestsAspectJ191; import org.aspectj.systemtest.ajc192.AllTestsAspectJ192; import org.aspectj.systemtest.ajc193.AllTestsAspectJ193; +import org.aspectj.systemtest.ajc195.AllTestsAspectJ195; import junit.framework.Test; import junit.framework.TestSuite; +/** + * @author Andy Clement + */ public class AllTests19 { public static Test suite() { TestSuite suite = new TestSuite("AspectJ System Test Suite - 1.9"); - // $JUnit-BEGIN$ - suite.addTest(AllTestsAspectJ190.suite()); - suite.addTest(AllTestsAspectJ191.suite()); - suite.addTest(AllTestsAspectJ192.suite()); - suite.addTest(AllTestsAspectJ193.suite()); + // $JUnit-BEGIN$ + suite.addTest(AllTestsAspectJ190.suite()); + suite.addTest(AllTestsAspectJ191.suite()); + suite.addTest(AllTestsAspectJ192.suite()); + suite.addTest(AllTestsAspectJ193.suite()); + // there were no new tests for 1.9.4 + suite.addTest(AllTestsAspectJ195.suite()); suite.addTest(AllTests18.suite()); // $JUnit-END$ return suite; diff --git a/tests/src/test/java/org/aspectj/systemtest/ajc195/Ajc195Tests.java b/tests/src/test/java/org/aspectj/systemtest/ajc195/Ajc195Tests.java new file mode 100644 index 000000000..7c6034a3e --- /dev/null +++ b/tests/src/test/java/org/aspectj/systemtest/ajc195/Ajc195Tests.java @@ -0,0 +1,34 @@ +/******************************************************************************* + * Copyright (c) 2019 Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + *******************************************************************************/ +package org.aspectj.systemtest.ajc195; + +import org.aspectj.testing.XMLBasedAjcTestCase; + +import junit.framework.Test; + +/** + * @author Andy Clement + */ +public class Ajc195Tests extends XMLBasedAjcTestCase { + + public void testFinallyBlocksAndUnlinkingAndExceptions() { + runTest("around finally blocks and unlinking"); + } + + // --- + + public static Test suite() { + return XMLBasedAjcTestCase.loadSuite(Ajc195Tests.class); + } + + @Override + protected java.net.URL getSpecFile() { + return getClassResource("ajc195.xml"); + } + +} diff --git a/tests/src/test/java/org/aspectj/systemtest/ajc195/AllTestsAspectJ195.java b/tests/src/test/java/org/aspectj/systemtest/ajc195/AllTestsAspectJ195.java new file mode 100644 index 000000000..a7eb6604e --- /dev/null +++ b/tests/src/test/java/org/aspectj/systemtest/ajc195/AllTestsAspectJ195.java @@ -0,0 +1,23 @@ +/******************************************************************************* + * Copyright (c) 2018-2019 Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Andy Clement - initial API and implementation + *******************************************************************************/ +package org.aspectj.systemtest.ajc195; + +import junit.framework.Test; +import junit.framework.TestSuite; + +public class AllTestsAspectJ195 { + + public static Test suite() { + TestSuite suite = new TestSuite("AspectJ 1.9.4 tests"); + suite.addTest(Ajc195Tests.suite()); + return suite; + } +} diff --git a/tests/src/test/resources/org/aspectj/systemtest/ajc195/ajc195.xml b/tests/src/test/resources/org/aspectj/systemtest/ajc195/ajc195.xml new file mode 100644 index 000000000..10d1a0156 --- /dev/null +++ b/tests/src/test/resources/org/aspectj/systemtest/ajc195/ajc195.xml @@ -0,0 +1,19 @@ +<!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd"[]> + +<suite> + + <ajc-test dir="bugs195/333274" vm="1.8" title="around finally blocks and unlinking"> + <compile files="Code.java" options="-showWeaveInfo -1.8 -XnoInline"> + <message kind="weave" text="Join point 'method-call(void Code.print2(java.lang.String))' in Type 'Code' (Code.java:15) advised by around advice from 'Azpect' (Code.java:35)"/> + <message kind="weave" text="Join point 'method-call(void Code.print2(java.lang.String))' in Type 'Code' (Code.java:15) advised by around advice from 'Azpect' (Code.java:31)"/> + </compile> + + <run class="Code"> + <stdout> + <line text="abc"/> + <line text="Caught java.lang.IllegalStateException"/> + </stdout> + </run> + </ajc-test> + +</suite> diff --git a/weaver/src/main/java/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/main/java/org/aspectj/weaver/bcel/BcelShadow.java index 6d0cb4f60..7e1e0bfc4 100644 --- a/weaver/src/main/java/org/aspectj/weaver/bcel/BcelShadow.java +++ b/weaver/src/main/java/org/aspectj/weaver/bcel/BcelShadow.java @@ -73,52 +73,52 @@ import org.aspectj.weaver.patterns.ThisOrTargetPointcut; /* * Some fun implementation stuff: - * + * * * expressionKind advice is non-execution advice - * * may have a target. - * * if the body is extracted, it will be extracted into - * a static method. The first argument to the static + * * may have a target. + * * if the body is extracted, it will be extracted into + * a static method. The first argument to the static * method is the target * * advice may expose a this object, but that's the advice's * consideration, not ours. This object will NOT be cached in another * local, but will always come from frame zero. - * - * * non-expressionKind advice is execution advice + * + * * non-expressionKind advice is execution advice * * may have a this. * * target is same as this, and is exposed that way to advice * (i.e., target will not be cached, will always come from frame zero) * * if the body is extracted, it will be extracted into a method - * with same static/dynamic modifier as enclosing method. If non-static, - * target of callback call will be this. + * with same static/dynamic modifier as enclosing method. If non-static, + * target of callback call will be this. * - * * because of these two facts, the setup of the actual arguments (including + * * because of these two facts, the setup of the actual arguments (including * possible target) callback method is the same for both kinds of advice: * push the targetVar, if it exists (it will not exist for advice on static - * things), then push all the argVars. - * + * things), then push all the argVars. + * * Protected things: * * * the above is sufficient for non-expressionKind advice for protected things, * since the target will always be this. - * + * * * For expressionKind things, we have to modify the signature of the callback * method slightly. For non-static expressionKind things, we modify * the first argument of the callback method NOT to be the type specified * by the method/field signature (the owner), but rather we type it to * the currentlyEnclosing type. We are guaranteed this will be fine, * since the verifier verifies that the target is a subtype of the currently - * enclosingType. - * + * enclosingType. + * * Worries: - * + * * * ConstructorCalls will be weirder than all of these, since they * supposedly don't have a target (according to AspectJ), but they clearly * 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 { @@ -210,7 +210,7 @@ public class BcelShadow extends Shadow { * The new/dup (or new/dup_x1/swap) are removed and will be readded later (after the advice call) by the caller of this method. * The groovy compiler produces unusual code where the new/dup isn't visible (when making a this() call from an existing ctor), * an aload_0 is used to load the uninitialized object (as an example see the ctors in grails.util.BuildSettings). - * + * * @return true if managed to remove them */ private boolean deleteNewAndDup() { @@ -630,7 +630,7 @@ public class BcelShadow extends Shadow { /** * Create an initialization join point associated with a constructor, but not with any body of code yet. If this is actually * matched, it's range will be set when we inline self constructors. - * + * * @param constructor The constructor starting this initialization. */ public static BcelShadow makeUnfinishedInitialization(BcelWorld world, LazyMethodGen constructor) { @@ -882,8 +882,8 @@ public class BcelShadow extends Shadow { private Map<ResolvedType, AnnotationAccessVar> withinAnnotationVars = null; private Map<ResolvedType, AnnotationAccessVar> withincodeAnnotationVars = null; private boolean allArgVarsInitialized = false; - - // If in annotation style and the relevant advice is using PJP then this will + + // If in annotation style and the relevant advice is using PJP then this will // be set to true when the closure variable is initialized - if it gets set // (which means link() has been called) then we will need to call unlink() // after the code has been run. @@ -1143,7 +1143,7 @@ public class BcelShadow extends Shadow { /** * Get the Var for the xxxxJpStaticPart, xxx = this or enclosing - * + * * @param isEnclosingJp true to have the enclosingJpStaticPart * @return */ @@ -1165,7 +1165,7 @@ public class BcelShadow extends Shadow { /** * Get the Var for the enclosingJpStaticPart - * + * * @return */ public BcelVar getThisEnclosingJoinPointStaticPartBcelVar() { @@ -1702,11 +1702,11 @@ public class BcelShadow extends Shadow { /** * The basic strategy here is to add a set of instructions at the end of the shadow range that dispatch the advice, and then * return whatever the shadow was going to return anyway. - * + * * To achieve this, we note all the return statements in the advice, and replace them with code that: 1) stores the return value * on top of the stack in a temp var 2) jumps to the start of our advice block 3) restores the return value at the end of the * advice block before ultimately returning - * + * * We also need to bind the return value into a returning parameter, if the advice specified one. */ public void weaveAfterReturning(BcelAdvice munger) { @@ -1759,11 +1759,11 @@ public class BcelShadow extends Shadow { * making this the ultimate return. If the shadow has a non-void return type, we also create a temporary variable to hold the * return value, and load the value from this var before returning (see pr148007 for why we do this - it works around a JRockit * bug, and is also closer to what javac generates) - * + * * Sometimes the 'last return' isnt the right one - some rogue code can include the real return from the body of a subroutine * that exists at the end of the method. In this case the last return is RETURN but that may not be correct for a method with a * non-void return type... pr151673 - * + * * @param returns list of all the return instructions in the shadow * @param returnInstructions instruction list into which the return instructions should be generated * @return the variable holding the return value, if needed @@ -1802,7 +1802,7 @@ public class BcelShadow extends Shadow { /** * Get the list of instructions used to dispatch to the after advice - * + * * @param munger * @param firstInstructionInReturnSequence * @return @@ -1821,7 +1821,7 @@ public class BcelShadow extends Shadow { /** * If the after() returning(Foo f) form is used, bind the return value to the parameter. If the shadow returns void, bind null. - * + * * @param advice * @return */ @@ -1842,7 +1842,7 @@ public class BcelShadow extends Shadow { /** * Helper method for weaveAfterReturning - * + * * Each return instruction in the method body is retargeted by calling this method. The return instruction is replaced by up to * three instructions: 1) if the shadow returns a value, and that value is bound to an after returning parameter, then we DUP * the return value on the top of the stack 2) if the shadow returns a value, we store it in the returnValueVar (it will be @@ -2009,13 +2009,13 @@ public class BcelShadow extends Shadow { } ResolvedType aspectRT = munger.getConcreteAspect(); BcelWorld.getBcelObjectType(aspectRT); - + // Although matched, if the visibility rules prevent the aspect from seeing this type, don't // insert any code (easier to do it here than try to affect the matching logic, unfortunately) if (!(tResolved.canBeSeenBy(aspectRT) || aspectRT.isPrivilegedAspect())) { return; } - + final InstructionFactory fact = getFactory(); InstructionList entryInstructions = new InstructionList(); @@ -2130,25 +2130,25 @@ public class BcelShadow extends Shadow { /* * Implementation notes: - * + * * AroundInline still extracts the instructions of the original shadow into an extracted method. This allows inlining of even * that advice that doesn't call proceed or calls proceed more than once. - * + * * It extracts the instructions of the original shadow into a method. - * + * * 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. - * + * * At such a call, there is stuff on the stack representing the arguments to proceed. Pop these into the frame. - * + * * Now build the stack for the call to the extracted method, taking values either from the join point state or from the new * frame locs from proceed. Now call the extracted method. The right return value should be on the stack, so no cast is * necessary. - * + * * 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. */ @@ -2200,7 +2200,7 @@ public class BcelShadow extends Shadow { String extractedShadowMethodName = NameMangler.aroundShadowMethodName(getSignature(), shadowClass.getNewGeneratedNameTag()); List<String> parameterNames = new ArrayList<String>(); boolean shadowClassIsInterface = shadowClass.isInterface(); - LazyMethodGen extractedShadowMethod = extractShadowInstructionsIntoNewMethod(extractedShadowMethodName, + LazyMethodGen extractedShadowMethod = extractShadowInstructionsIntoNewMethod(extractedShadowMethodName, shadowClassIsInterface?Modifier.PUBLIC:Modifier.PRIVATE, munger.getSourceLocation(), parameterNames,shadowClassIsInterface); @@ -2543,13 +2543,13 @@ public class BcelShadow extends Shadow { /** * Annotation style handling for inlining. - * + * * Note: The proceedingjoinpoint is already on the stack (since the user was calling pjp.proceed(...) - * + * * The proceed map is ignored (in terms of argument repositioning) since we have a fixed expected format for annotation style. * The aim here is to change the proceed() call into a call to the xxx_aroundBody0 method. - * - * + * + * */ private InstructionList getRedoneProceedCallForAnnotationStyle(InstructionFactory fact, LazyMethodGen callbackMethod, BcelAdvice munger, LazyMethodGen localAdviceMethod, List<BcelVar> argVarList, boolean isProceedWithArgs) { @@ -2852,7 +2852,7 @@ public class BcelShadow extends Shadow { int linenumber = getSourceLine(); // MOVE OUT ALL THE INSTRUCTIONS IN MY SHADOW INTO ANOTHER METHOD! - + // callbackMethod will be something like: "static final void m_aroundBody0(I)" boolean shadowClassIsInterface = getEnclosingClass().isInterface(); LazyMethodGen callbackMethod = extractShadowInstructionsIntoNewMethod( @@ -2944,7 +2944,7 @@ public class BcelShadow extends Shadow { } closureVarInitialized = false; - + // ATAJ for @AJ aspect we need to link the closure with the joinpoint instance if (munger.getConcreteAspect() != null && munger.getConcreteAspect().isAnnotationStyleAspect() && munger.getDeclaringAspect() != null && munger.getDeclaringAspect().resolve(world).isAnnotationStyleAspect()) { @@ -2952,18 +2952,18 @@ public class BcelShadow extends Shadow { aroundClosureInstance = genTempVar(AjcMemberMaker.AROUND_CLOSURE_TYPE); closureInstantiation.append(fact.createDup(1)); aroundClosureInstance.appendStore(closureInstantiation, fact); - + // stick the bitflags on the stack and call the variant of linkClosureAndJoinPoint that takes an int closureInstantiation.append(fact.createConstant(Integer.valueOf(bitflags))); if (needAroundClosureStacking) { closureInstantiation.append(Utility.createInvoke(getFactory(), getWorld(), new MemberImpl(Member.METHOD, UnresolvedType.forName("org.aspectj.runtime.internal.AroundClosure"), Modifier.PUBLIC, "linkStackClosureAndJoinPoint", String.format("%s%s", "(I)", "Lorg/aspectj/lang/ProceedingJoinPoint;")))); - + } else { closureInstantiation.append(Utility.createInvoke(getFactory(), getWorld(), new MemberImpl(Member.METHOD, UnresolvedType.forName("org.aspectj.runtime.internal.AroundClosure"), - Modifier.PUBLIC, "linkClosureAndJoinPoint", String.format("%s%s", "(I)", "Lorg/aspectj/lang/ProceedingJoinPoint;")))); + Modifier.PUBLIC, "linkClosureAndJoinPoint", String.format("%s%s", "(I)", "Lorg/aspectj/lang/ProceedingJoinPoint;")))); } } @@ -2973,36 +2973,37 @@ public class BcelShadow extends Shadow { // invoke the advice InstructionHandle tryUnlinkPosition = advice.append(munger.getNonTestAdviceInstructions(this)); - + if (needAroundClosureStacking) { // Call AroundClosure.unlink() in a 'finally' block if (munger.getConcreteAspect() != null && munger.getConcreteAspect().isAnnotationStyleAspect() && munger.getDeclaringAspect() != null && munger.getDeclaringAspect().resolve(world).isAnnotationStyleAspect() && closureVarInitialized) { - + // Call unlink when 'normal' flow occurring aroundClosureInstance.appendLoad(advice, fact); InstructionHandle unlinkInsn = advice.append(Utility.createInvoke(getFactory(), getWorld(), new MemberImpl(Member.METHOD, UnresolvedType .forName("org.aspectj.runtime.internal.AroundClosure"), Modifier.PUBLIC, "unlink", "()V"))); - + InstructionHandle jumpOverHandler = advice.append(InstructionConstants.NOP); - + // Call unlink in finally block - InstructionHandle handlerStart = advice.append(InstructionConstants.POP); - aroundClosureInstance.appendLoad(advice, fact); + + // Do not POP the exception off, we need to rethrow it + InstructionHandle handlerStart = advice.append(aroundClosureInstance.createLoad(fact)); advice.append(Utility.createInvoke(getFactory(), getWorld(), new MemberImpl(Member.METHOD, UnresolvedType .forName("org.aspectj.runtime.internal.AroundClosure"), Modifier.PUBLIC, "unlink", "()V"))); - advice.append(InstructionConstants.ACONST_NULL); + // After that exception is on the top of the stack again advice.append(InstructionConstants.ATHROW); InstructionHandle jumpTarget = advice.append(InstructionConstants.NOP); jumpOverHandler.setInstruction(InstructionFactory.createBranchInstruction(Constants.GOTO, jumpTarget)); enclosingMethod.addExceptionHandler(tryUnlinkPosition, unlinkInsn, handlerStart, null/* ==finally */, false); - } + } } - + advice.append(returnConversionCode); if (getKind() == Shadow.MethodExecution && linenumber > 0) { advice.getStart().addTargeter(new LineNumberTag(linenumber)); @@ -3111,7 +3112,7 @@ public class BcelShadow extends Shadow { } /** - * + * * @param callbackMethod the method we will call back to when our run method gets called. * @param proceedMap A map from state position to proceed argument position. May be non covering on state position. */ @@ -3183,7 +3184,7 @@ public class BcelShadow extends Shadow { /** * Extract the instructions in the shadow to a new method. - * + * * @param extractedMethodName name for the new method * @param extractedMethodVisibilityModifier visibility modifiers for the new method * @param adviceSourceLocation source location of the advice affecting the shadow |