Re: [asm] Problem instrumenting EMMA-instrumented code

[Eugene Kuleshov <eu@xxxxxxxx>]

 I am lost. Can you please specify what is the order of 
instrumentation? If you think there is a bug, please open an issue in 
the tracker at http://forge.objectweb.org/tracker/?group_id=23  and also 
attach all the bytecode up there.

 Also note that ASM does not check the validity of the input bytecode 
structures. So, it is possible that retransforming something half-broken 
will make verifier to fail (i.e. when left over have references on 
absolute constant pool indexe, which ASM can shuffle). You can tell ASM 
to copy constant pool as is to the result class by passing ClassReader 
into ClassWriter constructor.

 regards,
 Eugene


Luke Blanshard wrote:
> Hi ASMers,
>
>  I ran into a strange problem with ASM 3.0 that I thought you should know
> about.
>
>  I'm developing a tool that uses ASM 3.0 to replace all or part of a
> class during a unit test, so that you can stub out classes that were not
> written to be unit-test friendly.  The partial replacement works by
> adding some code to the beginning of each method to check whether the
> method has been replaced and to call the replacement if it has.
>
>  Now, I must make this tool work with EMMA -- we use EMMA at work to
> track our test coverage.  But when I ran my tests with EMMA the verifier
> complained about illegal values in the local variable table.  Running
> without EMMA there was no problem.
>
>  It turns out that EMMA leaves the local variable table in a funny state.
> Funny, but not illegal.  And when you pass such an EMMA-instrumented
> class through ASM, ASM leaves the table in an illegal state.  So, I
> believe this is a bug in ASM 3.0.  (I've worked around the problem by
> adding a visitLocalVariable implementation that checks for the illegal
> state and just discards it.  Since EMMA's transformation has already
> made the local variable table useless, this is no real loss.)
>
>  The "funny state" that EMMA leaves these tables in is that the length
> for each local variable's scope is set to 0.  When ASM does its
> labeling, it somehow sets the end label's location to 0, so that the
> length ends up negative.
>
>  Here is an example method in four states of instrumentation: bare, my
> ASM instrumentation, EMMA's instrumentation, and both.  Since my
> instrumentation happens at run time, it always must follow EMMA rather
> than the other way around.  Note the lengths in the local variable
> tables that follow the methods.
>
> The original method (as reported by javap):
>
> public java.lang.String getStringValue();
>  Code:
>   Stack=1, Locals=1, Args_size=1
>   0:        aload_0
>   1:        getfield        #34; //Field stringValue:Ljava/lang/String;
>   4:        areturn
>  LocalVariableTable:
>   Start  Length  Slot  Name   Signature
>   0      5      0    this      
> Lus/blanshard/tests/stubout/model/ModelClass;
>
>
> My ASM instrumentation added:
>
> public java.lang.String getStringValue();
>  Code:
>   Stack=1, Locals=1, Args_size=1
>   0:        ldc        #21; //String 
> us/blanshard/tests/stubout/model/ModelClass
>   2:        ldc        #67; //String getStringValue()Ljava/lang/String;
>   4:        invokestatic        #29; //Method
> us/blanshard/stubout/Stubber.isMethodReplaced:(Ljava/lang/String;Ljava/lang/String;)Z 
>
>   7:        ifeq        26
>   10:        ldc        #21; //String 
> us/blanshard/tests/stubout/model/ModelClass
>   12:        ldc        #67; //String getStringValue()Ljava/lang/String;
>   14:        aload_0
>   15:        iconst_0
>   16:        anewarray        #4; //class java/lang/Object
>   19:        invokestatic        #33; //Method
> us/blanshard/stubout/Stubber.callReplacement:(Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; 
>
>   22:        checkcast        #69; //class java/lang/String
>   25:        areturn
>   26:        aload_0
>   27:        getfield        #71; //Field stringValue:Ljava/lang/String;
>   30:        areturn
>  LocalVariableTable:
>   Start  Length  Slot  Name   Signature
>   26      5      0    this      
> Lus/blanshard/tests/stubout/model/ModelClass;
>
>
> EMMA's version:
>
> public java.lang.String getStringValue();
>  Code:
>   Stack=4, Locals=2, Args_size=1
>   0:        getstatic        #23; //Field $VRc:[[Z
>   3:        dup
>   4:        ifnonnull        11
>   7:        pop
>   8:        invokestatic        #27; //Method $VRi:()[[Z
>   11:        iconst_5
>   12:        aaload
>   13:        astore_1
>   14:        aload_0
>   15:        getfield        #46; //Field stringValue:Ljava/lang/String;
>   18:        aload_1
>   19:        iconst_0
>   20:        iconst_1
>   21:        bastore
>   22:        areturn
>  LocalVariableTable:
>   Start  Length  Slot  Name   Signature
>   0      0      0    this      
> Lus/blanshard/tests/stubout/model/ModelClass;
>
>
> And finally, my ASM-ified version of EMMA's instrumented code:
>
> public java.lang.String getStringValue();
>  Code:
>   Stack=4, Locals=2, Args_size=1
>   0:        ldc        #33; //String 
> us/blanshard/tests/stubout/model/ModelClass
>   2:        ldc        #79; //String getStringValue()Ljava/lang/String;
>   4:        invokestatic        #41; //Method
> us/blanshard/stubout/Stubber.isMethodReplaced:(Ljava/lang/String;Ljava/lang/String;)Z 
>
>   7:        ifeq        26
>   10:        ldc        #33; //String 
> us/blanshard/tests/stubout/model/ModelClass
>   12:        ldc        #79; //String getStringValue()Ljava/lang/String;
>   14:        aload_0
>   15:        iconst_0
>   16:        anewarray        #4; //class java/lang/Object
>   19:        invokestatic        #45; //Method
> us/blanshard/stubout/Stubber.callReplacement:(Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; 
>
>   22:        checkcast        #81; //class java/lang/String
>   25:        areturn
>   26:        getstatic        #23; //Field $VRc:[[Z
>   29:        dup
>   30:        ifnonnull        37
>   33:        pop
>   34:        invokestatic        #27; //Method $VRi:()[[Z
>   37:        iconst_5
>   38:        aaload
>   39:        astore_1
>   40:        aload_0
>   41:        getfield        #83; //Field stringValue:Ljava/lang/String;
>   44:        aload_1
>   45:        iconst_0
>   46:        iconst_1
>   47:        bastore
>   48:        areturn
>  LocalVariableTable:
>   Start  Length  Slot  Name   Signature
>   26      -26      0    this      
> Lus/blanshard/tests/stubout/model/ModelClass;
>
>
>  Again, check out that -26 length in the version that combined EMMA and
> ASM transformations.  That seems like a bug to me.
>
> Best regards,
> Luke Blanshard