From 631959ced04f5ac7fd8d6e21897ad462ba6f873d Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Tue, 25 Sep 2018 04:08:08 -0700 Subject: [PATCH] [starvation] refactor method matching ; kill dev-android-strict-mode option Summary: The method matcher is now used sufficiently it warrants refactoring out into its own module. Also, kill dev-android-strict-mode and leave starvation-strict-mode as the stronger option. Reviewed By: jeremydubreil Differential Revision: D9990753 fbshipit-source-id: 626a70a19 --- Makefile | 3 +- infer/src/base/Config.ml | 8 - infer/src/base/Config.mli | 2 - infer/src/concurrency/ConcurrencyModels.ml | 92 +------- infer/src/concurrency/ConcurrencyModels.mli | 26 --- infer/src/concurrency/MethodMatcher.ml | 91 ++++++++ infer/src/concurrency/MethodMatcher.mli | 51 +++++ infer/src/concurrency/StarvationModels.ml | 199 ++++++++---------- infer/src/concurrency/StrictModeModels.ml | 66 +++--- infer/src/concurrency/starvation.ml | 170 +++++++-------- .../codetoanalyze/java/devstrictmode/Makefile | 13 -- .../devstrictmode/NonPublicGuardTest.java | 30 --- .../dalvik/system/BlockGuard.java | 30 --- .../java/devstrictmode/issues.exp | 4 - .../java/devstrictmode/java/GuardTest.java | 32 --- 15 files changed, 341 insertions(+), 476 deletions(-) create mode 100644 infer/src/concurrency/MethodMatcher.ml create mode 100644 infer/src/concurrency/MethodMatcher.mli delete mode 100644 infer/tests/codetoanalyze/java/devstrictmode/Makefile delete mode 100644 infer/tests/codetoanalyze/java/devstrictmode/NonPublicGuardTest.java delete mode 100644 infer/tests/codetoanalyze/java/devstrictmode/dalvik/system/BlockGuard.java delete mode 100644 infer/tests/codetoanalyze/java/devstrictmode/issues.exp delete mode 100644 infer/tests/codetoanalyze/java/devstrictmode/java/GuardTest.java diff --git a/Makefile b/Makefile index bf43f9250..1643e8da7 100644 --- a/Makefile +++ b/Makefile @@ -92,8 +92,7 @@ BUILD_SYSTEMS_TESTS += \ DIRECT_TESTS += \ java_checkers java_eradicate java_infer java_lab java_tracing java_quandary \ - java_racerd java_stability java_crashcontext java_hoisting java_starvation java_performance \ - java_devstrictmode + java_racerd java_stability java_crashcontext java_hoisting java_starvation java_performance ifneq ($(ANT),no) BUILD_SYSTEMS_TESTS += ant endif diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index 3451846eb..e1c66b96b 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -2105,12 +2105,6 @@ and stats_report = "Write a report of the analysis results to a file" -and dev_android_strict_mode = - CLOpt.mk_bool ~long:"dev-android-strict-mode" ~default:false - "Developer mode for starvation analysis, only for use on android implementation sources; \ - detects methods in the Android library core which throw strict mode violation exceptions" - - and subtype_multirange = CLOpt.mk_bool ~deprecated:["subtype_multirange"] ~long:"subtype-multirange" ~default:true "Use the multirange subtyping domain" @@ -2626,8 +2620,6 @@ and default_linters = !default_linters and dependency_mode = !dependencies -and dev_android_strict_mode = !dev_android_strict_mode - and developer_mode = !developer_mode and differential_filter_files = !differential_filter_files diff --git a/infer/src/base/Config.mli b/infer/src/base/Config.mli index d8df30acf..ceb2c255e 100644 --- a/infer/src/base/Config.mli +++ b/infer/src/base/Config.mli @@ -328,8 +328,6 @@ val default_linters : bool val dependency_mode : bool -val dev_android_strict_mode : bool - val developer_mode : bool val differential_filter_files : string option diff --git a/infer/src/concurrency/ConcurrencyModels.ml b/infer/src/concurrency/ConcurrencyModels.ml index 8fe0154b4..f55746159 100644 --- a/infer/src/concurrency/ConcurrencyModels.ml +++ b/infer/src/concurrency/ConcurrencyModels.ml @@ -7,7 +7,6 @@ open! IStd module F = Format -module L = Logging module MF = MarkupFormatter type lock = Lock | Unlock | LockedIfTrue | NoEffect @@ -159,92 +158,9 @@ let find_annotated_or_overriden_annotated_method is_annot pname tenv = tenv pname -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 - - -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 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 open MethodMatcher in + let superclasses = {empty with search_superclasses= Some true} in let fragment_methods = (* sort police: this is in lifecycle order *) [ "onAttach" @@ -280,9 +196,9 @@ let ui_matcher_records = let is_ui_method = - let matchers = List.map ui_matcher_records ~f:make_matcher_from_record in + let matchers = List.map ui_matcher_records ~f:MethodMatcher.of_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 []) + fun tenv pname -> MethodMatcher.of_list matchers tenv pname [] let runs_on_ui_thread = diff --git a/infer/src/concurrency/ConcurrencyModels.mli b/infer/src/concurrency/ConcurrencyModels.mli index b5a2e5097..80a1b2172 100644 --- a/infer/src/concurrency/ConcurrencyModels.mli +++ b/infer/src/concurrency/ConcurrencyModels.mli @@ -34,29 +34,3 @@ 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 - -> 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/MethodMatcher.ml b/infer/src/concurrency/MethodMatcher.ml new file mode 100644 index 000000000..58afe15f1 --- /dev/null +++ b/infer/src/concurrency/MethodMatcher.ml @@ -0,0 +1,91 @@ +(* + * Copyright (c) 2017-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. + *) + +open! IStd +module L = Logging + +let call_matches ?(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 + + +type t = Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool + +type record = + { search_superclasses: bool option + ; method_prefix: bool option + ; actuals_pred: (HilExp.t list -> bool) option + ; classname: string + ; methods: string list } + +let of_record {search_superclasses; method_prefix; actuals_pred; classname; methods} = + call_matches ?search_superclasses ?method_prefix ?actuals_pred classname methods + |> Staged.unstage + + +let empty = + {search_superclasses= None; method_prefix= None; actuals_pred= None; classname= ""; methods= []} + + +let of_list matchers tenv pn actuals = List.exists matchers ~f:(fun m -> m tenv pn actuals) + +let of_json top_json = + let error json = + L.(die UserError "Could not parse json matcher(s): %s" (Yojson.Basic.to_string json)) + in + 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 | _ -> error json) |> of_record + in + match top_json with + | `List matchers_json -> + List.map matchers_json ~f:make_matcher_from_json |> of_list + | _ -> + error top_json diff --git a/infer/src/concurrency/MethodMatcher.mli b/infer/src/concurrency/MethodMatcher.mli new file mode 100644 index 000000000..befc2a575 --- /dev/null +++ b/infer/src/concurrency/MethodMatcher.mli @@ -0,0 +1,51 @@ +(* + * Copyright (c) 2017-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. + *) + +open! IStd + +(** pattern matcher for Java methods *) +type t = Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool + +val call_matches : + ?search_superclasses:bool + -> ?method_prefix:bool + -> ?actuals_pred:(HilExp.t list -> bool) + -> string + -> string list + -> t Staged.t + [@@warning "-32"] +(** [call_matches 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 [true]. + - [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]. *) + +type record = + { search_superclasses: bool option + ; method_prefix: bool option + ; actuals_pred: (HilExp.t list -> bool) option + ; classname: string + ; methods: string list } + +val empty : record +(** record with empty strings, lists and None where appropriate. Useful for [with] expressions *) + +val of_record : record -> t +(** make a matcher out of a record; optional values use defaults *) + +val of_json : Yojson.Basic.json -> t +(** 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. *) + +val of_list : t list -> t +(** Or combinator *) diff --git a/infer/src/concurrency/StarvationModels.ml b/infer/src/concurrency/StarvationModels.ml index 04c8b8b3b..68f092003 100644 --- a/infer/src/concurrency/StarvationModels.ml +++ b/infer/src/concurrency/StarvationModels.ml @@ -6,7 +6,6 @@ *) open! IStd -open ConcurrencyModels let is_synchronized_library_call = let targets = ["java.lang.StringBuffer"; "java.util.Hashtable"; "java.util.Vector"] in @@ -21,7 +20,7 @@ let is_synchronized_library_call = false -let should_skip_analysis = ConcurrencyModels.matcher_of_json Config.starvation_skip_analysis +let should_skip_analysis = MethodMatcher.of_json Config.starvation_skip_analysis (** magical value from https://developer.android.com/topic/performance/vitals/anr *) let android_anr_time_limit = 5.0 @@ -90,125 +89,107 @@ let empty_or_excessive_timeout actuals = false -(* 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) - ; (is_file_io, High) - ; (is_runtime_gc, High) - ; (is_socket_connect, High) - ; (is_system_gc, High) ] - - -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 - - -let strict_mode_matchers = +let strict_mode_matcher = + let open MethodMatcher in let open StarvationDomain.Event in - (StrictModeModels.is_strict_mode_violation, High) :: strict_mode_common_matchers + (* NB [empty] searches superclasses too. Most of the classes below are final and we don't + really want to search superclasses for those that aren't, so for performance, disable that *) + let empty = {empty with search_superclasses= Some false} in + let matcher_records = + [ { empty with + classname= "dalvik.system.BlockGuard$Policy"; methods= ["on"]; method_prefix= Some true } + ; {empty with classname= "java.lang.System"; methods= ["gc"; "runFinalization"]} + ; {empty with classname= "java.lang.Runtime"; methods= ["gc"]} + ; {empty with classname= "java.net.Socket"; methods= ["connect"]} + (* all public constructors of Socket with two or more arguments call connect *) + ; { empty with + classname= "java.net.Socket" + ; methods= [Typ.Procname.Java.constructor_method_name] + ; actuals_pred= Some (function [] | [_] -> false | _ -> true) } + ; {empty with classname= "java.net.DatagramSocket"; methods= ["connect"]} + ; { empty with + classname= "java.io.File" + ; methods= + [ "canRead" + ; "canWrite" + ; "createNewFile" + ; "createTempFile" + ; "delete" + ; "getCanonicalPath" + ; "getFreeSpace" + ; "getTotalSpace" + ; "getUsableSpace" + ; "isDirectory" + ; "isFile" + ; "isHidden" + ; "lastModified" + ; "length" + ; "list" + ; "listFiles" + ; "mkdir" + ; "renameTo" + ; "setExecutable" + ; "setLastModified" + ; "setReadable" + ; "setReadOnly" + ; "setWritable" ] } ] + in + let matcher = + of_list (StrictModeModels.is_strict_mode_violation :: List.map matcher_records ~f:of_record) + in + (matcher, High) 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 MethodMatcher in let open StarvationDomain.Event in - [ (is_accountManager_setUserData, High) - ; (is_two_way_binder_transact, High) - ; (is_countdownlatch_await, High) - ; (is_thread_sleep, High) - ; (is_object_wait, High) - ; (is_asyncTask_get, Low) - ; (is_future_get, Low) ] + let high_sev = + [ {empty with classname= "java.lang.Thread"; methods= ["sleep"]} + ; { empty with + classname= "java.lang.Object" + ; methods= ["wait"] + ; actuals_pred= Some empty_or_excessive_timeout } + ; { empty with + classname= "java.util.concurrent.CountDownLatch" + ; methods= ["await"] + ; actuals_pred= Some empty_or_excessive_timeout } + (* 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. *) + ; { empty with + classname= "android.os.IBinder" + ; methods= ["transact"] + ; actuals_pred= + Some + (fun actuals -> + List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero ) } + ; { empty with + classname= "android.accounts.AccountManager" + ; methods= ["setUserData"] + ; search_superclasses= Some false } ] + in + let low_sev = + [ { empty with + classname= "java.util.concurrent.Future" + ; methods= ["get"] + ; actuals_pred= Some empty_or_excessive_timeout + ; search_superclasses= Some false } + ; { empty with + classname= "android.os.AsyncTask" + ; methods= ["get"] + ; actuals_pred= Some empty_or_excessive_timeout } ] + in + let high_sev_matcher = List.map high_sev ~f:of_record |> of_list in + let low_sev_matcher = List.map low_sev ~f:of_record |> of_list in + [(high_sev_matcher, High); (low_sev_matcher, Low)] (* sort from High to Low *) let may_block = let matchers = - if Config.dev_android_strict_mode then strict_mode_seed_matchers - else if Config.starvation_strict_mode then strict_mode_matchers @ standard_matchers + if Config.starvation_strict_mode then strict_mode_matcher :: standard_matchers else standard_matchers in fun tenv pn actuals -> diff --git a/infer/src/concurrency/StrictModeModels.ml b/infer/src/concurrency/StrictModeModels.ml index 83a0d1df1..7a1b2868d 100644 --- a/infer/src/concurrency/StrictModeModels.ml +++ b/infer/src/concurrency/StrictModeModels.ml @@ -6,11 +6,11 @@ *) open! IStd -open ConcurrencyModels +open MethodMatcher (* frameworks/base/core/java/android/app/backup/BackupAgent.java *) let is_BackupAgent_method = - is_call_of_class "android.app.backup.BackupAgent" + call_matches "android.app.backup.BackupAgent" ["onRestoreFile" (* onRestoreFile(ParcelFileDescriptor,long,int,String,String,long,long) *) ] |> Staged.unstage @@ -18,14 +18,14 @@ let is_BackupAgent_method = (* frameworks/base/core/java/android/app/DownloadManager.java *) let is_DownloadManager_method = - is_call_of_class "android.app.DownloadManager" ["rename" (* rename(Context,long,String) *) - ] + call_matches "android.app.DownloadManager" ["rename" (* rename(Context,long,String) *) + ] |> Staged.unstage (* frameworks/base/core/java/android/app/NotificationManager.java *) let is_NotificationManager_method = - is_call_of_class "android.app.NotificationManager" + call_matches "android.app.NotificationManager" ["notifyAsUser" (* notifyAsUser(String,int,Notification,UserHandle) *) ] |> Staged.unstage @@ -33,14 +33,14 @@ let is_NotificationManager_method = (* frameworks/base/core/java/android/content/pm/ActivityInfo.java *) let is_ActivityInfo_method = - is_call_of_class "android.content.pm.ActivityInfo" ["dump" (* dump(Printer,String,int) *) - ] + call_matches "android.content.pm.ActivityInfo" ["dump" (* dump(Printer,String,int) *) + ] |> Staged.unstage (* frameworks/base/core/java/android/content/pm/ApplicationInfo.java *) let is_ApplicationInfo_method = - is_call_of_class "android.content.pm.ApplicationInfo" + call_matches "android.content.pm.ApplicationInfo" [ "dump" ; (* dump(Printer,String,int) *) "getHiddenApiEnforcementPolicy" @@ -53,28 +53,28 @@ let is_ApplicationInfo_method = (* frameworks/base/core/java/android/content/pm/ProviderInfo.java *) let is_ProviderInfo_method = - is_call_of_class "android.content.pm.ProviderInfo" ["dump" (* dump(Printer,String,int) *) - ] + call_matches "android.content.pm.ProviderInfo" ["dump" (* dump(Printer,String,int) *) + ] |> Staged.unstage (* frameworks/base/core/java/android/content/pm/ResolveInfo.java *) let is_ResolveInfo_method = - is_call_of_class "android.content.pm.ResolveInfo" ["dump" (* dump(Printer,String,int) *) - ] + call_matches "android.content.pm.ResolveInfo" ["dump" (* dump(Printer,String,int) *) + ] |> Staged.unstage (* frameworks/base/core/java/android/content/pm/ServiceInfo.java *) let is_ServiceInfo_method = - is_call_of_class "android.content.pm.ServiceInfo" ["dump" (* dump(Printer,String,int) *) - ] + call_matches "android.content.pm.ServiceInfo" ["dump" (* dump(Printer,String,int) *) + ] |> Staged.unstage (* frameworks/base/core/java/android/database/sqlite/SQLiteDatabase.java *) let is_SQLiteDatabase_method = - is_call_of_class "android.database.sqlite.SQLiteDatabase" + call_matches "android.database.sqlite.SQLiteDatabase" [ "addCustomFunction" ; (* addCustomFunction(String,int,SQLiteDatabase$CustomFunction) *) "reopenReadWrite" @@ -85,20 +85,20 @@ let is_SQLiteDatabase_method = (* frameworks/base/core/java/android/ddm/DdmHandleHeap.java *) let is_DdmHandleHeap_method = - is_call_of_class "android.ddm.DdmHandleHeap" ["handleChunk" (* handleChunk(Chunk) *) - ] + call_matches "android.ddm.DdmHandleHeap" ["handleChunk" (* handleChunk(Chunk) *) + ] |> Staged.unstage (* frameworks/base/core/java/android/net/Uri.java *) let is_Uri_method = - is_call_of_class "android.net.Uri" ["getCanonicalUri" (* getCanonicalUri() *) - ] |> Staged.unstage + call_matches "android.net.Uri" ["getCanonicalUri" (* getCanonicalUri() *) + ] |> Staged.unstage (* frameworks/base/core/java/android/os/Environment.java *) let is_Environment_method = - is_call_of_class "android.os.Environment" + call_matches "android.os.Environment" ["classifyExternalStorageDirectory" (* classifyExternalStorageDirectory(File) *) ] |> Staged.unstage @@ -106,14 +106,14 @@ let is_Environment_method = (* frameworks/base/core/java/android/os/Parcel.java *) let is_Parcel_method = - is_call_of_class "android.os.Parcel" ["readExceptionCode" (* readExceptionCode() *) - ] + call_matches "android.os.Parcel" ["readExceptionCode" (* readExceptionCode() *) + ] |> Staged.unstage (* frameworks/base/core/java/android/os/RecoverySystem.java *) let is_RecoverySystem_method = - is_call_of_class "android.os.RecoverySystem" + call_matches "android.os.RecoverySystem" [ "handleAftermath" ; (* handleAftermath(Context) *) "rebootPromptAndWipeUserData" @@ -134,7 +134,7 @@ let is_RecoverySystem_method = (* frameworks/base/core/java/android/os/storage/StorageManager.java *) let is_StorageManager_method = - is_call_of_class "android.os.storage.StorageManager" + call_matches "android.os.storage.StorageManager" [ "getPrimaryStoragePathAndSize" ; (* getPrimaryStoragePathAndSize() *) "getPrimaryStorageSize" @@ -151,7 +151,7 @@ let is_StorageManager_method = (* frameworks/base/core/java/android/os/StrictMode.java *) let is_StrictMode_method = - is_call_of_class "android.os.StrictMode" + call_matches "android.os.StrictMode" [ "conditionallyCheckInstanceCounts" ; (* conditionallyCheckInstanceCounts() *) "decrementExpectedActivityCount" @@ -172,7 +172,7 @@ let is_StrictMode_method = (* frameworks/base/core/java/android/util/AtomicFile.java *) let is_AtomicFile_method = - is_call_of_class "android.util.AtomicFile" + call_matches "android.util.AtomicFile" ["getLastModifiedTime"; (* getLastModifiedTime() *) "startWrite" (* startWrite(long) *) ] |> Staged.unstage @@ -180,7 +180,7 @@ let is_AtomicFile_method = (* frameworks/base/core/java/android/webkit/WebViewFactory.java *) let is_WebViewFactory_method = - is_call_of_class "android.webkit.WebViewFactory" + call_matches "android.webkit.WebViewFactory" ["onWebViewProviderChanged" (* onWebViewProviderChanged(PackageInfo) *) ] |> Staged.unstage @@ -188,7 +188,7 @@ let is_WebViewFactory_method = (* frameworks/base/core/java/android/webkit/WebViewLibraryLoader.java *) let is_WebViewLibraryLoader_method = - is_call_of_class "android.webkit.WebViewLibraryLoader" + call_matches "android.webkit.WebViewLibraryLoader" ["getWebViewNativeLibrary" (* getWebViewNativeLibrary(PackageInfo,boolean) *) ] |> Staged.unstage @@ -196,7 +196,7 @@ let is_WebViewLibraryLoader_method = (* frameworks/base/media/java/android/media/MiniThumbFile.java *) let is_MiniThumbFile_method = - is_call_of_class "android.media.MiniThumbFile" + call_matches "android.media.MiniThumbFile" [ "eraseMiniThumb" ; (* eraseMiniThumb(long) *) "getMagic" @@ -211,7 +211,7 @@ let is_MiniThumbFile_method = (* frameworks/base/media/java/android/media/RingtoneManager.java *) let is_RingtoneManager_method = - is_call_of_class "android.media.RingtoneManager" + call_matches "android.media.RingtoneManager" ["deleteExternalRingtone" (* deleteExternalRingtone(Uri) *) ] |> Staged.unstage @@ -219,7 +219,7 @@ let is_RingtoneManager_method = (* frameworks/multidex/library/src/androidx/multidex/MultiDex.java *) let is_MultiDex_method = - is_call_of_class "androidx.multidex.MultiDex" + call_matches "androidx.multidex.MultiDex" [ "install" ; (* install(Context) *) "installInstrumentation" @@ -230,8 +230,8 @@ let is_MultiDex_method = (* libcore/ojluni/src/main/java/java/util/logging/FileHandler.java *) let is_FileHandler_method = - is_call_of_class "java.util.logging.FileHandler" ["run" (* run() *) - ] |> Staged.unstage + call_matches "java.util.logging.FileHandler" ["run" (* run() *) + ] |> Staged.unstage let is_strict_mode_violation = diff --git a/infer/src/concurrency/starvation.ml b/infer/src/concurrency/starvation.ml index 6348ff8f5..f5ab5913f 100644 --- a/infer/src/concurrency/starvation.ml +++ b/infer/src/concurrency/starvation.ml @@ -286,20 +286,10 @@ let should_report_deadlock_on_current_proc current_elem endpoint_elem = < 0 -let public_package_prefixes = ["java"; "android"] - let should_report pdesc = match Procdesc.get_proc_name pdesc with | Typ.Procname.Java java_pname -> - ( if Config.dev_android_strict_mode then - match Typ.Procname.Java.get_package java_pname with - | None -> - false - | Some package -> - (* the proc must be package-public, but we can't check for that *) - List.exists public_package_prefixes ~f:(fun prefix -> String.is_prefix package ~prefix) - else true ) - && Procdesc.get_access pdesc <> PredSymb.Private + Procdesc.get_access pdesc <> PredSymb.Private && (not (Typ.Procname.Java.is_autogen_method java_pname)) && not (Typ.Procname.Java.is_class_initializer java_pname) | _ -> @@ -331,75 +321,73 @@ let fold_reportable_summaries (tenv, current_pdesc) clazz ~init ~f = The net effect of the above issues is that we will only see these locks in conflicting pairs once, as opposed to twice with all other deadlock pairs. *) let report_deadlocks env {StarvationDomain.order; ui} report_map' = - if Config.dev_android_strict_mode then ReportMap.empty - else - let open StarvationDomain in - let tenv, current_pdesc = env in - let current_pname = Procdesc.get_proc_name current_pdesc in - let pp_acquire fmt (lock, loc, pname) = - F.fprintf fmt "%a (%a in %a)" Lock.pp lock Location.pp loc - (MF.wrap_monospaced Typ.Procname.pp) - pname - in - let pp_thread fmt - ( pname - , { Order.loc= loc1 - ; trace= trace1 - ; elem= {first= lock1; eventually= {elem= event; loc= loc2; trace= trace2}} } ) = - match event with - | LockAcquire lock2 -> - let pname1 = List.last trace1 |> Option.value_map ~default:pname ~f:CallSite.pname in - let pname2 = List.last trace2 |> Option.value_map ~default:pname1 ~f:CallSite.pname in - F.fprintf fmt "first %a and then %a" pp_acquire (lock1, loc1, pname1) pp_acquire - (lock2, loc2, pname2) - | _ -> - L.die InternalError "Trying to report a deadlock without two lock events." - in - let report_endpoint_elem current_elem endpoint_pname elem report_map = - if - not - ( Order.may_deadlock current_elem elem - && should_report_deadlock_on_current_proc current_elem elem ) - then report_map - else - let () = debug "Possible deadlock:@.%a@.%a@." Order.pp current_elem Order.pp elem in - match (current_elem.Order.elem.eventually.elem, elem.Order.elem.eventually.elem) with - | LockAcquire _, LockAcquire _ -> - let error_message = - Format.asprintf - "Potential deadlock.@.Trace 1 (starts at %a) %a.@.Trace 2 (starts at %a), %a." - (MF.wrap_monospaced Typ.Procname.pp) - current_pname pp_thread (current_pname, current_elem) - (MF.wrap_monospaced Typ.Procname.pp) - endpoint_pname pp_thread (endpoint_pname, elem) - in - let first_trace = Order.make_trace ~header:"[Trace 1] " current_pname current_elem in - let second_trace = Order.make_trace ~header:"[Trace 2] " endpoint_pname elem in - let ltr = first_trace @ second_trace in - let loc = Order.get_loc current_elem in - ReportMap.add_deadlock tenv current_pdesc loc ltr error_message report_map - | _, _ -> - report_map - in - let report_on_current_elem elem report_map = - match elem.Order.elem.eventually.elem with - | MayBlock _ -> + let open StarvationDomain in + let tenv, current_pdesc = env in + let current_pname = Procdesc.get_proc_name current_pdesc in + let pp_acquire fmt (lock, loc, pname) = + F.fprintf fmt "%a (%a in %a)" Lock.pp lock Location.pp loc + (MF.wrap_monospaced Typ.Procname.pp) + pname + in + let pp_thread fmt + ( pname + , { Order.loc= loc1 + ; trace= trace1 + ; elem= {first= lock1; eventually= {elem= event; loc= loc2; trace= trace2}} } ) = + match event with + | LockAcquire lock2 -> + let pname1 = List.last trace1 |> Option.value_map ~default:pname ~f:CallSite.pname in + let pname2 = List.last trace2 |> Option.value_map ~default:pname1 ~f:CallSite.pname in + F.fprintf fmt "first %a and then %a" pp_acquire (lock1, loc1, pname1) pp_acquire + (lock2, loc2, pname2) + | _ -> + L.die InternalError "Trying to report a deadlock without two lock events." + in + let report_endpoint_elem current_elem endpoint_pname elem report_map = + if + not + ( Order.may_deadlock current_elem elem + && should_report_deadlock_on_current_proc current_elem elem ) + then report_map + else + let () = debug "Possible deadlock:@.%a@.%a@." Order.pp current_elem Order.pp elem in + match (current_elem.Order.elem.eventually.elem, elem.Order.elem.eventually.elem) with + | LockAcquire _, LockAcquire _ -> + let error_message = + Format.asprintf + "Potential deadlock.@.Trace 1 (starts at %a) %a.@.Trace 2 (starts at %a), %a." + (MF.wrap_monospaced Typ.Procname.pp) + current_pname pp_thread (current_pname, current_elem) + (MF.wrap_monospaced Typ.Procname.pp) + endpoint_pname pp_thread (endpoint_pname, elem) + in + let first_trace = Order.make_trace ~header:"[Trace 1] " current_pname current_elem in + let second_trace = Order.make_trace ~header:"[Trace 2] " endpoint_pname elem in + let ltr = first_trace @ second_trace in + let loc = Order.get_loc current_elem in + ReportMap.add_deadlock tenv current_pdesc loc ltr error_message report_map + | _, _ -> report_map - | LockAcquire endpoint_lock -> - Lock.owner_class endpoint_lock - |> Option.value_map ~default:report_map ~f:(fun endpoint_class -> - (* get the class of the root variable of the lock in the endpoint elem + in + let report_on_current_elem elem report_map = + match elem.Order.elem.eventually.elem with + | MayBlock _ -> + report_map + | LockAcquire endpoint_lock -> + Lock.owner_class endpoint_lock + |> Option.value_map ~default:report_map ~f:(fun endpoint_class -> + (* get the class of the root variable of the lock in the endpoint elem and retrieve all the summaries of the methods of that class *) - (* for each summary related to the endpoint, analyse and report on its pairs *) - fold_reportable_summaries env endpoint_class ~init:report_map - ~f:(fun acc (endp_pname, endpoint_summary) -> - let endp_order = endpoint_summary.order in - let endp_ui = endpoint_summary.ui in - if UIThreadDomain.is_empty ui || UIThreadDomain.is_empty endp_ui then - OrderDomain.fold (report_endpoint_elem elem endp_pname) endp_order acc - else acc ) ) - in - OrderDomain.fold report_on_current_elem order report_map' + (* for each summary related to the endpoint, analyse and report on its pairs *) + fold_reportable_summaries env endpoint_class ~init:report_map + ~f:(fun acc (endp_pname, endpoint_summary) -> + let endp_order = endpoint_summary.order in + let endp_ui = endpoint_summary.ui in + if UIThreadDomain.is_empty ui || UIThreadDomain.is_empty endp_ui then + OrderDomain.fold (report_endpoint_elem elem endp_pname) endp_order acc + else acc ) ) + in + OrderDomain.fold report_on_current_elem order report_map' let report_starvation env {StarvationDomain.events; ui} report_map' = @@ -460,27 +448,11 @@ let report_starvation env {StarvationDomain.events; ui} report_map' = order acc else acc ) ) in - let report_strict_mode event rmap = - match event.Event.elem with - | MayBlock (_, sev) -> - let error_message = - Format.asprintf "Method %a commits a strict mode violation; %a." - (MF.wrap_monospaced Typ.Procname.pp) - current_pname Event.pp event - in - let loc = Event.get_loc event in - let ltr = Event.make_trace current_pname event in - ReportMap.add_starvation tenv sev current_pdesc loc ltr error_message rmap - | _ -> - rmap - in - if Config.dev_android_strict_mode then EventDomain.fold report_strict_mode events report_map' - else - match ui with - | AbstractDomain.Types.Bottom -> - report_map' - | AbstractDomain.Types.NonBottom ui_explain -> - EventDomain.fold (report_on_current_elem ui_explain) events report_map' + match ui with + | AbstractDomain.Types.Bottom -> + report_map' + | AbstractDomain.Types.NonBottom ui_explain -> + EventDomain.fold (report_on_current_elem ui_explain) events report_map' let reporting {Callbacks.procedures; source_file} = diff --git a/infer/tests/codetoanalyze/java/devstrictmode/Makefile b/infer/tests/codetoanalyze/java/devstrictmode/Makefile deleted file mode 100644 index dbc66c651..000000000 --- a/infer/tests/codetoanalyze/java/devstrictmode/Makefile +++ /dev/null @@ -1,13 +0,0 @@ -# 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. - -TESTS_DIR = ../../.. - -ANALYZER = checkers -INFER_OPTIONS = --starvation-only --dev-android-strict-mode --debug-exceptions -INFERPRINT_OPTIONS = --issues-tests -SOURCES = dalvik/system/BlockGuard.java java/GuardTest.java $(wildcard *.java) - -include $(TESTS_DIR)/javac.make diff --git a/infer/tests/codetoanalyze/java/devstrictmode/NonPublicGuardTest.java b/infer/tests/codetoanalyze/java/devstrictmode/NonPublicGuardTest.java deleted file mode 100644 index f24bea6d5..000000000 --- a/infer/tests/codetoanalyze/java/devstrictmode/NonPublicGuardTest.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * 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 dalvik.system.BlockGuard; - -public class NonPublicGuardTest { - public void testWriteOk() { - BlockGuard.getThreadPolicy().onWriteToDisk(); - } - - public void testReadOk() { - BlockGuard.getThreadPolicy().onReadFromDisk(); - } - - public void testNetOk() { - BlockGuard.getThreadPolicy().onNetwork(); - } - - private void privateOk() { - testReadOk(); - } - - public void intraprocOk() { - privateOk(); - } - -} diff --git a/infer/tests/codetoanalyze/java/devstrictmode/dalvik/system/BlockGuard.java b/infer/tests/codetoanalyze/java/devstrictmode/dalvik/system/BlockGuard.java deleted file mode 100644 index 3aa15fc14..000000000 --- a/infer/tests/codetoanalyze/java/devstrictmode/dalvik/system/BlockGuard.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * 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. - */ - -package dalvik.system; - -public final class BlockGuard { - public interface Policy { - void onWriteToDisk(); - void onReadFromDisk(); - void onNetwork(); - int getPolicyMask(); - } - - public static final Policy threadPolicy = new Policy() { - public void onWriteToDisk() {} - public void onReadFromDisk() {} - public void onNetwork() {} - public int getPolicyMask() { - return 0; - } - }; - - public static Policy getThreadPolicy() { - return threadPolicy; - } -} diff --git a/infer/tests/codetoanalyze/java/devstrictmode/issues.exp b/infer/tests/codetoanalyze/java/devstrictmode/issues.exp deleted file mode 100644 index 40b303380..000000000 --- a/infer/tests/codetoanalyze/java/devstrictmode/issues.exp +++ /dev/null @@ -1,4 +0,0 @@ -codetoanalyze/java/devstrictmode/java/GuardTest.java, java.GuardTest.intraprocBad():void, 29, STARVATION, no_bucket, ERROR, [`void GuardTest.intraprocBad()`,Method call: `void GuardTest.privateOk()`,Method call: `void GuardTest.testReadBad()`,calls `void BlockGuard$Policy.onReadFromDisk()` from `void GuardTest.testReadBad()`] -codetoanalyze/java/devstrictmode/java/GuardTest.java, java.GuardTest.testNetBad():void, 21, STARVATION, no_bucket, ERROR, [`void GuardTest.testNetBad()`,calls `void BlockGuard$Policy.onNetwork()` from `void GuardTest.testNetBad()`] -codetoanalyze/java/devstrictmode/java/GuardTest.java, java.GuardTest.testReadBad():void, 17, STARVATION, no_bucket, ERROR, [`void GuardTest.testReadBad()`,calls `void BlockGuard$Policy.onReadFromDisk()` from `void GuardTest.testReadBad()`] -codetoanalyze/java/devstrictmode/java/GuardTest.java, java.GuardTest.testWriteBad():void, 13, STARVATION, no_bucket, ERROR, [`void GuardTest.testWriteBad()`,calls `void BlockGuard$Policy.onWriteToDisk()` from `void GuardTest.testWriteBad()`] diff --git a/infer/tests/codetoanalyze/java/devstrictmode/java/GuardTest.java b/infer/tests/codetoanalyze/java/devstrictmode/java/GuardTest.java deleted file mode 100644 index 0a7f63fcf..000000000 --- a/infer/tests/codetoanalyze/java/devstrictmode/java/GuardTest.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * 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. - */ -package java; - -import dalvik.system.BlockGuard; - -public class GuardTest { - public void testWriteBad() { - BlockGuard.getThreadPolicy().onWriteToDisk(); - } - - public void testReadBad() { - BlockGuard.getThreadPolicy().onReadFromDisk(); - } - - public void testNetBad() { - BlockGuard.getThreadPolicy().onNetwork(); - } - - private void privateOk() { - testReadBad(); - } - - public void intraprocBad() { - privateOk(); - } - -}