Cannot load JUnit 4.11 JAR file about soot HOT 22 CLOSED

Steven are you sure the bytecode is valid? Did you check with javap -c?

Eric

On 29.05.2013, at 17:30, Steven Arzt [email protected]
wrote:

When trying to load the JUnit 4.11 JAR file, Soot fails in the CG phase. Looking for a cause, I found that the code produced by COFFI contains a type mismatch. Thus, the local splitter cannot find a valid split, type assignment fails to find consistent Java types and that inconsistency is what breaks SPARK in the end.

In the code below, take a look at label4. i$ is clearly an integer (constant zero when coming from label3) here and it is also used as such in the following integer comparison. If this comparison evaluates to true, we jump to label5 which does nothing except for an unconditional jump to label1. In label1, i$ is however used like an object.

public void (java.lang.Class)
{
    unknown this, klass, $stack0, $stack1, $stack2, i$, eachClass, arr$, len$, eachMethod, $stack3, eachField;

    this := @this: org.junit.runners.model.TestClass;
    klass := @parameter0: java.lang.Class;
    $stack0 = this;
    specialinvoke $stack0.()>();
    $stack0 = this;
    $stack1 = new java.util.HashMap;
    $stack2 = $stack1;
    specialinvoke $stack2.()>();
    $stack0. = $stack1;
    $stack0 = this;
    $stack1 = new java.util.HashMap;
    $stack2 = $stack1;
    specialinvoke $stack2.()>();
    $stack0. = $stack1;
    $stack0 = this;
    $stack1 = klass;
    $stack0. = $stack1;
    $stack0 = klass;
    if $stack0 == null goto label0;

    $stack0 = klass;
    $stack0 = virtualinvoke $stack0.();
    $stack0 = lengthof $stack0;
    $stack1 = 1;
    if $stack0 (java.lang.String)>($stack2);
    throw $stack0;

 label0:
    $stack0 = this;
    $stack1 = this;
    $stack1 = $stack1.;
    $stack0 = specialinvoke $stack0.($stack1);
    $stack0 = interfaceinvoke $stack0.();
    i$ = $stack0;

 label1:
    $stack0 = i$;
    $stack0 = interfaceinvoke $stack0.();
    if $stack0 == 0 goto label6;

    $stack0 = i$;
    $stack0 = interfaceinvoke $stack0.();
    $stack0 = (java.lang.Class) $stack0;
    eachClass = $stack0;
    $stack0 = eachClass;
    $stack0 = staticinvoke ($stack0);
    arr$ = $stack0;
    $stack0 = arr$;
    $stack0 = lengthof $stack0;
    len$ = $stack0;
    $stack0 = 0;
    i$ = $stack0;

 label2:
    $stack0 = i$;
    $stack1 = len$;
    if $stack0 >= $stack1 goto label3;

    $stack0 = arr$;
    $stack1 = i$;
    $stack0 = $stack0[$stack1];
    eachMethod = $stack0;
    $stack0 = this;
    $stack1 = new org.junit.runners.model.FrameworkMethod;
    $stack2 = $stack1;
    $stack3 = eachMethod;
    specialinvoke $stack2.(java.lang.reflect.Method)>($stack3);
    $stack2 = this;
    $stack2 = $stack2.;
    specialinvoke $stack0.($stack1, $stack2);
    i$ = i$ + 1;
    goto label2;

 label3:
    $stack0 = eachClass;
    $stack0 = virtualinvoke $stack0.();
    arr$ = $stack0;
    $stack0 = arr$;
    $stack0 = lengthof $stack0;
    len$ = $stack0;
    $stack0 = 0;
    i$ = $stack0;

 label4:
    $stack0 = i$;
    $stack1 = len$;
    if $stack0 >= $stack1 goto label5;

    $stack0 = arr$;
    $stack1 = i$;
    $stack0 = $stack0[$stack1];
    eachField = $stack0;
    $stack0 = this;
    $stack1 = new org.junit.runners.model.FrameworkField;
    $stack2 = $stack1;
    $stack3 = eachField;
    specialinvoke $stack2.(java.lang.reflect.Field)>($stack3);
    $stack2 = this;
    $stack2 = $stack2.;
    specialinvoke $stack0.($stack1, $stack2);
    i$ = i$ + 1;
    goto label4;

 label5:
    goto label1;

 label6:
    return;
}

