From 53eda0f89c090c14e6ce341688cedd8f977d6db6 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 15:10:36 -0400
Subject: [PATCH 01/25] rules import

---
 rules.csv | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/rules.csv b/rules.csv
index 891cbefd8d..8662b29d3d 100755
--- a/rules.csv
+++ b/rules.csv
@@ -485,18 +485,18 @@ c,CERT-C,ARR36-C,Yes,Rule,,,Do not subtract or compare two pointers that do not
 c,CERT-C,ARR37-C,Yes,Rule,,,Do not add or subtract an integer to a pointer to a non-array object,,InvalidMemory,Medium,
 c,CERT-C,ARR38-C,Yes,Rule,,,Guarantee that library functions do not form invalid pointers,,Pointers2,Very Hard,
 c,CERT-C,ARR39-C,Yes,Rule,,,Do not add or subtract a scaled integer to a pointer,,Pointers2,Medium,
-c,CERT-C,CON30-C,Yes,Rule,,,Clean up thread-specific storage,,Concurrency2,Very Hard,
+c,CERT-C,CON30-C,Yes,Rule,,,Clean up thread-specific storage,,Concurrency3,Very Hard,
 c,CERT-C,CON31-C,Yes,Rule,,,Do not destroy a mutex while it is locked,CON50-CPP,Concurrency2,Very Hard,
 c,CERT-C,CON32-C,Yes,Rule,,,Prevent data races when accessing bit-fields from multiple threads,,Concurrency1,Easy,
 c,CERT-C,CON33-C,Yes,Rule,,,Avoid race conditions when using library functions,,Concurrency1,Easy,
-c,CERT-C,CON34-C,Yes,Rule,,,Declare objects shared between threads with appropriate storage durations,,Concurrency2,Hard,
+c,CERT-C,CON34-C,Yes,Rule,,,Declare objects shared between threads with appropriate storage durations,,Concurrency3,Hard,
 c,CERT-C,CON35-C,Yes,Rule,,,Avoid deadlock by locking in a predefined order,CON53-CPP,Concurrency2,Medium,
 c,CERT-C,CON36-C,Yes,Rule,,,Wrap functions that can spuriously wake up in a loop,CON54-CPP,Concurrency2,Medium,
 c,CERT-C,CON37-C,Yes,Rule,,,Do not call signal() in a multithreaded program,,Concurrency1,Easy,
-c,CERT-C,CON38-C,Yes,Rule,,,Preserve thread safety and liveness when using condition variables,CON55-CPP,Concurrency2,Medium,
-c,CERT-C,CON39-C,Yes,Rule,,,Do not join or detach a thread that was previously joined or detached,,Concurrency2,Hard,
-c,CERT-C,CON40-C,Yes,Rule,,,Do not refer to an atomic variable twice in an expression,,Concurrency2,Medium,
-c,CERT-C,CON41-C,Yes,Rule,,,Wrap functions that can fail spuriously in a loop,CON53-CPP,Concurrency2,Medium,
+c,CERT-C,CON38-C,Yes,Rule,,,Preserve thread safety and liveness when using condition variables,CON55-CPP,Concurrency3,Medium,
+c,CERT-C,CON39-C,Yes,Rule,,,Do not join or detach a thread that was previously joined or detached,,Concurrency3,Hard,
+c,CERT-C,CON40-C,Yes,Rule,,,Do not refer to an atomic variable twice in an expression,,Concurrency3,Medium,
+c,CERT-C,CON41-C,Yes,Rule,,,Wrap functions that can fail spuriously in a loop,CON53-CPP,Concurrency3,Medium,
 c,CERT-C,CON43-C,OutOfScope,Rule,,,Do not allow data races in multithreaded code,,,,
 c,CERT-C,DCL30-C,Yes,Rule,,,Declare objects with appropriate storage durations,,Declarations,Hard,
 c,CERT-C,DCL31-C,Yes,Rule,,,Declare identifiers before using them,,Declarations,Medium,

From 69a25ddb57ffa8aee2a0340d55085f834078d57c Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 15:24:27 -0400
Subject: [PATCH 02/25] fix

---
 .vscode/tasks.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.vscode/tasks.json b/.vscode/tasks.json
index 9de0bd3ae7..5aba3a984c 100644
--- a/.vscode/tasks.json
+++ b/.vscode/tasks.json
@@ -223,7 +223,7 @@
                 "Preprocessor2",
                 "IntegerConversion",
                 "Expressions",
-                "DeadCode"
+                "DeadCode",
                 "VirtualFunctions"
             ]
         },

From 920a2a4273a782fd7b6b9cfec17933b57db6b603 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 15:24:49 -0400
Subject: [PATCH 03/25] packages

---
 .../cpp/exclusions/c/Concurrency2.qll         | 58 +++++++++++++++++
 .../cpp/exclusions/c/RuleMetadata.qll         |  3 +
 rule_packages/c/Concurrency2.json             | 64 +++++++++++++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency2.qll
 create mode 100644 rule_packages/c/Concurrency2.json

diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency2.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency2.qll
new file mode 100644
index 0000000000..4776537ab3
--- /dev/null
+++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency2.qll
@@ -0,0 +1,58 @@
+//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY.  **/
+import cpp
+import RuleMetadata
+import codingstandards.cpp.exclusions.RuleMetadata
+
+newtype Concurrency2Query =
+  TDoNotDestroyAMutexWhileItIsLockedQuery() or
+  TDeadlockByLockingInPredefinedOrderQuery() or
+  TWrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery()
+
+predicate isConcurrency2QueryMetadata(Query query, string queryId, string ruleId) {
+  query =
+    // `Query` instance for the `doNotDestroyAMutexWhileItIsLocked` query
+    Concurrency2Package::doNotDestroyAMutexWhileItIsLockedQuery() and
+  queryId =
+    // `@id` for the `doNotDestroyAMutexWhileItIsLocked` query
+    "c/cert/do-not-destroy-a-mutex-while-it-is-locked" and
+  ruleId = "CON31-C"
+  or
+  query =
+    // `Query` instance for the `deadlockByLockingInPredefinedOrder` query
+    Concurrency2Package::deadlockByLockingInPredefinedOrderQuery() and
+  queryId =
+    // `@id` for the `deadlockByLockingInPredefinedOrder` query
+    "c/cert/deadlock-by-locking-in-predefined-order" and
+  ruleId = "CON35-C"
+  or
+  query =
+    // `Query` instance for the `wrapFunctionsThatCanSpuriouslyWakeUpInLoop` query
+    Concurrency2Package::wrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery() and
+  queryId =
+    // `@id` for the `wrapFunctionsThatCanSpuriouslyWakeUpInLoop` query
+    "c/cert/wrap-functions-that-can-spuriously-wake-up-in-loop" and
+  ruleId = "CON36-C"
+}
+
+module Concurrency2Package {
+  Query doNotDestroyAMutexWhileItIsLockedQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `doNotDestroyAMutexWhileItIsLocked` query
+      TQueryC(TConcurrency2PackageQuery(TDoNotDestroyAMutexWhileItIsLockedQuery()))
+  }
+
+  Query deadlockByLockingInPredefinedOrderQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `deadlockByLockingInPredefinedOrder` query
+      TQueryC(TConcurrency2PackageQuery(TDeadlockByLockingInPredefinedOrderQuery()))
+  }
+
+  Query wrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `wrapFunctionsThatCanSpuriouslyWakeUpInLoop` query
+      TQueryC(TConcurrency2PackageQuery(TWrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery()))
+  }
+}
diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll
index 321a86d7bd..946f498b47 100644
--- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll
+++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll
@@ -3,6 +3,7 @@ import cpp
 import codingstandards.cpp.exclusions.RuleMetadata
 //** Import packages for this language **/
 import Concurrency1
+import Concurrency2
 import IO1
 import IO2
 import IO3
@@ -21,6 +22,7 @@ import Syntax
 /** The TQuery type representing this language * */
 newtype TCQuery =
   TConcurrency1PackageQuery(Concurrency1Query q) or
+  TConcurrency2PackageQuery(Concurrency2Query q) or
   TIO1PackageQuery(IO1Query q) or
   TIO2PackageQuery(IO2Query q) or
   TIO3PackageQuery(IO3Query q) or
