From 682f8c5355db6b7bc9f4dbd94271f9a4c4e9fb6d Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Fri, 7 Feb 2020 09:03:55 -0800 Subject: [PATCH] [SelfInBlock] Add the procname to the is_no_escape_block flag to improve the error message of the weakSelf In Noescape block check Summary: After looking at some reports with blocks inside blocks, it seemed more obvious that adding which method we are talking about makes more clear which block we are talking about. Reviewed By: mityal Differential Revision: D19789285 fbshipit-source-id: 20e0e6804 --- infer/src/IR/ProcAttributes.ml | 11 ++++++----- infer/src/IR/ProcAttributes.mli | 6 +++--- infer/src/checkers/SelfInBlock.ml | 14 +++++++++----- infer/src/clang/CType_decl.ml | 8 ++++---- infer/src/clang/CType_decl.mli | 4 ++-- infer/src/clang/cFrontend_decl.ml | 10 +++++----- infer/src/clang/cMethodSignature.ml | 10 +++++----- infer/src/clang/cMethodSignature.mli | 4 ++-- infer/src/clang/cMethod_trans.ml | 2 +- infer/src/clang/cModule_type.ml | 2 +- infer/src/clang/cTrans.ml | 13 +++++++------ infer/src/clang/cTrans_utils.ml | 8 ++++---- infer/src/clang/cTrans_utils.mli | 2 +- 13 files changed, 50 insertions(+), 44 deletions(-) diff --git a/infer/src/IR/ProcAttributes.ml b/infer/src/IR/ProcAttributes.ml index 7cef3e4f0..30e1d791c 100644 --- a/infer/src/IR/ProcAttributes.ml +++ b/infer/src/IR/ProcAttributes.ml @@ -48,8 +48,9 @@ type t = ; is_bridge_method: bool (** the procedure is a bridge method *) ; is_defined: bool (** true if the procedure is defined, and not just declared *) ; is_java_synchronized_method: bool (** the procedure is a Java synchronized method *) - ; is_no_escape_block: bool - (** The procedure is an Objective-C block that has the NS_NOESCAPE attribute *) + ; passed_as_noescape_block_to: Procname.t option + (** Present if the procedure is an Objective-C block that has been passed to the given + method in a position annotated with the NS_NOESCAPE attribute. *) ; is_no_return: bool (** the procedure is known not to return *) ; is_specialized: bool (** the procedure is a clone specialized for dynamic dispatch handling *) ; is_synthetic_method: bool (** the procedure is a synthetic method *) @@ -76,7 +77,7 @@ let default translation_unit proc_name = ; is_bridge_method= false ; is_defined= false ; is_java_synchronized_method= false - ; is_no_escape_block= false + ; passed_as_noescape_block_to= None ; is_no_return= false ; is_specialized= false ; is_synthetic_method= false @@ -108,7 +109,7 @@ let pp f ; is_bridge_method ; is_defined ; is_java_synchronized_method - ; is_no_escape_block + ; passed_as_noescape_block_to ; is_no_return ; is_specialized ; is_synthetic_method @@ -149,7 +150,7 @@ let pp f pp_bool_default ~default:default.is_defined "is_defined" is_defined f () ; pp_bool_default ~default:default.is_java_synchronized_method "is_java_synchronized_method" is_java_synchronized_method f () ; - pp_bool_default ~default:default.is_no_escape_block "is_no_escape_block" is_no_escape_block f () ; + F.fprintf f "; passed_as_noescape_block_to %a" (Pp.option Procname.pp) passed_as_noescape_block_to ; pp_bool_default ~default:default.is_no_return "is_no_return" is_no_return f () ; pp_bool_default ~default:default.is_specialized "is_specialized" is_specialized f () ; pp_bool_default ~default:default.is_synthetic_method "is_synthetic_method" is_synthetic_method f diff --git a/infer/src/IR/ProcAttributes.mli b/infer/src/IR/ProcAttributes.mli index 42ca416f3..4190eda8e 100644 --- a/infer/src/IR/ProcAttributes.mli +++ b/infer/src/IR/ProcAttributes.mli @@ -32,9 +32,9 @@ type t = ; is_bridge_method: bool (** the procedure is a bridge method *) ; is_defined: bool (** true if the procedure is defined, and not just declared *) ; is_java_synchronized_method: bool (** the procedure is a Java synchronized method *) - ; is_no_escape_block: bool - (** The procedure is an Objective-C block that is passed to a method in a position annotated - with NS_NOESCAPE *) + ; passed_as_noescape_block_to: Procname.t option + (** Present if the procedure is an Objective-C block that has been passed to the given + method in a position annotated with the NS_NOESCAPE attribute. *) ; is_no_return: bool (** the procedure is known not to return *) ; is_specialized: bool (** the procedure is a clone specialized for dynamic dispatch handling *) ; is_synthetic_method: bool (** the procedure is a synthetic method *) diff --git a/infer/src/checkers/SelfInBlock.ml b/infer/src/checkers/SelfInBlock.ml index 4fb538508..2cd259178 100644 --- a/infer/src/checkers/SelfInBlock.ml +++ b/infer/src/checkers/SelfInBlock.ml @@ -409,12 +409,13 @@ let report_mix_self_weakself_issues summary domain (weakSelf : DomainData.t) (se Reporting.log_error summary ~ltr ~loc:self.loc IssueType.mixed_self_weakself message -let report_weakself_in_no_escape_block_issues summary domain (weakSelf : DomainData.t) = +let report_weakself_in_no_escape_block_issues summary domain (weakSelf : DomainData.t) procname = let message = F.asprintf - "This block uses `%a` at %a. This is probably not needed since the block is passed to a \ - method in a position annotated with NS_NOESCAPE. Use `self` instead." + "This block uses `%a` at %a. This is probably not needed since the block is passed to the \ + method `%s` in a position annotated with NS_NOESCAPE. Use `self` instead." (Pvar.pp Pp.text) weakSelf.pvar Location.pp weakSelf.loc + (Procname.to_simplified_string procname) in let ltr = make_trace_use_self_weakself domain in Reporting.log_error summary ~ltr ~loc:weakSelf.loc IssueType.weak_self_in_noescape_block message @@ -455,8 +456,11 @@ let report_issues summary domain attributes = report_captured_strongself_issue domain summary domain_data ; (weakSelfList, selfList) | DomainData.WEAK_SELF -> - if attributes.ProcAttributes.is_no_escape_block then - report_weakself_in_no_escape_block_issues summary domain domain_data ; + ( match attributes.ProcAttributes.passed_as_noescape_block_to with + | Some procname -> + report_weakself_in_no_escape_block_issues summary domain domain_data procname + | None -> + () ) ; (domain_data :: weakSelfList, selfList) | DomainData.SELF -> (weakSelfList, domain_data :: selfList) diff --git a/infer/src/clang/CType_decl.ml b/infer/src/clang/CType_decl.ml index bd6de37ae..f505b1149 100644 --- a/infer/src/clang/CType_decl.ml +++ b/infer/src/clang/CType_decl.ml @@ -156,7 +156,7 @@ module BuildMethodSignature = struct let method_signature_of_decl qual_type_to_sil_type tenv method_decl ?block_return_type - ?(is_no_escape_block = false) procname = + ?(passed_as_noescape_block_to = None) procname = let decl_info = Clang_ast_proj.get_decl_tuple method_decl in let loc = decl_info.Clang_ast_t.di_source_range in let ret_type, return_param_typ, ret_typ_annot, has_added_return_param = @@ -182,7 +182,7 @@ module BuildMethodSignature = struct ; loc ; method_kind ; is_cpp_virtual - ; is_no_escape_block + ; passed_as_noescape_block_to ; is_no_return ; is_variadic ; pointer_to_parent @@ -191,12 +191,12 @@ module BuildMethodSignature = struct let method_signature_body_of_decl qual_type_to_sil_type tenv method_decl ?block_return_type - ?is_no_escape_block procname = + ?passed_as_noescape_block_to procname = let body = CMethodProperties.get_method_body method_decl in let init_list_instrs = CMethodProperties.get_init_list_instrs method_decl in let ms = method_signature_of_decl qual_type_to_sil_type tenv method_decl ?block_return_type - ?is_no_escape_block procname + ?passed_as_noescape_block_to procname in (ms, body, init_list_instrs) end diff --git a/infer/src/clang/CType_decl.mli b/infer/src/clang/CType_decl.mli index 62c199dfb..e1de737cc 100644 --- a/infer/src/clang/CType_decl.mli +++ b/infer/src/clang/CType_decl.mli @@ -53,7 +53,7 @@ val method_signature_of_decl : Tenv.t -> Clang_ast_t.decl -> ?block_return_type:Clang_ast_t.qual_type - -> ?is_no_escape_block:bool + -> ?passed_as_noescape_block_to:Procname.t option -> Procname.t -> CMethodSignature.t @@ -61,7 +61,7 @@ val method_signature_body_of_decl : Tenv.t -> Clang_ast_t.decl -> ?block_return_type:Clang_ast_t.qual_type - -> ?is_no_escape_block:bool + -> ?passed_as_noescape_block_to:Procname.t option -> Procname.t -> CMethodSignature.t * Clang_ast_t.stmt option diff --git a/infer/src/clang/cFrontend_decl.ml b/infer/src/clang/cFrontend_decl.ml index d5926973d..320e1ec06 100644 --- a/infer/src/clang/cFrontend_decl.ml +++ b/infer/src/clang/cFrontend_decl.ml @@ -75,16 +75,16 @@ module CFrontend_decl_funct (T : CModule_type.CTranslation) : CModule_type.CFron | None -> ([], None) in - let is_no_escape_block, procname, block_return_type = + let passed_as_noescape_block_to, procname, block_return_type = match block_data_opt with - | Some {CModule_type.is_no_escape_block_arg; procname; return_type} -> - (is_no_escape_block_arg, procname, Some return_type) + | Some {CModule_type.passed_as_noescape_block_to; procname; return_type} -> + (passed_as_noescape_block_to, procname, Some return_type) | _ -> - (false, CType_decl.CProcname.from_decl ~tenv func_decl, None) + (None, CType_decl.CProcname.from_decl ~tenv func_decl, None) in let ms, body_opt, extra_instrs = CType_decl.method_signature_body_of_decl tenv func_decl ?block_return_type - ~is_no_escape_block procname + ~passed_as_noescape_block_to procname in match body_opt with | Some body -> diff --git a/infer/src/clang/cMethodSignature.ml b/infer/src/clang/cMethodSignature.ml index 08de5caa1..001309976 100644 --- a/infer/src/clang/cMethodSignature.ml +++ b/infer/src/clang/cMethodSignature.ml @@ -35,7 +35,7 @@ type t = ; loc: Clang_ast_t.source_range ; method_kind: ClangMethodKind.t ; is_cpp_virtual: bool - ; is_no_escape_block: bool + ; passed_as_noescape_block_to: Procname.t option ; is_no_return: bool ; is_variadic: bool ; pointer_to_parent: Clang_ast_t.pointer option @@ -56,7 +56,7 @@ let is_setter {pointer_to_property_opt; params} = let mk name class_param params ret_type ?(has_added_return_param = false) attributes loc method_kind - ?(is_cpp_virtual = false) ?(is_no_escape_block = false) ?(is_no_return = false) + ?(is_cpp_virtual = false) ?(passed_as_noescape_block_to = None) ?(is_no_return = false) ?(is_variadic = false) pointer_to_parent pointer_to_property_opt return_param_typ access = { name ; access @@ -68,7 +68,7 @@ let mk name class_param params ret_type ?(has_added_return_param = false) attrib ; loc ; method_kind ; is_cpp_virtual - ; is_no_escape_block + ; passed_as_noescape_block_to ; is_no_return ; is_variadic ; pointer_to_parent @@ -81,8 +81,8 @@ let pp fmt ms = F.fprintf fmt "%a, %a (is_no_escape_block=%b)" Mangled.pp name (Typ.pp Pp.text) typ is_no_escape_block_arg in - Format.fprintf fmt "Method %a [%a]->%a %a(is_no_escape_block=%b)" + Format.fprintf fmt "Method %a [%a]->%a %a(passed_as_noescape_block_to=%a)" (Pp.of_string ~f:Procname.to_string) ms.name (Pp.comma_seq pp_param) ms.params (Typ.pp Pp.text) (fst ms.ret_type) (Pp.of_string ~f:Clang_ast_j.string_of_source_range) - ms.loc ms.is_no_escape_block + ms.loc (Pp.option Procname.pp) ms.passed_as_noescape_block_to diff --git a/infer/src/clang/cMethodSignature.mli b/infer/src/clang/cMethodSignature.mli index b7d7a1dff..b60182417 100644 --- a/infer/src/clang/cMethodSignature.mli +++ b/infer/src/clang/cMethodSignature.mli @@ -28,7 +28,7 @@ type t = ; loc: Clang_ast_t.source_range ; method_kind: ClangMethodKind.t ; is_cpp_virtual: bool - ; is_no_escape_block: bool + ; passed_as_noescape_block_to: Procname.t option ; is_no_return: bool ; is_variadic: bool ; pointer_to_parent: Clang_ast_t.pointer option @@ -50,7 +50,7 @@ val mk : -> Clang_ast_t.source_range -> ClangMethodKind.t -> ?is_cpp_virtual:bool - -> ?is_no_escape_block:bool + -> ?passed_as_noescape_block_to:Procname.t option -> ?is_no_return:bool -> ?is_variadic:bool -> Clang_ast_t.pointer option diff --git a/infer/src/clang/cMethod_trans.ml b/infer/src/clang/cMethod_trans.ml index 1a6b46eda..18dfd842c 100644 --- a/infer/src/clang/cMethod_trans.ml +++ b/infer/src/clang/cMethod_trans.ml @@ -245,7 +245,7 @@ let create_local_procdesc ?(set_objc_accessor_attr = false) trans_unit_ctx cfg t ; access ; is_defined= defined ; is_biabduction_model= Config.biabduction_models_mode - ; is_no_escape_block= ms.CMethodSignature.is_no_escape_block + ; passed_as_noescape_block_to= ms.CMethodSignature.passed_as_noescape_block_to ; is_no_return= ms.CMethodSignature.is_no_return ; is_variadic= ms.CMethodSignature.is_variadic ; sentinel_attr= find_sentinel_attribute ms.CMethodSignature.attributes diff --git a/infer/src/clang/cModule_type.ml b/infer/src/clang/cModule_type.ml index 205c32518..eae026dbc 100644 --- a/infer/src/clang/cModule_type.ml +++ b/infer/src/clang/cModule_type.ml @@ -10,7 +10,7 @@ open! IStd type block_data = { captured_vars: (Pvar.t * Typ.t) list ; context: CContext.t - ; is_no_escape_block_arg: bool + ; passed_as_noescape_block_to: Procname.t option ; procname: Procname.t ; return_type: Clang_ast_t.qual_type } diff --git a/infer/src/clang/cTrans.ml b/infer/src/clang/cTrans.ml index 26a0e3a10..ee39f9fe5 100644 --- a/infer/src/clang/cTrans.ml +++ b/infer/src/clang/cTrans.ml @@ -1253,14 +1253,14 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s let find_arg j _ = Int.equal i j in List.findi ~f:find_arg callee_ms.CMethodSignature.params in - let is_no_escape_block_arg = + let passed_as_noescape_block_to = match ms_param_type_i with | Some (_, {CMethodSignature.is_no_escape_block_arg}) -> - is_no_escape_block_arg + if is_no_escape_block_arg then Some callee_ms.CMethodSignature.name else None | None -> - false + None in - {trans_state_param with is_no_escape_block_arg} + {trans_state_param with passed_as_noescape_block_to} | _ -> trans_state_param in @@ -2798,10 +2798,11 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s CVar_decl.captured_vars_from_block_info context stmt_info.Clang_ast_t.si_source_range block_decl_info.Clang_ast_t.bdi_captured_variables in - let is_no_escape_block_arg = trans_state.is_no_escape_block_arg in + let passed_as_noescape_block_to = trans_state.passed_as_noescape_block_to in let res = closure_trans procname captured_vars context stmt_info expr_info in let block_data = - Some {CModule_type.captured_vars; context; is_no_escape_block_arg; procname; return_type} + Some + {CModule_type.captured_vars; context; passed_as_noescape_block_to; procname; return_type} in F.function_decl context.translation_unit_context context.tenv context.cfg decl block_data ; res diff --git a/infer/src/clang/cTrans_utils.ml b/infer/src/clang/cTrans_utils.ml index 36a80b723..17cc70cf6 100644 --- a/infer/src/clang/cTrans_utils.ml +++ b/infer/src/clang/cTrans_utils.ml @@ -127,9 +127,9 @@ type trans_state = ; var_exp_typ: (Exp.t * Typ.t) option ; opaque_exp: (Exp.t * Typ.t) option ; is_fst_arg_objc_instance_method_call: bool - ; is_no_escape_block_arg: bool - (** Current to-be-translated instruction is being passed as argument in a position annotated - with NS_NOESCAPE *) } + ; passed_as_noescape_block_to: Procname.t option + (** Current to-be-translated instruction is being passed as argument to the given method in + a position annotated with NS_NOESCAPE *) } let default_trans_state context = { context @@ -139,7 +139,7 @@ let default_trans_state context = ; var_exp_typ= None ; opaque_exp= None ; is_fst_arg_objc_instance_method_call= false - ; is_no_escape_block_arg= false } + ; passed_as_noescape_block_to= None } type control = diff --git a/infer/src/clang/cTrans_utils.mli b/infer/src/clang/cTrans_utils.mli index a9dc9ac1a..e506305a9 100644 --- a/infer/src/clang/cTrans_utils.mli +++ b/infer/src/clang/cTrans_utils.mli @@ -27,7 +27,7 @@ type trans_state = ; var_exp_typ: (Exp.t * Typ.t) option ; opaque_exp: (Exp.t * Typ.t) option ; is_fst_arg_objc_instance_method_call: bool - ; is_no_escape_block_arg: bool } + ; passed_as_noescape_block_to: Procname.t option } val default_trans_state : CContext.t -> trans_state