diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index a7a97a0e8b..1dca827dd2 100644 --- a/infer/man/man1/infer-full.txt +++ b/infer/man/man1/infer-full.txt @@ -2664,9 +2664,9 @@ INTERNAL OPTIONS --complete-capture-from-reset Cancel the effect of --complete-capture-from. - --compute-captured-context - Activates: Compute context information for captured variables in - Objective-C blocks (Conversely: --no-compute-captured-context) + --no-compute-captured-context + Deactivates: Compute context information for captured variables in + Objective-C blocks (Conversely: --compute-captured-context) --config-impact-config-field-patterns +regex Register known config fields that have a config value. The matched diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index 0873e643e1..8e3c537315 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -1333,7 +1333,7 @@ and complete_capture_from = and compute_captured_context = - CLOpt.mk_bool ~long:"compute-captured-context" ~default:false + CLOpt.mk_bool ~long:"compute-captured-context" ~default:true "Compute context information for captured variables in Objective-C blocks" diff --git a/infer/src/checkers/ComputeCapturedInfo.ml b/infer/src/checkers/ComputeCapturedInfo.ml index a7bd8098cb..fc5e41cd6a 100644 --- a/infer/src/checkers/ComputeCapturedInfo.ml +++ b/infer/src/checkers/ComputeCapturedInfo.ml @@ -118,11 +118,41 @@ module CheckedForNull = struct end module Analyzer = AbstractInterpreter.MakeWTO (TransferFunctions) + + let flatten_checked_for_null_domain domain = + let process_mem mem domain_elements = + let process_pvar_domain pvar (domain : DomainData.t) domain_elements = + match domain.captured_in_closure with + | Some procname when domain.checked -> + Procname.Map.update procname + (fun pvars -> + match pvars with Some pvars -> Some (pvar :: pvars) | None -> Some [pvar] ) + domain_elements + | _ -> + domain_elements + in + PVars.fold process_pvar_domain mem.Mem.pvars domain_elements + in + Domain.fold process_mem domain Procname.Map.empty + + + let process_checked_for_null_pvar checked_for_null_data captured_pvar = + let pvar_name = Pvar.get_name captured_pvar.CapturedVar.pvar in + let checked = List.exists ~f:(fun pvar -> Mangled.equal pvar pvar_name) checked_for_null_data in + let context_info = + match captured_pvar.CapturedVar.context_info with + | Some context_info -> + {context_info with CapturedVar.is_checked_for_null= checked} + | None -> + {CapturedVar.is_checked_for_null= checked; is_internal_pointer_of= None} + in + {captured_pvar with CapturedVar.context_info= Some context_info} end module InternalStringPointer = struct module VarsDomainData = struct - type t = {internal_pointer_of: Typ.t} [@@deriving compare] + type t = {internal_pointer_of: Typ.t; captured_in_closure: Procname.t option} + [@@deriving compare] let pp fmt {internal_pointer_of} = F.fprintf fmt "internal_pointer_of=%a" (Typ.pp Pp.text) internal_pointer_of @@ -134,10 +164,18 @@ module InternalStringPointer = struct let compare = compare VarsDomainData.compare end + module PVarsDomainData = struct + type t = {id: Ident.t; captured_in_closure: Procname.t option} [@@deriving compare] + + let pp fmt {id; captured_in_closure} = + F.fprintf fmt "id=%a, captured_in_closure=%a" Ident.pp id (Pp.option Procname.pp) + captured_in_closure + end + module PVars = struct - include PrettyPrintable.MakePPMonoMap (Mangled) (Ident) + include PrettyPrintable.MakePPMonoMap (Mangled) (PVarsDomainData) - let compare = compare Ident.compare + let compare = compare PVarsDomainData.compare end module Mem = struct @@ -146,24 +184,42 @@ module InternalStringPointer = struct let pp fmt {vars; pvars} = F.fprintf fmt "Vars= %a@\nPVars= %a@\n" Vars.pp vars PVars.pp pvars let set_internal_pointer var typ astate = - let vars = Vars.add var {VarsDomainData.internal_pointer_of= typ} astate.vars in + let vars = + Vars.add var + {VarsDomainData.internal_pointer_of= typ; captured_in_closure= None} + astate.vars + in {astate with vars} - let is_internal_pointer_of pvar astate = - match PVars.find_opt pvar astate.pvars with - | Some var -> ( - match Vars.find_opt var astate.vars with - | Some domainData -> - Some domainData.internal_pointer_of - | None -> - None ) - | None -> - None + let call exps astate = + let update_captured_in_closure_in_arg astate (exp, _) = + let update_captured_in_closure closure_pname astate (_, captured_var) = + Logging.d_printfln "captured var is %a" CapturedVar.pp captured_var ; + let pvars = + PVars.update + (Pvar.get_name captured_var.CapturedVar.pvar) + (fun data_opt -> + Option.value_map + ~f:(fun data -> + Some {data with PVarsDomainData.captured_in_closure= Some closure_pname} ) + data_opt ~default:None ) + astate.pvars + in + {astate with pvars} + in + match exp with + | Exp.Closure {name; captured_vars} -> + Logging.d_printfln "closure name is %a" Procname.pp name ; + List.fold ~f:(update_captured_in_closure name) captured_vars ~init:astate + | _ -> + astate + in + List.fold ~f:update_captured_in_closure_in_arg exps ~init:astate let store pvar id astate = - let pvars = PVars.add (Pvar.get_name pvar) id astate.pvars in + let pvars = PVars.add (Pvar.get_name pvar) {id; captured_in_closure= None} astate.pvars in {astate with pvars} end @@ -172,14 +228,9 @@ module InternalStringPointer = struct let set_internal_pointer var typ astate : t = map (Mem.set_internal_pointer var typ) astate - let is_internal_pointer_of pvar astate = - fold - (fun state typ_opt -> - match typ_opt with Some _ -> typ_opt | None -> Mem.is_internal_pointer_of pvar state ) - astate None - - let store pvar id astate = map (Mem.store pvar id) astate + + let call exps astate = map (Mem.call exps) astate end module TransferFunctions = struct @@ -226,59 +277,93 @@ module InternalStringPointer = struct else astate | _ -> astate ) - | Call ((var, _), Exp.Const (Const.Cfun procname), (_, typ) :: _args, _, _) -> - let procname_qualifiers = Procname.get_qualifiers procname in - if - QualifiedCppName.Match.match_qualifiers cstring_using_encoding procname_qualifiers - || QualifiedCppName.Match.match_qualifiers utf8string procname_qualifiers - then - match typ.desc with - | Tptr (inside_typ, _) -> - Domain.set_internal_pointer var inside_typ astate - | _ -> - astate - else astate + | Call ((var, _), Exp.Const (Const.Cfun procname), (_, typ) :: _args, _, _) + when QualifiedCppName.Match.match_qualifiers cstring_using_encoding + (Procname.get_qualifiers procname) + || QualifiedCppName.Match.match_qualifiers utf8string + (Procname.get_qualifiers procname) -> ( + match typ.desc with + | Tptr (inside_typ, _) -> + Domain.set_internal_pointer var inside_typ astate + | _ -> + astate ) + | Call (_, _, args, _loc, _call_flags) -> + Domain.call args astate | _ -> astate end module Analyzer = AbstractInterpreter.MakeWTO (TransferFunctions) + + let flatten_is_internal_pointer_domain domain = + let process_mem mem domain_elements = + let process_var_domain pvar {PVarsDomainData.id; captured_in_closure} domain_elements = + match captured_in_closure with + | Some procname -> ( + match Vars.find_opt id mem.Mem.vars with + | Some (vars_domain_data : VarsDomainData.t) -> + Procname.Map.update procname + (fun pvars -> + let tuple = (pvar, vars_domain_data.internal_pointer_of) in + match pvars with Some pvars -> Some (tuple :: pvars) | None -> Some [tuple] ) + domain_elements + | None -> + domain_elements ) + | _ -> + domain_elements + in + PVars.fold process_var_domain mem.Mem.pvars domain_elements + in + Domain.fold process_mem domain Procname.Map.empty + + + let process_is_internal_pointer_pvar internal_string_pointer_data captured_pvar = + let pvar_name = Pvar.get_name captured_pvar.CapturedVar.pvar in + match + List.find ~f:(fun (pvar, _) -> Mangled.equal pvar pvar_name) internal_string_pointer_data + with + | Some (_, is_internal_pointer_of) -> + let context_info = + match captured_pvar.CapturedVar.context_info with + | Some context_info -> + {context_info with CapturedVar.is_internal_pointer_of= Some is_internal_pointer_of} + | None -> + { CapturedVar.is_internal_pointer_of= Some is_internal_pointer_of + ; CapturedVar.is_checked_for_null= false } + in + {captured_pvar with CapturedVar.context_info= Some context_info} + | None -> + captured_pvar end -let update_captured_in_closures cfg checked_for_null_domain_opt internal_string_pointer_domain_opt = - let process_mem {CheckedForNull.Mem.pvars} = - let process_pvar pvar {CheckedForNull.DomainData.checked; captured_in_closure} = - match captured_in_closure with - | Some procname -> - let proc_desc = Procname.Hash.find cfg procname in - let attributes = Procdesc.get_attributes proc_desc in - let update_captured captured_var = - let pvar_name = Pvar.get_name captured_var.CapturedVar.pvar in - if Mangled.equal pvar pvar_name then - let is_internal_pointer_of = - Option.bind - ~f:(fun domain -> InternalStringPointer.Domain.is_internal_pointer_of pvar domain) - internal_string_pointer_domain_opt - in - let context_info = - {CapturedVar.is_checked_for_null= checked; is_internal_pointer_of} - in - {captured_var with CapturedVar.context_info= Some context_info} - else captured_var +let update_captured_context_closures blocks checked_for_null_domain_opt + internal_string_pointer_domain_opt = + let update_context_in_closure block attributes procname domain_opt process_pvar = + match domain_opt with + | Some domain -> ( + match Procname.Map.find_opt procname domain with + | Some internal_string_pointer_data -> + let updated_captured = + List.map + ~f:(process_pvar internal_string_pointer_data) + attributes.ProcAttributes.captured in - let captured = List.map ~f:update_captured attributes.ProcAttributes.captured in - let attributes = {attributes with captured} in - Procdesc.set_attributes proc_desc attributes + let attributes = {attributes with ProcAttributes.captured= updated_captured} in + Procdesc.set_attributes block attributes | None -> - () - in - CheckedForNull.PVars.iter process_pvar pvars + () ) + | None -> + () in - match checked_for_null_domain_opt with - | Some checked_for_null_domain -> - CheckedForNull.Domain.iter process_mem checked_for_null_domain - | None -> - () + let process_block block = + let attributes = Procdesc.get_attributes block in + let procname = Procdesc.get_proc_name block in + update_context_in_closure block attributes procname checked_for_null_domain_opt + CheckedForNull.process_checked_for_null_pvar ; + update_context_in_closure block attributes procname internal_string_pointer_domain_opt + InternalStringPointer.process_is_internal_pointer_pvar + in + List.iter ~f:process_block blocks let process cfg = @@ -290,6 +375,11 @@ let process cfg = let checked_for_null_domain = CheckedForNull.Analyzer.compute_post proc_desc ~initial proc_desc in + let flat_checked_for_null_domain = + Option.map + ~f:(fun map -> CheckedForNull.flatten_checked_for_null_domain map) + checked_for_null_domain + in let initial = InternalStringPointer.Domain.singleton { InternalStringPointer.Mem.vars= InternalStringPointer.Vars.empty @@ -298,6 +388,20 @@ let process cfg = let internal_string_pointer_domain = InternalStringPointer.Analyzer.compute_post proc_desc ~initial proc_desc in - update_captured_in_closures cfg checked_for_null_domain internal_string_pointer_domain + let flat_internal_string_pointer_domain = + Option.map + ~f:(fun map -> InternalStringPointer.flatten_is_internal_pointer_domain map) + internal_string_pointer_domain + in + let get_blocks cfg = + Cfg.fold_sorted cfg + ~f:(fun blocks proc_desc -> + let procname = Procdesc.get_proc_name proc_desc in + if Procname.is_objc_block procname then proc_desc :: blocks else blocks ) + ~init:[] + in + let blocks = get_blocks cfg in + update_captured_context_closures blocks flat_checked_for_null_domain + flat_internal_string_pointer_domain in Procname.Hash.iter (process cfg) cfg diff --git a/infer/src/checkers/ParameterNotNullChecked.ml b/infer/src/checkers/ParameterNotNullChecked.ml index cd133188cc..2bfc251569 100644 --- a/infer/src/checkers/ParameterNotNullChecked.ml +++ b/infer/src/checkers/ParameterNotNullChecked.ml @@ -241,13 +241,20 @@ let is_block_param formals name = let get_captured_formals attributes = let captured = attributes.ProcAttributes.captured in let formal_of_captured (captured : CapturedVar.t) = - match (captured.captured_from, captured.context_info) with - | Some {is_formal= Some proc}, Some {CapturedVar.is_checked_for_null} -> ( + match captured.captured_from with + | Some {is_formal= Some proc} -> ( match IRAttributes.load proc with | Some proc_attributes -> let formals = find_block_param proc_attributes.ProcAttributes.formals (Pvar.get_name captured.pvar) in + let is_checked_for_null = + match captured.CapturedVar.context_info with + | Some {CapturedVar.is_checked_for_null} -> + is_checked_for_null + | None -> + false + in Option.map formals ~f:(fun formals -> (formals, proc_attributes, is_checked_for_null)) | None -> None ) diff --git a/infer/tests/codetoanalyze/objc/parameter-not-null-checked/issues.exp b/infer/tests/codetoanalyze/objc/parameter-not-null-checked/issues.exp index bcf80dec23..ad2c6cd455 100644 --- a/infer/tests/codetoanalyze/objc/parameter-not-null-checked/issues.exp +++ b/infer/tests/codetoanalyze/objc/parameter-not-null-checked/issues.exp @@ -4,4 +4,5 @@ codetoanalyze/objc/parameter-not-null-checked/Blocks_as_parameters.m, Blocks_as_ codetoanalyze/objc/parameter-not-null-checked/Blocks_as_parameters.m, Blocks_as_parameters.twoBlocksNotCheckedBad:and:and:, 3, BLOCK_PARAMETER_NOT_NULL_CHECKED, no_bucket, WARNING, [Parameter `block2` of Blocks_as_parameters.twoBlocksNotCheckedBad:and:and:,Executing `block2`], "block2("=>"BLOCK_CALL_SAFE(block2"@54:5 codetoanalyze/objc/parameter-not-null-checked/Blocks_as_parameters.m, Blocks_as_parameters.nonnullBlockTwoBlocksBad:and:and:, 4, BLOCK_PARAMETER_NOT_NULL_CHECKED, no_bucket, WARNING, [Parameter `block2` of Blocks_as_parameters.nonnullBlockTwoBlocksBad:and:and:,Executing `block2`], "block2("=>"BLOCK_CALL_SAFE(block2"@82:3 codetoanalyze/objc/parameter-not-null-checked/Blocks_as_parameters.m, Blocks_as_parameters.blockCheckedAssignNULLBad:and:, 3, BLOCK_PARAMETER_NOT_NULL_CHECKED, no_bucket, WARNING, [Parameter `block` of Blocks_as_parameters.blockCheckedAssignNULLBad:and:,Checking `block` for nil,Assigned,Executing `block`], "block("=>"BLOCK_CALL_SAFE(block"@88:5 +codetoanalyze/objc/parameter-not-null-checked/Blocks_as_parameters.m, objc_block_Blocks_as_parameters.m:121, 1, BLOCK_PARAMETER_NOT_NULL_CHECKED, no_bucket, WARNING, [Parameter `completion` of Blocks_as_parameters.uploadTaskWithRequestBad:fromFile:delegate:delegateQueue:completion:,Executing `completion`], "completion("=>"BLOCK_CALL_SAFE(completion"@122:5 codetoanalyze/objc/parameter-not-null-checked/Blocks_as_parameters.m, blockNotCheckedBadNoAutofix, 0, BLOCK_PARAMETER_NOT_NULL_CHECKED, no_bucket, WARNING, [Parameter `block` of blockNotCheckedBadNoAutofix,Executing `block`] diff --git a/infer/tests/codetoanalyze/objc/self-in-block/issues.exp b/infer/tests/codetoanalyze/objc/self-in-block/issues.exp index 76937faa19..e07f1a70d3 100644 --- a/infer/tests/codetoanalyze/objc/self-in-block/issues.exp +++ b/infer/tests/codetoanalyze/objc/self-in-block/issues.exp @@ -1,3 +1,5 @@ +codetoanalyze/objc/self-in-block/CStringUsingEncodingTest.m, objc_block_CStringUsingEncodingTest.m:18, 1, NSSTRING_INTERNAL_PTR_CAPTURED_IN_BLOCK, no_bucket, ERROR, [Captured variable &encoding defined here,Using captured &encoding] +codetoanalyze/objc/self-in-block/CStringUsingEncodingTest.m, objc_block_CStringUsingEncodingTest.m:34, 1, NSSTRING_INTERNAL_PTR_CAPTURED_IN_BLOCK, no_bucket, ERROR, [Captured variable &utf8string defined here,Using captured &utf8string] codetoanalyze/objc/self-in-block/NoescapeBlock.m, objc_block_NoescapeBlock.m:35, 1, MULTIPLE_WEAKSELF, no_bucket, ERROR, [Using &weakSelf,Using &weakSelf] codetoanalyze/objc/self-in-block/NoescapeBlock.m, objc_block_NoescapeBlock.m:35, 1, WEAK_SELF_IN_NO_ESCAPE_BLOCK, no_bucket, ERROR, [Using &weakSelf,Using &weakSelf] codetoanalyze/objc/self-in-block/NoescapeBlock.m, objc_block_NoescapeBlock.m:35, 2, WEAK_SELF_IN_NO_ESCAPE_BLOCK, no_bucket, ERROR, [Using &weakSelf,Using &weakSelf] diff --git a/infer/tests/codetoanalyze/objcpp/self-in-block/issues.exp b/infer/tests/codetoanalyze/objcpp/self-in-block/issues.exp index b879e64682..d5ee1a3630 100644 --- a/infer/tests/codetoanalyze/objcpp/self-in-block/issues.exp +++ b/infer/tests/codetoanalyze/objcpp/self-in-block/issues.exp @@ -2,3 +2,4 @@ codetoanalyze/objcpp/self-in-block/CxxRefInBlock.mm, objc_block_CxxRefInBlock.mm codetoanalyze/objcpp/self-in-block/CxxRefInBlock.mm, objc_block_CxxRefInBlock.mm:52, 1, CXX_REF_CAPTURED_IN_BLOCK, no_bucket, ERROR, [Captured variable &y defined here,Using captured &y] codetoanalyze/objcpp/self-in-block/CxxRefInBlock.mm, objc_block_CxxRefInBlock.mm:114, 1, CXX_REF_CAPTURED_IN_BLOCK, no_bucket, ERROR, [Captured variable &y defined here,Using captured &y] codetoanalyze/objcpp/self-in-block/CxxRefInBlock.mm, objc_block_CxxRefInBlock.mm:114, 2, CXX_REF_CAPTURED_IN_BLOCK, no_bucket, ERROR, [Captured variable &z defined here,Using captured &z] +codetoanalyze/objcpp/self-in-block/CxxStringInBlock.mm, objc_block_CxxStringInBlock.mm:27, 1, CXX_STRING_CAPTURED_IN_BLOCK, no_bucket, ERROR, [Captured variable &c defined here,Using captured &c]