diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index 66dc95a19..696a61094 100644 --- a/infer/src/base/Config.ml +++ b/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 diff --git a/infer/src/base/Config.mli b/infer/src/base/Config.mli index c6e8fe945..d8df30acf 100644 --- a/infer/src/base/Config.mli +++ b/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 diff --git a/infer/src/concurrency/ConcurrencyModels.ml b/infer/src/concurrency/ConcurrencyModels.ml index 69c954ada..022d69f1d 100644 --- a/infer/src/concurrency/ConcurrencyModels.ml +++ b/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,70 +191,96 @@ 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"] - |> Staged.unstage +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 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 + 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 - fun tenv pname actuals -> - fragment_v4_matcher tenv pname actuals || fragment_matcher tenv pname actuals + (match json with `Assoc fields -> parse_fields fields empty_matcher | _ -> error json) + |> make_matcher_from_record 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 + 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" + ; "onCreate" + ; "onCreateView" + ; "onActivityCreated" + ; "onStart" + ; "onResume" + ; "onPause" + ; "onStop" + ; "onDestroyView" + ; "onDestroy" + ; "onDetach" ] in - let is_view_ui_method = - (* according to Android documentation, *all* methods of the View class run on UI thread, but + [ {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 []) diff --git a/infer/src/concurrency/ConcurrencyModels.mli b/infer/src/concurrency/ConcurrencyModels.mli index 1f0fb6279..b5a2e5097 100644 --- a/infer/src/concurrency/ConcurrencyModels.mli +++ b/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. *) diff --git a/infer/src/concurrency/StarvationModels.ml b/infer/src/concurrency/StarvationModels.ml index d65d06639..04c8b8b3b 100644 --- a/infer/src/concurrency/StarvationModels.ml +++ b/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,105 +90,51 @@ 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 - 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 = - is_call_of_class "java.io.File" - [ "canRead" - ; "canWrite" - ; "createNewFile" - ; "createTempFile" - ; "delete" - ; "getCanonicalPath" - ; "getFreeSpace" - ; "getTotalSpace" - ; "getUsableSpace" - ; "isDirectory" - ; "isFile" - ; "isHidden" - ; "lastModified" - ; "length" - ; "list" - ; "listFiles" - ; "mkdir" - ; "renameTo" - ; "setExecutable" - ; "setLastModified" - ; "setReadable" - ; "setReadOnly" - ; "setWritable" ] - |> Staged.unstage - - -let is_socket_connect = is_call_of_class "java.net.Socket" ["connect"] |> Staged.unstage - -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 = - 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 = + let is_system_gc = + is_call_of_class "java.lang.System" ["gc"; "runFinalization"] |> Staged.unstage + in + 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" + ; "createNewFile" + ; "createTempFile" + ; "delete" + ; "getCanonicalPath" + ; "getFreeSpace" + ; "getTotalSpace" + ; "getUsableSpace" + ; "isDirectory" + ; "isFile" + ; "isHidden" + ; "lastModified" + ; "length" + ; "list" + ; "listFiles" + ; "mkdir" + ; "renameTo" + ; "setExecutable" + ; "setLastModified" + ; "setReadable" + ; "setReadOnly" + ; "setWritable" ] + |> Staged.unstage + 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 + in + let is_datagram_socket_connect = + is_call_of_class "java.net.DatagramSocket" ["connect"] |> Staged.unstage + 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) diff --git a/infer/tests/codetoanalyze/java/starvation/.inferconfig b/infer/tests/codetoanalyze/java/starvation/.inferconfig new file mode 100644 index 000000000..858b980cb --- /dev/null +++ b/infer/tests/codetoanalyze/java/starvation/.inferconfig @@ -0,0 +1,4 @@ +{ + "force-delete-results-dir": true, + "starvation-skip-analysis" : [ { "classname": "SkipAnalysis", "methods": ["doTransact"] } ] +} diff --git a/infer/tests/codetoanalyze/java/starvation/SkipAnalysis.java b/infer/tests/codetoanalyze/java/starvation/SkipAnalysis.java new file mode 100644 index 000000000..ac985e4f4 --- /dev/null +++ b/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(); + } +}