[starvation] Catch java IO blocking calls on UI thread

Reviewed By: sblackshear

Differential Revision: D7685918

fbshipit-source-id: 0ec46f2

infer/src/concurrency/RacerDConfig.ml

@@ -540,4 +540,42 @@ module Models = struct
              false )
        && Procdesc.get_access proc_desc <> PredSymb.Private
        && not (Annotations.pdesc_return_annot_ends_with proc_desc Annotations.visibleForTesting)
+
+
+  let is_blocking_java_io 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
+          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
+    | _ ->
+        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_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
 end

infer/src/concurrency/RacerDConfig.mli

@@ -78,4 +78,20 @@ module Models : sig
   val should_report_on_proc : Procdesc.t -> Tenv.t -> bool
   (** return true if procedure is at an abstraction boundary or reporting has been explicitly
      requested via @ThreadSafe *)
+
+  val is_blocking_java_io : Tenv.t -> Typ.Procname.t -> bool
+  (** It's surprisingly difficult making any sense of the official documentation on java.io classes
+          as to whether methods may block.  We approximate these by calls in traditionally blocking
+          classes (non-channel based I/O), to methods `(Reader|InputStream).read*`.
+          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_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.
+        *)
 end

infer/src/concurrency/starvation.ml

@@ -20,32 +20,6 @@ let is_java_static pname =
       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
-
-
-(* 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 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_on_main_thread pn =
   RacerDConfig.(match Models.get_thread pn with Models.MainThread -> true | _ -> false)
 
@@ -106,8 +80,9 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
           astate
       | NoEffect ->
           if
-            is_countdownlatch_await callee_pname
+            Models.is_countdownlatch_await callee_pname
-            || is_two_way_binder_transact tenv actuals callee_pname
+            || Models.is_two_way_binder_transact tenv actuals callee_pname
+            || Models.is_blocking_java_io tenv callee_pname
           then Domain.blocking_call callee_pname loc astate
           else if is_on_main_thread callee_pname then Domain.set_on_main_thread astate
           else

infer/tests/codetoanalyze/java/starvation/JavaIO.java

@@ -0,0 +1,70 @@
+/*
+ * 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 java.io.FileReader;
+import java.io.DataInputStream;
+import java.io.FileWriter;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import android.support.annotation.UiThread;
+
+class JavaIO {
+  FileReader reader;
+  DataInputStream inputStream;
+  FileWriter writer;
+  DataOutputStream outputStream;
+
+  int doFileRead() throws IOException {
+    return reader.read();
+  }
+
+  String doStreamRead() throws IOException {
+    return inputStream.readUTF();
+  }
+
+  @UiThread
+  void fileReadBad() throws IOException {
+    doFileRead();
+  }
+
+  @UiThread
+  void streamReadBad() throws IOException {
+    doStreamRead();
+  }
+
+  @UiThread
+  void writerMethodsOk() throws IOException {
+    writer = new FileWriter("bla");
+    writer.write('a');
+    writer.append('b');
+    String enc = writer.getEncoding();
+  }
+
+  @UiThread
+  void readerMethodsOk() throws IOException {
+    reader = new FileReader("bla");
+    String enc = reader.getEncoding();
+    reader.markSupported();
+    reader.reset();
+    reader.close();
+  }
+
+  @UiThread
+  void outputStreamMethodsOk() throws IOException {
+    outputStream.write('a');
+    outputStream.size();
+  }
+
+  @UiThread
+  void inputStreamMethodsOk() throws IOException {
+    inputStream.available();
+    inputStream.reset();
+    inputStream.close();
+  }
+}

infer/tests/codetoanalyze/java/starvation/issues.exp

@@ -8,4 +8,6 @@ codetoanalyze/java/starvation/InnerClass.java, void InnerClass.outerInnerBad(Inn
 codetoanalyze/java/starvation/Interclass.java, void InterclassA.interclass2Bad(Interclass), 0, STARVATION, ERROR, [[Trace 1] Locks this in class InterclassA*,Method call: void Interclass.interclass2Bad(),Locks this in class Interclass*,[Trace 2] Locks this in class Interclass*,Method call: void InterclassA.interclass1Bad(),Locks this in class InterclassA*]
 codetoanalyze/java/starvation/Interproc.java, void Interproc.interproc1Bad(InterprocA), 0, STARVATION, ERROR, [[Trace 1] Locks this in class Interproc*,Method call: void Interproc.interproc2Bad(InterprocA),Locks b in class InterprocA*,[Trace 2] Locks this in class InterprocA*,Method call: void InterprocA.interproc2Bad(Interproc),Locks d in class Interproc*]
 codetoanalyze/java/starvation/Intraproc.java, void IntraprocA.intraBad(Intraproc), 0, STARVATION, ERROR, [[Trace 1] Method start: void IntraprocA.intraBad(Intraproc),Locks this in class IntraprocA*,Locks o in class Intraproc*,[Trace 2] Method start: void Intraproc.intraBad(IntraprocA),Locks this in class Intraproc*,Locks o in class IntraprocA*]
+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()]
+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()]
 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*]