work

Author: jsinglet

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

@@ -14,34 +14,10 @@
 
 import cpp
 import codingstandards.cpp.cert
-import codingstandards.cpp.Concurrency
-import semmle.code.cpp.dataflow.DataFlow
-import semmle.code.cpp.dataflow.TaintTracking
+import codingstandards.cpp.rules.donotallowamutextogooutofscopewhilelocked.DoNotAllowAMutexToGoOutOfScopeWhileLocked
 
-/*
- * This query finds potential misuse of mutexes passed to threads by considering
- * cases where the underlying mutex is a local variable; such a variable would
- * go out of scope at the end of the calling function and thus would potentially
- * create issues for the thread depending on the mutex. This query is primarily 
- * targeted at C usages since in the case of CPP, many more cases can be covered
- * via tracking of destructors. The main difference is that this query doesn't 
- * expect an explicitly deleted call to be made. 
- *
- * In order to safely destroy a dependent mutex, it is necessary both to not delete 
- * it, but also if deletes do happen, one must wait for a thread to exit prior to 
- * deleting it. We broadly model this by using standard language support for thread
- * synchronization. 
- */
-from ThreadDependentMutex dm, LocalVariable lv 
-where 
-  not isExcluded(dm.asExpr(), ConcurrencyPackage::doNotDestroyAMutexWhileItIsLockedQuery()) and
-  not isExcluded(lv, ConcurrencyPackage::doNotDestroyAMutexWhileItIsLockedQuery()) and
-  not lv.isStatic() and 
-  TaintTracking::localTaint(dm.getAUsage(), DataFlow::exprNode(lv.getAnAssignedValue())) 
-  // ensure that each dependent thread is followed by some sort of joining
-  // behavior. 
-  and exists(DataFlow::Node n | n = dm.getADependentThreadCreationExpr()  | forall(ThreadWait tw |
-    not (tw = n.asExpr().getASuccessor*())
-  ))
-
-  select dm, "Mutex used by thread potentially destroyed while in use."
+class DoNotAllowAMutexToGoOutOfScopeWhileLockedQuery extends DoNotAllowAMutexToGoOutOfScopeWhileLockedSharedQuery {
+  DoNotAllowAMutexToGoOutOfScopeWhileLockedQuery() {
+    this = ConcurrencyPackage::doNotAllowAMutexToGoOutOfScopeWhileLockedQuery()
+  }
+}

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

@@ -10,47 +10,13 @@
  *       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 
- * that it performs this analysis both locally within the function but can also
- * look through to the called thread to identify mutexes it may not own. 
- * query is that it considers this behavior locally within the procedure. 
- *
- * In order to safely destroy a dependent mutex, it is necessary both to not delete 
- * it, but also if deletes do happen, one must wait for a thread to exit prior to 
- * deleting it. We broadly model this by using standard language support for thread
- * synchronization. 
- */
-from ThreadDependentMutex dm, MutexDestroyer md
-where 
-  not isExcluded(dm.asExpr(), ConcurrencyPackage::doNotDestroyAMutexWhileItIsLockedQuery()) and
-  not isExcluded(md, ConcurrencyPackage::doNotDestroyAMutexWhileItIsLockedQuery()) and
-  // find all instances where a usage of a dependent mutex flows into a
-  // expression that will destroy it. 
-  TaintTracking::localTaint(dm.getAUsage(), DataFlow::exprNode(md.getMutexExpr()))
-  and 
-  (
-    // firstly, we assume it is never safe to destroy a global mutex, but it is
-    // difficult to make assumptions about the intended control flow. Note that
-    // this means the point at where the mutex is defined -- not where the variable
-    // that contains it is scoped -- a `ThreadDependentMutex` is bound to the
-    // function that creates an initialized mutex. For example, in `C`
-    // `mtx_init` is called to initialize the mutex and in C++, the constructor 
-    // of std::mutex is called.     
-    not exists(dm.asExpr().getEnclosingFunction()) or 
-    // secondly, we assume it is never safe to destroy a mutex created by
-    // another function scope -- which includes trying to destroy a mutex that
-    // was passed into a function. 
-    not md.getMutexExpr().getEnclosingFunction() = dm.asExpr().getEnclosingFunction() or
-    // this leaves only destructions of mutexes locally near the thread that may
-    // consume them. We allow this only if there has been some effort to
-    // synchronize the threads prior to destroying the mutex.
-    not exists(ThreadWait tw | tw = md.getAPredecessor*())
-  )
-select dm, "Mutex used by thread potentially $@ while in use.", md, "destroyed"
+import codingstandards.cpp.rules.donotdestroyamutexwhileitislocked.DoNotDestroyAMutexWhileItIsLocked
+
+class DoNotDestroyAMutexWhileItIsLockedQuery extends DoNotDestroyAMutexWhileItIsLockedSharedQuery {
+  DoNotDestroyAMutexWhileItIsLockedQuery() {
+    this = ConcurrencyPackage::doNotDestroyAMutexWhileItIsLockedQuery()
+  }
+}