Skip to content

Commit 01b605b

Browse files
MichaelRFairhurstMichaelRFairhurst
committed
Implement Rule 4-6-1, memory operations should be properly sequenced.
Split configs for C++14 and C++17 sequencing. Fix bug in reciprocity.
1 parent c607e3a commit 01b605b

File tree

15 files changed

+243
-23
lines changed

15 files changed

+243
-23
lines changed

‎cpp/autosar/src/rules/A5-0-1/ExpressionShouldNotRelyOnOrderOfEvaluation.ql‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,21 @@
1515

1616
import cpp
1717
import codingstandards.cpp.autosar
18-
import codingstandards.cpp.SideEffect
19-
import codingstandards.cpp.sideeffect.DefaultEffects
18+
import codingstandards.cpp.rules.expressionwithunsequencedsideeffects.ExpressionWithUnsequencedSideEffects
2019
import codingstandards.cpp.Ordering
2120
import codingstandards.cpp.orderofevaluation.VariableAccessOrdering
22-
import Ordering::Make<VariableAccessInFullExpressionOrdering> as FullExprOrdering
21+
import Ordering::Make<Cpp14VariableAccessInFullExpressionOrdering> as FullExprOrdering
2322

24-
from FullExpr e, VariableEffect ve, VariableAccess va1, VariableAccess va2, Variable v
25-
where
26-
not isExcluded(e, OrderOfEvaluationPackage::expressionShouldNotRelyOnOrderOfEvaluationQuery()) and
27-
e = va1.(ConstituentExpr).getFullExpr() and
28-
va1 = ve.getAnAccess() and
29-
FullExprOrdering::isUnsequenced(va1, va2) and
30-
v = va1.getTarget()
31-
select e,
32-
"The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@.",
33-
va1, "sub-expression", ve, "expression", va2, "sub-expression", v, v.getName()
23+
module ExpressionShouldNotRelyOnOrderOfEvaluationConfig implements
24+
ExpressionWithUnsequencedSideEffectsConfigSig
25+
{
26+
Query getQuery() {
27+
result = OrderOfEvaluationPackage::expressionShouldNotRelyOnOrderOfEvaluationQuery()
28+
}
29+
30+
predicate isUnsequenced(VariableAccess va1, VariableAccess va2) {
31+
FullExprOrdering::isUnsequenced(va1, va2)
32+
}
33+
}
34+
35+
import ExpressionWithUnsequencedSideEffects<ExpressionShouldNotRelyOnOrderOfEvaluationConfig>

