[infer][nullable checker] report an error when a nullable value is stored into an NSArray without being checked for null

Summary: Adding a nil object to an NSArray will crash. Adding this case to the checker.

Reviewed By: sblackshear

Differential Revision: D6346241

fbshipit-source-id: 3fe6f20

infer/src/checkers/NullabilityCheck.ml

@@ -125,6 +125,27 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
       match ap with _, [] -> None | p -> longest_nullable_prefix (AccessPath.truncate p) astate
 
 
+  let check_ap proc_data loc ap astate =
+    match longest_nullable_prefix ap astate with
+    | None ->
+        astate
+    | Some (nullable_ap, call_sites) ->
+        report_nullable_dereference nullable_ap call_sites proc_data loc ;
+        assume_pnames_notnull ap astate
+
+
+  let rec check_nil_in_nsarray proc_data loc args astate =
+    match args with
+    | [] ->
+        astate
+    | [arg] when HilExp.is_null_literal arg ->
+        astate
+    | (HilExp.AccessPath ap) :: other_args ->
+        check_nil_in_nsarray proc_data loc other_args (check_ap proc_data loc ap astate)
+    | _ :: other_args ->
+        check_nil_in_nsarray proc_data loc other_args astate
+
+
   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
@@ -147,13 +168,11 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
         let call_site = CallSite.make callee_pname loc in
         add_nullable_ap (ret_var, []) (CallSites.singleton call_site) astate
     | Call (_, Direct callee_pname, (HilExp.AccessPath receiver) :: _, _, loc)
-      when is_instance_method callee_pname -> (
+      when is_instance_method callee_pname ->
-      match longest_nullable_prefix receiver astate with
+        check_ap proc_data loc receiver astate
-      | None ->
+    | Call (_, Direct callee_pname, args, _, loc)
-          astate
+      when Typ.Procname.equal callee_pname BuiltinDecl.nsArray_arrayWithObjectsCount ->
-      | Some (nullable_ap, call_sites) ->
+        check_nil_in_nsarray proc_data loc args astate
-          report_nullable_dereference nullable_ap call_sites proc_data loc ;
-          assume_pnames_notnull receiver astate )
     | Call (Some ret_var, _, _, _, _) ->
         remove_nullable_ap (ret_var, []) astate
     | Assign (lhs, rhs, loc)

infer/tests/codetoanalyze/objc/checkers/Nullable.m

@@ -6,7 +6,7 @@
  * LICENSE file in the root directory of this source tree. An additional grant
  * of patent rights can be found in the PATENTS file in the same directory.
  */
-#import <Foundation/NSString.h>
+#import <Foundation/Foundation.h>
 
 int* __nullable returnsNull();
 
@@ -107,13 +107,37 @@ int* __nullable returnsNull();
 }
 
 - (NSString*)dereferenceNullableMethodOkay {
-  NSObject* object = [self nullableMethod];
+  NSObject* nullableObject = [self nullableMethod];
-  return [object description]; // does not report here
+  return [nullableObject description]; // does not report here
 }
 
 - (void)reassigningNullableObjectOkay {
-  NSObject* object = [self nullableMethod];
+  NSObject* nullableObject = [self nullableMethod];
-  object = nil; // does not report here
+  nullableObject = nil; // does not report here
+}
+
+- (NSArray*)nullableObjectInNSArrayBad {
+  NSObject* nullableObject = [self nullableMethod];
+  NSArray* array = @[ nullableObject ];
+  return array;
+}
+
+- (NSArray*)secondElementNullableObjectInNSArrayBad {
+  NSObject* allocatedObject = [NSObject alloc];
+  NSObject* nullableObject = [self nullableMethod];
+  NSArray* array = @[ allocatedObject, nullableObject ];
+  return array;
+}
+
+- (NSArray*)nullableObjectInNSArrayOkay {
+  NSObject* nullableObject = [self nullableMethod];
+  NSArray* array;
+  if (nullableObject) {
+    array = @[ nullableObject ];
+  } else {
+    array = @[ @"String" ];
+  }
+  return array;
 }
 
 @end

infer/tests/codetoanalyze/objc/checkers/issues.exp

@@ -7,7 +7,11 @@ codetoanalyze/objc/checkers/Nullable.m, T_dereferenceNullableFunctionBad, 2, NUL
 codetoanalyze/objc/checkers/Nullable.m, T_dereferenceNullableFunctionBad, 2, NULL_DEREFERENCE, [start of procedure dereferenceNullableFunctionBad,Skipping returnsNull(): function or method not found]
 codetoanalyze/objc/checkers/Nullable.m, T_dereferenceUnnanotatedFieldAfterTestForNullBad, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotatedField is compared to null here]
 codetoanalyze/objc/checkers/Nullable.m, T_dereferenceUnnanotatedFieldAfterTestForNullBad, 2, NULL_DEREFERENCE, [start of procedure dereferenceUnnanotatedFieldAfterTestForNullBad,Condition is true]
+codetoanalyze/objc/checkers/Nullable.m, T_nullableObjectInNSArrayBad, 2, NULLABLE_DEREFERENCE, [dereference of &nullableObject,assignment of the nullable value,definition of nullableMethod]
+codetoanalyze/objc/checkers/Nullable.m, T_nullableObjectInNSArrayBad, 2, NULL_DEREFERENCE, [start of procedure nullableObjectInNSArrayBad,start of procedure nullableMethod,return from a call to T_nullableMethod]
 codetoanalyze/objc/checkers/Nullable.m, T_reassigningNullableObjectOkay, 1, DEAD_STORE, [Write of unused value]
+codetoanalyze/objc/checkers/Nullable.m, T_secondElementNullableObjectInNSArrayBad, 3, NULLABLE_DEREFERENCE, [dereference of &nullableObject,assignment of the nullable value,definition of nullableMethod]
+codetoanalyze/objc/checkers/Nullable.m, T_secondElementNullableObjectInNSArrayBad, 3, NULL_DEREFERENCE, [start of procedure secondElementNullableObjectInNSArrayBad,start of procedure nullableMethod,return from a call to T_nullableMethod]
 codetoanalyze/objc/checkers/Nullable.m, T_testNonnullFieldForNullBad, 1, FIELD_SHOULD_BE_NULLABLE, [Field nonnullField is compared to null here]
 codetoanalyze/objc/checkers/Nullable.m, T_testUnnanotatedFieldForNullBad, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotatedField is compared to null here]
 codetoanalyze/objc/checkers/Nullable.m, objc_blockT_DeadStoreFP_testUnnanotatedFieldInClosureBad_1, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotatedField is compared to null here]