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
master
Dulma Rodriguez 9 years ago committed by facebook-github-bot-7
parent a1c1b10862
commit 37fdeb84bc

@ -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 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); ignore (ObjcInterface_decl.interface_impl_declaration type_ptr_to_sil_type tenv dec);
CMethod_declImpl.process_methods tenv cg cfg curr_class decl_list; 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, _) | CXXMethodDecl (decl_info, name_info, type_ptr, function_decl_info, _)
| CXXConstructorDecl (decl_info, name_info, type_ptr, function_decl_info, _) -> | CXXConstructorDecl (decl_info, name_info, type_ptr, function_decl_info, _) ->

@ -14,7 +14,7 @@ open General_utils
(* === Warnings on properties === *) (* === Warnings on properties === *)
(* Strong Delegate Warning: a property with name delegate should not be declared strong *) (* 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"; Printing.log_out "Checking for STRONG_DELEGATE property warning\n";
let delegate_regexp = Str.regexp_string_case_fold "delegate" in let delegate_regexp = Str.regexp_string_case_fold "delegate" in
let pname_contains_delegate = try let pname_contains_delegate = try
@ -22,11 +22,12 @@ let checker_strong_delegate_warning pname ptype =
with Not_found -> false in with Not_found -> false in
let condition = pname_contains_delegate let condition = pname_contains_delegate
&& ObjcProperty_decl.is_strong_property ptype in && ObjcProperty_decl.is_strong_property ptype in
let warning_desc= {name = "STRONG_DELEGATE_WARNING"; let warning_desc =
description = "Property or ivar "^pname.Clang_ast_t.ni_name^" declared strong"; { name = "STRONG_DELEGATE_WARNING";
suggestion = "In general delegates should be declared weak or assign"; description = "Property or ivar "^pname.Clang_ast_t.ni_name^" declared strong";
loc = string_of_int (ObjcProperty_decl.property_line ptype); suggestion = "In general delegates should be declared weak or assign";
} in loc = CLocation.get_sil_location_from_range class_decl_info.Clang_ast_t.di_source_range true;
} in
(condition, warning_desc) (condition, warning_desc)
(* Direct Atomic Property access: (* 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 Ast_utils.get_class_name_from_member n
| _ -> Ident.create_fieldname (Mangled.from_string "") 0, "" in | _ -> Ident.create_fieldname (Mangled.from_string "") 0, "" in
let tname = Sil.TN_csu (Sil.Class, Mangled.from_string cname) 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 let loc = CLocation.get_sil_location_from_range stmt_info.Clang_ast_t.si_source_range true in
| Some l -> string_of_int l
| _ -> "-1" in
match Sil.tenv_lookup tenv tname with match Sil.tenv_lookup tenv tname with
| Some Sil.Tstruct (flds1, flds2, _, _, _, _, _) -> | Some Sil.Tstruct (flds1, flds2, _, _, _, _, _) ->
(* We give the warning when: (* 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) ^ description = "Direct access to ivar " ^ (Ident.fieldname_to_string ivar) ^
" of an atomic property"; " of an atomic property";
suggestion = "Accessing an ivar of an atomic property makes the property nonatomic"; suggestion = "Accessing an ivar of an atomic property makes the property nonatomic";
loc = line; loc = loc;
} in } in
(condition, Some warning_desc) (condition, Some warning_desc)
| _ -> (false, None) (* No warning *) | _ -> (false, None) (* No warning *)

@ -11,7 +11,7 @@
(* === Warnings on properties === *) (* === Warnings on properties === *)
(* Strong Delegate Warning: a property with name delegate should not be declared strong *) (* 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 -> ObjcProperty_decl.property_type ->
(bool * CFrontend_utils.General_utils.warning_desc) (bool * CFrontend_utils.General_utils.warning_desc)

@ -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 *) (* 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 log_frontend_warning pdesc warn_desc =
let loc = { let loc = warn_desc.loc in
Location.line = int_of_string warn_desc.loc;
Location.col = -1;
Location.file = DB.source_file_empty;
Location.nLOC = -1;
} in
let errlog = Cfg.Procdesc.get_err_log pdesc in let errlog = Cfg.Procdesc.get_err_log pdesc in
let err_desc = let err_desc =
Errdesc.explain_frontend_warning warn_desc.description warn_desc.suggestion loc in 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) Reporting.log_error_from_errlog errlog exn ~loc:(Some loc)
(* Call all checkers on properties of class c *) (* Call all checkers on properties of class c *)
let check_for_property_errors cfg c = let check_for_property_errors cfg cg tenv c decl_info =
let qual_setter setter_name = let class_name = CContext.get_curr_class_name c in
let cname = CContext.get_curr_class_name c in
mk_procname_from_objc_method cname setter_name Procname.Instance_objc_method in
let do_one_property (pname, property) = let do_one_property (pname, property) =
let (_, _, _, _, (setter_name, _), ndi) = property in IList.iter (fun checker ->
let qual_setter = qual_setter setter_name in let (condition, warning_desc) = checker decl_info pname property in
match Cfg.Procdesc.find_from_name cfg qual_setter with if condition then
| Some pdesc_setter -> (* Property warning will be added to the proc desc of the setter *) let proc_desc =
Printing.log_out "Found setter for property %s\n" pname.Clang_ast_t.ni_name; CMethod_trans.get_method_for_frontend_checks cfg cg tenv class_name decl_info in
IList.iter (fun checker -> log_frontend_warning proc_desc warning_desc) property_checkers_list in
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
let properties = ObjcProperty_decl.find_properties_class c in let properties = ObjcProperty_decl.find_properties_class c in
Printing.log_out "Retrieved all properties of the class...\n"; Printing.log_out "Retrieved all properties of the class...\n";
IList.iter do_one_property properties IList.iter do_one_property properties

@ -12,7 +12,8 @@
(* Checks for warnings on properties of class c *) (* 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 *) (* Call checkers on a specific access of an ivar *)
val check_for_ivar_errors : val check_for_ivar_errors :

@ -398,7 +398,7 @@ struct
name : string; name : string;
description : string; description : string;
suggestion : string; (* an optional suggestion or correction *) 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 type var_info = Clang_ast_t.decl_info * Clang_ast_t.type_ptr * Clang_ast_t.var_decl_info * bool

@ -129,7 +129,7 @@ sig
name : string; name : string;
description : string; description : string;
suggestion : string; (* an optional suggestion or correction *) 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 type var_info = Clang_ast_t.decl_info * Clang_ast_t.type_ptr * Clang_ast_t.var_decl_info * bool

@ -397,3 +397,22 @@ let create_procdesc_with_pointer context pointer class_name_opt name tp =
let instance_to_method_call_type instance = let instance_to_method_call_type instance =
if instance then MCVirtual if instance then MCVirtual
else MCStatic 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

@ -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 -> val create_procdesc_with_pointer : CContext.t -> Clang_ast_t.pointer -> string option ->
string -> Clang_ast_t.type_ptr -> Procname.t 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

@ -226,12 +226,10 @@ let is_strong_property property_type =
| `Strong -> true | `Strong -> true
| _ -> false) attrs | _ -> false) attrs
let property_line property_type = let property_loc property_type =
let _, attrs, decl_info, _, _, _ = property_type in let _, attrs, decl_info, _, _, _ = property_type in
let src_range = fst decl_info.Clang_ast_t.di_source_range in let src_range = decl_info.Clang_ast_t.di_source_range in
match src_range.Clang_ast_t.sl_line with CLocation.get_sil_location_from_range src_range true
| Some l -> l
| _ -> -1
let prepare_dynamic_property curr_class decl_info property_impl_decl_info = let prepare_dynamic_property curr_class decl_info property_impl_decl_info =
let pname = Ast_utils.property_name property_impl_decl_info in let pname = Ast_utils.property_name property_impl_decl_info in

@ -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 val is_strong_property : property_type -> bool
(* Given a property type returns whether the property line *) (* 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 *) (* Checks whether mname is a getter or setter of the ivar given in dr_name *)
val is_getter_setter : val is_getter_setter :

@ -56,11 +56,7 @@ public class StrongDelegateTest {
STRONG_DELEGATE_WARNING, STRONG_DELEGATE_WARNING,
FILE, FILE,
new String[]{ new String[]{
"setDelegate:", "frontendChecks",
"setDelegateFile:",
"setDelegated:",
"setFileDelegate:",
"setOneDelegateFile:"
} }
) )
); );

Loading…
Cancel
Save