[pulse][objc][nullptr] A separate issue type for calling nil blocks

Summary: Facebook

Reviewed By: jvillard

Differential Revision: D28604781

fbshipit-source-id: c321161de

infer/man/man1/infer-full.txt

@@ -541,6 +541,7 @@ OPTIONS
              MULTIPLE_WEAKSELF (enabled by default),
              MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE (enabled by default),
              Missing_fld (enabled by default),
+             NIL_BLOCK_CALL (enabled by default),
              NIL_INSERTION_INTO_COLLECTION (enabled by default),
              NIL_MESSAGING_TO_NON_POD (enabled by default),
              NULLPTR_DEREFERENCE (enabled by default),

infer/man/man1/infer-report.txt

@@ -218,6 +218,7 @@ OPTIONS
              MULTIPLE_WEAKSELF (enabled by default),
              MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE (enabled by default),
              Missing_fld (enabled by default),
+             NIL_BLOCK_CALL (enabled by default),
              NIL_INSERTION_INTO_COLLECTION (enabled by default),
              NIL_MESSAGING_TO_NON_POD (enabled by default),
              NULLPTR_DEREFERENCE (enabled by default),

infer/man/man1/infer.txt

@@ -541,6 +541,7 @@ OPTIONS
              MULTIPLE_WEAKSELF (enabled by default),
              MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE (enabled by default),
              Missing_fld (enabled by default),
+             NIL_BLOCK_CALL (enabled by default),
              NIL_INSERTION_INTO_COLLECTION (enabled by default),
              NIL_MESSAGING_TO_NON_POD (enabled by default),
              NULLPTR_DEREFERENCE (enabled by default),

infer/src/base/IssueType.ml

@@ -789,6 +789,11 @@ let mutable_local_variable_in_component_file =
       [%blob "../../documentation/issues/MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE.md"]
 
 
+let nil_block_call =
+  register ~id:"NIL_BLOCK_CALL" Error Pulse
+    ~user_documentation:"Calling a nil block is an error in Objective-C."
+
+
 let nil_insertion_into_collection =
   register ~id:"NIL_INSERTION_INTO_COLLECTION" Error Pulse
     ~user_documentation:"Inserting nil into a collection is an error in Objective-C."

infer/src/base/IssueType.mli

@@ -276,10 +276,12 @@ val multiple_weakself : t
 
 val mutable_local_variable_in_component_file : t
 
-val nil_messaging_to_non_pod : t
+val nil_block_call : t
 
 val nil_insertion_into_collection : t
 
+val nil_messaging_to_non_pod : t
+
 val null_dereference : t
 
 val nullptr_dereference : t

infer/src/pulse/Pulse.ml

@@ -43,7 +43,9 @@ module PulseTransferFunctions = struct
           ~ret ~actuals ~formals_opt astate
     | _ ->
         (* dereference call expression to catch nil issues *)
-        let<*> astate, _ = PulseOperations.eval_deref call_loc call_exp astate in
+        let<*> astate, _ =
+          PulseOperations.eval_deref ~must_be_valid_reason:BlockCall call_loc call_exp astate
+        in
         L.d_printfln "Skipping indirect call %a@\n" Exp.pp call_exp ;
         let astate =
           let arg_values = List.map actuals ~f:(fun ((value, _), _) -> value) in

infer/src/pulse/PulseInvalidation.ml

@@ -52,12 +52,14 @@ type t =
   | JavaIterator of java_iterator_function
 [@@deriving compare, equal]
 
-type must_be_valid_reason = SelfOfNonPODReturnMethod | InsertionIntoCollection
+type must_be_valid_reason = BlockCall | InsertionIntoCollection | SelfOfNonPODReturnMethod
 [@@deriving compare, equal]
 
 let pp_must_be_valid_reason f = function
   | None ->
       F.fprintf f "None"
+  | Some BlockCall ->
+      F.fprintf f "Block"
   | Some InsertionIntoCollection ->
       F.fprintf f "InsertionIntoCollection"
   | Some SelfOfNonPODReturnMethod ->
@@ -72,6 +74,8 @@ let issue_type_of_cause invalidation must_be_valid_reason =
     match must_be_valid_reason with
     | None ->
         IssueType.nullptr_dereference
+    | Some BlockCall ->
+        IssueType.nil_block_call
     | Some InsertionIntoCollection ->
         IssueType.nil_insertion_into_collection
     | Some SelfOfNonPODReturnMethod ->

infer/src/pulse/PulseInvalidation.mli

@@ -40,7 +40,7 @@ val pp : F.formatter -> t -> unit
 
 val describe : F.formatter -> t -> unit
 
-type must_be_valid_reason = SelfOfNonPODReturnMethod | InsertionIntoCollection
+type must_be_valid_reason = BlockCall | InsertionIntoCollection | SelfOfNonPODReturnMethod
 [@@deriving compare, equal]
 
 val pp_must_be_valid_reason : F.formatter -> must_be_valid_reason option -> unit

infer/src/pulse/PulseOperations.ml

@@ -279,9 +279,9 @@ let prune location ~condition astate =
   prune_aux ~negated:false condition astate
 
 
-let eval_deref location exp astate =
+let eval_deref ?must_be_valid_reason location exp astate =
   let* astate, addr_hist = eval Read location exp astate in
