Skip to content

Commit 30e5e74

Browse files
jketemajketema
authored
Merge pull request #15005 from jketema/ir-guards-ternary-fix
C++: Fix IRGuards ternary behaviour
2 parents e6a5c50 + 4d702e2 commit 30e5e74

File tree

3 files changed

+49
-3
lines changed

3 files changed

+49
-3
lines changed

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
import cpp
77
import semmle.code.cpp.ir.IR
8+
private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedExpr
9+
private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag
810

911
/**
1012
* Holds if `block` consists of an `UnreachedInstruction`.
@@ -201,10 +203,25 @@ private class GuardConditionFromIR extends GuardCondition {
201203
* `&&` and `||`. See the detailed explanation on predicate `controls`.
202204
*/
203205
private predicate controlsBlock(BasicBlock controlled, boolean testIsTrue) {
204-
exists(IRBlock irb |
206+
exists(IRBlock irb, Instruction instr |
205207
ir.controls(irb, testIsTrue) and
206-
irb.getAnInstruction().getAst().(ControlFlowNode).getBasicBlock() = controlled and
207-
not isUnreachedBlock(irb)
208+
instr = irb.getAnInstruction() and
209+
instr.getAst().(ControlFlowNode).getBasicBlock() = controlled and
210+
not isUnreachedBlock(irb) and
211+
not this.excludeAsControlledInstruction(instr)
212+
)
213+
}
214+
215+
private predicate excludeAsControlledInstruction(Instruction instr) {
216+
// Exclude the temporaries generated by a ternary expression.
217+
exists(TranslatedConditionalExpr tce |
218+
instr = tce.getInstruction(ConditionValueFalseStoreTag())
219+
or
220+
instr = tce.getInstruction(ConditionValueTrueStoreTag())
221+
or
222+
instr = tce.getInstruction(ConditionValueTrueTempAddressTag())
223+
or
224+
instr = tce.getInstruction(ConditionValueFalseTempAddressTag())
208225
)
209226
}
210227
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,3 +167,10 @@ int ptr_test(int *x, int *y) {
167167

168168
return 0;
169169
}
170+
171+
int foo(const char*, int);
172+
173+
int ternary_test(const char *path, int mode)
174+
{
175+
return (foo(path, mode) == 0 ? 1 : 0);
176+
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ astGuards
3434
| test.c:159:9:159:19 | ... == ... |
3535
| test.c:162:9:162:18 | ... < ... |
3636
| test.c:165:9:165:18 | ... < ... |
37+
| test.c:175:13:175:32 | ... == ... |
3738
| test.cpp:18:8:18:10 | call to get |
3839
| test.cpp:31:7:31:13 | ... == ... |
3940
| test.cpp:42:13:42:20 | call to getABool |
@@ -158,6 +159,10 @@ astGuardsCompare
158159
| 165 | x >= y+-42 when ... < ... is false |
159160
| 165 | y < x+43 when ... < ... is false |
160161
| 165 | y >= x+43 when ... < ... is true |
162+
| 175 | 0 != call to foo+0 when ... == ... is false |
163+
| 175 | 0 == call to foo+0 when ... == ... is true |
164+
| 175 | call to foo != 0+0 when ... == ... is false |
165+
| 175 | call to foo == 0+0 when ... == ... is true |
161166
astGuardsControl
162167
| test.c:7:9:7:13 | ... > ... | false | 10 | 11 |
163168
| test.c:7:9:7:13 | ... > ... | true | 7 | 9 |
@@ -248,6 +253,8 @@ astGuardsControl
248253
| test.c:159:9:159:19 | ... == ... | true | 159 | 160 |
249254
| test.c:162:9:162:18 | ... < ... | true | 162 | 163 |
250255
| test.c:165:9:165:18 | ... < ... | true | 165 | 166 |
256+
| test.c:175:13:175:32 | ... == ... | false | 175 | 175 |
257+
| test.c:175:13:175:32 | ... == ... | true | 175 | 175 |
251258
| test.cpp:18:8:18:10 | call to get | true | 19 | 19 |
252259
| test.cpp:31:7:31:13 | ... == ... | false | 30 | 30 |
253260
| test.cpp:31:7:31:13 | ... == ... | false | 34 | 34 |
@@ -420,6 +427,10 @@ astGuardsEnsure
420427
| test.c:165:9:165:18 | ... < ... | test.c:165:9:165:9 | x | < | test.c:165:13:165:18 | ... - ... | 0 | 165 | 166 |
421428
| test.c:165:9:165:18 | ... < ... | test.c:165:13:165:13 | y | >= | test.c:165:9:165:9 | x | 43 | 165 | 166 |
422429
| test.c:165:9:165:18 | ... < ... | test.c:165:13:165:18 | ... - ... | >= | test.c:165:9:165:9 | x | 1 | 165 | 166 |
430+
| test.c:175:13:175:32 | ... == ... | test.c:175:13:175:15 | call to foo | != | test.c:175:32:175:32 | 0 | 0 | 175 | 175 |
431+
| test.c:175:13:175:32 | ... == ... | test.c:175:13:175:15 | call to foo | == | test.c:175:32:175:32 | 0 | 0 | 175 | 175 |
432+
| test.c:175:13:175:32 | ... == ... | test.c:175:32:175:32 | 0 | != | test.c:175:13:175:15 | call to foo | 0 | 175 | 175 |
433+
| test.c:175:13:175:32 | ... == ... | test.c:175:32:175:32 | 0 | == | test.c:175:13:175:15 | call to foo | 0 | 175 | 175 |
423434
| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | 30 | 30 |
424435
| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | 34 | 34 |
425436
| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | test.cpp:31:12:31:13 | - ... | 0 | 30 | 30 |
@@ -458,6 +469,7 @@ irGuards
458469
| test.c:159:9:159:19 | CompareEQ: ... == ... |
459470
| test.c:162:9:162:18 | CompareLT: ... < ... |
460471
| test.c:165:9:165:18 | CompareLT: ... < ... |
472+
| test.c:175:13:175:32 | CompareEQ: ... == ... |
461473
| test.cpp:18:8:18:12 | CompareNE: (bool)... |
462474
| test.cpp:31:7:31:13 | CompareEQ: ... == ... |
463475
| test.cpp:42:13:42:20 | Call: call to getABool |
@@ -566,6 +578,10 @@ irGuardsCompare
566578
| 165 | x >= y+-42 when CompareLT: ... < ... is false |
567579
| 165 | y < x+43 when CompareLT: ... < ... is false |
568580
| 165 | y >= x+43 when CompareLT: ... < ... is true |
581+
| 175 | 0 != call to foo+0 when CompareEQ: ... == ... is false |
582+
| 175 | 0 == call to foo+0 when CompareEQ: ... == ... is true |
583+
| 175 | call to foo != 0+0 when CompareEQ: ... == ... is false |
584+
| 175 | call to foo == 0+0 when CompareEQ: ... == ... is true |
569585
irGuardsControl
570586
| test.c:7:9:7:13 | CompareGT: ... > ... | false | 11 | 11 |
571587
| test.c:7:9:7:13 | CompareGT: ... > ... | true | 8 | 8 |
@@ -649,6 +665,8 @@ irGuardsControl
649665
| test.c:159:9:159:19 | CompareEQ: ... == ... | true | 159 | 160 |
650666
| test.c:162:9:162:18 | CompareLT: ... < ... | true | 162 | 163 |
651667
| test.c:165:9:165:18 | CompareLT: ... < ... | true | 165 | 166 |
668+
| test.c:175:13:175:32 | CompareEQ: ... == ... | false | 175 | 175 |
669+
| test.c:175:13:175:32 | CompareEQ: ... == ... | true | 175 | 175 |
652670
| test.cpp:18:8:18:12 | CompareNE: (bool)... | true | 19 | 19 |
653671
| test.cpp:31:7:31:13 | CompareEQ: ... == ... | false | 34 | 34 |
654672
| test.cpp:31:7:31:13 | CompareEQ: ... == ... | true | 30 | 30 |
@@ -804,6 +822,10 @@ irGuardsEnsure
804822
| test.c:165:9:165:18 | CompareLT: ... < ... | test.c:165:9:165:9 | Load: x | < | test.c:165:13:165:18 | PointerSub: ... - ... | 0 | 165 | 166 |
805823
| test.c:165:9:165:18 | CompareLT: ... < ... | test.c:165:13:165:13 | Load: y | >= | test.c:165:9:165:9 | Load: x | 43 | 165 | 166 |
806824
| test.c:165:9:165:18 | CompareLT: ... < ... | test.c:165:13:165:18 | PointerSub: ... - ... | >= | test.c:165:9:165:9 | Load: x | 1 | 165 | 166 |
825+
| test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:13:175:15 | Call: call to foo | != | test.c:175:32:175:32 | Constant: 0 | 0 | 175 | 175 |
826+
| test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:13:175:15 | Call: call to foo | == | test.c:175:32:175:32 | Constant: 0 | 0 | 175 | 175 |
827+
| test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:32:175:32 | Constant: 0 | != | test.c:175:13:175:15 | Call: call to foo | 0 | 175 | 175 |
828+
| test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:32:175:32 | Constant: 0 | == | test.c:175:13:175:15 | Call: call to foo | 0 | 175 | 175 |
807829
| test.cpp:18:8:18:12 | CompareNE: (bool)... | test.cpp:18:8:18:10 | Call: call to get | != | test.cpp:18:8:18:12 | Constant: (bool)... | 0 | 19 | 19 |
808830
| test.cpp:18:8:18:12 | CompareNE: (bool)... | test.cpp:18:8:18:12 | Constant: (bool)... | != | test.cpp:18:8:18:10 | Call: call to get | 0 | 19 | 19 |
809831
| test.cpp:31:7:31:13 | CompareEQ: ... == ... | test.cpp:31:7:31:7 | Load: x | != | test.cpp:31:12:31:13 | Constant: - ... | 0 | 34 | 34 |

0 commit comments

Comments
 (0)