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

@@ -1 +1 @@
-Subproject commit 8e5c361a01e60283aadbda5932f606a958372741
+Subproject commit 155fb6d52891843b14c3837083aeb492b7b20d16

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

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

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)

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

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 direct_atomic_property_access_warning context stmt_info ivar_decl_ref =
-  let tenv = CContext.get_tenv context in
+  match Ast_utils.get_decl ivar_decl_ref.Clang_ast_t.dr_decl_pointer with
-  let mname = proc_name_from_context context in
+  | Some (ObjCIvarDecl (_, named_decl_info, _, _, _) as d) ->
-  let ivar, cname = match ivar_name with
+      let ivar_name = named_decl_info.Clang_ast_t.ni_name in
-    | Some n ->
+      let mname = proc_name_from_context context in
-        General_utils.mk_class_field_name n,
+      let condition =
-        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 } ->
         (* We give the warning when:
-             (1) the property has the atomic attribute and
+           (1) the property has the atomic attribute and
-               (2) the access of the ivar is not in a getter or setter method.
+           (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
+           (3) the access of the ivar is not in the init method
-                   Last two conditions avoids false positives *)
+           Last two conditions avoids false positives *)
-        (CField_decl.is_ivar_atomic ivar (instance_fields @ static_fields))
+        is_ivar_atomic (get_ivar_attributes d)
-        && not (CContext.is_curr_proc_objc_getter context ivar)
+        && not (CContext.is_curr_proc_objc_getter context ivar_name)
-        && not (CContext.is_curr_proc_objc_setter context ivar)
+        && not (CContext.is_curr_proc_objc_setter context ivar_name)
         && not (Procname.is_constructor mname)
-        && not (Procname.is_objc_dealloc mname)
+        && not (Procname.is_objc_dealloc mname) in
-    | _ -> false in
+      if condition then
-  if condition then
+        Some {
-    Some {
+          name = "DIRECT_ATOMIC_PROPERTY_ACCESS";
-      name = "DIRECT_ATOMIC_PROPERTY_ACCESS";
+          description = "Direct access to ivar " ^ ivar_name ^
-      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 = location_from_sinfo stmt_info; }
-      loc = location_from_sinfo stmt_info; }
+      else None
-  else None
+  | _ -> None
 
 
 (* CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK: C++ references

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 ->
+  CContext.t -> Clang_ast_t.stmt_info -> Clang_ast_t.decl_ref -> warning_desc option
-  warning_desc option
 
 (* CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK: C++ references
    should not be captured in blocks.  *)

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"

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

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 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_name 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

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

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

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