Merge pull request #339 from github/jsinglet/lcartey-338-fix

Fix for `PreventDeadlockByLockingInPredefinedOrder`

Author: lcartey

cpp/common/src/codingstandards/cpp/Concurrency.qll

@@ -114,7 +114,10 @@ class CPPMutexFunctionCall extends MutexFunctionCall {
   /**
    * Holds if this `CPPMutexFunctionCall` is a lock.
    */
-  override predicate isLock() { getTarget().getName() = "lock" }
+  override predicate isLock() {
+    not isLockingOperationWithinLockingOperation(this) and
+    getTarget().getName() = "lock"
+  }
 
   /**
    * Holds if this `CPPMutexFunctionCall` is a speculative lock, defined as calling
@@ -172,6 +175,7 @@ class CMutexFunctionCall extends MutexFunctionCall {
    * Holds if this `CMutexFunctionCall` is a lock.
    */
   override predicate isLock() {
+    not isLockingOperationWithinLockingOperation(this) and
     getTarget().getName() = ["mtx_lock", "mtx_timedlock", "mtx_trylock"]
   }
 
@@ -296,6 +300,16 @@ abstract class LockingOperation extends FunctionCall {
    * Holds if this is an unlock operation
    */
   abstract predicate isUnlock();
+
+  /**
+   * Holds if this locking operation is really a locking operation within a
+   * designated locking operation. This library assumes the underlying locking
+   * operations are implemented correctly in that calling a `LockingOperation`
+   * results in the creation of a singular lock.
+   */
+  predicate isLockingOperationWithinLockingOperation(LockingOperation inner) {
+    exists(LockingOperation outer | outer.getTarget() = inner.getEnclosingFunction())
+  }
 }
 
 /**
@@ -317,6 +331,7 @@ class RAIIStyleLock extends LockingOperation {
    * Holds if this is a lock operation
    */
   override predicate isLock() {
+    not isLockingOperationWithinLockingOperation(this) and
     this instanceof ConstructorCall and
     lock = getArgument(0).getAChild*() and
     // defer_locks don't cause a lock

cpp/common/src/codingstandards/cpp/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.qll

@@ -24,7 +24,11 @@ predicate getAnOrderedLockPair(
   lock1 = node.coveredByLock() and
   lock2 = node.coveredByLock() and
   not lock1 = lock2 and
-  lock1.getEnclosingFunction() = lock2.getEnclosingFunction() and
+  exists(Function f |
+    lock1.getEnclosingFunction() = f and
+    lock2.getEnclosingFunction() = f and
+    node.getBasicBlock().getEnclosingFunction() = f
+  ) and
   exists(Location l1Loc, Location l2Loc |
     l1Loc = lock1.getLocation() and
     l2Loc = lock2.getLocation()