From 0cd533f89218a65b658e834490fd55a68f22b688 Mon Sep 17 00:00:00 2001 From: jrm Date: Tue, 10 Nov 2015 15:55:33 -0800 Subject: [PATCH] Enforcing subtyping rules for @Expensive and @PerformanceCritical Summary: public This adds the following subtyping rules: - methods that are not annotated with Expensive cannot be overwritten by a method annotated with Expensive - methods annotated with PerformanceCritical must be overwitten by method annotated with PerformanceCritical Reviewed By: cristianoc Differential Revision: D2636076 fb-gh-sync-id: eb616c9 --- infer/src/checkers/performanceCritical.ml | 62 +++++++++++++++++-- .../java/checkers/ExpensiveCallExample.java | 5 ++ .../checkers/ExpensiveInterfaceExample.java | 38 ++++++++++++ .../checkers/ExpensiveSubtypingExample.java | 21 +++++++ .../PerformanceCriticalSubtypingExample.java | 18 ++++++ .../java/checkers/ExpensiveCallTest.java | 8 ++- .../java/checkers/ExpensiveSubtypingTest.java | 54 ++++++++++++++++ .../PerformanceCriticalSubtypingTest.java | 54 ++++++++++++++++ 8 files changed, 252 insertions(+), 8 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/checkers/ExpensiveInterfaceExample.java create mode 100644 infer/tests/codetoanalyze/java/checkers/ExpensiveSubtypingExample.java create mode 100644 infer/tests/codetoanalyze/java/checkers/PerformanceCriticalSubtypingExample.java create mode 100644 infer/tests/endtoend/java/checkers/ExpensiveSubtypingTest.java create mode 100644 infer/tests/endtoend/java/checkers/PerformanceCriticalSubtypingTest.java diff --git a/infer/src/checkers/performanceCritical.ml b/infer/src/checkers/performanceCritical.ml index a814a5411..08759612c 100644 --- a/infer/src/checkers/performanceCritical.ml +++ b/infer/src/checkers/performanceCritical.ml @@ -10,8 +10,14 @@ module L = Logging -let calls_expensive_method = "CHECKERS_CALLS_EXPENSIVE_METHOD" +let calls_expensive_method = + "CHECKERS_CALLS_EXPENSIVE_METHOD" +let expensive_overrides_unexpensive = + "CHECKERS_EXPENSIVE_OVERRIDES_UNANNOTATED" + +let unannotated_overrides_performance_critical = + "CHECKERS_UNANNOTATED_OVERRIDES_PERFOMANCE_CRITICAL" let search_expensive_call checked_pnames expensive_callee (pname, _) = match expensive_callee with @@ -47,17 +53,63 @@ let is_expensive attributes = check_attributes Annotations.ia_is_expensive attributes -let callback_performance_checker _ _ _ tenv pname pdesc : unit = +let check_method check pname = + match AttributesTable.load_attributes pname with + | None -> false + | Some attributes -> check_attributes check attributes + + +let method_is_performance_critical pname = + check_method Annotations.ia_is_performance_critical pname + + +let method_is_expensive pname = + check_method Annotations.ia_is_expensive pname + + +let callback_performance_checker _ _ _ tenv pname pdesc = + + let loc = Cfg.Procdesc.get_loc pdesc in let attributes = Cfg.Procdesc.get_attributes pdesc in + let expensive = is_expensive attributes + and performance_critical = is_performance_critical attributes 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 + + let check_expensive_subtyping_rules overridden_pname = + if not (method_is_expensive overridden_pname) then + let description = + Printf.sprintf + "Method %s overrides unannotated method %s and cannot be annotated with @%s" + (Procname.to_string pname) + (Procname.to_string overridden_pname) + Annotations.expensive in + Checkers.ST.report_error + pname pdesc expensive_overrides_unexpensive loc description + + and check_performance_critical_subtyping_rules overridden_pname = + if method_is_performance_critical overridden_pname then + let description = + Printf.sprintf + "Method %s overrides method %s annotated with %s and should also be annotated" + (Procname.to_string pname) + (Procname.to_string overridden_pname) + Annotations.performance_critical in + Checkers.ST.report_error + pname pdesc unannotated_overrides_performance_critical loc description in + + if expensive then + PatternMatch.proc_iter_overridden_methods check_expensive_subtyping_rules tenv pname; + if not performance_critical then + PatternMatch.proc_iter_overridden_methods check_performance_critical_subtyping_rules tenv pname; + match expensive_call_found with | None -> () - | Some callee_pname when is_performance_critical attributes -> + | Some callee_pname when performance_critical -> let description = Printf.sprintf "Method %s annotated with @%s calls method %s annotated with @%s" (Procname.to_simplified_string pname) @@ -65,8 +117,8 @@ let callback_performance_checker _ _ _ tenv pname pdesc : unit = (Procname.to_string callee_pname) Annotations.expensive in Checkers.ST.report_error - pname pdesc calls_expensive_method (Cfg.Procdesc.get_loc pdesc) description - | Some _ when not (is_expensive attributes) -> + pname pdesc calls_expensive_method loc description + | Some _ when not expensive -> let ret_annot, param_annot = attributes.ProcAttributes.method_annotation in let updated_method_annot = (Annotations.expensive_annotation, true) :: ret_annot, param_annot in diff --git a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java index 1271d2d63..1713a5249 100644 --- a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java +++ b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java @@ -42,4 +42,9 @@ public class ExpensiveCallExample { methodWrapper(); } + @PerformanceCritical + void callingExpensiveMethodFromInterface(ExpensiveInterfaceExample object) { + object.m5(); + } + } diff --git a/infer/tests/codetoanalyze/java/checkers/ExpensiveInterfaceExample.java b/infer/tests/codetoanalyze/java/checkers/ExpensiveInterfaceExample.java new file mode 100644 index 000000000..00b1e1eb2 --- /dev/null +++ b/infer/tests/codetoanalyze/java/checkers/ExpensiveInterfaceExample.java @@ -0,0 +1,38 @@ +/* +* 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 interface ExpensiveInterfaceExample { + + interface I { + + @PerformanceCritical + public void m1(); + + public void m2(); + + } + + class C { + + public void m3() {} + + public void m4() {} + + } + + @Expensive + public void m5(); + +} diff --git a/infer/tests/codetoanalyze/java/checkers/ExpensiveSubtypingExample.java b/infer/tests/codetoanalyze/java/checkers/ExpensiveSubtypingExample.java new file mode 100644 index 000000000..caae9950d --- /dev/null +++ b/infer/tests/codetoanalyze/java/checkers/ExpensiveSubtypingExample.java @@ -0,0 +1,21 @@ +/* +* 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; + +public class ExpensiveSubtypingExample extends ExpensiveInterfaceExample.C { + + @Expensive + public void m3() {} + + public void m4() {} + +} diff --git a/infer/tests/codetoanalyze/java/checkers/PerformanceCriticalSubtypingExample.java b/infer/tests/codetoanalyze/java/checkers/PerformanceCriticalSubtypingExample.java new file mode 100644 index 000000000..71bdd45a4 --- /dev/null +++ b/infer/tests/codetoanalyze/java/checkers/PerformanceCriticalSubtypingExample.java @@ -0,0 +1,18 @@ +/* +* 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; + +public class PerformanceCriticalSubtypingExample implements ExpensiveInterfaceExample.I { + + public void m1() {} + + public void m2() {} + +} diff --git a/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java b/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java index be7c28ef5..3b5a6b93d 100644 --- a/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java +++ b/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java @@ -23,9 +23,10 @@ import utils.InferResults; public class ExpensiveCallTest { public static final String SOURCE_FILE = - "infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java"; + "infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java"; - public static final String CALLS_EXPENSIVE_METHOD = "CHECKERS_CALLS_EXPENSIVE_METHOD"; + public static final String CALLS_EXPENSIVE_METHOD = + "CHECKERS_CALLS_EXPENSIVE_METHOD"; private static InferResults inferResults; @@ -40,7 +41,8 @@ public class ExpensiveCallTest { throws IOException, InterruptedException, InferException { String[] methods = { "directlyCallingExpensiveMethod", - "indirectlyCallingExpensiveMethod" + "indirectlyCallingExpensiveMethod", + "callingExpensiveMethodFromInterface", }; assertThat( "Results should contain " + CALLS_EXPENSIVE_METHOD, diff --git a/infer/tests/endtoend/java/checkers/ExpensiveSubtypingTest.java b/infer/tests/endtoend/java/checkers/ExpensiveSubtypingTest.java new file mode 100644 index 000000000..c2d8c2df1 --- /dev/null +++ b/infer/tests/endtoend/java/checkers/ExpensiveSubtypingTest.java @@ -0,0 +1,54 @@ +/* +* 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 ExpensiveSubtypingTest { + + public static final String SOURCE_FILE = + "infer/tests/codetoanalyze/java/checkers/ExpensiveSubtypingExample.java"; + + public static final String EXPENSIVE_OVERRIDES_UNANNOTATED = + "CHECKERS_EXPENSIVE_OVERRIDES_UNANNOTATED"; + + private static InferResults inferResults; + + @BeforeClass + public static void loadResults() throws InterruptedException, IOException { + inferResults = + InferResults.loadCheckersResults(ExpensiveSubtypingTest.class, SOURCE_FILE); + } + + @Test + public void matchErrors() + throws IOException, InterruptedException, InferException { + String[] methods = { + "m3", + }; + assertThat( + "Results should contain " + EXPENSIVE_OVERRIDES_UNANNOTATED, + inferResults, + containsExactly( + EXPENSIVE_OVERRIDES_UNANNOTATED, + SOURCE_FILE, + methods)); + } + +} diff --git a/infer/tests/endtoend/java/checkers/PerformanceCriticalSubtypingTest.java b/infer/tests/endtoend/java/checkers/PerformanceCriticalSubtypingTest.java new file mode 100644 index 000000000..c6faadcc4 --- /dev/null +++ b/infer/tests/endtoend/java/checkers/PerformanceCriticalSubtypingTest.java @@ -0,0 +1,54 @@ +/* +* 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 PerformanceCriticalSubtypingTest { + + public static final String SOURCE_FILE = + "infer/tests/codetoanalyze/java/checkers/PerformanceCriticalSubtypingExample.java"; + + public static final String UNANNOTATED_OVERRIDES_PERFOMANCE_CRITICAL = + "CHECKERS_UNANNOTATED_OVERRIDES_PERFOMANCE_CRITICAL"; + + private static InferResults inferResults; + + @BeforeClass + public static void loadResults() throws InterruptedException, IOException { + inferResults = + InferResults.loadCheckersResults(PerformanceCriticalSubtypingTest.class, SOURCE_FILE); + } + + @Test + public void matchErrors() + throws IOException, InterruptedException, InferException { + String[] methods = { + "m1", + }; + assertThat( + "Results should contain " + UNANNOTATED_OVERRIDES_PERFOMANCE_CRITICAL, + inferResults, + containsExactly( + UNANNOTATED_OVERRIDES_PERFOMANCE_CRITICAL, + SOURCE_FILE, + methods)); + } + +}