[infer][clang] the nullability checker was not reporting when dereferencing a field of a nullable object

Summary: The checker was not previously reporting in those cases.

Reviewed By: sblackshear

Differential Revision: D6133710

fbshipit-source-id: e2e6363

infer/src/checkers/NullabilityCheck.ml

@@ -80,6 +80,12 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
     Reporting.log_error summary ~loc ~ltr:trace exn
 
 
+  let rec longest_nullable_prefix ap astate =
+    try Some (ap, Domain.find ap astate)
+    with Not_found ->
+      match ap with _, [] -> None | p -> longest_nullable_prefix (AccessPath.truncate p) astate
+
+
   let exec_instr (astate: Domain.astate) proc_data _ (instr: HilInstr.t) : Domain.astate =
     match instr with
     | Call (Some ret_var, Direct callee_pname, _, _, loc)
@@ -89,21 +95,31 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
         Domain.add (ret_var, []) (CallSites.singleton call_site) astate
     | Call (_, Direct callee_pname, (HilExp.AccessPath receiver) :: _, _, loc)
       when is_instance_method callee_pname -> (
-      match Domain.find_opt receiver astate with
+      match longest_nullable_prefix receiver astate with
       | None ->
           astate
-      | Some call_sites ->
-          report_nullable_dereference receiver call_sites proc_data loc ;
+      | Some (nullable_ap, call_sites) ->
+          report_nullable_dereference nullable_ap call_sites proc_data loc ;
           Domain.remove receiver astate )
     | Call (Some ret_var, _, _, _, _) ->
         Domain.remove (ret_var, []) astate
-    | Assign (lhs, _, loc) when Domain.mem lhs astate ->
-        report_nullable_dereference lhs (Domain.find lhs astate) proc_data loc ;
-        Domain.remove lhs astate
-    | Assign (lhs, HilExp.AccessPath rhs, _) when Domain.mem rhs astate ->
-        Domain.add lhs (Domain.find rhs astate) astate
-    | Assign (lhs, _, _) ->
-        Domain.remove lhs astate
+    | Assign (lhs, rhs, loc)
+      -> (
+        Option.iter
+          ~f:(fun (nullable_ap, call_sites) ->
+            report_nullable_dereference nullable_ap call_sites proc_data loc)
+          (longest_nullable_prefix lhs astate) ;
+        match rhs with
+        | HilExp.AccessPath ap -> (
+          try
+            (* Add the lhs to the list of nullable values if the rhs is nullable *)
+            Domain.add lhs (Domain.find ap astate) astate
+          with Not_found ->
+            (* Remove the lhs from the list of nullable values if the rhs is not nullable *)
+            Domain.remove lhs astate )
+        | _ ->
+            (* Remove the lhs from the list of nullable values if the rhs is not an access path *)
+            Domain.remove lhs astate )
     | Assume (HilExp.AccessPath ap, _, _, _) ->
         Domain.remove ap astate
     | Assume (HilExp.BinaryOperator (Binop.Ne, HilExp.AccessPath ap, exp), _, _, _)
@@ -121,3 +137,4 @@ let checker {Callbacks.summary; proc_desc; tenv} =
   let proc_data = ProcData.make proc_desc tenv summary in
   ignore (Analyzer.compute_post proc_data ~initial ~debug:false) ;
   summary
+

infer/tests/codetoanalyze/cpp/nullable/issues.exp

@@ -11,6 +11,12 @@ codetoanalyze/cpp/nullable/method.cpp, FP_reAssigningNullableValueOk, 1, DEAD_ST
 codetoanalyze/cpp/nullable/method.cpp, FP_reAssigningNullableValueOk, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullPointer]
 codetoanalyze/cpp/nullable/method.cpp, assignNullableValueBad, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullPointer]
 codetoanalyze/cpp/nullable/method.cpp, assignNullableValueBad, 2, NULL_DEREFERENCE, [start of procedure assignNullableValueBad(),start of procedure mayReturnNullPointer,Condition is true,return from a call to T_mayReturnNullPointer]
+codetoanalyze/cpp/nullable/method.cpp, avoidDoubleReportingBad, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullObject]
+codetoanalyze/cpp/nullable/method.cpp, avoidDoubleReportingBad, 2, NULL_DEREFERENCE, [start of procedure avoidDoubleReportingBad(),start of procedure mayReturnNullObject,Condition is true,return from a call to T_mayReturnNullObject]
 codetoanalyze/cpp/nullable/method.cpp, callMethodOnNullableObjectBad, 1, NULLABLE_DEREFERENCE, [deference of n$2,definition of mayReturnNullObject]
 codetoanalyze/cpp/nullable/method.cpp, callMethodOnNullableObjectBad, 1, NULL_DEREFERENCE, [start of procedure callMethodOnNullableObjectBad(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject]
 codetoanalyze/cpp/nullable/method.cpp, callMethodOnNullableObjectOk, 2, NULL_TEST_AFTER_DEREFERENCE, [start of procedure callMethodOnNullableObjectOk(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false]
+codetoanalyze/cpp/nullable/method.cpp, dereferenceFieldOfNullableObjectBad, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullObject]
+codetoanalyze/cpp/nullable/method.cpp, dereferenceFieldOfNullableObjectBad, 2, NULL_DEREFERENCE, [start of procedure dereferenceFieldOfNullableObjectBad(),start of procedure mayReturnNullObject,Condition is true,return from a call to T_mayReturnNullObject]
+codetoanalyze/cpp/nullable/method.cpp, methodCallOnFieldOfNullableObjectBad, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullObject]
+codetoanalyze/cpp/nullable/method.cpp, methodCallOnFieldOfNullableObjectBad, 2, NULL_DEREFERENCE, [start of procedure methodCallOnFieldOfNullableObjectBad(),start of procedure mayReturnNullObject,Condition is true,return from a call to T_mayReturnNullObject]

infer/tests/codetoanalyze/cpp/nullable/method.cpp

@@ -9,6 +9,9 @@
 bool star();
 
 class T {
+ public:
+  int x;
+  T* field;
 
  public:
   int* _Nullable mayReturnNullPointer() {
@@ -53,3 +56,19 @@ void callMethodOnNullableObjectOk(T* t) {
     p->doSomething();
   }
 }
+
+void dereferenceFieldOfNullableObjectBad(T* t) {
+  T* p = t->mayReturnNullObject();
+  p->x = 42;
+}
+
+void methodCallOnFieldOfNullableObjectBad(T* t) {
+  T* p = t->mayReturnNullObject();
+  p->field->doSomething();
+}
+
+void avoidDoubleReportingBad(T* t) {
+  T* p = t->mayReturnNullObject();
+  p->doSomething(); // should report here
+  p->doSomething(); // should not report here
+}