diff --git a/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.expected b/c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.expected similarity index 100% rename from c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.expected rename to c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.expected diff --git a/c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql b/c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql new file mode 100644 index 0000000000..05d148fb1f --- /dev/null +++ b/c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql @@ -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 diff --git a/c/common/test/rules/possibledataracebetweenthreadsshared/test.c b/c/common/test/rules/possibledataracebetweenthreadsshared/test.c new file mode 100644 index 0000000000..5f392105e6 --- /dev/null +++ b/c/common/test/rules/possibledataracebetweenthreadsshared/test.c @@ -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); + } +} \ No newline at end of file diff --git a/c/misra/src/rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql b/c/misra/src/rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql index edf3705a9b..187c926278 100644 --- a/c/misra/src/rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql +++ b/c/misra/src/rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql @@ -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 diff --git a/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.qlref b/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.qlref deleted file mode 100644 index 737cf79505..0000000000 --- a/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.qlref +++ /dev/null @@ -1 +0,0 @@ -rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql \ No newline at end of file diff --git a/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.testref b/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.testref new file mode 100644 index 0000000000..c15a87a20e --- /dev/null +++ b/c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.testref @@ -0,0 +1 @@ +c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql diff --git a/change_notes/2026-03-09-possible-data-race-between-threads-shared.md b/change_notes/2026-03-09-possible-data-race-between-threads-shared.md new file mode 100644 index 0000000000..7580a1fc2a --- /dev/null +++ b/change_notes/2026-03-09-possible-data-race-between-threads-shared.md @@ -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`. diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Undefined.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Undefined.qll index 1d8dc022ac..37ae63fa53 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Undefined.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Undefined.qll @@ -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())) + } } diff --git a/cpp/common/src/codingstandards/cpp/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.qll b/cpp/common/src/codingstandards/cpp/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.qll new file mode 100644 index 0000000000..966d6bcba3 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.qll @@ -0,0 +1,178 @@ +/** + * Provides a configurable module PossibleDataRaceBetweenThreadsShared with a `problems` predicate + * for the following issue: + * Threads shall not access the same memory location concurrently without utilization + * of thread synchronization objects. + */ + +import cpp +import codingstandards.cpp.Customizations +import codingstandards.cpp.Exclusions +import codingstandards.cpp.Concurrency +import codingstandards.cpp.lifetimes.StorageDuration + +signature module PossibleDataRaceBetweenThreadsSharedConfigSig { + Query getQuery(); + + class ObjectIdentity extends Element { + StorageDuration getStorageDuration(); + } + + class SubObject { + string toString(); + + ObjectIdentity getRootIdentity(); + + Expr getAnAccess(); + } +} + +module PossibleDataRaceBetweenThreadsShared { + newtype TNonReentrantOperation = + TReadWrite(Config::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" + ]) + } + + class NonReentrantOperation extends TNonReentrantOperation { + string toString() { + exists(Config::SubObject object | + this = TReadWrite(object) and + result = object.toString() + ) + or + exists(FunctionCall call | + this = TStdFunctionCall(call) and + result = call.getTarget().getName() + ) + } + + Expr getAReadExpr() { + exists(Config::SubObject object | + this = TReadWrite(object) and + result = object.getAnAccess() + ) + or + this = TStdFunctionCall(result) + } + + Expr getAWriteExpr() { + exists(Config::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(Config::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 + ) + ) + ) + } + + query predicate problems( + Expr write, string message, Element operationSource, string operationName, WritingThread wt, + string wtName, Expr read, string readConcurrent, ReadingThread rt, string rtName + ) { + exists(NonReentrantOperation operation, string writeString, string readString | + not isExcluded(write, Config::getQuery()) 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 + operationSource = operation.getSourceElement() and + operationName = operation.toString() and + wtName = wt.getName() and + readConcurrent = "concurrent " + readString and + rtName = rt.getName() 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 $@." + ) + } +} diff --git a/cpp/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.expected b/cpp/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.expected new file mode 100644 index 0000000000..f928076b1c --- /dev/null +++ b/cpp/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.expected @@ -0,0 +1,20 @@ +| test.cpp:39:3:39:8 | ... = ... | Threaded write to object $@ from thread function $@ is not synchronized with $@ from thread function $@. | test.cpp:19:5:19:6 | g2 | g2 | test.cpp:38:6:38:29 | single_thread4_writes_g2 | single_thread4_writes_g2 | test.cpp:35:3:35:4 | g2 | concurrent read operation | test.cpp:34:6:34:28 | single_thread3_reads_g2 | single_thread3_reads_g2 | +| test.cpp:43:3:43:8 | ... = ... | Threaded write to object $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:20:5:20:6 | g3 | g3 | test.cpp:42:6:42:27 | many_thread5_writes_g3 | many_thread5_writes_g3 | test.cpp:43:3:43:4 | g3 | concurrent read operation | test.cpp:42:6:42:27 | many_thread5_writes_g3 | many_thread5_writes_g3 | +| test.cpp:79:3:79:11 | ... = ... | Threaded write to object $@ from thread function $@ is not synchronized with $@ from thread function $@. | test.cpp:76:3:76:4 | g7 | g7.m1 | test.cpp:78:6:78:33 | single_thread11_writes_g7_m1 | single_thread11_writes_g7_m1 | test.cpp:83:6:83:7 | m1 | concurrent read operation | test.cpp:82:6:82:33 | single_thread12_writes_g7_m1 | single_thread12_writes_g7_m1 | +| test.cpp:83:3:83:11 | ... = ... | Threaded write to object $@ from thread function $@ is not synchronized with $@ from thread function $@. | test.cpp:76:3:76:4 | g7 | g7.m1 | test.cpp:82:6:82:33 | single_thread12_writes_g7_m1 | single_thread12_writes_g7_m1 | test.cpp:79:6:79:7 | m1 | concurrent read operation | test.cpp:78:6:78:33 | single_thread11_writes_g7_m1 | single_thread11_writes_g7_m1 | +| test.cpp:90:3:90:16 | call to setlocale | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:90:3:90:16 | call to setlocale | setlocale | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:90:3:90:16 | call to setlocale | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:91:3:91:11 | call to setlocale | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:91:3:91:11 | call to setlocale | setlocale | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:91:3:91:11 | call to setlocale | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:92:3:92:13 | call to tmpnam | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:92:3:92:13 | call to tmpnam | tmpnam | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:92:3:92:13 | call to tmpnam | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:93:3:93:6 | call to rand | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:93:3:93:6 | call to rand | rand | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:93:3:93:6 | call to rand | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:94:3:94:11 | call to rand | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:94:3:94:11 | call to rand | rand | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:94:3:94:11 | call to rand | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:96:3:96:8 | call to getenv | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:96:3:96:8 | call to getenv | getenv | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:96:3:96:8 | call to getenv | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:101:3:101:10 | call to strerror | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:101:3:101:10 | call to strerror | strerror | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:101:3:101:10 | call to strerror | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:102:3:102:15 | call to strerror | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:102:3:102:15 | call to strerror | strerror | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:102:3:102:15 | call to strerror | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:103:3:103:9 | call to asctime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:103:3:103:9 | call to asctime | asctime | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:103:3:103:9 | call to asctime | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:104:3:104:14 | call to asctime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:104:3:104:14 | call to asctime | asctime | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:104:3:104:14 | call to asctime | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:105:3:105:7 | call to ctime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:105:3:105:7 | call to ctime | ctime | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:105:3:105:7 | call to ctime | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:106:3:106:12 | call to ctime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:106:3:106:12 | call to ctime | ctime | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:106:3:106:12 | call to ctime | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:107:3:107:8 | call to gmtime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:107:3:107:8 | call to gmtime | gmtime | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:107:3:107:8 | call to gmtime | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:108:3:108:13 | call to gmtime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:108:3:108:13 | call to gmtime | gmtime | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:108:3:108:13 | call to gmtime | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:109:3:109:11 | call to localtime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:109:3:109:11 | call to localtime | localtime | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:109:3:109:11 | call to localtime | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | +| test.cpp:110:3:110:16 | call to localtime | Threaded call to non-reentrant function $@ not synchronized from thread function $@ spawned from a loop. | test.cpp:110:3:110:16 | call to localtime | localtime | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | test.cpp:110:3:110:16 | call to localtime | concurrent call to non-reentrant function | test.cpp:86:6:86:43 | many_thread13_calls_nonreentrant_funcs | many_thread13_calls_nonreentrant_funcs | diff --git a/cpp/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql b/cpp/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql new file mode 100644 index 0000000000..7f536d4863 --- /dev/null +++ b/cpp/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql @@ -0,0 +1,14 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.possibledataracebetweenthreadsshared.PossibleDataRaceBetweenThreadsShared +import codingstandards.cpp.lifetimes.CppObjects as CppObjects +import codingstandards.cpp.lifetimes.CppSubObjects as CppSubObjects + +module TestFileConfig implements PossibleDataRaceBetweenThreadsSharedConfigSig { + Query getQuery() { result instanceof TestQuery } + + class ObjectIdentity = CppObjects::ObjectIdentity; + + class SubObject = CppSubObjects::SubObject; +} + +import PossibleDataRaceBetweenThreadsShared diff --git a/cpp/common/test/rules/possibledataracebetweenthreadsshared/test.cpp b/cpp/common/test/rules/possibledataracebetweenthreadsshared/test.cpp new file mode 100644 index 0000000000..e2943c9a1b --- /dev/null +++ b/cpp/common/test/rules/possibledataracebetweenthreadsshared/test.cpp @@ -0,0 +1,153 @@ +#include "locale.h" +#include "stdio.h" +#include "stdlib.h" +#include "string.h" +#include "time.h" +#include "uchar.h" +#include "wchar.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +int g1; +int g2; +int g3; +int g4; +std::mutex g4_lock; +int g5; +std::mutex 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) { + g4_lock.lock(); + g4; // COMPLIANT +} + +void single_thread7_writes_g4_locked(void *p) { + g4_lock.lock(); + g4 = 1; // COMPLIANT +} + +void many_thread8_writes_g5_locked(void *p) { + g5_lock.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) { + // Copied from the c version. + // Not all of these functions are defined, which c allows but cpp does not. + // Not all are defined in std:: in our stubs. + std::setlocale(LC_ALL, "C"); // NON-COMPLIANT + setlocale(LC_ALL, "C"); // NON-COMPLIANT + std::tmpnam(""); // NON-COMPLIANT + rand(); // NON-COMPLIANT + std::rand(); // NON-COMPLIANT + // srand(0); // NON-COMPLIANT + getenv("PATH"); // NON-COMPLIANT + // std::getenv("PATH"); // NON-COMPLIANT + ////getenv_s(NULL, NULL, 0, NULL); // NON-COMPLIANT + strtok("a", "b"); // NON-COMPLIANT[False negative] + std::strtok("a", "b"); // NON-COMPLIANT[False negative] + strerror(0); // NON-COMPLIANT + std::strerror(0); // NON-COMPLIANT + asctime(NULL); // NON-COMPLIANT + std::asctime(NULL); // NON-COMPLIANT + ctime(NULL); // NON-COMPLIANT + std::ctime(NULL); // NON-COMPLIANT + gmtime(NULL); // NON-COMPLIANT + std::gmtime(NULL); // NON-COMPLIANT + localtime(NULL); // NON-COMPLIANT + std::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[]) { + std::thread single_thread1; + std::thread many_thread2; + std::thread single_thread3; + std::thread single_thread4; + std::thread many_thread5; + std::thread single_thread6; + std::thread single_thread7; + std::thread many_thread8; + std::thread single_thread9; + std::thread single_thread10; + std::thread single_thread11; + std::thread single_thread12; + std::thread many_thread13; + + single_thread1 = std::thread(single_thread1_reads_g1, nullptr); + single_thread3 = std::thread(single_thread3_reads_g2, nullptr); + single_thread4 = std::thread(single_thread4_writes_g2, nullptr); + single_thread6 = std::thread(single_thread6_reads_g4_locked, nullptr); + single_thread7 = std::thread(single_thread7_writes_g4_locked, nullptr); + single_thread9 = std::thread(single_thread9_writes_g6_m1, nullptr); + single_thread10 = std::thread(single_thread10_writes_g6_m2, nullptr); + single_thread11 = std::thread(single_thread11_writes_g7_m1, nullptr); + single_thread12 = std::thread(single_thread12_writes_g7_m1, nullptr); + for (;;) { + many_thread2 = std::thread(many_thread2_reads_g1, nullptr); + many_thread5 = std::thread(many_thread5_writes_g3, nullptr); + many_thread8 = std::thread(many_thread8_writes_g5_locked, nullptr); + many_thread13 = + std::thread(many_thread13_calls_nonreentrant_funcs, nullptr); + } +} \ No newline at end of file diff --git a/cpp/misra/src/rules/RULE-4-1-3/PossibleDataRaceBetweenThreads.ql b/cpp/misra/src/rules/RULE-4-1-3/PossibleDataRaceBetweenThreads.ql new file mode 100644 index 0000000000..ee76b4ce5d --- /dev/null +++ b/cpp/misra/src/rules/RULE-4-1-3/PossibleDataRaceBetweenThreads.ql @@ -0,0 +1,32 @@ +/** + * @id cpp/misra/possible-data-race-between-threads + * @name RULE-4-1-3: Data races between threads lead to undefined behavior + * @description Threads accessing the same memory location concurrently without utilization of + * thread synchronization objects results in undefined behavior. + * @kind problem + * @precision medium + * @problem.severity error + * @tags external/misra/id/rule-4-1-3 + * correctness + * concurrency + * scope/system + * external/misra/enforcement/undecidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.lifetimes.CppObjects as CppObjects +import codingstandards.cpp.lifetimes.CppSubObjects as CppSubObjects +import codingstandards.cpp.rules.possibledataracebetweenthreadsshared.PossibleDataRaceBetweenThreadsShared + +module PossibleDataRaceBetweenThreadsConfig implements PossibleDataRaceBetweenThreadsSharedConfigSig +{ + Query getQuery() { result = UndefinedPackage::possibleDataRaceBetweenThreadsQuery() } + + class ObjectIdentity = CppObjects::ObjectIdentity; + + class SubObject = CppSubObjects::SubObject; +} + +import PossibleDataRaceBetweenThreadsShared diff --git a/cpp/misra/test/rules/RULE-4-1-3/PossibleDataRaceBetweenThreads.testref b/cpp/misra/test/rules/RULE-4-1-3/PossibleDataRaceBetweenThreads.testref new file mode 100644 index 0000000000..072317902b --- /dev/null +++ b/cpp/misra/test/rules/RULE-4-1-3/PossibleDataRaceBetweenThreads.testref @@ -0,0 +1 @@ +cpp/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql \ No newline at end of file diff --git a/rule_packages/c/Concurrency9.json b/rule_packages/c/Concurrency9.json index 6ae1df8173..0422887ce0 100644 --- a/rule_packages/c/Concurrency9.json +++ b/rule_packages/c/Concurrency9.json @@ -11,6 +11,7 @@ "name": "There shall be no data races between threads", "precision": "medium", "severity": "error", + "shared_implementation_short_name": "PossibleDataRaceBetweenThreadsShared", "short_name": "PossibleDataRaceBetweenThreads", "tags": [ "correctness", diff --git a/rule_packages/cpp/Undefined.json b/rule_packages/cpp/Undefined.json index 2a3f96342c..bc0b10af3d 100644 --- a/rule_packages/cpp/Undefined.json +++ b/rule_packages/cpp/Undefined.json @@ -55,6 +55,20 @@ "scope/system", "external/misra/audit" ] + }, + { + "description": "Threads accessing the same memory location concurrently without utilization of thread synchronization objects results in undefined behavior.", + "kind": "problem", + "name": "Data races between threads lead to undefined behavior", + "precision": "medium", + "severity": "error", + "shared_implementation_short_name": "PossibleDataRaceBetweenThreadsShared", + "short_name": "PossibleDataRaceBetweenThreads", + "tags": [ + "correctness", + "concurrency", + "scope/system" + ] } ], "title": "There shall be no occurrence of undefined or critical unspecified behaviour"