From cf6ced0580f575c26f17bd09d706254e48672375 Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Thu, 4 Apr 2019 02:53:47 -0700 Subject: [PATCH] [racerd] on-annotations Reviewed By: jvillard Differential Revision: D14756737 fbshipit-source-id: 434859ce7 --- infer/src/checkers/annotations.ml | 36 +----- infer/src/checkers/annotations.mli | 19 +-- infer/src/concurrency/ConcurrencyModels.ml | 118 ++++++++++-------- infer/src/concurrency/ConcurrencyModels.mli | 2 +- infer/src/concurrency/RacerD.ml | 8 +- infer/src/concurrency/RacerDModels.ml | 4 +- infer/src/istd/IOption.ml | 4 + infer/src/istd/IOption.mli | 10 ++ .../java/racerd/Annotations.java | 15 +++ 9 files changed, 111 insertions(+), 105 deletions(-) diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index 3caa41fbb..176a8b213 100644 --- a/infer/src/checkers/annotations.ml +++ b/infer/src/checkers/annotations.ml @@ -57,16 +57,6 @@ let no_allocation = "NoAllocation" let nullable = "Nullable" -let on_bind = "OnBind" - -let on_event = "OnEvent" - -let on_mount = "OnMount" - -let on_unbind = "OnUnbind" - -let on_unmount = "OnUnmount" - let mainthread = "MainThread" let nonblocking = "NonBlocking" @@ -121,15 +111,13 @@ let ma_has_annotation_with ({return; params} : Annot.Method.t) (predicate : Anno has_annot return || List.exists ~f:has_annot params +let get_annot_ending ({class_name} : Annot.t) = + String.rsplit2 class_name ~on:'.' |> Option.value_map ~default:class_name ~f:snd + + (** [annot_ends_with annot ann_name] returns true if the class name of [annot], without the package, is equal to [ann_name] *) -let annot_ends_with annot ann_name = - match String.rsplit2 annot.Annot.class_name ~on:'.' with - | None -> - String.equal annot.Annot.class_name ann_name - | Some (_, annot_class_name) -> - String.equal annot_class_name ann_name - +let annot_ends_with annot ann_name = String.equal ann_name (get_annot_ending annot) let class_name_matches s ((annot : Annot.t), _) = String.equal s annot.class_name @@ -234,20 +222,6 @@ let ia_is_inject ia = ia_ends_with ia inject let ia_is_suppress_lint ia = ia_ends_with ia suppress_lint -let ia_is_on_event ia = ia_ends_with ia on_event - -let ia_is_on_bind ia = ia_ends_with ia on_bind - -let ia_is_on_mount ia = ia_ends_with ia on_mount - -let ia_is_on_unbind ia = ia_ends_with ia on_unbind - -let ia_is_on_unmount ia = ia_ends_with ia on_unmount - -let ia_is_ui_thread ia = ia_ends_with ia ui_thread - -let ia_is_mainthread ia = ia_ends_with ia mainthread - let ia_is_thread_confined ia = ia_ends_with ia thread_confined let ia_is_worker_thread ia = ia_ends_with ia worker_thread diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index c1d5d6e64..69df06165 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -41,12 +41,17 @@ val thread_confined : string val thread_safe : string +val mainthread : string + val ui_thread : string val visibleForTesting : string val generated_graphql : string +val get_annot_ending : Annot.t -> string +(** get the '.'-last component of an annotation *) + val annot_ends_with : Annot.t -> string -> bool (** [annot_ends_with annot ann_name] returns true if the class name of [annot], without the package, is equal to [ann_name] *) @@ -92,18 +97,6 @@ val ia_is_inject : Annot.Item.t -> bool val ia_is_suppress_lint : Annot.Item.t -> bool -val ia_is_on_event : Annot.Item.t -> bool - -val ia_is_on_bind : Annot.Item.t -> bool - -val ia_is_on_mount : Annot.Item.t -> bool - -val ia_is_on_unbind : Annot.Item.t -> bool - -val ia_is_on_unmount : Annot.Item.t -> bool - -val ia_is_mainthread : Annot.Item.t -> bool - val ia_is_not_thread_safe : Annot.Item.t -> bool val ia_is_nonblocking : Annot.Item.t -> bool @@ -118,8 +111,6 @@ val ia_is_thread_confined : Annot.Item.t -> bool val ia_is_thrift_service : Annot.Item.t -> bool -val ia_is_ui_thread : Annot.Item.t -> bool - val ia_is_volatile : Annot.Item.t -> bool val ia_is_worker_thread : Annot.Item.t -> bool diff --git a/infer/src/concurrency/ConcurrencyModels.ml b/infer/src/concurrency/ConcurrencyModels.ml index d1f16f043..4262f5c0a 100644 --- a/infer/src/concurrency/ConcurrencyModels.ml +++ b/infer/src/concurrency/ConcurrencyModels.ml @@ -312,12 +312,21 @@ let get_current_class_and_annotated_superclasses is_annot tenv pname = None -let find_annotated_or_overriden_annotated_method ~attrs_of_pname is_annot pname tenv = +let is_class_or_superclasses_annotated is_annot tenv pname = + get_current_class_and_annotated_superclasses is_annot tenv pname + |> Option.exists ~f:(fun (_, annotated) -> not (List.is_empty annotated)) + + +let find_method_or_override_annotated ~attrs_of_pname is_annot pname tenv = PatternMatch.override_find (fun pn -> Annotations.pname_has_return_annot pn ~attrs_of_pname is_annot) tenv pname +let is_method_or_override_annotated ~attrs_of_pname is_annot pname tenv = + find_method_or_override_annotated ~attrs_of_pname is_annot pname tenv |> Option.is_some + + let ui_matcher_records = let open MethodMatcher in let fragment_methods = @@ -362,57 +371,64 @@ let is_ui_method = fun tenv pname -> MethodMatcher.of_list matchers tenv pname [] -let runs_on_ui_thread = - (* assume that methods annotated with @UiThread, @OnEvent, @OnBind, @OnMount, @OnUnbind, - @OnUnmount always run on the UI thread *) - let annotation_matchers = - [ Annotations.ia_is_mainthread - ; Annotations.ia_is_ui_thread - ; Annotations.ia_is_on_bind - ; Annotations.ia_is_on_event - ; Annotations.ia_is_on_mount - ; Annotations.ia_is_on_unbind - ; Annotations.ia_is_on_unmount ] +let if_pred_evalopt ~pred ~f x = + IOption.if_none_evalopt x ~f:(fun () -> if pred () then Some (f ()) else None) + + +(* assume that methods annotated with @MainThread, @UiThread, + and any string starting with "On" always run on the UI thread. + We do the latter because there are too many to precisely list. *) +let is_uithread annots = + let f (annot, _) = + let ending = Annotations.get_annot_ending annot in + String.equal ending Annotations.mainthread + || String.equal ending Annotations.ui_thread + || String.is_prefix ~prefix:"On" ending in - let is_annot annot = List.exists annotation_matchers ~f:(fun m -> m annot) in - let mono_pname = MF.wrap_monospaced Typ.Procname.pp in - fun ~attrs_of_pname tenv proc_desc -> - let pname = Procdesc.get_proc_name proc_desc in - if - Annotations.pdesc_has_return_annot proc_desc Annotations.ia_is_worker_thread - || find_annotated_or_overriden_annotated_method ~attrs_of_pname - Annotations.ia_is_worker_thread pname tenv - |> Option.is_some - || get_current_class_and_annotated_superclasses Annotations.ia_is_worker_thread tenv pname - |> Option.value_map ~default:false ~f:(function _, [] -> false | _ -> true) - then None - else if is_ui_method tenv pname then - Some (F.asprintf "%a is a standard UI-thread method" mono_pname pname) - else if Annotations.pdesc_has_return_annot proc_desc is_annot then - Some - (F.asprintf "%a is annotated %s" mono_pname pname - (MF.monospaced_to_string Annotations.ui_thread)) - else - match find_annotated_or_overriden_annotated_method ~attrs_of_pname is_annot pname tenv with - | Some override_pname -> - Some - (F.asprintf "class %a overrides %a, which is annotated %s" mono_pname pname mono_pname - override_pname - (MF.monospaced_to_string Annotations.ui_thread)) - | None -> ( - match get_current_class_and_annotated_superclasses is_annot tenv pname with - | Some (current_class, (super_class :: _ as super_classes)) -> - let middle = - if List.exists super_classes ~f:(Typ.Name.equal current_class) then "" - else F.asprintf " extends %a, which" (MF.wrap_monospaced Typ.Name.pp) super_class - in - Some - (F.asprintf "class %s%s is annotated %s" - (MF.monospaced_to_string (Typ.Name.name current_class)) - middle - (MF.monospaced_to_string Annotations.ui_thread)) - | _ -> - None ) + List.exists annots ~f + + +let mono_pname = MF.wrap_monospaced Typ.Procname.pp + +let runs_on_ui_thread ~attrs_of_pname tenv proc_desc = + let pname = Procdesc.get_proc_name proc_desc in + if + is_method_or_override_annotated ~attrs_of_pname Annotations.ia_is_worker_thread pname tenv + || is_class_or_superclasses_annotated Annotations.ia_is_worker_thread tenv pname + then None + else + None + |> if_pred_evalopt + ~pred:(fun () -> is_ui_method tenv pname) + ~f:(fun () -> F.asprintf "%a is a standard UI-thread method" mono_pname pname) + |> if_pred_evalopt + ~pred:(fun () -> Annotations.pdesc_has_return_annot proc_desc is_uithread) + ~f:(fun () -> + F.asprintf "%a is annotated %s" mono_pname pname + (MF.monospaced_to_string Annotations.ui_thread) ) + |> IOption.if_none_evalopt ~f:(fun () -> + find_method_or_override_annotated ~attrs_of_pname is_uithread pname tenv + |> Option.map ~f:(fun override_pname -> + F.asprintf "class %a overrides %a, which is annotated %s" mono_pname pname + mono_pname override_pname + (MF.monospaced_to_string Annotations.ui_thread) ) ) + |> IOption.if_none_evalopt ~f:(fun () -> + get_current_class_and_annotated_superclasses is_uithread tenv pname + |> Option.bind ~f:(function + | current_class, (super_class :: _ as super_classes) -> + let middle = + if List.exists super_classes ~f:(Typ.Name.equal current_class) then "" + else + F.asprintf " extends %a, which" (MF.wrap_monospaced Typ.Name.pp) + super_class + in + Some + (F.asprintf "class %s%s is annotated %s" + (MF.monospaced_to_string (Typ.Name.name current_class)) + middle + (MF.monospaced_to_string Annotations.ui_thread)) + | _ -> + None ) ) let cpp_lock_types_matcher = Clang.lock_types_matcher diff --git a/infer/src/concurrency/ConcurrencyModels.mli b/infer/src/concurrency/ConcurrencyModels.mli index 14918bb40..c2907a42e 100644 --- a/infer/src/concurrency/ConcurrencyModels.mli +++ b/infer/src/concurrency/ConcurrencyModels.mli @@ -47,7 +47,7 @@ val runs_on_ui_thread : val get_current_class_and_annotated_superclasses : (Annot.Item.t -> bool) -> Tenv.t -> Typ.Procname.t -> (Typ.name * Typ.name list) option -val find_annotated_or_overriden_annotated_method : +val find_method_or_override_annotated : attrs_of_pname:(Typ.Procname.t -> ProcAttributes.t option) -> (Annot.Item.t -> bool) -> Typ.Procname.t diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index 3c7b2b4d1..4717b3eb2 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/infer/src/concurrency/RacerD.ml @@ -480,18 +480,14 @@ module TransferFunctions (CFG : ProcCfg.S) = struct {astate' with accesses} - let if_none_then = IOption.value_default_f - - let if_none_do ~f x = match x with None -> f () | Some _ -> x - let exec_instr (astate : Domain.t) ({ProcData.pdesc} as proc_data) _ (instr : HilInstr.t) = match instr with | Call (ret_base, Direct callee_pname, actuals, call_flags, loc) -> let astate = add_reads actuals loc astate proc_data in treat_call_acquiring_ownership ret_base callee_pname actuals loc proc_data astate () - |> if_none_do + |> IOption.if_none_evalopt ~f:(treat_container_accesses ret_base callee_pname actuals loc proc_data astate) - |> if_none_then + |> IOption.if_none_eval ~f:(do_proc_call ret_base callee_pname actuals call_flags loc proc_data astate) | Call (_, Indirect _, _, _, _) -> if Typ.Procname.is_java (Procdesc.get_proc_name pdesc) then diff --git a/infer/src/concurrency/RacerDModels.ml b/infer/src/concurrency/RacerDModels.ml index ffcf0bd9b..5f620ac15 100644 --- a/infer/src/concurrency/RacerDModels.ml +++ b/infer/src/concurrency/RacerDModels.ml @@ -342,8 +342,8 @@ let is_thread_safe_class pname tenv = let is_thread_safe_method pname tenv = - find_annotated_or_overriden_annotated_method ~attrs_of_pname:Summary.proc_resolve_attributes - is_thread_safe pname tenv + find_method_or_override_annotated ~attrs_of_pname:Summary.proc_resolve_attributes is_thread_safe + pname tenv |> Option.is_some diff --git a/infer/src/istd/IOption.ml b/infer/src/istd/IOption.ml index 7ca0edbd1..3570c94a9 100644 --- a/infer/src/istd/IOption.ml +++ b/infer/src/istd/IOption.ml @@ -10,3 +10,7 @@ open! IStd let find_value_exn = function None -> raise Caml.Not_found | Some v -> v let value_default_f ~f = function None -> f () | Some v -> v + +let if_none_evalopt ~f x = match x with None -> f () | Some _ -> x + +let if_none_eval = value_default_f diff --git a/infer/src/istd/IOption.mli b/infer/src/istd/IOption.mli index 6c5177d37..dde2d6567 100644 --- a/infer/src/istd/IOption.mli +++ b/infer/src/istd/IOption.mli @@ -12,3 +12,13 @@ val find_value_exn : 'a option -> 'a val value_default_f : f:(unit -> 'a) -> 'a option -> 'a (** Like [Option.value ~default:(f ())] but [f] is called only if [None]. *) + +val if_none_evalopt : f:(unit -> 'a option) -> 'a option -> 'a option +(** [if_none_evalopt ~f x] evaluates to [f ()] if [x = None], otherwise returns [x]. + Useful for chaining matchers where the first returning non-[None] determines + the result. *) + +val if_none_eval : f:(unit -> 'a) -> 'a option -> 'a +(** [if_none_eval ~f x] evaluates to [y] if [x=Some y] else to [f ()]. + Useful for terminating chains built with [if_none_evalopt]. + This is exactly the same as [value_default_f] but with a better name. *) diff --git a/infer/tests/codetoanalyze/java/racerd/Annotations.java b/infer/tests/codetoanalyze/java/racerd/Annotations.java index 7a8e9449f..33c76c421 100644 --- a/infer/tests/codetoanalyze/java/racerd/Annotations.java +++ b/infer/tests/codetoanalyze/java/racerd/Annotations.java @@ -439,3 +439,18 @@ class ExtendsClassOnUiThread extends AllMethodsOnUiThread { return super.bar(); } } + +// All annotations that start with "On" are assumed to be on the main thread +@Target(ElementType.METHOD) +@Retention(RetentionPolicy.CLASS) +@interface OnXYZ {} + +@ThreadSafe +class WeirdAnnotation { + int f; + + @OnXYZ + void fooOk() { + f = 0; + } +}