work

Author: jsinglet

cpp/cert/src/rules/CON50-CPP/DoNotDestroyAMutexWhileItIsLocked.ql

@@ -10,14 +10,11 @@
  *       concurrency
  *       external/cert/obligation/rule
  */
-
 import cpp
 import codingstandards.cpp.cert
 import codingstandards.cpp.Concurrency
 import semmle.code.cpp.dataflow.DataFlow
 import semmle.code.cpp.dataflow.TaintTracking
-
-
 /*
  * This query finds potential misuse of mutexes passed to threads by considering
  * cases where the underlying mutex may be destroyed. The scope of this query is 

cpp/cert/test/rules/CON50-CPP/test.c

@@ -1,41 +1,132 @@
 #include <stdatomic.h>
 #include <stddef.h>
 #include <threads.h>
-  
-mtx_t mx;
- 
-int t1(void *dummy) {
-  mtx_lock(&mx);
-  mtx_unlock(&mx);
+
+/// Note that since this is a C test case, mutexes
+/// are not created/valid until `mtx_init` is called.
+mtx_t mx1;
+mtx_t mx2;
+
+int t1(void *data) {
+  mtx_lock(&mx1);
+  mtx_unlock(&mx1);
+  return 0;
+}
+
+int t2(void *data) {
+  mtx_lock(&mx2);
+  mtx_unlock(&mx2);
+  return 0;
+}
+
+int t3(void *data) {
+  mtx_t *mxl = (mtx_t *)data;
+  mtx_lock(mxl);
+  mtx_unlock(mxl);
   return 0;
 }
- 
+
+int t4(void *data) {
+  mtx_t *mxl = (mtx_t *)data;
+  mtx_lock(mxl);
+  mtx_unlock(mxl);
+  return 0;
+}
+
+// case 1 - correctly waiting for a well-behaved thread
 int f1() {
   thrd_t threads[5];
-   
-  mtx_init(&mx, mtx_plain);
-  
-  for (size_t i = 0; i < 5; i++) {
+
+  mtx_init(&mx1, mtx_plain); // COMPLIANT
+
+  for (size_t i = 0; i < 1; i++) {
     thrd_create(&threads[i], t1, NULL);
-    
   }
-  for (size_t i = 0; i < 5; i++) {
+  for (size_t i = 0; i < 1; i++) {
     thrd_join(threads[i], 0);
   }
- 
-  mtx_destroy(&mx);
+
+  mtx_destroy(&mx1);
   return 0;
 }
 
+// case 2 - incorrectly waiting for a well behaved thread.
 int f2() {
   thrd_t threads[5];
-   
-  mtx_init(&mx, mtx_plain);
-  
-  for (size_t i = 0; i < 5; i++) {
-    thrd_create(&threads[i], t1, NULL);   
+
+  mtx_init(&mx2, mtx_plain); // NON_COMPLIANT
+
+  for (size_t i = 0; i < 1; i++) {
+    thrd_create(&threads[i], t2, NULL);
+  }
+
+  mtx_destroy(&mx2);
+  return 0;
+}
+
+// case 3 - correctly waiting for a well-behaved thread, with a local mutex.
+int f3() {
+  thrd_t threads[5];
+
+  mtx_t mxl;
+
+  mtx_init(&mxl, mtx_plain); // COMPLIANT
+
+  for (size_t i = 0; i < 1; i++) {
+    thrd_create(&threads[i], t3, &mxl);
+  }
+
+  for (size_t i = 0; i < 1; i++) {
+    thrd_join(threads[i], 0);
   }
 
-  mtx_destroy(&mx);
+  mtx_destroy(&mxl);
   return 0;
-}
+}
+
+// case 4 - incorrectly waiting for a well behaved thread, with a local mutex.
+int f4() {
+  thrd_t threads[5];
+  mtx_t mxl;
+
+  mtx_init(&mxl, mtx_plain); // NON_COMPLIANT
+
+  for (size_t i = 0; i < 1; i++) {
+    thrd_create(&threads[i], t3, &mxl);
+  }
+
+  mtx_destroy(&mxl);
+  return 0;
+}
+
+// case 5 - incorrectly waiting with a stack local variable.
+int f5() {
+  thrd_t threads[5];
+  mtx_t mxl;
+
+  mtx_init(&mxl, mtx_plain); // NON_COMPLIANT
+
+  for (size_t i = 0; i < 1; i++) {
+    thrd_create(&threads[i], t4, &mxl);
+  }
+
+  return 0;
+}
+
+// case 6 - correctly waiting with a stack local variable.
+int f6() {
+  thrd_t threads[5];
+  mtx_t mxl;
+
+  mtx_init(&mxl, mtx_plain); // COMPLIANT
+
+  for (size_t i = 0; i < 1; i++) {
+    thrd_create(&threads[i], t4, &mxl);
+  }
+
+  for (size_t i = 0; i < 1; i++) {
+    thrd_join(threads[i], 0);
+  }
+
+  return 0;
+}

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

@@ -758,7 +758,10 @@ class LocalMutexDestroyer extends MutexDestroyer {
 
   LocalMutexDestroyer() {
     exists(LocalVariable lv |
+      // static types aren't destroyers
       not lv.isStatic() and
+      // neither are pointers
+      not lv.getType() instanceof PointerType and
       lv.getAnAssignedValue() = assignedValue and
       // map the location to the return statements of the
       // enclosing function