From 78ad6e6d00409cad9abbecac9b974d2cecbcd168 Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Fri, 11 May 2018 02:46:21 -0700 Subject: [PATCH] [concurrency] extend notion of ui thread Summary: The annotation UiThread can, and is, used on classes as opposed to just methods, so extend the modelling to account for this. Also, consider the annotation hereditary. Reviewed By: jeremydubreil Differential Revision: D7910762 fbshipit-source-id: 0df2c81 --- infer/src/concurrency/RacerD.ml | 4 +- infer/src/concurrency/RacerDConfig.ml | 58 ++++++++++++------- infer/src/concurrency/RacerDConfig.mli | 5 +- infer/src/concurrency/starvation.ml | 2 +- .../java/racerd/Annotations.java | 27 +++++++++ 5 files changed, 72 insertions(+), 24 deletions(-) diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index 80988218c..edecccc55 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/infer/src/concurrency/RacerD.ml @@ -688,7 +688,9 @@ let analyze_procedure {Callbacks.proc_desc; get_proc_desc; tenv; summary} = let proc_data = ProcData.make proc_desc tenv get_proc_desc in let initial = let threads = - if Models.runs_on_ui_thread proc_desc || Models.is_thread_confined_method tenv proc_desc + if + Models.runs_on_ui_thread tenv proc_desc + || Models.is_thread_confined_method tenv proc_desc then ThreadsDomain.AnyThreadButSelf else if Procdesc.is_java_synchronized proc_desc || Models.is_marked_thread_safe proc_desc tenv diff --git a/infer/src/concurrency/RacerDConfig.ml b/infer/src/concurrency/RacerDConfig.ml index 6b8a3906e..8c2a59b49 100644 --- a/infer/src/concurrency/RacerDConfig.ml +++ b/infer/src/concurrency/RacerDConfig.ml @@ -431,17 +431,6 @@ module Models = struct (Procdesc.get_proc_name pdesc) - (* we don't want to warn on methods that run on the UI thread because they should always be - single-threaded *) - let runs_on_ui_thread proc_desc = - (* assume that methods annotated with @UiThread, @OnEvent, @OnBind, @OnMount, @OnUnbind, - @OnUnmount always run on the UI thread *) - Annotations.pdesc_has_return_annot proc_desc (fun annot -> - Annotations.ia_is_ui_thread annot || Annotations.ia_is_on_bind annot - || Annotations.ia_is_on_event annot || Annotations.ia_is_on_mount annot - || Annotations.ia_is_on_unbind annot || Annotations.ia_is_on_unmount annot ) - - let threadsafe_annotations = Annotations.thread_safe :: AnnotationAliases.of_json Config.threadsafe_aliases @@ -493,18 +482,51 @@ module Models = struct && not (should_skip pn) - let get_current_class_and_threadsafe_superclasses tenv pname = + let get_current_class_and_annotated_superclasses is_annot tenv pname = match pname with | Typ.Procname.Java java_pname -> let current_class = Typ.Procname.Java.get_class_type_name java_pname in - let thread_safe_annotated_classes = - PatternMatch.find_superclasses_with_attributes is_thread_safe tenv current_class + let annotated_classes = + PatternMatch.find_superclasses_with_attributes is_annot tenv current_class in - Some (current_class, thread_safe_annotated_classes) + Some (current_class, annotated_classes) | _ -> None + let is_annotated_or_overriden_annotated_method is_annot pname tenv = + PatternMatch.override_exists + (fun pn -> + Annotations.pname_has_return_annot pn ~attrs_of_pname:Summary.proc_resolve_attributes + is_annot ) + tenv pname + + + (* we don't want to warn on methods that run on the UI thread because they should always be + single-threaded *) + let runs_on_ui_thread tenv proc_desc = + (* assume that methods annotated with @UiThread, @OnEvent, @OnBind, @OnMount, @OnUnbind, + @OnUnmount always run on the UI thread *) + let is_annot annot = + Annotations.ia_is_ui_thread annot || Annotations.ia_is_on_bind annot + || Annotations.ia_is_on_event annot || Annotations.ia_is_on_mount annot + || Annotations.ia_is_on_unbind annot || Annotations.ia_is_on_unmount annot + in + let pname = Procdesc.get_proc_name proc_desc in + Annotations.pdesc_has_return_annot proc_desc is_annot + || is_annotated_or_overriden_annotated_method is_annot pname tenv + || + match get_current_class_and_annotated_superclasses Annotations.ia_is_ui_thread tenv pname with + | Some (_, _ :: _) -> + true + | _ -> + false + + + let get_current_class_and_threadsafe_superclasses tenv pname = + get_current_class_and_annotated_superclasses is_thread_safe tenv pname + + let is_thread_safe_class pname tenv = not ((* current class not marked thread-safe *) @@ -519,11 +541,7 @@ module Models = struct let is_thread_safe_method pname tenv = - PatternMatch.override_exists - (fun pn -> - Annotations.pname_has_return_annot pn ~attrs_of_pname:Summary.proc_resolve_attributes - is_thread_safe ) - tenv pname + is_annotated_or_overriden_annotated_method is_thread_safe pname tenv let is_marked_thread_safe pdesc tenv = diff --git a/infer/src/concurrency/RacerDConfig.mli b/infer/src/concurrency/RacerDConfig.mli index 48a5f3236..2ecdc41d9 100644 --- a/infer/src/concurrency/RacerDConfig.mli +++ b/infer/src/concurrency/RacerDConfig.mli @@ -54,10 +54,11 @@ module Models : sig completely different classes that don't necessarily run on the same thread as the confined object. *) - val runs_on_ui_thread : Procdesc.t -> bool + val runs_on_ui_thread : Tenv.t -> Procdesc.t -> bool (** We don't want to warn on methods that run on the UI thread because they should always be single-threaded. Assume that methods annotated with @UiThread, @OnEvent, @OnBind, @OnMount, - @OnUnbind, @OnUnmount always run on the UI thread *) + @OnUnbind, @OnUnmount always run on the UI thread. Also assume that any superclass + marked @UiThread implies all methods are on UI thread. *) val should_analyze_proc : Procdesc.t -> Tenv.t -> bool (** return true if we should compute a summary for the procedure. if this returns false, we won't diff --git a/infer/src/concurrency/starvation.ml b/infer/src/concurrency/starvation.ml index 5b9d1b706..ea47377cb 100644 --- a/infer/src/concurrency/starvation.ml +++ b/infer/src/concurrency/starvation.ml @@ -116,7 +116,7 @@ let analyze_procedure {Callbacks.proc_desc; tenv; summary} = ~f:(StarvationDomain.acquire StarvationDomain.empty loc) in let initial = - if RacerDConfig.Models.runs_on_ui_thread proc_desc then + if RacerDConfig.Models.runs_on_ui_thread tenv proc_desc then StarvationDomain.set_on_main_thread initial else initial in diff --git a/infer/tests/codetoanalyze/java/racerd/Annotations.java b/infer/tests/codetoanalyze/java/racerd/Annotations.java index a937864d5..2a70765d5 100644 --- a/infer/tests/codetoanalyze/java/racerd/Annotations.java +++ b/infer/tests/codetoanalyze/java/racerd/Annotations.java @@ -408,3 +408,30 @@ class Annotations implements Interface { } } + + +@UiThread +@ThreadSafe +class AllMethodsOnUiThread { + int f; + + void fooOk() { + f = 5; + } + + int bar() { + return f; + } +} + +class ExtendsClassOnUiThread extends AllMethodsOnUiThread { + @Override + void fooOk() { + f = 9; + } + + @Override + int bar() { + return super.bar(); + } +}