-  let+ astate = check_addr_access Read location addr_hist astate in
+  let+ astate = check_addr_access ?must_be_valid_reason Read location addr_hist astate in
   Memory.eval_edge addr_hist Dereference astate
 
 

infer/src/pulse/PulseOperations.mli

@@ -100,7 +100,12 @@ val eval_structure_isl :
 
 val prune : Location.t -> condition:Exp.t -> t -> t AccessResult.t
 
-val eval_deref : Location.t -> Exp.t -> t -> (t * (AbstractValue.t * ValueHistory.t)) AccessResult.t
+val eval_deref :
+     ?must_be_valid_reason:Invalidation.must_be_valid_reason
+  -> Location.t
+  -> Exp.t
+  -> t
+  -> (t * (AbstractValue.t * ValueHistory.t)) AccessResult.t
 (** Like [eval] but evaluates [*exp]. *)
 
 val eval_deref_isl :

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

@@ -9,10 +9,10 @@ codetoanalyze/objc/pulse/MemoryLeaks.m, call_cfrelease_interproc_leak_ok_FP, 3,
 codetoanalyze/objc/pulse/MemoryLeaksInBlocks.m, block_captured_var_leak_bad, 7, MEMORY_LEAK, no_bucket, ERROR, [allocation part of the trace starts here,allocated by `malloc_no_fail` here,memory becomes unreachable here]
 codetoanalyze/objc/pulse/NPEBlocks.m, Singleton.dispatch_once_captured_vars_bad, 8, NULLPTR_DEREFERENCE, no_bucket, ERROR, [is the null pointer,assigned,in call to `objc_blockSingleton.dispatch_once_captured_vars_bad_2`,parameter `x` of objc_blockSingleton.dispatch_once_captured_vars_bad_2,assigned,return from call to `objc_blockSingleton.dispatch_once_captured_vars_bad_2`,invalid access occurs here]
 codetoanalyze/objc/pulse/NPEBlocks.m, captured_npe_bad, 5, NULLPTR_DEREFERENCE, no_bucket, ERROR, [is the null pointer,assigned,when calling `objc_blockcaptured_npe_bad_4` here,parameter `x` of objc_blockcaptured_npe_bad_4,invalid access occurs here]
-codetoanalyze/objc/pulse/NPENilBlocks.m, BlockA.assignNilBad, 5, NULLPTR_DEREFERENCE, no_bucket, ERROR, [is the null pointer,assigned,invalid access occurs here]
+codetoanalyze/objc/pulse/NPENilBlocks.m, BlockA.assignNilBad, 5, NIL_BLOCK_CALL, no_bucket, ERROR, [is the null pointer,assigned,invalid access occurs here]
-codetoanalyze/objc/pulse/NPENilBlocks.m, BlockA.paramAssignNilBad:, 3, NULLPTR_DEREFERENCE, no_bucket, ERROR, [is the null pointer,assigned,invalid access occurs here]
+codetoanalyze/objc/pulse/NPENilBlocks.m, BlockA.paramAssignNilBad:, 3, NIL_BLOCK_CALL, no_bucket, ERROR, [is the null pointer,assigned,invalid access occurs here]
-codetoanalyze/objc/pulse/NPENilBlocks.m, BlockA.paramReassignNilBad:, 6, NULLPTR_DEREFERENCE, no_bucket, ERROR, [is the null pointer,assigned,assigned,invalid access occurs here]
+codetoanalyze/objc/pulse/NPENilBlocks.m, BlockA.paramReassignNilBad:, 6, NIL_BLOCK_CALL, no_bucket, ERROR, [is the null pointer,assigned,assigned,invalid access occurs here]
-codetoanalyze/objc/pulse/NPENilBlocks.m, calldoSomethingThenCallbackWithNilBad, 2, NULLPTR_DEREFERENCE, no_bucket, ERROR, [is the null pointer,when calling `BlockA.doSomethingThenCallback:` here,parameter `my_block` of BlockA.doSomethingThenCallback:,invalid access occurs here]
+codetoanalyze/objc/pulse/NPENilBlocks.m, calldoSomethingThenCallbackWithNilBad, 2, NIL_BLOCK_CALL, no_bucket, ERROR, [is the null pointer,when calling `BlockA.doSomethingThenCallback:` here,parameter `my_block` of BlockA.doSomethingThenCallback:,invalid access occurs here]
 codetoanalyze/objc/pulse/uninit.m, Uninit.call_setter_c_struct_bad, 3, PULSE_UNINITIALIZED_VALUE, no_bucket, ERROR, [struct field address `x` created,when calling `Uninit.setS:` here,parameter `s` of Uninit.setS:,read to uninitialized value occurs here]
 codetoanalyze/objc/pulse/use_after_free.m, PulseTest.use_after_free_simple_in_objc_method_bad:, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidation part of the trace starts here,parameter `x` of PulseTest.use_after_free_simple_in_objc_method_bad:,was invalidated by call to `free()`,use-after-lifetime part of the trace starts here,parameter `x` of PulseTest.use_after_free_simple_in_objc_method_bad:,invalid access occurs here]
 codetoanalyze/objc/pulse/use_after_free.m, use_after_free_simple_bad, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidation part of the trace starts here,parameter `x` of use_after_free_simple_bad,was invalidated by call to `free()`,use-after-lifetime part of the trace starts here,parameter `x` of use_after_free_simple_bad,invalid access occurs here]