From 63aafd33810f4d0994dafbd8fdf68c401f0871cc Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Thu, 26 Apr 2018 01:52:31 -0700 Subject: [PATCH] [starvation] catch calls to View.getVisibleDisplayFrame Reviewed By: jeremydubreil Differential Revision: D7758016 fbshipit-source-id: f3ff474 --- infer/src/concurrency/RacerDConfig.ml | 49 ++++++++++--------- infer/src/concurrency/RacerDConfig.mli | 8 ++- infer/src/concurrency/starvation.ml | 3 +- .../java/starvation/VisDispFrame.java | 26 ++++++++++ .../codetoanalyze/java/starvation/issues.exp | 1 + 5 files changed, 60 insertions(+), 27 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/starvation/VisDispFrame.java diff --git a/infer/src/concurrency/RacerDConfig.ml b/infer/src/concurrency/RacerDConfig.ml index eac61ce14..770113490 100644 --- a/infer/src/concurrency/RacerDConfig.ml +++ b/infer/src/concurrency/RacerDConfig.ml @@ -542,40 +542,41 @@ module Models = struct && not (Annotations.pdesc_return_annot_ends_with proc_desc Annotations.visibleForTesting) - let is_blocking_java_io tenv pn = + let is_call_of_class_or_superclass class_names ?(method_prefix= false) method_name tenv pn = match pn with | Typ.Procname.Java java_pname -> let classname = Typ.Procname.Java.get_class_type_name java_pname in let mthd = Typ.Procname.Java.get_method java_pname in - let is_reader_or_inputstream = - let targets_str = ["java.io.Reader"; "java.io.InputStream"] in - let targets = List.map targets_str ~f:Typ.Name.Java.from_string 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 - String.is_prefix mthd ~prefix:"read" - && PatternMatch.supertype_exists tenv is_reader_or_inputstream classname + ( if method_prefix then String.is_prefix mthd ~prefix:method_name + else String.equal mthd method_name ) + && PatternMatch.supertype_exists tenv is_target_class classname | _ -> false - let is_countdownlatch_await pn = - match pn with - | Typ.Procname.Java java_pname -> - let classname = Typ.Procname.Java.get_class_name java_pname in - let mthd = Typ.Procname.Java.get_method java_pname in - String.equal classname "java.util.concurrent.CountDownLatch" && String.equal mthd "await" - | _ -> - false + let is_blocking_java_io = + let target_classes = ["java.io.Reader"; "java.io.InputStream"] in + fun tenv pn -> is_call_of_class_or_superclass target_classes ~method_prefix:true "read" tenv pn - let is_two_way_binder_transact tenv actuals pn = - match pn with - | Typ.Procname.Java java_pname -> - let classname = Typ.Procname.Java.get_class_type_name java_pname in - let mthd = Typ.Procname.Java.get_method java_pname in - String.equal mthd "transact" - && PatternMatch.is_subtype_of_str tenv classname "android.os.IBinder" - && List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero - | _ -> - false + let is_countdownlatch_await = + let target_classes = ["java.util.concurrent.CountDownLatch"] in + fun tenv pn -> is_call_of_class_or_superclass target_classes "await" tenv pn + + + let is_two_way_binder_transact = + let target_classes = ["android.os.IBinder"] in + fun tenv actuals pn -> + List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero + && is_call_of_class_or_superclass target_classes "transact" tenv pn + + + let is_getWindowVisibleDisplayFrame = + let target_classes = ["android.view.View"] in + fun tenv pn -> + is_call_of_class_or_superclass target_classes "getWindowVisibleDisplayFrame" tenv pn end diff --git a/infer/src/concurrency/RacerDConfig.mli b/infer/src/concurrency/RacerDConfig.mli index 4648cc33c..584fc0280 100644 --- a/infer/src/concurrency/RacerDConfig.mli +++ b/infer/src/concurrency/RacerDConfig.mli @@ -86,12 +86,16 @@ module Models : sig Initially, (Writer|OutputStream).(flush|close) were also matched, but this generated too many reports *) - val is_countdownlatch_await : Typ.Procname.t -> bool - (** is the method called CountDownLath.await? *) + val is_countdownlatch_await : Tenv.t -> Typ.Procname.t -> bool + (** is the method called CountDownLath.await or on subclass? *) val is_two_way_binder_transact : Tenv.t -> HilExp.t list -> Typ.Procname.t -> bool + (* 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. *) + + val is_getWindowVisibleDisplayFrame : Tenv.t -> Typ.Procname.t -> bool + (* is it a call to android.view.View.getWindowVisibleDisplayFrame or on sublass? *) end diff --git a/infer/src/concurrency/starvation.ml b/infer/src/concurrency/starvation.ml index 6814c8764..43890f71a 100644 --- a/infer/src/concurrency/starvation.ml +++ b/infer/src/concurrency/starvation.ml @@ -80,9 +80,10 @@ module TransferFunctions (CFG : ProcCfg.S) = struct astate | NoEffect -> if - Models.is_countdownlatch_await callee + Models.is_countdownlatch_await tenv callee || Models.is_two_way_binder_transact tenv actuals callee || Models.is_blocking_java_io tenv callee + || Models.is_getWindowVisibleDisplayFrame tenv callee then let caller = Procdesc.get_proc_name pdesc in Domain.blocking_call ~caller ~callee loc astate diff --git a/infer/tests/codetoanalyze/java/starvation/VisDispFrame.java b/infer/tests/codetoanalyze/java/starvation/VisDispFrame.java new file mode 100644 index 000000000..a513312fd --- /dev/null +++ b/infer/tests/codetoanalyze/java/starvation/VisDispFrame.java @@ -0,0 +1,26 @@ +/* + * Copyright (c) 2018 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +import android.view.View; +import android.graphics.Rect; +import android.support.annotation.UiThread; + +class VisDispFrame { + View view; + Rect rect; + + @UiThread + void callsGetVisibleDisplayFrameOnUiThreadBad() { + view.getWindowVisibleDisplayFrame(rect); + } + + void callsGetVisibleDisplayFrameOk() { + view.getWindowVisibleDisplayFrame(rect); + } +} diff --git a/infer/tests/codetoanalyze/java/starvation/issues.exp b/infer/tests/codetoanalyze/java/starvation/issues.exp index 86174ea10..26b526ac4 100644 --- a/infer/tests/codetoanalyze/java/starvation/issues.exp +++ b/infer/tests/codetoanalyze/java/starvation/issues.exp @@ -11,3 +11,4 @@ codetoanalyze/java/starvation/Intraproc.java, void IntraprocA.intraBad(Intraproc codetoanalyze/java/starvation/JavaIO.java, void JavaIO.fileReadBad(), 0, STARVATION, ERROR, [Method start: void JavaIO.fileReadBad(),Method call: int JavaIO.doFileRead(),calls int InputStreamReader.read() from int JavaIO.doFileRead()] codetoanalyze/java/starvation/JavaIO.java, void JavaIO.streamReadBad(), 0, STARVATION, ERROR, [Method start: void JavaIO.streamReadBad(),Method call: String JavaIO.doStreamRead(),calls String DataInputStream.readUTF() from String JavaIO.doStreamRead()] codetoanalyze/java/starvation/StaticLock.java, void StaticLock.lockOtherClassOneWayBad(), 0, STARVATION, ERROR, [[Trace 1] Method start: void StaticLock.lockOtherClassOneWayBad(),Locks StaticLock$0 in class java.lang.Class*,Locks this in class StaticLock*,[Trace 2] Locks this in class StaticLock*,Method call: void StaticLock.staticSynced(),Locks StaticLock$0 in class java.lang.Class*] +codetoanalyze/java/starvation/VisDispFrame.java, void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad(), 0, STARVATION, ERROR, [Method start: void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad(),calls void View.getWindowVisibleDisplayFrame(Rect) from void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad()]