[starvation] model legacy synchronized containers

Reviewed By: ddino

Differential Revision: D7968334

fbshipit-source-id: 3e79e3d

infer/src/concurrency/RacerDConfig.ml

@@ -685,4 +685,17 @@ module Models = struct
       ; is_asyncTask_get ]
     in
     fun tenv pn actuals -> List.exists matchers ~f:(fun matcher -> matcher tenv pn actuals)
+
+
+  let is_synchronized_library_call =
+    let targets = ["java.lang.StringBuffer"; "java.util.Hashtable"; "java.util.Vector"] in
+    fun tenv pn ->
+      not (Typ.Procname.is_constructor pn)
+      &&
+      match pn with
+      | Typ.Procname.Java java_pname ->
+          let classname = Typ.Procname.Java.get_class_type_name java_pname in
+          List.exists targets ~f:(PatternMatch.is_subtype_of_str tenv classname)
+      | _ ->
+          false
 end

infer/src/concurrency/RacerDConfig.mli

@@ -82,4 +82,8 @@ module Models : sig
 
   val may_block : Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool
   (** is the method call potentially blocking, given the actuals passed? *)
+
+  val is_synchronized_library_call : Tenv.t -> Typ.Procname.t -> bool
+  (** does the method call lock-then-unlock the underlying object?
+      legacy Java containers like Vector do this, and can interact with explicit locking *)
 end

infer/src/concurrency/starvation.ml

@@ -61,15 +61,24 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
       | _ ->
           None
     in
+    let do_lock actuals loc astate =
+      get_path actuals |> Option.value_map ~default:astate ~f:(Domain.acquire astate loc)
+    in
+    let do_unlock actuals astate =
+      get_path actuals |> Option.value_map ~default:astate ~f:(Domain.release astate)
+    in
     match instr with
     | Call (_, Direct callee, actuals, _, loc) -> (
       match Models.get_lock callee actuals with
       | Lock ->
-          get_path actuals |> Option.value_map ~default:astate ~f:(Domain.acquire astate loc)
+          do_lock actuals loc astate
       | Unlock ->
-          get_path actuals |> Option.value_map ~default:astate ~f:(Domain.release astate)
+          do_unlock actuals astate
       | LockedIfTrue ->
           astate
+      | NoEffect when Models.is_synchronized_library_call tenv callee ->
+          (* model a synchronized call without visible internal behaviour *)
+          do_lock actuals loc astate |> do_unlock actuals
       | NoEffect when Models.may_block tenv callee actuals ->
           let caller = Procdesc.get_proc_name pdesc in
           Domain.blocking_call ~caller ~callee loc astate

infer/tests/codetoanalyze/java/starvation/LegacySync.java

@@ -0,0 +1,29 @@
+/*
+ * 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.util.concurrent.Future;
+import java.util.concurrent.ExecutionException;
+import android.support.annotation.UiThread;
+import java.util.Hashtable;
+
+class LegacySync {
+  Hashtable table;
+  Future future;
+
+  void notOnUiThreadSyncedBad() throws InterruptedException, ExecutionException {
+    synchronized(table) {
+      future.get();
+    }
+  }
+
+  @UiThread
+  Object onUiThreadOpBad() {
+     return table.get("blabla");
+  }
+}

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

@@ -21,6 +21,7 @@ codetoanalyze/java/starvation/Interproc.java, void Interproc.interproc1Bad(Inter
 codetoanalyze/java/starvation/Intraproc.java, void Intraproc.intraBad(IntraprocA), 11, DEADLOCK, ERROR, [[Trace 1]  void Intraproc.intraBad(IntraprocA),locks `this` in class `Intraproc*`,locks `o` in class `IntraprocA*`,[Trace 2]  void IntraprocA.intraBad(Intraproc),locks `this` in class `IntraprocA*`,locks `o` in class `Intraproc*`]
 codetoanalyze/java/starvation/JavaIO.java, void JavaIO.fileReadBad(), 32, STARVATION, ERROR, [ void JavaIO.fileReadBad(),Method call: int JavaIO.doFileRead(),calls int InputStreamReader.read() from int JavaIO.doFileRead()]
 codetoanalyze/java/starvation/JavaIO.java, void JavaIO.streamReadBad(), 37, STARVATION, ERROR, [ void JavaIO.streamReadBad(),Method call: String JavaIO.doStreamRead(),calls String DataInputStream.readUTF() from String JavaIO.doStreamRead()]
+codetoanalyze/java/starvation/LegacySync.java, Object LegacySync.onUiThreadOpBad(), 26, STARVATION, ERROR, [[Trace 1]  Object LegacySync.onUiThreadOpBad(),locks `this.LegacySync.table` in class `LegacySync*`,[Trace 2]  void LegacySync.notOnUiThreadSyncedBad(),locks `this.LegacySync.table` in class `LegacySync*`,calls Object Future.get() from void LegacySync.notOnUiThreadSyncedBad()]
 codetoanalyze/java/starvation/ServiceOnUIThread.java, IBinder ServiceOnUIThread.onBind(Intent), 20, STARVATION, ERROR, [ IBinder ServiceOnUIThread.onBind(Intent),Method call: void ServiceOnUIThread.transactBad(),calls boolean IBinder.transact(int,Parcel,Parcel,int) from void ServiceOnUIThread.transactBad()]
 codetoanalyze/java/starvation/ServiceOnUIThread.java, void ServiceOnUIThread.transactBad(), 25, STARVATION, ERROR, [ void ServiceOnUIThread.transactBad(),calls boolean IBinder.transact(int,Parcel,Parcel,int) from void ServiceOnUIThread.transactBad()]
 codetoanalyze/java/starvation/StaticLock.java, void StaticLock.lockOtherClassOneWayBad(), 24, DEADLOCK, ERROR, [[Trace 1]  void StaticLock.lockOtherClassOneWayBad(),locks `StaticLock$0` in class `java.lang.Class*`,locks `this` in class `StaticLock*`,[Trace 2]  void StaticLock.lockOtherClassAnotherWayNad(),locks `this` in class `StaticLock*`,Method call: void StaticLock.staticSynced(),locks `StaticLock$0` in class `java.lang.Class*`]