Community
Participate
Working Groups
Build ID: M20070212-1330 Steps To Reproduce: I'll attach the complete test program once I enter the bug report here. In a nutshell, the static method "method_aroundBody1$advice" is introduced in my class where the aspect should be applied. Problem is that the LocalVariableTable code attributes are wrong for this method. It looks like they have been copied from the aspect class and have not been updated for the new method paramters and the changes in the slots where the original locals are stored. The woven aspect method has this as it's LocalVaraibleTable: LocalVariableTable: Start Length Slot Name Signature 0 87 0 this Ltest/ServiceInterceptor; 0 87 1 pjp Lorg/aspectj/lang/ProceedingJoinPoint; 9 78 2 args [Ljava/lang/Object; 21 66 3 id J Problem is that the method has a bunch of new paramters to it, it no longer an instance method, and the locals "args" and "id" are not stored in the slots indicated. More information: AJDT Version: 1.4.1.200611230655 AspectJ version: 1.5.3.200611221118
Created attachment 72409 [details] A small test case demostrating the problem In the attached file, look at the test.Service class after the aspects are woven into it. The LocalVariableTable for the method "method_aroundBody1$advice" is not correct (it has not been adjusted by the ajc)
This glitch makes it difficult to match method parameter names on those methods using debug info about param names. I think Spring framework is using such strategy in some cases.
A number of cases to look at here: - more than the aroundBody1 method is incorrect, the aroundBody0 method is also incorrect - whether the advice uses: this, target, this & target, args binding, thisJoinPoint, thisEnclosingJoinPoint or ProceedingJoinPoint all affect the ordering - annotation and code style aspects exercise different code paths == a few testcases.
Created attachment 119960 [details] patch to fix this The attached patch modifies the weaver to create beautiful local variable tables. But currently it is not 'free' - needs a bit of work to reduce the cost of getting the tables right.
Would it be possible to provide this as an option that can be switched on and off? Also, are you talking about compile time cost or runtime cost?
i considered it, but really it should be done properly and thats that. No point in creating wrong local variable tables so work would even be involved in the OFF case to delete the (wrong) tables that are there.
do let me know if this is breaking the eclipse debugger due to the incorrect entries, that makes it more critical.
gradually integrating this - but complex series of changes in that patch (and fool that I am, I didn't make sure all the tests passed before I put the patch here, DOH).
Ok - it seems mostly working apart from two situations: The newarray joinpoint (only accessible if using a -X switch) has a parameter that is the dimensions for the array. We don't assign a name to this parameter right now (we would do that in ResolvedType.lookupsyntheticmember() where it deals with ArrayConstructionJoinpoints). around advice applying to ITDs. A mangled member that exists due to an ITD has an effective signature attribute that describes the actual signature of the join point that the ITD would represent. This effective signature knows nothing about parameter names. That information could be copied across from the member when the effective one is created, but a better solution would be for an effective signature object to go and fetch the names from the real one if it is ever asked. However, I might commit what is working so far as it will cover many cases.
Decided to try and address it for ITDs. The problem is definetly all the infrastructure methods created to manage a single ITD. By changing BcelClassWeaver.matchInvokeInstruction() we can ensure that the effective signature member used to create a method gets some parameter names. Unfortunately it is getting them from the interMethodDispatcher - and that never got them set correctly when constructed. The interMethodDispatcher for the itd 'public I.foo(String s,int i,String[] ss)' is called: 'ajc$interMethodDispatch1$X$I$foo' and is created in the aspect. The local variables for this can be setup during compilation in the InterTypeMethodDeclaration.generateDispatchMethod() code. It is quite straightforward because they should match those declared on the ITD (itself represented as the member 'ajc$interMethod$X$I$foo' in the aspect. There is a bit of trickery to ensure the JDT compiler does not throw out these seemingly unused locals, but once that is done we can see them clearly in the classfile. Here is the declaration: aspect X { public void I.foo(String s,int i,String[] ss) {} } and here are the two generated support methods in the aspect: public static void ajc$interMethod$X$I$foo(I, java.lang.String, int, java.lang.String[]); Code: Stack=0, Locals=4, Args_size=4 0: return LocalVariableTable: Start Length Slot Name Signature 0 0 0 ajc$this_ LI; 0 0 1 s Ljava/lang/String; 0 0 2 i I 0 0 3 ss [Ljava/lang/String; public static void ajc$interMethodDispatch1$X$I$foo(I, java.lang.String, int, java.lang.String[]); Code: Stack=4, Locals=4, Args_size=4 0: aload_0 1: aload_1 2: iload_2 3: aload_3 4: invokeinterface #69, 4; //InterfaceMethod I.foo:(Ljava/lang/String;I[Ljava/lang/String;)V 9: return LocalVariableTable: Start Length Slot Name Signature 0 10 0 ajc$this_ LI; 0 10 1 s Ljava/lang/String; 0 10 2 i I 0 10 3 ss [Ljava/lang/String; Once the dispatch method has the right local variables, they can be copied to the effective signature member created for matching purposes. Once they are correctly recorded there, the around advice against the ITD will get them. void around(): call(* foo(..)) {} producing: private static final void foo_aroundBody1$advice(C, I, java.lang.String, int, java.lang.String[], X, org.aspectj.runtime.internal.AroundClosure); Synthetic: true Code: Stack=0, Locals=7, Args_size=7 0: return LocalVariableTable: Start Length Slot Name Signature 0 0 0 this LC; 0 0 1 target LI; 0 0 2 s Ljava/lang/String; 0 0 3 i I 0 0 4 ss [Ljava/lang/String; 0 0 5 otherThis LX; 0 0 6 ajc_aroundClosure Lorg/aspectj/runtime/internal/AroundClosure; running tests now...
Ok. Resolved. here is the local variable table for the method_aroundBody1$advice created based on the code initially attached to the bug report: LocalVariableTable: Start Length Slot Name Signature 0 86 0 this Ltest/Service; 0 86 1 l J 0 86 3 thisJoinPoint Lorg/aspectj/lang/JoinPoint; 0 86 4 ajc$aspectInstance Ltest/ServiceInterceptor; 0 86 5 pjp Lorg/aspectj/lang/ProceedingJoinPoint; 9 77 6 args [Ljava/lang/Object; 21 65 7 id J It is also fixed for around advice on ITDs and newarray joinpoints.
