diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index da9ab1078..940e26077 100644 --- a/infer/src/checkers/annotations.ml +++ b/infer/src/checkers/annotations.ml @@ -67,6 +67,8 @@ let on_unbind = "OnUnbind" let on_unmount = "OnUnmount" +let mainthread = "MainThread" + let nonblocking = "NonBlocking" let notnull = "NotNull" @@ -243,6 +245,8 @@ 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 b244b8ed2..c1d5d6e64 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -102,6 +102,8 @@ 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 diff --git a/infer/src/concurrency/ConcurrencyModels.ml b/infer/src/concurrency/ConcurrencyModels.ml index 022d69f1d..8fe0154b4 100644 --- a/infer/src/concurrency/ConcurrencyModels.ml +++ b/infer/src/concurrency/ConcurrencyModels.ml @@ -285,50 +285,53 @@ let is_ui_method = fun tenv pname -> List.exists matchers ~f:(fun m -> m tenv pname []) -let runs_on_ui_thread tenv proc_desc = +let runs_on_ui_thread = (* 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 + @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 ] in - let pname = Procdesc.get_proc_name proc_desc 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 - 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 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 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 Annotations.ia_is_ui_thread 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 + fun 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 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 is_annot pname tenv with + | Some override_pname -> Some - (F.asprintf "class %s%s is annotated %s" - (MF.monospaced_to_string (Typ.Name.name current_class)) - middle + (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 ) + | 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 ) diff --git a/infer/tests/codetoanalyze/java/starvation/MainThreadTest.java b/infer/tests/codetoanalyze/java/starvation/MainThreadTest.java new file mode 100644 index 000000000..5ff91df6c --- /dev/null +++ b/infer/tests/codetoanalyze/java/starvation/MainThreadTest.java @@ -0,0 +1,32 @@ +/* + * 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.MainThread; + +class MainThreadTest { + Binder b; + + void doTransact() { + try { + b.transact(0, null, null, 0); + } catch (Exception e) {} + } + + @MainThread + void callTransactBad() { + doTransact(); + } +} + +@MainThread +class AnnotatedClass { + void callTransactBad(MainThreadTest m) { + m.doTransact(); + } +} diff --git a/infer/tests/codetoanalyze/java/starvation/issues.exp b/infer/tests/codetoanalyze/java/starvation/issues.exp index 0d8f9bd98..f96fbe4ed 100644 --- a/infer/tests/codetoanalyze/java/starvation/issues.exp +++ b/infer/tests/codetoanalyze/java/starvation/issues.exp @@ -26,6 +26,8 @@ codetoanalyze/java/starvation/Interclass.java, Interclass.interclass1Bad(Intercl codetoanalyze/java/starvation/Interproc.java, Interproc.interproc1Bad(InterprocA):void, 9, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Interproc.interproc1Bad(InterprocA)`,locks `this` in class `Interproc*`,Method call: `void Interproc.interproc2Bad(InterprocA)`,locks `b` in class `InterprocA*`,[Trace 2] `void InterprocA.interproc1Bad(Interproc)`,locks `this` in class `InterprocA*`,Method call: `void InterprocA.interproc2Bad(Interproc)`,locks `d` in class `Interproc*`] codetoanalyze/java/starvation/Intraproc.java, Intraproc.intraBad(IntraprocA):void, 10, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Intraproc.intraBad(IntraprocA)`,locks `this` in class `Intraproc*`,locks `o` in class `IntraprocA*`,[Trace 2] `void IntraprocA.intraBad(Intraproc)`,locks `this` in class `IntraprocA*`,locks `o` in class `Intraproc*`] codetoanalyze/java/starvation/LegacySync.java, LegacySync.onUiThreadOpBad():java.lang.Object, 25, STARVATION, no_bucket, ERROR, [[Trace 1] `Object LegacySync.onUiThreadOpBad()`,locks `this.LegacySync.table` in class `LegacySync*`,[Trace 2] `void LegacySync.notOnUiThreadSyncedBad()`,locks `this.LegacySync.table` in class `LegacySync*`,calls `Object Future.get()` from `void LegacySync.notOnUiThreadSyncedBad()`,[Trace 1 on UI thread] `Object LegacySync.onUiThreadOpBad()`,`Object LegacySync.onUiThreadOpBad()` is annotated `UiThread`] +codetoanalyze/java/starvation/MainThreadTest.java, AnnotatedClass.callTransactBad(MainThreadTest):void, 30, STARVATION, no_bucket, ERROR, [`void AnnotatedClass.callTransactBad(MainThreadTest)`,Method call: `void MainThreadTest.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)` from `void MainThreadTest.doTransact()`,[Trace on UI thread] `void AnnotatedClass.callTransactBad(MainThreadTest)`,class `AnnotatedClass` is annotated `UiThread`] +codetoanalyze/java/starvation/MainThreadTest.java, MainThreadTest.callTransactBad():void, 23, STARVATION, no_bucket, ERROR, [`void MainThreadTest.callTransactBad()`,Method call: `void MainThreadTest.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)` from `void MainThreadTest.doTransact()`,[Trace on UI thread] `void MainThreadTest.callTransactBad()`,`void MainThreadTest.callTransactBad()` is annotated `UiThread`] codetoanalyze/java/starvation/MyActivity.java, MyActivity.onCreate(android.os.Bundle):void, 23, STARVATION, no_bucket, ERROR, [`void MyActivity.onCreate(Bundle)`,Method call: `void MyActivity.bad()`,calls `void AccountManager.setUserData(Account,String,String)` from `void MyActivity.bad()`,[Trace on UI thread] `void MyActivity.onCreate(Bundle)`,`void MyActivity.onCreate(Bundle)` is a standard UI-thread method] codetoanalyze/java/starvation/MyActivity.java, MyActivity.onDestroy():void, 53, STARVATION, no_bucket, ERROR, [`void MyActivity.onDestroy()`,Method call: `void MyActivity.bad()`,calls `void AccountManager.setUserData(Account,String,String)` from `void MyActivity.bad()`,[Trace on UI thread] `void MyActivity.onDestroy()`,`void MyActivity.onDestroy()` is a standard UI-thread method] codetoanalyze/java/starvation/MyActivity.java, MyActivity.onPause():void, 43, STARVATION, no_bucket, ERROR, [`void MyActivity.onPause()`,Method call: `void MyActivity.bad()`,calls `void AccountManager.setUserData(Account,String,String)` from `void MyActivity.bad()`,[Trace on UI thread] `void MyActivity.onPause()`,`void MyActivity.onPause()` is a standard UI-thread method]