Add general context to linters framework to implement smarter checks

Summary:
Make checks context-aware, to increase flexibility.
As an example application of this change, whenever an atomic property is accessed from within a synchronized block, skip reporting a `DIRECT_ATOMIC_PROPERTY_ACCESS` warning.

Reviewed By: jvillard

Differential Revision: D3648831

fbshipit-source-id: c033f45

infer/src/clang/CLintersContext.ml

@@ -0,0 +1,14 @@
+(*
+ * Copyright (c) 2016 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *)
+
+type context = {
+  in_synchronized_block: bool;
+}
+
+let empty = { in_synchronized_block = false }

infer/src/clang/cFrontend_checkers.ml

@@ -154,7 +154,7 @@ let captured_variables_cxx_ref captured_vars =
 (* === Warnings on properties === *)
 
 (* Assing Pointer Warning: a property with a pointer type should not be declared `assign` *)
-let assign_pointer_warning decl_info pname obj_c_property_decl_info =
+let assign_pointer_warning _ decl_info pname obj_c_property_decl_info =
   let open Clang_ast_t in
   let condition =
     let has_assign_property () = ObjcProperty_decl.is_assign_property obj_c_property_decl_info in
@@ -180,7 +180,7 @@ let assign_pointer_warning decl_info pname obj_c_property_decl_info =
   else None
 
 (* Strong Delegate Warning: a property with name delegate should not be declared strong *)
-let strong_delegate_warning decl_info pname obj_c_property_decl_info =
+let strong_delegate_warning _ decl_info pname obj_c_property_decl_info =
   let condition = (name_contains_word pname "delegate")
                   && not (name_contains_word pname "queue")
                   && ObjcProperty_decl.is_strong_property obj_c_property_decl_info in
@@ -195,7 +195,7 @@ let strong_delegate_warning decl_info pname obj_c_property_decl_info =
 (* GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL warning: a global variable initialization should not *)
 (* contain calls to functions or methods as these can be expensive an delay the starting time *)
 (* of an app *)
-let global_var_init_with_calls_warning decl =
+let global_var_init_with_calls_warning _ decl =
   let decl_info, gvar =
     match Clang_ast_proj.get_named_decl_tuple decl with
     | Some (di, ndi) -> di, ndi.ni_name
@@ -217,7 +217,7 @@ 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 method_decl stmt_info ivar_decl_ref =
+let direct_atomic_property_access_warning context method_decl stmt_info ivar_decl_ref =
   let ivar_pointer = ivar_decl_ref.Clang_ast_t.dr_decl_pointer in
   match Ast_utils.get_decl ivar_pointer with
   | Some (ObjCIvarDecl (_, named_decl_info, _, _, _) as d) ->
@@ -226,12 +226,14 @@ let direct_atomic_property_access_warning method_decl stmt_info ivar_decl_ref =
         | _ -> "" in
       let ivar_name = named_decl_info.Clang_ast_t.ni_name 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
