Concurrency 4

.vscode/tasks.json

@@ -197,6 +197,7 @@
                 "Concurrency1",
                 "Concurrency2",
                 "Concurrency3",
+                "Concurrency4",                
                 "Conditionals",
                 "Const",
                 "DeadCode",

c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.md

@@ -0,0 +1,18 @@
+# CON34-C: Declare objects shared between threads with appropriate storage durations
+
+This query implements the CERT-C rule CON34-C:
+
+> Declare objects shared between threads with appropriate storage durations
+
+
+## CERT
+
+** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **
+
+## Implementation notes
+
+This query does not consider Windows implementations or OpenMP implementations. This query is primarily about excluding cases wherein the storage duration of a variable is appropriate. As such, this query is not concerned if the appropriate synchronization mechanisms are used, such as sequencing calls to `thrd_join` and `free`. An audit query is supplied to handle those cases.
+
+## References
+
+* CERT-C: [CON34-C: Declare objects shared between threads with appropriate storage durations](https://wiki.sei.cmu.edu/confluence/display/c)

c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.ql

@@ -0,0 +1,46 @@
+/**
+ * @id c/cert/appropriate-thread-object-storage-durations
+ * @name CON34-C: Declare objects shared between threads with appropriate storage durations
+ * @description Accessing thread-local variables with automatic storage durations can lead to
+ *              unpredictable program behavior.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/cert/id/con34-c
+ *       correctness
+ *       concurrency
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.cpp.Concurrency
+import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.dataflow.DataFlow
+
+class MallocFunctionCall extends FunctionCall {
+  MallocFunctionCall() { getTarget().getName() = "malloc" }
+}
+
+from C11ThreadCreateCall tcc, StackVariable sv, Expr arg, Expr acc
+where
+  not isExcluded(tcc, Concurrency4Package::appropriateThreadObjectStorageDurationsQuery()) and
+  tcc.getArgument(2) = arg and
+  sv.getAnAccess() = acc and
+  // a stack variable that is given as an argument to a thread
+  TaintTracking::localTaint(DataFlow::exprNode(acc), DataFlow::exprNode(arg)) and
+  // it's either not static
+  not sv.isStatic() and
+  // or isn't one of the allowed usage patterns
+  not exists(MallocFunctionCall mfc |
+    sv.getAnAssignedValue() = mfc and acc.getAPredecessor*() = mfc
+  ) and
+  not exists(TSSGetFunctionCall tsg, TSSSetFunctionCall tss, DataFlow::Node src |
+    sv.getAnAssignedValue() = tsg and
+    acc.getAPredecessor*() = tsg and
+    // there should be dataflow from somewhere (the same somewhere)
+    // into each of the first arguments
+    DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and
+    DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0)))
+  )
+select tcc, "$@ not declared with appropriate storage duration", arg, "Shared object"

c/cert/src/rules/CON34-C/ThreadObjectStorageDurationsNotInitialized.md

@@ -0,0 +1,16 @@
+# CON34-C: (Audit) Declare objects shared between threads with appropriate storage durations
+
+This query implements the CERT-C rule CON34-C:
+
+> Declare objects shared between threads with appropriate storage durations
+## CERT
+
+** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **
+
+## Implementation notes
+
+None
+
+## References
+
+* CERT-C: [CON34-C: Declare objects shared between threads with appropriate storage durations](https://wiki.sei.cmu.edu/confluence/display/c)

c/cert/src/rules/CON34-C/ThreadObjectStorageDurationsNotInitialized.ql

@@ -0,0 +1,35 @@
+/**
+ * @id c/cert/thread-object-storage-durations-not-initialized
+ * @name CON34-C: (Audit) Declare objects shared between threads with appropriate storage durations
+ * @description Storage durations not correctly initialized can cause unpredictable program
+ *              behavior.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/cert/id/con34-c
+ *       external/autosar/audit
+ *       correctness
+ *       concurrency
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.cpp.Concurrency
+import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.dataflow.DataFlow
+
+from TSSGetFunctionCall tsg, ThreadedFunction tf
+where
+  not isExcluded(tsg, Concurrency4Package::threadObjectStorageDurationsNotInitializedQuery()) and
+  // from within a threaded function there is a call to tsg
+  tsg.getEnclosingFunction() = tf and
+  // however, there does not exist a proper sequencing.
+  not exists(TSSSetFunctionCall tss, DataFlow::Node src |
+    // there should be dataflow from somewhere (the same somewhere)
+    // into each of the first arguments
+    DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and
+    DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0)))
+  )
+select tsg,
+  "Call to a thread specific storage function from within a threaded context on an object that may not be owned by this thread."

c/cert/test/rules/CON34-C/AppropriateThreadObjectStorageDurations.expected

@@ -0,0 +1,4 @@
+| main.c:23:3:23:13 | call to thrd_create | $@ not declared with appropriate storage duration | main.c:23:24:23:29 | & ... | Shared object |
+| main.c:74:3:74:13 | call to thrd_create | $@ not declared with appropriate storage duration | main.c:74:24:74:24 | p | Shared object |
+| main.c:85:3:85:13 | call to thrd_create | $@ not declared with appropriate storage duration | main.c:85:24:85:24 | p | Shared object |
+| main.c:94:3:94:13 | call to thrd_create | $@ not declared with appropriate storage duration | main.c:94:24:94:24 | p | Shared object |

c/cert/test/rules/CON34-C/AppropriateThreadObjectStorageDurations.qlref

@@ -0,0 +1 @@
+rules/CON34-C/AppropriateThreadObjectStorageDurations.ql

c/cert/test/rules/CON34-C/ThreadObjectStorageDurationsNotInitialized.expected

@@ -0,0 +1 @@
+| main.c:14:7:14:13 | call to tss_get | Call to a thread specific storage function from within a threaded context on an object that may not be owned by this thread. |

c/cert/test/rules/CON34-C/ThreadObjectStorageDurationsNotInitialized.qlref

@@ -0,0 +1 @@
+rules/CON34-C/ThreadObjectStorageDurationsNotInitialized.ql

c/cert/test/rules/CON34-C/main.c

@@ -0,0 +1,116 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <threads.h>
+
+static tss_t k;
+
+void t1(void *v) {
+  int *value = (int *)v;
+  int a = *value + 1;
+}
+
+void t2(void *v) {
+  int *value =
+      tss_get(k); // NON_COMPLIANT (AUDIT) - A threaded function without a
+                  // `tss_set` should be considered suspicious.
+  int a = *value + 1;
+}
+
+void m1() {
+  thrd_t id;
+  int value;
+
+  thrd_create(&id, t1, &value); // NON_COMPLIANT
+}
+
+void m2() {
+  thrd_t id;
+  int *value = (int *)malloc(sizeof(int));
+
+  thrd_create(&id, t1, value); // COMPLIANT - free is never called
+}
+
+void m3() {
+  thrd_t id;
+  int *value = (int *)malloc(sizeof(int));
+
+  thrd_create(&id, t1,
+              value); // COMPLIANT - free is called without synchronization,
+                      // however this is beyond the scope of this query.
+  free(value);
+}
+
+void m4() {
+  thrd_t id;
+  int *value = (int *)malloc(sizeof(int));
+
+  thrd_create(&id, t1, value); // COMPLIANT
+
+  thrd_join(id, NULL);
+
+  free(value);
+}
+
+void m5() {
+  thrd_t id;
+  int *value = (int *)malloc(sizeof(int));
+
+  tss_create(&k, free);
+  tss_set(k, value);
+
+  void *p = tss_get(k);
+
+  thrd_create(&id, t1, p); // COMPLIANT
+}
+
+void m5a() {
+  thrd_t id;
+  int *value = (int *)malloc(sizeof(int));
+
+  tss_set(k, value);
+
+  void *p = tss_get(k);
+
+  thrd_create(&id, t1, p); // NON_COMPLIANT - k not initialized.
+}
+
+void m6() {
+  thrd_t id;
+  int *value = (int *)malloc(sizeof(int));
+
+  tss_create(&k, free);
+
+  void *p = tss_get(k);
+
+  thrd_create(&id, t1, p); // NON_COMPLIANT -- get without set
+}
+
+void m6a() {
+  thrd_t id;
+  int *value = (int *)malloc(sizeof(int));
+
+  void *p = tss_get(k);
+
+  thrd_create(&id, t1, p); // NON_COMPLIANT -- get without set
+}
+
+void m7(void *v) {
+  int *value =
+      tss_get(k); // COMPLIANT (AUDIT) - A non-threaded function without a
+                  // `tss_set` should not be considered suspicious.
+  int a = *value + 1;
+}
+
+void m8() {
+  thrd_t id;
+  int *value = (int *)malloc(sizeof(int));
+  thrd_create(&id, t2,
+              value); // COMPLIANT - note that t2 (which is now a threaded
+                      // function) is NON_COMPLIANT in an audit query.
+}
+
+void m9() {
+  thrd_t id;
+  static int value = 100;
+  thrd_create(&id, t1, &value); // COMPLIANT - compliant for static values.
+}

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

@@ -806,3 +806,22 @@ class ConditionalFunction extends Function {
     exists(ConditionalVariable cv | cv.getAnAccess().getEnclosingFunction() = this)
   }
 }
+
+/**
+ * Models calls to thread specific storage function calls.
+ */
+abstract class ThreadSpecificStorageFunctionCall extends FunctionCall { }
+
+/**
+ * Models calls to `tss_get`.
+ */
+class TSSGetFunctionCall extends ThreadSpecificStorageFunctionCall {
+  TSSGetFunctionCall() { getTarget().getName() = "tss_get" }
+}
+
+/**
+ * Models calls to `tss_set`.
+ */
+class TSSSetFunctionCall extends ThreadSpecificStorageFunctionCall {
+  TSSSetFunctionCall() { getTarget().getName() = "tss_set" }
+}

cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency4.qll

@@ -0,0 +1,90 @@
+//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY.  **/
+import cpp
+import RuleMetadata
+import codingstandards.cpp.exclusions.RuleMetadata
+
+newtype Concurrency4Query =
+  TCleanUpThreadSpecificStorageQuery() or
+  TAppropriateThreadObjectStorageDurationsQuery() or
+  TThreadObjectStorageDurationsNotInitializedQuery() or
+  TThreadWasPreviouslyJoinedOrDetachedQuery() or
+  TDoNotReferToAnAtomicVariableTwiceInExpressionQuery()
+
+predicate isConcurrency4QueryMetadata(Query query, string queryId, string ruleId) {
+  query =
+    // `Query` instance for the `cleanUpThreadSpecificStorage` query
+    Concurrency4Package::cleanUpThreadSpecificStorageQuery() and
+  queryId =
+    // `@id` for the `cleanUpThreadSpecificStorage` query
+    "c/cert/clean-up-thread-specific-storage" and
+  ruleId = "CON30-C"
+  or
+  query =
+    // `Query` instance for the `appropriateThreadObjectStorageDurations` query
+    Concurrency4Package::appropriateThreadObjectStorageDurationsQuery() and
+  queryId =
+    // `@id` for the `appropriateThreadObjectStorageDurations` query
+    "c/cert/appropriate-thread-object-storage-durations" and
+  ruleId = "CON34-C"
+  or
+  query =
+    // `Query` instance for the `threadObjectStorageDurationsNotInitialized` query
+    Concurrency4Package::threadObjectStorageDurationsNotInitializedQuery() and
+  queryId =
+    // `@id` for the `threadObjectStorageDurationsNotInitialized` query
+    "c/cert/thread-object-storage-durations-not-initialized" and
+  ruleId = "CON34-C"
+  or
+  query =
+    // `Query` instance for the `threadWasPreviouslyJoinedOrDetached` query
+    Concurrency4Package::threadWasPreviouslyJoinedOrDetachedQuery() and
+  queryId =
+    // `@id` for the `threadWasPreviouslyJoinedOrDetached` query
+    "c/cert/thread-was-previously-joined-or-detached" and
+  ruleId = "CON39-C"
+  or
+  query =
+    // `Query` instance for the `doNotReferToAnAtomicVariableTwiceInExpression` query
+    Concurrency4Package::doNotReferToAnAtomicVariableTwiceInExpressionQuery() and
+  queryId =
+    // `@id` for the `doNotReferToAnAtomicVariableTwiceInExpression` query
+    "c/cert/do-not-refer-to-an-atomic-variable-twice-in-expression" and
+  ruleId = "CON40-C"
+}
+
+module Concurrency4Package {
+  Query cleanUpThreadSpecificStorageQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `cleanUpThreadSpecificStorage` query
+      TQueryC(TConcurrency4PackageQuery(TCleanUpThreadSpecificStorageQuery()))
+  }
+
+  Query appropriateThreadObjectStorageDurationsQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `appropriateThreadObjectStorageDurations` query
+      TQueryC(TConcurrency4PackageQuery(TAppropriateThreadObjectStorageDurationsQuery()))
+  }
+
+  Query threadObjectStorageDurationsNotInitializedQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `threadObjectStorageDurationsNotInitialized` query
+      TQueryC(TConcurrency4PackageQuery(TThreadObjectStorageDurationsNotInitializedQuery()))
+  }
+
+  Query threadWasPreviouslyJoinedOrDetachedQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `threadWasPreviouslyJoinedOrDetached` query
+      TQueryC(TConcurrency4PackageQuery(TThreadWasPreviouslyJoinedOrDetachedQuery()))
+  }
+
+  Query doNotReferToAnAtomicVariableTwiceInExpressionQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `doNotReferToAnAtomicVariableTwiceInExpression` query
+      TQueryC(TConcurrency4PackageQuery(TDoNotReferToAnAtomicVariableTwiceInExpressionQuery()))
+  }
+}

cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll

@@ -6,6 +6,7 @@ import Banned
 import Concurrency1
 import Concurrency2
 import Concurrency3
+import Concurrency4
 import Contracts1
 import Declarations1
 import Expressions
@@ -34,6 +35,7 @@ newtype TCQuery =
   TConcurrency1PackageQuery(Concurrency1Query q) or
   TConcurrency2PackageQuery(Concurrency2Query q) or
   TConcurrency3PackageQuery(Concurrency3Query q) or
+  TConcurrency4PackageQuery(Concurrency4Query q) or
   TContracts1PackageQuery(Contracts1Query q) or
   TDeclarations1PackageQuery(Declarations1Query q) or
   TExpressionsPackageQuery(ExpressionsQuery q) or
@@ -62,6 +64,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId) {
   isConcurrency1QueryMetadata(query, queryId, ruleId) or
   isConcurrency2QueryMetadata(query, queryId, ruleId) or
   isConcurrency3QueryMetadata(query, queryId, ruleId) or
+  isConcurrency4QueryMetadata(query, queryId, ruleId) or
   isContracts1QueryMetadata(query, queryId, ruleId) or
   isDeclarations1QueryMetadata(query, queryId, ruleId) or
   isExpressionsQueryMetadata(query, queryId, ruleId) or