diff options
author | jhugunin <jhugunin> | 2004-01-14 15:24:06 +0000 |
---|---|---|
committer | jhugunin <jhugunin> | 2004-01-14 15:24:06 +0000 |
commit | 5834de97836ebcc056415736c17c46e8b1dfaf5a (patch) | |
tree | a7b87ccc35e25aafe73d24d17f08cc33f62b22c4 | |
parent | 7bbd1f419239dc9e8b46e7fd912b2bc007bbd76a (diff) | |
download | aspectj-5834de97836ebcc056415736c17c46e8b1dfaf5a.tar.gz aspectj-5834de97836ebcc056415736c17c46e8b1dfaf5a.zip |
Fix for Bugzilla Bug 44587
Erroneous exception conversion
and Bugzilla Bug 34206
before():execution(new(..)) does not throw NoAspectBoundException
All exceptions that occur during the static intialization of a persingleton
aspect will be swallowed. When using that aspect (via aspectOf())
a NoAspectBoundException will be thrown with the original exception
from the staitc initializer as the cause.
-rw-r--r-- | lib/test/aspectjrt.jar | bin | 29350 -> 29632 bytes | |||
-rw-r--r-- | org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectClinit.java | 23 | ||||
-rw-r--r-- | org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java | 15 | ||||
-rw-r--r-- | runtime/src/org/aspectj/lang/NoAspectBoundException.java | 12 | ||||
-rw-r--r-- | runtime/testsrc/RuntimeModuleTests.java | 8 | ||||
-rw-r--r-- | tests/ajcTests.xml | 14 | ||||
-rw-r--r-- | tests/bugs/ErroneousExceptionConversion.java | 44 | ||||
-rw-r--r-- | tests/bugs/ErroneousExceptionConversion1.java | 26 | ||||
-rw-r--r-- | tests/new/ConstructorExecInitFails.java | 6 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/AjcMemberMaker.java | 30 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/NameMangler.java | 4 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java | 18 |
12 files changed, 187 insertions, 13 deletions
diff --git a/lib/test/aspectjrt.jar b/lib/test/aspectjrt.jar Binary files differindex 63c73ca1a..7e4e30f0f 100644 --- a/lib/test/aspectjrt.jar +++ b/lib/test/aspectjrt.jar diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectClinit.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectClinit.java index 653f3d537..ebead22eb 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectClinit.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectClinit.java @@ -18,12 +18,16 @@ import org.aspectj.weaver.AjcMemberMaker; import org.eclipse.jdt.internal.compiler.CompilationResult; import org.eclipse.jdt.internal.compiler.ast.Clinit; import org.eclipse.jdt.internal.compiler.codegen.CodeStream; +import org.eclipse.jdt.internal.compiler.codegen.ExceptionLabel; +import org.eclipse.jdt.internal.compiler.codegen.Label; import org.eclipse.jdt.internal.compiler.lookup.ClassScope; +import org.eclipse.jdt.internal.compiler.lookup.FieldBinding; public class AspectClinit extends Clinit { private boolean hasPre, hasPost; + private FieldBinding initFailureField; - public AspectClinit(Clinit old, CompilationResult compilationResult, boolean hasPre, boolean hasPost) { + public AspectClinit(Clinit old, CompilationResult compilationResult, boolean hasPre, boolean hasPost, FieldBinding initFailureField) { super(compilationResult); this.needFreeReturn = old.needFreeReturn; this.sourceEnd = old.sourceEnd; @@ -33,12 +37,19 @@ public class AspectClinit extends Clinit { this.hasPre = hasPre; this.hasPost = hasPost; + this.initFailureField = initFailureField; } + + private ExceptionLabel handlerLabel; protected void generateSyntheticCode( ClassScope classScope, CodeStream codeStream) { + if (initFailureField != null) { + handlerLabel = new ExceptionLabel(codeStream, classScope.getJavaLangThrowable()); + } + if (hasPre) { final EclipseFactory world = EclipseFactory.fromScopeLookupEnvironment(classScope); @@ -64,6 +75,16 @@ public class AspectClinit extends Clinit { ))); } + if (initFailureField != null) { + handlerLabel.placeEnd(); + Label endLabel = new Label(codeStream); + codeStream.goto_(endLabel); + + handlerLabel.place(); + codeStream.putstatic(initFailureField); + endLabel.place(); + } + } } 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 6fd3dd5f3..a823f10b0 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 @@ -151,7 +151,7 @@ public class AspectDeclaration extends TypeDeclaration { } } - + private FieldBinding initFailureField= null; public void generateCode(ClassFile enclosingClassFile) { if (ignoreFurtherInvestigation) { @@ -170,18 +170,22 @@ public class AspectDeclaration extends TypeDeclaration { if (!isAbstract()) { + initFailureField = factory.makeFieldBinding(AjcMemberMaker.initFailureCauseField(typeX)); + binding.addField(initFailureField); + if (perClause == null) { // we've already produced an error for this } else if (perClause.getKind() == PerClause.SINGLETON) { binding.addField(factory.makeFieldBinding(AjcMemberMaker.perSingletonField( typeX))); - methods[0] = new AspectClinit((Clinit)methods[0], compilationResult, false, true); + + methods[0] = new AspectClinit((Clinit)methods[0], compilationResult, false, true, initFailureField); } else if (perClause.getKind() == PerClause.PERCFLOW) { binding.addField( factory.makeFieldBinding( AjcMemberMaker.perCflowField( typeX))); - methods[0] = new AspectClinit((Clinit)methods[0], compilationResult, true, false); + methods[0] = new AspectClinit((Clinit)methods[0], compilationResult, true, false, null); } else if (perClause.getKind() == PerClause.PEROBJECT) { // binding.addField( // world.makeFieldBinding( @@ -522,10 +526,13 @@ public class AspectDeclaration extends TypeDeclaration { codeStream.ifnull(isNull); codeStream.areturn(); isNull.place(); + codeStream.new_(world.makeTypeBinding(AjcMemberMaker.NO_ASPECT_BOUND_EXCEPTION)); codeStream.dup(); + codeStream.ldc(typeX.getNameAsIdentifier()); + codeStream.getstatic(initFailureField); codeStream.invokespecial(world.makeMethodBindingForCall( - AjcMemberMaker.noAspectBoundExceptionInit() + AjcMemberMaker.noAspectBoundExceptionInitWithCause() )); codeStream.athrow(); // body ends here diff --git a/runtime/src/org/aspectj/lang/NoAspectBoundException.java b/runtime/src/org/aspectj/lang/NoAspectBoundException.java index 4bebcfb47..b21d3895c 100644 --- a/runtime/src/org/aspectj/lang/NoAspectBoundException.java +++ b/runtime/src/org/aspectj/lang/NoAspectBoundException.java @@ -18,4 +18,16 @@ package org.aspectj.lang; * when there is no aspect of that type currently bound. */ public class NoAspectBoundException extends RuntimeException { + Throwable cause; + public NoAspectBoundException(String aspectName, Throwable inner) { + super(inner == null ? aspectName : + "Exception while initializing " +aspectName + ": " +inner); + this.cause = inner; + } + + public NoAspectBoundException() { + } + + public Throwable getCause() { return cause; } + } diff --git a/runtime/testsrc/RuntimeModuleTests.java b/runtime/testsrc/RuntimeModuleTests.java index fe36f3055..a426025c5 100644 --- a/runtime/testsrc/RuntimeModuleTests.java +++ b/runtime/testsrc/RuntimeModuleTests.java @@ -14,6 +14,8 @@ // default package +import org.aspectj.lang.NoAspectBoundException; + import junit.framework.*; public class RuntimeModuleTests extends TestCase { @@ -27,4 +29,10 @@ public class RuntimeModuleTests extends TestCase { public RuntimeModuleTests(String name) { super(name); } public void testNothing() {} + + public void testNoAspectBoundException() { + RuntimeException fun = new RuntimeException("fun"); + NoAspectBoundException nab = new NoAspectBoundException("Foo", fun); + assertEquals(fun,nab.getCause()); + } } diff --git a/tests/ajcTests.xml b/tests/ajcTests.xml index 6a68bc4a6..e07ae9507 100644 --- a/tests/ajcTests.xml +++ b/tests/ajcTests.xml @@ -7092,4 +7092,18 @@ <run class="Main1"/> </ajc-test> + <ajc-test dir="bugs" pr="44587" + title="Erroneous exception conversion"> + <compile files="ErroneousExceptionConversion.java"> + </compile> + <run class="ErroneousExceptionConversion"/> + </ajc-test> + + <ajc-test dir="bugs" pr="34206" + title="before():execution(new(..)) does not throw NoAspectBoundException"> + <compile files="ErroneousExceptionConversion1.java"> + </compile> + <run class="ErroneousExceptionConversion1"/> + </ajc-test> + </suite> diff --git a/tests/bugs/ErroneousExceptionConversion.java b/tests/bugs/ErroneousExceptionConversion.java new file mode 100644 index 000000000..13db17a18 --- /dev/null +++ b/tests/bugs/ErroneousExceptionConversion.java @@ -0,0 +1,44 @@ +// pr 44587 +import org.aspectj.testing.Tester; +import org.aspectj.lang.NoAspectBoundException; +public class ErroneousExceptionConversion { + + public static void main(String[] args) { + try { + new ErroneousExceptionConversion(); + Tester.checkFailed("Wanted an exception in initializer error"); + } catch (NoAspectBoundException nabEx) { + // good + // check nabEx.getCause instanceof RuntimeException and has explanation "boom..." + Throwable cause = nabEx.getCause(); + if (!(cause instanceof RuntimeException)) { + Tester.checkFailed("Should have a RuntimeException as cause"); + } + } catch(Throwable t) { + Tester.checkFailed("Wanted an ExceptionInInitializerError but got " + t); + } + } + + +} + +aspect A { + + int ErroneousExceptionConversion.someField = throwIt(); + + public static int throwIt() { + throw new RuntimeException("Exception during aspect initialization"); + } + + public A() { + System.err.println("boom in 5..."); + throw new RuntimeException("boom"); + } + + // if I change this to execution the test passes... + after() throwing : initialization(ErroneousExceptionConversion.new(..)) { + System.out.println("After throwing"); + } + +} + diff --git a/tests/bugs/ErroneousExceptionConversion1.java b/tests/bugs/ErroneousExceptionConversion1.java new file mode 100644 index 000000000..9a8f0e2ab --- /dev/null +++ b/tests/bugs/ErroneousExceptionConversion1.java @@ -0,0 +1,26 @@ +import org.aspectj.lang.*; +import org.aspectj.testing.Tester; + +aspect Watchcall { + pointcut myConstructor(): execution(new(..)); + + before(): myConstructor() { + System.err.println("Entering Constructor"); + } + + after(): myConstructor() { + System.err.println("Leaving Constructor"); + } +} + +public class ErroneousExceptionConversion1 { + public static void main(String[] args) { + try { + ErroneousExceptionConversion1 c = new ErroneousExceptionConversion1(); + Tester.checkFailed("shouldn't get here"); + } catch (NoAspectBoundException nab) { + // expected + } + + } +} diff --git a/tests/new/ConstructorExecInitFails.java b/tests/new/ConstructorExecInitFails.java index 4b5e775e4..e042f10ff 100644 --- a/tests/new/ConstructorExecInitFails.java +++ b/tests/new/ConstructorExecInitFails.java @@ -1,4 +1,5 @@ import org.aspectj.testing.*; +import org.aspectj.lang.*; /** * -usejavac mode: no error @@ -8,7 +9,10 @@ public class ConstructorExecInitFails { public static void main(String[] args) { try { new ConstructorExecInitFails(); - } catch (ExceptionInInitializerError e) { + } catch (NoAspectBoundException e) { + + Tester.check(e.getCause() instanceof NoAspectBoundException, + "Expected NoAspectBoundException, found " + e.getCause()); return; } Tester.checkFailed("shouldn't be able to run"); diff --git a/weaver/src/org/aspectj/weaver/AjcMemberMaker.java b/weaver/src/org/aspectj/weaver/AjcMemberMaker.java index d91d5f65c..52d6d6144 100644 --- a/weaver/src/org/aspectj/weaver/AjcMemberMaker.java +++ b/weaver/src/org/aspectj/weaver/AjcMemberMaker.java @@ -15,6 +15,8 @@ package org.aspectj.weaver; import java.lang.reflect.Modifier; +import org.aspectj.weaver.ResolvedTypeX.Name; + public class AjcMemberMaker { private static final int PUBLIC_STATIC_FINAL = @@ -60,14 +62,23 @@ public class AjcMemberMaker { public static Member noAspectBoundExceptionInit() { return new ResolvedMember( - Member.METHOD, - NO_ASPECT_BOUND_EXCEPTION, - Modifier.PUBLIC, - "<init>", - "()V"); + Member.METHOD, + NO_ASPECT_BOUND_EXCEPTION, + Modifier.PUBLIC, + "<init>", + "()V"); } + public static Member noAspectBoundExceptionInitWithCause() { + return new ResolvedMember( + Member.METHOD, + NO_ASPECT_BOUND_EXCEPTION, + Modifier.PUBLIC, + "<init>", + "(Ljava/lang/String;Ljava/lang/Throwable;)V"); + } + public static ResolvedMember perCflowPush(TypeX declaringType) { return new ResolvedMember( Member.METHOD, @@ -95,6 +106,15 @@ public class AjcMemberMaker { declaringType.getSignature()); } + + public static ResolvedMember initFailureCauseField(TypeX declaringType) { + return new ResolvedMember( + Member.FIELD, + declaringType, + PRIVATE_STATIC, + NameMangler.INITFAILURECAUSE_FIELD_NAME, + TypeX.THROWABLE.getSignature()); + } public static ResolvedMember perObjectField(TypeX declaringType, ResolvedTypeX aspectType) { int modifiers = Modifier.PRIVATE; diff --git a/weaver/src/org/aspectj/weaver/NameMangler.java b/weaver/src/org/aspectj/weaver/NameMangler.java index d620ecad8..7db5107ce 100644 --- a/weaver/src/org/aspectj/weaver/NameMangler.java +++ b/weaver/src/org/aspectj/weaver/NameMangler.java @@ -40,8 +40,8 @@ public class NameMangler { public static final String AJC_POST_CLINIT_NAME = PREFIX + "postClinit"; - - + public static final String INITFAILURECAUSE_FIELD_NAME = PREFIX + "initFailureCause"; + public static String perObjectInterfaceGet(TypeX aspectType) { return makeName(aspectType.getNameAsIdentifier(), "perObjectGet"); } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java index fde2d9d0e..8ef653f87 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java @@ -904,6 +904,8 @@ class BcelClassWeaver implements IClassWeaver { // assert t.getHandler() == ih ExceptionRange er = (ExceptionRange) t; if (er.getCatchType() == null) continue; + if (isInitFailureHandler(ih)) return; + match( BcelShadow.makeExceptionHandler( world, @@ -915,6 +917,14 @@ class BcelClassWeaver implements IClassWeaver { } } + private boolean isInitFailureHandler(InstructionHandle ih) { + if (ih.getInstruction() instanceof PUTSTATIC) { + String name = ((PUTSTATIC)ih.getInstruction()).getFieldName(cpg); + if (name.equals(NameMangler.INITFAILURECAUSE_FIELD_NAME)) return true; + } + return false; + } + private void matchSetInstruction( LazyMethodGen mg, @@ -923,6 +933,10 @@ class BcelClassWeaver implements IClassWeaver { List shadowAccumulator) { FieldInstruction fi = (FieldInstruction) ih.getInstruction(); Member field = BcelWorld.makeFieldSignature(clazz, fi); + + // synthetic fields are never join points + if (field.getName().startsWith(NameMangler.PREFIX)) return; + ResolvedMember resolvedField = field.resolve(world); if (resolvedField == null) { // we can't find the field, so it's not a join point. @@ -946,6 +960,10 @@ class BcelClassWeaver implements IClassWeaver { private void matchGetInstruction(LazyMethodGen mg, InstructionHandle ih, BcelShadow enclosingShadow, List shadowAccumulator) { FieldInstruction fi = (FieldInstruction) ih.getInstruction(); Member field = BcelWorld.makeFieldSignature(clazz, fi); + + // synthetic fields are never join points + if (field.getName().startsWith(NameMangler.PREFIX)) return; + ResolvedMember resolvedField = field.resolve(world); if (resolvedField == null) { // we can't find the field, so it's not a join point. |