Skip to content

Commit 3882e87

Browse files
Address feedback
1 parent fcb870c commit 3882e87

File tree

7 files changed

+155
-22
lines changed

7 files changed

+155
-22
lines changed

c/misra/src/rules/RULE-21-26/TimedlockOnInappropriateMutexType.ql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,6 @@ where
7171
not isExcluded(sink.getNode().asExpr(),
7272
Concurrency7Package::timedlockOnInappropriateMutexTypeQuery()) and
7373
Flow::flowPath(source, sink)
74-
select sink.getNode(), source, sink, "Call to mtx_timedlock with mutex not of type 'mtx_timed'."
74+
select sink.getNode(), source, sink,
75+
"Call to mtx_timedlock with mutex which is $@ without flag 'mtx_timed'.",
76+
source.getNode(), "initialized"

c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import cpp
1616
import codingstandards.c.misra
17+
import codingstandards.cpp.StdFunctionOrMacro
1718
import semmle.code.cpp.controlflow.Dominance
1819

1920
class ThreadSpawningFunction extends Function {
@@ -29,18 +30,14 @@ class ThreadSpawningFunction extends Function {
2930
}
3031
}
3132

32-
class AtomicInitAddressOfExpr extends FunctionCall {
33-
Expr addressedExpr;
33+
private string atomicInit() { result = "atomic_init" }
3434

35+
class AtomicInitAddressOfExpr extends AddressOfExpr {
3536
AtomicInitAddressOfExpr() {
36-
exists(AddressOfExpr addrOf |
37-
getArgument(0) = addrOf and
38-
addrOf.getOperand() = addressedExpr and
39-
getTarget().getName() = "__c11_atomic_init"
37+
exists(StdFunctionOrMacro<C11FunctionWrapperMacro, atomicInit/0>::Call c |
38+
this = c.getArgument(0)
4039
)
4140
}
42-
43-
Expr getAddressedExpr() { result = addressedExpr }
4441
}
4542

4643
ControlFlowNode getARequiredInitializationPoint(LocalScopeVariable v) {
@@ -66,9 +63,15 @@ where
6663
not exists(v.getInitializer()) and
6764
exists(ControlFlowNode missingInitPoint |
6865
missingInitPoint = getARequiredInitializationPoint(v) and
66+
// Check for `atomic_init(&v)`
6967
not exists(AtomicInitAddressOfExpr initialization |
70-
initialization.getAddressedExpr().(VariableAccess).getTarget() = v and
68+
initialization.getOperand().(VariableAccess).getTarget() = v and
7169
dominates(initialization, missingInitPoint)
70+
) and
71+
// Check for `unknown_func(&v)` which may call `atomic_init` on `v`.
72+
not exists(FunctionCall fc |
73+
fc.getAnArgument().(AddressOfExpr).getOperand().(VariableAccess).getTarget() = v and
74+
dominates(fc, missingInitPoint)
7275
)
7376
)
7477
select decl,

c/misra/test/rules/RULE-21-26/TimedlockOnInappropriateMutexType.expected

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ nodes
3535
| test.c:44:15:44:16 | *l3 [m] | semmle.label | *l3 [m] |
3636
subpaths
3737
#select
38-
| 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'. |
39-
| 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'. |
40-
| 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'. |
41-
| 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'. |
42-
| 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'. |
43-
| 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'. |
44-
| 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'. |
45-
| 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'. |
38+
| 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 |
39+
| 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 |
40+
| 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 |
41+
| 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 |
42+
| 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 |
43+
| 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 |
44+
| 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 |
45+
| 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 |
Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1-
| test.c:22:15:22:16 | definition of l3 | Atomic object 'l3' has no initializer or corresponding use of 'atomic_init()'. |
2-
| test.c:25:15:25:16 | definition of l4 | Atomic object 'l4' has no initializer or corresponding use of 'atomic_init()'. |
3-
| test.c:29:15:29:16 | definition of l5 | Atomic object 'l5' has no initializer or corresponding use of 'atomic_init()'. |
1+
| test.c:24:15:24:16 | definition of l3 | Atomic object 'l3' has no initializer or corresponding use of 'atomic_init()'. |
2+
| test.c:27:15:27:16 | definition of l4 | Atomic object 'l4' has no initializer or corresponding use of 'atomic_init()'. |
3+
| test.c:31:15:31:16 | definition of l5 | Atomic object 'l5' has no initializer or corresponding use of 'atomic_init()'. |
4+
| test.c:41:15:41:16 | definition of l7 | Atomic object 'l7' has no initializer or corresponding use of 'atomic_init()'. |

c/misra/test/rules/RULE-9-7/test.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ void f_starts_thread() {
1111
thrd_create(&t, f_thread, 0);
1212
}
1313

14+
void f_may_initialize_argument(void *p1) {}
15+
1416
void main() {
1517
_Atomic int l1 = 1; // COMPLIANT
1618
f_starts_thread();
@@ -31,4 +33,14 @@ void main() {
3133
atomic_init(&l5, 0);
3234
}
3335
f_starts_thread();
36+
37+
_Atomic int l6; // COMPLIANT
38+
f_may_initialize_argument(&l6);
39+
f_starts_thread();
40+
41+
_Atomic int l7; // NON_COMPLIANT
42+
if (g1 == 0) {
43+
f_may_initialize_argument(&l7);
44+
}
45+
f_starts_thread();
3446
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/**
2+
* This module intends to reduce the difficulty of handling the pattern where implementations
3+
* implement a function as a macro: the class `StdFunctionOrMacro<...>::Call` matches both std
4+
* function calls as well as std function macro expansions.
5+
*
6+
* For instance, `atomic_init` may be implemented as a function, but is also implemented as
7+
* `#DEFINE atomic_init(x) __c11_atomic_init(x)` on some platforms. This module aids in finding
8+
* calls to any standard function which may be a macro, and has predefined behavior for
9+
* handling `__c11_*` macros.
10+
*
11+
* Since a macro can be defined to expand to any expression, we cannot know generally which
12+
* expanded expressions in `f(x, y)` correspond to arguments `x` or `y`. To handle this, the
13+
* following inference options are available:
14+
* - `NoMacroExpansionInference`: Assume any expression in the macro expansion could correspond to
15+
* any macro argument.
16+
* - `C11FunctionWrapperMacro`: Check if the macro expands to a function call prefixed with
17+
* `__c11_` and if so, return the corresponding argument. Otherwise, fall back to
18+
* `NoMacroExpansionInference`.
19+
* - `InferMacroExpansionArguments`: Implement your own logic for inferring the argument.
20+
*
21+
* To use this module, pick one of the above inference strategies, and then create a predicate for
22+
* the name you wish to match. For instance:
23+
*
24+
* ```codeql
25+
* private string atomicInit() { result = "atomic_init" }
26+
*
27+
* from StdFunctionOrMacro<C11FunctionWrapperMacro, atomicInit/0>::Call c
28+
* select c.getArgument(0)
29+
* ```
30+
*/
31+
32+
import cpp as cpp
33+
34+
/** Specify the name of your function as a predicate */
35+
signature string getName();
36+
37+
/** Signature module to implement custom argument resolution behavior in expanded macros */
38+
signature module InferMacroExpansionArguments {
39+
bindingset[mi, argumentIdx]
40+
cpp::Expr inferArgument(cpp::MacroInvocation mi, int argumentIdx);
41+
}
42+
43+
/** Assume all subexpressions of an expanded macro may be the result of any ith argument */
44+
module NoMacroExpansionInference implements InferMacroExpansionArguments {
45+
bindingset[mi, argumentIdx]
46+
cpp::Expr inferArgument(cpp::MacroInvocation mi, int argumentIdx) {
47+
result.getParent*() = mi.getExpr()
48+
}
49+
}
50+
51+
/** Assume macro `f(x, y, ...)` expands to `__c11_f(x, y, ...)`. */
52+
module C11FunctionWrapperMacro implements InferMacroExpansionArguments {
53+
bindingset[mi, argumentIdx]
54+
cpp::Expr inferArgument(cpp::MacroInvocation mi, int argumentIdx) {
55+
if mi.getExpr().(cpp::FunctionCall).getTarget().hasName("__c11_" + mi.getMacroName())
56+
then result = mi.getExpr().(cpp::FunctionCall).getArgument(argumentIdx)
57+
else result = NoMacroExpansionInference::inferArgument(mi, argumentIdx)
58+
}
59+
}
60+
61+
/**
62+
* A module to find calls to standard functions, or expansions of macros with the same name.
63+
*
64+
* To use this module, specify a name predicate and an inference strategy for correlating macro
65+
* expansions to macro arguments.
66+
*
67+
* For example:
68+
*
69+
* ```codeql
70+
* private string atomicInit() { result = "atomic_init" }
71+
* from StdFunctionOrMacro<C11FunctionWrapperMacro, atomicInit/0>::Call c
72+
* select c.getArgument(0)
73+
* ```
74+
*/
75+
module StdFunctionOrMacro<InferMacroExpansionArguments InferExpansion, getName/0 getStdName> {
76+
final private class Expr = cpp::Expr;
77+
78+
final private class FunctionCall = cpp::FunctionCall;
79+
80+
final private class MacroInvocation = cpp::MacroInvocation;
81+
82+
private newtype TStdCall =
83+
TStdFunctionCall(FunctionCall fc) { fc.getTarget().hasName(getStdName()) } or
84+
TStdMacroInvocation(MacroInvocation mi) { mi.getMacro().hasName(getStdName()) }
85+
86+
/**
87+
* A call to a standard function or an expansion of a macro with the same name.
88+
*/
89+
class Call extends TStdCall {
90+
bindingset[this, argumentIdx]
91+
Expr getArgument(int argumentIdx) {
92+
exists(FunctionCall fc |
93+
this = TStdFunctionCall(fc) and
94+
result = fc.getArgument(argumentIdx)
95+
)
96+
or
97+
exists(MacroInvocation mi |
98+
this = TStdMacroInvocation(mi) and
99+
result = InferExpansion::inferArgument(mi, argumentIdx)
100+
)
101+
}
102+
103+
string toString() {
104+
this = TStdFunctionCall(_) and
105+
result = "Standard function call"
106+
or
107+
this = TStdMacroInvocation(_) and
108+
result = "Invocation of a standard function implemented as a macro"
109+
}
110+
}
111+
}

rule_packages/c/Concurrency7.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@
1515
"tags": [
1616
"concurrency",
1717
"external/misra/c/2012/amendment4"
18-
]
18+
],
19+
"implementation_scope": {
20+
"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.",
21+
"items": []
22+
}
1923
}
2024
],
2125
"title": "Atomic objects shall be appropriately initialized before being accessed"

0 commit comments

Comments
 (0)