@@ -39,6 +41,7 @@ newtype TCQuery =
 /** The metadata predicate * */
 predicate isQueryMetadata(Query query, string queryId, string ruleId) {
   isConcurrency1QueryMetadata(query, queryId, ruleId) or
+  isConcurrency2QueryMetadata(query, queryId, ruleId) or
   isIO1QueryMetadata(query, queryId, ruleId) or
   isIO2QueryMetadata(query, queryId, ruleId) or
   isIO3QueryMetadata(query, queryId, ruleId) or
diff --git a/rule_packages/c/Concurrency2.json b/rule_packages/c/Concurrency2.json
new file mode 100644
index 0000000000..159e8ec3a3
--- /dev/null
+++ b/rule_packages/c/Concurrency2.json
@@ -0,0 +1,64 @@
+{
+  "CERT-C": {
+    "CON31-C": {
+      "properties": {
+        "obligation": "rule"
+      },
+      "queries": [
+        {
+          "description": "Allowing a mutex to go out of scope while it is locked removes protections around shared resources.",
+          "kind": "problem",
+          "name": "Do not destroy a mutex while it is locked",
+          "precision": "medium",
+          "severity": "error",
+          "short_name": "DoNotDestroyAMutexWhileItIsLocked",
+          "tags": [
+            "correctness",
+            "concurrency"
+          ]
+        }
+      ],
+      "title": "Do not destroy a mutex while it is locked"
+    },
+    "CON35-C": {
+      "properties": {
+        "obligation": "rule"
+      },
+      "queries": [
+        {
+          "description": "Circular waits leading to thread deadlocks may be avoided by locking in a predefined order.",
+          "kind": "problem",
+          "name": "Avoid deadlock by locking in a predefined order",
+          "precision": "very-high",
+          "severity": "error",
+          "short_name": "DeadlockByLockingInPredefinedOrder",
+          "tags": [
+            "correctness",
+            "concurrency"
+          ]
+        }
+      ],
+      "title": "Avoid deadlock by locking in a predefined order"
+    },
+    "CON36-C": {
+      "properties": {
+        "obligation": "rule"
+      },
+      "queries": [
+        {
+          "description": "Not wrapping functions that can wake up spuriously in a conditioned loop can result race conditions.",
+          "kind": "problem",
+          "name": "Wrap functions that can spuriously wake up in a loop",
+          "precision": "very-high",
+          "severity": "error",
+          "short_name": "WrapFunctionsThatCanSpuriouslyWakeUpInLoop",
+          "tags": [
+            "correctness",
+            "concurrency"
+          ]
+        }
+      ],
+      "title": "Wrap functions that can spuriously wake up in a loop"
+    }
+  }
+}
\ No newline at end of file

From baeb6c544d3eb0c5c4950fd4b746af855d138f2e Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 16:39:06 -0400
Subject: [PATCH 04/25] shared

---
 rule_packages/c/Concurrency2.json  | 1 +
 rule_packages/cpp/Concurrency.json | 1 +
 2 files changed, 2 insertions(+)

diff --git a/rule_packages/c/Concurrency2.json b/rule_packages/c/Concurrency2.json
index 159e8ec3a3..5ff80a4d85 100644
--- a/rule_packages/c/Concurrency2.json
+++ b/rule_packages/c/Concurrency2.json
@@ -52,6 +52,7 @@
           "precision": "very-high",
           "severity": "error",
           "short_name": "WrapFunctionsThatCanSpuriouslyWakeUpInLoop",
+          "shared_implementation_short_name": "WrapSpuriousFunctionInLoop",
           "tags": [
             "correctness",
             "concurrency"
diff --git a/rule_packages/cpp/Concurrency.json b/rule_packages/cpp/Concurrency.json
index c79c7dc777..61008a6142 100644
--- a/rule_packages/cpp/Concurrency.json
+++ b/rule_packages/cpp/Concurrency.json
@@ -105,6 +105,7 @@
           "precision": "very-high",
           "severity": "error",
           "short_name": "WrapFunctionsThatCanSpuriouslyWakeUpInLoop",
+          "shared_implementation_short_name": "WrapSpuriousFunctionInLoop",
           "tags": [
             "correctness",
             "concurrency"

From 546fc3b05c0d42d8ac01df4a3a1d074a0bff8a2a Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 16:39:46 -0400
Subject: [PATCH 05/25] refactoring

---
 .../src/codingstandards/cpp/Concurrency.qll   | 17 ++++-
 .../WrapSpuriousFunctionInLoop.qll            | 19 ++++++
 .../WrapSpuriousFunctionInLoop.expected       |  3 +
 .../WrapSpuriousFunctionInLoop.ql             |  2 +
 .../rules/wrapspuriousfunctioninloop/test.cpp | 62 +++++++++++++++++++
 5 files changed, 101 insertions(+), 2 deletions(-)
 create mode 100644 cpp/common/src/codingstandards/cpp/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.qll
 create mode 100644 cpp/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.expected
 create mode 100644 cpp/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql
 create mode 100644 cpp/common/test/rules/wrapspuriousfunctioninloop/test.cpp

diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll
index a7234d601e..fe6c46c60d 100644
--- a/cpp/common/src/codingstandards/cpp/Concurrency.qll
+++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll
@@ -421,8 +421,12 @@ class LockProtectedControlFlowNode extends ThreadedCFN {
 /**
  * Models a function that conditionally waits.
  */
-class ConditionalWait extends FunctionCall {
-  ConditionalWait() {
+abstract class ConditionalWait extends FunctionCall {}
+/**
+ * Models a function in CPP that will conditionally wait. 
+ */
+class CPPConditionalWait extends ConditionalWait {
+  CPPConditionalWait() {
     exists(MemberFunction mf |
       mf = getTarget() and
       mf.getDeclaringType().hasQualifiedName("std", "condition_variable") and
@@ -431,6 +435,15 @@ class ConditionalWait extends FunctionCall {
   }
 }
 
+/**
+ * Models a function in C that will conditionally wait. 
+ */
+class CConditionalWait extends ConditionalWait {
+  CConditionalWait() {
+    getTarget().getName() in ["cnd_wait"]
+  }
+}
+
 /**
  * Models a call to a `std::thread` constructor that depends on a mutex.
  */
diff --git a/cpp/common/src/codingstandards/cpp/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.qll b/cpp/common/src/codingstandards/cpp/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.qll
new file mode 100644
index 0000000000..99bdbeee5d
--- /dev/null
+++ b/cpp/common/src/codingstandards/cpp/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.qll
@@ -0,0 +1,19 @@
+/**
+ * Provides a library which includes a `problems` predicate for reporting
+ * functions that should be wrapped in a loop because they may wake up spuriously.
+ */
+
+import cpp
+import codingstandards.cpp.Customizations
+import codingstandards.cpp.Exclusions
+import codingstandards.cpp.Concurrency
+
+abstract class WrapSpuriousFunctionInLoopSharedQuery extends Query { }
+
+Query getQuery() { result instanceof WrapSpuriousFunctionInLoopSharedQuery }
+
+query predicate problems(ConditionalWait cw, string message) {
+  not isExcluded(cw, getQuery()) and
+  not cw.getEnclosingStmt().getParentStmt*() instanceof Loop and
+  message = "Use of a function that may wake up spuriously without a controlling loop."
+}
diff --git a/cpp/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.expected b/cpp/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.expected
new file mode 100644
index 0000000000..218612ef21
--- /dev/null
+++ b/cpp/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.expected
@@ -0,0 +1,3 @@
+| test.cpp:11:8:11:11 | call to wait | Use of a function that may wake up spuriously without a controlling loop. |
+| test.cpp:49:6:49:9 | call to wait | Use of a function that may wake up spuriously without a controlling loop. |
+| test.cpp:59:8:59:11 | call to wait | Use of a function that may wake up spuriously without a controlling loop. |
diff --git a/cpp/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql b/cpp/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql
new file mode 100644
index 0000000000..6f4ad4c40e
--- /dev/null
+++ b/cpp/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql
@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.wrapspuriousfunctioninloop.WrapSpuriousFunctionInLoop
diff --git a/cpp/common/test/rules/wrapspuriousfunctioninloop/test.cpp b/cpp/common/test/rules/wrapspuriousfunctioninloop/test.cpp
new file mode 100644
index 0000000000..29c5a217d0
--- /dev/null
+++ b/cpp/common/test/rules/wrapspuriousfunctioninloop/test.cpp
@@ -0,0 +1,62 @@
+#include <condition_variable>
+#include <mutex>
+
+static std::mutex mu;
+static std::condition_variable cv;
+
+void f1() {
+  std::unique_lock<std::mutex> lk(mu);
+
+  if (1) {
+    cv.wait(lk); // NON_COMPLIANT
+  }
+}
+
+void f2() {
+  std::unique_lock<std::mutex> lk(mu);
+  int i = 2;
+  while (i > 0) {
+    cv.wait(lk); // COMPLIANT
+    i--;
+  }
+}
+
+void f3() {
+  std::unique_lock<std::mutex> lk(mu);
+  int i = 2;
+  do {
+    cv.wait(lk); // COMPLIANT
+    i--;
+  } while (i > 0);
+}
+
+void f4() {
+  std::unique_lock<std::mutex> lk(mu);
+
+  for (;;) {
+    cv.wait(lk); // COMPLIANT
+  }
+}
+
+void f5() {
+  std::unique_lock<std::mutex> lk(mu);
+
+  int i = 2;
+  while (i > 0) {
+    i--;
+  }
+
+  cv.wait(lk); // NON_COMPLIANT
+}
+
+void f6() {
+  std::unique_lock<std::mutex> lk(mu);
+
+  for (int i = 0; i < 10; i++) {
+  }
+  int i = 0;
+  if (i > 0) {
+    cv.wait(lk); // NON_COMPLIANT
+    i--;
+  }
+}

From a6cb0a8ae8178672db92afaadca6c63bf058800f Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 16:40:46 -0400
Subject: [PATCH 06/25] test ref

---
 .../CON54-CPP/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 cpp/cert/test/rules/CON54-CPP/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref

diff --git a/cpp/cert/test/rules/CON54-CPP/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref b/cpp/cert/test/rules/CON54-CPP/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref
new file mode 100644
index 0000000000..dff5c1b232
--- /dev/null
+++ b/cpp/cert/test/rules/CON54-CPP/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref
@@ -0,0 +1 @@
+cpp/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql
\ No newline at end of file

From ede86dc3a9bc2de97be0916d7ae603975d8d73c9 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 16:41:08 -0400
Subject: [PATCH 07/25] refactor

---
 .../WrapFunctionsThatCanSpuriouslyWakeUpInLoop.ql    | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/cpp/cert/src/rules/CON54-CPP/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.ql b/cpp/cert/src/rules/CON54-CPP/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.ql
index 25d5123815..5584b7bec2 100644
--- a/cpp/cert/src/rules/CON54-CPP/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.ql
+++ b/cpp/cert/src/rules/CON54-CPP/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.ql
@@ -14,10 +14,10 @@
 
 import cpp
 import codingstandards.cpp.cert
-import codingstandards.cpp.Concurrency
+import codingstandards.cpp.rules.wrapspuriousfunctioninloop.WrapSpuriousFunctionInLoop
 
-from ConditionalWait cw
-where
-  not isExcluded(cw, ConcurrencyPackage::wrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery()) and
-  not cw.getEnclosingStmt().getParentStmt*() instanceof Loop
-select cw, "Use of a function that may wake up spuriously without a controlling loop."
+class WrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery extends WrapSpuriousFunctionInLoopSharedQuery {
+  WrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery() {
+    this = ConcurrencyPackage::wrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery()
+  }
+}

From 64307b15114f6bb415ac53c073518ef62b165a83 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 16:41:42 -0400
Subject: [PATCH 08/25] test

---
 .../WrapSpuriousFunctionInLoop.expected       |  3 +
 .../WrapSpuriousFunctionInLoop.ql             |  2 +
 .../rules/wrapspuriousfunctioninloop/test.c   | 62 +++++++++++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.expected
 create mode 100644 c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql
 create mode 100644 c/common/test/rules/wrapspuriousfunctioninloop/test.c

diff --git a/c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.expected b/c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.expected
new file mode 100644
index 0000000000..c38811788f
--- /dev/null
+++ b/c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.expected
@@ -0,0 +1,3 @@
+| test.c:11:5:11:12 | call to cnd_wait | Use of a function that may wake up spuriously without a controlling loop. |
+| test.c:49:3:49:10 | call to cnd_wait | Use of a function that may wake up spuriously without a controlling loop. |
+| test.c:59:5:59:12 | call to cnd_wait | Use of a function that may wake up spuriously without a controlling loop. |
diff --git a/c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql b/c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql
new file mode 100644
index 0000000000..6f4ad4c40e
--- /dev/null
+++ b/c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql
@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.wrapspuriousfunctioninloop.WrapSpuriousFunctionInLoop
diff --git a/c/common/test/rules/wrapspuriousfunctioninloop/test.c b/c/common/test/rules/wrapspuriousfunctioninloop/test.c
new file mode 100644
index 0000000000..84484364fd
--- /dev/null
+++ b/c/common/test/rules/wrapspuriousfunctioninloop/test.c
@@ -0,0 +1,62 @@
+#include <stddef.h>
+#include <threads.h>
+
+static mtx_t lk;
+static cnd_t cnd;
+
+void f1() {
+  mtx_lock(&lk);
+
+  if (1) {
+    cnd_wait(&cnd, &lk); // NON_COMPLIANT
+  }
+}
+
+void f2() {
+  mtx_lock(&lk);
+  int i = 2;
+  while (i > 0) {
+    cnd_wait(&cnd, &lk); // COMPLIANT
+    i--;
+  }
+}
+
+void f3() {
+  mtx_lock(&lk);
+  int i = 2;
+  do {
+    cnd_wait(&cnd, &lk); // COMPLIANT
+    i--;
+  } while (i > 0);
+}
+
+void f4() {
+  mtx_lock(&lk);
+  
+  for (;;) {
+    cnd_wait(&cnd, &lk); // COMPLIANT
+  }
+}
+
+void f5() {
+  mtx_lock(&lk);
+  
+  int i = 2;
+  while (i > 0) {
+    i--;
+  }
+
+  cnd_wait(&cnd, &lk); // NON_COMPLIANT
+}
+
+void f6() {
+  mtx_lock(&lk);
+  
+  for (int i = 0; i < 10; i++) {
+  }
+  int i = 0;
+  if (i > 0) {
+    cnd_wait(&cnd, &lk); // NON_COMPLIANT
+    i--;
+  }
+}

From f43e223195fe77f7ad54f19673d98b4d43819971 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 16:41:59 -0400
Subject: [PATCH 09/25] test

---
 .../CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref   | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 c/cert/test/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref

diff --git a/c/cert/test/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref b/c/cert/test/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref
new file mode 100644
index 0000000000..562c8a0f1b
--- /dev/null
+++ b/c/cert/test/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref
@@ -0,0 +1 @@
+c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql
\ No newline at end of file

From ee9625d9bb21c610fac3e074157b58da229f87dd Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Tue, 12 Jul 2022 16:42:18 -0400
Subject: [PATCH 10/25] rule

---
 ...pFunctionsThatCanSpuriouslyWakeUpInLoop.md | 18 +++++++++++++++
 ...pFunctionsThatCanSpuriouslyWakeUpInLoop.ql | 23 +++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.md
 create mode 100644 c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.ql

diff --git a/c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.md b/c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.md
new file mode 100644
index 0000000000..2eaff7aa78
--- /dev/null
+++ b/c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.md
@@ -0,0 +1,18 @@
+# CON36-C: Wrap functions that can spuriously wake up in a loop
+
+This query implements the CERT-C rule CON36-C:
+
+> Wrap functions that can spuriously wake up in a loop
+
+
+## CERT
+
+** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **
+
+## Implementation notes
+
+None
+
+## References
+
+* CERT-C: [CON36-C: Wrap functions that can spuriously wake up in a loop](https://wiki.sei.cmu.edu/confluence/display/c)
diff --git a/c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.ql b/c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.ql
new file mode 100644
index 0000000000..430a0e7c19
--- /dev/null
+++ b/c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.ql
@@ -0,0 +1,23 @@
+/**
+ * @id c/cert/wrap-functions-that-can-spuriously-wake-up-in-loop
+ * @name CON36-C: Wrap functions that can spuriously wake up in a loop
+ * @description Not wrapping functions that can wake up spuriously in a conditioned loop can result
+ *              race conditions.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/cert/id/con36-c
+ *       correctness
+ *       concurrency
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.cpp.rules.wrapspuriousfunctioninloop.WrapSpuriousFunctionInLoop
+
+class WrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery extends WrapSpuriousFunctionInLoopSharedQuery {
+  WrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery() {
+    this = Concurrency2Package::wrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery()
+  }
+}

From af84f243c0c5f124e6d502992c2375b7f873c670 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Wed, 13 Jul 2022 18:28:35 -0400
Subject: [PATCH 11/25] changes for making query more robust

---
 .../DeadlockByLockingInPredefinedOrder.ql     | 25 ++++++-
 cpp/cert/test/rules/CON53-CPP/test.cpp        | 74 ++++++++++++++++++-
 .../src/codingstandards/cpp/Concurrency.qll   |  2 +-
 3 files changed, 94 insertions(+), 7 deletions(-)

diff --git a/cpp/cert/src/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.ql b/cpp/cert/src/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.ql
index e8eb79351b..86995338fe 100644
--- a/cpp/cert/src/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.ql
+++ b/cpp/cert/src/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.ql
@@ -15,6 +15,7 @@
 import cpp
 import codingstandards.cpp.cert
 import codingstandards.cpp.Concurrency
+import semmle.code.cpp.controlflow.Dominance
 
 /**
  * Gets a pair of locks guarding a `LockProtectedControlFlowNode` in an order
@@ -27,7 +28,8 @@ predicate getAnOrderedLockPair(
   lock1 = node.coveredByLock() and
   lock2 = node.coveredByLock() and
   not lock1 = lock2 and
-  lock1.getFile() = lock2.getFile() and
+  lock1.getEnclosingFunction() = lock2.getEnclosingFunction() and
+  node.(Expr).getEnclosingFunction() = lock1.getEnclosingFunction() and
   exists(Location l1Loc, Location l2Loc |
     l1Loc = lock1.getLocation() and
     l2Loc = lock2.getLocation()
@@ -53,15 +55,30 @@ predicate getAnOrderedLockPair(
  * same time that are invoked from a thread.
  */
 
+predicate isUnSerializedLock(LockingOperation lock) {
+  exists(VariableAccess va |
+    va = lock.getArgument(0).getAChild().(VariableAccess) and
+    not exists(Assignment assn |
+      assn = va.getTarget().getAnAssignment() and
+      not bbDominates(assn.getBasicBlock(), lock.getBasicBlock())
+    )
+  )
+}
+
+predicate isSerializedLock(LockingOperation lock) { not isUnSerializedLock(lock) }
+
 from LockProtectedControlFlowNode node, Function f, FunctionCall lock1, FunctionCall lock2
 where
   not isExcluded(node, ConcurrencyPackage::deadlockByLockingInPredefinedOrderQuery()) and
   // we can get into trouble when we get into a situation where there may be two
   // locks in the same threaded function active at the same time.
-  // simple ordering
-  // lock1 = node.coveredByLock() and
-  // lock2 = node.coveredByLock() and
+  // simple ordering is applied here for presentation purposes.
   getAnOrderedLockPair(lock1, lock2, node) and
+  // it is difficult to determine if the ordering applied to the locks is "safe"
+  // so here we simply look to see that there exists at least one other program
+  // path that would yield different argument values to the lock functions
+  // perhaps arising from some logic that applies an ordering to the locking.
+  not (isSerializedLock(lock1) and isSerializedLock(lock2)) and
   // To reduce the noise (and increase usefulness) we alert the user at the
   // level of the function, which is the unit the synchronization should be
   // performed.
diff --git a/cpp/cert/test/rules/CON53-CPP/test.cpp b/cpp/cert/test/rules/CON53-CPP/test.cpp
index af1e6dfdc7..2d6c49e191 100644
--- a/cpp/cert/test/rules/CON53-CPP/test.cpp
+++ b/cpp/cert/test/rules/CON53-CPP/test.cpp
@@ -1,4 +1,5 @@
 #include <mutex>
+#include <stdlib.h>
 #include <thread>
 
 class B {
@@ -54,9 +55,64 @@ int d3(B *from, B *to, int amount) { // NON_COMPLIANT
   return 0;
 }
 
+int getA() { return 0; }
+int getARand() { return rand(); }
+
+int d4(B *from, B *to, int amount) { // COMPLIANT
+  std::mutex *one;
+  std::mutex *two;
+
+  int a = getARand();
+
+  // here a may take on multiple different
+  // values and thus different values may flow
+  // into the locks
+  if (a == 9) {
+    one = &from->mu;
+    two = &to->mu;
+  } else {
+    one = &to->mu;
+    two = &from->mu;
+  }
+
+  std::lock_guard<std::mutex> from_lock(*one);
+  std::lock_guard<std::mutex> to_lock(*two);
+
+  from->set(from->get() - amount);
+  to->set(to->get() + amount);
+
+  return 0;
+}
+
+int d5(B *from, B *to, int amount) { // NON_COMPLIANT
+  std::mutex *one;
+  std::mutex *two;
+
+  int a = getARand();
+
+  // here a may take on multiple different
+  // values and thus different values may flow
+  // into the locks
+  if (a == 9) {
+    one = &from->mu;
+    two = &to->mu;
+  } else {
+    one = &to->mu;
+    two = &from->mu;
+  }
+
+  std::lock_guard<std::mutex> from_lock(from->mu);
+  std::lock_guard<std::mutex> to_lock(to->mu);
+
+  from->set(from->get() - amount);
+  to->set(to->get() + amount);
+
+  return 0;
+}
+
 void f(B *ba1, B *ba2) {
   std::thread thr1(d1, ba1, ba2, 100);
-  std::thread thr2(d2, ba2, ba1, 100);
+  std::thread thr2(d1, ba2, ba1, 100);
   thr1.join();
   thr2.join();
 }
@@ -73,4 +129,18 @@ void f3(B *ba1, B *ba2) {
   std::thread thr2(d3, ba1, ba2, 100);
   thr1.join();
   thr2.join();
-}
\ No newline at end of file
+}
+
+void f4(B *ba1, B *ba2) {
+  std::thread thr1(d4, ba1, ba2, 100);
+  std::thread thr2(d4, ba1, ba2, 100);
+  thr1.join();
+  thr2.join();
+}
+
+void f5(B *ba1, B *ba2) {
+  std::thread thr1(d5, ba1, ba2, 100);
+  std::thread thr2(d5, ba1, ba2, 100);
+  thr1.join();
+  thr2.join();
+}
diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll
index fe6c46c60d..3d23edb35d 100644
--- 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) and
+    lock = getArgument(0).getAChild() and
     // defer_locks don't cause a lock
     not exists(Expr exp |
       exp = getArgument(1) and

From 9b11600c0331404693f11d4fd38d8c4115c20b0b Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Wed, 13 Jul 2022 18:30:40 -0400
Subject: [PATCH 12/25] changenote

---
 change_notes/2022-07-13-improvements-to-lock-detection.md | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 change_notes/2022-07-13-improvements-to-lock-detection.md

diff --git a/change_notes/2022-07-13-improvements-to-lock-detection.md b/change_notes/2022-07-13-improvements-to-lock-detection.md
new file mode 100644
index 0000000000..1dc622db45
--- /dev/null
+++ b/change_notes/2022-07-13-improvements-to-lock-detection.md
@@ -0,0 +1,3 @@
+- `CON53-CPP` - `DeadlockByLockingInPredefinedOrder.ql`
+  - Optimized performance and expanded coverage to include cases where locking
+    order is not serialized 
\ No newline at end of file

From 2b4a46a7ebe2afb1dde1e86c95ac626f8c598e8d Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:10:49 -0400
Subject: [PATCH 13/25] move rules

---
 rule_packages/c/Concurrency2.json  | 21 +--------------------
 rule_packages/cpp/Concurrency.json |  1 +
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/rule_packages/c/Concurrency2.json b/rule_packages/c/Concurrency2.json
index 5ff80a4d85..d9102a07df 100644
--- a/rule_packages/c/Concurrency2.json
+++ b/rule_packages/c/Concurrency2.json
@@ -1,25 +1,5 @@
 {
   "CERT-C": {
-    "CON31-C": {
-      "properties": {
-        "obligation": "rule"
-      },
-      "queries": [
-        {
-          "description": "Allowing a mutex to go out of scope while it is locked removes protections around shared resources.",
-          "kind": "problem",
-          "name": "Do not destroy a mutex while it is locked",
-          "precision": "medium",
-          "severity": "error",
-          "short_name": "DoNotDestroyAMutexWhileItIsLocked",
-          "tags": [
-            "correctness",
-            "concurrency"
-          ]
-        }
-      ],
-      "title": "Do not destroy a mutex while it is locked"
-    },
     "CON35-C": {
       "properties": {
         "obligation": "rule"
@@ -32,6 +12,7 @@
           "precision": "very-high",
           "severity": "error",
           "short_name": "DeadlockByLockingInPredefinedOrder",
+          "shared_implementation_short_name": "PreventDeadlockByLockingInPredefinedOrder",
           "tags": [
             "correctness",
             "concurrency"
diff --git a/rule_packages/cpp/Concurrency.json b/rule_packages/cpp/Concurrency.json
index 61008a6142..80bd09dfd8 100644
--- a/rule_packages/cpp/Concurrency.json
+++ b/rule_packages/cpp/Concurrency.json
@@ -85,6 +85,7 @@
           "precision": "medium",
           "severity": "error",
           "short_name": "DeadlockByLockingInPredefinedOrder",
+          "shared_implementation_short_name": "PreventDeadlockByLockingInPredefinedOrder",
           "tags": [
             "correctness",
             "concurrency"

From 09e7d31c85d78bde2e22b5c96e6f27eb2d022293 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:11:33 -0400
Subject: [PATCH 14/25] refactor query

---
 .../DeadlockByLockingInPredefinedOrder.ql     | 84 ++-----------------
 1 file changed, 5 insertions(+), 79 deletions(-)

diff --git a/cpp/cert/src/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.ql b/cpp/cert/src/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.ql
index 86995338fe..3989464f70 100644
--- a/cpp/cert/src/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.ql
+++ b/cpp/cert/src/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.ql
@@ -14,84 +14,10 @@
 
 import cpp
 import codingstandards.cpp.cert
-import codingstandards.cpp.Concurrency
-import semmle.code.cpp.controlflow.Dominance
+import codingstandards.cpp.rules.preventdeadlockbylockinginpredefinedorder.PreventDeadlockByLockingInPredefinedOrder
 
-/**
- * Gets a pair of locks guarding a `LockProtectedControlFlowNode` in an order
- * specified by the locking function's call site.
- */
-pragma[inline]
-predicate getAnOrderedLockPair(
-  FunctionCall lock1, FunctionCall lock2, LockProtectedControlFlowNode node
-) {
-  lock1 = node.coveredByLock() and
-  lock2 = node.coveredByLock() and
-  not lock1 = lock2 and
-  lock1.getEnclosingFunction() = lock2.getEnclosingFunction() and
-  node.(Expr).getEnclosingFunction() = lock1.getEnclosingFunction() and
-  exists(Location l1Loc, Location l2Loc |
-    l1Loc = lock1.getLocation() and
-    l2Loc = lock2.getLocation()
-  |
-    l1Loc.getEndLine() < l2Loc.getStartLine()
-    or
-    l1Loc.getStartLine() = l2Loc.getEndLine() and
-    l1Loc.getEndColumn() < l2Loc.getStartColumn()
-  )
-}
-
-/*
- * There are two ways to safely avoid deadlock. One involves doing the locking
- * in a specific order that is guaranteed to be the same across all thread
- * invocations. This is especially hard to check and thus we adopt an
- * alternative viewpoint wherein we view a "safe" usage of multiple locks to be
- * one that uses the built in `std::lock` functionality which avoids this
- * problem.
- *
- * To properly deadlock, a thread must have at least two different locks (i.e.,
- * mutual exclusion) which are used in an order that causes a problem.  Thus we
- * look for functions with CFNs wherein there may be two locks active at the
- * same time that are invoked from a thread.
- */
-
-predicate isUnSerializedLock(LockingOperation lock) {
-  exists(VariableAccess va |
-    va = lock.getArgument(0).getAChild().(VariableAccess) and
-    not exists(Assignment assn |
-      assn = va.getTarget().getAnAssignment() and
-      not bbDominates(assn.getBasicBlock(), lock.getBasicBlock())
-    )
-  )
+class DeadlockByLockingInPredefinedOrderQuery extends PreventDeadlockByLockingInPredefinedOrderSharedQuery {
+  DeadlockByLockingInPredefinedOrderQuery() {
+    this = ConcurrencyPackage::deadlockByLockingInPredefinedOrderQuery()
+  }
 }
-
-predicate isSerializedLock(LockingOperation lock) { not isUnSerializedLock(lock) }
-
-from LockProtectedControlFlowNode node, Function f, FunctionCall lock1, FunctionCall lock2
-where
-  not isExcluded(node, ConcurrencyPackage::deadlockByLockingInPredefinedOrderQuery()) and
-  // we can get into trouble when we get into a situation where there may be two
-  // locks in the same threaded function active at the same time.
-  // simple ordering is applied here for presentation purposes.
-  getAnOrderedLockPair(lock1, lock2, node) and
-  // it is difficult to determine if the ordering applied to the locks is "safe"
-  // so here we simply look to see that there exists at least one other program
-  // path that would yield different argument values to the lock functions
-  // perhaps arising from some logic that applies an ordering to the locking.
-  not (isSerializedLock(lock1) and isSerializedLock(lock2)) and
-  // To reduce the noise (and increase usefulness) we alert the user at the
-  // level of the function, which is the unit the synchronization should be
-  // performed.
-  f = node.getEnclosingStmt().getEnclosingFunction() and
-  // Because `std::lock` isn't included in our definition of a 'lock'
-  // it is not necessary to check to see if it is in fact what is protecting
-  // these CNFs.
-  // However, to reduce noise, we shall require that the function we are
-  // reporting makes some sort of locking call since this is likely where the
-  // user intends to perform the locking operations. Our implementation will
-  // currently look into all of these nodes which is less helpful for the user
-  // but useful for our analysis.
-  any(LockingOperation l).getEnclosingFunction() = f
-select f,
-  "Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks.",
-  lock1, "lock 1", lock2, "lock 2"

From af0d076f24312c8d05155332652bf4331f2d9bb9 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:12:06 -0400
Subject: [PATCH 15/25] refactor

---
 ...eadlockByLockingInPredefinedOrder.expected |   2 -
 .../DeadlockByLockingInPredefinedOrder.qlref  |   1 -
 ...DeadlockByLockingInPredefinedOrder.testref |   1 +
 cpp/cert/test/rules/CON53-CPP/test.cpp        | 146 ------------------
 4 files changed, 1 insertion(+), 149 deletions(-)
 delete mode 100644 cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.expected
 delete mode 100644 cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.qlref
 create mode 100644 cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.testref
 delete mode 100644 cpp/cert/test/rules/CON53-CPP/test.cpp

diff --git a/cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.expected b/cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.expected
deleted file mode 100644
index f0bf4fcc85..0000000000
--- a/cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.expected
+++ /dev/null
@@ -1,2 +0,0 @@
-| test.cpp:29:5:29:6 | d2 | Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks. | test.cpp:30:41:30:49 | call to lock_guard | lock 1 | test.cpp:35:39:35:45 | call to lock_guard | lock 2 |
-| test.cpp:43:5:43:6 | d3 | Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks. | test.cpp:44:41:44:49 | call to lock_guard | lock 1 | test.cpp:49:39:49:45 | call to lock_guard | lock 2 |
diff --git a/cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.qlref b/cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.qlref
deleted file mode 100644
index 1263f38964..0000000000
--- a/cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.qlref
+++ /dev/null
@@ -1 +0,0 @@
-rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.ql
\ No newline at end of file
diff --git a/cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.testref b/cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.testref
new file mode 100644
index 0000000000..7992d86205
--- /dev/null
+++ b/cpp/cert/test/rules/CON53-CPP/DeadlockByLockingInPredefinedOrder.testref
@@ -0,0 +1 @@
+cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql
\ No newline at end of file
diff --git a/cpp/cert/test/rules/CON53-CPP/test.cpp b/cpp/cert/test/rules/CON53-CPP/test.cpp
deleted file mode 100644
index 2d6c49e191..0000000000
--- a/cpp/cert/test/rules/CON53-CPP/test.cpp
+++ /dev/null
@@ -1,146 +0,0 @@
-#include <mutex>
-#include <stdlib.h>
-#include <thread>
-
-class B {
-  int sum;
-
-public:
-  std::mutex mu;
-  B() = delete;
-  explicit B(int initialAmount) : sum(initialAmount) {}
-  int get() const { return sum; }
-  void set(int amount) { sum = amount; }
-};
-
-int d1(B *from, B *to, int amount) { // COMPLIANT
-  std::unique_lock<std::mutex> lk1(from->mu, std::defer_lock);
-  std::unique_lock<std::mutex> lk2(to->mu, std::defer_lock);
-
-  std::lock(lk1, lk2);
-
-  if (from->get() >= amount) {
-    from->set(from->get() - amount);
-    to->set(to->get() + amount);
-    return 0;
-  }
-  return -1;
-}
-
-int d2(B *from, B *to, int amount) { // NON_COMPLIANT
-  std::lock_guard<std::mutex> from_lock(from->mu);
-
-  if (from->get() < amount) {
-    return -1;
-  }
-  std::lock_guard<std::mutex> to_lock(to->mu);
-
-  from->set(from->get() - amount);
-  to->set(to->get() + amount);
-
-  return 0;
-}
-
-int d3(B *from, B *to, int amount) { // NON_COMPLIANT
-  std::lock_guard<std::mutex> from_lock(from->mu);
-
-  if (from->get() < amount) {
-    return -1;
-  }
-  std::lock_guard<std::mutex> to_lock(to->mu);
-
-  from->set(from->get() - amount);
-  to->set(to->get() + amount);
-
-  return 0;
-}
-
-int getA() { return 0; }
-int getARand() { return rand(); }
-
-int d4(B *from, B *to, int amount) { // COMPLIANT
-  std::mutex *one;
-  std::mutex *two;
-
-  int a = getARand();
-
-  // here a may take on multiple different
-  // values and thus different values may flow
-  // into the locks
-  if (a == 9) {
-    one = &from->mu;
-    two = &to->mu;
-  } else {
-    one = &to->mu;
-    two = &from->mu;
-  }
-
-  std::lock_guard<std::mutex> from_lock(*one);
-  std::lock_guard<std::mutex> to_lock(*two);
-
-  from->set(from->get() - amount);
-  to->set(to->get() + amount);
-
-  return 0;
-}
-
-int d5(B *from, B *to, int amount) { // NON_COMPLIANT
-  std::mutex *one;
-  std::mutex *two;
-
-  int a = getARand();
-
-  // here a may take on multiple different
-  // values and thus different values may flow
-  // into the locks
-  if (a == 9) {
-    one = &from->mu;
-    two = &to->mu;
-  } else {
-    one = &to->mu;
-    two = &from->mu;
-  }
-
-  std::lock_guard<std::mutex> from_lock(from->mu);
-  std::lock_guard<std::mutex> to_lock(to->mu);
-
-  from->set(from->get() - amount);
-  to->set(to->get() + amount);
-
-  return 0;
-}
-
-void f(B *ba1, B *ba2) {
-  std::thread thr1(d1, ba1, ba2, 100);
-  std::thread thr2(d1, ba2, ba1, 100);
-  thr1.join();
-  thr2.join();
-}
-
-void f2(B *ba1, B *ba2) {
-  std::thread thr1(d2, ba1, ba2, 100);
-  std::thread thr2(d2, ba2, ba1, 100);
-  thr1.join();
-  thr2.join();
-}
-
-void f3(B *ba1, B *ba2) {
-  std::thread thr1(d3, ba1, ba2, 100);
-  std::thread thr2(d3, ba1, ba2, 100);
-  thr1.join();
-  thr2.join();
-}
-
-void f4(B *ba1, B *ba2) {
-  std::thread thr1(d4, ba1, ba2, 100);
-  std::thread thr2(d4, ba1, ba2, 100);
-  thr1.join();
-  thr2.join();
-}
-
-void f5(B *ba1, B *ba2) {
-  std::thread thr1(d5, ba1, ba2, 100);
-  std::thread thr2(d5, ba1, ba2, 100);
-  thr1.join();
-  thr2.join();
-}

From adfb4f0a544984e1d0b8fbcfbb9de23b3897e151 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:12:29 -0400
Subject: [PATCH 16/25] CPP test case

---
 ...eadlockByLockingInPredefinedOrder.expected |   3 +
 ...eventDeadlockByLockingInPredefinedOrder.ql |   2 +
 .../test.cpp                                  | 145 ++++++++++++++++++
 3 files changed, 150 insertions(+)
 create mode 100644 cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.expected
 create mode 100644 cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql
 create mode 100644 cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/test.cpp

diff --git a/cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.expected b/cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.expected
new file mode 100644
index 0000000000..3fc6c0b277
--- /dev/null
+++ b/cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.expected
@@ -0,0 +1,3 @@
+| test.cpp:30:5:30:6 | d2 | Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks. | test.cpp:31:41:31:49 | call to lock_guard | lock 1 | test.cpp:36:39:36:45 | call to lock_guard | lock 2 |
+| test.cpp:44:5:44:6 | d3 | Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks. | test.cpp:45:41:45:49 | call to lock_guard | lock 1 | test.cpp:50:39:50:45 | call to lock_guard | lock 2 |
+| test.cpp:86:5:86:6 | d5 | Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks. | test.cpp:103:41:103:49 | call to lock_guard | lock 1 | test.cpp:104:39:104:45 | call to lock_guard | lock 2 |
diff --git a/cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql b/cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql
new file mode 100644
index 0000000000..6412db389a
--- /dev/null
+++ b/cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql
@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.preventdeadlockbylockinginpredefinedorder.PreventDeadlockByLockingInPredefinedOrder
diff --git a/cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/test.cpp b/cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/test.cpp
new file mode 100644
index 0000000000..52aaf912b7
--- /dev/null
+++ b/cpp/common/test/rules/preventdeadlockbylockinginpredefinedorder/test.cpp
@@ -0,0 +1,145 @@
+#include <mutex>
+#include <stdlib.h>
+#include <thread>
+
+class B {
+  int sum;
+
+public:
+  std::mutex mu;
+  B() = delete;
+  explicit B(int initialAmount) : sum(initialAmount) {}
+  int get() const { return sum; }
+  void set(int amount) { sum = amount; }
+};
+
+int d1(B *from, B *to, int amount) { // COMPLIANT
+  std::unique_lock<std::mutex> lk1(from->mu, std::defer_lock);
+  std::unique_lock<std::mutex> lk2(to->mu, std::defer_lock);
+
+  std::lock(lk1, lk2);
+
+  if (from->get() >= amount) {
+    from->set(from->get() - amount);
+    to->set(to->get() + amount);
+    return 0;
+  }
+  return -1;
+}
+
+int d2(B *from, B *to, int amount) { // NON_COMPLIANT
+  std::lock_guard<std::mutex> from_lock(from->mu);
+
+  if (from->get() < amount) {
+    return -1;
+  }
+  std::lock_guard<std::mutex> to_lock(to->mu);
+
+  from->set(from->get() - amount);
+  to->set(to->get() + amount);
+
+  return 0;
+}
+
+int d3(B *from, B *to, int amount) { // NON_COMPLIANT
+  std::lock_guard<std::mutex> from_lock(from->mu);
+
+  if (from->get() < amount) {
+    return -1;
+  }
+  std::lock_guard<std::mutex> to_lock(to->mu);
+
+  from->set(from->get() - amount);
+  to->set(to->get() + amount);
+
+  return 0;
+}
+
+int getA() { return 0; }
+
+int d4(B *from, B *to, int amount) { // COMPLIANT
+  std::mutex *one;
+  std::mutex *two;
+
+  int a = getA();
+
+  // here a may take on multiple different
+  // values and thus different values may flow
+  // into the locks
+  if (a == 9) {
+    one = &from->mu;
+    two = &to->mu;
+  } else {
+    one = &to->mu;
+    two = &from->mu;
+  }
+
+  std::lock_guard<std::mutex> from_lock(*one);
+  std::lock_guard<std::mutex> to_lock(*two);
+
+  from->set(from->get() - amount);
+  to->set(to->get() + amount);
+
+  return 0;
+}
+
+int d5(B *from, B *to, int amount) { // NON_COMPLIANT
+  std::mutex *one;
+  std::mutex *two;
+
+  int a = getA();
+
+  // here a may take on multiple different
+  // values and thus different values may flow
+  // into the locks
+  if (a == 9) {
+    one = &from->mu;
+    two = &to->mu;
+  } else {
+    one = &to->mu;
+    two = &from->mu;
+  }
+
+  std::lock_guard<std::mutex> from_lock(from->mu);
+  std::lock_guard<std::mutex> to_lock(to->mu);
+
+  from->set(from->get() - amount);
+  to->set(to->get() + amount);
+
+  return 0;
+}
+
+void f(B *ba1, B *ba2) {
+  std::thread thr1(d1, ba1, ba2, 100);
+  std::thread thr2(d1, ba2, ba1, 100);
+  thr1.join();
+  thr2.join();
+}
+
+void f2(B *ba1, B *ba2) {
+  std::thread thr1(d2, ba1, ba2, 100);
+  std::thread thr2(d2, ba2, ba1, 100);
+  thr1.join();
+  thr2.join();
+}
+
+void f3(B *ba1, B *ba2) {
+  std::thread thr1(d3, ba1, ba2, 100);
+  std::thread thr2(d3, ba1, ba2, 100);
+  thr1.join();
+  thr2.join();
+}
+
+void f4(B *ba1, B *ba2) {
+  std::thread thr1(d4, ba1, ba2, 100);
+  std::thread thr2(d4, ba1, ba2, 100);
+  thr1.join();
+  thr2.join();
+}
+
+void f5(B *ba1, B *ba2) {
+  std::thread thr1(d5, ba1, ba2, 100);
+  std::thread thr2(d5, ba1, ba2, 100);
+  thr1.join();
+  thr2.join();
+}

From 8741010600e876ef50f980b5ee21df4eeeb317b6 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:12:42 -0400
Subject: [PATCH 17/25] refactor shared rule

---
 .../cpp/exclusions/c/Concurrency2.qll         | 16 ---
 ...ventDeadlockByLockingInPredefinedOrder.qll | 99 +++++++++++++++++++
 2 files changed, 99 insertions(+), 16 deletions(-)
 create mode 100644 cpp/common/src/codingstandards/cpp/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.qll

diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency2.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency2.qll
index 4776537ab3..7e3bbe10a7 100644
--- a/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency2.qll
+++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency2.qll
@@ -4,19 +4,10 @@ import RuleMetadata
 import codingstandards.cpp.exclusions.RuleMetadata
 
 newtype Concurrency2Query =
-  TDoNotDestroyAMutexWhileItIsLockedQuery() or
   TDeadlockByLockingInPredefinedOrderQuery() or
   TWrapFunctionsThatCanSpuriouslyWakeUpInLoopQuery()
 
 predicate isConcurrency2QueryMetadata(Query query, string queryId, string ruleId) {
-  query =
-    // `Query` instance for the `doNotDestroyAMutexWhileItIsLocked` query
-    Concurrency2Package::doNotDestroyAMutexWhileItIsLockedQuery() and
-  queryId =
-    // `@id` for the `doNotDestroyAMutexWhileItIsLocked` query
-    "c/cert/do-not-destroy-a-mutex-while-it-is-locked" and
-  ruleId = "CON31-C"
-  or
   query =
     // `Query` instance for the `deadlockByLockingInPredefinedOrder` query
     Concurrency2Package::deadlockByLockingInPredefinedOrderQuery() and
@@ -35,13 +26,6 @@ predicate isConcurrency2QueryMetadata(Query query, string queryId, string ruleId
 }
 
 module Concurrency2Package {
-  Query doNotDestroyAMutexWhileItIsLockedQuery() {
-    //autogenerate `Query` type
-    result =
-      // `Query` type for `doNotDestroyAMutexWhileItIsLocked` query
-      TQueryC(TConcurrency2PackageQuery(TDoNotDestroyAMutexWhileItIsLockedQuery()))
-  }
-
   Query deadlockByLockingInPredefinedOrderQuery() {
     //autogenerate `Query` type
     result =
diff --git a/cpp/common/src/codingstandards/cpp/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.qll b/cpp/common/src/codingstandards/cpp/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.qll
new file mode 100644
index 0000000000..ca54e3c2d8
--- /dev/null
+++ b/cpp/common/src/codingstandards/cpp/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.qll
@@ -0,0 +1,99 @@
+/**
+ * Provides a library which includes a `problems` predicate for locks that do not
+ * lock in a predefined order.
+ */
+
+import cpp
+import codingstandards.cpp.Customizations
+import codingstandards.cpp.Exclusions
+import codingstandards.cpp.Concurrency
+import semmle.code.cpp.controlflow.Dominance
+
+abstract class PreventDeadlockByLockingInPredefinedOrderSharedQuery extends Query { }
+
+Query getQuery() { result instanceof PreventDeadlockByLockingInPredefinedOrderSharedQuery }
+
+/**
+ * Gets a pair of locks guarding a `LockProtectedControlFlowNode` in an order
+ * specified by the locking function's call site.
+ */
+pragma[inline]
+predicate getAnOrderedLockPair(
+  FunctionCall lock1, FunctionCall lock2, LockProtectedControlFlowNode node
+) {
+  lock1 = node.coveredByLock() and
+  lock2 = node.coveredByLock() and
+  not lock1 = lock2 and
+  lock1.getEnclosingFunction() = lock2.getEnclosingFunction() and
+  node.(Expr).getEnclosingFunction() = lock1.getEnclosingFunction() and
+  exists(Location l1Loc, Location l2Loc |
+    l1Loc = lock1.getLocation() and
+    l2Loc = lock2.getLocation()
+  |
+    l1Loc.getEndLine() < l2Loc.getStartLine()
+    or
+    l1Loc.getStartLine() = l2Loc.getEndLine() and
+    l1Loc.getEndColumn() < l2Loc.getStartColumn()
+  )
+}
+
+/*
+ * There are two ways to safely avoid deadlock. One involves doing the locking
+ * in a specific order that is guaranteed to be the same across all thread
+ * invocations. This is especially hard to check and thus we adopt an
+ * alternative viewpoint wherein we view a "safe" usage of multiple locks to be
+ * one that uses the built in `std::lock` functionality which avoids this
+ * problem.
+ *
+ * To properly deadlock, a thread must have at least two different locks (i.e.,
+ * mutual exclusion) which are used in an order that causes a problem.  Thus we
+ * look for functions with CFNs wherein there may be two locks active at the
+ * same time that are invoked from a thread.
+ */
+
+predicate isUnSerializedLock(LockingOperation lock) {
+  exists(VariableAccess va |
+    va = lock.getArgument(0).getAChild().(VariableAccess) and
+    not exists(Assignment assn |
+      assn = va.getTarget().getAnAssignment() and
+      not bbDominates(assn.getBasicBlock(), lock.getBasicBlock())
+    )
+  )
+}
+
+predicate isSerializedLock(LockingOperation lock) { not isUnSerializedLock(lock) }
+
+query predicate problems(
+  Function f, string message, FunctionCall lock1, string lock1String, FunctionCall lock2,
+  string lock2String
+) {
+  exists(LockProtectedControlFlowNode node |
+    not isExcluded(node, getQuery()) and
+    // we can get into trouble when we get into a situation where there may be two
+    // locks in the same threaded function active at the same time.
+    // simple ordering is applied here for presentation purposes.
+    getAnOrderedLockPair(lock1, lock2, node) and
+    // it is difficult to determine if the ordering applied to the locks is "safe"
+    // so here we simply look to see that there exists at least one other program
+    // path that would yield different argument values to the lock functions
+    // perhaps arising from some logic that applies an ordering to the locking.
+    not (isSerializedLock(lock1) and isSerializedLock(lock2)) and
+    // To reduce the noise (and increase usefulness) we alert the user at the
+    // level of the function, which is the unit the synchronization should be
+    // performed.
+    f = node.getEnclosingStmt().getEnclosingFunction() and
+    // Because `std::lock` isn't included in our definition of a 'lock'
+    // it is not necessary to check to see if it is in fact what is protecting
+    // these CNFs.
+    // However, to reduce noise, we shall require that the function we are
+    // reporting makes some sort of locking call since this is likely where the
+    // user intends to perform the locking operations. Our implementation will
+    // currently look into all of these nodes which is less helpful for the user
+    // but useful for our analysis.
+    any(LockingOperation l).getEnclosingFunction() = f and
+    lock1String = "lock 1" and
+    lock2String = "lock 2"
+  ) and
+  message =
+    "Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks."
+}

From bcc8c31cb683bccbd6138d7799e9514ea1d0b469 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:12:58 -0400
Subject: [PATCH 18/25] C test case

---
 ...eadlockByLockingInPredefinedOrder.expected |   3 +
 ...eventDeadlockByLockingInPredefinedOrder.ql |   2 +
 .../test.c                                    | 166 ++++++++++++++++++
 3 files changed, 171 insertions(+)
 create mode 100644 c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.expected
 create mode 100644 c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql
 create mode 100644 c/common/test/rules/preventdeadlockbylockinginpredefinedorder/test.c

diff --git a/c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.expected b/c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.expected
new file mode 100644
index 0000000000..2d0b60ab9b
--- /dev/null
+++ b/c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.expected
@@ -0,0 +1,3 @@
+| test.c:15:5:15:6 | d1 | Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks. | test.c:22:3:22:10 | call to mtx_lock | lock 1 | test.c:23:3:23:10 | call to mtx_lock | lock 2 |
+| test.c:33:5:33:6 | d2 | Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks. | test.c:41:3:41:10 | call to mtx_lock | lock 1 | test.c:45:3:45:10 | call to mtx_lock | lock 2 |
+| test.c:88:5:88:6 | d4 | Threaded function may be called from a context that uses $@ and $@ which may lead to deadlocks. | test.c:111:3:111:10 | call to mtx_lock | lock 1 | test.c:112:3:112:10 | call to mtx_lock | lock 2 |
diff --git a/c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql b/c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql
new file mode 100644
index 0000000000..6412db389a
--- /dev/null
+++ b/c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql
@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.preventdeadlockbylockinginpredefinedorder.PreventDeadlockByLockingInPredefinedOrder
diff --git a/c/common/test/rules/preventdeadlockbylockinginpredefinedorder/test.c b/c/common/test/rules/preventdeadlockbylockinginpredefinedorder/test.c
new file mode 100644
index 0000000000..6d8d25755b
--- /dev/null
+++ b/c/common/test/rules/preventdeadlockbylockinginpredefinedorder/test.c
@@ -0,0 +1,166 @@
+#include <stdlib.h>
+#include <threads.h>
+
+typedef struct {
+  int sum;
+  mtx_t mu;
+} B;
+
+typedef struct {
+  B *from;
+  B *to;
+  int amount;
+} thread_arg;
+
+int d1(void *arg) { // NON_COMPLIANT
+  thread_arg *targ = (thread_arg *)arg;
+
+  B *from = targ->from;
+  B *to = targ->to;
+  int amount = targ->amount;
+
+  mtx_lock(&from->mu);
+  mtx_lock(&to->mu);
+
+  if (from->sum >= amount) {
+    from->sum = from->sum - amount;
+    to->sum = to->sum + amount;
+    return 0;
+  }
+  return -1;
+}
+
+int d2(void *arg) { // NON_COMPLIANT
+
+  thread_arg *targ = (thread_arg *)arg;
+
+  B *from = targ->from;
+  B *to = targ->to;
+  int amount = targ->amount;
+
+  mtx_lock(&from->mu);
+  if (from->sum < amount) {
+    return -1;
+  }
+  mtx_lock(&to->mu);
+  from->sum = (from->sum - amount);
+  to->sum = (to->sum + amount);
+
+  return 0;
+}
+
+int getA() { return 0; }
+int getARand() { return rand(); }
+
+int d3(void *arg) { // COMPLIANT
+
+  thread_arg *targ = (thread_arg *)arg;
+
+  B *from = targ->from;
+  B *to = targ->to;
+  int amount = targ->amount;
+
+  mtx_t *one;
+  mtx_t *two;
+
+  int a = getARand();
+
+  // here a may take on multiple different
+  // values and thus different values may flow
+  // into the locks
+  if (a == 9) {
+    one = &from->mu;
+    two = &to->mu;
+  } else {
+    one = &to->mu;
+    two = &from->mu;
+  }
+
+  mtx_lock(one);
+  mtx_lock(two);
+
+  from->sum = (from->sum - amount);
+  to->sum = (to->sum + amount);
+
+  return 0;
+}
+
+int d4(void *arg) { // NON_COMPLIANT
+
+  thread_arg *targ = (thread_arg *)arg;
+
+  B *from = targ->from;
+  B *to = targ->to;
+  int amount = targ->amount;
+
+  mtx_t *one;
+  mtx_t *two;
+  int a = getARand();
+
+  // here a may take on multiple different
+  // values and thus different values may flow
+  // into the locks
+  if (a == 9) {
+    one = &from->mu;
+    two = &to->mu;
+  } else {
+    one = &to->mu;
+    two = &from->mu;
+  }
+
+  mtx_lock(&from->mu);
+  mtx_lock(&to->mu);
+
+  from->sum = (from->sum - amount);
+  to->sum = (to->sum + amount);
+
+  return 0;
+}
+
+void f(B *ba1, B *ba2) {
+  thrd_t t1, t2;
+
+  // unsafe - but used for testing
+  thread_arg a1 = {.from = ba1, .to = ba2, .amount = 100};
+
+  thread_arg a2 = {.from = ba2, .to = ba1, .amount = 100};
+
+  thrd_create(&t1, d1, &a1);
+  thrd_create(&t2, d1, &a2);
+}
+
+void f2(B *ba1, B *ba2) {
+  thrd_t t1, t2;
+
+  // unsafe - but used for testing
+  thread_arg a1 = {.from = ba1, .to = ba2, .amount = 100};
+
+  thread_arg a2 = {.from = ba2, .to = ba1, .amount = 100};
+
+  thrd_create(&t1, d2, &a1);
+  thrd_create(&t2, d2, &a2);
+}
+
+void f3(B *ba1, B *ba2) {
+  thrd_t t1, t2;
+
+  // unsafe - but used for testing
+  thread_arg a1 = {.from = ba1, .to = ba2, .amount = 100};
+
+  thread_arg a2 = {.from = ba2, .to = ba1, .amount = 100};
+
+  thrd_create(&t1, d3, &a1);
+  thrd_create(&t2, d3, &a2);
+}
+
+void f4(B *ba1, B *ba2) {
+  thrd_t t1, t2;
+
+  // unsafe - but used for testing
+  thread_arg a1 = {.from = ba1, .to = ba2, .amount = 100};
+
+  thread_arg a2 = {.from = ba2, .to = ba1, .amount = 100};
+
+  thrd_create(&t1, d4, &a1);
+  thrd_create(&t2, d4, &a2);
+}

From 630dbdae4d4fd643c668b5b1f41fb50fa1d3eadc Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:13:24 -0400
Subject: [PATCH 19/25] shared query

---
 .../DeadlockByLockingInPredefinedOrder.md     | 17 ++++++++++++++
 .../DeadlockByLockingInPredefinedOrder.ql     | 23 +++++++++++++++++++
 ...DeadlockByLockingInPredefinedOrder.testref |  1 +
 3 files changed, 41 insertions(+)
 create mode 100644 c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md
 create mode 100644 c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.ql
 create mode 100644 c/cert/test/rules/CON35-C/DeadlockByLockingInPredefinedOrder.testref

diff --git a/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md b/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md
new file mode 100644
index 0000000000..60f3dd9d3b
--- /dev/null
+++ b/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md
@@ -0,0 +1,17 @@
+# CON35-C: Avoid deadlock by locking in a predefined order
+
+This query implements the CERT-C rule CON35-C:
+
+> Avoid deadlock by locking in a predefined order
+
+## CERT
+
+** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **
+
+## Implementation notes
+
+None
+
+## References
+
+* CERT-C: [CON35-C: Avoid deadlock by locking in a predefined order](https://wiki.sei.cmu.edu/confluence/display/c)
diff --git a/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.ql b/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.ql
new file mode 100644
index 0000000000..00fa021878
--- /dev/null
+++ b/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.ql
@@ -0,0 +1,23 @@
+/**
+ * @id c/cert/deadlock-by-locking-in-predefined-order
+ * @name CON35-C: Avoid deadlock by locking in a predefined order
+ * @description Circular waits leading to thread deadlocks may be avoided by locking in a predefined
+ *              order.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/cert/id/con35-c
+ *       correctness
+ *       concurrency
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.cpp.rules.preventdeadlockbylockinginpredefinedorder.PreventDeadlockByLockingInPredefinedOrder
+
+class DeadlockByLockingInPredefinedOrderQuery extends PreventDeadlockByLockingInPredefinedOrderSharedQuery {
+  DeadlockByLockingInPredefinedOrderQuery() {
+    this = Concurrency2Package::deadlockByLockingInPredefinedOrderQuery()
+  }
+}
diff --git a/c/cert/test/rules/CON35-C/DeadlockByLockingInPredefinedOrder.testref b/c/cert/test/rules/CON35-C/DeadlockByLockingInPredefinedOrder.testref
new file mode 100644
index 0000000000..4625d1a24d
--- /dev/null
+++ b/c/cert/test/rules/CON35-C/DeadlockByLockingInPredefinedOrder.testref
@@ -0,0 +1 @@
+c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql
\ No newline at end of file

From 343cf647c963aea1ca200e47f8d1c3fd3749421f Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:16:19 -0400
Subject: [PATCH 20/25] fix to rules

---
 rules.csv | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rules.csv b/rules.csv
index 8662b29d3d..d7584faf03 100755
--- a/rules.csv
+++ b/rules.csv
@@ -486,7 +486,7 @@ c,CERT-C,ARR37-C,Yes,Rule,,,Do not add or subtract an integer to a pointer to a
 c,CERT-C,ARR38-C,Yes,Rule,,,Guarantee that library functions do not form invalid pointers,,Pointers2,Very Hard,
 c,CERT-C,ARR39-C,Yes,Rule,,,Do not add or subtract a scaled integer to a pointer,,Pointers2,Medium,
 c,CERT-C,CON30-C,Yes,Rule,,,Clean up thread-specific storage,,Concurrency3,Very Hard,
-c,CERT-C,CON31-C,Yes,Rule,,,Do not destroy a mutex while it is locked,CON50-CPP,Concurrency2,Very Hard,
+c,CERT-C,CON31-C,Yes,Rule,,,Do not destroy a mutex while it is locked,CON50-CPP,Concurrency3,Very Hard,
 c,CERT-C,CON32-C,Yes,Rule,,,Prevent data races when accessing bit-fields from multiple threads,,Concurrency1,Easy,
 c,CERT-C,CON33-C,Yes,Rule,,,Avoid race conditions when using library functions,,Concurrency1,Easy,
 c,CERT-C,CON34-C,Yes,Rule,,,Declare objects shared between threads with appropriate storage durations,,Concurrency3,Hard,

From c0d36be7bdb62957c6d0714b8df5dcd04c1d907b Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:37:04 -0400
Subject: [PATCH 21/25] formatting

---
 c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md b/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md
index 60f3dd9d3b..d5c72376fa 100644
--- a/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md
+++ b/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md
@@ -4,6 +4,7 @@ This query implements the CERT-C rule CON35-C:
 
 > Avoid deadlock by locking in a predefined order
 
+
 ## CERT
 
 ** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **

From eb776a7de96c9cee2a98a0649fbd8df2cd91b32d Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Thu, 14 Jul 2022 11:39:56 -0400
Subject: [PATCH 22/25] formatting

---
 cpp/common/src/codingstandards/cpp/Concurrency.qll | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll
index 3d23edb35d..a3250d2fd1 100644
--- a/cpp/common/src/codingstandards/cpp/Concurrency.qll
+++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll
@@ -421,9 +421,10 @@ class LockProtectedControlFlowNode extends ThreadedCFN {
 /**
  * Models a function that conditionally waits.
  */
-abstract class ConditionalWait extends FunctionCall {}
+abstract class ConditionalWait extends FunctionCall { }
+
 /**
- * Models a function in CPP that will conditionally wait. 
+ * Models a function in CPP that will conditionally wait.
  */
 class CPPConditionalWait extends ConditionalWait {
   CPPConditionalWait() {
@@ -436,12 +437,10 @@ class CPPConditionalWait extends ConditionalWait {
 }
 
 /**
- * Models a function in C that will conditionally wait. 
+ * Models a function in C that will conditionally wait.
  */
 class CConditionalWait extends ConditionalWait {
-  CConditionalWait() {
-    getTarget().getName() in ["cnd_wait"]
-  }
+  CConditionalWait() { getTarget().getName() in ["cnd_wait"] }
 }
 
 /**

From 7a4ca0bba00b8aac7d09e3e9088ba51fe5d0bbe3 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Fri, 15 Jul 2022 11:12:43 -0400
Subject: [PATCH 23/25] formatting

---
 c/common/test/rules/wrapspuriousfunctioninloop/test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/c/common/test/rules/wrapspuriousfunctioninloop/test.c b/c/common/test/rules/wrapspuriousfunctioninloop/test.c
index 84484364fd..31299be82e 100644
--- a/c/common/test/rules/wrapspuriousfunctioninloop/test.c
+++ b/c/common/test/rules/wrapspuriousfunctioninloop/test.c
@@ -32,7 +32,7 @@ void f3() {
 
 void f4() {
   mtx_lock(&lk);
-  
+
   for (;;) {
     cnd_wait(&cnd, &lk); // COMPLIANT
   }
@@ -40,7 +40,7 @@ void f4() {
 
 void f5() {
   mtx_lock(&lk);
-  
+
   int i = 2;
   while (i > 0) {
     i--;
@@ -51,7 +51,7 @@ void f5() {
 
 void f6() {
   mtx_lock(&lk);
-  
+
   for (int i = 0; i < 10; i++) {
   }
   int i = 0;

From 19da19057beb889c230ad3b35c396b3eb60bca8c Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Fri, 15 Jul 2022 16:58:12 -0400
Subject: [PATCH 24/25] fixes

---
 ...22-07-13-improvements-to-lock-detection.md |  5 ++++-
 .../src/codingstandards/cpp/Concurrency.qll   |  2 +-
 .../GuardAccessToBitFields.qll                | 20 ++++++++++++++++++-
 .../GuardAccessToBitFields.expected           |  2 +-
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/change_notes/2022-07-13-improvements-to-lock-detection.md b/change_notes/2022-07-13-improvements-to-lock-detection.md
index 1dc622db45..bf07c25778 100644
--- 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 
\ No newline at end of file
+    order is not serialized 
+- `CON52-CPP` - `PreventBitFieldAccessFromMultipleThreads.ql`
+  - Fixed an issue with RAII-style locks and scope causing locks to not be
+    correctly identified. 
\ No newline at end of file
diff --git a/cpp/common/src/codingstandards/cpp/Concurrency.qll b/cpp/common/src/codingstandards/cpp/Concurrency.qll
index a3250d2fd1..badd7d8fea 100644
--- 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
index f07d9d94bd..5b03a4f8bd 100644
--- 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
index 3f45e93d06..dd317d0778 100644
--- 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. |

From 9ff563573b3fa18f73a6b3b54faa281ece05dca2 Mon Sep 17 00:00:00 2001
From: "John L. Singleton" <jsinglet@github.com>
Date: Fri, 15 Jul 2022 17:07:09 -0400
Subject: [PATCH 25/25] remove markdowns

---
 .../DeadlockByLockingInPredefinedOrder.md      | 18 ------------------
 ...apFunctionsThatCanSpuriouslyWakeUpInLoop.md | 18 ------------------
 2 files changed, 36 deletions(-)
 delete mode 100644 c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md
 delete mode 100644 c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.md

diff --git a/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md b/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md
deleted file mode 100644
index d5c72376fa..0000000000
--- a/c/cert/src/rules/CON35-C/DeadlockByLockingInPredefinedOrder.md
+++ /dev/null
@@ -1,18 +0,0 @@
-# CON35-C: Avoid deadlock by locking in a predefined order
-
-This query implements the CERT-C rule CON35-C:
-
-> Avoid deadlock by locking in a predefined order
-
-
-## CERT
-
-** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **
-
-## Implementation notes
-
-None
-
-## References
-
-* CERT-C: [CON35-C: Avoid deadlock by locking in a predefined order](https://wiki.sei.cmu.edu/confluence/display/c)
diff --git a/c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.md b/c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.md
deleted file mode 100644
index 2eaff7aa78..0000000000
--- a/c/cert/src/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.md
+++ /dev/null
@@ -1,18 +0,0 @@
-# CON36-C: Wrap functions that can spuriously wake up in a loop
-
-This query implements the CERT-C rule CON36-C:
-
-> Wrap functions that can spuriously wake up in a loop
-
-
-## CERT
-
-** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **
-
-## Implementation notes
-
-None
-
-## References
-
-* CERT-C: [CON36-C: Wrap functions that can spuriously wake up in a loop](https://wiki.sei.cmu.edu/confluence/display/c)