From 14556f52b4ca6423d8020788273573bbb4a78526 Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Fri, 24 Aug 2018 01:39:13 -0700 Subject: [PATCH] [starvation] whitelist @WorkerThread methods Reviewed By: mbouaziz, ddino Differential Revision: D9480061 fbshipit-source-id: 4648319e1 --- infer/src/checkers/annotations.ml | 4 ++ infer/src/checkers/annotations.mli | 2 + infer/src/concurrency/RacerDConfig.ml | 9 ++++- .../java/starvation/Workers.java | 37 +++++++++++++++++++ .../codetoanalyze/java/starvation/issues.exp | 1 + 5 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 infer/tests/codetoanalyze/java/starvation/Workers.java diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index f89e37efe..35d485d7f 100644 --- a/infer/src/checkers/annotations.ml +++ b/infer/src/checkers/annotations.ml @@ -107,6 +107,8 @@ let visibleForTesting = "VisibleForTesting" let volatile = "volatile" +let worker_thread = "WorkerThread" + let ia_has_annotation_with (ia: Annot.Item.t) (predicate: Annot.t -> bool) : bool = List.exists ~f:(fun (a, _) -> predicate a) ia @@ -242,3 +244,5 @@ 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_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 f7a04bf1b..779e528c4 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -120,6 +120,8 @@ 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 + val pdesc_get_return_annot : Procdesc.t -> Annot.Item.t (** get the list of annotations on the return value of [pdesc] *) diff --git a/infer/src/concurrency/RacerDConfig.ml b/infer/src/concurrency/RacerDConfig.ml index 0d1868129..bedc6ca31 100644 --- a/infer/src/concurrency/RacerDConfig.ml +++ b/infer/src/concurrency/RacerDConfig.ml @@ -512,7 +512,14 @@ module Models = struct || Annotations.ia_is_on_unbind annot || Annotations.ia_is_on_unmount annot in let pname = Procdesc.get_proc_name proc_desc in - if Annotations.pdesc_has_return_annot proc_desc is_annot then + if + Annotations.pdesc_has_return_annot proc_desc Annotations.ia_is_worker_thread + || find_annotated_or_overriden_annotated_method 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 Annotations.pdesc_has_return_annot proc_desc is_annot then Some (F.asprintf "%a is annotated %s" (MF.wrap_monospaced Typ.Procname.pp) diff --git a/infer/tests/codetoanalyze/java/starvation/Workers.java b/infer/tests/codetoanalyze/java/starvation/Workers.java new file mode 100644 index 000000000..46e4d27b6 --- /dev/null +++ b/infer/tests/codetoanalyze/java/starvation/Workers.java @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2018-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import android.os.Binder; +import android.os.RemoteException; +import android.support.annotation.UiThread; +import android.support.annotation.WorkerThread; + +class Workers { + Binder b; + + void doTransact() throws RemoteException { + b.transact(0, null, null, 0); + } + + @WorkerThread + void workerOk() throws RemoteException { + doTransact(); + } + + // WorkerThread does not propagate up the call stack + @UiThread + void uiThreadBad() throws RemoteException { + workerOk(); + } + + // WorkerThread wins + @WorkerThread + @UiThread + void bothOk() throws RemoteException { + workerOk(); + } +} diff --git a/infer/tests/codetoanalyze/java/starvation/issues.exp b/infer/tests/codetoanalyze/java/starvation/issues.exp index 1874a41e9..84a442336 100644 --- a/infer/tests/codetoanalyze/java/starvation/issues.exp +++ b/infer/tests/codetoanalyze/java/starvation/issues.exp @@ -41,3 +41,4 @@ codetoanalyze/java/starvation/SuppLint.java, SuppLint.onUiThreadBad():void, 26, codetoanalyze/java/starvation/ThreadSleep.java, ThreadSleep.indirectSleepOnUIThreadBad():void, 25, STARVATION, no_bucket, ERROR, [[Trace 1] `void ThreadSleep.indirectSleepOnUIThreadBad()`,locks `this.ThreadSleep.lock` in class `ThreadSleep*`,[Trace 2] `void ThreadSleep.lockAndSleepOnNonUIThread()`,locks `this.ThreadSleep.lock` in class `ThreadSleep*`,Method call: `void ThreadSleep.sleepOnAnyThreadOk()`,calls `void Thread.sleep(long)` from `void ThreadSleep.sleepOnAnyThreadOk()`] codetoanalyze/java/starvation/ThreadSleep.java, ThreadSleep.sleepOnUIThreadBad():void, 18, STARVATION, no_bucket, ERROR, [`void ThreadSleep.sleepOnUIThreadBad()`,calls `void Thread.sleep(long)` from `void ThreadSleep.sleepOnUIThreadBad()`] codetoanalyze/java/starvation/UIDeadlock.java, UIDeadlock.onUIThreadBad():void, 26, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UIDeadlock.onUIThreadBad()`,locks `this` in class `UIDeadlock*`,locks `this.UIDeadlock.lockB` in class `UIDeadlock*`,[Trace 2] `void UIDeadlock.notOnUIThreadBad()`,locks `this.UIDeadlock.lockB` in class `UIDeadlock*`,locks `this` in class `UIDeadlock*`] +codetoanalyze/java/starvation/Workers.java, Workers.uiThreadBad():void, 28, STARVATION, no_bucket, ERROR, [`void Workers.uiThreadBad()`,Method call: `void Workers.workerOk()`,Method call: `void Workers.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)` from `void Workers.doTransact()`]