[starvation] add tests documenting path sensitivity FPs and FNs

Summary:
Starvation is currently path insensitive.  Two special cases of sensitivity cover a large range of useful cases:
- sensitivity on whether the current thread is a UI/background thread;
- sensitivity on whether a lock can be acquired (without blocking) or not.

We add a few tests capturing some of the false positives and negatives of the current analysis.

Reviewed By: mityal

Differential Revision: D17907492

fbshipit-source-id: fbce896ac

infer/tests/codetoanalyze/java/starvation/LockSensitivity.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// Tests documenting FPs and FNs due to lack of sensitivity in starvation analysis
+
+import java.util.concurrent.locks.Lock;
+
+class LockSensitivity {
+  Lock lockA, lockB;
+
+  // In the following two methods, AB vs BA deadlock pattern
+  // remains undetected since one of the locks happens via `tryLock` and result check.
+
+  public void FN_tryLockDeadlockAB_Bad() {
+    boolean locked = lockA.tryLock();
+    if (locked) {
+      lockB.lock();
+      lockB.unlock();
+      lockA.unlock();
+    } else {
+    }
+  }
+
+  public void FN_tryLockDeadlockBA_Bad() {
+    lockB.lock();
+    lockA.lock(); // deadlock: `lockA` may be locked via `tryLock()` above
+    lockA.unlock();
+    lockB.unlock();
+  }
+
+  // Asserting a lock is held is not the same as taking it wrt deadlocks.
+  // In the following two methods, AB vs BA pattern is wrongly detected.
+
+  Object monitorA, monitorB;
+
+  public void FP_assertHoldsLockAB_Ok() {
+    OurThreadUtils.assertHoldsLock(monitorA);
+    OurThreadUtils.assertHoldsLock(monitorB);
+  }
+
+  public void FP_assertHoldsLockBA_Ok() {
+    OurThreadUtils.assertHoldsLock(monitorB);
+    OurThreadUtils.assertHoldsLock(monitorA);
+  }
+}

infer/tests/codetoanalyze/java/starvation/OurThreadUtils.java

@@ -8,7 +8,13 @@
 class OurThreadUtils {
   static native boolean isMainThread();
 
+  static native boolean isUiThread();
+
   static void assertMainThread() {}
 
+  static void assertOnUiThread() {}
+
+  static void assertOnBackgroundThread() {}
+
   static void assertHoldsLock(Object lock) {}
 }

infer/tests/codetoanalyze/java/starvation/ThreadSensitivity.java

@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// Examples where the code checks what thread it's currently running on.
+class ThreadSensitivity {
+  Object monitorA, monitorB;
+
+  void FN_conditionalAssertMainThread_Bad(boolean b) {
+    if (b) {
+      // this branch asserts on Main thread
+      OurThreadUtils.assertMainThread();
+      synchronized (monitorA) {
+        synchronized (monitorB) {
+        }
+      }
+    } else {
+      // this branch asserts nothing, so may run in parallel with the other branch
+      synchronized (monitorB) {
+        synchronized (monitorA) {
+        }
+      }
+    }
+  }
+
+  Object monitorC, monitorD;
+
+  // the branches in the following two methods are both on main/UI thread so cannot deadlock
+
+  void FP_conditionalIsMainThread_Ok() {
+    if (OurThreadUtils.isMainThread()) {
+      synchronized (monitorC) {
+        synchronized (monitorD) {
+        }
+      }
+    }
+  }
+
+  void FP_conditionalIsUiThread_Ok() {
+    if (OurThreadUtils.isUiThread()) {
+      synchronized (monitorD) {
+        synchronized (monitorC) {
+        }
+      }
+    }
+  }
+
+  Object monitorE, monitorF;
+  // identical to the first case above but negated
+  void conditionalNegatedIsMainThread_Bad() {
+    if (!OurThreadUtils.isMainThread()) {
+      synchronized (monitorE) {
+        synchronized (monitorF) {
+        }
+      }
+    } else {
+      synchronized (monitorF) {
+        synchronized (monitorE) {
+        }
+      }
+    }
+  }
+
+  Object monitorG, monitorH;
+
+  public void FN_confusedAssertBad(boolean b, boolean c) {
+    if (b) {
+      OurThreadUtils.assertOnBackgroundThread();
+    } else {
+      OurThreadUtils.assertOnUiThread();
+    }
+
+    // not sure if we're on UI or background, should report
+    if (c) {
+      synchronized (monitorG) {
+        synchronized (monitorH) {
+        }
+      }
+    } else {
+      synchronized (monitorH) {
+        synchronized (monitorG) {
+        }
+      }
+    }
+  }
+}

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

