Community
Participate
Working Groups
Created attachment 254594 [details] Zip containing all files to reproduce the issue Hi :-) I have found an issue during weaving, which can be reproduced by the attached files. The weaved class after running ajc has some corrupted Local Variable Tables, where some slots are missing. When opening the weaved class - com/crashlytics/android/v.class - and looking at it using Sublime for example, I could see the following: LocalVariableTable: Start Length Slot Name Signature 0 154 0 arg0 Lcom/crashlytics/android/internal/aq; 0 154 1 arg1 I 0 154 2 arg2 J 0 154 4 arg3 Ljava/lang/String; As seen above, Slot 3 is missing... This issue causes some problems with Android's build framework. To reproduce the problem please do the following: - Unzip the attached - Define a ASPECTJ_HOME env variable, or simply edit the run.cmd file - Execute run.cmd The result would be ajc weaving the class v.class, using FilesApsect.aj found under the src dir. And the weaved jar will contain a v.class with the problem. Thanks! Ariel
Created attachment 254595 [details] The class with the issue after weaving Adding also the outcome of the weaving where the issue can be seen.
What precisely is telling you that the local variable table is corrupted? What is the exact error? The type of the third argument in your method is 'J' which is a 'long' which means it is a double size value, hence it takes up both slots 2 and 3. If I take this: public class Code { public static void main(String []argv) { } public void m(int i, long j, boolean z) { System.out.println(i); System.out.println(j); System.out.println(z); } } and 'javac -g Code.java' then 'javap -verbose Code' then I see: LocalVariableTable: Start Length Slot Name Signature 0 23 0 this LCode; 0 23 1 i I 0 23 2 j J 0 23 4 z Z The 'J' is taking up two slots ('D' for double would act in the same way) What *is* different between your inpath and outjar is that your inpath code does not have local variable tables for these methods, where AspectJ is trying to 'reconstruct them' during weaving. Maybe from the error message I can understand what it is complaining about.
Thank you for looking at this issue. I probably misinterpreted the issue. The problem arises while trying to convert the out.jar into a dex file by the Android Dexer. The error is as follows: EXCEPTION FROM SIMULATION: local variable type mismatch: attempt to set or access a value of type java.lang.Object using a local variable of type int. This is symptomatic of .class transformation tools that ignore local variable information. ...at bytecode offset 0000006a locals[0000]: Lcom/crashlytics/android/internal/aq; locals[0001]: I locals[0002]: J locals[0003]: <invalid> locals[0004]: Ljava/lang/String; stack[top0]: [B ...while working on block 006a ...while working on method a:(Lcom/crashlytics/android/internal/aq;IJLjava/lang/String;)V ...while processing a (Lcom/crashlytics/android/internal/aq;IJLjava/lang/String;)V ...while processing com/crashlytics/android/v.class It is actually complaining about different variable types, and I misconcluded that it was related to the slot numbers. For your convenience I will attach an updated zip file, together with the Android Dexer and a script that will run ajc and immediately after run dx for easy simulation of the issue. Thanks! Ariel
Created attachment 254650 [details] Zip containing all files and also Android Dexer to reproduce the issue
Sorry I'm slow to look at this. So it seems the input code that is being passed in is a bit 'naughty'. it is a static method declared like this: static void a(com.crashlytics.android.internal.aq, int, long, java.lang.String); which should mean there are 4 local variables and they'd take up slots 0,1,2,4 (this is a static method so we don't store 'this' in 0, there is no slot 3 because the third variable is a double size one). That means anytime the code in the method does an 'aload 1' it would be grabbing an int (for code generated by a regular compiler) However, the byte code at offset 106 in the method does this: 103: invokevirtual #428 // Method java/lang/String.getBytes:(Ljava/lang/String;)[B 106: astore_1 The invokevirtual leaves a byte[] on the stack and that is stored into 1 - where we'd probably expect to find an 'int' if you strictly obey method specification. Now you can get away with this kind of thing if generating byte code but you couldn't write the source to do it: static void a(com.crashlytics.android.internal.aq arg0, int arg1, long arg2, java.lang.String arg3) { arg1 = "abc".getBytes(); // NOT ALLOWED } You can get away with it in generated code if you know no other code is going to try and find an int in there. And that seems to be what is happening here. No one is accessing 1 so it is used as a free variable to hold the byte array. Because the method is not marked synthetic AspectJ is adding a local variable table (we have had other situations where byte code transformers complain if there *isnt* a local variable table for non synthetic methods). The very simplest thing I can do is a flag to say this kind of thing is happening and not to fix up a local variable table if the incoming methods don't have one.
New option added: -Xset:generateNewLocalVariableTables=false for these kinds of scenario. I can't make it the default because other scenarios do want local variable tables. Analysing the byte code to see if it is doing strange things like this would be too costly to do in all situations, given that this is the less likely case. I've tried the option and it works here, the dexer runs OK.
Thank you! Is the change expected to come out in AspectJ 1.8.7? Is there already a schedule for this release? Thanks again, Ariel
I am loosely planning 1.8.7 for the next few weeks (up to 4). If you want to try it out in the interim I just uploaded a dev build for you, see the AspectJ download page.
Hi Andrew, Thank you so much for handling this problem. I downloaded the development version and tried it, and it indeed solves the problem! :-) However: 1. There was an issue with aspectjrt - Android's Dex generates the following warning: Unknown source file: warning: Ignoring InnerClasses attribute for an anonymous inner class (org.aspectj.runtime.internal.cflowstack.ThreadStackFactoryImpl$1) that doesn't come with an associated EnclosingMethod attribute. This class was probably produced by a compiler that did not target the modern .class file format. The recommended solution is to recompile the class from source, using an up-to-date compiler and without specifying any "-target" type options. The consequence of ignoring this warning is that reflective operations on this class will incorrectly indicate that it is *not* an inner class. I therefore grabbed aspectjrt from version 1.8.6, and everything worked fine. 2. I tried looking for the new option by running ajc with -X, but the new option wasn't listed there. You probably didn't yet add it to the command line help. If you generate another development build - I'll be more than happy to re-run the tests with it. Thank you! Ariel
Hi again :-) I ran into an issue which resembles the above, but a bit different. This time it seems that ajc modifies the existing local variable table, and that causes an exception in Android's dexer. From what I see, it seems ajc changes the length of the local variables, and I think that due to this change the dexer again falls onto an illegal astore_1 which tries to store an object into a long. I'm attaching a zip with a run command to simulate the problem. Also, in the folder "Weaved versus Original" you can find the original class and the one after weaving and see the difference. Am I correct in my analysis, and if yes - do you have an idea how this can be solved? Thanks! Ariel
Created attachment 256177 [details] Zip to reproduce problem with modification of existing local variable table
Yes, AspectJ can modify local variable tables. I can imagine that it would extend the range over one of these rogue instructions that is sticking something into an object that is considered to be a local variable slot. Supporting this is a bit trickier, thanks for attaching the scenario. I'll try to get to it and get something into 1.8.7 (due before SpringOne which is in September).
Hi Andrew, Any news about this issue? As far as I could see this didn't get into 1.8.7, or am I wrong? Thanks, Ariel
Sorry for my slow reply. It didn't make it into 1.8.7 and still hasn't been addressed.
I extended the meaning of the flag we are using here to also avoid expanding the range of local variables that represent method parameters. Without expanding the range the run of dx.jar runs fine for me on the woven output.
