Community
Participate
Working Groups
Given an @AspectJ aspect that extends from some super class, for example: class ASuperClass { protected void takesAPjp(ProceedingJoinPoint pjp) { //... } } @Aspect public class AroundAdvicePassingPjpAsArgToSuper extends ASuperClass { @Around("execution(* foo())") public Object passesPjp(ProceedingJoinPoint pjp) throws Throwable { takesAPjp(pjp); Object ret = pjp.proceed(); return ret; } } The resulting compiled class gives a verify error on the generated ajc$inlineAccessMethod. Overriding the method inside the aspect (and calling super within the overriden method) works just fine.
see bugs152/pr139749 for test case to reproduce
additional info... the problem only happens when the super-method is not public, thus forcing us to create the inline accessor. In the generated bytecode (extracted into a static method), we use the invokespecial instruction to call the super-type method, this instruction is not appropriate in the extracted context. BcelAccessForInlineMunger has explicit handling for this case in the openAroundAdvice method (L168 "specific handling for super.foo() calls, where foo is non-public"). However, when compiling from @AspectJ, we never meet the guard condition for the special case logic (callee class name equals aspect supertype name). When compiling from regular AJ we *do* meet the guard. Forcing the execution down this branch does indeed produce correct code. So the question becomes, why is it that in the @AspectJ style the callee type name is returning as the sub-aspect name, not the super-type name that would be required to execute the logic. I suspect a visitor in the AJ front-end compiler fixes this up... going to look.
OK, as suspected, when we compile an AdviceDeclaration we run an AccessForInlineVisitor over it. One of the things this does is fix up MessageSends where send.isSuperAccess() is true. The binding is replaced with a getSuperAccessMethod that is generated, and the receiver is replaced with "this". So in fact we never go down the special case route in BcelAccessForInline here either, because it is all handled for us upfront before we get there...
... and with this information the problem is easily fixed by improving the test in BcelAccessForInlineMunger.openAroundAdvice. The old test compared the aspect supertype name against the class name of the invoke instruction target. This was failing to match, because we have an invokespecial with a target of the sub-aspect. Changing the test to match on the condition we really care about (does the aspect supertype name match the declaring type of the resolved member we are about to dispatch to) cause everything to work smoothly. This test will work for a super method in an immediate parent only. After verifying this change doesn't cause any test suite failures, I need to add another test with a three-level deep hierarchy and update the test to check whether the declaring type of the resolved member is an ancestor of the aspect type... this should be the correct long term fix.
Even better news (and something I should have noticed to start with - but hey, I was doing this at 6am in the morning!), the BcelAccessForInlineMunger is *only* used for @AspectJ aspects. Which means two things: 1) it's very plausable there's a bug here which hasn't been exercised before (would have been much less likely in AJ style code paths) 2) the fix is localized to only affect @AspectJ declared advice I fear I'm turning into Andy Clement with all this commentary... ;)
Test failure... excellent (!). The test suite already had the super-super method case in it. Just not the direct super case that I stumbled upon. Changed the test to check for (!aspectType.equals(memberType) && memberType.isAssignableFrom(aspectType)). All tests now pass. Fix commited in tree.
Fix now available in latest dev build
Changing OS from Mac OS to Mac OS X as per bug 185991
