From 7a538c5004f79beba9c09161a558ed911313a4e7 Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Fri, 29 Nov 2019 06:04:08 -0800 Subject: [PATCH] [starvation][whole-program] thread class Summary: One standard way to schedule work is by starting a thread. We treat this by - Treating invocations of `start` on a receiver with the `Runnable _` attribute as scheduling that runnable for parallel execution in the background (as opposed to on the UI thread). - If `start` is used on an object of a subclass of `Thread` everything already works thanks to the `get_exp_attributes` function which will implicitly ascribe to an expression the attribute `Runnable _` if the expression points to an object with a known `run` method. This will even take care of some degree of dynamic dispatch, yay! - If `start` is used on a `Thread` object which has been created with a constructor call provided with a `Runnable` argument, we have to appropriately model that constructor call, which is what is done in `do_call`. Reviewed By: jvillard Differential Revision: D18726676 fbshipit-source-id: 0bd83c28e --- infer/src/concurrency/StarvationModels.ml | 12 ++- infer/src/concurrency/StarvationModels.mli | 2 + infer/src/concurrency/starvation.ml | 85 +++++++++++----- infer/src/istd/IList.ml | 3 + infer/src/istd/IList.mli | 3 + .../ThreadScheduling.java | 98 +++++++++++++++++++ .../java/starvation-whole-program/issues.exp | 4 + 7 files changed, 179 insertions(+), 28 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/starvation-whole-program/ThreadScheduling.java diff --git a/infer/src/concurrency/StarvationModels.ml b/infer/src/concurrency/StarvationModels.ml index 75c9f4c62..1d11126d6 100644 --- a/infer/src/concurrency/StarvationModels.ml +++ b/infer/src/concurrency/StarvationModels.ml @@ -236,7 +236,8 @@ let schedules_work_on_ui_thread = let schedules_work_on_bg_thread = let open MethodMatcher in let matcher = - [{default with classname= "java.lang.Object"; methods= ["scheduleGuaranteedDelayed"]}] + [ {default with classname= "java.lang.Object"; methods= ["scheduleGuaranteedDelayed"]} + ; {default with classname= "java.lang.Thread"; methods= ["start"]} ] |> of_records in fun tenv pname -> matcher tenv pname [] @@ -328,3 +329,12 @@ let is_handler_constructor = ; methods= [Typ.Procname.Java.constructor_method_name] ; actuals_pred= (fun actuals -> not (List.is_empty actuals)) } ] |> of_records + + +let is_thread_constructor = + let open MethodMatcher in + [ { default with + classname= "java.lang.Thread" + ; search_superclasses= false + ; methods= [Typ.Procname.Java.constructor_method_name] } ] + |> of_records diff --git a/infer/src/concurrency/StarvationModels.mli b/infer/src/concurrency/StarvationModels.mli index 2e8c9cc3a..3aa47286d 100644 --- a/infer/src/concurrency/StarvationModels.mli +++ b/infer/src/concurrency/StarvationModels.mli @@ -64,3 +64,5 @@ val schedules_work_on_bg_thread : Tenv.t -> Typ.Procname.t -> bool val is_getMainLooper : Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool val is_handler_constructor : Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool + +val is_thread_constructor : Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool diff --git a/infer/src/concurrency/starvation.ml b/infer/src/concurrency/starvation.ml index a77a2acc2..3d6705072 100644 --- a/infer/src/concurrency/starvation.ml +++ b/infer/src/concurrency/starvation.ml @@ -129,49 +129,80 @@ module TransferFunctions (CFG : ProcCfg.S) = struct let make_thread thread = {empty_summary with thread} in let get_returned_executor_summary () = StarvationModels.get_returned_executor ~attrs_of_pname tenv callee actuals - |> Option.map ~f:(fun thread_constraint -> - make_ret_attr (Attribute.WorkScheduler thread_constraint) ) + |> Option.map ~f:(fun thread_constraint -> make_ret_attr (WorkScheduler thread_constraint)) in let get_thread_assert_summary () = match ConcurrencyModels.get_thread_assert_effect callee with | BackgroundThread -> - Some (make_thread ThreadDomain.BGThread) + Some (make_thread BGThread) | MainThread -> - Some (make_thread ThreadDomain.UIThread) + Some (make_thread UIThread) | MainThreadIfTrue -> - Some (make_ret_attr Attribute.ThreadGuard) + Some (make_ret_attr ThreadGuard) | UnknownThread -> None in let get_mainLooper_summary () = if StarvationModels.is_getMainLooper tenv callee actuals then - Some (make_ret_attr (Attribute.Looper StarvationModels.ForUIThread)) + Some (make_ret_attr (Looper ForUIThread)) else None in let get_callee_summary () = Payload.read ~caller_summary:summary ~callee_pname:callee in - let callsite = CallSite.make callee loc in + let treat_handler_constructor () = + if StarvationModels.is_handler_constructor tenv callee actuals then + match actuals with + | HilExp.AccessExpression receiver :: HilExp.AccessExpression exp :: _ -> + let constr = + AttributeDomain.find_opt exp astate.attributes + |> Option.bind ~f:(function Attribute.Looper c -> Some c | _ -> None) + |> Option.value ~default:StarvationModels.ForUnknownThread + in + let attributes = + AttributeDomain.add receiver (WorkScheduler constr) astate.attributes + in + Some {astate with attributes} + | _ -> + None + else None + in + let treat_thread_constructor () = + if StarvationModels.is_thread_constructor tenv callee actuals then + match actuals with + | HilExp.AccessExpression receiver :: rest -> + ( match rest with + | HilExp.AccessExpression exp1 :: HilExp.AccessExpression exp2 :: _ -> + (* two additional arguments, either could be a runnable, see docs *) + [exp1; exp2] + | HilExp.AccessExpression runnable :: _ -> + (* either just one argument, or more but 2nd is not an access expression *) + [runnable] + | _ -> + [] ) + |> List.map ~f:(fun r () -> StarvationModels.get_run_method_from_runnable tenv r) + |> IList.eval_until_first_some + |> Option.map ~f:(fun procname -> + let attributes = + AttributeDomain.add receiver (Runnable procname) astate.attributes + in + {astate with attributes} ) + | _ -> + None + else None + in (* constructor calls are special-cased because they side-effect the receiver and do not return anything *) - if StarvationModels.is_handler_constructor tenv callee actuals then - match actuals with - | HilExp.AccessExpression receiver :: HilExp.AccessExpression exp :: _ -> - let thread_constraint_attr = - AttributeDomain.find_opt exp astate.attributes - |> Option.bind ~f:(function Attribute.Looper c -> Some c | _ -> None) - |> Option.value ~default:StarvationModels.ForUnknownThread - |> fun constr -> Attribute.WorkScheduler constr - in - let attributes = AttributeDomain.add receiver thread_constraint_attr astate.attributes in - {astate with attributes} - | _ -> - astate - else - get_returned_executor_summary () - |> IOption.if_none_evalopt ~f:get_thread_assert_summary - |> IOption.if_none_evalopt ~f:get_mainLooper_summary - (* [get_callee_summary] should come after all models *) - |> IOption.if_none_evalopt ~f:get_callee_summary - |> Option.fold ~init:astate ~f:(Domain.integrate_summary ~tenv ~lhs callsite) + let treat_modeled_summaries () = + let callsite = CallSite.make callee loc in + IList.eval_until_first_some + [ get_returned_executor_summary + ; get_thread_assert_summary + ; get_mainLooper_summary + ; get_callee_summary ] + |> Option.map ~f:(Domain.integrate_summary ~tenv ~lhs callsite astate) + in + IList.eval_until_first_some + [treat_handler_constructor; treat_thread_constructor; treat_modeled_summaries] + |> Option.value ~default:astate let exec_instr (astate : Domain.t) ({ProcData.summary; tenv; extras} as procdata) _ diff --git a/infer/src/istd/IList.ml b/infer/src/istd/IList.ml index d20780e14..e588a352c 100644 --- a/infer/src/istd/IList.ml +++ b/infer/src/istd/IList.ml @@ -201,3 +201,6 @@ let pp_print_list ~max ?(pp_sep = Format.pp_print_cut) pp_v ppf = let fold2_result ~init ~f l1 l2 = List.fold2 l1 l2 ~init:(Ok init) ~f:(fun result x1 x2 -> Result.bind result ~f:(fun acc -> f acc x1 x2) ) + + +let eval_until_first_some thunks = List.find_map thunks ~f:(fun f -> f ()) diff --git a/infer/src/istd/IList.mli b/infer/src/istd/IList.mli index 0b7bae698..e59df7496 100644 --- a/infer/src/istd/IList.mli +++ b/infer/src/istd/IList.mli @@ -50,6 +50,9 @@ val force_until_first_some : 'a option lazy_t list -> 'a option (** [force_until_first_some xs] forces the computation of each element of [xs] and returns the first that matches (Some _); or, if no such element exists, it returns None. *) +val eval_until_first_some : (unit -> 'a option) list -> 'a option +(** given a list of functions taking unit, evaluate and return the first one to return [Some x] *) + val pp_print_list : max:int -> ?pp_sep:(Format.formatter -> unit -> unit) diff --git a/infer/tests/codetoanalyze/java/starvation-whole-program/ThreadScheduling.java b/infer/tests/codetoanalyze/java/starvation-whole-program/ThreadScheduling.java new file mode 100644 index 000000000..041a908f6 --- /dev/null +++ b/infer/tests/codetoanalyze/java/starvation-whole-program/ThreadScheduling.java @@ -0,0 +1,98 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * 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; + +class ThreadScheduling { + static Binder binder; + + private static void doTransact() { + try { + binder.transact(0, null, null, 0); + } catch (RemoteException e) { + } + } + + Object monitorA; + + public void scheduleBlockingCallOnContendedLockBad() { + Thread t = + new Thread( + new Runnable() { + @Override + public void run() { + synchronized (monitorA) { + doTransact(); + } + } + }); + t.start(); + + Executors.runOnUiThread( + new Runnable() { + @Override + public void run() { + synchronized (monitorA) { + } + } + }); + } + + Object monitorB, monitorC; + + public void scheduleDeadlockBad() { + Thread t = + new Thread( + new Runnable() { + @Override + public void run() { + synchronized (monitorB) { + synchronized (monitorC) { + } + } + } + }); + t.start(); + + Executors.runOnUiThread( + new Runnable() { + @Override + public void run() { + synchronized (monitorC) { + synchronized (monitorB) { + } + } + } + }); + } + + Object monitorD; + + class BadThread extends Thread { + @Override + public void run() { + synchronized (monitorD) { + doTransact(); + } + } + } + + public void scheduleBlockingCallOnContendedLockViaInheritanceBad() { + Thread t = new BadThread(); + t.start(); + + Executors.runOnUiThread( + new Runnable() { + @Override + public void run() { + synchronized (monitorD) { + } + } + }); + } +} diff --git a/infer/tests/codetoanalyze/java/starvation-whole-program/issues.exp b/infer/tests/codetoanalyze/java/starvation-whole-program/issues.exp index f5ccaf2e6..9e68d0a4a 100644 --- a/infer/tests/codetoanalyze/java/starvation-whole-program/issues.exp +++ b/infer/tests/codetoanalyze/java/starvation-whole-program/issues.exp @@ -34,6 +34,10 @@ codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceC codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onNullBinding(android.content.ComponentName):void, 31, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onNullBinding(ComponentName)`,Method call: `void MyServiceConnection.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`] codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onServiceConnected(android.content.ComponentName,android.os.IBinder):void, 36, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onServiceConnected(ComponentName,IBinder)`,Method call: `void MyServiceConnection.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`] codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onServiceDisconnected(android.content.ComponentName):void, 41, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onServiceDisconnected(ComponentName)`,Method call: `void MyServiceConnection.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`] +codetoanalyze/java/starvation-whole-program/ThreadScheduling.java, ThreadScheduling.scheduleBlockingCallOnContendedLockBad():void, 36, STARVATION, no_bucket, ERROR, [[Trace 1] `void ThreadScheduling.scheduleBlockingCallOnContendedLockBad()`,Method call: `void ThreadScheduling$2.run()`, locks `this.monitorA` in `class ThreadScheduling`,[Trace 2] `void ThreadScheduling.scheduleBlockingCallOnContendedLockBad()`,Method call: `void ThreadScheduling$1.run()`, locks `this.monitorA` in `class ThreadScheduling`,Method call: `void ThreadScheduling.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`] +codetoanalyze/java/starvation-whole-program/ThreadScheduling.java, ThreadScheduling.scheduleBlockingCallOnContendedLockViaInheritanceBad():void, 89, STARVATION, no_bucket, ERROR, [[Trace 1] `void ThreadScheduling.scheduleBlockingCallOnContendedLockViaInheritanceBad()`,Method call: `void ThreadScheduling$5.run()`, locks `this.monitorD` in `class ThreadScheduling`,[Trace 2] `void ThreadScheduling.scheduleBlockingCallOnContendedLockViaInheritanceBad()`,Method call: `void ThreadScheduling$BadThread.run()`, locks `this.monitorD` in `class ThreadScheduling`,Method call: `void ThreadScheduling.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`] +codetoanalyze/java/starvation-whole-program/ThreadScheduling.java, ThreadScheduling.scheduleDeadlockBad():void, 60, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void ThreadScheduling.scheduleDeadlockBad()`,Method call: `void ThreadScheduling$3.run()`, locks `this.monitorB` in `class ThreadScheduling`, locks `this.monitorC` in `class ThreadScheduling`,[Trace 2] `void ThreadScheduling.scheduleDeadlockBad()`,Method call: `void ThreadScheduling$4.run()`, locks `this.monitorC` in `class ThreadScheduling`, locks `this.monitorB` in `class ThreadScheduling`] +codetoanalyze/java/starvation-whole-program/ThreadScheduling.java, ThreadScheduling.scheduleDeadlockBad():void, 62, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void ThreadScheduling.scheduleDeadlockBad()`,Method call: `void ThreadScheduling$4.run()`, locks `this.monitorC` in `class ThreadScheduling`, locks `this.monitorB` in `class ThreadScheduling`,[Trace 2] `void ThreadScheduling.scheduleDeadlockBad()`,Method call: `void ThreadScheduling$3.run()`, locks `this.monitorB` in `class ThreadScheduling`, locks `this.monitorC` in `class ThreadScheduling`] codetoanalyze/java/starvation-whole-program/UnknownThread.java, UnknownThread.postDeadlockToUIAndBackgroundBad():void, 84, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UnknownThread.postDeadlockToUIAndBackgroundBad()`,Method call: `void UnknownThread$5.run()`, locks `this.monitorC` in `class UnknownThread`, locks `this.monitorD` in `class UnknownThread`,[Trace 2] `void UnknownThread.postDeadlockToUIAndBackgroundBad()`,Method call: `void UnknownThread$6.run()`, locks `this.monitorD` in `class UnknownThread`, locks `this.monitorC` in `class UnknownThread`] codetoanalyze/java/starvation-whole-program/UnknownThread.java, UnknownThread.postDeadlockToUIAndBackgroundBad():void, 95, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UnknownThread.postDeadlockToUIAndBackgroundBad()`,Method call: `void UnknownThread$6.run()`, locks `this.monitorD` in `class UnknownThread`, locks `this.monitorC` in `class UnknownThread`,[Trace 2] `void UnknownThread.postDeadlockToUIAndBackgroundBad()`,Method call: `void UnknownThread$5.run()`, locks `this.monitorC` in `class UnknownThread`, locks `this.monitorD` in `class UnknownThread`] codetoanalyze/java/starvation-whole-program/UnknownThread.java, UnknownThread.postDeadlockToUnknownAndBackgroundBad():void, 57, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UnknownThread.postDeadlockToUnknownAndBackgroundBad()`,Method call: `void UnknownThread$3.run()`, locks `this.monitorA` in `class UnknownThread`, locks `this.monitorB` in `class UnknownThread`,[Trace 2] `void UnknownThread.postDeadlockToUnknownAndBackgroundBad()`,Method call: `void UnknownThread$4.run()`, locks `this.monitorB` in `class UnknownThread`, locks `this.monitorA` in `class UnknownThread`]