diff --git a/infer/src/concurrency/ConcurrencyModels.ml b/infer/src/concurrency/ConcurrencyModels.ml index f84714833..69c954ada 100644 --- a/infer/src/concurrency/ConcurrencyModels.ml +++ b/infer/src/concurrency/ConcurrencyModels.ml @@ -158,11 +158,109 @@ let find_annotated_or_overriden_annotated_method is_annot pname tenv = tenv pname -(* we don't want to warn on methods that run on the UI thread because they should always be - single-threaded *) +let is_call_of_class ?(search_superclasses = true) ?(method_prefix = false) + ?(actuals_pred = fun _ -> true) clazz methods = + let method_matcher = + if method_prefix then fun current_method target_method -> + String.is_prefix current_method ~prefix:target_method + else fun current_method target_method -> String.equal current_method target_method + in + let class_matcher = + let is_target_class = + let target = Typ.Name.Java.from_string clazz in + fun tname -> Typ.Name.equal tname target + in + if search_superclasses then fun tenv classname -> + let is_target_struct tname _ = is_target_class tname in + PatternMatch.supertype_exists tenv is_target_struct classname + else fun _ classname -> is_target_class classname + in + (fun tenv pn actuals -> + actuals_pred actuals + && + match pn with + | Typ.Procname.Java java_pname -> + let mthd = Typ.Procname.Java.get_method java_pname in + List.exists methods ~f:(method_matcher mthd) + && + let classname = Typ.Procname.Java.get_class_type_name java_pname in + class_matcher tenv classname + | _ -> + false ) + |> Staged.stage + + +let is_ui_method = + let is_activity_ui_method = + (* lifecycle methods of Activity *) + is_call_of_class ~search_superclasses:true "android.app.Activity" + (* sort police: this is in lifecycle order *) + ["onCreate"; "onStart"; "onRestart"; "onResume"; "onPause"; "onStop"; "onDestroy"] + |> Staged.unstage + in + let is_fragment_ui_method = + (* lifecycle methods of Fragment *) + let fragment_methods = + (* sort police: this is in lifecycle order *) + [ "onAttach" + ; "onCreate" + ; "onCreateView" + ; "onActivityCreated" + ; "onStart" + ; "onResume" + ; "onPause" + ; "onStop" + ; "onDestroyView" + ; "onDestroy" + ; "onDetach" ] + in + let fragment_v4_matcher = + is_call_of_class ~search_superclasses:true "android.support.v4.app.Fragment" fragment_methods + |> Staged.unstage + in + let fragment_matcher = + is_call_of_class ~search_superclasses:true "android.app.Fragment" fragment_methods + |> Staged.unstage + in + fun tenv pname actuals -> + fragment_v4_matcher tenv pname actuals || fragment_matcher tenv pname actuals + in + let is_service_ui_method = + (* lifecycle methods of Service *) + is_call_of_class ~search_superclasses:true "android.app.Service" + ["onBind"; "onCreate"; "onDestroy"; "onStartCommand"] + |> Staged.unstage + in + let is_content_provider_ui_method = + is_call_of_class ~search_superclasses:true "android.content.ContentProvider" ["onCreate"] + |> Staged.unstage + in + let is_broadcast_receiver_ui_method = + is_call_of_class ~search_superclasses:true "android.content.BroadcastReceiver" ["onReceive"] + |> Staged.unstage + in + let is_view_ui_method = + (* according to Android documentation, *all* methods of the View class run on UI thread, but + let's be a bit conservative and catch all methods that start with "on". + https://developer.android.com/reference/android/view/View.html *) + is_call_of_class ~search_superclasses:true ~method_prefix:true "android.view.View" ["on"] + |> Staged.unstage + in + let matchers = + [ is_activity_ui_method + ; is_fragment_ui_method + ; is_service_ui_method + ; is_content_provider_ui_method + ; is_broadcast_receiver_ui_method + ; is_view_ui_method ] + in + (* we pass an empty actuals list because all matching is done on class and method name here *) + fun tenv pname -> List.exists matchers ~f:(fun m -> m tenv pname []) + + 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 *) + @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 @@ -170,6 +268,7 @@ let runs_on_ui_thread tenv proc_desc = || Annotations.ia_is_on_unmount annot in let pname = Procdesc.get_proc_name proc_desc 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 @@ -177,34 +276,23 @@ let runs_on_ui_thread tenv proc_desc = || 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" - (MF.wrap_monospaced Typ.Procname.pp) - pname + (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" - (MF.wrap_monospaced Typ.Procname.pp) - pname - (MF.wrap_monospaced Typ.Procname.pp) + (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, _) - when let open PatternMatch in - is_subtype_of_str tenv current_class "android.app.Service" - && not (is_subtype_of_str tenv current_class "android.app.IntentService") -> - Some - (F.asprintf "class %s extends %s" - (MF.monospaced_to_string (Typ.Name.name current_class)) - (MF.monospaced_to_string "android.app.Service")) | Some (current_class, (super_class :: _ as super_classes)) -> let middle = if List.exists super_classes ~f:(Typ.Name.equal current_class) then "" @@ -217,35 +305,3 @@ let runs_on_ui_thread tenv proc_desc = (MF.monospaced_to_string Annotations.ui_thread)) | _ -> None ) - - -let is_call_of_class ?(search_superclasses = true) ?(method_prefix = false) - ?(actuals_pred = fun _ -> true) clazz methods = - let method_matcher = - if method_prefix then fun current_method target_method -> - String.is_prefix current_method ~prefix:target_method - else fun current_method target_method -> String.equal current_method target_method - in - let class_matcher = - let is_target_class = - let target = Typ.Name.Java.from_string clazz in - fun tname -> Typ.Name.equal tname target - in - if search_superclasses then fun tenv classname -> - let is_target_struct tname _ = is_target_class tname in - PatternMatch.supertype_exists tenv is_target_struct classname - else fun _ classname -> is_target_class classname - in - (fun tenv pn actuals -> - actuals_pred actuals - && - match pn with - | Typ.Procname.Java java_pname -> - let mthd = Typ.Procname.Java.get_method java_pname in - List.exists methods ~f:(method_matcher mthd) - && - let classname = Typ.Procname.Java.get_class_type_name java_pname in - class_matcher tenv classname - | _ -> - false ) - |> Staged.stage diff --git a/infer/tests/codetoanalyze/java/starvation/MyActivity.java b/infer/tests/codetoanalyze/java/starvation/MyActivity.java new file mode 100644 index 000000000..095381efa --- /dev/null +++ b/infer/tests/codetoanalyze/java/starvation/MyActivity.java @@ -0,0 +1,55 @@ +/* + * 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.app.Activity; +import android.accounts.AccountManager; +import android.os.Bundle; + +// test is for recognizing Activity lifecycle methods +class MyActivity extends Activity { + AccountManager am; + private void bad() { + am.setUserData(null, null, null); + } + + // overrides so no Bad suffixes + + @Override + protected void onCreate (Bundle savedInstanceState) { + bad(); + } + + @Override + public void onStart() { + bad(); + } + + @Override + public void onRestart() { + bad(); + } + + @Override + public void onResume() { + bad(); + } + + @Override + public void onPause() { + bad(); + } + + @Override + public void onStop() { + bad(); + } + + @Override + public void onDestroy() { + bad(); + } +} diff --git a/infer/tests/codetoanalyze/java/starvation/issues.exp b/infer/tests/codetoanalyze/java/starvation/issues.exp index 60de11819..0d8f9bd98 100644 --- a/infer/tests/codetoanalyze/java/starvation/issues.exp +++ b/infer/tests/codetoanalyze/java/starvation/issues.exp @@ -26,6 +26,13 @@ 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/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] +codetoanalyze/java/starvation/MyActivity.java, MyActivity.onRestart():void, 33, STARVATION, no_bucket, ERROR, [`void MyActivity.onRestart()`,Method call: `void MyActivity.bad()`,calls `void AccountManager.setUserData(Account,String,String)` from `void MyActivity.bad()`,[Trace on UI thread] `void MyActivity.onRestart()`,`void MyActivity.onRestart()` is a standard UI-thread method] +codetoanalyze/java/starvation/MyActivity.java, MyActivity.onResume():void, 38, STARVATION, no_bucket, ERROR, [`void MyActivity.onResume()`,Method call: `void MyActivity.bad()`,calls `void AccountManager.setUserData(Account,String,String)` from `void MyActivity.bad()`,[Trace on UI thread] `void MyActivity.onResume()`,`void MyActivity.onResume()` is a standard UI-thread method] +codetoanalyze/java/starvation/MyActivity.java, MyActivity.onStart():void, 28, STARVATION, no_bucket, ERROR, [`void MyActivity.onStart()`,Method call: `void MyActivity.bad()`,calls `void AccountManager.setUserData(Account,String,String)` from `void MyActivity.bad()`,[Trace on UI thread] `void MyActivity.onStart()`,`void MyActivity.onStart()` is a standard UI-thread method] +codetoanalyze/java/starvation/MyActivity.java, MyActivity.onStop():void, 48, STARVATION, no_bucket, ERROR, [`void MyActivity.onStop()`,Method call: `void MyActivity.bad()`,calls `void AccountManager.setUserData(Account,String,String)` from `void MyActivity.bad()`,[Trace on UI thread] `void MyActivity.onStop()`,`void MyActivity.onStop()` is a standard UI-thread method] codetoanalyze/java/starvation/NonBlk.java, NonBlk.deadlockABBad():void, 34, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void NonBlk.deadlockABBad()`,locks `this` in class `NonBlk*`,locks `this.NonBlk.future` in class `NonBlk*`,[Trace 2] `void NonBlk.deadlockBABad()`,locks `this.NonBlk.future` in class `NonBlk*`,locks `this` in class `NonBlk*`] codetoanalyze/java/starvation/ObjWait.java, ObjWait.indirectWaitOnMainWithoutTimeoutBad():void, 46, STARVATION, no_bucket, ERROR, [[Trace 1] `void ObjWait.indirectWaitOnMainWithoutTimeoutBad()`,locks `this.ObjWait.lock` in class `ObjWait*`,[Trace 2] `void ObjWait.lockAndWaitOnAnyWithoutTimeoutBad()`,locks `this.ObjWait.lock` in class `ObjWait*`,calls `void Object.wait()` from `void ObjWait.lockAndWaitOnAnyWithoutTimeoutBad()`,[Trace 1 on UI thread] `void ObjWait.indirectWaitOnMainWithoutTimeoutBad()`,`void ObjWait.indirectWaitOnMainWithoutTimeoutBad()` is annotated `UiThread`] codetoanalyze/java/starvation/ObjWait.java, ObjWait.waitOnMainWithExcessiveTimeout1Bad():void, 31, STARVATION, no_bucket, ERROR, [`void ObjWait.waitOnMainWithExcessiveTimeout1Bad()`,calls `void Object.wait(long)` from `void ObjWait.waitOnMainWithExcessiveTimeout1Bad()`,[Trace on UI thread] `void ObjWait.waitOnMainWithExcessiveTimeout1Bad()`,`void ObjWait.waitOnMainWithExcessiveTimeout1Bad()` is annotated `UiThread`] @@ -34,8 +41,7 @@ codetoanalyze/java/starvation/ObjWait.java, ObjWait.waitOnMainWithoutTimeoutBad( codetoanalyze/java/starvation/PubPriv.java, PubPriv.alsoBad():void, 25, STARVATION, no_bucket, ERROR, [`void PubPriv.alsoBad()`,Method call: `void PubPriv.transactBad()`,Method call: `void PubPriv.doTransactOk()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)` from `void PubPriv.doTransactOk()`,[Trace on UI thread] `void PubPriv.alsoBad()`,Method call: `void PubPriv.transactBad()`,Method call: `void PubPriv.doTransactOk()`,`void PubPriv.doTransactOk()` is annotated `UiThread`] codetoanalyze/java/starvation/PubPriv.java, PubPriv.callOneWayBad():void, 47, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void PubPriv.callOneWayBad()`,Method call: `void PubPriv.oneWayOk()`,locks `this.PubPriv.lockA` in class `PubPriv*`,locks `this.PubPriv.lockB` in class `PubPriv*`,[Trace 2] `void PubPriv.callAnotherWayBad()`,Method call: `void PubPriv.anotherWayOk()`,locks `this.PubPriv.lockB` in class `PubPriv*`,locks `this.PubPriv.lockA` in class `PubPriv*`] codetoanalyze/java/starvation/PubPriv.java, PubPriv.transactBad():void, 21, STARVATION, no_bucket, ERROR, [`void PubPriv.transactBad()`,Method call: `void PubPriv.doTransactOk()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)` from `void PubPriv.doTransactOk()`,[Trace on UI thread] `void PubPriv.transactBad()`,Method call: `void PubPriv.doTransactOk()`,`void PubPriv.doTransactOk()` is annotated `UiThread`] -codetoanalyze/java/starvation/ServiceOnUIThread.java, ServiceOnUIThread.onBind(android.content.Intent):android.os.IBinder, 19, STARVATION, no_bucket, ERROR, [`IBinder ServiceOnUIThread.onBind(Intent)`,Method call: `void ServiceOnUIThread.transactBad()`,calls `boolean IBinder.transact(int,Parcel,Parcel,int)` from `void ServiceOnUIThread.transactBad()`,[Trace on UI thread] `IBinder ServiceOnUIThread.onBind(Intent)`,class `ServiceOnUIThread` extends `android.app.Service`] -codetoanalyze/java/starvation/ServiceOnUIThread.java, ServiceOnUIThread.transactBad():void, 25, STARVATION, no_bucket, ERROR, [`void ServiceOnUIThread.transactBad()`,calls `boolean IBinder.transact(int,Parcel,Parcel,int)` from `void ServiceOnUIThread.transactBad()`,[Trace on UI thread] `void ServiceOnUIThread.transactBad()`,class `ServiceOnUIThread` extends `android.app.Service`] +codetoanalyze/java/starvation/ServiceOnUIThread.java, ServiceOnUIThread.onBind(android.content.Intent):android.os.IBinder, 19, STARVATION, no_bucket, ERROR, [`IBinder ServiceOnUIThread.onBind(Intent)`,Method call: `void ServiceOnUIThread.transactBad()`,calls `boolean IBinder.transact(int,Parcel,Parcel,int)` from `void ServiceOnUIThread.transactBad()`,[Trace on UI thread] `IBinder ServiceOnUIThread.onBind(Intent)`,`IBinder ServiceOnUIThread.onBind(Intent)` is a standard UI-thread method] codetoanalyze/java/starvation/StaticLock.java, StaticLock.lockOtherClassOneWayBad():void, 23, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void StaticLock.lockOtherClassOneWayBad()`,locks `StaticLock$0` in class `java.lang.Class*`,locks `this` in class `StaticLock*`,[Trace 2] `void StaticLock.lockOtherClassAnotherWayNad()`,locks `this` in class `StaticLock*`,Method call: `void StaticLock.staticSynced()`,locks `StaticLock$0` in class `java.lang.Class*`] codetoanalyze/java/starvation/SuppLint.java, SuppLint.onUiThreadBad():void, 26, STARVATION, no_bucket, ERROR, [`void SuppLint.onUiThreadBad()`,calls `Object Future.get()` from `void SuppLint.onUiThreadBad()`,[Trace on UI thread] `void SuppLint.onUiThreadBad()`,`void SuppLint.onUiThreadBad()` is annotated `UiThread`] 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()`,[Trace 1 on UI thread] `void ThreadSleep.indirectSleepOnUIThreadBad()`,`void ThreadSleep.indirectSleepOnUIThreadBad()` is annotated `UiThread`]