Skip to content

Commit 25647ba

Browse files
MathiasVPMathiasVP
committed
C++: Fix the AST wrapper for binary logical operators.
1 parent 6445fd8 commit 25647ba

File tree

4 files changed

+38
-14
lines changed

4 files changed

+38
-14
lines changed

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

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -684,24 +684,26 @@ final class GuardCondition = GuardConditionImpl;
684684
*/
685685
private class GuardConditionFromBinaryLogicalOperator extends GuardConditionImpl instanceof Cpp::BinaryLogicalOperation
686686
{
687+
GuardConditionImpl l;
688+
GuardConditionImpl r;
689+
690+
GuardConditionFromBinaryLogicalOperator() {
691+
super.getLeftOperand() = l and
692+
super.getRightOperand() = r
693+
}
694+
687695
override predicate valueControls(Cpp::BasicBlock controlled, GuardValue v) {
688-
exists(Cpp::BinaryLogicalOperation binop, GuardCondition lhs, GuardCondition rhs |
689-
this = binop and
690-
lhs = binop.getLeftOperand() and
691-
rhs = binop.getRightOperand() and
692-
lhs.valueControls(controlled, v) and
693-
rhs.valueControls(controlled, v)
694-
)
696+
// `l || r` does not control `r` even though `l` does.
697+
not r.(Cpp::Expr).getBasicBlock() = controlled and
698+
l.valueControls(controlled, v)
699+
or
700+
r.valueControls(controlled, v)
695701
}
696702

697703
override predicate valueControlsEdge(Cpp::BasicBlock pred, Cpp::BasicBlock succ, GuardValue v) {
698-
exists(Cpp::BinaryLogicalOperation binop, GuardCondition lhs, GuardCondition rhs |
699-
this = binop and
700-
lhs = binop.getLeftOperand() and
701-
rhs = binop.getRightOperand() and
702-
lhs.valueControlsEdge(pred, succ, v) and
703-
rhs.valueControlsEdge(pred, succ, v)
704-
)
704+
l.valueControlsEdge(pred, succ, v)
705+
or
706+
r.valueControlsEdge(pred, succ, v)
705707
}
706708

707709
pragma[nomagic]

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,9 @@ astGuardsControl
384384
| test.c:126:7:126:7 | 1 | true | 131 | 132 |
385385
| test.c:126:7:126:7 | 1 | true | 134 | 123 |
386386
| test.c:126:7:126:28 | ... && ... | true | 126 | 128 |
387+
| test.c:126:7:126:28 | ... && ... | true | 131 | 131 |
387388
| test.c:126:7:126:28 | ... && ... | true | 131 | 132 |
389+
| test.c:126:7:126:28 | ... && ... | true | 134 | 123 |
388390
| test.c:126:12:126:26 | call to test3_condition | true | 126 | 128 |
389391
| test.c:126:12:126:26 | call to test3_condition | true | 131 | 132 |
390392
| test.c:131:7:131:7 | b | true | 131 | 132 |
@@ -787,13 +789,21 @@ astGuardsEnsure_const
787789
| test.c:126:7:126:7 | 1 | test.c:126:7:126:7 | 1 | == | 1 | 131 | 132 |
788790
| test.c:126:7:126:7 | 1 | test.c:126:7:126:7 | 1 | == | 1 | 134 | 123 |
789791
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | != | 0 | 126 | 128 |
792+
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | != | 0 | 131 | 131 |
790793
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | != | 0 | 131 | 132 |
794+
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | != | 0 | 134 | 123 |
791795
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | == | 1 | 126 | 128 |
796+
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | == | 1 | 131 | 131 |
792797
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | == | 1 | 131 | 132 |
798+
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | == | 1 | 134 | 123 |
793799
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | != | 0 | 126 | 128 |
800+
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | != | 0 | 131 | 131 |
794801
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | != | 0 | 131 | 132 |
802+
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | != | 0 | 134 | 123 |
795803
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | == | 1 | 126 | 128 |
804+
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | == | 1 | 131 | 131 |
796805
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | == | 1 | 131 | 132 |
806+
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | == | 1 | 134 | 123 |
797807
| test.c:126:12:126:26 | call to test3_condition | test.c:126:12:126:26 | call to test3_condition | != | 0 | 126 | 128 |
798808
| test.c:126:12:126:26 | call to test3_condition | test.c:126:12:126:26 | call to test3_condition | != | 0 | 131 | 132 |
799809
| test.c:126:12:126:26 | call to test3_condition | test.c:126:12:126:26 | call to test3_condition | == | 1 | 126 | 128 |

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,13 @@
105105
| test.c:126:7:126:7 | 1 | true | test.c:131:10:132:16 | { ... } |
106106
| test.c:126:7:126:7 | 1 | true | test.c:134:1:123:10 | return ... |
107107
| test.c:126:7:126:28 | ... && ... | not 0 | test.c:126:31:128:16 | { ... } |
108+
| test.c:126:7:126:28 | ... && ... | not 0 | test.c:131:3:131:7 | if (...) ... |
108109
| test.c:126:7:126:28 | ... && ... | not 0 | test.c:131:10:132:16 | { ... } |
110+
| test.c:126:7:126:28 | ... && ... | not 0 | test.c:134:1:123:10 | return ... |
109111
| test.c:126:7:126:28 | ... && ... | true | test.c:126:31:128:16 | { ... } |
112+
| test.c:126:7:126:28 | ... && ... | true | test.c:131:3:131:7 | if (...) ... |
110113
| test.c:126:7:126:28 | ... && ... | true | test.c:131:10:132:16 | { ... } |
114+
| test.c:126:7:126:28 | ... && ... | true | test.c:134:1:123:10 | return ... |
111115
| test.c:126:12:126:26 | call to test3_condition | not 0 | test.c:126:31:128:16 | { ... } |
112116
| test.c:126:12:126:26 | call to test3_condition | not 0 | test.c:131:10:132:16 | { ... } |
113117
| test.c:126:12:126:26 | call to test3_condition | true | test.c:126:31:128:16 | { ... } |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,13 +758,21 @@ unary
758758
| test.c:126:7:126:7 | 1 | test.c:126:7:126:7 | 1 | == | 1 | test.c:131:10:132:16 | { ... } |
759759
| test.c:126:7:126:7 | 1 | test.c:126:7:126:7 | 1 | == | 1 | test.c:134:1:123:10 | return ... |
760760
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | != | 0 | test.c:126:31:128:16 | { ... } |
761+
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | != | 0 | test.c:131:3:131:7 | if (...) ... |
761762
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | != | 0 | test.c:131:10:132:16 | { ... } |
763+
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | != | 0 | test.c:134:1:123:10 | return ... |
762764
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | == | 1 | test.c:126:31:128:16 | { ... } |
765+
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | == | 1 | test.c:131:3:131:7 | if (...) ... |
763766
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | == | 1 | test.c:131:10:132:16 | { ... } |
767+
| test.c:126:7:126:28 | ... && ... | test.c:126:7:126:7 | 1 | == | 1 | test.c:134:1:123:10 | return ... |
764768
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | != | 0 | test.c:126:31:128:16 | { ... } |
769+
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | != | 0 | test.c:131:3:131:7 | if (...) ... |
765770
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | != | 0 | test.c:131:10:132:16 | { ... } |
771+
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | != | 0 | test.c:134:1:123:10 | return ... |
766772
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | == | 1 | test.c:126:31:128:16 | { ... } |
773+
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | == | 1 | test.c:131:3:131:7 | if (...) ... |
767774
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | == | 1 | test.c:131:10:132:16 | { ... } |
775+
| test.c:126:7:126:28 | ... && ... | test.c:126:12:126:26 | call to test3_condition | == | 1 | test.c:134:1:123:10 | return ... |
768776
| test.c:126:12:126:26 | call to test3_condition | test.c:126:12:126:26 | call to test3_condition | != | 0 | test.c:126:31:128:16 | { ... } |
769777
| test.c:126:12:126:26 | call to test3_condition | test.c:126:12:126:26 | call to test3_condition | != | 0 | test.c:131:10:132:16 | { ... } |
770778
| test.c:126:12:126:26 | call to test3_condition | test.c:126:12:126:26 | call to test3_condition | == | 1 | test.c:126:31:128:16 | { ... } |

0 commit comments

Comments
 (0)