From 9ff699b63c073f5be42b6007864e605a01bc9dc1 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Sun, 17 Nov 2024 16:58:14 -0800 Subject: [PATCH 1/7] Implement Concurrency7 package --- .../includes/standard-library/stdatomic.h | 1 + .../TimedlockOnInappropriateMutexType.ql | 74 +++++++++++++++++++ .../RULE-9-7/UninitializedAtomicObject.ql | 73 ++++++++++++++++++ ...TimedlockOnInappropriateMutexType.expected | 45 +++++++++++ .../TimedlockOnInappropriateMutexType.qlref | 1 + c/misra/test/rules/RULE-21-26/test.c | 45 +++++++++++ .../UninitializedAtomicObject.expected | 3 + .../RULE-9-7/UninitializedAtomicObject.qlref | 1 + c/misra/test/rules/RULE-9-7/test.c | 34 +++++++++ .../cpp/exclusions/c/Concurrency7.qll | 44 +++++++++++ .../cpp/exclusions/c/RuleMetadata.qll | 3 + rule_packages/c/Concurrency7.json | 45 +++++++++++ rules.csv | 4 +- 13 files changed, 371 insertions(+), 2 deletions(-) create mode 100644 c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql create mode 100644 c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql create mode 100644 c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.expected create mode 100644 c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.qlref create mode 100644 c/misra/test/rules/RULE-21-26/test.c create mode 100644 c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.expected create mode 100644 c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.qlref create mode 100644 c/misra/test/rules/RULE-9-7/test.c create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency7.qll create mode 100644 rule_packages/c/Concurrency7.json diff --git a/c/common/test/includes/standard-library/stdatomic.h b/c/common/test/includes/standard-library/stdatomic.h index 66b74ae61a..229b8db906 100644 --- a/c/common/test/includes/standard-library/stdatomic.h +++ b/c/common/test/includes/standard-library/stdatomic.h @@ -5,5 +5,6 @@ #define atomic_store(a, b) 0 #define atomic_store_explicit(a, b, c) 0 #define ATOMIC_VAR_INIT(value) (value) +#define atomic_init __c11_atomic_init #define atomic_is_lock_free(obj) __c11_atomic_is_lock_free(sizeof(*(obj))) typedef _Atomic(int) atomic_int; \ No newline at end of file diff --git a/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql b/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql new file mode 100644 index 0000000000..d8d465045e --- /dev/null +++ b/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql @@ -0,0 +1,74 @@ +/** + * @id c/misra/timedlock-on-inappropriate-mutex-type + * @name RULE-21-26: The Standard Library function mtx_timedlock() shall only be invoked on mutexes of type mtx_timed + * @description The Standard Library function mtx_timedlock() shall only be invoked on mutex objects + * of appropriate mutex type + * @kind path-problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-21-26 + * correctness + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import semmle.code.cpp.dataflow.new.DataFlow + +class MutexTimed extends EnumConstant { + MutexTimed() { hasName("mtx_timed") } +} + +class MutexInitCall extends FunctionCall { + Expr mutexExpr; + Expr mutexTypeExpr; + + MutexInitCall() { + getTarget().hasName("mtx_init") and + mutexExpr = getArgument(0) and + mutexTypeExpr = getArgument(1) + } + + predicate isTimedMutexType() { + exists(EnumConstantAccess baseTypeAccess | + ( + baseTypeAccess = mutexTypeExpr + or + baseTypeAccess = mutexTypeExpr.(BinaryBitwiseOperation).getAnOperand() + ) and + baseTypeAccess.getTarget() instanceof MutexTimed + ) + or + mutexTypeExpr.getValue().toInt() = any(MutexTimed m).getValue().toInt() + } + + Expr getMutexExpr() { result = mutexExpr } +} + +module MutexTimedlockFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { + exists(MutexInitCall init | + node.asDefiningArgument() = init.getMutexExpr() and not init.isTimedMutexType() + ) + } + + predicate isSink(DataFlow::Node node) { + exists(FunctionCall fc | + fc.getTarget().hasName("mtx_timedlock") and + node.asIndirectExpr() = fc.getArgument(0) + ) + } +} + +module Flow = DataFlow::Global; + +import Flow::PathGraph + +from Flow::PathNode source, Flow::PathNode sink +where + not isExcluded(sink.getNode().asExpr(), + Concurrency7Package::timedlockOnInappropriateMutexTypeQuery()) and + Flow::flowPath(source, sink) +select sink.getNode(), source, sink, "Call to mtx_timedlock with mutex not of type 'mtx_timed'." diff --git a/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql b/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql new file mode 100644 index 0000000000..40f833a740 --- /dev/null +++ b/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql @@ -0,0 +1,73 @@ +/** + * @id c/misra/uninitialized-atomic-object + * @name RULE-9-7: Atomic objects shall be appropriately initialized before being accessed + * @description Atomic objects that do not have static storage duration shall be initialized with a + * value or by using 'atomic_init()'. + * @kind problem + * @precision high + * @problem.severity warning + * @tags external/misra/id/rule-9-7 + * concurrency + * external/misra/c/2012/amendment4 + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra +import semmle.code.cpp.controlflow.Dominance + +class ThreadSpawningFunction extends Function { + ThreadSpawningFunction() { + this.hasName("pthread_create") or + this.hasName("thrd_create") or + exists(FunctionCall fc | + fc.getTarget() instanceof ThreadSpawningFunction and + fc.getEnclosingFunction() = this) + } +} + +class AtomicInitAddressOfExpr extends FunctionCall { + Expr addressedExpr; + + AtomicInitAddressOfExpr() { + exists(AddressOfExpr addrOf | + getArgument(0) = addrOf and + addrOf.getOperand() = addressedExpr and + getTarget().getName() = "__c11_atomic_init" + ) + } + + Expr getAddressedExpr() { + result = addressedExpr + } +} + +ControlFlowNode getARequiredInitializationPoint(LocalScopeVariable v) { + result = v.getParentScope().(BlockStmt).getFollowingStmt() + or + exists(DeclStmt decl | + decl.getADeclaration() = v and + result = any(FunctionCall fc + | fc.getTarget() instanceof ThreadSpawningFunction and + fc.getEnclosingBlock().getEnclosingBlock*() = v.getParentScope() and + fc.getAPredecessor*() = decl + ) + ) +} + +from VariableDeclarationEntry decl, Variable v +where + not isExcluded(decl, Concurrency7Package::uninitializedAtomicObjectQuery()) and + v = decl.getVariable() and + v.getUnderlyingType().hasSpecifier("atomic") and + not v.isTopLevel() and + not exists(v.getInitializer()) and + exists(ControlFlowNode missingInitPoint | + missingInitPoint = getARequiredInitializationPoint(v) + and not exists(AtomicInitAddressOfExpr initialization | + initialization.getAddressedExpr().(VariableAccess).getTarget() = v and + dominates(initialization, missingInitPoint) + ) + ) +select decl, + "Atomic object '" + v.getName() + "' has no initializer or corresponding use of 'atomic_init()'." diff --git a/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.expected b/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.expected new file mode 100644 index 0000000000..442f20bf73 --- /dev/null +++ b/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.expected @@ -0,0 +1,45 @@ +edges +| test.c:10:24:10:24 | *m | test.c:10:43:10:43 | *m | provenance | | +| test.c:13:12:13:14 | mtx_init output argument | test.c:14:17:14:19 | *& ... | provenance | | +| test.c:13:12:13:14 | mtx_init output argument | test.c:15:14:15:16 | *& ... | provenance | | +| test.c:15:14:15:16 | *& ... | test.c:10:24:10:24 | *m | provenance | | +| test.c:17:12:17:14 | mtx_init output argument | test.c:18:17:18:19 | *& ... | provenance | | +| test.c:17:12:17:14 | mtx_init output argument | test.c:19:14:19:16 | *& ... | provenance | | +| test.c:19:14:19:16 | *& ... | test.c:10:24:10:24 | *m | provenance | | +| test.c:30:12:30:14 | mtx_init output argument | test.c:31:17:31:19 | *& ... | provenance | | +| test.c:30:12:30:14 | mtx_init output argument | test.c:32:14:32:16 | *& ... | provenance | | +| test.c:32:14:32:16 | *& ... | test.c:10:24:10:24 | *m | provenance | | +| test.c:42:12:42:16 | mtx_init output argument | test.c:42:13:42:14 | *l3 [post update] [m] | provenance | | +| test.c:42:13:42:14 | *l3 [post update] [m] | test.c:43:18:43:19 | *l3 [m] | provenance | | +| test.c:42:13:42:14 | *l3 [post update] [m] | test.c:44:15:44:16 | *l3 [m] | provenance | | +| test.c:43:18:43:19 | *l3 [m] | test.c:43:17:43:21 | *& ... | provenance | | +| test.c:44:14:44:18 | *& ... | test.c:10:24:10:24 | *m | provenance | | +| test.c:44:15:44:16 | *l3 [m] | test.c:44:14:44:18 | *& ... | provenance | | +nodes +| test.c:10:24:10:24 | *m | semmle.label | *m | +| test.c:10:43:10:43 | *m | semmle.label | *m | +| test.c:13:12:13:14 | mtx_init output argument | semmle.label | mtx_init output argument | +| test.c:14:17:14:19 | *& ... | semmle.label | *& ... | +| test.c:15:14:15:16 | *& ... | semmle.label | *& ... | +| test.c:17:12:17:14 | mtx_init output argument | semmle.label | mtx_init output argument | +| test.c:18:17:18:19 | *& ... | semmle.label | *& ... | +| test.c:19:14:19:16 | *& ... | semmle.label | *& ... | +| test.c:30:12:30:14 | mtx_init output argument | semmle.label | mtx_init output argument | +| test.c:31:17:31:19 | *& ... | semmle.label | *& ... | +| test.c:32:14:32:16 | *& ... | semmle.label | *& ... | +| test.c:42:12:42:16 | mtx_init output argument | semmle.label | mtx_init output argument | +| test.c:42:13:42:14 | *l3 [post update] [m] | semmle.label | *l3 [post update] [m] | +| test.c:43:17:43:21 | *& ... | semmle.label | *& ... | +| test.c:43:18:43:19 | *l3 [m] | semmle.label | *l3 [m] | +| test.c:44:14:44:18 | *& ... | semmle.label | *& ... | +| test.c:44:15:44:16 | *l3 [m] | semmle.label | *l3 [m] | +subpaths +#select +| test.c:10:43:10:43 | *m | test.c:13:12:13:14 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | +| test.c:10:43:10:43 | *m | test.c:17:12:17:14 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | +| test.c:10:43:10:43 | *m | test.c:30:12:30:14 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | +| test.c:10:43:10:43 | *m | test.c:42:12:42:16 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | +| test.c:14:17:14:19 | *& ... | test.c:13:12:13:14 | mtx_init output argument | test.c:14:17:14:19 | *& ... | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | +| test.c:18:17:18:19 | *& ... | test.c:17:12:17:14 | mtx_init output argument | test.c:18:17:18:19 | *& ... | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | +| test.c:31:17:31:19 | *& ... | test.c:30:12:30:14 | mtx_init output argument | test.c:31:17:31:19 | *& ... | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | +| test.c:43:17:43:21 | *& ... | test.c:42:12:42:16 | mtx_init output argument | test.c:43:17:43:21 | *& ... | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | diff --git a/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.qlref b/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.qlref new file mode 100644 index 0000000000..9ffe7e7494 --- /dev/null +++ b/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.qlref @@ -0,0 +1 @@ +rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-21-26/test.c b/c/misra/test/rules/RULE-21-26/test.c new file mode 100644 index 0000000000..d26f9c1f2f --- /dev/null +++ b/c/misra/test/rules/RULE-21-26/test.c @@ -0,0 +1,45 @@ +#include "threads.h" + +mtx_t g1; +mtx_t g2; +mtx_t g3; +mtx_t g4; + +struct timespec ts = {0, 0}; + +void doTimeLock(mtx_t *m) { mtx_timedlock(m, &ts); } + +void main(void) { + mtx_init(&g1, mtx_plain); + mtx_timedlock(&g1, &ts); // NON-COMPLIANT + doTimeLock(&g1); // NON-COMPLIANT + + mtx_init(&g2, mtx_plain | mtx_recursive); + mtx_timedlock(&g2, &ts); // NON-COMPLIANT + doTimeLock(&g2); // NON-COMPLIANT + + mtx_init(&g3, mtx_timed); + mtx_timedlock(&g3, &ts); // COMPLIANT + doTimeLock(&g3); // COMPLIANT + + mtx_init(&g4, mtx_timed | mtx_recursive); + mtx_timedlock(&g4, &ts); // COMPLIANT + doTimeLock(&g4); // COMPLIANT + + mtx_t l1; + mtx_init(&l1, mtx_plain); + mtx_timedlock(&l1, &ts); // NON-COMPLIANT + doTimeLock(&l1); // NON-COMPLIANT + + mtx_t l2; + mtx_init(&l2, mtx_timed); + mtx_timedlock(&l2, &ts); // COMPLIANT + doTimeLock(&l2); // COMPLIANT + + struct s { + mtx_t m; + } l3; + mtx_init(&l3.m, mtx_plain); + mtx_timedlock(&l3.m, &ts); // NON-COMPLIANT + doTimeLock(&l3.m); // NON-COMPLIANT +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.expected b/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.expected new file mode 100644 index 0000000000..89facda9bb --- /dev/null +++ b/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.expected @@ -0,0 +1,3 @@ +| test.c:22:15:22:16 | definition of l3 | Atomic object 'l3' has no initializer or corresponding use of 'atomic_init()'. | +| test.c:25:15:25:16 | definition of l4 | Atomic object 'l4' has no initializer or corresponding use of 'atomic_init()'. | +| test.c:29:15:29:16 | definition of l5 | Atomic object 'l5' has no initializer or corresponding use of 'atomic_init()'. | diff --git a/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.qlref b/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.qlref new file mode 100644 index 0000000000..11219b0741 --- /dev/null +++ b/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.qlref @@ -0,0 +1 @@ +rules/RULE-9-7/UninitializedAtomicObject.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-9-7/test.c b/c/misra/test/rules/RULE-9-7/test.c new file mode 100644 index 0000000000..5b3d8e36ec --- /dev/null +++ b/c/misra/test/rules/RULE-9-7/test.c @@ -0,0 +1,34 @@ +#include "stdatomic.h" +#include "threads.h" + +_Atomic int g1; // COMPLIANT +_Atomic int g2 = 0; // COMPLIANT + +void f_thread(void *x); + +void f_starts_thread() { + thrd_t t; + thrd_create(&t, f_thread, 0); +} + +void main() { + _Atomic int l1 = 1; // COMPLIANT + f_starts_thread(); + + _Atomic int l2; // COMPLIANT + atomic_init(&l2, 0); + f_starts_thread(); + + _Atomic int l3; // NON-COMPLIANT + f_starts_thread(); + + _Atomic int l4; // NON-COMPLIANT + f_starts_thread(); + atomic_init(&l4, 0); + + _Atomic int l5; // NON-COMPLIANT + if (g1 == 0) { + atomic_init(&l5, 0); + } + f_starts_thread(); +} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency7.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency7.qll new file mode 100644 index 0000000000..ba492b2a6b --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency7.qll @@ -0,0 +1,44 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Concurrency7Query = + TUninitializedAtomicObjectQuery() or + TTimedlockOnInappropriateMutexTypeQuery() + +predicate isConcurrency7QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `uninitializedAtomicObject` query + Concurrency7Package::uninitializedAtomicObjectQuery() and + queryId = + // `@id` for the `uninitializedAtomicObject` query + "c/misra/uninitialized-atomic-object" and + ruleId = "RULE-9-7" and + category = "mandatory" + or + query = + // `Query` instance for the `timedlockOnInappropriateMutexType` query + Concurrency7Package::timedlockOnInappropriateMutexTypeQuery() and + queryId = + // `@id` for the `timedlockOnInappropriateMutexType` query + "c/misra/timedlock-on-inappropriate-mutex-type" and + ruleId = "RULE-21-26" and + category = "required" +} + +module Concurrency7Package { + Query uninitializedAtomicObjectQuery() { + //autogenerate `Query` type + result = + // `Query` type for `uninitializedAtomicObject` query + TQueryC(TConcurrency7PackageQuery(TUninitializedAtomicObjectQuery())) + } + + Query timedlockOnInappropriateMutexTypeQuery() { + //autogenerate `Query` type + result = + // `Query` type for `timedlockOnInappropriateMutexType` query + TQueryC(TConcurrency7PackageQuery(TTimedlockOnInappropriateMutexTypeQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index 3833533d50..b980584877 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -11,6 +11,7 @@ import Concurrency2 import Concurrency3 import Concurrency4 import Concurrency5 +import Concurrency7 import Contracts1 import Contracts2 import Contracts3 @@ -87,6 +88,7 @@ newtype TCQuery = TConcurrency3PackageQuery(Concurrency3Query q) or TConcurrency4PackageQuery(Concurrency4Query q) or TConcurrency5PackageQuery(Concurrency5Query q) or + TConcurrency7PackageQuery(Concurrency7Query q) or TContracts1PackageQuery(Contracts1Query q) or TContracts2PackageQuery(Contracts2Query q) or TContracts3PackageQuery(Contracts3Query q) or @@ -163,6 +165,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isConcurrency3QueryMetadata(query, queryId, ruleId, category) or isConcurrency4QueryMetadata(query, queryId, ruleId, category) or isConcurrency5QueryMetadata(query, queryId, ruleId, category) or + isConcurrency7QueryMetadata(query, queryId, ruleId, category) or isContracts1QueryMetadata(query, queryId, ruleId, category) or isContracts2QueryMetadata(query, queryId, ruleId, category) or isContracts3QueryMetadata(query, queryId, ruleId, category) or diff --git a/rule_packages/c/Concurrency7.json b/rule_packages/c/Concurrency7.json new file mode 100644 index 0000000000..c544cb88c7 --- /dev/null +++ b/rule_packages/c/Concurrency7.json @@ -0,0 +1,45 @@ +{ + "MISRA-C-2012": { + "RULE-9-7": { + "properties": { + "obligation": "mandatory" + }, + "queries": [ + { + "description": "Atomic objects that do not have static storage duration shall be initialized with a value or by using 'atomic_init()'.", + "kind": "problem", + "name": "Atomic objects shall be appropriately initialized before being accessed", + "precision": "high", + "severity": "warning", + "short_name": "UninitializedAtomicObject", + "tags": [ + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "Atomic objects shall be appropriately initialized before being accessed" + }, + "RULE-21-26": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type", + "kind": "problem", + "name": "The Standard Library function mtx_timedlock() shall only be invoked on mutexes of type mtx_timed", + "precision": "high", + "severity": "error", + "short_name": "TimedlockOnInappropriateMutexType", + "tags": [ + "correctness", + "concurrency", + "external/misra/c/2012/amendment4" + ] + } + ], + "title": "The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 475ea1d66c..c07aeb58ed 100644 --- a/rules.csv +++ b/rules.csv @@ -678,7 +678,7 @@ c,MISRA-C-2012,RULE-9-3,Yes,Required,,,Arrays shall not be partially initialized c,MISRA-C-2012,RULE-9-4,Yes,Required,,,An element of an object shall not be initialized more than once,,Memory1,Medium, c,MISRA-C-2012,RULE-9-5,No,Required,,,Where designated initializers are used to initialize an array object the size of the array shall be specified explicitly,,,Medium, c,MISRA-C-2012,RULE-9-6,Yes,Required,,,An initializer using chained designators shall not contain initializers without designators,,Declarations9,Hard, -c,MISRA-C-2012,RULE-9-7,Yes,Mandatory,,,Atomic objects shall be appropriately initialized before being accessed,,Concurrency6,Hard, +c,MISRA-C-2012,RULE-9-7,Yes,Mandatory,,,Atomic objects shall be appropriately initialized before being accessed,,Concurrency7,Hard, c,MISRA-C-2012,RULE-10-1,Yes,Required,,,Operands shall not be of an inappropriate essential type,,EssentialTypes,Hard, c,MISRA-C-2012,RULE-10-2,Yes,Required,,,Expressions of essentially character type shall not be used inappropriately in addition and subtraction operations,,EssentialTypes,Medium, c,MISRA-C-2012,RULE-10-3,Yes,Required,,,The value of an expression shall not be assigned to an object with a narrower essential type or of a different essential type category,,EssentialTypes,Hard, @@ -791,7 +791,7 @@ c,MISRA-C-2012,RULE-21-22,Yes,Mandatory,,,All operand arguments to any type-gene c,MISRA-C-2012,RULE-21-23,Yes,Required,,,All operand arguments to any multi-argument type-generic macros in shall have the same standard type,Rule-21-22,EssentialTypes2,Easy, c,MISRA-C-2012,RULE-21-24,Yes,Required,,,The random number generator functions of shall not be used,MSC30-C,Banned2,Easy, c,MISRA-C-2012,RULE-21-25,Yes,Required,,,All memory synchronization operations shall be executed in sequentially consistent order,,Concurrency6,Medium, -c,MISRA-C-2012,RULE-21-26,Yes,Required,,,The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type,,Concurrency6,Hard, +c,MISRA-C-2012,RULE-21-26,Yes,Required,,,The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type,,Concurrency8,Hard, c,MISRA-C-2012,RULE-22-1,Yes,Required,,,All resources obtained dynamically by means of Standard Library functions shall be explicitly released,,Memory2,Hard, c,MISRA-C-2012,RULE-22-2,Yes,Mandatory,,,A block of memory shall only be freed if it was allocated by means of a Standard Library function,,Memory2,Hard, c,MISRA-C-2012,RULE-22-3,Yes,Required,,,The same file shall not be open for read and write access at the same time on different streams,,IO3,Hard, From 8a2076e5934d46a0ef70478ffd82b6bc146f884b Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Mon, 18 Nov 2024 00:03:04 -0800 Subject: [PATCH 2/7] Fix query metadata --- rule_packages/c/Concurrency7.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rule_packages/c/Concurrency7.json b/rule_packages/c/Concurrency7.json index c544cb88c7..f764468ae3 100644 --- a/rule_packages/c/Concurrency7.json +++ b/rule_packages/c/Concurrency7.json @@ -27,7 +27,7 @@ "queries": [ { "description": "The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type", - "kind": "problem", + "kind": "path-problem", "name": "The Standard Library function mtx_timedlock() shall only be invoked on mutexes of type mtx_timed", "precision": "high", "severity": "error", From 6f36b1c322d9c48599de3bc6829aeaa158c76704 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Mon, 18 Nov 2024 12:48:30 -0800 Subject: [PATCH 3/7] Fix query metadata, format --- .../TimedlockOnInappropriateMutexType.ql | 2 +- .../RULE-9-7/UninitializedAtomicObject.ql | 28 ++++++++++--------- rule_packages/c/Concurrency7.json | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql b/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql index d8d465045e..e6dda61d79 100644 --- a/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql +++ b/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql @@ -2,7 +2,7 @@ * @id c/misra/timedlock-on-inappropriate-mutex-type * @name RULE-21-26: The Standard Library function mtx_timedlock() shall only be invoked on mutexes of type mtx_timed * @description The Standard Library function mtx_timedlock() shall only be invoked on mutex objects - * of appropriate mutex type + * of appropriate mutex type. * @kind path-problem * @precision high * @problem.severity error diff --git a/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql b/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql index 40f833a740..006e8e8178 100644 --- a/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql +++ b/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql @@ -18,11 +18,14 @@ import semmle.code.cpp.controlflow.Dominance class ThreadSpawningFunction extends Function { ThreadSpawningFunction() { - this.hasName("pthread_create") or - this.hasName("thrd_create") or + this.hasName("pthread_create") + or + this.hasName("thrd_create") + or exists(FunctionCall fc | fc.getTarget() instanceof ThreadSpawningFunction and - fc.getEnclosingFunction() = this) + fc.getEnclosingFunction() = this + ) } } @@ -37,9 +40,7 @@ class AtomicInitAddressOfExpr extends FunctionCall { ) } - Expr getAddressedExpr() { - result = addressedExpr - } + Expr getAddressedExpr() { result = addressedExpr } } ControlFlowNode getARequiredInitializationPoint(LocalScopeVariable v) { @@ -47,11 +48,12 @@ ControlFlowNode getARequiredInitializationPoint(LocalScopeVariable v) { or exists(DeclStmt decl | decl.getADeclaration() = v and - result = any(FunctionCall fc - | fc.getTarget() instanceof ThreadSpawningFunction and - fc.getEnclosingBlock().getEnclosingBlock*() = v.getParentScope() and - fc.getAPredecessor*() = decl - ) + result = + any(FunctionCall fc | + fc.getTarget() instanceof ThreadSpawningFunction and + fc.getEnclosingBlock().getEnclosingBlock*() = v.getParentScope() and + fc.getAPredecessor*() = decl + ) ) } @@ -63,8 +65,8 @@ where not v.isTopLevel() and not exists(v.getInitializer()) and exists(ControlFlowNode missingInitPoint | - missingInitPoint = getARequiredInitializationPoint(v) - and not exists(AtomicInitAddressOfExpr initialization | + missingInitPoint = getARequiredInitializationPoint(v) and + not exists(AtomicInitAddressOfExpr initialization | initialization.getAddressedExpr().(VariableAccess).getTarget() = v and dominates(initialization, missingInitPoint) ) diff --git a/rule_packages/c/Concurrency7.json b/rule_packages/c/Concurrency7.json index f764468ae3..6fdc49984b 100644 --- a/rule_packages/c/Concurrency7.json +++ b/rule_packages/c/Concurrency7.json @@ -26,7 +26,7 @@ }, "queries": [ { - "description": "The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type", + "description": "The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type.", "kind": "path-problem", "name": "The Standard Library function mtx_timedlock() shall only be invoked on mutexes of type mtx_timed", "precision": "high", From 9ac5159694585358fe6bf267d26b4062350b726f Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Mon, 18 Nov 2024 12:56:07 -0800 Subject: [PATCH 4/7] Fix rule 21-26 packages in rules.csv: Concurrency7 not 8. --- rules.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules.csv b/rules.csv index c07aeb58ed..b914f7cad5 100644 --- a/rules.csv +++ b/rules.csv @@ -791,7 +791,7 @@ c,MISRA-C-2012,RULE-21-22,Yes,Mandatory,,,All operand arguments to any type-gene c,MISRA-C-2012,RULE-21-23,Yes,Required,,,All operand arguments to any multi-argument type-generic macros in shall have the same standard type,Rule-21-22,EssentialTypes2,Easy, c,MISRA-C-2012,RULE-21-24,Yes,Required,,,The random number generator functions of shall not be used,MSC30-C,Banned2,Easy, c,MISRA-C-2012,RULE-21-25,Yes,Required,,,All memory synchronization operations shall be executed in sequentially consistent order,,Concurrency6,Medium, -c,MISRA-C-2012,RULE-21-26,Yes,Required,,,The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type,,Concurrency8,Hard, +c,MISRA-C-2012,RULE-21-26,Yes,Required,,,The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type,,Concurrency7,Hard, c,MISRA-C-2012,RULE-22-1,Yes,Required,,,All resources obtained dynamically by means of Standard Library functions shall be explicitly released,,Memory2,Hard, c,MISRA-C-2012,RULE-22-2,Yes,Mandatory,,,A block of memory shall only be freed if it was allocated by means of a Standard Library function,,Memory2,Hard, c,MISRA-C-2012,RULE-22-3,Yes,Required,,,The same file shall not be open for read and write access at the same time on different streams,,IO3,Hard, From 3882e8754fc578874b1c77cd524f580807d1d179 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Fri, 14 Feb 2025 21:29:38 -0800 Subject: [PATCH 5/7] Address feedback --- .../TimedlockOnInappropriateMutexType.ql | 4 +- .../RULE-9-7/UninitializedAtomicObject.ql | 21 ++-- ...TimedlockOnInappropriateMutexType.expected | 16 +-- .../UninitializedAtomicObject.expected | 7 +- c/misra/test/rules/RULE-9-7/test.c | 12 ++ .../cpp/StdFunctionOrMacro.qll | 111 ++++++++++++++++++ rule_packages/c/Concurrency7.json | 6 +- 7 files changed, 155 insertions(+), 22 deletions(-) create mode 100644 cpp/common/src/codingstandards/cpp/StdFunctionOrMacro.qll diff --git a/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql b/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql index e6dda61d79..4401d06e2c 100644 --- a/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql +++ b/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql @@ -71,4 +71,6 @@ where not isExcluded(sink.getNode().asExpr(), Concurrency7Package::timedlockOnInappropriateMutexTypeQuery()) and Flow::flowPath(source, sink) -select sink.getNode(), source, sink, "Call to mtx_timedlock with mutex not of type 'mtx_timed'." +select sink.getNode(), source, sink, + "Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'.", + source.getNode(), "initialized" diff --git a/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql b/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql index 006e8e8178..b6e8bc82bc 100644 --- a/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql +++ b/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql @@ -14,6 +14,7 @@ import cpp import codingstandards.c.misra +import codingstandards.cpp.StdFunctionOrMacro import semmle.code.cpp.controlflow.Dominance class ThreadSpawningFunction extends Function { @@ -29,18 +30,14 @@ class ThreadSpawningFunction extends Function { } } -class AtomicInitAddressOfExpr extends FunctionCall { - Expr addressedExpr; +private string atomicInit() { result = "atomic_init" } +class AtomicInitAddressOfExpr extends AddressOfExpr { AtomicInitAddressOfExpr() { - exists(AddressOfExpr addrOf | - getArgument(0) = addrOf and - addrOf.getOperand() = addressedExpr and - getTarget().getName() = "__c11_atomic_init" + exists(StdFunctionOrMacro::Call c | + this = c.getArgument(0) ) } - - Expr getAddressedExpr() { result = addressedExpr } } ControlFlowNode getARequiredInitializationPoint(LocalScopeVariable v) { @@ -66,9 +63,15 @@ where not exists(v.getInitializer()) and exists(ControlFlowNode missingInitPoint | missingInitPoint = getARequiredInitializationPoint(v) and + // Check for `atomic_init(&v)` not exists(AtomicInitAddressOfExpr initialization | - initialization.getAddressedExpr().(VariableAccess).getTarget() = v and + initialization.getOperand().(VariableAccess).getTarget() = v and dominates(initialization, missingInitPoint) + ) and + // Check for `unknown_func(&v)` which may call `atomic_init` on `v`. + not exists(FunctionCall fc | + fc.getAnArgument().(AddressOfExpr).getOperand().(VariableAccess).getTarget() = v and + dominates(fc, missingInitPoint) ) ) select decl, diff --git a/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.expected b/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.expected index 442f20bf73..0a4c0a496a 100644 --- a/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.expected +++ b/c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.expected @@ -35,11 +35,11 @@ nodes | test.c:44:15:44:16 | *l3 [m] | semmle.label | *l3 [m] | subpaths #select -| test.c:10:43:10:43 | *m | test.c:13:12:13:14 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | -| test.c:10:43:10:43 | *m | test.c:17:12:17:14 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | -| test.c:10:43:10:43 | *m | test.c:30:12:30:14 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | -| test.c:10:43:10:43 | *m | test.c:42:12:42:16 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | -| test.c:14:17:14:19 | *& ... | test.c:13:12:13:14 | mtx_init output argument | test.c:14:17:14:19 | *& ... | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | -| test.c:18:17:18:19 | *& ... | test.c:17:12:17:14 | mtx_init output argument | test.c:18:17:18:19 | *& ... | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | -| test.c:31:17:31:19 | *& ... | test.c:30:12:30:14 | mtx_init output argument | test.c:31:17:31:19 | *& ... | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | -| test.c:43:17:43:21 | *& ... | test.c:42:12:42:16 | mtx_init output argument | test.c:43:17:43:21 | *& ... | Call to mtx_timedlock with mutex not of type 'mtx_timed'. | +| test.c:10:43:10:43 | *m | test.c:13:12:13:14 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'. | test.c:13:12:13:14 | mtx_init output argument | initialized | +| test.c:10:43:10:43 | *m | test.c:17:12:17:14 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'. | test.c:17:12:17:14 | mtx_init output argument | initialized | +| test.c:10:43:10:43 | *m | test.c:30:12:30:14 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'. | test.c:30:12:30:14 | mtx_init output argument | initialized | +| test.c:10:43:10:43 | *m | test.c:42:12:42:16 | mtx_init output argument | test.c:10:43:10:43 | *m | Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'. | test.c:42:12:42:16 | mtx_init output argument | initialized | +| test.c:14:17:14:19 | *& ... | test.c:13:12:13:14 | mtx_init output argument | test.c:14:17:14:19 | *& ... | Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'. | test.c:13:12:13:14 | mtx_init output argument | initialized | +| test.c:18:17:18:19 | *& ... | test.c:17:12:17:14 | mtx_init output argument | test.c:18:17:18:19 | *& ... | Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'. | test.c:17:12:17:14 | mtx_init output argument | initialized | +| test.c:31:17:31:19 | *& ... | test.c:30:12:30:14 | mtx_init output argument | test.c:31:17:31:19 | *& ... | Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'. | test.c:30:12:30:14 | mtx_init output argument | initialized | +| test.c:43:17:43:21 | *& ... | test.c:42:12:42:16 | mtx_init output argument | test.c:43:17:43:21 | *& ... | Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'. | test.c:42:12:42:16 | mtx_init output argument | initialized | diff --git a/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.expected b/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.expected index 89facda9bb..f96fc6aa13 100644 --- a/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.expected +++ b/c/misra/test/rules/RULE-9-7/UninitializedAtomicObject.expected @@ -1,3 +1,4 @@ -| test.c:22:15:22:16 | definition of l3 | Atomic object 'l3' has no initializer or corresponding use of 'atomic_init()'. | -| test.c:25:15:25:16 | definition of l4 | Atomic object 'l4' has no initializer or corresponding use of 'atomic_init()'. | -| test.c:29:15:29:16 | definition of l5 | Atomic object 'l5' has no initializer or corresponding use of 'atomic_init()'. | +| test.c:24:15:24:16 | definition of l3 | Atomic object 'l3' has no initializer or corresponding use of 'atomic_init()'. | +| test.c:27:15:27:16 | definition of l4 | Atomic object 'l4' has no initializer or corresponding use of 'atomic_init()'. | +| test.c:31:15:31:16 | definition of l5 | Atomic object 'l5' has no initializer or corresponding use of 'atomic_init()'. | +| test.c:41:15:41:16 | definition of l7 | Atomic object 'l7' has no initializer or corresponding use of 'atomic_init()'. | diff --git a/c/misra/test/rules/RULE-9-7/test.c b/c/misra/test/rules/RULE-9-7/test.c index 5b3d8e36ec..da367c0bd1 100644 --- a/c/misra/test/rules/RULE-9-7/test.c +++ b/c/misra/test/rules/RULE-9-7/test.c @@ -11,6 +11,8 @@ void f_starts_thread() { thrd_create(&t, f_thread, 0); } +void f_may_initialize_argument(void *p1) {} + void main() { _Atomic int l1 = 1; // COMPLIANT f_starts_thread(); @@ -31,4 +33,14 @@ void main() { atomic_init(&l5, 0); } f_starts_thread(); + + _Atomic int l6; // COMPLIANT + f_may_initialize_argument(&l6); + f_starts_thread(); + + _Atomic int l7; // NON_COMPLIANT + if (g1 == 0) { + f_may_initialize_argument(&l7); + } + f_starts_thread(); } \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/StdFunctionOrMacro.qll b/cpp/common/src/codingstandards/cpp/StdFunctionOrMacro.qll new file mode 100644 index 0000000000..5ae370183d --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/StdFunctionOrMacro.qll @@ -0,0 +1,111 @@ +/** + * This module intends to reduce the difficulty of handling the pattern where implementations + * implement a function as a macro: the class `StdFunctionOrMacro<...>::Call` matches both std + * function calls as well as std function macro expansions. + * + * For instance, `atomic_init` may be implemented as a function, but is also implemented as + * `#DEFINE atomic_init(x) __c11_atomic_init(x)` on some platforms. This module aids in finding + * calls to any standard function which may be a macro, and has predefined behavior for + * handling `__c11_*` macros. + * + * Since a macro can be defined to expand to any expression, we cannot know generally which + * expanded expressions in `f(x, y)` correspond to arguments `x` or `y`. To handle this, the + * following inference options are available: + * - `NoMacroExpansionInference`: Assume any expression in the macro expansion could correspond to + * any macro argument. + * - `C11FunctionWrapperMacro`: Check if the macro expands to a function call prefixed with + * `__c11_` and if so, return the corresponding argument. Otherwise, fall back to + * `NoMacroExpansionInference`. + * - `InferMacroExpansionArguments`: Implement your own logic for inferring the argument. + * + * To use this module, pick one of the above inference strategies, and then create a predicate for + * the name you wish to match. For instance: + * + * ```codeql + * private string atomicInit() { result = "atomic_init" } + * + * from StdFunctionOrMacro::Call c + * select c.getArgument(0) + * ``` + */ + +import cpp as cpp + +/** Specify the name of your function as a predicate */ +signature string getName(); + +/** Signature module to implement custom argument resolution behavior in expanded macros */ +signature module InferMacroExpansionArguments { + bindingset[mi, argumentIdx] + cpp::Expr inferArgument(cpp::MacroInvocation mi, int argumentIdx); +} + +/** Assume all subexpressions of an expanded macro may be the result of any ith argument */ +module NoMacroExpansionInference implements InferMacroExpansionArguments { + bindingset[mi, argumentIdx] + cpp::Expr inferArgument(cpp::MacroInvocation mi, int argumentIdx) { + result.getParent*() = mi.getExpr() + } +} + +/** Assume macro `f(x, y, ...)` expands to `__c11_f(x, y, ...)`. */ +module C11FunctionWrapperMacro implements InferMacroExpansionArguments { + bindingset[mi, argumentIdx] + cpp::Expr inferArgument(cpp::MacroInvocation mi, int argumentIdx) { + if mi.getExpr().(cpp::FunctionCall).getTarget().hasName("__c11_" + mi.getMacroName()) + then result = mi.getExpr().(cpp::FunctionCall).getArgument(argumentIdx) + else result = NoMacroExpansionInference::inferArgument(mi, argumentIdx) + } +} + +/** + * A module to find calls to standard functions, or expansions of macros with the same name. + * + * To use this module, specify a name predicate and an inference strategy for correlating macro + * expansions to macro arguments. + * + * For example: + * + * ```codeql + * private string atomicInit() { result = "atomic_init" } + * from StdFunctionOrMacro::Call c + * select c.getArgument(0) + * ``` + */ +module StdFunctionOrMacro { + final private class Expr = cpp::Expr; + + final private class FunctionCall = cpp::FunctionCall; + + final private class MacroInvocation = cpp::MacroInvocation; + + private newtype TStdCall = + TStdFunctionCall(FunctionCall fc) { fc.getTarget().hasName(getStdName()) } or + TStdMacroInvocation(MacroInvocation mi) { mi.getMacro().hasName(getStdName()) } + + /** + * A call to a standard function or an expansion of a macro with the same name. + */ + class Call extends TStdCall { + bindingset[this, argumentIdx] + Expr getArgument(int argumentIdx) { + exists(FunctionCall fc | + this = TStdFunctionCall(fc) and + result = fc.getArgument(argumentIdx) + ) + or + exists(MacroInvocation mi | + this = TStdMacroInvocation(mi) and + result = InferExpansion::inferArgument(mi, argumentIdx) + ) + } + + string toString() { + this = TStdFunctionCall(_) and + result = "Standard function call" + or + this = TStdMacroInvocation(_) and + result = "Invocation of a standard function implemented as a macro" + } + } +} diff --git a/rule_packages/c/Concurrency7.json b/rule_packages/c/Concurrency7.json index 6fdc49984b..bda8881934 100644 --- a/rule_packages/c/Concurrency7.json +++ b/rule_packages/c/Concurrency7.json @@ -15,7 +15,11 @@ "tags": [ "concurrency", "external/misra/c/2012/amendment4" - ] + ], + "implementation_scope": { + "description": "This query tracks which functions may start threads, either indirectly or directly (\"thread spawning functions\"), and checks for local atomic variables that are not passed by address into `atomic_init` or other function calls, before such a thread spawning function is called.", + "items": [] + } } ], "title": "Atomic objects shall be appropriately initialized before being accessed" From b294477a2c2b79cf8242e8f12c0bc8e5a7689f3b Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Fri, 14 Feb 2025 21:37:26 -0800 Subject: [PATCH 6/7] CI/CD fixes: format, rules.csv package -> Concurrency8 for unhandled rules --- .../TimedlockOnInappropriateMutexType.ql | 4 ++-- rules.csv | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql b/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql index 4401d06e2c..929eb5bd0a 100644 --- a/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql +++ b/c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql @@ -72,5 +72,5 @@ where Concurrency7Package::timedlockOnInappropriateMutexTypeQuery()) and Flow::flowPath(source, sink) select sink.getNode(), source, sink, - "Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'.", - source.getNode(), "initialized" + "Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'.", source.getNode(), + "initialized" diff --git a/rules.csv b/rules.csv index 10478a3da8..3f959542a7 100644 --- a/rules.csv +++ b/rules.csv @@ -617,7 +617,7 @@ c,MISRA-C-2012,DIR-4-12,Yes,Required,,,Dynamic memory allocation shall not be us c,MISRA-C-2012,DIR-4-13,No,Advisory,,,Functions which are designed to provide operations on a resource should be called in an appropriate sequence,,,,"Rule 22.1, 22.2 and 22.6 cover aspects of this rule. In other cases this is a design issue and needs to be checked manually." c,MISRA-C-2012,DIR-4-14,Yes,Required,,,The validity of values received from external sources shall be checked,,Contracts8,Hard,This is supported by CodeQLs default C security queries. c,MISRA-C-2012,DIR-4-15,Yes,Required,,,Evaluation of floating-point expressions shall not lead to the undetected generation of infinities and NaNs,FLP32-C and FLP04-C,FloatingTypes2,Medium, -c,MISRA-C-2012,DIR-5-1,Yes,Required,,,There shall be no data races between threads,CON43-C and CON32-C,Concurrency7,Very Hard, +c,MISRA-C-2012,DIR-5-1,Yes,Required,,,There shall be no data races between threads,CON43-C and CON32-C,Concurrency8,Very Hard, c,MISRA-C-2012,DIR-5-2,Yes,Required,,,There shall be no deadlocks between threads,CON35-C,Concurrency6,Import, c,MISRA-C-2012,DIR-5-3,Yes,Required,,,There shall be no dynamic thread creation,,Concurrency6,Easy, c,MISRA-C-2012,RULE-1-1,No,Required,,,"The program shall contain no violations of the standard C syntax and constraints, and shall not exceed the implementation's translation limits",,,Easy,"This should be checked via the compiler output, rather than CodeQL, which adds unnecessary steps." @@ -803,15 +803,15 @@ c,MISRA-C-2012,RULE-22-8,Yes,Required,,,The value of errno shall be set to zero c,MISRA-C-2012,RULE-22-9,Yes,Required,,,The value of errno shall be tested against zero after calling an errno-setting-function,,Contracts3,Medium, c,MISRA-C-2012,RULE-22-10,Yes,Required,,,The value of errno shall only be tested when the last function to be called was an errno-setting-function,,Contracts3,Medium, c,MISRA-C-2012,RULE-22-11,Yes,Required,,,A thread that was previously either joined or detached shall not be subsequently joined nor detached,CON39-C,Concurrency6,Import, -c,MISRA-C-2012,RULE-22-12,Yes,Mandatory,,,"Thread objects, thread synchronization objects, and thread-specific storage pointers shall only be accessed by the appropriate Standard Library functions",,Concurrency7,Medium, -c,MISRA-C-2012,RULE-22-13,Yes,Required,,,"Thread objects, thread synchronization objects, and thread specific storage pointers shall have appropriate storage duration",EXP54-CPP and CON34-C,Concurrency7,Medium, -c,MISRA-C-2012,RULE-22-14,Yes,Mandatory,,,Thread synchronization objects shall be initialized before being accessed,EXP53-CPP,Concurrency7,Hard, -c,MISRA-C-2012,RULE-22-15,Yes,Required,,,Thread synchronization objects and thread-specific storage pointers shall not be destroyed until after all threads accessing them have terminated,,Concurrency7,Hard, -c,MISRA-C-2012,RULE-22-16,Yes,Required,,,All mutex objects locked by a thread shall be explicitly unlocked by the same thread,MEM51-CPP,Concurrency7,Hard, -c,MISRA-C-2012,RULE-22-17,Yes,Required,,,No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it has not locked before,Rule 22.2,Concurrency7,Medium, -c,MISRA-C-2012,RULE-22-18,Yes,Required,,,Non-recursive mutexes shall not be recursively locked,CON56-CPP,Concurrency7,Medium, -c,MISRA-C-2012,RULE-22-19,Yes,Required,,,A condition variable shall be associated with at most one mutex object,,Concurrency7,Medium, -c,MISRA-C-2012,RULE-22-20,Yes,Mandatory,,,Thread-specific storage pointers shall be created before being accessed,,Concurrency7,Hard, +c,MISRA-C-2012,RULE-22-12,Yes,Mandatory,,,"Thread objects, thread synchronization objects, and thread-specific storage pointers shall only be accessed by the appropriate Standard Library functions",,Concurrency8,Medium, +c,MISRA-C-2012,RULE-22-13,Yes,Required,,,"Thread objects, thread synchronization objects, and thread specific storage pointers shall have appropriate storage duration",EXP54-CPP and CON34-C,Concurrency8,Medium, +c,MISRA-C-2012,RULE-22-14,Yes,Mandatory,,,Thread synchronization objects shall be initialized before being accessed,EXP53-CPP,Concurrency8,Hard, +c,MISRA-C-2012,RULE-22-15,Yes,Required,,,Thread synchronization objects and thread-specific storage pointers shall not be destroyed until after all threads accessing them have terminated,,Concurrency8,Hard, +c,MISRA-C-2012,RULE-22-16,Yes,Required,,,All mutex objects locked by a thread shall be explicitly unlocked by the same thread,MEM51-CPP,Concurrency8,Hard, +c,MISRA-C-2012,RULE-22-17,Yes,Required,,,No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it has not locked before,Rule 22.2,Concurrency8,Medium, +c,MISRA-C-2012,RULE-22-18,Yes,Required,,,Non-recursive mutexes shall not be recursively locked,CON56-CPP,Concurrency8,Medium, +c,MISRA-C-2012,RULE-22-19,Yes,Required,,,A condition variable shall be associated with at most one mutex object,,Concurrency8,Medium, +c,MISRA-C-2012,RULE-22-20,Yes,Mandatory,,,Thread-specific storage pointers shall be created before being accessed,,Concurrency8,Hard, c,MISRA-C-2012,RULE-23-1,Yes,Advisory,,,A generic selection should only be expanded from a macro,,Generics,Medium, c,MISRA-C-2012,RULE-23-2,Yes,Required,,,A generic selection that is not expanded from a macro shall not contain potential side effects in the controlling expression,,Generics,Hard, c,MISRA-C-2012,RULE-23-3,Yes,Advisory,,,A generic selection should contain at least one non-default association,,Generics,Easy, From 3eecfa05ec5bea6900fbf149f4a3e070b4517afe Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Tue, 18 Feb 2025 20:55:16 -0800 Subject: [PATCH 7/7] Update StdFunctionOrMacro with feedback --- .../RULE-9-7/UninitializedAtomicObject.ql | 8 +---- .../cpp/StdFunctionOrMacro.qll | 29 +++++++++---------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql b/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql index b6e8bc82bc..dfb096189f 100644 --- a/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql +++ b/c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql @@ -30,14 +30,8 @@ class ThreadSpawningFunction extends Function { } } -private string atomicInit() { result = "atomic_init" } - class AtomicInitAddressOfExpr extends AddressOfExpr { - AtomicInitAddressOfExpr() { - exists(StdFunctionOrMacro::Call c | - this = c.getArgument(0) - ) - } + AtomicInitAddressOfExpr() { exists(AtomicInitCall c | this = c.getArgument(0)) } } ControlFlowNode getARequiredInitializationPoint(LocalScopeVariable v) { diff --git a/cpp/common/src/codingstandards/cpp/StdFunctionOrMacro.qll b/cpp/common/src/codingstandards/cpp/StdFunctionOrMacro.qll index 5ae370183d..1067b7ad09 100644 --- a/cpp/common/src/codingstandards/cpp/StdFunctionOrMacro.qll +++ b/cpp/common/src/codingstandards/cpp/StdFunctionOrMacro.qll @@ -31,30 +31,28 @@ import cpp as cpp +private string atomicInit() { result = "atomic_init" } + +class AtomicInitCall = StdFunctionOrMacro::Call; + /** Specify the name of your function as a predicate */ -signature string getName(); +private signature string getName(); /** Signature module to implement custom argument resolution behavior in expanded macros */ -signature module InferMacroExpansionArguments { +private signature module InferMacroExpansionArguments { bindingset[mi, argumentIdx] cpp::Expr inferArgument(cpp::MacroInvocation mi, int argumentIdx); } -/** Assume all subexpressions of an expanded macro may be the result of any ith argument */ -module NoMacroExpansionInference implements InferMacroExpansionArguments { - bindingset[mi, argumentIdx] - cpp::Expr inferArgument(cpp::MacroInvocation mi, int argumentIdx) { - result.getParent*() = mi.getExpr() - } -} - /** Assume macro `f(x, y, ...)` expands to `__c11_f(x, y, ...)`. */ -module C11FunctionWrapperMacro implements InferMacroExpansionArguments { +private module C11FunctionWrapperMacro implements InferMacroExpansionArguments { bindingset[mi, argumentIdx] cpp::Expr inferArgument(cpp::MacroInvocation mi, int argumentIdx) { - if mi.getExpr().(cpp::FunctionCall).getTarget().hasName("__c11_" + mi.getMacroName()) - then result = mi.getExpr().(cpp::FunctionCall).getArgument(argumentIdx) - else result = NoMacroExpansionInference::inferArgument(mi, argumentIdx) + exists(cpp::FunctionCall fc | + fc = mi.getExpr() and + fc.getTarget().hasName("__c11_" + mi.getMacroName()) and + result = mi.getExpr().(cpp::FunctionCall).getArgument(argumentIdx) + ) } } @@ -72,7 +70,8 @@ module C11FunctionWrapperMacro implements InferMacroExpansionArguments { * select c.getArgument(0) * ``` */ -module StdFunctionOrMacro { +private module StdFunctionOrMacro +{ final private class Expr = cpp::Expr; final private class FunctionCall = cpp::FunctionCall;