Does anyone have an idea on this?

—
Reply to this email directly or view it on GitHub.

Eric Bodden, Ph.D., http://sse.ec-spride.de/ http://bodden.de/
Head of Secure Software Engineering Group at EC SPRIDE
Tel: +49 6151 16-75422 Fax: +49 6151 16-72051
Room 3.2.14, Mornewegstr. 30, 64293 Darmstadt

from soot.

Is the bytecode reusing the same local slot for storing variables with different data types, I think Java allows that. Maybe Coffi get confused by such practice.

from soot.

Is the bytecode is reusing the same local slot for storing variables with different data types, I think Java allows that. Maybe Coffi get confused by such practice.

Right. But Java does this all the time. Hence I would be quite surprised if coffi got this wrong.

Eric

from soot.

@ericbodden The output of javap seems ok to me:

public class org.junit.runners.model.TestClass {
public org.junit.runners.model.TestClass(java.lang.Class); public java.util.List getAnnotatedMethods(java.lang.Class); public java.util.List getAnnotatedFields(java.lang.Class); public java.lang.Class getJavaClass();
public java.lang.String getName();
public java.lang.reflect.Constructor<?> getOnlyConstructor();
public java.lang.annotation.Annotation[] getAnnotations();
public <T extends java/lang/Object> java.util.List getAnnotatedFieldValues(java.lang.Object, java.lang.Class<? extends java.lang.annotation.Annotation>, java.lang.Class);
public <T extends java/lang/Object> java.util.List getAnnotatedMethodValues(java.lang.Object, java.lang.Class<? extends java.lang.annotation.Annotation>, java.lang.Class);
public boolean isANonStaticInnerClass();
}

The issue here is that in my opinion, this code does have a logical problem as explained above. There is a path through the code in which you run into incompatible types if I have not overlooked anything.

from soot.

javap -c should print the disassembled bytecode.

Is the OpenJDK able to run the bytecode? Wouldn't the bytecode verifier reject the incompatibility? Unless the path running into incompatible types isn't actually realizable and the verifier can see that.

from soot.

Steven you have to use the "-c" flag to actually print the code. Your output just shows signatures, which is not telling us anything.

Eric

On 29.05.2013, at 18:01, Steven Arzt [email protected] wrote:

@ericbodden The output of javap seems ok to me:

public class org.junit.runners.model.TestClass {
public org.junit.runners.model.TestClass(java.lang.Class); public java.util.List getAnnotatedMethods(java.lang.Class); public java.util.List getAnnotatedFields(java.lang.Class); public java.lang.Class getJavaClass();
public java.lang.String getName();
public java.lang.reflect.Constructor<?> getOnlyConstructor();
public java.lang.annotation.Annotation[] getAnnotations();
public java.util.List getAnnotatedFieldValues(java.lang.Object, java.lang.Class<? extends java.lang.annotation.Annotation>, java.lang.Class);
public java.util.List getAnnotatedMethodValues(java.lang.Object, java.lang.Class<? extends java.lang.annotation.Annotation>, java.lang.Class);
public boolean isANonStaticInnerClass();
}

—
Reply to this email directly or view it on GitHub.

Eric Bodden, Ph.D., http://sse.ec-spride.de/ http://bodden.de/
Head of Secure Software Engineering Group at EC SPRIDE
Tel: +49 6151 16-75422 Fax: +49 6151 16-72051
Room 3.2.14, Mornewegstr. 30, 64293 Darmstadt

from soot.

@ericbodden oops, there must have been something wrong with the command line to javap. Anyhow, here's the real output. The constructor is causing the problem.

