Community
Participate
Working Groups
Get ---- java.lang.VerifyError: (class: bugs/DoubleAround, method: one signature: ()V) Unable to pop operand off an empty stack ---- running ---- public class DoubleAround { public static void main(String[] args) { DoubleAround me = new DoubleAround(); me.one(); // me.two(); } void one() {} //void two() {} static abstract aspect Super { int count = 0; pointcut pc(DoubleAround s) : this(s) && execution(void one()); void around(DoubleAround s) : pc(s) { proceed(s); System.out.println(thisJoinPoint + " count: " + count++ + " by " + this + " of " + s); } } static aspect Sub extends Super { // no VerifyError if this is commented out pointcut pc(DoubleAround s) : Super.pc(s) || (this(s) && execution(void two())); } } ----
*** Bug 113448 has been marked as a duplicate of this bug. ***
Created attachment 28781 [details] patch containing simplified testcase Apply this patch to the tests project. Running this test with the PointcutRewriter.WATCH_PROGRESS flag set to true gives the following output: Initial pointcut is ==> (persingleton(Super) && ((this(BindingTypePattern(PR113447, 0)) && execution(void method1())) || (this(BindingTypePattern(PR113447, 0)) && execution(void method2())))) Distributing NOT gives ==> (persingleton(Super) && ((this(BindingTypePattern(PR113447, 0)) && execution(void method1())) || (this(BindingTypePattern(PR113447, 0)) && execution(void method2())))) Pull up disjunctions gives ==> ((persingleton(Super) && (this(BindingTypePattern(PR113447, 0)) && execution(void method1()))) || (persingleton(Super) && (this(BindingTypePattern(PR113447, 0)) && execution(void method2())))) Simplifying ANDs gives ==> (((execution(void method1()) && this(BindingTypePattern(PR113447, 0))) && persingleton(Super)) || ((execution(void method2()) && this(BindingTypePattern(PR113447, 0))) && persingleton(Super))) Sorting ORs gives ==> (((execution(void method1()) && this(BindingTypePattern(PR113447, 0))) && persingleton(Super)) || ((execution(void method2()) && this(BindingTypePattern(PR113447, 0))) && persingleton(Super)))
Comparing the decompiled output of two methods where one is matched by the pointcut: pointcut pc2(PR113447 s): this(s) && execution(void method3()); (which runs fine) and the other is matched by the pointcut: pointcut pc(PR113447 s) : (this(s) && execution(void method1())) || (this(s) && execution(void method2())); (which causes the verify error) the output for method3 is 0: invokestatic #34; //Method Super.aspectOf:()LSuper; 3: aload_0 <<--- extra line 4: invokevirtual #41; //Method Super.ajc$before$Super$1$c4a93b3f:(LPR113447 ;)V 7: return LocalVariableTable: Start Length Slot Name Signature 7 0 0 this LPR113447; whereas the output for method1() is: 0: invokestatic #34; //Method Super.aspectOf:()LSuper; 3: invokevirtual #38; //Method Super.ajc$before$Super$2$657ed6b:(LPR11344 7;)V 6: return LocalVariableTable: Start Length Slot Name Signature 6 0 0 this LPR113447; It therefore looks like in the case where there are two this pcd's, "this" isn't being put on the stack.
The problem is that in the case of method1 (see comment #3), the first time we enter ThisOrTargetPointcut.findResidueInternal(..) the test (state.get(btp.getFormalIndex())!=null) && (lastMatchedShadowId != shadow.shadowId) is false. However, the second time we enter (for the second "this" pcd) the test is true and we record that an error has been marked against it (although no error is printed or logged). Then, in BcelAdvice.getAdviceArgSetup(..) we check whether an error has been marked against it and continue without doing anything if it has. Consequently, "this" is not put on the stack.
Created attachment 28840 [details] zip file containing updated testcase patch and patch containing fix The attached zip contains two patches: - pr113447-tests-patch2.txt - apply this to the tests project - pr113447-weaver-patch.txt - apply this to the weaver project The fix involved changing the logic mentioned in comment #4 to first check whether the formal has been bound and that we're looking at a different object to that which did the binding. If this is true then we further check whether this pointcut was involved in the matching. If it wasn't then we return "no residue", otherwise we mark this pointcut as having reported an error against it.
The following program: @Retention(RetentionPolicy.RUNTIME) @interface Annotation{}; @Annotation public class PR113447a { public static void main(String[] args) { PR113447a me = new PR113447a(); me.method1(); me.method3(); } public void method1(){} public void method3(){} } aspect Super { pointcut pc1(Annotation a) : (@this(a) && execution(void method1())) || (@this(a) && execution(void method2())); before(Annotation a) : pc1(a) { } } also fails with the same VerifyError. The fix therefore needs to also be applied to the ThisOrTargetAnnotationPointcut class.
first patch integrated.
A summary of progress/investigation so far: The verifyErrors are coming from calls to ExposedState.setErroneousVar(Var) and there are 6 places where this called: - ThisOrTargetPointcut.findResidueInternal(Shadow, ExposedState) - ThisOrTargetAnnotationPointcut.findResidueInternal(Shadow, ExposedState) - WithinAnnotationPointcut.findResidueInternal(Shadow, ExposedState) - WithinCodeAnnotationPointcut.findResidueInternal(Shadow, ExposedState) - AnnotationPointcut.findResidueInternal(Shadow, ExposedState) - ArgsPointcut.findResidueInternal(Shadow, ExposedState) Looking at each of these separately: ThisOrTargetPointcut -------------------- The call to setErroneousVar(..) is reached when you have a pointcut which contains two this statements, both bound to the same thing i.e. (this(s) && call(void myMethod1()) || (this(s) && call(void myMethod2()) Here, when we enter findResidueInternal for the myMethod1 shadow the LHS has matched and we haven't been here before, so everything's ok. Then when we enter again for myMethod1 but with the "this" from the RHS, we have previously found a "this", however, with a different object. By checking whether the field "lastMatchedShadowId" is equal to the id of the myMethod1 shadow we can tell if the current "this" was involved in the matching of myMethod1. In the example above it isn't so we return without doing anything. The only way we would mark the current "this" (or "target") to be "erroneous" is if we're not looking at the same object (as in the case when a pointcut is used in multiple advice) and it was used in the matching of the given shadow. This fix has been checked in in comment #7. ArgsPointcut ------------ This is similar to ThisOrTargetPointcut in that the call to setErroneousVar(..) can be reached through pointcuts like (args(i) && call(void method1(int))) || (args(i) && call(void method2(int))) However, I think the fix should be slightly different because the information which is available at this point is slightly different than the ThisOrTargetPointcut case. I haven't had a chance to look into this in much detail yet. ThisOrTargetAnnotationPointcut ------------------------------ The call to setErroneousVar(..) is reached when you have a pointcut which contains two @this statements i.e. (@this(a) && execution(void myMethod1())) || (@this(a) && execution(void myMethod2())). The reasoning here is similar to the ThisOrTargetPointcut case, apart from the fact that we should return if the "lastMatchedShadowId" field is equal to the id of the myMethod1 shadow id (rather than vice versa). The reason for this being that we match this pointcut again at the beginning of the findResidueInternal method and as expected, it does match. WithinAnnotationPointcut ------------------------ The call to setErroneousVar(..) is currently reached if you have two @within pointcuts which both have the "lastMatchedShadowId" equal to the current shadow id. The following pointcut doesn't cause any problems (@within(a) && execution(void myMethod1())) || (@within(a) && execution(void myMethod2())). because on the second time round the "lastMatchedShadowId" doesn't equal the current shadow id. However, a pointcut of the form @within(a) && (call(void method4(int)) || call(void method5(int))); does cause a verify error (different to the one reported in this bug - (class: PR113447b, method: main signature: ([Ljava/lang/String;)V) Expecting to find object/array on stack). In this case the two objects are the same and so, if the fix for ThisOrTargetPointcut is applied to WithinAnnotationPointcut the object equality check fixes this problem. I'm not sure how much sense it is to check whether "lastMatchedShadowId" equals the current shadow id though....... WithincodeAnnotationPointcut ---------------------------- This is the same as WithinAnnotationPointcut - the call to setErroneousVar(..) is reached if you have a pointcut of the form: @withincode(a) && (call(void method4(int)) || call(void method5(int))); when again the two objects are the same. Also, a pointcut of the form: (@withincode(a) && execution(void myMethod1())) || (@withincode(a) && execution(void myMethod2())). doesn't cause a verify error because in this case the lastMatchedShadowId isn't equal to the current shadow id. AnnotationPointcut ------------------ This is the same as WithinAnnotationPointcut and WithincodeAnnotationPointcut. The call to setErroneousVar(..) is reached if you have a pointcut of the form: @annotation(a) && (call(void method4(int)) || call(void method5(int))); when the two objects are the same.Again, pointcuts of the form: (@annotation(a) && call(void method4(int))) || (@annotation(a) && call(void method5(int))) do not enter this code because the lastMatchedShadowId isn't equal to the current shadow id.
Created attachment 28922 [details] patch containing testcases Apply the patch to the tests project. This contains testcases for all 6 situations detailed in the above comment (note that because there are now more tests for this bug, the patch also moves the checked in test to the same directory to keep all the test classes together)
I've fixed this based on Helens investigations and also applied her testcases. The problem is that when the rewriting was put in, we added code to check for ambiguous bindings across a pointcut and since this was added there was no longer a need to check it at every single shadow. As discussed, checking it at every shadow is problematic. So, I removed all the messy code to check it at each shadow. All the tests pass - even those that verify ambiguous binding messages come out correctly. waiting on build before closing.
full fix available.
hooray - that's great news...
