[starvation] allow specifying method patterns for skipping analysis via cmdline and inferconfig

Reviewed By: jberdine

Differential Revision: D9862213

fbshipit-source-id: a20b8855f

infer/src/base/Config.ml

@@ -2023,6 +2023,11 @@ and sources = CLOpt.mk_string_list ~long:"sources" "Specify the list of source f
 
 and sourcepath = CLOpt.mk_string_opt ~long:"sourcepath" "Specify the sourcepath"
 
+and starvation_skip_analysis =
+  CLOpt.mk_json ~long:"starvation-skip-analysis"
+    "Specify combinations of class/method list that should be skipped during starvation analysis"
+
+
 and spec_abs_level =
   CLOpt.mk_int ~deprecated:["spec_abs_level"] ~long:"spec-abs-level" ~default:1 ~meta:"int"
     {|Set the level of abstracting the postconditions of discovered specs:
@@ -2963,6 +2968,8 @@ and stacktraces_dir = !stacktraces_dir
 
 and starvation = !starvation
 
+and starvation_skip_analysis = !starvation_skip_analysis
+
 and starvation_strict_mode = !starvation_strict_mode
 
 and stats_report = !stats_report

infer/src/base/Config.mli

@@ -625,6 +625,8 @@ val stacktraces_dir : string option
 
 val starvation : bool
 
+val starvation_skip_analysis : Yojson.Basic.json
+
 val starvation_strict_mode : bool
 
 val stats_report : string option

infer/src/concurrency/ConcurrencyModels.ml

@@ -7,6 +7,7 @@
 
 open! IStd
 module F = Format
+module L = Logging
 module MF = MarkupFormatter
 
 type lock = Lock | Unlock | LockedIfTrue | NoEffect
@@ -190,16 +191,60 @@ let is_call_of_class ?(search_superclasses = true) ?(method_prefix = 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"]
+type matcher = Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool
+
+type java_matcher_record =
+  { search_superclasses: bool option
+  ; method_prefix: bool option
+  ; actuals_pred: (HilExp.t list -> bool) option
+  ; classname: string
+  ; methods: string list }
+
+let make_matcher_from_record {search_superclasses; method_prefix; actuals_pred; classname; methods}
+    =
+  is_call_of_class ?search_superclasses ?method_prefix ?actuals_pred classname methods
   |> Staged.unstage
+
+
+let empty_matcher =
+  {search_superclasses= None; method_prefix= None; actuals_pred= None; classname= ""; methods= []}
+
+
+let matcher_of_json top_json =
+  let error json =
+    L.(die UserError "Could not parse json matcher(s): %s" (Yojson.Basic.to_string json))
   in
-  let is_fragment_ui_method =
-    (* lifecycle methods of Fragment *)
+  let make_matcher_from_json json =
+    let parse_method_name = function `String methodname -> methodname | _ -> error json in
+    let rec parse_fields assoclist acc =
+      match assoclist with
+      | ("search_superclasses", `Bool b) :: rest ->
+          {acc with search_superclasses= Some b} |> parse_fields rest
+      | ("method_prefix", `Bool b) :: rest ->
+          {acc with method_prefix= Some b} |> parse_fields rest
+      | ("classname", `String classname) :: rest ->
+          {acc with classname} |> parse_fields rest
+      | ("methods", `List methodnames) :: rest ->
+          let methods = List.map methodnames ~f:parse_method_name in
+          {acc with methods} |> parse_fields rest
+      | [] ->
+          if String.equal acc.classname "" || List.is_empty acc.methods then error json else acc
+      | _ ->
+          error json
+    in
+    (match json with `Assoc fields -> parse_fields fields empty_matcher | _ -> error json)
+    |> make_matcher_from_record
+  in
+  match top_json with
+  | `List matchers_json ->
+      let matchers = List.map matchers_json ~f:make_matcher_from_json in
+      fun tenv pn actuals -> List.exists matchers ~f:(fun m -> m tenv pn actuals)
+  | _ ->
+      error top_json
+
+
+let ui_matcher_records =
+  let superclasses = {empty_matcher with search_superclasses= Some true} in
   let fragment_methods =
     (* sort police: this is in lifecycle order *)
     [ "onAttach"
@@ -214,46 +259,28 @@ let is_ui_method =
     ; "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 =
+  [ {superclasses with classname= "android.support.v4.app.Fragment"; methods= fragment_methods}
+  ; {superclasses with classname= "android.app.Fragment"; methods= fragment_methods}
+  ; {superclasses with classname= "android.content.ContentProvider"; methods= ["onCreate"]}
+  ; {superclasses with classname= "android.content.BroadcastReceiver"; methods= ["onReceive"]}
+  ; { superclasses with
+      classname= "android.app.Service"
+    ; methods= ["onBind"; "onCreate"; "onDestroy"; "onStartCommand"] }
+  ; { superclasses with
+      classname= "android.app.Activity"
+    ; methods= ["onCreate"; "onStart"; "onRestart"; "onResume"; "onPause"; "onStop"; "onDestroy"]
+    }
+  ; { superclasses with
       (* 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
+      method_prefix= Some true
+    ; classname= "android.view.View"
+    ; methods= ["on"] } ]
+
+
+let is_ui_method =
+  let matchers = List.map ui_matcher_records ~f:make_matcher_from_record 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 [])
 

infer/src/concurrency/ConcurrencyModels.mli

@@ -35,10 +35,28 @@ val get_current_class_and_annotated_superclasses :
 val find_annotated_or_overriden_annotated_method :
   (Annot.Item.t -> bool) -> BuiltinDecl.t -> Tenv.t -> BuiltinDecl.t sexp_option
 
+(** pattern matcher for Java methods *)
+type matcher = Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool
+
 val is_call_of_class :
      ?search_superclasses:bool
   -> ?method_prefix:bool
   -> ?actuals_pred:(HilExp.t list -> bool)
   -> string
   -> string list
-  -> (Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool) Staged.t
+  -> matcher Staged.t
+(** [is_call_of_class C methods] builds a method matcher for calls [C.foo] where
+    [foo] is in [methods].  Optional arguments change default behaviour:
+    - [search_superclasses=true] will match calls [S.foo] where [S] is a superclass of [C].
+      Defaults to [false].
+    - [method_prefix=true] will match calls [C.foo] where [foo] is a prefix of a string in [methods]
+      Defaults to [false].
+    - [actuals_pred] is a predicate that runs on the expressions fed as arguments to the call, and
+      which must return [true] for the matcher to return [true]. The default returns [true]. *)
+
+val matcher_of_json : Yojson.Basic.json -> matcher
+(** Parse a JSon object into a matcher.  The Json object must be a list of records, each
+    corresponding to a single matcher.  Each record must have a ["classname"] field with a [string]
+    value, and a ["methods"] field with a list of strings.  The record may also have boolean
+    fields ["search_superclasses"] and ["method_prefix"].  If absent, the defaults are used.
+    The resulting matcher matches if one of the matchers in the list does. *)

infer/src/concurrency/StarvationModels.ml

@@ -21,14 +21,7 @@ let is_synchronized_library_call =
         false
 
 
-let is_futures_getdone =
-  is_call_of_class "com.google.common.util.concurrent.Futures" ["getDone"] |> Staged.unstage
-
-
-let should_skip_analysis =
-  let matchers = [is_futures_getdone] in
-  fun tenv pn actuals -> List.exists matchers ~f:(fun matcher -> matcher tenv pn actuals)
-
+let should_skip_analysis = ConcurrencyModels.matcher_of_json Config.starvation_skip_analysis
 
 (** magical value from https://developer.android.com/topic/performance/vitals/anr *)
 let android_anr_time_limit = 5.0
@@ -97,61 +90,15 @@ let empty_or_excessive_timeout actuals =
       false
 
 
-(** is the method called Object.wait or on subclass, without timeout or with excessive timeout ? *)
-let is_object_wait =
-  is_call_of_class ~actuals_pred:empty_or_excessive_timeout "java.lang.Object" ["wait"]
-  |> Staged.unstage
-
-
-(** is the method called CountDownLath.await or on subclass? *)
-let is_countdownlatch_await =
-  is_call_of_class ~actuals_pred:empty_or_excessive_timeout "java.util.concurrent.CountDownLatch"
-    ["await"]
-  |> Staged.unstage
-
-
-(** an IBinder.transact call is an RPC.  If the 4th argument (5th counting `this` as the first)
-           is int-zero then a reply is expected and returned from the remote process, thus potentially
-           blocking.  If the 4th argument is anything else, we assume a one-way call which doesn't block.
-        *)
-let is_two_way_binder_transact =
-  let actuals_pred actuals =
-    List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero
+(* matchers used for normal analysis as well as in --dev-android-strict-mode *)
+(* selection is a bit arbitrary as some would be generated anyway if not here; no harm though *)
+(* some, like [file], need manual addition due to our lack of handling dynamic dispatch *)
+let strict_mode_common_matchers =
+  let is_system_gc =
+    is_call_of_class "java.lang.System" ["gc"; "runFinalization"] |> Staged.unstage
   in
-  is_call_of_class ~actuals_pred "android.os.IBinder" ["transact"] |> Staged.unstage
-
-
-(** is it a call to Future.get()? *)
-let is_future_get =
-  is_call_of_class ~search_superclasses:false ~actuals_pred:empty_or_excessive_timeout
-    "java.util.concurrent.Future" ["get"]
-  |> Staged.unstage
-
-
-let is_accountManager_setUserData =
-  is_call_of_class ~search_superclasses:false "android.accounts.AccountManager" ["setUserData"]
-  |> Staged.unstage
-
-
-let is_asyncTask_get =
-  is_call_of_class ~actuals_pred:empty_or_excessive_timeout "android.os.AsyncTask" ["get"]
-  |> Staged.unstage
-
-
-(* consider any call to sleep as bad, even with timeouts lower than the anr limit *)
-let is_thread_sleep = is_call_of_class "java.lang.Thread" ["sleep"] |> Staged.unstage
-
-(* matcher for strict mode throws in Android libcore implementation,
-   used with --dev-android strict mode *)
-let is_blockguard_on =
-  is_call_of_class ~method_prefix:true "dalvik.system.BlockGuard$Policy" ["on"] |> Staged.unstage
-
-
-let is_system_gc = is_call_of_class "java.lang.System" ["gc"; "runFinalization"] |> Staged.unstage
-
-let is_runtime_gc = is_call_of_class "java.lang.Runtime" ["gc"] |> Staged.unstage
-
-let is_file_io =
+  let is_runtime_gc = is_call_of_class "java.lang.Runtime" ["gc"] |> Staged.unstage in
+  let is_file_io =
     is_call_of_class "java.io.File"
       [ "canRead"
       ; "canWrite"
@@ -177,25 +124,17 @@ let is_file_io =
       ; "setReadOnly"
       ; "setWritable" ]
     |> Staged.unstage
-
-
-let is_socket_connect = is_call_of_class "java.net.Socket" ["connect"] |> Staged.unstage
-
-let is_connected_socket_constructor =
+  in
+  let is_socket_connect = is_call_of_class "java.net.Socket" ["connect"] |> Staged.unstage in
+  let is_connected_socket_constructor =
     (* all public constructors of Socket with two or more arguments call connect *)
     let actuals_pred = function [] | [_] -> false | _ -> true in
     is_call_of_class ~actuals_pred "java.net.Socket" [Typ.Procname.Java.constructor_method_name]
     |> Staged.unstage
-
-
-let is_datagram_socket_connect =
+  in
+  let is_datagram_socket_connect =
     is_call_of_class "java.net.DatagramSocket" ["connect"] |> Staged.unstage
-
-
-(* matchers used for normal analysis as well as in --dev-android-strict-mode *)
-(* selection is a bit arbitrary as some would be generated anyway if not here; no harm though *)
-(* some, like [file], need manual addition due to our lack of handling dynamic dispatch *)
-let strict_mode_common_matchers =
+  in
   let open StarvationDomain.Event in
   [ (is_connected_socket_constructor, High)
   ; (is_datagram_socket_connect, High)
@@ -206,6 +145,11 @@ let strict_mode_common_matchers =
 
 
 let strict_mode_seed_matchers =
+  (* matcher for strict mode throws in Android libcore implementation,
+     used with --dev-android strict mode *)
+  let is_blockguard_on =
+    is_call_of_class ~method_prefix:true "dalvik.system.BlockGuard$Policy" ["on"] |> Staged.unstage
+  in
   (is_blockguard_on, StarvationDomain.Event.High) :: strict_mode_common_matchers
 
 
@@ -215,6 +159,41 @@ let strict_mode_matchers =
 
 
 let standard_matchers =
+  (* is the method called Object.wait or on subclass, without timeout or with excessive timeout ? *)
+  let is_object_wait =
+    is_call_of_class ~actuals_pred:empty_or_excessive_timeout "java.lang.Object" ["wait"]
+    |> Staged.unstage
+  in
+  (* is the method called CountDownLath.await or on subclass? *)
+  let is_countdownlatch_await =
+    is_call_of_class ~actuals_pred:empty_or_excessive_timeout "java.util.concurrent.CountDownLatch"
+      ["await"]
+    |> Staged.unstage
+  in
+  (* an IBinder.transact call is an RPC.  If the 4th argument (5th counting `this` as the first)
+   is int-zero then a reply is expected and returned from the remote process, thus potentially
+   blocking.  If the 4th argument is anything else, we assume a one-way call which doesn't block. *)
+  let is_two_way_binder_transact =
+    let actuals_pred actuals =
+      List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero
+    in
+    is_call_of_class ~actuals_pred "android.os.IBinder" ["transact"] |> Staged.unstage
+  in
+  let is_future_get =
+    is_call_of_class ~search_superclasses:false ~actuals_pred:empty_or_excessive_timeout
+      "java.util.concurrent.Future" ["get"]
+    |> Staged.unstage
+  in
+  let is_accountManager_setUserData =
+    is_call_of_class ~search_superclasses:false "android.accounts.AccountManager" ["setUserData"]
+    |> Staged.unstage
+  in
+  let is_asyncTask_get =
+    is_call_of_class ~actuals_pred:empty_or_excessive_timeout "android.os.AsyncTask" ["get"]
+    |> Staged.unstage
+  in
+  (* consider any call to sleep as bad, even with timeouts lower than the anr limit *)
+  let is_thread_sleep = is_call_of_class "java.lang.Thread" ["sleep"] |> Staged.unstage in
   let open StarvationDomain.Event in
   [ (is_accountManager_setUserData, High)
   ; (is_two_way_binder_transact, High)

infer/tests/codetoanalyze/java/starvation/.inferconfig

@@ -0,0 +1,4 @@
+{
+  "force-delete-results-dir": true,
+  "starvation-skip-analysis" : [ { "classname": "SkipAnalysis", "methods": ["doTransact"] } ]
+}

infer/tests/codetoanalyze/java/starvation/SkipAnalysis.java

@@ -0,0 +1,23 @@
+/*
+ * 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;
+
+class SkipAnalysis {
+  Binder b;
+
+  void doTransact() throws RemoteException {
+    b.transact(0, null, null, 0);
+  }
+
+  @UiThread
+  void callTransact() throws RemoteException {
+    doTransact();
+  }
+}