From bcb1422a9a3cf6cd15178755548efe73ee4d3b3a Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Tue, 12 Jul 2016 05:06:41 -0700 Subject: [PATCH] Implement direct atomic var check with the new ivar to property map Reviewed By: akotulski Differential Revision: D3542678 fbshipit-source-id: 93eed9e --- facebook-clang-plugins | 2 +- infer/src/clang/cContext.ml | 4 +- infer/src/clang/cContext.mli | 4 +- infer/src/clang/cField_decl.ml | 14 ------ infer/src/clang/cField_decl.mli | 2 - infer/src/clang/cFrontend_checkers.ml | 66 +++++++++++++++----------- infer/src/clang/cFrontend_checkers.mli | 3 +- infer/src/clang/cFrontend_config.ml | 2 + infer/src/clang/cFrontend_config.mli | 2 + infer/src/clang/cFrontend_errors.ml | 4 +- infer/src/clang/cFrontend_utils.ml | 5 ++ infer/src/clang/cFrontend_utils.mli | 2 + infer/src/clang/cMain.ml | 4 +- 13 files changed, 60 insertions(+), 54 deletions(-) diff --git a/facebook-clang-plugins b/facebook-clang-plugins index 8e5c361a0..155fb6d52 160000 --- a/facebook-clang-plugins +++ b/facebook-clang-plugins @@ -1 +1 @@ -Subproject commit 8e5c361a01e60283aadbda5932f606a958372741 +Subproject commit 155fb6d52891843b14c3837083aeb492b7b20d16 diff --git a/infer/src/clang/cContext.ml b/infer/src/clang/cContext.ml index b40b7f830..a0f2c9a41 100644 --- a/infer/src/clang/cContext.ml +++ b/infer/src/clang/cContext.ml @@ -163,12 +163,12 @@ let is_curr_proc_objc_getter context field_name = let attrs = Cfg.Procdesc.get_attributes context.procdesc in match attrs.ProcAttributes.objc_accessor with | Some ProcAttributes.Objc_getter field -> - Ident.fieldname_equal field field_name + (Ident.fieldname_to_string field) = field_name | _ -> false let is_curr_proc_objc_setter context field_name = let attrs = Cfg.Procdesc.get_attributes context.procdesc in match attrs.ProcAttributes.objc_accessor with | Some ProcAttributes.Objc_setter field -> - Ident.fieldname_equal field field_name + (Ident.fieldname_to_string field) = field_name | _ -> false diff --git a/infer/src/clang/cContext.mli b/infer/src/clang/cContext.mli index d7a6137bf..46306ff25 100644 --- a/infer/src/clang/cContext.mli +++ b/infer/src/clang/cContext.mli @@ -71,6 +71,6 @@ val is_objc_instance : t -> bool val get_outer_procname : t -> Procname.t -val is_curr_proc_objc_getter : t -> Ident.fieldname -> bool +val is_curr_proc_objc_getter : t -> string -> bool -val is_curr_proc_objc_setter : t -> Ident.fieldname -> bool +val is_curr_proc_objc_setter : t -> string -> bool diff --git a/infer/src/clang/cField_decl.ml b/infer/src/clang/cField_decl.ml index 421193de7..a57a88c8b 100644 --- a/infer/src/clang/cField_decl.ml +++ b/infer/src/clang/cField_decl.ml @@ -92,17 +92,3 @@ let add_missing_fields tenv class_name ck fields = Printing.log_out " Updating info for class '%s' in tenv\n" class_name; Tenv.add tenv class_tn_name class_type_info | _ -> () - -(* checks if ivar is defined among a set of fields and if it is atomic *) -let is_ivar_atomic ivar fields = - let do_one_annot a = - (a.Typ.class_name = Config.property_attributes) && - IList.exists (fun p -> p = CFrontend_config.atomic_att) a.Typ.parameters in - let has_atomic_annot ans = - IList.exists (fun (a, _) -> do_one_annot a) ans in - try - let _, _, annot = IList.find (fun (fn, _, _) -> Ident.fieldname_equal ivar fn) fields in - has_atomic_annot annot - with Not_found -> ( - Printing.log_out "NOT Found field ivar = '%s' " (Ident.fieldname_to_string ivar); - false) diff --git a/infer/src/clang/cField_decl.mli b/infer/src/clang/cField_decl.mli index 3461f5dd0..d806ab1e1 100644 --- a/infer/src/clang/cField_decl.mli +++ b/infer/src/clang/cField_decl.mli @@ -24,5 +24,3 @@ val build_sil_field : Ast_utils.type_ptr_to_sil_type -> Tenv.t -> Clang_ast_t.na Clang_ast_t.type_ptr -> Clang_ast_t.property_attribute list -> field_type val add_missing_fields : Tenv.t -> string -> Csu.class_kind -> field_type list -> unit - -val is_ivar_atomic : Ident.fieldname -> Typ.struct_fields -> bool diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml index f7fe757a4..60d929485 100644 --- a/infer/src/clang/cFrontend_checkers.ml +++ b/infer/src/clang/cFrontend_checkers.ml @@ -31,6 +31,21 @@ type warning_desc = { (* Helper functions *) +let get_ivar_attributes ivar_decl = + let open Clang_ast_t in + match ivar_decl with + | ObjCIvarDecl (ivar_decl_info, _, _, _, _) -> + (match Ast_utils.get_property_of_ivar ivar_decl_info.Clang_ast_t.di_pointer with + | Some ObjCPropertyDecl (_, _, obj_c_property_decl_info) -> + obj_c_property_decl_info.Clang_ast_t.opdi_property_attributes + | _ -> []) + | _ -> [] + + +(* checks if ivar is defined among a set of fields and if it is atomic *) +let is_ivar_atomic attributes = + IList.exists (Ast_utils.property_attribute_eq `Atomic) attributes + let name_contains_word pname word = let rexp = Str.regexp_string_case_fold word in try @@ -195,36 +210,31 @@ let global_var_init_with_calls_warning decl = (* Direct Atomic Property access: a property declared atomic should not be accessed directly via its ivar *) -let direct_atomic_property_access_warning context stmt_info ivar_name = - let tenv = CContext.get_tenv context in - let mname = proc_name_from_context context in - let ivar, cname = match ivar_name with - | Some n -> - General_utils.mk_class_field_name n, - Ast_utils.get_class_name_from_member n - | _ -> Ident.create_fieldname (Mangled.from_string "") 0, "" in - let tname = Typename.TN_csu (Csu.Class Csu.Objc, Mangled.from_string cname) in - let condition = match Tenv.lookup tenv tname with - | Some { Typ.instance_fields; static_fields } -> +let direct_atomic_property_access_warning context stmt_info ivar_decl_ref = + match Ast_utils.get_decl ivar_decl_ref.Clang_ast_t.dr_decl_pointer with + | Some (ObjCIvarDecl (_, named_decl_info, _, _, _) as d) -> + let ivar_name = named_decl_info.Clang_ast_t.ni_name in + let mname = proc_name_from_context context in + let condition = (* We give the warning when: - (1) the property has the atomic attribute and - (2) the access of the ivar is not in a getter or setter method. - (3) the access of the ivar is not in the init method - Last two conditions avoids false positives *) - (CField_decl.is_ivar_atomic ivar (instance_fields @ static_fields)) - && not (CContext.is_curr_proc_objc_getter context ivar) - && not (CContext.is_curr_proc_objc_setter context ivar) + (1) the property has the atomic attribute and + (2) the access of the ivar is not in a getter or setter method. + (3) the access of the ivar is not in the init method + Last two conditions avoids false positives *) + is_ivar_atomic (get_ivar_attributes d) + && not (CContext.is_curr_proc_objc_getter context ivar_name) + && not (CContext.is_curr_proc_objc_setter context ivar_name) && not (Procname.is_constructor mname) - && not (Procname.is_objc_dealloc mname) - | _ -> false in - if condition then - Some { - name = "DIRECT_ATOMIC_PROPERTY_ACCESS"; - 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 = location_from_sinfo stmt_info; } - else None + && not (Procname.is_objc_dealloc mname) in + if condition then + Some { + name = "DIRECT_ATOMIC_PROPERTY_ACCESS"; + description = "Direct access to ivar " ^ ivar_name ^ + " of an atomic property"; + suggestion = "Accessing an ivar of an atomic property makes the property nonatomic"; + loc = location_from_sinfo stmt_info; } + else None + | _ -> None (* CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK: C++ references diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index 7b30543ed..0ee535dad 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/infer/src/clang/cFrontend_checkers.mli @@ -29,8 +29,7 @@ val assign_pointer_warning : Clang_ast_t.decl_info -> Clang_ast_t.named_decl_inf (* Direct Atomic Property access: a property declared atomic should not be accesses directly via its iva *) val direct_atomic_property_access_warning : - CContext.t -> Clang_ast_t.stmt_info -> Clang_ast_t.named_decl_info option -> - warning_desc option + CContext.t -> Clang_ast_t.stmt_info -> Clang_ast_t.decl_ref -> warning_desc option (* CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK: C++ references should not be captured in blocks. *) diff --git a/infer/src/clang/cFrontend_config.ml b/infer/src/clang/cFrontend_config.ml index a883e767f..6c58f6696 100644 --- a/infer/src/clang/cFrontend_config.ml +++ b/infer/src/clang/cFrontend_config.ml @@ -49,6 +49,8 @@ let pointer_decl_index = ref Clang_ast_main.PointerMap.empty let pointer_stmt_index = ref Clang_ast_main.PointerMap.empty +let ivar_to_property_index = ref Clang_ast_main.PointerMap.empty + let emtpy_name_category ="EMPTY_NAME_CATEGORY_FOR_" let objc_object = "objc_object" diff --git a/infer/src/clang/cFrontend_config.mli b/infer/src/clang/cFrontend_config.mli index ff4d1ed21..7f99a375f 100644 --- a/infer/src/clang/cFrontend_config.mli +++ b/infer/src/clang/cFrontend_config.mli @@ -21,6 +21,8 @@ val pointer_decl_index : Clang_ast_t.decl Clang_ast_main.PointerMap.t ref val pointer_stmt_index : Clang_ast_t.stmt Clang_ast_main.PointerMap.t ref +val ivar_to_property_index : Clang_ast_t.decl Clang_ast_main.PointerMap.t ref + val objc_object : string val id_cl : string diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index fb5e370fa..2a0e40cbc 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -109,8 +109,8 @@ let run_frontend_checkers_on_stmt trans_state instr = let cfg = context.CContext.cfg in match instr with | ObjCIvarRefExpr(stmt_info, _, _, obj_c_ivar_ref_expr_info) -> - let dr_name = obj_c_ivar_ref_expr_info.Clang_ast_t.ovrei_decl_ref.Clang_ast_t.dr_name in - let call_checker_for_ivar = checkers_for_ivar context stmt_info dr_name in + let dr_ref = obj_c_ivar_ref_expr_info.Clang_ast_t.ovrei_decl_ref in + let call_checker_for_ivar = checkers_for_ivar context stmt_info dr_ref in invoke_set_of_checkers call_checker_for_ivar cfg cg (Some pdesc) ivar_access_checker_list | BlockExpr(stmt_info, _ , _, Clang_ast_t.BlockDecl (_, block_decl_info)) -> let captured_block_vars = block_decl_info.Clang_ast_t.bdi_captured_variables in diff --git a/infer/src/clang/cFrontend_utils.ml b/infer/src/clang/cFrontend_utils.ml index 4b0e8deb4..b1778fc63 100644 --- a/infer/src/clang/cFrontend_utils.ml +++ b/infer/src/clang/cFrontend_utils.ml @@ -291,6 +291,11 @@ struct | Some decl_ref -> get_decl decl_ref.Clang_ast_t.dr_decl_pointer | None -> None + let get_property_of_ivar decl_ptr = + try + Some (Clang_ast_main.PointerMap.find decl_ptr !CFrontend_config.ivar_to_property_index) + with Not_found -> Printing.log_stats "property with pointer %d not found\n" decl_ptr; None + let update_sil_types_map type_ptr sil_type = CFrontend_config.sil_types_map := Clang_ast_types.TypePointerMap.add type_ptr sil_type !CFrontend_config.sil_types_map diff --git a/infer/src/clang/cFrontend_utils.mli b/infer/src/clang/cFrontend_utils.mli index 960972c09..527081a74 100644 --- a/infer/src/clang/cFrontend_utils.mli +++ b/infer/src/clang/cFrontend_utils.mli @@ -86,6 +86,8 @@ sig val get_decl_opt_with_decl_ref : Clang_ast_t.decl_ref option -> Clang_ast_t.decl option + val get_property_of_ivar : Clang_ast_t.pointer -> Clang_ast_t.decl option + val update_sil_types_map : Clang_ast_t.type_ptr -> Typ.t -> unit val update_enum_map : Clang_ast_t.pointer -> Sil.exp -> unit diff --git a/infer/src/clang/cMain.ml b/infer/src/clang/cMain.ml index 65a1126c4..005885811 100644 --- a/infer/src/clang/cMain.ml +++ b/infer/src/clang/cMain.ml @@ -49,10 +49,12 @@ let do_run source_path ast_path = | None -> "stdin of " ^ source_path, validate_decl_from_stdin () in - let decl_index, stmt_index, type_index = Clang_ast_main.index_node_pointers ast_decl in + let decl_index, stmt_index, type_index, ivar_to_property_index = + Clang_ast_main.index_node_pointers ast_decl in CFrontend_config.pointer_decl_index := decl_index; CFrontend_config.pointer_stmt_index := stmt_index; CFrontend_config.pointer_type_index := type_index; + CFrontend_config.ivar_to_property_index := ivar_to_property_index; CFrontend_config.json := ast_filename; CLocation.check_source_file source_path; let source_file = CLocation.source_file_from_path source_path in