search

soot-oss / soot

Soot - A Java optimization framework
GNU Lesser General Public License v2.1
2.85k stars 705 forks source link

NullnessAnalysis treats the receiver object of a method invocation as non-null #1929

Closed thebesttv closed 1 year ago

thebesttv commented 1 year ago

Describe the bug

NullnessAnalysis treats the receiver object of a method invocation as non-null even if the object may be null, causing false negatives. For example, the method:

    void possibleNullOnSplit(int n) {
        Object a = null;
        if (n > 0) {
            a = "notNull";
        }
        Class c = a.getClass();
    }

has IR like this:

    void possibleNullOnSplit(int)
    {
        NullDeref2 this;
        int n;
        java.lang.Object a, temp$0;
        java.lang.Class c, temp$1;
        this := @this: NullDeref2;
        n := @parameter0: int;
        a = null;
        if n > 0 goto label1;
        goto label2;
     label1:
        nop;
        temp$0 = "notNull";
        a = temp$0;
     label2:
        nop;
        temp$1 = virtualinvoke a.<java.lang.Object: java.lang.Class getClass()>();
        c = temp$1;
        return;
    }

The local variable a is neither always-null nor always-nonnull before statement temp$1 = virtualinvoke a.<java.lang.Object: java.lang.Class getClass()>();. But after that statement, a becomes always-nonnull.

This is caused by flowThrough function in NullnessAnalysis simply setting the base to NON_NULL:

  protected void flowThrough(AnalysisInfo in, Unit u, List<AnalysisInfo> fallOut, List<AnalysisInfo> branchOuts) {
    // ...
    // for invoke expr, set the receiver object to non-null, if there is one
    if (s.containsInvokeExpr()) {
      handleInvokeExpr(s.getInvokeExpr(), out);
    } // ...
  }

  private void handleInvokeExpr(InvokeExpr invokeExpr, AnalysisInfo out) {
    if (invokeExpr instanceof InstanceInvokeExpr) {
      InstanceInvokeExpr instanceInvokeExpr = (InstanceInvokeExpr) invokeExpr;
      // here we know that the receiver must point to an object
      out.put(instanceInvokeExpr.getBase(), NON_NULL);
    }
  }

Input file

The input file is soot-null-dereference.zip, a Gradle project mainly composed of src/main/java/thebesttv/Main.java and src/main/resources/test/NullDeref2.java.

To reproduce

Unzip the input file and run ./gradlew run. At the end of the output is:

  nop
    Null:     []
    Non-null: [this, temp$0]
  temp$1 = virtualinvoke a.<java.lang.Object: java.lang.Class getClass()>()
    Null:     []
    Non-null: [this, a, temp$0]
  c = temp$1
    Null:     []
    Non-null: [this, a, temp$0]
  return

You can see the local variable a becomes always-nonnull after the statement temp$1 = virtualinvoke a.<java.lang.Object: java.lang.Class getClass()>();.

Expected behavior

The local variable a is neither always-null nor always-nonnull after the statement.

patricklam commented 1 year ago

If a is null then execution does not proceed to the successor because there is a NullPointerException thrown. So I would assume that a is non null after successful completion of the statement.

On Sun, Nov 20, 2022, 3:29 PM thebesttv @.***> wrote:

Describe the bug

NullnessAnalysis treats the receiver object of a method invocation as non-null even if the object may be null, causing false negatives. For example, the method:

void possibleNullOnSplit(int n) {
    Object a = null;
    if (n > 0) {
        a = "notNull";
    }
    Class c = a.getClass();
}

has IR like this:

void possibleNullOnSplit(int)
{
    NullDeref2 this;
    int n;
    java.lang.Object a, temp$0;
    java.lang.Class c, temp$1;
    this := @this: NullDeref2;
    n := @parameter0: int;
    a = null;
    if n > 0 goto label1;
    goto label2;
 label1:
    nop;
    temp$0 = "notNull";
    a = temp$0;
 label2:
    nop;
    temp$1 = virtualinvoke a.<java.lang.Object: java.lang.Class getClass()>();
    c = temp$1;
    return;
}

The local variable a is neither always-null nor always-nonnull before statement temp$1 = virtualinvoke a.<java.lang.Object: java.lang.Class getClass()>();. But after that statement, a becomes always-nonnull.

This is caused by flowThrough function in NullnessAnalysis simply setting the base to NON_NULL:

protected void flowThrough(AnalysisInfo in, Unit u, List fallOut, List branchOuts) { // ... // for invoke expr, set the receiver object to non-null, if there is one if (s.containsInvokeExpr()) { handleInvokeExpr(s.getInvokeExpr(), out); } // ... }

private void handleInvokeExpr(InvokeExpr invokeExpr, AnalysisInfo out) { if (invokeExpr instanceof InstanceInvokeExpr) { InstanceInvokeExpr instanceInvokeExpr = (InstanceInvokeExpr) invokeExpr; // here we know that the receiver must point to an object out.put(instanceInvokeExpr.getBase(), NON_NULL); } }

Input file

The input file is soot-null-dereference.zip https://github.com/soot-oss/soot/files/10039818/soot-null-dereference.zip, a Gradle project mainly composed of src/main/java/thebesttv/Main.java and src/main/resources/test/NullDeref2.java.

To reproduce

Unzip the input file and run ./gradlew run. At the end of the output is:

nop Null: [] Non-null: [this, temp$0] temp$1 = virtualinvoke a.<java.lang.Object: java.lang.Class getClass()>() Null: [] Non-null: [this, a, temp$0] c = temp$1 Null: [] Non-null: [this, a, temp$0] return

You can see the local variable a becomes always-nonnull after the statement temp$1 = virtualinvoke a.<java.lang.Object: java.lang.Class getClass()>();.

Expected behavior

The local variable a is neither always-null nor always-nonnull after the statement.

— Reply to this email directly, view it on GitHub https://github.com/soot-oss/soot/issues/1929, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOKE5TQDVMR43SMOKPXYUDWJGEJZANCNFSM6AAAAAASFTNX5Y . You are receiving this because you are subscribed to this thread.Message ID: @.***>

thebesttv commented 1 year ago

Thanks a lot for you reply, that's very helpful!

I have just another question: for some code

a = mayReturnNull();
a.f1();
a.f2();

At statement a = mayReturnNull();, if a is neither always-null nor always-nonnull, then I will report a.f1() as a possible NPE. But is it possible to also report a.f2() as NPE?

I assume, from your reply, only a.f1() can be reported, and a.f2() can not be reported since a is always-nonnull.

patricklam commented 1 year ago

I can't see a way that the a.f2() will trigger. Either f1() will trigger or neither will.

On Sun, Nov 20, 2022, 4:58 PM thebesttv @.***> wrote:

Thanks a lot for you reply, that's very helpful!

I have just another question: for some code

a = mayReturnNull();a.f1();a.f2();

At statement a = mayReturnNull();, if a is neither always-null nor always-nonnull, then I will report a.f1() as a possible NPE. But is it possible to also report a.f2() as NPE?

I assume, from your reply, only a.f1() can be reported, and a.f2() can not be reported since a is always-nonnull.

— Reply to this email directly, view it on GitHub https://github.com/soot-oss/soot/issues/1929#issuecomment-1321028893, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOKE5RRZWYR5FIQKPJAFXTWJGOXRANCNFSM6AAAAAASFTNX5Y . You are receiving this because you commented.Message ID: @.***>

thebesttv commented 1 year ago

Thanks!