+        (* We warn when:
+           (1) we are not inside a @synchronized block
+           (2) the property has the atomic attribute and
+           (3) the access of the ivar is not in a getter or setter method
+           (4) 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 (context.CLintersContext.in_synchronized_block)
+        && is_ivar_atomic (get_ivar_attributes d)
         && not (is_method_property_accessor_of_ivar method_decl ivar_pointer)
         && not (Procname.is_objc_constructor method_name)
         && not (Procname.is_objc_dealloc method_name) in
@@ -250,7 +252,7 @@ let direct_atomic_property_access_warning method_decl stmt_info ivar_decl_ref =
 
 (* CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK: C++ references
    should not be captured in blocks.  *)
-let captured_cxx_ref_in_objc_block_warning stmt_info captured_vars =
+let captured_cxx_ref_in_objc_block_warning _ stmt_info captured_vars =
   let capt_refs = captured_variables_cxx_ref captured_vars in
   let var_descs =
     let var_desc vars var_named_decl_info =
@@ -272,7 +274,7 @@ let captured_cxx_ref_in_objc_block_warning stmt_info captured_vars =
 
 (* BAD_POINTER_COMPARISON: Fires whenever a NSNumber is dangerously coerced to
     a boolean in a comparison *)
-let bad_pointer_comparison_warning stmt_info stmts =
+let bad_pointer_comparison_warning _ stmt_info stmts =
   let rec condition stmts =
     let condition_aux stmt =
       match stmt with
@@ -309,7 +311,7 @@ let bad_pointer_comparison_warning stmt_info stmts =
 
 
 (* exist m1:  m1.body|- EF call_method(addObserver:) => exists m2 : m2.body |- EF call_method(removeObserver:) *)
-let checker_NSNotificationCenter decl_info decls =
+let checker_NSNotificationCenter _ decl_info decls =
   let exists_method_calling_addObserver =
     IList.exists (dec_body_eventually (call_method_on_nth is_self 1) "addObserver:selector:name:object:") decls in
   let exists_method_calling_addObserverForName =

infer/src/clang/cFrontend_checkers.mli

@@ -12,32 +12,39 @@ open! Utils
 (* === Warnings on properties === *)
 
 (* Strong Delegate Warning: a property with name delegate should not be declared strong *)
-val strong_delegate_warning : Clang_ast_t.decl_info -> Clang_ast_t.named_decl_info ->
+val strong_delegate_warning :
+  CLintersContext.context -> Clang_ast_t.decl_info -> Clang_ast_t.named_decl_info ->
   Clang_ast_t.obj_c_property_decl_info -> CIssue.issue_desc option
 
 (* Assing Pointer Warning: a property with a pointer type should not be declared `assign` *)
-val assign_pointer_warning : Clang_ast_t.decl_info -> Clang_ast_t.named_decl_info ->
+val assign_pointer_warning :
+  CLintersContext.context -> Clang_ast_t.decl_info -> Clang_ast_t.named_decl_info ->
   Clang_ast_t.obj_c_property_decl_info -> CIssue.issue_desc option
 
 (* Direct Atomic Property access:
    a property declared atomic should not be accesses directly via its iva *)
-val direct_atomic_property_access_warning : Clang_ast_t.decl -> Clang_ast_t.stmt_info ->
+val direct_atomic_property_access_warning :
+  CLintersContext.context -> Clang_ast_t.decl -> Clang_ast_t.stmt_info ->
   Clang_ast_t.decl_ref -> CIssue.issue_desc option
 
 (* CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK: C++ references
    should not be captured in blocks.  *)
-val captured_cxx_ref_in_objc_block_warning : Clang_ast_t.stmt_info ->
+val captured_cxx_ref_in_objc_block_warning :
+  CLintersContext.context -> Clang_ast_t.stmt_info ->
   Clang_ast_t.block_captured_variable list -> CIssue.issue_desc option
 
 val bad_pointer_comparison_warning :
+  CLintersContext.context ->
   Clang_ast_t.stmt_info -> Clang_ast_t.stmt list -> CIssue.issue_desc option
 
 (* REGISTERED_OBSERVER_BEING_DEALLOCATED: an object is registered in a notification center
    but not removed before deallocation *)
-val checker_NSNotificationCenter : Clang_ast_t.decl_info -> Clang_ast_t.decl list ->
-  CIssue.issue_desc option
+val checker_NSNotificationCenter :
+  CLintersContext.context ->
+  Clang_ast_t.decl_info -> Clang_ast_t.decl list -> CIssue.issue_desc option
 
 (* GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL warning: a global variable initialization should not *)
 (* contain calls to functions or methods as these can be expensive an delay the starting time *)
 (* of a program *)
-val global_var_init_with_calls_warning : Clang_ast_t.decl -> CIssue.issue_desc option
+val global_var_init_with_calls_warning :
+  CLintersContext.context -> Clang_ast_t.decl -> CIssue.issue_desc option

infer/src/clang/cFrontend_checkers_main.ml

@@ -7,12 +7,12 @@
  * of patent rights can be found in the PATENTS file in the same directory.
  *)
 
-let rec do_frontend_checks_stmt cfg cg method_decl stmt =
-  CFrontend_errors.run_frontend_checkers_on_stmt cfg cg method_decl stmt;
+let rec do_frontend_checks_stmt (context:CLintersContext.context) cfg cg method_decl stmt =
+  let context' = CFrontend_errors.run_frontend_checkers_on_stmt context cfg cg method_decl stmt in
   let stmts = CFrontend_utils.Ast_utils.get_stmts_from_stmt stmt in
