From f5ddb983fe2881ef57cec54f99f36bd399e5eea4 Mon Sep 17 00:00:00 2001 From: jrm Date: Mon, 9 Nov 2015 11:55:52 -0800 Subject: [PATCH] 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 --- .../facebook/infer/annotation/Expensive.java | 19 ++++++ .../infer/annotation/PerformanceCritical.java | 19 ++++++ infer/src/backend/cfg.ml | 15 +++-- infer/src/backend/cfg.mli | 7 ++- infer/src/backend/config.ml | 4 ++ infer/src/checkers/annotations.ml | 8 +++ infer/src/checkers/annotations.mli | 5 ++ infer/src/checkers/checkers.ml | 48 ++++++++------- infer/src/checkers/performanceCritical.ml | 61 +++++++++++++++++++ infer/src/checkers/performanceCritical.mli | 11 ++++ infer/src/checkers/registerCheckers.ml | 1 + infer/tests/codetoanalyze/java/checkers/BUCK | 4 +- .../java/checkers/ExpensiveCallExample.java | 29 +++++++++ .../java/checkers/ExpensiveCallTest.java | 53 ++++++++++++++++ 14 files changed, 253 insertions(+), 31 deletions(-) create mode 100644 infer/annotations/com/facebook/infer/annotation/Expensive.java create mode 100644 infer/annotations/com/facebook/infer/annotation/PerformanceCritical.java create mode 100644 infer/src/checkers/performanceCritical.ml create mode 100644 infer/src/checkers/performanceCritical.mli create mode 100644 infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java create mode 100644 infer/tests/endtoend/java/checkers/ExpensiveCallTest.java diff --git a/infer/annotations/com/facebook/infer/annotation/Expensive.java b/infer/annotations/com/facebook/infer/annotation/Expensive.java new file mode 100644 index 000000000..6c56ded37 --- /dev/null +++ b/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 {} diff --git a/infer/annotations/com/facebook/infer/annotation/PerformanceCritical.java b/infer/annotations/com/facebook/infer/annotation/PerformanceCritical.java new file mode 100644 index 000000000..6206fd1e1 --- /dev/null +++ b/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 {} diff --git a/infer/src/backend/cfg.ml b/infer/src/backend/cfg.ml index b4278e8c9..44a46a268 100644 --- a/infer/src/backend/cfg.ml +++ b/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 diff --git a/infer/src/backend/cfg.mli b/infer/src/backend/cfg.mli index c7cb05621..f8d9840bb 100644 --- a/infer/src/backend/cfg.mli +++ b/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 diff --git a/infer/src/backend/config.ml b/infer/src/backend/config.ml index 228142c61..16cdc415e 100644 --- a/infer/src/backend/config.ml +++ b/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 *) diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index 440caf803..b5ab06449 100644 --- a/infer/src/checkers/annotations.ml +++ b/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 diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index aee1b94e5..d450fe7fc 100644 --- a/infer/src/checkers/annotations.mli +++ b/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 diff --git a/infer/src/checkers/checkers.ml b/infer/src/checkers/checkers.ml index 9be249ad4..011d60005 100644 --- a/infer/src/checkers/checkers.ml +++ b/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 = diff --git a/infer/src/checkers/performanceCritical.ml b/infer/src/checkers/performanceCritical.ml new file mode 100644 index 000000000..03e78a1a3 --- /dev/null +++ b/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 _ -> () diff --git a/infer/src/checkers/performanceCritical.mli b/infer/src/checkers/performanceCritical.mli new file mode 100644 index 000000000..9e47924b1 --- /dev/null +++ b/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 diff --git a/infer/src/checkers/registerCheckers.ml b/infer/src/checkers/registerCheckers.ml index 226582360..6eba5d26c 100644 --- a/infer/src/checkers/registerCheckers.ml +++ b/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 = diff --git a/infer/tests/codetoanalyze/java/checkers/BUCK b/infer/tests/codetoanalyze/java/checkers/BUCK index b245db692..5dc49c1d3 100644 --- a/infer/tests/codetoanalyze/java/checkers/BUCK +++ b/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', diff --git a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java new file mode 100644 index 000000000..8ddc03407 --- /dev/null +++ b/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(); + } + +} diff --git a/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java b/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java new file mode 100644 index 000000000..da2ce1e38 --- /dev/null +++ b/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)); + } + +}