From b089486b5aadaf89e0fc8339910623de42160d8b Mon Sep 17 00:00:00 2001
From: Jeremy Dubreil <jrm@fb.com>
Date: Wed, 3 Jan 2018 17:55:23 -0800
Subject: [PATCH] [infer][nullable checker] take subtyping into account when
 detecting pointer assignment

Reviewed By: sblackshear

Differential Revision: D6625407

fbshipit-source-id: 3bf8ea7
---
 infer/src/checkers/NullabilityCheck.ml        | 25 ++++++++++++++++---
 .../java/checkers/NullableViolation.java      | 21 ++++++++++------
 .../codetoanalyze/objc/nullable/Examples.m    |  7 ++++++
 3 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/infer/src/checkers/NullabilityCheck.ml b/infer/src/checkers/NullabilityCheck.ml
index 69b58d6bf..501615328 100644
--- a/infer/src/checkers/NullabilityCheck.ml
+++ b/infer/src/checkers/NullabilityCheck.ml
@@ -21,6 +21,18 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
 
   type extras = Specs.summary
 
+  let rec is_pointer_subtype tenv typ1 typ2 =
+    match (typ1.Typ.desc, typ2.Typ.desc) with
+    | Typ.Tptr (t1, _), Typ.Tptr (t2, _) -> (
+      match (t1.Typ.desc, t2.Typ.desc) with
+      | Typ.Tstruct n1, Typ.Tstruct n2 ->
+          Subtype.is_known_subtype tenv n1 n2
+      | _ ->
+          Typ.equal t1 t2 || is_pointer_subtype tenv t1 t2 )
+    | _ ->
+        false
+
+
   let is_non_objc_instance_method callee_pname =
     if Typ.Procname.is_java callee_pname then not (Typ.Procname.java_is_static callee_pname)
     else
@@ -195,11 +207,18 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
 
   let exec_instr ((_, checked_pnames) as astate) proc_data _ (instr: HilInstr.t) : Domain.astate =
     let is_pointer_assignment tenv lhs rhs =
-      HilExp.is_null_literal rhs
       (* the rhs has type int when assigning the lhs to null *)
-      || Option.equal Typ.equal (AccessPath.get_typ lhs tenv) (HilExp.get_typ tenv rhs)
-      (* the lhs and rhs have the same type in the case of pointer assignment
+      if HilExp.is_null_literal rhs then true
+        (* the lhs and rhs have the same type in the case of pointer assignment
          but the types are different when assigning the pointee *)
+      else
+        match (AccessPath.get_typ lhs tenv, HilExp.get_typ tenv rhs) with
+        (* defensive assumption when the types are not known *)
+        | None, _ | _, None ->
+            true
+        (* the rhs can be a subtype of the lhs *)
+        | Some lhs_typ, Some rhs_typ ->
+            is_pointer_subtype tenv rhs_typ lhs_typ
     in
     match instr with
     | Call (Some ret_var, Direct callee_pname, _, _, _)
diff --git a/infer/tests/codetoanalyze/java/checkers/NullableViolation.java b/infer/tests/codetoanalyze/java/checkers/NullableViolation.java
index 460fd7277..403e3feb4 100644
--- a/infer/tests/codetoanalyze/java/checkers/NullableViolation.java
+++ b/infer/tests/codetoanalyze/java/checkers/NullableViolation.java
@@ -15,10 +15,11 @@ public class NullableViolation {
 
   class T {
     int x;
+
     void doSomething() {}
   }
 
-  native static @Nullable T returnsNullable();
+  static native @Nullable T returnsNullable();
 
   void dereferenceNullableReturnValueBad() {
     T t = returnsNullable();
@@ -70,18 +71,24 @@ public class NullableViolation {
   }
 
   void usePreconditionsCheckNotNullOnVariableOkay() {
-      T t = returnsNullable();
-      Preconditions.checkNotNull(t);
-      t.doSomething(); // does not report here
+    T t = returnsNullable();
+    Preconditions.checkNotNull(t);
+    t.doSomething(); // does not report here
   }
 
   void usePreconditionsCheckNotNullOnMethodOkay() {
-      Preconditions.checkNotNull(returnsNullable()).doSomething(); // does not report here
+    Preconditions.checkNotNull(returnsNullable()).doSomething(); // does not report here
   }
 
   void usePreconditionsCheckNotNullRepeatedCallOkay() {
-      Preconditions.checkNotNull(returnsNullable());
-      returnsNullable().doSomething(); // does not report here
+    Preconditions.checkNotNull(returnsNullable());
+    returnsNullable().doSomething(); // does not report here
   }
 
+  native @Nullable Object getNullableObject();
+
+  void pointerAssignmentWithSubtype() {
+    Object object = getNullableObject();
+    object = "Hello";
+  }
 }
diff --git a/infer/tests/codetoanalyze/objc/nullable/Examples.m b/infer/tests/codetoanalyze/objc/nullable/Examples.m
index 0cbe77274..4c7bb9ab7 100644
--- a/infer/tests/codetoanalyze/objc/nullable/Examples.m
+++ b/infer/tests/codetoanalyze/objc/nullable/Examples.m
@@ -245,6 +245,13 @@ typedef struct s_ {
   return s;
 }
 
+- (NSArray*)pointerAssignmentWithSubtypeOkay:(NSString*)string {
+  NSObject* nullableObject = [self nullableMethod];
+  nullableObject = string;
+  NSArray* array = @[ nullableObject ]; // does not report here
+  return array;
+}
+
 @end
 
 @protocol P