From d11852af9a431a13bc2f61effd045fbd62045799 Mon Sep 17 00:00:00 2001 From: Daiva Naudziuniene Date: Mon, 24 May 2021 03:41:40 -0700 Subject: [PATCH] [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 | 1 + infer/man/man1/infer-report.txt | 1 + infer/man/man1/infer.txt | 1 + infer/src/base/IssueType.ml | 5 +++++ infer/src/base/IssueType.mli | 4 +++- infer/src/pulse/Pulse.ml | 4 +++- infer/src/pulse/PulseInvalidation.ml | 6 +++++- infer/src/pulse/PulseInvalidation.mli | 2 +- infer/src/pulse/PulseOperations.ml | 4 ++-- infer/src/pulse/PulseOperations.mli | 7 ++++++- infer/tests/codetoanalyze/objc/pulse/issues.exp | 8 ++++---- 11 files changed, 32 insertions(+), 11 deletions(-) diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index fa994f1ba..6e48c3cc0 100644 --- a/infer/man/man1/infer-full.txt +++ b/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), diff --git a/infer/man/man1/infer-report.txt b/infer/man/man1/infer-report.txt index 8c4afc4f3..fe195e959 100644 --- a/infer/man/man1/infer-report.txt +++ b/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), diff --git a/infer/man/man1/infer.txt b/infer/man/man1/infer.txt index c1768b12c..33949c53f 100644 --- a/infer/man/man1/infer.txt +++ b/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), diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index afdd85750..984e42167 100644 --- a/infer/src/base/IssueType.ml +++ b/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." diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index a5845950b..d4db52979 100644 --- a/infer/src/base/IssueType.mli +++ b/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 diff --git a/infer/src/pulse/Pulse.ml b/infer/src/pulse/Pulse.ml index f5a48472f..27fc3fe38 100644 --- a/infer/src/pulse/Pulse.ml +++ b/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 diff --git a/infer/src/pulse/PulseInvalidation.ml b/infer/src/pulse/PulseInvalidation.ml index e3b56fa25..e53c4821c 100644 --- a/infer/src/pulse/PulseInvalidation.ml +++ b/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 -> diff --git a/infer/src/pulse/PulseInvalidation.mli b/infer/src/pulse/PulseInvalidation.mli index 3119545df..7db7a45e7 100644 --- a/infer/src/pulse/PulseInvalidation.mli +++ b/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 diff --git a/infer/src/pulse/PulseOperations.ml b/infer/src/pulse/PulseOperations.ml index 26ebf4446..ea24da943 100644 --- a/infer/src/pulse/PulseOperations.ml +++ b/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 diff --git a/infer/src/pulse/PulseOperations.mli b/infer/src/pulse/PulseOperations.mli index cfa0c878c..790f05652 100644 --- a/infer/src/pulse/PulseOperations.mli +++ b/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 : diff --git a/infer/tests/codetoanalyze/objc/pulse/issues.exp b/infer/tests/codetoanalyze/objc/pulse/issues.exp index 6e3ee98ac..6e7e5baca 100644 --- a/infer/tests/codetoanalyze/objc/pulse/issues.exp +++ b/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.paramAssignNilBad:, 3, NULLPTR_DEREFERENCE, 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, 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, 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, NIL_BLOCK_CALL, no_bucket, ERROR, [is the null pointer,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, 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]