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

@@ -1,4 +1,7 @@
 {
+ "enable_checks": [                                                                   
+      "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL"                         
+    ],  
   "skip_translation": [
     {
       "language": "Java",

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 = [

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

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 =

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

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
   | _ -> ()

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

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 :

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 <Foundation/NSObject.h>
+
+@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;
+}

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<String> 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}));
+  }
+
+}

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") ||