Initial version of the @Expensive checker

Summary: public
This is an initial version of the Expensive checker which only report violations on direct calls. The main objective is to setup all the files for this new checker.

The next steps are:
1) run the checker in interprocedural mode
2) Save in the summary of a method foo() the annotation attribute Expensive if a direct callee of foo is annotated with Expensive
3) Check that Expensive is enforced by subtyping, i.e. check that non-expensive method cannot be overwritten by a method annotated with Expensive

Reviewed By: cristianoc

Differential Revision: D2629947

fb-gh-sync-id: 0e06f85

infer/annotations/com/facebook/infer/annotation/Expensive.java

@@ -0,0 +1,19 @@
+/*
+* 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 com.facebook.infer.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.CLASS)
+@Target(ElementType.METHOD)
+public @interface Expensive {}

infer/annotations/com/facebook/infer/annotation/PerformanceCritical.java

@@ -0,0 +1,19 @@
+/*
+* 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 com.facebook.infer.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.CLASS)
+@Target(ElementType.METHOD)
+public @interface PerformanceCritical {}

infer/src/backend/cfg.ml

@@ -552,16 +552,18 @@ module Node = struct
     IList.iter f (IList.rev (proc_desc_get_nodes proc_desc))
 
   let proc_desc_fold_nodes f acc proc_desc =
-    (*list_fold_left (fun acc node -> f acc node) acc (IList.rev (proc_desc_get_nodes proc_desc))*)
     IList.fold_left f acc (IList.rev (proc_desc_get_nodes proc_desc))
 
+  let proc_desc_fold_calls f acc pdesc =
+    let do_node a node =
+      IList.fold_left
+        (fun b callee_pname -> f b (callee_pname, get_loc node))
+        a (get_callees node) in
+    IList.fold_left do_node acc (proc_desc_get_nodes pdesc)
+
   (** iterate over the calls from the procedure: (callee,location) pairs *)
   let proc_desc_iter_calls f pdesc =
-    let do_node node =
-      IList.iter
-        (fun callee_pname -> f (callee_pname, get_loc node))
-        (get_callees node) in
-    IList.iter do_node (proc_desc_get_nodes pdesc)
+    proc_desc_fold_calls (fun _ call -> f call) () pdesc
 
   let proc_desc_iter_slope f proc_desc =
     let visited = ref NodeSet.empty in
@@ -659,6 +661,7 @@ module Procdesc = struct
   let get_start_node = Node.proc_desc_get_start_node
   let is_defined = Node.proc_desc_is_defined
   let iter_nodes = Node.proc_desc_iter_nodes
+  let fold_calls = Node.proc_desc_fold_calls
   let iter_calls = Node.proc_desc_iter_calls
   let iter_instrs = Node.proc_desc_iter_instrs
   let fold_instrs = Node.proc_desc_fold_instrs

infer/src/backend/cfg.mli

@@ -97,8 +97,11 @@ module Procdesc : sig
   (** iterate over all the nodes of a procedure *)
   val iter_nodes : (node -> unit) -> t -> unit
 
-  (** iterate over the calls from the procedure: (callee,location) pairs *)
-  val iter_calls : ((Procname.t * Location.t) -> unit) -> t -> unit
+  (** fold over the calls from the procedure: (callee, location) pairs *)
+  val fold_calls : ('a -> Procname.t * Location.t -> 'a) -> 'a -> t -> 'a
+
+  (** iterate over the calls from the procedure: (callee, location) pairs *)
+  val iter_calls : (Procname.t * Location.t -> unit) -> t -> unit
 
   (** iterate over all nodes and their instructions *)
   val iter_instrs : (node -> Sil.instr -> unit) -> t -> unit

infer/src/backend/config.ml

@@ -360,6 +360,10 @@ let default_failure_name = "ASSERTION_FAILURE"
 
 let analyze_models = from_env_variable "INFER_ANALYZE_MODELS"
 
+(** report expensive calls warnings *)
+let report_expensive_calls = from_env_variable "INFER_REPORT_EXPENSIVE_CALLS"
+
+
 module Experiment = struct
 
   (** if true, activate the subtyping routines in C++ as well, not just in Java *)

infer/src/checkers/annotations.ml

@@ -109,6 +109,8 @@ let present = "Present"
 let strict = "com.facebook.infer.annotation.Strict"
 let true_on_null = "TrueOnNull"
 let verify_annotation = "com.facebook.infer.annotation.Verify"
+let expensive = "Expensive"
+let performance_critical = "PerformanceCritical"
 
 let ia_is_nullable ia =
   ia_ends_with ia nullable
@@ -147,6 +149,12 @@ let ia_get_strict ia =
 let ia_is_verify ia =
   ia_contains ia verify_annotation
 
+let ia_is_expensive ia =
+  ia_ends_with ia expensive
+
+let ia_is_performance_critical ia =
+  ia_ends_with ia performance_critical
+
 type annotation =
   | Nullable
   | Present

infer/src/checkers/annotations.mli

@@ -11,6 +11,9 @@
 
 val suppressLint : string
 
+val expensive : string
+val performance_critical : string
+
 type annotation =
   | Nullable
   | Present
@@ -65,6 +68,8 @@ val ia_is_nullable : Sil.item_annotation -> bool
 val ia_is_present : Sil.item_annotation -> bool
 val ia_is_true_on_null : Sil.item_annotation -> bool
 val ia_is_verify : Sil.item_annotation -> bool
+val ia_is_expensive : Sil.item_annotation -> bool
+val ia_is_performance_critical : Sil.item_annotation -> bool
 
 val ia_iter : (Sil.annotation -> unit) -> Sil.item_annotation -> unit
 

infer/src/checkers/checkers.ml

@@ -383,28 +383,32 @@ let callback_find_deserialization all_procs get_proc_desc idenv tenv proc_name p
       | None -> "?" in
 
   let get_actual_arguments node instr = match instr with
-    | Sil.Call (ret_ids, Sil.Const (Sil.Cfun pn), (te, tt):: args, loc, cf) -> (try
-                                                                                  let find_const exp typ =
-                                                                                    let expanded = Idenv.expand_expr idenv exp in
-                                                                                    match expanded with
-                                                                                    | Sil.Const (Sil.Cclass n) -> Ident.name_to_string n
-                                                                                    | Sil.Lvar p -> (
-                                                                                        let is_call_instr set call = match set, call with
-                                                                                          | Sil.Set (_, _, Sil.Var (i1), _), Sil.Call (i2::[], _, _, _, _) when Ident.equal i1 i2 -> true
-                                                                                          | _ -> false in
-                                                                                        let is_set_instr = function
-                                                                                          | Sil.Set (e1, t, e2, l) when Sil.exp_equal expanded e1 -> true
-                                                                                          | _ -> false in
-                                                                                        match reverse_find_instr is_set_instr node with                  (** Look for ivar := tmp *)
-                                                                                        | Some s -> (
-                                                                                            match reverse_find_instr (is_call_instr s) node with           (** Look for tmp := foo() *)
-                                                                                            | Some (Sil.Call (_, Sil.Const (Sil.Cfun pn), _, l, _)) -> get_return_const pn
-                                                                                            | _ -> "?")
-                                                                                        | _ -> "?")
-                                                                                    | _ -> "?" in
-                                                                                  let arg_name (exp, typ) = find_const exp typ in
-                                                                                  Some (IList.map arg_name args)
-                                                                                with _ -> None)
+    | Sil.Call (ret_ids, Sil.Const (Sil.Cfun pn), (te, tt):: args, loc, cf) ->
+        (try
+           let find_const exp typ =
+             let expanded = Idenv.expand_expr idenv exp in
+             match expanded with
+             | Sil.Const (Sil.Cclass n) -> Ident.name_to_string n
+             | Sil.Lvar p -> (
+                 let is_call_instr set call = match set, call with
+                   | Sil.Set (_, _, Sil.Var (i1), _), Sil.Call (i2::[], _, _, _, _)
+                     when Ident.equal i1 i2 -> true
+                   | _ -> false in
+                 let is_set_instr = function
+                   | Sil.Set (e1, t, e2, l) when Sil.exp_equal expanded e1 -> true
+                   | _ -> false in
+                 match reverse_find_instr is_set_instr node with
+                 (** Look for ivar := tmp *)
+                 | Some s -> (
+                     match reverse_find_instr (is_call_instr s) node with
+                     (** Look for tmp := foo() *)
+                     | Some (Sil.Call (_, Sil.Const (Sil.Cfun pn), _, l, _)) -> get_return_const pn
+                     | _ -> "?")
+                 | _ -> "?")
+             | _ -> "?" in
+           let arg_name (exp, typ) = find_const exp typ in
+           Some (IList.map arg_name args)
+         with _ -> None)
     | _ -> None in
 
   let process_result instr result =

infer/src/checkers/performanceCritical.ml

@@ -0,0 +1,61 @@
+(*
+ * 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.
+ *)
+
+module L = Logging
+
+
+let calls_expensive_method = "CHECKERS_CALLS_EXPENSIVE_METHOD"
+
+
+let search_expensive_call checked_pnames expensive_callee (pname, _) =
+  match expensive_callee with
+  | Some callee_pname -> Some callee_pname
+  | None ->
+      if Procname.Set.mem pname !checked_pnames then None
+      else
+        begin
+          checked_pnames := Procname.Set.add pname !checked_pnames;
+          match AttributesTable.load_attributes pname with
+          | None -> None
+          | Some attributes ->
+              let annotated_signature = Annotations.get_annotated_signature attributes in
+              let ret_annotation, _ = annotated_signature.Annotations.ret in
+              if Annotations.ia_is_expensive ret_annotation then
+                Some pname
+              else
+                None
+        end
+
+
+let is_performance_critical attributes =
+  let annotated_signature = Annotations.get_annotated_signature attributes in
+  let ret_annotation, _ = annotated_signature.Annotations.ret in
+  Annotations.ia_is_performance_critical ret_annotation
+
+
+let callback_performance_checker _ _ _ tenv pname pdesc : unit =
+  let attributes = Cfg.Procdesc.get_attributes pdesc in
+  let expensive_call_found =
+    let checked_pnames = ref Procname.Set.empty in
+    Cfg.Procdesc.fold_calls
+      (search_expensive_call checked_pnames)
+      None
+      pdesc in
+  match expensive_call_found with
+  | None -> ()
+  | Some callee_pname when is_performance_critical attributes ->
+      let description =
+        Printf.sprintf "Method %s annotated with @%s calls method %s annotated with @%s"
+          (Procname.to_simplified_string pname)
+          Annotations.performance_critical
+          (Procname.to_string callee_pname)
+          Annotations.expensive in
+      Checkers.ST.report_error
+        pname pdesc calls_expensive_method (Cfg.Procdesc.get_loc pdesc) description
+  | Some _ -> ()

infer/src/checkers/performanceCritical.mli

@@ -0,0 +1,11 @@
+(*
+ * 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.
+ *)
+
+
+val callback_performance_checker : Callbacks.proc_callback_t

infer/src/checkers/registerCheckers.ml

@@ -35,6 +35,7 @@ let active_procedure_checkers () =
         ImmutableChecker.callback_check_immutable_cast, checkers_enabled;
         RepeatedCallsChecker.callback_check_repeated_calls, checkers_enabled;
         PrintfArgs.callback_printf_args, checkers_enabled;
+        PerformanceCritical.callback_performance_checker, Config.report_expensive_calls;
       ] in
     IList.map (fun (x, y) -> (x, y, Some Config.Java)) l in
   let c_cpp_checkers =

infer/tests/codetoanalyze/java/checkers/BUCK

@@ -1,6 +1,7 @@
 sources = glob(['**/*.java'])
 
 dependencies = [
+    '//infer/annotations:annotations',
     '//infer/lib/java/android:android',
 ]
 
