Michaelrfairhurst/rule 4 1 3 detect data races (#1077)

* Detect data races

* Commit missing files

* Regenenate metadata

Author: MichaelRFairhurst

…/PossibleDataRaceBetweenThreads.expected …bleDataRaceBetweenThreadsShared.expectedc/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.expected renamed to c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.expected c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.expected renamed to c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.expected

@@ -0,0 +1,14 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.possibledataracebetweenthreadsshared.PossibleDataRaceBetweenThreadsShared
+import codingstandards.c.Objects as CObjects
+import codingstandards.c.SubObjects as CSubObjects
+
+module TestFileConfig implements PossibleDataRaceBetweenThreadsSharedConfigSig {
+  Query getQuery() { result instanceof TestQuery }
+
+  class ObjectIdentity = CObjects::ObjectIdentity;
+
+  class SubObject = CSubObjects::SubObject;
+}
+
+import PossibleDataRaceBetweenThreadsShared<TestFileConfig>

c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql

@@ -0,0 +1,132 @@
+#include "locale.h"
+#include "stdio.h"
+#include "stdlib.h"
+#include "string.h"
+#include "threads.h"
+#include "time.h"
+#include "uchar.h"
+#include "wchar.h"
+
+int g1;
+int g2;
+int g3;
+int g4;
+mtx_t g4_lock;
+int g5;
+mtx_t g5_lock;
+
+void single_thread1_reads_g1(void *p) {
+  g1; // COMPLIANT
+}
+
+void many_thread2_reads_g1(void *p) {
+  g1; // COMPLIANT
+}
+
+void single_thread3_reads_g2(void *p) {
+  g2; // COMPLIANT
+}
+
+void single_thread4_writes_g2(void *p) {
+  g2 = 1; // NON-COMPLIANT
+}
+
+void many_thread5_writes_g3(void *p) {
+  g3 = 1; // NON-COMPLIANT
+}
+
+void single_thread6_reads_g4_locked(void *p) {
+  mtx_lock(&g4_lock);
+  g4; // COMPLIANT
+}
+
+void single_thread7_writes_g4_locked(void *p) {
+  mtx_lock(&g4_lock);
+  g4 = 1; // COMPLIANT
+}
+
+void many_thread8_writes_g5_locked(void *p) {
+  mtx_lock(&g5_lock);
+  g5 = 1; // COMPLIANT
+}
+
+struct {
+  int m1;
+  int m2;
+} g6;
+
+void single_thread9_writes_g6_m1(void *p) {
+  g6.m1 = 1; // COMPLIANT
+}
+
+void single_thread10_writes_g6_m2(void *p) {
+  g6.m2 = 1; // COMPLIANT
+}
+
+struct {
+  int m1;
+} g7;
+
+void single_thread11_writes_g7_m1(void *p) {
+  g7.m1 = 1; // NON-COMPLIANT
+}
+
+void single_thread12_writes_g7_m1(void *p) {
+  g7.m1 = 1; // NON-COMPLIANT
+}
+
+void many_thread13_calls_nonreentrant_funcs(void *p) {
+  setlocale(LC_ALL, "C");         // NON-COMPLIANT
+  tmpnam("");                     // NON-COMPLIANT
+  rand();                         // NON-COMPLIANT
+  srand(0);                       // NON-COMPLIANT
+  getenv("PATH");                 // NON-COMPLIANT
+  getenv_s(NULL, NULL, 0, NULL);  // NON-COMPLIANT
+  strtok("a", "b");               // NON-COMPLIANT
+  strerror(0);                    // NON-COMPLIANT
+  asctime(NULL);                  // NON-COMPLIANT
+  ctime(NULL);                    // NON-COMPLIANT
+  gmtime(NULL);                   // NON-COMPLIANT
+  localtime(NULL);                // NON-COMPLIANT
+  mbrtoc16(NULL, NULL, 0, NULL);  // NON-COMPLIANT
+  mbrtoc32(NULL, NULL, 0, NULL);  // NON-COMPLIANT
+  c16rtomb(NULL, 0, NULL);        // NON-COMPLIANT
+  c32rtomb(NULL, 0, NULL);        // NON-COMPLIANT
+  mbrlen(NULL, 0, NULL);          // NON-COMPLIANT
+  mbrtowc(NULL, NULL, 0, NULL);   // NON-COMPLIANT
+  wcrtomb(NULL, 0, NULL);         // NON-COMPLIANT
+  mbsrtowcs(NULL, NULL, 0, NULL); // NON-COMPLIANT
+  wcsrtombs(NULL, NULL, 0, NULL); // NON-COMPLIANT
+}
+
+int main(int argc, char *argv[]) {
+  thrd_t single_thread1;
+  thrd_t many_thread2;
+  thrd_t single_thread3;
+  thrd_t single_thread4;
+  thrd_t many_thread5;
+  thrd_t single_thread6;
+  thrd_t single_thread7;
+  thrd_t many_thread8;
+  thrd_t single_thread9;
+  thrd_t single_thread10;
+  thrd_t single_thread11;
+  thrd_t single_thread12;
+  thrd_t many_thread13;
+
+  thrd_create(&single_thread1, single_thread1_reads_g1, NULL);
+  thrd_create(&single_thread3, single_thread3_reads_g2, NULL);
+  thrd_create(&single_thread4, single_thread4_writes_g2, NULL);
+  thrd_create(&single_thread6, single_thread6_reads_g4_locked, NULL);
+  thrd_create(&single_thread7, single_thread7_writes_g4_locked, NULL);
+  thrd_create(&single_thread9, single_thread9_writes_g6_m1, NULL);
+  thrd_create(&single_thread10, single_thread10_writes_g6_m2, NULL);
+  thrd_create(&single_thread11, single_thread11_writes_g7_m1, NULL);
+  thrd_create(&single_thread12, single_thread12_writes_g7_m1, NULL);
+  for (;;) {
+    thrd_create(&many_thread2, many_thread2_reads_g1, NULL);
+    thrd_create(&many_thread5, many_thread5_writes_g3, NULL);
+    thrd_create(&many_thread8, many_thread8_writes_g5_locked, NULL);
+    thrd_create(&many_thread13, many_thread13_calls_nonreentrant_funcs, NULL);
+  }
+}

c/common/test/rules/possibledataracebetweenthreadsshared/test.c

@@ -15,148 +15,17 @@
 
 import cpp
 import codingstandards.c.misra
-import codingstandards.c.Objects
-import codingstandards.c.SubObjects
-import codingstandards.cpp.Concurrency
+import codingstandards.c.Objects as CObjects
+import codingstandards.c.SubObjects as CSubObjects
+import codingstandards.cpp.rules.possibledataracebetweenthreadsshared.PossibleDataRaceBetweenThreadsShared
 
-newtype TNonReentrantOperation =
-  TReadWrite(SubObject object) {
-    object.getRootIdentity().getStorageDuration().isStatic()
-    or
-    object.getRootIdentity().getStorageDuration().isAllocated()
-  } or
-  TStdFunctionCall(FunctionCall call) {
-    call.getTarget()
-        .hasName([
-            "setlocale", "tmpnam", "rand", "srand", "getenv", "getenv_s", "strok", "strerror",
-            "asctime", "ctime", "gmtime", "localtime", "mbrtoc16", "c16rtomb", "mbrtoc32",
-            "c32rtomb", "mbrlen", "mbrtowc", "wcrtomb", "mbsrtowcs", "wcsrtombs"
-          ])
-  }
+module PossibleDataRaceBetweenThreadsConfig implements PossibleDataRaceBetweenThreadsSharedConfigSig
+{
+  Query getQuery() { result = Concurrency9Package::possibleDataRaceBetweenThreadsQuery() }
 
-class NonReentrantOperation extends TNonReentrantOperation {
-  string toString() {
-    exists(SubObject object |
-      this = TReadWrite(object) and
-      result = object.toString()
-    )
-    or
-    exists(FunctionCall call |
-      this = TStdFunctionCall(call) and
-      result = call.getTarget().getName()
-    )
-  }
+  class ObjectIdentity = CObjects::ObjectIdentity;
 
-  Expr getAReadExpr() {
-    exists(SubObject object |
-      this = TReadWrite(object) and
-      result = object.getAnAccess()
-    )
-    or
-    this = TStdFunctionCall(result)
-  }
-
-  Expr getAWriteExpr() {
-    exists(SubObject object, Assignment assignment |
-      this = TReadWrite(object) and
-      result = assignment and
-      assignment.getLValue() = object.getAnAccess()
-    )
-    or
-    this = TStdFunctionCall(result)
-  }
-
-  string getReadString() {
-    this = TReadWrite(_) and
-    result = "read operation"
-    or
-    this = TStdFunctionCall(_) and
-    result = "call to non-reentrant function"
-  }
-
-  string getWriteString() {
-    this = TReadWrite(_) and
-    result = "write to object"
-    or
-    this = TStdFunctionCall(_) and
-    result = "call to non-reentrant function"
-  }
-
-  Element getSourceElement() {
-    exists(SubObject object |
-      this = TReadWrite(object) and
-      result = object.getRootIdentity()
-    )
-    or
-    this = TStdFunctionCall(result)
-  }
-}
-
-class WritingThread extends ThreadedFunction {
-  NonReentrantOperation aWriteObject;
-  Expr aWriteExpr;
-
-  WritingThread() {
-    aWriteExpr = aWriteObject.getAWriteExpr() and
-    // This function directly contains the write expression, or transitively calls the function
-    // that contains the write expression.
-    this.calls*(aWriteExpr.getEnclosingFunction()) and
-    // The write isn't synchronized with a mutex or condition object.
-    not aWriteExpr instanceof LockProtectedControlFlowNode and
-    // The write doesn't seem to be during a special initialization phase of the program.
-    not aWriteExpr.getEnclosingFunction().getName().matches(["%init%", "%boot%", "%start%"])
-  }
-
-  Expr getAWriteExpr() { result = aWriteExpr }
-}
-
-class ReadingThread extends ThreadedFunction {
-  Expr aReadExpr;
-
-  ReadingThread() {
-    exists(NonReentrantOperation op |
-      aReadExpr = op.getAReadExpr() and
-      this.calls*(aReadExpr.getEnclosingFunction()) and
-      not aReadExpr instanceof LockProtectedControlFlowNode
-    )
-  }
-
-  Expr getAReadExpr() { result = aReadExpr }
-}
-
-predicate mayBeDataRace(Expr write, Expr read, NonReentrantOperation operation) {
-  exists(WritingThread wt |
-    wt.getAWriteExpr() = write and
-    write = operation.getAWriteExpr() and
-    exists(ReadingThread rt |
-      read = rt.getAReadExpr() and
-      read = operation.getAReadExpr() and
-      (
-        wt.isMultiplySpawned() or
-        not wt = rt
-      )
-    )
-  )
+  class SubObject = CSubObjects::SubObject;
 }
 
-from
-  WritingThread wt, ReadingThread rt, Expr write, Expr read, NonReentrantOperation operation,
-  string message, string writeString, string readString
-where
-  not isExcluded(write, Concurrency9Package::possibleDataRaceBetweenThreadsQuery()) and
-  mayBeDataRace(write, read, operation) and
-  wt = min(WritingThread f | f.getAWriteExpr() = write | f order by f.getName()) and
-  rt = min(ReadingThread f | f.getAReadExpr() = read | f order by f.getName()) and
-  writeString = operation.getWriteString() and
-  readString = operation.getReadString() and
-  if wt.isMultiplySpawned()
-  then
-    message =
-      "Threaded " + writeString +
-        " $@ not synchronized from thread function $@ spawned from a loop."
-  else
-    message =
-      "Threaded " + writeString +
-        " $@ from thread function $@ is not synchronized with $@ from thread function $@."
-select write, message, operation.getSourceElement(), operation.toString(), wt, wt.getName(), read,
-  "concurrent " + readString, rt, rt.getName()
+import PossibleDataRaceBetweenThreadsShared<PossibleDataRaceBetweenThreadsConfig>

c/misra/src/rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql

@@ -0,0 +1 @@
+c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql

c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.qlref

@@ -0,0 +1,2 @@
+ - `DIR-5-1` - `PossibleDataRaceBetweenThreads.ql`:
+   - Refactored implementation into a shared library (`PossibleDataRaceBetweenThreadsShared.qll`) to allow reuse by MISRA C++ 2023 `RULE-4-1-3`. No change in results is expected for `DIR-5-1`.

c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.testref

@@ -7,7 +7,8 @@ newtype UndefinedQuery =
   TUndefinedBehaviorQuery() or
   TCriticalUnspecifiedBehaviorQuery() or
   TUndefinedBehaviorAuditQuery() or
-  TCriticalUnspecifiedBehaviorAuditQuery()
+  TCriticalUnspecifiedBehaviorAuditQuery() or
+  TPossibleDataRaceBetweenThreadsQuery()
 
 predicate isUndefinedQueryMetadata(Query query, string queryId, string ruleId, string category) {
   query =
@@ -45,6 +46,15 @@ predicate isUndefinedQueryMetadata(Query query, string queryId, string ruleId, s
     "cpp/misra/critical-unspecified-behavior-audit" and
   ruleId = "RULE-4-1-3" and
   category = "required"
+  or
+  query =
+    // `Query` instance for the `possibleDataRaceBetweenThreads` query
+    UndefinedPackage::possibleDataRaceBetweenThreadsQuery() and
+  queryId =
+    // `@id` for the `possibleDataRaceBetweenThreads` query
+    "cpp/misra/possible-data-race-between-threads" and
+  ruleId = "RULE-4-1-3" and
+  category = "required"
 }
 
 module UndefinedPackage {
@@ -75,4 +85,11 @@ module UndefinedPackage {
       // `Query` type for `criticalUnspecifiedBehaviorAudit` query
       TQueryCPP(TUndefinedPackageQuery(TCriticalUnspecifiedBehaviorAuditQuery()))
   }
+
+  Query possibleDataRaceBetweenThreadsQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `possibleDataRaceBetweenThreads` query
+      TQueryCPP(TUndefinedPackageQuery(TPossibleDataRaceBetweenThreadsQuery()))
+  }
 }