fixes

jsinglet · jsinglet · commit 19da19057beb · 2022-07-15T16:58:12.000-04:00
diff --git a/change_notes/2022-07-13-improvements-to-lock-detection.md b/change_notes/2022-07-13-improvements-to-lock-detection.md
@@ -1,3 +1,6 @@
 - `CON53-CPP` - `DeadlockByLockingInPredefinedOrder.ql`
   - Optimized performance and expanded coverage to include cases where locking
-    order is not serialized 
+    order is not serialized 
+- `CON52-CPP` - `PreventBitFieldAccessFromMultipleThreads.ql`
+  - Fixed an issue with RAII-style locks and scope causing locks to not be
+    correctly identified. 
diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll
@@ -318,7 +318,7 @@ class RAIIStyleLock extends LockingOperation {
    */
   override predicate isLock() {
     this instanceof ConstructorCall and
-    lock = getArgument(0).getAChild() and
+    lock = getArgument(0).getAChild*() and
     // defer_locks don't cause a lock
     not exists(Expr exp |
       exp = getArgument(1) and
diff --git a/cpp/common/src/codingstandards/cpp/rules/guardaccesstobitfields/GuardAccessToBitFields.qll b/cpp/common/src/codingstandards/cpp/rules/guardaccesstobitfields/GuardAccessToBitFields.qll
@@ -42,6 +42,24 @@ ControlFlowNode getAReachableLockCFN(MutexFunctionCall mfc) {
 query predicate problems(BitFieldAccess ba, string message) {
   not isExcluded(ba, getQuery()) and
   ba instanceof ThreadedCFN and
-  not ba instanceof LockProtectedControlFlowNode and
+  // to be a valid bit field access there must be
+  // a RAII-style lock before this access
+  not exists(RAIIStyleLock lock |
+    // A lock came before this node
+    lock = ba.getAPredecessor*() and
+    lock.isLock() and
+    // But wasn't followed by an unlock
+    not exists(RAIIStyleLock unlock |
+      // That worked on the same underlying lock variable
+      unlock.isUnlock() and
+      unlock.getLock() = lock.getLock() and
+      // such that the unlock came after the lock
+      unlock.getAPredecessor*() = lock and
+      // and after before the access
+      ba.getAPredecessor*() = unlock
+    )
+  ) and
+  // or the bit field access must be protected by a lock region
+  not exists(MutexFunctionCall mfc | ba = getAReachableLockCFN(mfc)) and
   message = "Access to a bit-field without a concurrency guard."
 }
diff --git a/cpp/common/test/rules/guardaccesstobitfields/GuardAccessToBitFields.expected b/cpp/common/test/rules/guardaccesstobitfields/GuardAccessToBitFields.expected
@@ -1,3 +1,3 @@
 | test.cpp:67:7:67:8 | f2 | Access to a bit-field without a concurrency guard. |
 | test.cpp:91:7:91:8 | f2 | Access to a bit-field without a concurrency guard. |
-| test.cpp:97:7:97:8 | f2 | Access to a bit-field without a concurrency guard. |
+| test.cpp:102:7:102:8 | f2 | Access to a bit-field without a concurrency guard. |
