Skip to content

Commit aeb667c

Browse files
MathiasVPMathiasVP
authored
Merge pull request #15976 from MathiasVP/guards-eq-follow-up
C++: Fix interface for `GuardCondition.comparesEq` and `GuardCondition.ensuresEq`
2 parents ee411cc + c640bd6 commit aeb667c

File tree

4 files changed

+51
-54
lines changed

4 files changed

+51
-54
lines changed

‎cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll‎

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ class GuardCondition extends Expr {
138138
cached
139139
predicate ensuresEq(Expr left, Expr right, int k, BasicBlock block, boolean areEqual) { none() }
140140

141-
/** Holds if (determined by this guard) `e == k` evaluates to `areEqual` if this expression evaluates to `testIsTrue`. */
141+
/** Holds if (determined by this guard) `e == k` evaluates to `areEqual` if this expression evaluates to `value`. */
142142
cached
143-
predicate comparesEq(Expr e, int k, boolean areEqual, boolean testIsTrue) { none() }
143+
predicate comparesEq(Expr e, int k, boolean areEqual, AbstractValue value) { none() }
144144

145145
/**
146146
* Holds if (determined by this guard) `e == k` must be `areEqual` in `block`.
@@ -196,17 +196,18 @@ private class GuardConditionFromBinaryLogicalOperator extends GuardCondition {
196196
)
197197
}
198198

199-
override predicate comparesEq(Expr e, int k, boolean areEqual, boolean testIsTrue) {
200-
exists(boolean partIsTrue, GuardCondition part |
201-
this.(BinaryLogicalOperation).impliesValue(part, partIsTrue, testIsTrue)
199+
override predicate comparesEq(Expr e, int k, boolean areEqual, AbstractValue value) {
200+
exists(BooleanValue partValue, GuardCondition part |
201+
this.(BinaryLogicalOperation)
202+
.impliesValue(part, partValue.getValue(), value.(BooleanValue).getValue())
202203
|
203-
part.comparesEq(e, k, areEqual, partIsTrue)
204+
part.comparesEq(e, k, areEqual, partValue)
204205
)
205206
}
206207

207208
override predicate ensuresEq(Expr e, int k, BasicBlock block, boolean areEqual) {
208-
exists(boolean testIsTrue |
209-
this.comparesEq(e, k, areEqual, testIsTrue) and this.controls(block, testIsTrue)
209+
exists(AbstractValue value |
210+
this.comparesEq(e, k, areEqual, value) and this.valueControls(block, value)
210211
)
211212
}
212213
}
@@ -270,18 +271,18 @@ private class GuardConditionFromIR extends GuardCondition {
270271
)
271272
}
272273

273-
override predicate comparesEq(Expr e, int k, boolean areEqual, boolean testIsTrue) {
274+
override predicate comparesEq(Expr e, int k, boolean areEqual, AbstractValue value) {
274275
exists(Instruction i |
275276
i.getUnconvertedResultExpression() = e and
276-
ir.comparesEq(i.getAUse(), k, areEqual, testIsTrue)
277+
ir.comparesEq(i.getAUse(), k, areEqual, value)
277278
)
278279
}
279280

280281
override predicate ensuresEq(Expr e, int k, BasicBlock block, boolean areEqual) {
281-
exists(Instruction i, boolean testIsTrue |
282+
exists(Instruction i, AbstractValue value |
282283
i.getUnconvertedResultExpression() = e and
283-
ir.comparesEq(i.getAUse(), k, areEqual, testIsTrue) and
284-
this.controls(block, testIsTrue)
284+
ir.comparesEq(i.getAUse(), k, areEqual, value) and
285+
this.valueControls(block, value)
285286
)
286287
}
287288

@@ -492,19 +493,10 @@ class IRGuardCondition extends Instruction {
492493
)
493494
}
494495

495-
/** Holds if (determined by this guard) `op == k` evaluates to `areEqual` if this expression evaluates to `testIsTrue`. */
496+
/** Holds if (determined by this guard) `op == k` evaluates to `areEqual` if this expression evaluates to `value`. */
496497
cached
497-
predicate comparesEq(Operand op, int k, boolean areEqual, boolean testIsTrue) {
498-
exists(MatchValue mv |
499-
compares_eq(this, op, k, areEqual, mv) and
500-
// A match value cannot be dualized, so `testIsTrue` is always true
501-
testIsTrue = true
502-
)
503-
or
504-
exists(BooleanValue bv |
505-
compares_eq(this, op, k, areEqual, bv) and
506-
bv.getValue() = testIsTrue
507-
)
498+
predicate comparesEq(Operand op, int k, boolean areEqual, AbstractValue value) {
499+
compares_eq(this, op, k, areEqual, value)
508500
}
509501

510502
/**

‎cpp/ql/test/library-tests/controlflow/guards-ir/tests.ql‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import semmle.code.cpp.controlflow.IRGuards
44
query predicate astGuards(GuardCondition guard) { any() }
55

66
query predicate astGuardsCompare(int startLine, string msg) {
7-
exists(GuardCondition guard, Expr left, int k, string which, string op |
8-
exists(boolean sense |
7+
exists(GuardCondition guard, Expr left, int k, string op |
8+
exists(boolean sense, string which |
99
sense = true and which = "true"
1010
or
1111
sense = false and which = "false"
@@ -21,14 +21,16 @@ query predicate astGuardsCompare(int startLine, string msg) {
2121
|
2222
msg = left + op + right + "+" + k + " when " + guard + " is " + which
2323
)
24+
)
25+
or
26+
exists(AbstractValue value |
27+
guard.comparesEq(left, k, true, value) and op = " == "
2428
or
25-
(
26-
guard.comparesEq(left, k, true, sense) and op = " == "
27-
or
28-
guard.comparesEq(left, k, false, sense) and op = " != "
29-
) and
30-
msg = left + op + k + " when " + guard + " is " + which
31-
) and
29+
guard.comparesEq(left, k, false, value) and op = " != "
30+
|
31+
msg = left + op + k + " when " + guard + " is " + value
32+
)
33+
|
3234
startLine = guard.getLocation().getStartLine()
3335
)
3436
}
@@ -71,8 +73,8 @@ query predicate astGuardsEnsure_const(
7173
query predicate irGuards(IRGuardCondition guard) { any() }
7274

7375
query predicate irGuardsCompare(int startLine, string msg) {
74-
exists(IRGuardCondition guard, Operand left, int k, string which, string op |
75-
exists(boolean sense |
76+
exists(IRGuardCondition guard, Operand left, int k, string op |
77+
exists(boolean sense, string which |
7678
sense = true and which = "true"
7779
or
7880
sense = false and which = "false"
@@ -91,16 +93,18 @@ query predicate irGuardsCompare(int startLine, string msg) {
9193
right.getAnyDef().getUnconvertedResultExpression() + "+" + k + " when " + guard + " is "
9294
+ which
9395
)
96+
)
97+
or
98+
exists(AbstractValue value |
99+
guard.comparesEq(left, k, true, value) and op = " == "
94100
or
95-
(
96-
guard.comparesEq(left, k, true, sense) and op = " == "
97-
or
98-
guard.comparesEq(left, k, false, sense) and op = " != "
99-
) and
101+
guard.comparesEq(left, k, false, value) and op = " != "
102+
|
100103
msg =
101104
left.getAnyDef().getUnconvertedResultExpression() + op + k + " when " + guard + " is " +
102-
which
103-
) and
105+
value
106+
)
107+
|
104108
startLine = guard.getLocation().getStartLine()
105109
)
106110
}

‎cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@
5555
| 58 | y < 0+0 when ... < ... is true |
5656
| 58 | y >= 0+0 when ... < ... is false |
5757
| 58 | y >= 0+0 when ... \|\| ... is false |
58-
| 61 | i == 0 when i is true |
59-
| 61 | i == 1 when i is true |
60-
| 61 | i == 2 when i is true |
58+
| 61 | i == 0 when i is Case[0] |
59+
| 61 | i == 1 when i is Case[1] |
60+
| 61 | i == 2 when i is Case[2] |
6161
| 75 | 0 != x+0 when ... == ... is false |
6262
| 75 | 0 == x+0 when ... == ... is true |
6363
| 75 | x != 0 when ... == ... is false |

‎cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.ql‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
import cpp
88
import semmle.code.cpp.controlflow.Guards
99

10-
from GuardCondition guard, Expr left, int k, string which, string op, string msg
10+
from GuardCondition guard, Expr left, int k, string op, string msg
1111
where
12-
exists(boolean sense |
12+
exists(boolean sense, string which |
1313
sense = true and which = "true"
1414
or
1515
sense = false and which = "false"
@@ -25,12 +25,13 @@ where
2525
|
2626
msg = left + op + right + "+" + k + " when " + guard + " is " + which
2727
)
28+
)
29+
or
30+
exists(AbstractValue value |
31+
guard.comparesEq(left, k, true, value) and op = " == "
2832
or
29-
(
30-
guard.comparesEq(left, k, true, sense) and op = " == "
31-
or
32-
guard.comparesEq(left, k, false, sense) and op = " != "
33-
) and
34-
msg = left + op + k + " when " + guard + " is " + which
33+
guard.comparesEq(left, k, false, value) and op = " != "
34+
|
35+
msg = left + op + k + " when " + guard + " is " + value
3536
)
3637
select guard.getLocation().getStartLine(), msg

0 commit comments

Comments
 (0)