From 6b6b4d194957ffce428c29c7f117c31779a9eb05 Mon Sep 17 00:00:00 2001
From: jrm <jrm@fb.com>
Date: Tue, 10 Nov 2015 15:35:00 -0800
Subject: [PATCH] Detecting if methods annotated with @PerformanceCritical
 transitively call methods annotated with @Expensive

Summary: public
Just works by running the analysis bottom-up and promoting any method as virtually annotated with `Expensive` whenever one of its callee is annotated with `Expensive`

Reviewed By: cristianoc

Differential Revision: D2635242

fb-gh-sync-id: 4401be6
---
 infer/src/checkers/annotations.ml             |  6 +++++-
 infer/src/checkers/annotations.mli            |  2 ++
 infer/src/checkers/performanceCritical.ml     | 21 ++++++++++++++++---
 .../java/checkers/ExpensiveCallExample.java   | 18 +++++++++++++++-
 .../java/checkers/ExpensiveCallTest.java      |  3 ++-
 5 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml
index b5ab06449..6f7abef9e 100644
--- a/infer/src/checkers/annotations.ml
+++ b/infer/src/checkers/annotations.ml
@@ -112,6 +112,11 @@ let verify_annotation = "com.facebook.infer.annotation.Verify"
 let expensive = "Expensive"
 let performance_critical = "PerformanceCritical"
 
+let expensive_annotation = {
+  Sil.class_name = expensive;
+  Sil.parameters = []
+}
+
 let ia_is_nullable ia =
   ia_ends_with ia nullable
 
@@ -163,7 +168,6 @@ let ia_is ann ia = match ann with
   | Nullable -> ia_is_nullable ia
   | Present -> ia_is_present ia
 
-
 (** Get a method signature with annotations from a proc_attributes. *)
 let get_annotated_signature proc_attributes : annotated_signature =
   let method_annotation = proc_attributes.ProcAttributes.method_annotation in
diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli
index d450fe7fc..232bbd9a4 100644
--- a/infer/src/checkers/annotations.mli
+++ b/infer/src/checkers/annotations.mli
@@ -14,6 +14,8 @@ val suppressLint : string
 val expensive : string
 val performance_critical : string
 
+val expensive_annotation : Sil.annotation
+
 type annotation =
   | Nullable
   | Present
diff --git a/infer/src/checkers/performanceCritical.ml b/infer/src/checkers/performanceCritical.ml
index 03e78a1a3..a814a5411 100644
--- a/infer/src/checkers/performanceCritical.ml
+++ b/infer/src/checkers/performanceCritical.ml
@@ -33,10 +33,18 @@ let search_expensive_call checked_pnames expensive_callee (pname, _) =
         end
 
 
-let is_performance_critical attributes =
+let check_attributes check 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
+  check ret_annotation
+
+
+let is_performance_critical attributes =
+  check_attributes Annotations.ia_is_performance_critical attributes
+
+
+let is_expensive attributes =
+  check_attributes Annotations.ia_is_expensive attributes
 
 
 let callback_performance_checker _ _ _ tenv pname pdesc : unit =
@@ -58,4 +66,11 @@ let callback_performance_checker _ _ _ tenv pname pdesc : unit =
           Annotations.expensive in
       Checkers.ST.report_error
         pname pdesc calls_expensive_method (Cfg.Procdesc.get_loc pdesc) description
-  | Some _ -> ()
+  | Some _ when not (is_expensive attributes) ->
+      let ret_annot, param_annot = attributes.ProcAttributes.method_annotation in
+      let updated_method_annot =
+        (Annotations.expensive_annotation, true) :: ret_annot, param_annot in
+      let updated_attributes =
+        { attributes with ProcAttributes.method_annotation = updated_method_annot } in
+      AttributesTable.store_attributes updated_attributes
+  | Some _ -> () (* Nothing to do if method already annotated with @Expensive *)
diff --git a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java
index 8ddc03407..1271d2d63 100644
--- a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java
+++ b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java
@@ -16,14 +16,30 @@ public class ExpensiveCallExample {
 
   Object mObject;
 
+  void nonExpensiveMethod() {}
+
   @Expensive
   void expensiveMethod() {
     mObject = new Object();
   }
 
+  void methodWrapper() {
+    expensiveMethod();
+  }
+
+  @PerformanceCritical
+  void notCallingExpensiveMethod() {
+    nonExpensiveMethod();
+  }
+
   @PerformanceCritical
-  void shouldReportExpensiveCallWarning() {
+  void directlyCallingExpensiveMethod() {
     expensiveMethod();
   }
 
+  @PerformanceCritical
+  void indirectlyCallingExpensiveMethod() {
+    methodWrapper();
+  }
+
 }
diff --git a/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java b/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java
index da2ce1e38..be7c28ef5 100644
--- a/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java
+++ b/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java
@@ -39,7 +39,8 @@ public class ExpensiveCallTest {
   public void matchErrors()
       throws IOException, InterruptedException, InferException {
     String[] methods = {
-        "shouldReportExpensiveCallWarning",
+        "directlyCallingExpensiveMethod",
+        "indirectlyCallingExpensiveMethod"
     };
     assertThat(
         "Results should contain " + CALLS_EXPENSIVE_METHOD,