From 83c1bbc83287531a95e972a70a31b11e6c44f657 Mon Sep 17 00:00:00 2001 From: Dino Distefano Date: Mon, 25 Apr 2016 07:16:40 -0700 Subject: [PATCH] Implementing a checker to warn initialization of global variables with mehod calls. Reviewed By: dulmarod Differential Revision: D3184584 fb-gh-sync-id: dbfc99c fbshipit-source-id: dbfc99c --- .inferconfig | 3 + infer/lib/python/inferlib/issues.py | 1 + infer/src/backend/inferconfig.ml | 4 +- infer/src/clang/cFrontend_checkers.ml | 41 +++++++++++++ infer/src/clang/cFrontend_checkers.mli | 5 ++ infer/src/clang/cFrontend_errors.ml | 10 ++++ infer/src/clang/cFrontend_utils.ml | 15 +++++ infer/src/clang/cFrontend_utils.mli | 9 +++ .../objcpp/frontend/global-var/B.mm | 45 ++++++++++++++ infer/tests/endtoend/objc/GlobalVarTest.java | 58 +++++++++++++++++++ infer/tests/utils/InferResults.java | 1 + 11 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 infer/tests/codetoanalyze/objcpp/frontend/global-var/B.mm create mode 100644 infer/tests/endtoend/objc/GlobalVarTest.java diff --git a/.inferconfig b/.inferconfig index 72a3f0d9f..aa8eb321f 100644 --- a/.inferconfig +++ b/.inferconfig @@ -1,4 +1,7 @@ { + "enable_checks": [ + "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL" + ], "skip_translation": [ { "language": "Java", diff --git a/infer/lib/python/inferlib/issues.py b/infer/lib/python/inferlib/issues.py index 82856e2f3..ff1641741 100644 --- a/infer/lib/python/inferlib/issues.py +++ b/infer/lib/python/inferlib/issues.py @@ -57,6 +57,7 @@ ISSUE_TYPES = [ 'CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK', 'REGISTERED_OBSERVER_BEING_DEALLOCATED', 'ASSIGN_POINTER_WARNING', + 'GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL', ] NULL_STYLE_ISSUE_TYPES = [ diff --git a/infer/src/backend/inferconfig.ml b/infer/src/backend/inferconfig.ml index 33b403ed9..2f10952de 100644 --- a/infer/src/backend/inferconfig.ml +++ b/infer/src/backend/inferconfig.ml @@ -333,7 +333,9 @@ module ModeledExpensiveMatcher = OverridesMatcher(struct let json_key = "modeled_expensive" end) -let disabled_checks_by_default = [] +let disabled_checks_by_default = [ + "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL" +] let inferconfig () = match !Config.inferconfig_home with diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml index 590e31ba7..69ae40aaf 100644 --- a/infer/src/clang/cFrontend_checkers.ml +++ b/infer/src/clang/cFrontend_checkers.ml @@ -59,6 +59,14 @@ let rec is_self s = | _ -> false) | _ -> false +let is_function_or_method_call _ st = + match st with + | Clang_ast_t.CallExpr _ (* for C *) + | CXXMemberCallExpr _ | CXXOperatorCallExpr _ + | CXXConstructExpr _ | CXXTemporaryObjectExpr _ (* for C++ *) + | Clang_ast_t.ObjCMessageExpr _ -> true (* for ObjC *) + | _ -> false + (* Call method m and on the pn parameter pred holds *) (* st |= call_method(m(p1,...,pn,...pk)) /\ pred(pn) *) let call_method_on_nth pred pn m st = @@ -78,6 +86,19 @@ let dec_body_eventually atomic_pred param dec = | _ -> false) | _ -> false +(* true if a variable is initialized with a method/function call.*) +(* implemented as decl |= EF is_function_or_method_call *) +let is_initialized_with_call decl = + match decl with + | Clang_ast_t.VarDecl (_, _ ,_, vdi) -> + (match vdi.vdi_init_expr with + | Some init_expr -> + CFrontend_utils.Ast_utils.exists_eventually_st is_function_or_method_call () init_expr + | _ -> false) + | _ -> false + + + (* === Warnings on properties === *) (* Assing Pointer Warning: a property with a pointer type should not be declared `assign` *) @@ -118,6 +139,26 @@ let strong_delegate_warning decl_info pname obj_c_property_decl_info = loc = location_from_dinfo decl_info; } else None +(* 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 decl_info, gvar = + match Clang_ast_proj.get_named_decl_tuple decl with + | Some (di, ndi) -> di, ndi.ni_name + | None -> assert false (* we cannot be here *) in + let condition = (CFrontend_utils.Ast_utils.is_objc () || CFrontend_utils.Ast_utils.is_objcpp ()) + && CFrontend_utils.Ast_utils.is_global_var decl + && is_initialized_with_call decl in + if condition then + Some { + name = "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL"; + description = "Global variable " ^ gvar ^ + " is initialized using a function or method call"; + suggestion = "If the function/method call is expensive, it can affect the starting time of the app."; + loc = location_from_dinfo decl_info; } + else None + (* 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 = diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index 7ae8b333f..6df6648fd 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/infer/src/clang/cFrontend_checkers.mli @@ -40,3 +40,8 @@ val captured_cxx_ref_in_objc_block_warning : Clang_ast_t.stmt_info -> (Pvar.t * (* 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 -> warning_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 -> warning_desc option diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 8eda22123..96cfd013c 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -40,6 +40,13 @@ let ns_notification_checker_list = [CFrontend_checkers.checker_NSNotificationCen let checkers_for_ns decl_info decls checker = checker 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 + (* 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 open CFrontend_checkers in @@ -133,4 +140,7 @@ let rec run_frontend_checkers_on_decl cfg cg dec = invoke_set_of_checkers call_ns_checker cfg cg None ns_notification_checker_list; IList.iter (run_frontend_checkers_on_decl cfg cg) decl_list) else () + | VarDecl _ -> + let call_global_checker = checker_for_global_var dec in + invoke_set_of_checkers call_global_checker cfg cg None global_var_checker_list | _ -> () diff --git a/infer/src/clang/cFrontend_utils.ml b/infer/src/clang/cFrontend_utils.ml index 439db43f5..6836ada7d 100644 --- a/infer/src/clang/cFrontend_utils.ml +++ b/infer/src/clang/cFrontend_utils.ml @@ -399,6 +399,21 @@ struct let _, st_list = Clang_ast_proj.get_stmt_tuple st in IList.exists (exists_eventually_st atomic_pred param) st_list + let is_global_var decl = + match decl with + | Clang_ast_t.VarDecl (_, _ ,_, vdi) -> vdi.vdi_is_global + | _ -> false + + let is_objc () = + match !CFrontend_config.language with + | CFrontend_config.OBJC -> true + | _ -> false + + let is_objcpp () = + match !CFrontend_config.language with + | CFrontend_config.OBJCPP -> true + | _ -> false + (* let rec getter_attribute_opt attributes = match attributes with diff --git a/infer/src/clang/cFrontend_utils.mli b/infer/src/clang/cFrontend_utils.mli index 09408c8df..0b9723303 100644 --- a/infer/src/clang/cFrontend_utils.mli +++ b/infer/src/clang/cFrontend_utils.mli @@ -135,6 +135,15 @@ sig val exists_eventually_st : ('a -> Clang_ast_t.stmt -> bool) -> 'a -> Clang_ast_t.stmt -> bool + (* true if a declaration is a global variable *) + val is_global_var : Clang_ast_t.decl -> bool + + (* true if CFrontend_config.language is set ot ObjC *) + val is_objc : unit -> bool + + (* true if CFrontend_config.language is set ot ObjC *) + val is_objcpp : unit -> bool + end module General_utils : diff --git a/infer/tests/codetoanalyze/objcpp/frontend/global-var/B.mm b/infer/tests/codetoanalyze/objcpp/frontend/global-var/B.mm new file mode 100644 index 000000000..5c7825693 --- /dev/null +++ b/infer/tests/codetoanalyze/objcpp/frontend/global-var/B.mm @@ -0,0 +1,45 @@ +/* + * 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. + */ + +#import + +@interface A : NSObject ++ (int)bar; ++ (int)scale; +@end +@implementation A + ++ (int)bar { + return 17; +} + ++ (int)scale { + return 19; +} +@end + +int foo() { return 23; } + +static const int kInsets = foo(); // Error + +static float kPadding = [A bar] ? 10.0 : 11.0; // Error + +static const float kLineSize = 1 / [A scale]; // Error + +static const float ok = 37; + +void bla() { + static const int kInsets = foo(); // Error + + static float kPadding = [A bar] ? 10.0 : 11.0; // Error + + static const float kLineSize = 1 / [A scale]; // Error + + static const float ok = 37; +} diff --git a/infer/tests/endtoend/objc/GlobalVarTest.java b/infer/tests/endtoend/objc/GlobalVarTest.java new file mode 100644 index 000000000..9e8804d37 --- /dev/null +++ b/infer/tests/endtoend/objc/GlobalVarTest.java @@ -0,0 +1,58 @@ +/* + * Copyright (c) 2015 - 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. + */ + +package endtoend.objc; + +import static org.hamcrest.MatcherAssert.assertThat; +import static utils.matchers.ResultContainsLineNumbers.containsLines; + +import com.google.common.collect.ImmutableList; + +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; + +import java.io.IOException; + +import utils.DebuggableTemporaryFolder; +import utils.InferException; +import utils.InferResults; +import utils.InferRunner; + +public class GlobalVarTest { + + public static final String FILE = + "infer/tests/codetoanalyze/objcpp/frontend/global-var/B.mm"; + + private static ImmutableList inferCmdFraction; + + public static final String GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL = "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL"; + + @ClassRule + public static DebuggableTemporaryFolder folder = + new DebuggableTemporaryFolder(); + + @BeforeClass + public static void runInfer() throws InterruptedException, IOException { + inferCmdFraction = InferRunner.createObjCPPInferCommand( + folder, + FILE); + } + + @Test + public void whenInferRunsGlobalVar() + throws InterruptedException, IOException, InferException { + InferResults inferResults = InferRunner.runInferObjC(inferCmdFraction); + assertThat( + "Results should contain " + GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL, + inferResults, + containsLines(new int[]{29, 31, 33})); + } + +} diff --git a/infer/tests/utils/InferResults.java b/infer/tests/utils/InferResults.java index b48a58425..d011ee5c4 100644 --- a/infer/tests/utils/InferResults.java +++ b/infer/tests/utils/InferResults.java @@ -63,6 +63,7 @@ public class InferResults { errorType.equals("DIRECT_ATOMIC_PROPERTY_ACCESS") || errorType.equals("CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK") || errorType.equals("REGISTERED_OBSERVER_BEING_DEALLOCATED") || + errorType.equals("GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL") || errorType.equals("IMMUTABLE_CAST") || errorType.equals("PARAMETER_NOT_NULL_CHECKED") || errorType.equals("DANGLING_POINTER_DEREFERENCE") ||