@@ -20,6 +20,8 @@ codetoanalyze/java/starvation/InnerClass.java, InnerClass.outerInnerBad(InnerCla
 codetoanalyze/java/starvation/Interclass.java, Interclass.interclass1Bad(InterclassA):void, 10, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Interclass.interclass1Bad(InterclassA)`, locks `this` in `class Interclass`,Method call: `void InterclassA.interclass1Bad()`, locks `this` in `class InterclassA`,[Trace 2] `void InterclassA.interclass2Bad(Interclass)`, locks `this` in `class InterclassA`,Method call: `void Interclass.interclass2Bad()`, locks `this` in `class Interclass`]
 codetoanalyze/java/starvation/Interclass.java, InterclassA.interclass2Bad(Interclass):void, 37, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void InterclassA.interclass2Bad(Interclass)`, locks `this` in `class InterclassA`,Method call: `void Interclass.interclass2Bad()`, locks `this` in `class Interclass`,[Trace 2] `void Interclass.interclass1Bad(InterclassA)`, locks `this` in `class Interclass`,Method call: `void InterclassA.interclass1Bad()`, locks `this` in `class InterclassA`]
 codetoanalyze/java/starvation/LegacySync.java, LegacySync.onUiThreadOpBad():java.lang.Object, 25, STARVATION, no_bucket, ERROR, [[Trace 1] `Object LegacySync.onUiThreadOpBad()`, locks `this.table` in `class LegacySync`,[Trace 2] `void LegacySync.notOnUiThreadSyncedBad()`, locks `this.table` in `class LegacySync`,calls `Object Future.get()`,[Trace 1 on UI thread] `Object LegacySync.onUiThreadOpBad()`,`Object LegacySync.onUiThreadOpBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/LockSensitivity.java, LockSensitivity.FP_assertHoldsLockAB_Ok():void, 41, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void LockSensitivity.FP_assertHoldsLockAB_Ok()`, locks `this.monitorA` in `class LockSensitivity`, locks `this.monitorB` in `class LockSensitivity`,[Trace 2] `void LockSensitivity.FP_assertHoldsLockBA_Ok()`, locks `this.monitorB` in `class LockSensitivity`, locks `this.monitorA` in `class LockSensitivity`]
+codetoanalyze/java/starvation/LockSensitivity.java, LockSensitivity.FP_assertHoldsLockBA_Ok():void, 46, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void LockSensitivity.FP_assertHoldsLockBA_Ok()`, locks `this.monitorB` in `class LockSensitivity`, locks `this.monitorA` in `class LockSensitivity`,[Trace 2] `void LockSensitivity.FP_assertHoldsLockAB_Ok()`, locks `this.monitorA` in `class LockSensitivity`, locks `this.monitorB` in `class LockSensitivity`]
 codetoanalyze/java/starvation/LocklessTests.java, LocklessTestsA.locklessMethod():void, 23, LOCKLESS_VIOLATION, no_bucket, ERROR, [`void LocklessTestsA.locklessMethod()`, locks `this` in `class LocklessTestsA`]
 codetoanalyze/java/starvation/LocklessTests.java, LocklessTestsB.locklessMethod():void, 39, LOCKLESS_VIOLATION, no_bucket, ERROR, [`void LocklessTestsB.locklessMethod()`, locks `this` in `class LocklessTestsB`]
 codetoanalyze/java/starvation/LocklessTests.java, LocklessTestsC.locklessMethod():void, 52, LOCKLESS_VIOLATION, no_bucket, ERROR, [`void LocklessTestsC.locklessMethod()`,Method call: `void LocklessTestsC.takeLock()`, locks `this` in `class LocklessTestsC`]
@@ -65,6 +67,10 @@ codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.viol
 codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 38, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.setReadOnly()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
 codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 39, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.setWritable(boolean)`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
 codetoanalyze/java/starvation/SuppLint.java, SuppLint.onUiThreadBad():void, 25, STARVATION, no_bucket, ERROR, [`void SuppLint.onUiThreadBad()`,calls `Object Future.get()`,[Trace on UI thread] `void SuppLint.onUiThreadBad()`,`void SuppLint.onUiThreadBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/ThreadSensitivity.java, ThreadSensitivity.FP_conditionalIsMainThread_Ok():void, 35, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void ThreadSensitivity.FP_conditionalIsMainThread_Ok()`, locks `this.monitorC` in `class ThreadSensitivity`, locks `this.monitorD` in `class ThreadSensitivity`,[Trace 2] `void ThreadSensitivity.FP_conditionalIsUiThread_Ok()`, locks `this.monitorD` in `class ThreadSensitivity`, locks `this.monitorC` in `class ThreadSensitivity`]
