From 37fdeb84bc295c9b1dfd282686e959307a1ec33d Mon Sep 17 00:00:00 2001 From: Dulma Rodriguez Date: Fri, 4 Dec 2015 07:30:29 -0800 Subject: [PATCH] Moving the property frontend checkers to a dummy procdesc Summary: public This puts the bugs found with the checker strong delegate to a dummy method. The error message will appear in the line of the class implementation definition, since the properties are likely to be defined in the h file, and getting the reporting in a file that is not the current is difficult. Reviewed By: ddino Differential Revision: D2718016 fb-gh-sync-id: 66273a4 --- infer/src/clang/cFrontend.ml | 2 +- infer/src/clang/cFrontend_checkers.ml | 19 ++++++------ infer/src/clang/cFrontend_checkers.mli | 2 +- infer/src/clang/cFrontend_errors.ml | 30 ++++++------------- infer/src/clang/cFrontend_errors.mli | 3 +- infer/src/clang/cFrontend_utils.ml | 2 +- infer/src/clang/cFrontend_utils.mli | 2 +- infer/src/clang/cMethod_trans.ml | 19 ++++++++++++ infer/src/clang/cMethod_trans.mli | 3 ++ infer/src/clang/objcProperty_decl.ml | 8 ++--- infer/src/clang/objcProperty_decl.mli | 2 +- .../endtoend/objc/StrongDelegateTest.java | 6 +--- 12 files changed, 51 insertions(+), 47 deletions(-) diff --git a/infer/src/clang/cFrontend.ml b/infer/src/clang/cFrontend.ml index a5164d80e..0e88a00ab 100644 --- a/infer/src/clang/cFrontend.ml +++ b/infer/src/clang/cFrontend.ml @@ -63,7 +63,7 @@ let rec translate_one_declaration tenv cg cfg parent_dec dec = let type_ptr_to_sil_type = CTypes_decl.type_ptr_to_sil_type in ignore (ObjcInterface_decl.interface_impl_declaration type_ptr_to_sil_type tenv dec); CMethod_declImpl.process_methods tenv cg cfg curr_class decl_list; - CFrontend_errors.check_for_property_errors cfg curr_class + CFrontend_errors.check_for_property_errors cfg cg tenv curr_class decl_info | CXXMethodDecl (decl_info, name_info, type_ptr, function_decl_info, _) | CXXConstructorDecl (decl_info, name_info, type_ptr, function_decl_info, _) -> diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml index 104fb6543..39b84fd91 100644 --- a/infer/src/clang/cFrontend_checkers.ml +++ b/infer/src/clang/cFrontend_checkers.ml @@ -14,7 +14,7 @@ open General_utils (* === Warnings on properties === *) (* Strong Delegate Warning: a property with name delegate should not be declared strong *) -let checker_strong_delegate_warning pname ptype = +let checker_strong_delegate_warning class_decl_info pname ptype = Printing.log_out "Checking for STRONG_DELEGATE property warning\n"; let delegate_regexp = Str.regexp_string_case_fold "delegate" in let pname_contains_delegate = try @@ -22,11 +22,12 @@ let checker_strong_delegate_warning pname ptype = with Not_found -> false in let condition = pname_contains_delegate && ObjcProperty_decl.is_strong_property ptype in - let warning_desc= {name = "STRONG_DELEGATE_WARNING"; - description = "Property or ivar "^pname.Clang_ast_t.ni_name^" declared strong"; - suggestion = "In general delegates should be declared weak or assign"; - loc = string_of_int (ObjcProperty_decl.property_line ptype); - } in + let warning_desc = + { name = "STRONG_DELEGATE_WARNING"; + description = "Property or ivar "^pname.Clang_ast_t.ni_name^" declared strong"; + suggestion = "In general delegates should be declared weak or assign"; + loc = CLocation.get_sil_location_from_range class_decl_info.Clang_ast_t.di_source_range true; + } in (condition, warning_desc) (* Direct Atomic Property access: @@ -41,9 +42,7 @@ let direct_atomic_property_access context stmt_info ivar_name = Ast_utils.get_class_name_from_member n | _ -> Ident.create_fieldname (Mangled.from_string "") 0, "" in let tname = Sil.TN_csu (Sil.Class, Mangled.from_string cname) in - let line = match (fst stmt_info.Clang_ast_t.si_source_range).Clang_ast_t.sl_line with - | Some l -> string_of_int l - | _ -> "-1" in + let loc = CLocation.get_sil_location_from_range stmt_info.Clang_ast_t.si_source_range true in match Sil.tenv_lookup tenv tname with | Some Sil.Tstruct (flds1, flds2, _, _, _, _, _) -> (* We give the warning when: @@ -57,7 +56,7 @@ let direct_atomic_property_access context stmt_info ivar_name = description = "Direct access to ivar " ^ (Ident.fieldname_to_string ivar) ^ " of an atomic property"; suggestion = "Accessing an ivar of an atomic property makes the property nonatomic"; - loc = line; + loc = loc; } in (condition, Some warning_desc) | _ -> (false, None) (* No warning *) diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index df99803d9..83d6ca9d3 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/infer/src/clang/cFrontend_checkers.mli @@ -11,7 +11,7 @@ (* === Warnings on properties === *) (* Strong Delegate Warning: a property with name delegate should not be declared strong *) -val checker_strong_delegate_warning : Clang_ast_t.named_decl_info -> +val checker_strong_delegate_warning : Clang_ast_t.decl_info -> Clang_ast_t.named_decl_info -> ObjcProperty_decl.property_type -> (bool * CFrontend_utils.General_utils.warning_desc) diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index d5fb087ed..0595b6788 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -26,12 +26,7 @@ let ivar_access_checker_list = [CFrontend_checkers.direct_atomic_property_acces (* Add a frontend warning with a description desc at location loc to the errlog of a proc desc *) let log_frontend_warning pdesc warn_desc = - let loc = { - Location.line = int_of_string warn_desc.loc; - Location.col = -1; - Location.file = DB.source_file_empty; - Location.nLOC = -1; - } in + let loc = warn_desc.loc in let errlog = Cfg.Procdesc.get_err_log pdesc in let err_desc = Errdesc.explain_frontend_warning warn_desc.description warn_desc.suggestion loc in @@ -41,22 +36,15 @@ let log_frontend_warning pdesc warn_desc = Reporting.log_error_from_errlog errlog exn ~loc:(Some loc) (* Call all checkers on properties of class c *) -let check_for_property_errors cfg c = - let qual_setter setter_name = - let cname = CContext.get_curr_class_name c in - mk_procname_from_objc_method cname setter_name Procname.Instance_objc_method in +let check_for_property_errors cfg cg tenv c decl_info = + let class_name = CContext.get_curr_class_name c in let do_one_property (pname, property) = - let (_, _, _, _, (setter_name, _), ndi) = property in - let qual_setter = qual_setter setter_name in - match Cfg.Procdesc.find_from_name cfg qual_setter with - | Some pdesc_setter -> (* Property warning will be added to the proc desc of the setter *) - Printing.log_out "Found setter for property %s\n" pname.Clang_ast_t.ni_name; - IList.iter (fun checker -> - let (condition, warning_desc) = checker pname property in - if condition then log_frontend_warning pdesc_setter warning_desc) property_checkers_list - | None -> - Printing.log_out "NO Setter Found for property %s\n" pname.Clang_ast_t.ni_name; - () in + IList.iter (fun checker -> + let (condition, warning_desc) = checker decl_info pname property in + if condition then + let proc_desc = + CMethod_trans.get_method_for_frontend_checks cfg cg tenv class_name decl_info in + log_frontend_warning proc_desc warning_desc) property_checkers_list in let properties = ObjcProperty_decl.find_properties_class c in Printing.log_out "Retrieved all properties of the class...\n"; IList.iter do_one_property properties diff --git a/infer/src/clang/cFrontend_errors.mli b/infer/src/clang/cFrontend_errors.mli index 2604ba115..6e9f7dfe2 100644 --- a/infer/src/clang/cFrontend_errors.mli +++ b/infer/src/clang/cFrontend_errors.mli @@ -12,7 +12,8 @@ (* Checks for warnings on properties of class c *) -val check_for_property_errors : Cfg.cfg -> CContext.curr_class -> unit +val check_for_property_errors : Cfg.cfg -> Cg.t -> Sil.tenv -> CContext.curr_class -> + Clang_ast_t.decl_info -> unit (* Call checkers on a specific access of an ivar *) val check_for_ivar_errors : diff --git a/infer/src/clang/cFrontend_utils.ml b/infer/src/clang/cFrontend_utils.ml index b7dc82a3a..da9d1f2ca 100644 --- a/infer/src/clang/cFrontend_utils.ml +++ b/infer/src/clang/cFrontend_utils.ml @@ -398,7 +398,7 @@ struct name : string; description : string; suggestion : string; (* an optional suggestion or correction *) - loc : string; + loc : Location.t; } type var_info = Clang_ast_t.decl_info * Clang_ast_t.type_ptr * Clang_ast_t.var_decl_info * bool diff --git a/infer/src/clang/cFrontend_utils.mli b/infer/src/clang/cFrontend_utils.mli index edb4e4935..2b8e6824b 100644 --- a/infer/src/clang/cFrontend_utils.mli +++ b/infer/src/clang/cFrontend_utils.mli @@ -129,7 +129,7 @@ sig name : string; description : string; suggestion : string; (* an optional suggestion or correction *) - loc : string; + loc : Location.t; } type var_info = Clang_ast_t.decl_info * Clang_ast_t.type_ptr * Clang_ast_t.var_decl_info * bool diff --git a/infer/src/clang/cMethod_trans.ml b/infer/src/clang/cMethod_trans.ml index 1e107c797..0169c2576 100644 --- a/infer/src/clang/cMethod_trans.ml +++ b/infer/src/clang/cMethod_trans.ml @@ -397,3 +397,22 @@ let create_procdesc_with_pointer context pointer class_name_opt name tp = let instance_to_method_call_type instance = if instance then MCVirtual else MCStatic + +let get_method_for_frontend_checks cfg cg tenv class_name decl_info = + let stmt_info = Ast_expressions.make_stmt_info decl_info in + let source_range = decl_info.Clang_ast_t.di_source_range in + let proc_name = General_utils.mk_procname_from_objc_method class_name "frontendChecks" + Procname.Class_objc_method in + match Cfg.Procdesc.find_from_name cfg proc_name with + | Some pdesc -> pdesc + | None -> + let ms = CMethod_signature.make_ms proc_name [] (Clang_ast_types.pointer_to_type_ptr "-1") + [] source_range false false CFrontend_config.OBJC None in + let body = [Clang_ast_t.CompoundStmt (stmt_info, [])] in + ignore (create_local_procdesc cfg tenv ms body [] false); + let pdesc = Option.get (Cfg.Procdesc.find_from_name cfg proc_name) in + let start_node = Cfg.Procdesc.get_start_node pdesc in + let end_node = Cfg.Procdesc.get_exit_node pdesc in + Cfg.Node.set_succs_exn start_node [end_node] []; + Cg.add_defined_node cg proc_name; + pdesc diff --git a/infer/src/clang/cMethod_trans.mli b/infer/src/clang/cMethod_trans.mli index f329a8419..4976583b3 100644 --- a/infer/src/clang/cMethod_trans.mli +++ b/infer/src/clang/cMethod_trans.mli @@ -45,3 +45,6 @@ val get_method_name_from_clang : Sil.tenv -> CMethod_signature.method_signature val create_procdesc_with_pointer : CContext.t -> Clang_ast_t.pointer -> string option -> string -> Clang_ast_t.type_ptr -> Procname.t + +val get_method_for_frontend_checks : Cfg.cfg -> Cg.t -> Sil.tenv -> string -> + Clang_ast_t.decl_info -> Cfg.Procdesc.t diff --git a/infer/src/clang/objcProperty_decl.ml b/infer/src/clang/objcProperty_decl.ml index 11e439d52..461b974bc 100644 --- a/infer/src/clang/objcProperty_decl.ml +++ b/infer/src/clang/objcProperty_decl.ml @@ -226,12 +226,10 @@ let is_strong_property property_type = | `Strong -> true | _ -> false) attrs -let property_line property_type = +let property_loc property_type = let _, attrs, decl_info, _, _, _ = property_type in - let src_range = fst decl_info.Clang_ast_t.di_source_range in - match src_range.Clang_ast_t.sl_line with - | Some l -> l - | _ -> -1 + let src_range = decl_info.Clang_ast_t.di_source_range in + CLocation.get_sil_location_from_range src_range true let prepare_dynamic_property curr_class decl_info property_impl_decl_info = let pname = Ast_utils.property_name property_impl_decl_info in diff --git a/infer/src/clang/objcProperty_decl.mli b/infer/src/clang/objcProperty_decl.mli index 3da5279e8..39a8ec3c7 100644 --- a/infer/src/clang/objcProperty_decl.mli +++ b/infer/src/clang/objcProperty_decl.mli @@ -81,7 +81,7 @@ val get_ivar_name : Clang_ast_t.named_decl_info -> Clang_ast_t.named_decl_info o val is_strong_property : property_type -> bool (* Given a property type returns whether the property line *) -val property_line : property_type -> int +val property_loc : property_type -> Location.t (* Checks whether mname is a getter or setter of the ivar given in dr_name *) val is_getter_setter : diff --git a/infer/tests/endtoend/objc/StrongDelegateTest.java b/infer/tests/endtoend/objc/StrongDelegateTest.java index cbaaf6363..1e72cca63 100644 --- a/infer/tests/endtoend/objc/StrongDelegateTest.java +++ b/infer/tests/endtoend/objc/StrongDelegateTest.java @@ -56,11 +56,7 @@ public class StrongDelegateTest { STRONG_DELEGATE_WARNING, FILE, new String[]{ - "setDelegate:", - "setDelegateFile:", - "setDelegated:", - "setFileDelegate:", - "setOneDelegateFile:" + "frontendChecks", } ) );