Compiled from "TestClass.java"
public class org.junit.runners.model.TestClass {
  public org.junit.runners.model.TestClass(java.lang.Class);
    Code:
       0: aload_0       
       1: invokespecial #1                  // Method java/lang/Object."":()V
       4: aload_0       
       5: new           #2                  // class java/util/HashMap
       8: dup           
       9: invokespecial #3                  // Method java/util/HashMap."":()V
      12: putfield      #4                  // Field fMethodsForAnnotations:Ljava/util/Map;
      15: aload_0       
      16: new           #2                  // class java/util/HashMap
      19: dup           
      20: invokespecial #3                  // Method java/util/HashMap."":()V
      23: putfield      #5                  // Field fFieldsForAnnotations:Ljava/util/Map;
      26: aload_0       
      27: aload_1       
      28: putfield      #6                  // Field fClass:Ljava/lang/Class;
      31: aload_1       
      32: ifnull        54
      35: aload_1       
      36: invokevirtual #7                  // Method java/lang/Class.getConstructors:()[Ljava/lang/reflect/Constructor;
      39: arraylength   
      40: iconst_1      
      41: if_icmple     54
      44: new           #8                  // class java/lang/IllegalArgumentException
      47: dup           
      48: ldc           #9                  // String Test class can only have one constructor
      50: invokespecial #10                 // Method java/lang/IllegalArgumentException."":(Ljava/lang/String;)V
      53: athrow        
      54: aload_0       
      55: aload_0       
      56: getfield      #6                  // Field fClass:Ljava/lang/Class;
      59: invokespecial #11                 // Method getSuperClasses:(Ljava/lang/Class;)Ljava/util/List;
      62: invokeinterface #12,  1           // InterfaceMethod java/util/List.iterator:()Ljava/util/Iterator;
      67: astore_2      
      68: aload_2       
      69: invokeinterface #13,  1           // InterfaceMethod java/util/Iterator.hasNext:()Z
      74: ifeq          192
      77: aload_2       
      78: invokeinterface #14,  1           // InterfaceMethod java/util/Iterator.next:()Ljava/lang/Object;
      83: checkcast     #15                 // class java/lang/Class
      86: astore_3      
      87: aload_3       
      88: invokestatic  #16                 // Method org/junit/internal/MethodSorter.getDeclaredMethods:(Ljava/lang/Class;)[Ljava/lang/reflect/Method;
      91: astore        4
      93: aload         4
      95: arraylength   
      96: istore        5
      98: iconst_0      
      99: istore        6
     101: iload         6
     103: iload         5
     105: if_icmpge     138
     108: aload         4
     110: iload         6
     112: aaload        
     113: astore        7
     115: aload_0       
     116: new           #17                 // class org/junit/runners/model/FrameworkMethod
     119: dup           
     120: aload         7
     122: invokespecial #18                 // Method org/junit/runners/model/FrameworkMethod."":(Ljava/lang/reflect/Method;)V
     125: aload_0       
     126: getfield      #4                  // Field fMethodsForAnnotations:Ljava/util/Map;
     129: invokespecial #19                 // Method addToAnnotationLists:(Lorg/junit/runners/model/FrameworkMember;Ljava/util/Map;)V
     132: iinc          6, 1
     135: goto          101
     138: aload_3       
     139: invokevirtual #20                 // Method java/lang/Class.getDeclaredFields:()[Ljava/lang/reflect/Field;
     142: astore        4
     144: aload         4
     146: arraylength   
     147: istore        5
     149: iconst_0      
     150: istore        6
     152: iload         6
     154: iload         5
     156: if_icmpge     189
     159: aload         4
     161: iload         6
     163: aaload        
     164: astore        7
     166: aload_0       
     167: new           #21                 // class org/junit/runners/model/FrameworkField
     170: dup           
     171: aload         7
     173: invokespecial #22                 // Method org/junit/runners/model/FrameworkField."":(Ljava/lang/reflect/Field;)V
     176: aload_0       
     177: getfield      #5                  // Field fFieldsForAnnotations:Ljava/util/Map;
     180: invokespecial #19                 // Method addToAnnotationLists:(Lorg/junit/runners/model/FrameworkMember;Ljava/util/Map;)V
     183: iinc          6, 1
     186: goto          152
     189: goto          68
     192: return        
  public java.util.List getAnnotatedMethods(java.lang.Class);
    Code:
       0: aload_0       
       1: aload_0       
       2: getfield      #4                  // Field fMethodsForAnnotations:Ljava/util/Map;
       5: aload_1       
       6: invokespecial #25                 // Method getAnnotatedMembers:(Ljava/util/Map;Ljava/lang/Class;)Ljava/util/List;
       9: areturn       
  public java.util.List getAnnotatedFields(java.lang.Class);
    Code:
       0: aload_0       
       1: aload_0       
       2: getfield      #5                  // Field fFieldsForAnnotations:Ljava/util/Map;
       5: aload_1       
       6: invokespecial #25                 // Method getAnnotatedMembers:(Ljava/util/Map;Ljava/lang/Class;)Ljava/util/List;
       9: areturn       
  public java.lang.Class getJavaClass();
    Code:
       0: aload_0       
       1: getfield      #6                  // Field fClass:Ljava/lang/Class;
       4: areturn       
  public java.lang.String getName();
    Code:
       0: aload_0       
       1: getfield      #6                  // Field fClass:Ljava/lang/Class;
       4: ifnonnull     10
       7: ldc           #41                 // String null
       9: areturn       
      10: aload_0       
      11: getfield      #6                  // Field fClass:Ljava/lang/Class;
      14: invokevirtual #42                 // Method java/lang/Class.getName:()Ljava/lang/String;
      17: areturn       
  public java.lang.reflect.Constructor getOnlyConstructor();
    Code:
       0: aload_0       
       1: getfield      #6                  // Field fClass:Ljava/lang/Class;
       4: invokevirtual #7                  // Method java/lang/Class.getConstructors:()[Ljava/lang/reflect/Constructor;
       7: astore_1      
       8: lconst_1      
       9: aload_1       
      10: arraylength   
      11: i2l           
      12: invokestatic  #43                 // Method org/junit/Assert.assertEquals:(JJ)V
      15: aload_1       
      16: iconst_0      
      17: aaload        
      18: areturn       
  public java.lang.annotation.Annotation[] getAnnotations();
    Code:
       0: aload_0       
       1: getfield      #6                  // Field fClass:Ljava/lang/Class;
       4: ifnonnull     12
       7: iconst_0      
       8: anewarray     #44                 // class java/lang/annotation/Annotation
      11: areturn       
      12: aload_0       
      13: getfield      #6                  // Field fClass:Ljava/lang/Class;
      16: invokevirtual #45                 // Method java/lang/Class.getAnnotations:()[Ljava/lang/annotation/Annotation;
      19: areturn       
  public  java.util.List getAnnotatedFieldValues(java.lang.Object, java.lang.Class, java.lang.Class);
    Code:
       0: new           #31                 // class java/util/ArrayList
       3: dup           
       4: invokespecial #32                 // Method java/util/ArrayList."":()V
       7: astore        4
       9: aload_0       
      10: aload_2       
      11: invokevirtual #46                 // Method getAnnotatedFields:(Ljava/lang/Class;)Ljava/util/List;
      14: invokeinterface #12,  1           // InterfaceMethod java/util/List.iterator:()Ljava/util/Iterator;
      19: astore        5
      21: aload         5
      23: invokeinterface #13,  1           // InterfaceMethod java/util/Iterator.hasNext:()Z
      28: ifeq          94
      31: aload         5
      33: invokeinterface #14,  1           // InterfaceMethod java/util/Iterator.next:()Ljava/lang/Object;
      38: checkcast     #21                 // class org/junit/runners/model/FrameworkField
      41: astore        6
      43: aload         6
      45: aload_1       
      46: invokevirtual #47                 // Method org/junit/runners/model/FrameworkField.get:(Ljava/lang/Object;)Ljava/lang/Object;
      49: astore        7
      51: aload_3       
      52: aload         7
      54: invokevirtual #48                 // Method java/lang/Class.isInstance:(Ljava/lang/Object;)Z
      57: ifeq          74
      60: aload         4
      62: aload_3       
      63: aload         7
      65: invokevirtual #49                 // Method java/lang/Class.cast:(Ljava/lang/Object;)Ljava/lang/Object;
      68: invokeinterface #29,  2           // InterfaceMethod java/util/List.add:(Ljava/lang/Object;)Z
      73: pop           
      74: goto          91
      77: astore        7
      79: new           #51                 // class java/lang/RuntimeException
      82: dup           
      83: ldc           #52                 // String How did getFields return a field we couldn't access?
      85: aload         7
      87: invokespecial #53                 // Method java/lang/RuntimeException."":(Ljava/lang/String;Ljava/lang/Throwable;)V
      90: athrow        
      91: goto          21
      94: aload         4
      96: areturn       
    Exception table:
       from    to  target type
          43    74    77   Class java/lang/IllegalAccessException
  public  java.util.List getAnnotatedMethodValues(java.lang.Object, java.lang.Class, java.lang.Class);
    Code:
       0: new           #31                 // class java/util/ArrayList
       3: dup           
       4: invokespecial #32                 // Method java/util/ArrayList."":()V
       7: astore        4
       9: aload_0       
      10: aload_2       
      11: invokevirtual #54                 // Method getAnnotatedMethods:(Ljava/lang/Class;)Ljava/util/List;
      14: invokeinterface #12,  1           // InterfaceMethod java/util/List.iterator:()Ljava/util/Iterator;
      19: astore        5
      21: aload         5
      23: invokeinterface #13,  1           // InterfaceMethod java/util/Iterator.hasNext:()Z
      28: ifeq          119
      31: aload         5
      33: invokeinterface #14,  1           // InterfaceMethod java/util/Iterator.next:()Ljava/lang/Object;
      38: checkcast     #17                 // class org/junit/runners/model/FrameworkMethod
      41: astore        6
      43: aload         6
      45: aload_1       
      46: iconst_0      
      47: anewarray     #55                 // class java/lang/Object
      50: invokevirtual #56                 // Method org/junit/runners/model/FrameworkMethod.invokeExplosively:(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;
      53: astore        7
      55: aload_3       
      56: aload         7
      58: invokevirtual #48                 // Method java/lang/Class.isInstance:(Ljava/lang/Object;)Z
      61: ifeq          78
      64: aload         4
      66: aload_3       
      67: aload         7
      69: invokevirtual #49                 // Method java/lang/Class.cast:(Ljava/lang/Object;)Ljava/lang/Object;
      72: invokeinterface #29,  2           // InterfaceMethod java/util/List.add:(Ljava/lang/Object;)Z
      77: pop           
      78: goto          116
      81: astore        7
      83: new           #51                 // class java/lang/RuntimeException
      86: dup           
      87: new           #58                 // class java/lang/StringBuilder
      90: dup           
      91: invokespecial #59                 // Method java/lang/StringBuilder."":()V
      94: ldc           #60                 // String Exception in 
      96: invokevirtual #61                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      99: aload         6
     101: invokevirtual #62                 // Method org/junit/runners/model/FrameworkMethod.getName:()Ljava/lang/String;
     104: invokevirtual #61                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
     107: invokevirtual #63                 // Method java/lang/StringBuilder.toString:()Ljava/lang/String;
     110: aload         7
     112: invokespecial #53                 // Method java/lang/RuntimeException."":(Ljava/lang/String;Ljava/lang/Throwable;)V
     115: athrow        
     116: goto          21
     119: aload         4
     121: areturn       
    Exception table:
       from    to  target type
          43    78    81   Class java/lang/Throwable
  public boolean isANonStaticInnerClass();
    Code:
       0: aload_0       
       1: getfield      #6                  // Field fClass:Ljava/lang/Class;
       4: invokevirtual #64                 // Method java/lang/Class.isMemberClass:()Z
       7: ifeq          27
      10: aload_0       
      11: getfield      #6                  // Field fClass:Ljava/lang/Class;
      14: invokevirtual #65                 // Method java/lang/Class.getModifiers:()I
      17: invokestatic  #66                 // Method java/lang/reflect/Modifier.isStatic:(I)Z
      20: ifne          27
      23: iconst_1      
      24: goto          28
      27: iconst_0      
      28: ireturn       
}