-  IList.iter (do_frontend_checks_stmt cfg cg method_decl) stmts
+  IList.iter (do_frontend_checks_stmt context' cfg cg method_decl) stmts
 
-let rec do_frontend_checks_decl cfg cg decl =
+let rec do_frontend_checks_decl context cfg cg decl =
   let open Clang_ast_t in
   let info = Clang_ast_proj.get_decl_tuple decl in
   CLocation.update_curr_file info;
@@ -23,20 +23,20 @@ let rec do_frontend_checks_decl cfg cg decl =
    | CXXConversionDecl (_, _, _, fdi, _)
    | CXXDestructorDecl (_, _, _, fdi, _) ->
        (match fdi.Clang_ast_t.fdi_body with
-        | Some stmt -> do_frontend_checks_stmt cfg cg decl stmt
+        | Some stmt -> do_frontend_checks_stmt context cfg cg decl stmt
         | None -> ())
    | ObjCMethodDecl (_, _, mdi) ->
        (match mdi.Clang_ast_t.omdi_body with
-        | Some stmt -> do_frontend_checks_stmt cfg cg decl stmt
+        | Some stmt -> do_frontend_checks_stmt context cfg cg decl stmt
         | None -> ())
    | _ -> ());
-  CFrontend_errors.run_frontend_checkers_on_decl cfg cg decl;
+  let context' = CFrontend_errors.run_frontend_checkers_on_decl context cfg cg decl in
   match Clang_ast_proj.get_decl_context_tuple decl with
-  | Some (decls, _) -> IList.iter (do_frontend_checks_decl cfg cg) decls
+  | Some (decls, _) -> IList.iter (do_frontend_checks_decl context' cfg cg) decls
   | None -> ()
 
 let do_frontend_checks cfg cg ast =
   match ast with
   | Clang_ast_t.TranslationUnitDecl(_, decl_list, _, _) ->
-      IList.iter (do_frontend_checks_decl cfg cg) decl_list
+      IList.iter (do_frontend_checks_decl CLintersContext.empty cfg cg) decl_list
   | _ -> assert false (* NOTE: Assumes that an AST alsways starts with a TranslationUnitDecl *)

infer/src/clang/cFrontend_errors.ml

@@ -16,64 +16,64 @@ let property_checkers_list = [CFrontend_checkers.strong_delegate_warning;
                               CFrontend_checkers.assign_pointer_warning;]
 
 (* Invocation of checker belonging to property_checker_list *)
-let checkers_for_property decl_info pname_info pdi checker =
-  checker decl_info pname_info pdi
+let checkers_for_property decl_info pname_info pdi checker context =
+  checker context decl_info pname_info pdi
 
 (* List of checkers on ivar access *)
 let ivar_access_checker_list =  [CFrontend_checkers.direct_atomic_property_access_warning]
 
 (* Invocation of checker belonging to ivar_access_checker_list *)
-let checkers_for_ivar stmt_info method_decl ivar_decl_ref checker =
-  checker stmt_info method_decl ivar_decl_ref
+let checkers_for_ivar stmt_info method_decl ivar_decl_ref checker context =
+  checker context stmt_info method_decl ivar_decl_ref
 
 (* List of checkers for captured vars in objc blocks *)
 let captured_vars_checker_list = [CFrontend_checkers.captured_cxx_ref_in_objc_block_warning]
 
 (* Invocation of checker belonging to captured_vars_checker_list *)
-let checkers_for_capture_vars stmt_info captured_vars checker =
-  checker stmt_info captured_vars
+let checkers_for_capture_vars stmt_info captured_vars checker context =
+  checker context stmt_info captured_vars
 
 (* List of checkers on NSNotificationCenter *)
 let ns_notification_checker_list = [CFrontend_checkers.checker_NSNotificationCenter]
 
 (* Invocation of checker belonging to ns_notification_center_list *)
-let checkers_for_ns decl_info decls checker =
-  checker decl_info decls
+let checkers_for_ns decl_info decls checker context =
+  checker context decl_info decls
 
 (* List of checkers on global variables *)
 let global_var_checker_list = [CFrontend_checkers.global_var_init_with_calls_warning]
 
 (* Invocation of checker belonging to global_var_checker_list *)
-let checker_for_global_var dec checker =
-  checker dec
+let checker_for_global_var dec checker context =
+  checker context dec
 
 (* List of checkers on conditional operator *)
 let conditional_op_checker_list = [CFrontend_checkers.bad_pointer_comparison_warning]
 
 (* Invocation of checker belonging to conditional_op_checker_list *)
-let checker_for_conditional_op stmt_info first_stmt checker =
-  checker stmt_info first_stmt
+let checker_for_conditional_op stmt_info first_stmt checker context =
+  checker context stmt_info first_stmt
 
 (* List of checkers on if-statement *)
 let if_stmt_checker_list = [CFrontend_checkers.bad_pointer_comparison_warning]
 
 (* Invocation of checker belonging to if_stmt_checker_list *)
-let checker_for_if_stmt stmt_info cond checker =
-  checker stmt_info cond
+let checker_for_if_stmt stmt_info cond checker context =
+  checker context stmt_info cond
 
 (* List of checkers on for statement *)
 let for_stmt_checker_list = [CFrontend_checkers.bad_pointer_comparison_warning]
 
 (* Invocation of checker belonging to for_stmt_checker_list *)
-let checker_for_for_stmt stmt_info cond checker =
-  checker stmt_info cond
+let checker_for_for_stmt stmt_info cond checker context =
+  checker context stmt_info cond
 
 (* List of checkers on while statement *)
 let while_stmt_checker_list = [CFrontend_checkers.bad_pointer_comparison_warning]
 
 (* Invocation of checker belonging to while_stmt_checker_list *)
-let checker_for_while_stmt stmt_info cond checker =
-  checker stmt_info cond
+let checker_for_while_stmt stmt_info cond checker context =
+  checker context stmt_info cond
 
 
 
@@ -117,13 +117,13 @@ let log_frontend_issue cfg cg method_decl_opt key issue_desc =
    1. f a particular way to apply a checker, it's a partial function
    2. context
    3. the list of checkers to be applied *)