@@ -16,6 +17,7 @@ java_library(
 out = 'out'
 clean_cmd = ' '.join(['rm', '-rf', out])
 classpath = ':'.join([('$(classpath ' + path + ')') for path in dependencies])
+env_cmd = ' '.join(['export', 'INFER_REPORT_EXPENSIVE_CALLS=1'])
 infer_cmd = ' '.join([
     'infer',
     '--absolute-paths',
@@ -27,7 +29,7 @@ infer_cmd = ' '.join([
     '$SRCS',
 ])
 copy_cmd = ' '.join(['cp', out + '/report.csv', '$OUT'])
-command = ' && '.join([clean_cmd, infer_cmd, copy_cmd])
+command = ' && '.join([clean_cmd, env_cmd, infer_cmd, copy_cmd])
 
 genrule(
     name = 'analyze',

infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java

@@ -0,0 +1,29 @@
+/*
+* Copyright (c) 2013 - 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 codetoanalyze.java.checkers;
+
+import com.facebook.infer.annotation.Expensive;
+import com.facebook.infer.annotation.PerformanceCritical;
+
+public class ExpensiveCallExample {
+
+  Object mObject;
+
+  @Expensive
+  void expensiveMethod() {
+    mObject = new Object();
+  }
+
+  @PerformanceCritical
+  void shouldReportExpensiveCallWarning() {
+    expensiveMethod();
+  }
+
+}

infer/tests/endtoend/java/checkers/ExpensiveCallTest.java

@@ -0,0 +1,53 @@
+/*
+* 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.java.checkers;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static utils.matchers.ResultContainsExactly.containsExactly;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import utils.InferException;
+import utils.InferResults;
+
+public class ExpensiveCallTest {
+
+  public static final String SOURCE_FILE =
+      "infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java";
+
+  public static final String CALLS_EXPENSIVE_METHOD = "CHECKERS_CALLS_EXPENSIVE_METHOD";
+
+  private static InferResults inferResults;
+
+  @BeforeClass
+  public static void loadResults() throws InterruptedException, IOException {
+    inferResults =
+      InferResults.loadCheckersResults(ImmutableCastTest.class, SOURCE_FILE);
+  }
+
+  @Test
+  public void matchErrors()
+      throws IOException, InterruptedException, InferException {
+    String[] methods = {
+        "shouldReportExpensiveCallWarning",
+    };
+    assertThat(
+        "Results should contain " + CALLS_EXPENSIVE_METHOD,
+        inferResults,
+        containsExactly(
+            CALLS_EXPENSIVE_METHOD,
+            SOURCE_FILE,
+            methods));
+  }
+
+}