‎cpp/common/src/codingstandards/cpp/Ordering.qll‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,15 @@ module Ordering {
132132
predicate cpp17Edge(ExprEvaluationNode n1, ExprEvaluationNode n2) {
133133
cpp14Edge(n1, n2)
134134
or
135+
// [expr.call] The "postfix expression" (the part before the `(...)`) is sequenced before each
136+
// expression in the expression list.
137+
exists(Call call, Expr qual, Expr arg |
138+
call.getQualifier() = qual and
139+
call.getAnArgument() = arg
140+
|
141+
qual = n1.toExpr().getParent*() and arg = n2.toExpr().getParent*()
142+
)
143+
or
135144
// [expr.sub] - (in) the expression E1[E2] ... E1 is sequenced before E2
136145
exists(ArrayExpr ce, ConstituentExpr lhs, ConstituentExpr rhs |
137146
lhs = ce.getArrayBase() and
@@ -140,10 +149,39 @@ module Ordering {
140149
lhs = n1.toExpr().getParent*() and rhs = n2.toExpr().getParent*()
141150
)
142151
or
143-
// [expr.sub] - (in) the expression E1[E2] ... E1 is sequenced before E2
144-
exists(NewExpr newExpr, ConstituentExpr lhs, ConstituentExpr rhs |
145-
n1.toExpr() = newExpr.getAllocatorCall() and
146-
n2.toExpr() = newExpr.getInitializer().getAChild()
152+
// [expr.new] -- invocation of the allocation function is sequenced before the expressions in
153+
// the new-initializer.
154+
exists(NewExpr newExpr, ConstituentExpr alloc, ConstituentExpr arg |
155+
alloc = newExpr.getAllocatorCall() and
156+
arg = newExpr.getInitializer().getAChild()
157+
|
158+
alloc = n1.toExpr().getParent*() and
159+
arg = n2.toExpr().getParent*()
160+
)
161+
or
162+
// [expr.mptr.oper] - In E.*E2, E1 is sequenced before E2. This is not the case for E->*E2.
163+
exists(PointerToMemberExpr ptrToMember, ConstituentExpr object, ConstituentExpr ptr |
164+
// TODO: distinguish between `.*` and `->*` operators.
165+
object = ptrToMember.getObjectExpr() and
166+
ptr = ptrToMember.getPointerExpr()
167+
|
168+
object = n1.toExpr().getParent*() and ptr = n2.toExpr().getParent*()
169+
)
170+
or
171+
// [expr.shift] In E1 << E2 and E1 >> E2, E1 is sequenced before E2.
172+
exists(BitShiftExpr shift, ConstituentExpr lhs, ConstituentExpr rhs |
173+
lhs = shift.getLeftOperand() and
174+
rhs = shift.getRightOperand()
175+
|
176+
lhs = n1.toExpr().getParent*() and rhs = n2.toExpr().getParent*()
177+
)
178+
or
179+
// [expr.ass] The right operand is sequenced before the left operand for all assignment operators.
180+
exists(Assignment assign, ConstituentExpr lhs, ConstituentExpr rhs |
181+
lhs = assign.getLValue() and
182+
rhs = assign.getRValue()
183+
|
184+
lhs = n1.toExpr().getParent*() and rhs = n2.toExpr().getParent*()
147185
)
148186
}
149187

@@ -172,5 +210,8 @@ module Ordering {
172210
predicate isValueComputation() { this = TValueComputationNode(_) }
173211

174212
predicate isSideEffect() { this = TSideEffectNode(_, _) }
213+
214+
Location getLocation() { result = toExpr().getLocation() }
175215
}
216+
176217
}

‎cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ import Representation
6060
import Scope
6161
import SideEffects1
6262
import SideEffects2
63+
import SideEffects4
6364
import SmartPointers1
6465
import SmartPointers2
6566
import Statements
@@ -132,6 +133,7 @@ newtype TCPPQuery =
132133
TScopePackageQuery(ScopeQuery q) or
133134
TSideEffects1PackageQuery(SideEffects1Query q) or
134135
TSideEffects2PackageQuery(SideEffects2Query q) or
136+
TSideEffects4PackageQuery(SideEffects4Query q) or
135137
TSmartPointers1PackageQuery(SmartPointers1Query q) or
136138
TSmartPointers2PackageQuery(SmartPointers2Query q) or
137139
TStatementsPackageQuery(StatementsQuery q) or
@@ -204,6 +206,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
204206
isScopeQueryMetadata(query, queryId, ruleId, category) or
205207
isSideEffects1QueryMetadata(query, queryId, ruleId, category) or
206208
isSideEffects2QueryMetadata(query, queryId, ruleId, category) or
209+
isSideEffects4QueryMetadata(query, queryId, ruleId, category) or
207210
isSmartPointers1QueryMetadata(query, queryId, ruleId, category) or
208211
isSmartPointers2QueryMetadata(query, queryId, ruleId, category) or
209212
isStatementsQueryMetadata(query, queryId, ruleId, category) or

‎cpp/common/src/codingstandards/cpp/exclusions/cpp/SideEffects4.qll‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/
2+
import cpp
3+
import RuleMetadata
4+
import codingstandards.cpp.exclusions.RuleMetadata
5+
6+
newtype SideEffects4Query = TMemoryUsageNotSequencedQuery()
7+
8+
predicate isSideEffects4QueryMetadata(Query query, string queryId, string ruleId, string category) {
9+
query =
10+
// `Query` instance for the `memoryUsageNotSequenced` query
11+
SideEffects4Package::memoryUsageNotSequencedQuery() and
12+
queryId =
13+
// `@id` for the `memoryUsageNotSequenced` query
14+
"cpp/misra/memory-usage-not-sequenced" and
15+
ruleId = "RULE-4-6-1" and
16+
category = "required"
17+
}
18+
19+
module SideEffects4Package {
20+
Query memoryUsageNotSequencedQuery() {
21+
//autogenerate `Query` type
22+
result =
23+
// `Query` type for `memoryUsageNotSequenced` query
24+
TQueryCPP(TSideEffects4PackageQuery(TMemoryUsageNotSequencedQuery()))
25+
}
26+
}

‎cpp/common/src/codingstandards/cpp/orderofevaluation/VariableAccessOrdering.qll‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import cpp
22
import codingstandards.cpp.Ordering
33

4-
module VariableAccessInFullExpressionOrdering implements Ordering::ConfigSig {
4+
module Cpp14VariableAccessInFullExpressionOrdering implements Ordering::ConfigSig {
55
import Ordering::CppConfigBase
66

77
predicate isCandidate(Expr e1, Expr e2) { isCandidate(_, e1, e2) }
@@ -12,6 +12,17 @@ module VariableAccessInFullExpressionOrdering implements Ordering::ConfigSig {
1212
}
1313
}
1414

15+
module Cpp17VariableAccessInFullExpressionOrdering implements Ordering::ConfigSig {
16+
import Ordering::CppConfigBase
17+
18+
predicate isCandidate(Expr e1, Expr e2) { isCandidate(_, e1, e2) }
19+
20+
pragma[inline]
21+
predicate sequencingEdge(Ordering::ExprEvaluationNode n1, Ordering::ExprEvaluationNode n2) {
22+
Ordering::cpp17Edge(n1, n2)
23+
}
24+
}
25+
1526
pragma[noinline]
1627
private predicate isConstituentOf(FullExpr e, VariableAccess ce) {
1728
ce.(ConstituentExpr).getFullExpr() = e
@@ -35,6 +46,9 @@ predicate isCandidatePair(
3546
* an other full expression that is sequenced after the value computation (and therefore cannot influence them at that point in the evaluation).
3647
*/
3748
predicate isCandidate(FullExpr e, VariableAccess va1, VariableAccess va2) {
49+
// Ensure va1 and va2 are reciprocal candidates.
50+
isCandidate(e, va2, va1)
51+
or
3852
exists(Variable v, VariableEffect ve |
3953
isCandidatePair(e, va1, va2, v, v) and
4054
ve.getAnAccess() = va1 and

‎cpp/common/src/codingstandards/cpp/rules/expressionwithunsequencedsideeffects/ExpressionWithUnsequencedSideEffects.qll‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Provides a configurable module ExpressionWithUnsequencedSideEffects with a `problems` predicate
3+
* for the following issue:
4+
* Code behavior should not have unsequenced or indeterminately sequenced operations on
5+
* the same memory location.
6+
*/
7+
8+
import cpp
9+
import codingstandards.cpp.Customizations
10+
import codingstandards.cpp.Exclusions
11+
import codingstandards.cpp.SideEffect
12+
import codingstandards.cpp.sideeffect.DefaultEffects
13+
import codingstandards.cpp.Ordering
14+
15+
signature module ExpressionWithUnsequencedSideEffectsConfigSig {
16+
Query getQuery();
17+
18+
predicate isUnsequenced(VariableAccess va1, VariableAccess va2);
19+
}
20+
21+
module ExpressionWithUnsequencedSideEffects<ExpressionWithUnsequencedSideEffectsConfigSig Config> {
22+
query predicate problems(
23+
FullExpr e, string message, VariableAccess va1, string va1Desc, VariableEffect ve,
24+
string veDesc, VariableAccess va2, string va2Desc, Variable v, string vName
25+
) {
26+
not isExcluded(e, Config::getQuery()) and
27+
e = va1.(ConstituentExpr).getFullExpr() and
28+
va1 = ve.getAnAccess() and
29+
Config::isUnsequenced(va1, va2) and
30+
v = va1.getTarget() and
31+
message =
32+
"The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@." and
33+
va1Desc = "sub-expression" and
34+
veDesc = "expression" and
35+
va2Desc = "sub-expression" and
36+
vName = v.getName()
37+
}
38+
}

‎cpp/common/test/rules/expressionwithunsequencedsideeffects/ExpressionWithUnsequencedSideEffects.expected‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
| test.cpp:5:12:5:24 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:5:21:5:22 | l1 | sub-expression | test.cpp:5:21:5:24 | ... ++ | expression | test.cpp:5:15:5:16 | l1 | sub-expression | test.cpp:2:7:2:8 | l1 | l1 |
2+
| test.cpp:13:12:13:13 | call to f2 | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:13:15:13:16 | l1 | sub-expression | test.cpp:13:15:13:18 | ... ++ | expression | test.cpp:13:21:13:22 | l1 | sub-expression | test.cpp:12:7:12:8 | l1 | l1 |
3+
| test.cpp:21:7:21:8 | call to m1 | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:21:10:21:11 | p1 | sub-expression | test.cpp:21:10:21:13 | ... ++ | expression | test.cpp:21:3:21:4 | p1 | sub-expression | test.cpp:20:13:20:14 | p1 | p1 |
4+
| test.cpp:26:29:26:53 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:26:34:26:35 | p1 | sub-expression | test.cpp:25:34:25:40 | ... += ... | expression | test.cpp:26:48:26:49 | p1 | sub-expression | test.cpp:26:14:26:15 | p1 | p1 |
5+
| test.cpp:26:29:26:53 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:26:48:26:49 | p1 | sub-expression | test.cpp:25:34:25:40 | ... += ... | expression | test.cpp:26:34:26:35 | p1 | sub-expression | test.cpp:26:14:26:15 | p1 | p1 |
6+
| test.cpp:32:3:32:16 | ... = ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:32:8:32:9 | l1 | sub-expression | test.cpp:32:8:32:16 | ... = ... | expression | test.cpp:32:13:32:14 | l1 | sub-expression | test.cpp:29:7:29:8 | l1 | l1 |
7+
| test.cpp:32:3:32:16 | ... = ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:32:13:32:14 | l1 | sub-expression | test.cpp:32:13:32:16 | ... ++ | expression | test.cpp:32:8:32:9 | l1 | sub-expression | test.cpp:29:7:29:8 | l1 | l1 |
8+
| test.cpp:39:12:39:18 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:39:12:39:13 | l1 | sub-expression | test.cpp:39:12:39:13 | l1 | expression | test.cpp:39:17:39:18 | l1 | sub-expression | test.cpp:37:16:37:17 | l1 | l1 |
9+
| test.cpp:39:12:39:18 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:39:17:39:18 | l1 | sub-expression | test.cpp:39:17:39:18 | l1 | expression | test.cpp:39:12:39:13 | l1 | sub-expression | test.cpp:37:16:37:17 | l1 | l1 |
10+
| test.cpp:62:11:62:12 | call to m1 | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:62:4:62:5 | p1 | sub-expression | test.cpp:62:4:62:7 | ... ++ | expression | test.cpp:62:14:62:15 | p1 | sub-expression | test.cpp:61:14:61:15 | p1 | p1 |
11+
| test.cpp:67:3:67:20 | ... >> ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:67:5:67:6 | p1 | sub-expression | test.cpp:67:5:67:8 | ... ++ | expression | test.cpp:67:16:67:17 | p1 | sub-expression | test.cpp:66:15:66:16 | p1 | p1 |
12+
| test.cpp:67:3:67:20 | ... >> ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:67:16:67:17 | p1 | sub-expression | test.cpp:67:16:67:19 | ... ++ | expression | test.cpp:67:5:67:6 | p1 | sub-expression | test.cpp:66:15:66:16 | p1 | p1 |

‎cpp/common/test/rules/expressionwithunsequencedsideeffects/ExpressionWithUnsequencedSideEffects.ql‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import codingstandards.cpp.rules.expressionwithunsequencedsideeffects.ExpressionWithUnsequencedSideEffects
2+
import codingstandards.cpp.Ordering
3+
import codingstandards.cpp.orderofevaluation.VariableAccessOrdering
4+
import Ordering::Make<Cpp14VariableAccessInFullExpressionOrdering> as FullExprOrdering
5+
6+
module TestFileConfig implements ExpressionWithUnsequencedSideEffectsConfigSig {
7+
Query getQuery() { result instanceof TestQuery }
8+
9+
predicate isUnsequenced(VariableAccess va1, VariableAccess va2) {
10+
FullExprOrdering::isUnsequenced(va1, va2)
11+
}
12+
}
13+
14+
import ExpressionWithUnsequencedSideEffects<TestFileConfig>

‎cpp/common/test/rules/expressionwithunsequencedsideeffects/ExpressionWithUnsequencedSideEffectsCpp17.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
| test.cpp:5:12:5:24 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:5:21:5:22 | l1 | sub-expression | test.cpp:5:21:5:24 | ... ++ | expression | test.cpp:5:15:5:16 | l1 | sub-expression | test.cpp:2:7:2:8 | l1 | l1 |
2+
| test.cpp:13:12:13:13 | call to f2 | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:13:15:13:16 | l1 | sub-expression | test.cpp:13:15:13:18 | ... ++ | expression | test.cpp:13:21:13:22 | l1 | sub-expression | test.cpp:12:7:12:8 | l1 | l1 |
3+
| test.cpp:26:29:26:53 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:26:34:26:35 | p1 | sub-expression | test.cpp:25:34:25:40 | ... += ... | expression | test.cpp:26:48:26:49 | p1 | sub-expression | test.cpp:26:14:26:15 | p1 | p1 |
4+
| test.cpp:26:29:26:53 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:26:48:26:49 | p1 | sub-expression | test.cpp:25:34:25:40 | ... += ... | expression | test.cpp:26:34:26:35 | p1 | sub-expression | test.cpp:26:14:26:15 | p1 | p1 |
5+
| test.cpp:39:12:39:18 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:39:12:39:13 | l1 | sub-expression | test.cpp:39:12:39:13 | l1 | expression | test.cpp:39:17:39:18 | l1 | sub-expression | test.cpp:37:16:37:17 | l1 | l1 |
6+
| test.cpp:39:12:39:18 | ... + ... | The evaluation is depended on the order of evaluation of $@, that is modified by $@ and $@, that both access $@. | test.cpp:39:17:39:18 | l1 | sub-expression | test.cpp:39:17:39:18 | l1 | expression | test.cpp:39:12:39:13 | l1 | sub-expression | test.cpp:37:16:37:17 | l1 | l1 |

‎cpp/common/test/rules/expressionwithunsequencedsideeffects/ExpressionWithUnsequencedSideEffectsCpp17.ql‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import codingstandards.cpp.rules.expressionwithunsequencedsideeffects.ExpressionWithUnsequencedSideEffects
2+
import codingstandards.cpp.Ordering
3+
import codingstandards.cpp.orderofevaluation.VariableAccessOrdering
4+
import Ordering::Make<Cpp17VariableAccessInFullExpressionOrdering> as FullExprOrdering
5+
6+
module TestFileConfig implements ExpressionWithUnsequencedSideEffectsConfigSig {
7+
Query getQuery() { result instanceof TestQuery }
8+
9+
predicate isUnsequenced(VariableAccess va1, VariableAccess va2) {
10+
FullExprOrdering::isUnsequenced(va1, va2)
11+
}
12+
}
13+
14+
import ExpressionWithUnsequencedSideEffects<TestFileConfig>

0 commit comments

Comments
 (0)