Skip to content

Commit d26d960

Browse files
MichaelRFairhurstMichaelRFairhurst
committed
Address feedback
1 parent 46b6fc9 commit d26d960

File tree

8 files changed

+84
-55
lines changed

8 files changed

+84
-55
lines changed

‎c/misra/src/codeql-suites/misra-c-audit.qls

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
- description: MISRA C 2012 (Audit)
2+
- qlpack: codeql/misra-c-coding-standards
3+
- include:
4+
kind:
5+
- problem
6+
- path-problem
7+
tags contain:
8+
- external/misra/c/audit

‎c/misra/src/rules/DIR-5-3/ThreadCreatedByThread.ql

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,27 +19,22 @@ import cpp
1919
import codingstandards.c.misra
2020
import codingstandards.cpp.Concurrency
2121

22-
Function callers(Function f) { result = f.getACallToThisFunction().getEnclosingFunction() }
22+
class CThreadRoot extends Function {
23+
CThreadCreateCall threadCreate;
2324

24-
class ThreadReachableFunction extends Function {
25-
/* The root threaded function from which this function is reachable */
26-
Function threadRoot;
27-
28-
ThreadReachableFunction() {
29-
exists(CThreadCreateCall tc |
30-
tc.getFunction() = callers*(this) and
31-
threadRoot = tc.getFunction()
32-
)
25+
CThreadRoot() {
26+
threadCreate.getFunction() = this
3327
}
3428

35-
/* Get the root threaded function from which this function is reachable */
36-
Function getThreadRoot() { result = threadRoot }
29+
/* Get a function which is reachable from this function */
30+
Function getAReachableFunction() { calls*(result) }
31+
32+
CThreadCreateCall getCThreadCreateCall() { result = threadCreate }
3733
}
3834

39-
from CThreadCreateCall tc, ThreadReachableFunction enclosingFunction, Function threadRoot
35+
from CThreadCreateCall tc, CThreadRoot threadRoot
4036
where
4137
not isExcluded(tc, Concurrency6Package::threadCreatedByThreadQuery()) and
42-
enclosingFunction = tc.getEnclosingFunction() and
43-
threadRoot = enclosingFunction.getThreadRoot()
44-
select tc, "Thread creation call reachable from threaded function '$@'.", threadRoot,
45-
threadRoot.toString()
38+
tc.getEnclosingFunction() = threadRoot.getAReachableFunction()
39+
select tc, "Thread creation call reachable from function '$@', which may also be $@.", threadRoot,
40+
threadRoot.toString(), threadRoot.getCThreadCreateCall(), "started as a thread"

‎c/misra/src/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ where
2222
(
2323
exists(FieldAccess fa |
2424
expr = fa and
25-
fa.getQualifier().getUnderlyingType().hasSpecifier("atomic") and
25+
fa.getQualifier().getType().hasSpecifier("atomic") and
2626
field = fa.getTarget()
2727
)
2828
or
2929
exists(PointerFieldAccess fa |
3030
expr = fa and
31-
fa.getQualifier().getUnderlyingType().(PointerType).getBaseType().hasSpecifier("atomic") and
31+
fa.getQualifier().getType().stripTopLevelSpecifiers().(PointerType).getBaseType().hasSpecifier("atomic") and
3232
field = fa.getTarget()
3333
)
3434
)

‎c/misra/src/rules/RULE-21-25/InvalidMemoryOrderArgument.ql

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class MemoryOrderConstantExpr extends Expr {
4646
}
4747

4848
/* Get the name of the `MemoryOrder` this expression is valued as. */
49-
string getMemoryOrderString() { result = ord.toString() }
49+
string getMemoryOrderString() { result = ord.getName() }
5050
}
5151

5252
/**
@@ -56,26 +56,26 @@ class MemoryOrderedStdAtomicFunction extends Function {
5656
int orderParamIdx;
5757

5858
MemoryOrderedStdAtomicFunction() {
59-
exists(int baseParamIdx, int baseParams, string prefix, string suffix |
60-
prefix = ["__", "__c11_"] and
61-
suffix = ["", ".*", "_explicit"] and
59+
exists(int baseParamIdx, int baseParams, string prefix, string regex, string basename |
60+
regex = "__(c11_)?atomic_([a-z_]+)" and
61+
prefix = getName().regexpCapture(regex, 1) and
62+
basename = "atomic_" + getName().regexpCapture(regex, 2) + ["", "_explicit"] and
6263
(
63-
getName().regexpMatch(prefix + ["atomic_thread_fence", "atomic_signal_fence"] + suffix) and
64+
basename in ["atomic_thread_fence", "atomic_signal_fence"] and
6465
baseParamIdx = 0 and
6566
baseParams = 1
6667
or
67-
getName()
68-
.regexpMatch(prefix + ["atomic_load", "atomic_flag_clear", "atomic_flag_test_and_set"] +
69-
suffix) and
68+
basename in ["atomic_load", "atomic_flag_clear", "atomic_flag_test_and_set"] and
7069
baseParamIdx = 1 and
7170
baseParams = 2
7271
or
73-
getName()
74-
.regexpMatch(prefix + ["atomic_store", "atomic_fetch_.*", "atomic_exchange"] + suffix) and
72+
basename in [
73+
"atomic_store", "atomic_fetch_" + ["add", "sub", "or", "xor", "and"], "atomic_exchange"
74+
] and
7575
baseParamIdx = 2 and
7676
baseParams = 3
7777
or
78-
getName().regexpMatch(prefix + "atomic_compare_exchange_.*" + suffix) and
78+
basename in ["atomic_compare_exchange_" + ["strong", "weak"]] and
7979
baseParamIdx = [3, 4] and
8080
baseParams = 5
8181
) and
@@ -84,8 +84,7 @@ class MemoryOrderedStdAtomicFunction extends Function {
8484
// __atomic_load(8, &repr->a, &desired, order)
8585
// or
8686
// __atomic_load_8(&repr->a, &desired, order)
87-
prefix = "__" and
88-
suffix = ".*" and
87+
prefix = "" and
8988
exists(int extraParams |
9089
extraParams = getNumberOfParameters() - baseParams and
9190
extraParams >= 0 and
@@ -94,13 +93,7 @@ class MemoryOrderedStdAtomicFunction extends Function {
9493
or
9594
// Clang case, no inserted parameters:
9695
// __c11_atomic_load(object, order)
97-
suffix = "" and
98-
prefix = "__c11_" and
99-
orderParamIdx = baseParamIdx
100-
or
101-
// Non-macro case, may occur in a subset of gcc/clang functions:
102-
prefix = "" and
103-
suffix = "_explicit" and
96+
prefix = "c11_" and
10497
orderParamIdx = baseParamIdx
10598
)
10699
)
@@ -122,17 +115,6 @@ module MemoryOrderFlowConfig implements DataFlow::ConfigSig {
122115
node.asExpr() = literal and
123116
not literal.getValue().toInt() = any(AllowedMemoryOrder a).getValue().toInt()
124117
)
125-
or
126-
// Everything else: not a memory order constant or an integer valued literal, also exclude
127-
// variables and functions, things that flow further back.
128-
exists(Expr e |
129-
node.asExpr() = e and
130-
not e instanceof MemoryOrderConstantAccess and
131-
not e instanceof Literal and
132-
not e instanceof VariableAccess and
133-
not e instanceof FunctionCall and
134-
not DataFlow::localFlowStep(_, node)
135-
)
136118
}
137119

138120
predicate isSink(DataFlow::Node node) {
@@ -169,4 +151,4 @@ where
169151
not value = any(AllowedMemoryOrder e).getName() and
170152
function.getACallToThisFunction().getAnArgument() = argument
171153
select argument, source, sink, "Invalid memory order '$@' in call to function '$@'.", value, value,
172-
function, function.toString()
154+
function, function.getName()

‎c/misra/test/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.expected

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,12 @@
22
| test.c:44:18:44:18 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
33
| test.c:45:13:45:13 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
44
| test.c:46:18:46:18 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
5+
| test.c:65:6:65:6 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
6+
| test.c:71:9:71:9 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
7+
| test.c:82:18:82:18 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
8+
| test.c:83:3:83:31 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
9+
| test.c:84:3:84:39 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
10+
| test.c:85:3:85:19 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
11+
| test.c:86:3:86:23 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
12+
| test.c:87:3:87:19 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |
13+
| test.c:88:3:88:23 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x |

‎c/misra/test/rules/RULE-12-6/test.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,32 @@ void f1() {
5858
*s1_atomic_ptr = (s1){0}; // COMPLIANT
5959
s1_atomic_ptr = l2; // COMPLIANT
6060
s1_atomic_ptr->x; // COMPLIANT
61+
62+
// Atomic specifier hidden behind a typedef, still atomic:
63+
typedef _Atomic s1 atomic_s1;
64+
atomic_s1 l3;
65+
l3.x; // NON_COMPLIANT
66+
67+
// Worst case scenario: a typedef of a volatile const pointer to an atomic
68+
// typedef type.
69+
typedef atomic_s1 *volatile const atomic_s1_specified_ptr;
70+
atomic_s1_specified_ptr l4;
71+
(l4)->x; // NON_COMPLIANT
72+
}
73+
74+
#define NOOP(x) (x)
75+
#define DOT_FIELD_ACCESS_X(v) (v).x
76+
#define POINTER_FIELD_ACCESS_X(v) (v)->x
77+
#define GET_X_ATOMIC_S1() atomic_s1.x
78+
#define GET_X_PTR_ATOMIC_S1() atomic_s1.x
79+
80+
void f2() {
81+
// Banned UB with user macros:
82+
NOOP(atomic_s1.x); // NON-COMPLIANT
83+
DOT_FIELD_ACCESS_X(atomic_s1); // NON-COMPLIANT
84+
POINTER_FIELD_ACCESS_X(ptr_atomic_s1); // NON-COMPLIANT
85+
GET_X_ATOMIC_S1(); // NON-COMPLIANT
86+
GET_X_PTR_ATOMIC_S1(); // NON-COMPLIANT
87+
GET_X_ATOMIC_S1() = 0; // NON-COMPLIANT
88+
GET_X_PTR_ATOMIC_S1() = 0; // NON-COMPLIANT
6189
}

‎c/misra/test/rules/RULE-21-25/InvalidMemoryOrderArgument.expected

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,35 @@ edges
1717
| test.c:4:5:4:6 | *g2 | test.c:68:23:68:24 | g2 | provenance | |
1818
| test.c:4:5:4:6 | *g2 | test.c:71:23:71:24 | g2 | provenance | |
1919
| test.c:4:10:4:29 | memory_order_relaxed | test.c:4:5:4:6 | *g2 | provenance | |
20+
| test.c:4:10:4:29 | memory_order_relaxed | test.c:4:10:4:29 | memory_order_relaxed | provenance | |
2021
| test.c:5:5:5:6 | *g3 | test.c:72:23:72:24 | g3 | provenance | |
2122
| test.c:5:10:5:29 | memory_order_acquire | test.c:5:5:5:6 | *g3 | provenance | |
23+
| test.c:5:10:5:29 | memory_order_acquire | test.c:5:10:5:29 | memory_order_acquire | provenance | |
2224
| test.c:6:5:6:6 | *g4 | test.c:73:23:73:24 | g4 | provenance | |
2325
| test.c:6:10:6:29 | memory_order_consume | test.c:6:5:6:6 | *g4 | provenance | |
26+
| test.c:6:10:6:29 | memory_order_consume | test.c:6:10:6:29 | memory_order_consume | provenance | |
2427
| test.c:7:5:7:6 | *g5 | test.c:74:23:74:24 | g5 | provenance | |
2528
| test.c:7:10:7:29 | memory_order_acq_rel | test.c:7:5:7:6 | *g5 | provenance | |
29+
| test.c:7:10:7:29 | memory_order_acq_rel | test.c:7:10:7:29 | memory_order_acq_rel | provenance | |
2630
| test.c:8:5:8:6 | *g6 | test.c:75:23:75:24 | g6 | provenance | |
2731
| test.c:8:10:8:29 | memory_order_release | test.c:8:5:8:6 | *g6 | provenance | |
32+
| test.c:8:10:8:29 | memory_order_release | test.c:8:10:8:29 | memory_order_release | provenance | |
2833
nodes
2934
| test.c:4:5:4:6 | *g2 | semmle.label | *g2 |
3035
| test.c:4:10:4:29 | memory_order_relaxed | semmle.label | memory_order_relaxed |
36+
| test.c:4:10:4:29 | memory_order_relaxed | semmle.label | memory_order_relaxed |
3137
| test.c:5:5:5:6 | *g3 | semmle.label | *g3 |
3238
| test.c:5:10:5:29 | memory_order_acquire | semmle.label | memory_order_acquire |
39+
| test.c:5:10:5:29 | memory_order_acquire | semmle.label | memory_order_acquire |
3340
| test.c:6:5:6:6 | *g4 | semmle.label | *g4 |
3441
| test.c:6:10:6:29 | memory_order_consume | semmle.label | memory_order_consume |
42+
| test.c:6:10:6:29 | memory_order_consume | semmle.label | memory_order_consume |
3543
| test.c:7:5:7:6 | *g5 | semmle.label | *g5 |
3644
| test.c:7:10:7:29 | memory_order_acq_rel | semmle.label | memory_order_acq_rel |
45+
| test.c:7:10:7:29 | memory_order_acq_rel | semmle.label | memory_order_acq_rel |
3746
| test.c:8:5:8:6 | *g6 | semmle.label | *g6 |
3847
| test.c:8:10:8:29 | memory_order_release | semmle.label | memory_order_release |
48+
| test.c:8:10:8:29 | memory_order_release | semmle.label | memory_order_release |
3949
| test.c:16:29:16:48 | memory_order_relaxed | semmle.label | memory_order_relaxed |
4050
| test.c:17:29:17:48 | memory_order_acquire | semmle.label | memory_order_acquire |
4151
| test.c:18:29:18:48 | memory_order_consume | semmle.label | memory_order_consume |
@@ -62,10 +72,8 @@ nodes
6272
| test.c:73:23:73:24 | g4 | semmle.label | g4 |
6373
| test.c:74:23:74:24 | g5 | semmle.label | g5 |
6474
| test.c:75:23:75:24 | g6 | semmle.label | g6 |
65-
| test.c:78:23:78:46 | ... * ... | semmle.label | ... * ... |
6675
| test.c:79:23:79:23 | 1 | semmle.label | 1 |
6776
| test.c:80:23:80:25 | 100 | semmle.label | 100 |
68-
| test.c:81:23:81:28 | ... + ... | semmle.label | ... + ... |
6977
subpaths
7078
#select
7179
| test.c:16:29:16:48 | memory_order_relaxed | test.c:16:29:16:48 | memory_order_relaxed | test.c:16:29:16:48 | memory_order_relaxed | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_load | __c11_atomic_load |
@@ -96,4 +104,3 @@ subpaths
96104
| test.c:75:23:75:24 | g6 | test.c:8:10:8:29 | memory_order_release | test.c:75:23:75:24 | g6 | Invalid memory order '$@' in call to function '$@'. | memory_order_release | memory_order_release | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence |
97105
| test.c:79:23:79:23 | 1 | test.c:79:23:79:23 | 1 | test.c:79:23:79:23 | 1 | Invalid memory order '$@' in call to function '$@'. | memory_order_consume | memory_order_consume | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence |
98106
| test.c:80:23:80:25 | 100 | test.c:80:23:80:25 | 100 | test.c:80:23:80:25 | 100 | Invalid memory order '$@' in call to function '$@'. | 100 | 100 | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence |
99-
| test.c:81:23:81:28 | ... + ... | test.c:81:23:81:28 | ... + ... | test.c:81:23:81:28 | ... + ... | Invalid memory order '$@' in call to function '$@'. | ... + ... | ... + ... | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence |

‎c/misra/test/rules/RULE-21-25/test.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ void f(int p) {
7878
atomic_thread_fence(memory_order_seq_cst * 1); // COMPLIANT
7979
atomic_thread_fence(1); // NON-COMPLIANT
8080
atomic_thread_fence(100); // NON-COMPLIANT
81-
atomic_thread_fence(g1 + 1); // NON-COMPLIANT
81+
atomic_thread_fence(g1 + 1); // NON_COMPLIANT[FALSE_NEGATIVE]
8282

8383
// No unsafe flow, currently accepted:
8484
atomic_thread_fence(p); // COMPLIANT

0 commit comments

Comments
 (0)