-let invoke_set_of_checkers f cfg cg method_decl_opt key checkers =
+let invoke_set_of_checkers f context cfg cg method_decl_opt key checkers  =
   IList.iter (fun checker ->
-      match f checker with
+      match f checker context with
       | Some issue_desc -> log_frontend_issue cfg cg method_decl_opt key issue_desc
       | None -> ()) checkers
 
-let run_frontend_checkers_on_stmt cfg cg method_decl instr =
+let run_frontend_checkers_on_stmt context cfg cg method_decl instr =
   let open Clang_ast_t in
   let decl_opt = Some method_decl in
   match instr with
@@ -131,32 +131,41 @@ let run_frontend_checkers_on_stmt cfg cg method_decl instr =
       let dr_ref = obj_c_ivar_ref_expr_info.Clang_ast_t.ovrei_decl_ref in
       let call_checker_for_ivar = checkers_for_ivar method_decl stmt_info dr_ref in
       let key = Ast_utils.generate_key_stmt instr in
-      invoke_set_of_checkers call_checker_for_ivar cfg cg decl_opt key ivar_access_checker_list
+      invoke_set_of_checkers
+        call_checker_for_ivar context cfg cg decl_opt key ivar_access_checker_list;
+      context
   | BlockExpr (stmt_info, _ , _, Clang_ast_t.BlockDecl (_, block_decl_info)) ->
       let captured_block_vars = block_decl_info.Clang_ast_t.bdi_captured_variables in
       let call_captured_vars_checker = checkers_for_capture_vars stmt_info captured_block_vars in
       let key = Ast_utils.generate_key_stmt instr in
-      invoke_set_of_checkers call_captured_vars_checker cfg cg decl_opt key
-        captured_vars_checker_list
+      invoke_set_of_checkers call_captured_vars_checker context cfg cg decl_opt key
+        captured_vars_checker_list;
+      context
   | IfStmt (stmt_info, _ :: cond :: _) ->
       let call_checker = checker_for_if_stmt stmt_info [cond] in
       let key = Ast_utils.generate_key_stmt cond in
-      invoke_set_of_checkers call_checker cfg cg decl_opt key if_stmt_checker_list
+      invoke_set_of_checkers call_checker context cfg cg decl_opt key if_stmt_checker_list;
+      context
   | ConditionalOperator (stmt_info, first_stmt :: _, _) ->
       let call_checker = checker_for_conditional_op stmt_info [first_stmt] in
       let key = Ast_utils.generate_key_stmt first_stmt in
