From 5834de97836ebcc056415736c17c46e8b1dfaf5a Mon Sep 17 00:00:00 2001 From: jhugunin Date: Wed, 14 Jan 2004 15:24:06 +0000 Subject: [PATCH] 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. --- lib/test/aspectjrt.jar | Bin 29350 -> 29632 bytes .../internal/compiler/ast/AspectClinit.java | 23 ++++++++- .../compiler/ast/AspectDeclaration.java | 15 ++++-- .../aspectj/lang/NoAspectBoundException.java | 12 +++++ runtime/testsrc/RuntimeModuleTests.java | 8 ++++ tests/ajcTests.xml | 14 ++++++ tests/bugs/ErroneousExceptionConversion.java | 44 ++++++++++++++++++ tests/bugs/ErroneousExceptionConversion1.java | 26 +++++++++++ tests/new/ConstructorExecInitFails.java | 6 ++- .../org/aspectj/weaver/AjcMemberMaker.java | 30 ++++++++++-- .../src/org/aspectj/weaver/NameMangler.java | 4 +- .../aspectj/weaver/bcel/BcelClassWeaver.java | 18 +++++++ 12 files changed, 187 insertions(+), 13 deletions(-) create mode 100644 tests/bugs/ErroneousExceptionConversion.java create mode 100644 tests/bugs/ErroneousExceptionConversion1.java diff --git a/lib/test/aspectjrt.jar b/lib/test/aspectjrt.jar index 63c73ca1a07757a48b2402ce7d7b62151a7ad17c..7e4e30f0fd891d8cd2a206483539167aa71828dc 100644 GIT binary patch delta 2263 zcmZWp2~-nT6rD_10wh2nkU)e)Sp*>j6-1ClTv-IE0tFRC+1CVIDQHAJDj*j5RX}Qs zic4DsF?FdPj$o?RoyA=Q4{Jf(XsLil+dpq?F*G@o_vilm-hJ=q&K&Jx)b%hpA%RQ{ zT>?JgPJ1}kHXp*AoauY|R|5xGW`cB-h#7mrCo-w9*%3mu3zNXv-BKzb0b@GE#SAxy zg^Xs3rHmm%8L+s(tRbldYSf#6inPWl3KvJojC^@ss)1jsuB=KrA~}6Ong8hWR@tfr zsfQP87(XobAB` zGgXuO>{}kzhXklXN5qBN0Y0JaAv<#Co3&hyiQBtqdO-auzcpFTChXzvqDviv-1Q@a zia&CDmJi(Va}gBZt+e{k!!~(g-hS&{WwY$WN&9nV?Z?u;_EQG+?hJQ+#d^K0OOP3K z?&kcoSN>hnEg1^&F?QF{7lSWyJh)K`LCx6~{=w{at>D*P@2g6On${*ddTlu~G~GI? zY?mq8ZRM$TdlB=<`^!Fs)1KAbP&^LvSKb(ZYE)Pz^E( zkfGSoX`AIeY~H)%%E!FWL92ka`qn$#n@LlD{prsxBkO*%;;EJ0eqZ)i|CVz!Uyy0- zcDc_Y!miebuL$lTdUr4_!0phx-f6 z3Fwg_=qQ`HrL_IijqlNmxh8!KE}l{Cd7F%UZN$oG{%Hn0JdJu5D8z<~qWPLAJylP+ z*vwvw+E6M)HJZS%3${VgsX9oUW`tJT3<5UJPl9sN%#<~@qy_*bUJ}%nW~pR2^plYO zM$!%oMC}|;w^WKJ&tt+)l1nZWR=#vUMk6I&d+CMLR|N}X7c4;s^{FEsH5lrT4}vYv z1R;5rh3Yv%FxE(-k(gq}&)#ESi(h(tY{#jezX^#mIzVnnu!a0#oZqYq&>J1&;v_bt z`!oz~+ltfb@THzUq6gf9L|Y&R>kJH3U{VLZB={Qrw6alS;tbF*ND`8dIgm$rb7_(h zz;X;<(#NnfmSs_0U!1a_0{JGGk|hQhwg6ZDJJHfRtn*E{yckcUuC&h(N=GM)p`}RI z)<<_SQG$u$(SGns#l=Kp>}3Ni2rV=w3(4n3fDUqvktBg0zgPC4G9Rb+kpv-9E0N7Z zzI;;@p2DX`ND-2YOfe)vCsITJ-NBHL07FxeWh!kNNve>XV2&Yc?0&u&8{b*CHliXz zlOQVe3BpNDVV4*a#i&YT=ok{nAbAA+7NL^LAB2=W#bh7cnp7=mrY&Nmd4YF)(}d)P ziKsr!50TXHOvD1a*2Pb?hLY7XZNFk$pPs4EgWPpE2 zw})~r7jo<+xc5vhLD;L6TjvV8hB7l~#Ze|CM?3&B_YnM7ayN!6y#c;1n*=3$lQ*tg zg5R34EQ0V>E9p54hzH~xdHxsbH?2WUi2AH_GN z?5)qRP%jp8RTCNv0Af@IP0T9RjcC^+h)A=sBCZS3%1jXo4>qDEf{to$Cz@==9l7IE maZ#(fW+{*@Gp&GlDEsvCMfhK+V~;g)_;O72(PG9?>Hh$H_vaM= delta 1886 zcmZvbdrVVz6vuCG>9xJyzG!*%0#;{HW})c7$E+APWsow#=zM@OR9Y;Rhb42-A;RV# zV$dnaO+g%(1rv-;&5eo9Il4@=s33D&=CX}7?&)hr-sdv`0%#(O<^ z^Dk*!_6rer54>%+#Wi;~3N4ur=Uso)(5>q1u`PQvqhZ7KN3{=k6o0g^)?2jAH_u-- zC&yF!LB?~+B~`WF%?td83X`(C($~8;E^BZ%I0d_KxKX$<*U}rbs;;OOL@sJs($u_< z-&2xbQp|3&M8wt`)}-w$QQo`%Sq;Z6K5|=V6zhc~ZoRdKi#D$eF-=Z`N@x|97vKKA z_%$b!?;53kqw(Ce5NT|lf+1^Vgqcy3uiSd*)g{A@HBsOacn~xa_J?;QKuE2HTbjGr z$q+xDc5-N=!)qZ2_768x7IS50NY;KFc7Z{63fGr(Uk_S4^y~YF&Pf4-7B80ijTY+M zj@(O~t>&+&^Lq3R>g2|TsN=Ada)Z|;K$Sub$2=MsC^t%1t$r15=(B7D@={F_heu#) zP71tV5hq=oz(%0rcxH<9WD>8G7ZUHdMh8Qt@l#4DeJ7)y-d10jLaNijyH&iDJ98D? zJ)Zd>?tnfkMXJox3mCP&@W-KXw1J~q11;4C z=|E8htu4}xLk>A^I^}P<>2#lPJF$TK?zw1ZdS;{DB~u zm;qg00fk+UPrTVn@H8T{hY3$ z35X0jiyQ^4F^9xCAaCQAyhe-s;Mckh17MKL=B&ppHfiFkJds(Wi-xALfk~HUCA(qMlsBb zVT}l;p*VgD-i+zpsTA4e>4=YIXrC$d6F7T3o7^>#+?5`7S7-(vqOUUz3@88ar4h01 znZVm-p?Dy3B(Pzf$c~$X*qNn;rR&CGJ~agYK6enC;KDlEs&5I_XA^9Nzja!8P!)}8 zr(a~h%OQB0+$uzE1c~!uLEhwH^q8Q60)ocC(ZU$WtES4LYLRuWK-^Mo<*u)m>-!I& CcNX#h 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 @@ + + + + + + + + + + + + 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, - "", - "()V"); + Member.METHOD, + NO_ASPECT_BOUND_EXCEPTION, + Modifier.PUBLIC, + "", + "()V"); } + public static Member noAspectBoundExceptionInitWithCause() { + return new ResolvedMember( + Member.METHOD, + NO_ASPECT_BOUND_EXCEPTION, + Modifier.PUBLIC, + "", + "(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. -- 2.39.5