diff --git a/infer/src/clang/CLintersContext.ml b/infer/src/clang/CLintersContext.ml new file mode 100644 index 000000000..a8dea40f0 --- /dev/null +++ b/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 } diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml index f659ef7bf..32f17b6a0 100644 --- a/infer/src/clang/cFrontend_checkers.ml +++ b/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 = diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index 9e5807a83..e596dc9dd 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/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 diff --git a/infer/src/clang/cFrontend_checkers_main.ml b/infer/src/clang/cFrontend_checkers_main.ml index ea5f0fb5f..933d02b6b 100644 --- a/infer/src/clang/cFrontend_checkers_main.ml +++ b/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 *) diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index ba60b0ca8..c9e967db8 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/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 diff --git a/infer/src/clang/cFrontend_errors.mli b/infer/src/clang/cFrontend_errors.mli index d3ed8970c..dadc7988a 100644 --- a/infer/src/clang/cFrontend_errors.mli +++ b/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 diff --git a/infer/tests/codetoanalyze/objc/linters/atomic_prop.m b/infer/tests/codetoanalyze/objc/linters/atomic_prop.m index d602f03d7..f633e1a6d 100644 --- a/infer/tests/codetoanalyze/objc/linters/atomic_prop.m +++ b/infer/tests/codetoanalyze/objc/linters/atomic_prop.m @@ -100,4 +100,12 @@ } } +- (void)accessWithinSynchronizedIsOk { + @synchronized(self) { + if (self->_f > 0) { + self->_f += 1; + } + } +} + @end