-      invoke_set_of_checkers call_checker cfg cg decl_opt key conditional_op_checker_list
+      invoke_set_of_checkers call_checker context cfg cg decl_opt key conditional_op_checker_list;
+      context
   | ForStmt (stmt_info, [_; _; cond; _; _]) ->
       let call_checker = checker_for_for_stmt stmt_info [cond] in
       let key = Ast_utils.generate_key_stmt cond in
-      invoke_set_of_checkers call_checker cfg cg decl_opt key for_stmt_checker_list
+      invoke_set_of_checkers call_checker context cfg cg decl_opt key for_stmt_checker_list;
+      context
   | WhileStmt (stmt_info, [_; cond; _]) ->
       let call_checker = checker_for_while_stmt stmt_info [cond] in
       let key = Ast_utils.generate_key_stmt cond in
-      invoke_set_of_checkers call_checker cfg cg decl_opt key while_stmt_checker_list
-  | _ -> ()
+      invoke_set_of_checkers call_checker context cfg cg decl_opt key while_stmt_checker_list;
+      context
+  | ObjCAtSynchronizedStmt _ ->
+      { context with CLintersContext.in_synchronized_block = true }
+  | _ -> context
 
-let run_frontend_checkers_on_decl cfg cg dec =
+let run_frontend_checkers_on_decl context cfg cg dec =
   let open Clang_ast_t in
   let decl_info = Clang_ast_proj.get_decl_tuple dec in
   if CLocation.should_do_frontend_check decl_info.Clang_ast_t.di_source_range then
@@ -165,13 +174,17 @@ let run_frontend_checkers_on_decl cfg cg dec =
     | ObjCProtocolDecl (decl_info, _, decl_list, _, _) ->
         let call_ns_checker = checkers_for_ns decl_info decl_list in
         let key = Ast_utils.generate_key_decl dec in
-        invoke_set_of_checkers call_ns_checker cfg cg None key ns_notification_checker_list
+        invoke_set_of_checkers call_ns_checker context cfg cg None key ns_notification_checker_list;
+        context
     | VarDecl _ ->
         let call_global_checker = checker_for_global_var dec in
         let key = Ast_utils.generate_key_decl dec in
-        invoke_set_of_checkers call_global_checker cfg cg None key global_var_checker_list
+        invoke_set_of_checkers call_global_checker context cfg cg None key global_var_checker_list;
+        context
     | ObjCPropertyDecl (decl_info, pname_info, pdi) ->
         let call_property_checker = checkers_for_property decl_info pname_info pdi in
         let key = Ast_utils.generate_key_decl dec in
-        invoke_set_of_checkers call_property_checker cfg cg None key property_checkers_list
-    | _ -> ()
+        invoke_set_of_checkers call_property_checker context cfg cg None key property_checkers_list;
+        context
+    | _ -> context
+  else context

infer/src/clang/cFrontend_errors.mli

@@ -13,9 +13,12 @@ open! Utils
 (* Module for warnings detected at translation time by the frontend *)
 
 (* Run frontend checkers on a statement *)
-val run_frontend_checkers_on_stmt : Cfg.cfg -> Cg.t -> Clang_ast_t.decl -> Clang_ast_t.stmt -> unit
+val run_frontend_checkers_on_stmt :
+  CLintersContext.context ->
+  Cfg.cfg -> Cg.t -> Clang_ast_t.decl -> Clang_ast_t.stmt -> CLintersContext.context
 
 (* Run frontend checkers on a declaration *)
-val run_frontend_checkers_on_decl : Cfg.cfg -> Cg.t -> Clang_ast_t.decl -> unit
+val run_frontend_checkers_on_decl :
+  CLintersContext.context -> Cfg.cfg -> Cg.t -> Clang_ast_t.decl -> CLintersContext.context
 
 val errLogMap : Errlog.t Procname.Map.t ref

infer/tests/codetoanalyze/objc/linters/atomic_prop.m

@@ -100,4 +100,12 @@
   }
 }
 
+- (void)accessWithinSynchronizedIsOk {
+  @synchronized(self) {
+    if (self->_f > 0) {
+      self->_f += 1;
+    }
+  }
+}
+
 @end