from soot.

The last statement of label0 and first statement of label1 comes from bytecodes:

67: astore_2
68: aload_2

So in the bytecode, local variable 2 is used.

The incrementation of i$ at the end of label4 comes from:

183: iinc          6, 1

Where the 6th local variable is used.

from soot.

From the jimple, I assume you are using the keep-original-names option. What about disabling it? It has already been the source of similar problems.

Looking at the Java code for that class [1], it seems like the "i$" variables are generated by the compiler to replace some foreach statements. The first one iterates over an Iterator (hence the reference type of i$). And the others i$ are index counters (hence the int type) iterating over arrays.

What if the compiler dumps the same name (i$) for every generated iterator/index variable, even if they overlap ?

[1]http://grepcode.com/file/repo1.maven.org/maven2/junit/junit/4.11/org/junit/runners/model/TestClass.java#TestClass.%3Cinit%3E%28java.lang.Class%29

from soot.

I just made a very simple test, and I can confirm that the compiler gives identical name to several generated variables with overlaping liveness and different types.
Here is an excerpt of the debug infos as shown by javap -c -l

LocalVariableTable:
  Start  Length  Slot  Name   Signature
        50      36     7    i$   I
        19      70     3    i$   Ljava/util/Iterator;

As you can see, overlapping bytecode ranges, different slot and signature, same name.