+codetoanalyze/java/starvation/ThreadSensitivity.java, ThreadSensitivity.FP_conditionalIsUiThread_Ok():void, 44, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void ThreadSensitivity.FP_conditionalIsUiThread_Ok()`, locks `this.monitorD` in `class ThreadSensitivity`, locks `this.monitorC` in `class ThreadSensitivity`,[Trace 2] `void ThreadSensitivity.FP_conditionalIsMainThread_Ok()`, locks `this.monitorC` in `class ThreadSensitivity`, locks `this.monitorD` in `class ThreadSensitivity`]
+codetoanalyze/java/starvation/ThreadSensitivity.java, ThreadSensitivity.conditionalNegatedIsMainThread_Bad():void, 55, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void ThreadSensitivity.conditionalNegatedIsMainThread_Bad()`, locks `this.monitorE` in `class ThreadSensitivity`, locks `this.monitorF` in `class ThreadSensitivity`,[Trace 2] `void ThreadSensitivity.conditionalNegatedIsMainThread_Bad()`, locks `this.monitorF` in `class ThreadSensitivity`, locks `this.monitorE` in `class ThreadSensitivity`]
+codetoanalyze/java/starvation/ThreadSensitivity.java, ThreadSensitivity.conditionalNegatedIsMainThread_Bad():void, 60, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void ThreadSensitivity.conditionalNegatedIsMainThread_Bad()`, locks `this.monitorF` in `class ThreadSensitivity`, locks `this.monitorE` in `class ThreadSensitivity`,[Trace 2] `void ThreadSensitivity.conditionalNegatedIsMainThread_Bad()`, locks `this.monitorE` in `class ThreadSensitivity`, locks `this.monitorF` in `class ThreadSensitivity`]
 codetoanalyze/java/starvation/ThreadSleep.java, ThreadSleep.indirectSleepOnUIThreadBad():void, 24, STARVATION, no_bucket, ERROR, [[Trace 1] `void ThreadSleep.indirectSleepOnUIThreadBad()`, locks `this.lock` in `class ThreadSleep`,[Trace 2] `void ThreadSleep.lockAndSleepOnNonUIThread()`, locks `this.lock` in `class ThreadSleep`,Method call: `void ThreadSleep.sleepOnAnyThreadOk()`,calls `void Thread.sleep(long)`,[Trace 1 on UI thread] `void ThreadSleep.indirectSleepOnUIThreadBad()`,`void ThreadSleep.indirectSleepOnUIThreadBad()` is annotated `UiThread`]
 codetoanalyze/java/starvation/ThreadSleep.java, ThreadSleep.sleepOnUIThreadBad():void, 17, STARVATION, no_bucket, ERROR, [`void ThreadSleep.sleepOnUIThreadBad()`,calls `void Thread.sleep(long)`,[Trace on UI thread] `void ThreadSleep.sleepOnUIThreadBad()`,`void ThreadSleep.sleepOnUIThreadBad()` is annotated `UiThread`]
 codetoanalyze/java/starvation/UIDeadlock.java, UIDeadlock.notOnUIThreadBad():void, 34, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UIDeadlock.notOnUIThreadBad()`, locks `this.lockB` in `class UIDeadlock`, locks `this` in `class UIDeadlock`,[Trace 2] `void UIDeadlock.onUIThreadBad()`, locks `this` in `class UIDeadlock`, locks `this.lockB` in `class UIDeadlock`]