Skip to content

Implement Concurrency7 package #799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Feb 19, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -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"
21 changes: 12 additions & 9 deletions c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import cpp
import codingstandards.c.misra
import codingstandards.cpp.StdFunctionOrMacro
import semmle.code.cpp.controlflow.Dominance

class ThreadSpawningFunction extends Function {
Expand All @@ -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<C11FunctionWrapperMacro, atomicInit/0>::Call c |
this = c.getArgument(0)
)
}

Expr getAddressedExpr() { result = addressedExpr }
}

ControlFlowNode getARequiredInitializationPoint(LocalScopeVariable v) {
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Original file line number Diff line number Diff line change
@@ -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()'. |
12 changes: 12 additions & 0 deletions c/misra/test/rules/RULE-9-7/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
}
111 changes: 111 additions & 0 deletions cpp/common/src/codingstandards/cpp/StdFunctionOrMacro.qll
Original file line number Diff line number Diff line change
@@ -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<C11FunctionWrapperMacro, atomicInit/0>::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<C11FunctionWrapperMacro, atomicInit/0>::Call c
* select c.getArgument(0)
* ```
*/
module StdFunctionOrMacro<InferMacroExpansionArguments InferExpansion, getName/0 getStdName> {
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"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import Concurrency2
import Concurrency3
import Concurrency4
import Concurrency5
import Concurrency6
import Concurrency7
import Contracts
import Contracts1
import Contracts2
import Contracts3
Expand Down Expand Up @@ -93,7 +95,9 @@ newtype TCQuery =
TConcurrency3PackageQuery(Concurrency3Query q) or
TConcurrency4PackageQuery(Concurrency4Query q) or
TConcurrency5PackageQuery(Concurrency5Query q) or
TConcurrency6PackageQuery(Concurrency6Query q) or
TConcurrency7PackageQuery(Concurrency7Query q) or
TContractsPackageQuery(ContractsQuery q) or
TContracts1PackageQuery(Contracts1Query q) or
TContracts2PackageQuery(Contracts2Query q) or
TContracts3PackageQuery(Contracts3Query q) or
Expand Down Expand Up @@ -174,7 +178,9 @@ 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
isConcurrency6QueryMetadata(query, queryId, ruleId, category) or
isConcurrency7QueryMetadata(query, queryId, ruleId, category) or
isContractsQueryMetadata(query, queryId, ruleId, category) or
isContracts1QueryMetadata(query, queryId, ruleId, category) or
isContracts2QueryMetadata(query, queryId, ruleId, category) or
isContracts3QueryMetadata(query, queryId, ruleId, category) or
Expand Down
6 changes: 5 additions & 1 deletion rule_packages/c/Concurrency7.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
20 changes: 10 additions & 10 deletions rules.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -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,
Expand Down
Loading