diff --git a/infer/annotations/com/facebook/infer/annotation/Expensive.java b/infer/annotations/com/facebook/infer/annotation/Expensive.java index 6d9994f74..72241449c 100644 --- a/infer/annotations/com/facebook/infer/annotation/Expensive.java +++ b/infer/annotations/com/facebook/infer/annotation/Expensive.java @@ -15,5 +15,5 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; @Retention(RetentionPolicy.CLASS) -@Target(ElementType.METHOD) +@Target({ ElementType.METHOD, ElementType.TYPE }) public @interface Expensive {} diff --git a/infer/annotations/com/facebook/infer/annotation/PerformanceCritical.java b/infer/annotations/com/facebook/infer/annotation/PerformanceCritical.java index 659f463f9..fb7aaba07 100644 --- a/infer/annotations/com/facebook/infer/annotation/PerformanceCritical.java +++ b/infer/annotations/com/facebook/infer/annotation/PerformanceCritical.java @@ -15,5 +15,5 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; @Retention(RetentionPolicy.CLASS) -@Target(ElementType.METHOD) +@Target(value={ElementType.METHOD, ElementType.TYPE}) public @interface PerformanceCritical {} diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index f0d4bcdac..4316bf185 100644 --- a/infer/src/checkers/annotations.ml +++ b/infer/src/checkers/annotations.ml @@ -44,6 +44,19 @@ let get_field_type_and_annotation fn = function with Not_found -> None) | _ -> None +(** Return the annotations on the declaring class of [pname]. Only works for Java *) +let get_declaring_class_annotations pname tenv = + match pname with + | Procname.Java pname_java -> + let receiver_typ_str = Procname.java_get_class_name pname_java in + begin + match Tenv.lookup_java_typ_from_string tenv receiver_typ_str with + | Sil.Tstruct { struct_annotations; } -> Some struct_annotations + | exception Tenv.Cannot_convert_string_to_typ _ -> None + | _ -> None + end + | _ -> None + let ia_iter f = let ann_iter (a, _) = f a in IList.iter ann_iter diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index 291f52897..d87602f01 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -54,6 +54,9 @@ val get_annotated_signature : ProcAttributes.t -> annotated_signature val get_field_type_and_annotation : Ident.fieldname -> Sil.typ -> (Sil.typ * Sil.item_annotation) option +(** Return the annotations on the declaring class of [pname]. Only works for Java *) +val get_declaring_class_annotations : Procname.t -> Tenv.t -> Sil.item_annotation option + val nullable : string val ia_contains : Sil.item_annotation -> string -> bool diff --git a/infer/src/checkers/performanceCritical.ml b/infer/src/checkers/performanceCritical.ml index 7526cf47b..9ce222f8c 100644 --- a/infer/src/checkers/performanceCritical.ml +++ b/infer/src/checkers/performanceCritical.ml @@ -39,39 +39,41 @@ let is_modeled_expensive = false -let check_attributes check attributes = - let annotated_signature = Annotations.get_annotated_signature attributes in - let ret_annotation, _ = annotated_signature.Annotations.ret in - check ret_annotation - - -let is_expensive attributes = - check_attributes Annotations.ia_is_expensive attributes +let check_attributes check tenv pname = + let check_class_attributes check tenv pname = + match Annotations.get_declaring_class_annotations pname tenv with + | Some annotations -> check annotations + | None -> false in + let check_method_attributes check pname = + match Specs.proc_resolve_attributes pname with + | None -> false + | Some attributes -> + let annotated_signature = Annotations.get_annotated_signature attributes in + let ret_annotation, _ = annotated_signature.Annotations.ret in + check ret_annotation in + check_class_attributes check tenv pname || check_method_attributes check pname -let check_method check pname = - match Specs.proc_resolve_attributes pname with - | None -> false - | Some attributes -> - check_attributes check attributes +let is_expensive tenv pname = + check_attributes Annotations.ia_is_expensive tenv pname -let method_is_performance_critical pname = - check_method Annotations.ia_is_performance_critical pname +let method_is_performance_critical tenv pname = + check_attributes Annotations.ia_is_performance_critical tenv pname -let method_is_no_allcation pname = - check_method Annotations.ia_is_no_allocation pname +let method_is_no_allocation tenv pname = + check_attributes Annotations.ia_is_no_allocation tenv pname let method_overrides is_annotated tenv pname = let overrides () = let found = ref false in PatternMatch.proc_iter_overridden_methods - (fun pn -> found := is_annotated pn) + (fun pn -> found := is_annotated tenv pn) tenv pname; !found in - is_annotated pname + is_annotated tenv pname || overrides () @@ -80,12 +82,12 @@ let method_overrides_performance_critical tenv pname = let method_overrides_no_allocation tenv pname = - method_overrides method_is_no_allcation tenv pname + method_overrides method_is_no_allocation tenv pname let method_is_expensive tenv pname = - is_modeled_expensive tenv pname - || check_method Annotations.ia_is_expensive pname + is_modeled_expensive tenv pname || + check_attributes Annotations.ia_is_expensive tenv pname let lookup_call_summary pname = @@ -131,7 +133,7 @@ let is_allocator tenv pname = match pname with let method_allocates tenv pname = let annotated_ignore_allocation = - check_method Annotations.ia_is_ignore_allocations pname in + check_attributes Annotations.ia_is_ignore_allocations tenv pname in let allocates () = match lookup_call_summary pname with | Some { Specs.allocations } -> @@ -265,10 +267,8 @@ let report_allocations pname pdesc loc calls = let check_one_procedure tenv pname pdesc = - let loc = Cfg.Procdesc.get_loc pdesc in - let attributes = Cfg.Procdesc.get_attributes pdesc in - let expensive = is_expensive attributes + let expensive = is_expensive tenv pname and performance_critical = method_overrides_performance_critical tenv pname and no_allocation = @@ -323,8 +323,3 @@ let callback_performance_checker check_one_procedure tenv proc_name proc_desc; Ondemand.unset_callbacks () end - -(* -let is_performance_critical attributes = - check_attributes Annotations.ia_is_performance_critical attributes -*) diff --git a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java index 39a909de6..ad3c3ca31 100644 --- a/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java +++ b/infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java @@ -35,6 +35,66 @@ class Other { expensive(); } + void inexpensiveMethod() { + } + +} + +@Expensive +class ExpensiveClass { + + void anExpensiveMethod() { + } + +} + +@PerformanceCritical +class PerformanceCriticalClass { + + void performanceCriticalMethod1(ExpensiveClass c) { + c.anExpensiveMethod(); // should report + } + + void performanceCriticalMethod2(Other o) { + o.expensive(); // should report + } + + void performanceCriticalMethod3(Other o) { + o.callsExpensive1(); // should report + } + + void performanceCriticalMethod4(Other o) { + o.inexpensiveMethod(); // should not report + } + + +} + +class ExpensiveSubclass extends ExpensiveClass { + + void anotherExpensiveMethod() { + } + +} + +class PerformanceCriticalSubclass extends PerformanceCriticalClass { + + void subclassPerformanceCriticalMethod1(ExpensiveClass c) { + c.anExpensiveMethod(); // should report + } + + void subclassPerformanceCriticalMethod2(ExpensiveSubclass c) { + c.anotherExpensiveMethod(); // should report + } + + void subclassPerformanceCriticalMethod3(Other o) { + o.callsExpensive1(); // should report; + } + + void subclassPerformanceCriticalMethod4(Other o) { + o.inexpensiveMethod(); // should not report; + } + } @@ -93,6 +153,11 @@ public class ExpensiveCallExample implements AnnotatedInterface { return activity.findViewById(id); } + @PerformanceCritical + void callMethodOnExpensiveClass(ExpensiveClass c) { + c.anExpensiveMethod(); + } + public void annotatedPerformanceCriticalInInterface() { mOther.callsExpensive1(); } diff --git a/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java b/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java index 92834502b..ba8f5a811 100644 --- a/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java +++ b/infer/tests/endtoend/java/checkers/ExpensiveCallTest.java @@ -47,6 +47,14 @@ public class ExpensiveCallTest { "callsFindViewByIdFromView", "callsFindViewByIdFromActivity", "annotatedPerformanceCriticalInInterface", + "performanceCriticalMethod1", + "performanceCriticalMethod2", + "performanceCriticalMethod3", + // TODO: make subclassing work + //"subclassPerformanceCriticalMethod1", + //"subclassPerformanceCriticalMethod2", + //"subclassPerformanceCriticalMethod3", + "callMethodOnExpensiveClass" }; assertThat( "Results should contain " + CALLS_EXPENSIVE_METHOD,