from soot.

@quentin Removing the "keep-original-names" flag actually seems to have resolved the issue. Thank you very much for the assistance. It would be interesting to know whether the JVM specification actually allows such behavior (which would be pretty weird, though it's unambiguous with the variable indices) or whether this is a bug in the compiler.

from soot.

I guess there is no restriction in the LocalVariableTable since it is annotations for debug purpose only and doesn't actually influence the semantics of the bytecode.

from soot.

The problem comes from method soot.coffi.Util.getLocalForIndex(), where locals are created based on the name found in LocalVariableTable if "keep-original-names" is enabled. Regardless of their index, slot, signature or range, locals with the same debug name will get the same Local object.

from soot.

Quentin thanks a lot for debugging this. I can confirm that the JVM does not at all care about names. For "use original names" Soot surely does care. Apparently the algorithm there is a bit flaky. Would be great if the two of you could work out a fix.

@steven: Make sure to keep a copy of that class file as a regression test.

Cheers,
Eric

On 29.05.2013, at 22:53, Quentin Sabah [email protected] wrote:

The problem comes from method soot.coffi.Util.getLocalForIndex(), where locals are created based on the name found in LocalVariableTable if "keep-original-names" is enabled. Regardless of their slot ("index" in Soot), signature or range, locals with the same debug name will get the same Local object.

—
Reply to this email directly or view it on GitHub.

Eric Bodden, Ph.D., http://sse.ec-spride.de/ http://bodden.de/
Head of Secure Software Engineering Group at EC SPRIDE
Tel: +49 6151 16-75422 Fax: +49 6151 16-72051
Room 3.2.14, Mornewegstr. 30, 64293 Darmstadt

from soot.

I'll try to write a fix.

from soot.

@StevenArzt: you may checkout my fix branch (mentioned in the previous post) and test against your classes. I also wouldn't be surprised if it solves issue #40, can you check that too ?

from soot.

@quentin The fix seems to work fine, I can now also use the use-original-names option without triggering type inconsistencies. Thank you very much!

Shall I push this to the global Soot develop branch or should we give it a bit more testing?

from soot.

Thanks to both of you! Before merging this in, could you still clean up the code by removing all commented-out stuff? Also it would be great if Quentin could add to the commit message a few more details about what he changed why. Thanks!

from soot.

Yes sure, I'll clean the modified code (actually Coffi would need a lot of cleaning) and add the explanations it deserves.

from soot.

So this fix will produce sound Local allocation if the LocalVariableTable is consistent enough, meaning each variable from the source code has at most one name in the table and the table reports the proper name at every bytecode position where the variable is used.

The only way of producing consistent Local allocation and taking original names into account is to allocate Local in default mode and record "most probable names" in each statement for each Local used or defined by the statement.
Then there would be a method like String[] Stmt.getMostProbableOriginalNames(Local), or String[] Body.getMostProbableOriginalNames(Local, Stmt).

from soot.

Thanks a lot Quention, this is awesome! Can you issue a pull request? I think this would be the easiest way for me to merge this in.

from soot.

Thanks again!

from soot.