Merge pull request #15 from jsinglet/jsinglet/concurrency-2-fork

Concurrency 2

Author: Nikita Kraiouchkine

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()
+  }
+}

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()
+  }
+}

c/cert/test/rules/CON35-C/DeadlockByLockingInPredefinedOrder.testref

@@ -0,0 +1 @@
+c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql

c/cert/test/rules/CON36-C/WrapFunctionsThatCanSpuriouslyWakeUpInLoop.testref

@@ -0,0 +1 @@
+c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql

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 |

c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql

@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.preventdeadlockbylockinginpredefinedorder.PreventDeadlockByLockingInPredefinedOrder

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);
+}

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. |

c/common/test/rules/wrapspuriousfunctioninloop/WrapSpuriousFunctionInLoop.ql

@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.wrapspuriousfunctioninloop.WrapSpuriousFunctionInLoop

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--;
+  }
+}