[starvation] Adjust blocking call severities to better reflect practice

Summary:
Currently, some blocking calls are turning up too many false positives.  Adjust severities by fix rate/preexisting numbers.

Also, restrict some calls to exact class, as opposed to superclasses.

Reviewed By: mbouaziz

Differential Revision: D8850599

fbshipit-source-id: 47543d04a
master
Nikos Gorogiannis 6 years ago committed by Facebook Github Bot
parent ae9ee41f78
commit c5ad94d825

@ -591,23 +591,28 @@ module Models = struct
&& not (Annotations.pdesc_return_annot_ends_with proc_desc Annotations.visibleForTesting) && not (Annotations.pdesc_return_annot_ends_with proc_desc Annotations.visibleForTesting)
let is_call_of_class_or_superclass ?(method_prefix= false) ?(actuals_pred= fun _ -> true) let is_call_of_class ?(search_superclasses= true) ?(method_prefix= false)
class_names method_name tenv pn actuals = ?(actuals_pred= fun _ -> true) class_names method_name =
let is_target_class =
let target_set = List.map class_names ~f:Typ.Name.Java.from_string |> Typ.Name.Set.of_list in
fun tname -> Typ.Name.Set.mem tname target_set
in
let is_target_struct tname _ = is_target_class tname in
Staged.stage (fun tenv pn actuals ->
actuals_pred actuals actuals_pred actuals
&& &&
match pn with match pn with
| Typ.Procname.Java java_pname -> | Typ.Procname.Java java_pname ->
let classname = Typ.Procname.Java.get_class_type_name java_pname in let classname = Typ.Procname.Java.get_class_type_name java_pname in
let mthd = Typ.Procname.Java.get_method java_pname in let mthd = Typ.Procname.Java.get_method java_pname in
let is_target_class =
let targets = List.map class_names ~f:Typ.Name.Java.from_string in
fun tname _ -> List.mem targets tname ~equal:Typ.Name.equal
in
( if method_prefix then String.is_prefix mthd ~prefix:method_name ( if method_prefix then String.is_prefix mthd ~prefix:method_name
else String.equal mthd method_name ) else String.equal mthd method_name )
&& PatternMatch.supertype_exists tenv is_target_class classname &&
if search_superclasses then
PatternMatch.supertype_exists tenv is_target_struct classname
else is_target_class classname
| _ -> | _ ->
false false )
(** magical value from https://developer.android.com/topic/performance/vitals/anr *) (** magical value from https://developer.android.com/topic/performance/vitals/anr *)
@ -649,7 +654,7 @@ module Models = struct
many reports *) many reports *)
(* let is_blocking_java_io = (* let is_blocking_java_io =
is_call_of_class_or_superclass ["java.io.Reader"; "java.io.InputStream"] ~method_prefix:true is_call_of_class ["java.io.Reader"; "java.io.InputStream"] ~method_prefix:true
"read" *) "read" *)
let actuals_are_empty_or_timeout = function let actuals_are_empty_or_timeout = function
@ -663,8 +668,9 @@ module Models = struct
(** is the method called CountDownLath.await or on subclass? *) (** is the method called CountDownLath.await or on subclass? *)
let is_countdownlatch_await = let is_countdownlatch_await =
is_call_of_class_or_superclass ~actuals_pred:actuals_are_empty_or_timeout is_call_of_class ~actuals_pred:actuals_are_empty_or_timeout
["java.util.concurrent.CountDownLatch"] "await" ["java.util.concurrent.CountDownLatch"] "await"
|> Staged.unstage
(** an IBinder.transact call is an RPC. If the 4th argument (5th counting `this` as the first) (** an IBinder.transact call is an RPC. If the 4th argument (5th counting `this` as the first)
@ -675,39 +681,43 @@ module Models = struct
let actuals_pred actuals = let actuals_pred actuals =
List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero
in in
is_call_of_class_or_superclass ~actuals_pred ["android.os.IBinder"] "transact" is_call_of_class ~actuals_pred ["android.os.IBinder"] "transact" |> Staged.unstage
(** is it a call to android.view.View.getWindowVisibleDisplayFrame or on sublass? *) (** is it a call to android.view.View.getWindowVisibleDisplayFrame? *)
let is_getWindowVisibleDisplayFrame = let is_getWindowVisibleDisplayFrame =
is_call_of_class_or_superclass ["android.view.View"] "getWindowVisibleDisplayFrame" is_call_of_class ~search_superclasses:false ["android.view.View"]
"getWindowVisibleDisplayFrame"
|> Staged.unstage
(** is it a call to Future.get() or on sublass? *) (** is it a call to Future.get()? *)
let is_future_get = let is_future_get =
is_call_of_class_or_superclass ~actuals_pred:actuals_are_empty_or_timeout is_call_of_class ~search_superclasses:false ~actuals_pred:actuals_are_empty_or_timeout
["java.util.concurrent.Future"] "get" ["java.util.concurrent.Future"] "get"
|> Staged.unstage
let is_accountManager_setUserData = let is_accountManager_setUserData =
is_call_of_class_or_superclass ["android.accounts.AccountManager"] "setUserData" is_call_of_class ~search_superclasses:false ["android.accounts.AccountManager"] "setUserData"
|> Staged.unstage
let is_asyncTask_get = let is_asyncTask_get =
is_call_of_class_or_superclass ~actuals_pred:actuals_are_empty_or_timeout is_call_of_class ~actuals_pred:actuals_are_empty_or_timeout ["android.os.AsyncTask"] "get"
["android.os.AsyncTask"] "get" |> Staged.unstage
(* at most one function is allowed to be true *) (* at most one function is allowed to be true, sort from High to Low *)
let may_block = let may_block =
let open StarvationDomain.Event in let open StarvationDomain.Event in
let matchers = let matchers =
[ (is_countdownlatch_await, Medium) [ (is_accountManager_setUserData, High)
; (is_two_way_binder_transact, High) ; (is_two_way_binder_transact, High)
; (is_getWindowVisibleDisplayFrame, Low) ; (is_countdownlatch_await, High)
; (is_future_get, High) ; (is_getWindowVisibleDisplayFrame, Medium)
; (is_accountManager_setUserData, High) ; (is_asyncTask_get, Low)
; (is_asyncTask_get, High) ] ; (is_future_get, Low) ]
in in
fun tenv pn actuals -> fun tenv pn actuals ->
List.find_map matchers ~f:(fun (matcher, sev) -> Option.some_if (matcher tenv pn actuals) sev) List.find_map matchers ~f:(fun (matcher, sev) -> Option.some_if (matcher tenv pn actuals) sev)

@ -10,7 +10,7 @@ codetoanalyze/java/starvation/Countdwn.java, void Countdwn.awaitOnMainByCallBad(
codetoanalyze/java/starvation/Dedup.java, void Dedup.callMethodWithMultipleBlocksBad(), 27, STARVATION, no_bucket, ERROR, [`void Dedup.callMethodWithMultipleBlocksBad()`,calls `Object Future.get()` from `void Dedup.callMethodWithMultipleBlocksBad()`] codetoanalyze/java/starvation/Dedup.java, void Dedup.callMethodWithMultipleBlocksBad(), 27, STARVATION, no_bucket, ERROR, [`void Dedup.callMethodWithMultipleBlocksBad()`,calls `Object Future.get()` from `void Dedup.callMethodWithMultipleBlocksBad()`]
codetoanalyze/java/starvation/Dedup.java, void Dedup.callMethodWithMultipleBlocksBad(), 28, STARVATION, no_bucket, ERROR, [`void Dedup.callMethodWithMultipleBlocksBad()`,calls `void CountDownLatch.await()` from `void Dedup.callMethodWithMultipleBlocksBad()`] codetoanalyze/java/starvation/Dedup.java, void Dedup.callMethodWithMultipleBlocksBad(), 28, STARVATION, no_bucket, ERROR, [`void Dedup.callMethodWithMultipleBlocksBad()`,calls `void CountDownLatch.await()` from `void Dedup.callMethodWithMultipleBlocksBad()`]
codetoanalyze/java/starvation/Dedup.java, void Dedup.callMethodWithMultipleBlocksBad(), 29, STARVATION, no_bucket, ERROR, [`void Dedup.callMethodWithMultipleBlocksBad()`,calls `Object Future.get()` from `void Dedup.callMethodWithMultipleBlocksBad()`] codetoanalyze/java/starvation/Dedup.java, void Dedup.callMethodWithMultipleBlocksBad(), 29, STARVATION, no_bucket, ERROR, [`void Dedup.callMethodWithMultipleBlocksBad()`,calls `Object Future.get()` from `void Dedup.callMethodWithMultipleBlocksBad()`]
codetoanalyze/java/starvation/Dedup.java, void Dedup.onUiThreadBad(), 21, STARVATION, no_bucket, ERROR, [`void Dedup.onUiThreadBad()`,Method call: `void Dedup.callMethodWithMultipleBlocksBad()`,calls `Object Future.get()` from `void Dedup.callMethodWithMultipleBlocksBad()`] codetoanalyze/java/starvation/Dedup.java, void Dedup.onUiThreadBad(), 21, STARVATION, no_bucket, ERROR, [`void Dedup.onUiThreadBad()`,Method call: `void Dedup.callMethodWithMultipleBlocksBad()`,calls `void CountDownLatch.await()` from `void Dedup.callMethodWithMultipleBlocksBad()`]
codetoanalyze/java/starvation/Dedup.java, void Dedup.oneWayBad(), 36, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Dedup.oneWayBad()`,locks `this.Dedup.lockA` in class `Dedup*`,locks `this.Dedup.lockB` in class `Dedup*`,[Trace 2] `void Dedup.anotherWayBad()`,locks `this.Dedup.lockB` in class `Dedup*`,locks `this.Dedup.lockA` in class `Dedup*`] codetoanalyze/java/starvation/Dedup.java, void Dedup.oneWayBad(), 36, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Dedup.oneWayBad()`,locks `this.Dedup.lockA` in class `Dedup*`,locks `this.Dedup.lockB` in class `Dedup*`,[Trace 2] `void Dedup.anotherWayBad()`,locks `this.Dedup.lockB` in class `Dedup*`,locks `this.Dedup.lockA` in class `Dedup*`]
codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getDirectBad(), 20, STARVATION, no_bucket, ERROR, [`void FutureGet.getDirectBad()`,calls `Object Future.get()` from `void FutureGet.getDirectBad()`] codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getDirectBad(), 20, STARVATION, no_bucket, ERROR, [`void FutureGet.getDirectBad()`,calls `Object Future.get()` from `void FutureGet.getDirectBad()`]
codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getIndirectBad(), 25, STARVATION, no_bucket, ERROR, [[Trace 1] `void FutureGet.getIndirectBad()`,locks `this.FutureGet.lock` in class `FutureGet*`,[Trace 2] `void FutureGet.getUnderLock()`,locks `this.FutureGet.lock` in class `FutureGet*`,calls `Object Future.get()` from `void FutureGet.getUnderLock()`] codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getIndirectBad(), 25, STARVATION, no_bucket, ERROR, [[Trace 1] `void FutureGet.getIndirectBad()`,locks `this.FutureGet.lock` in class `FutureGet*`,[Trace 2] `void FutureGet.getUnderLock()`,locks `this.FutureGet.lock` in class `FutureGet*`,calls `Object Future.get()` from `void FutureGet.getUnderLock()`]

Loading…
Cancel
Save