Skip to content

Commit b33dc38

Browse files
smowtonsmowton
committed
Fix hasBranchEdge for switch exprs with an internal CFG and incoming edges from a passing case guard
1 parent 9b5b496 commit b33dc38

File tree

5 files changed

+88
-7
lines changed

5 files changed

+88
-7
lines changed

‎java/ql/lib/semmle/code/java/controlflow/Guards.qll

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,16 +175,23 @@ class Guard extends ExprParent {
175175
bb2 = cb.getTestSuccessor(branch)
176176
)
177177
or
178-
exists(SwitchCase sc |
178+
exists(SwitchCase sc, ControlFlowNode pred |
179179
sc = this and
180180
// Pattern cases are handled as ConditionBlocks above.
181181
not sc instanceof PatternCase and
182182
branch = true and
183183
bb2.getFirstNode() = sc.getControlFlowNode() and
184-
bb1 = sc.getControlFlowNode().getAPredecessor().getBasicBlock() and
184+
pred = sc.getControlFlowNode().getAPredecessor() and
185185
// This is either the top of the switch block, or a preceding pattern case
186-
// if one exists.
187-
this.getBasicBlock() = bb1
186+
// if one exists (in which case the edge might come from the case itself or its guard)
187+
(
188+
pred.(Expr).getParent*() = sc.getSelectorExpr()
189+
or
190+
pred.(Expr).getParent*() = getClosestPrecedingPatternCase(sc).getGuard()
191+
or
192+
pred = getClosestPrecedingPatternCase(sc)
193+
) and
194+
bb1 = pred.getBasicBlock()
188195
)
189196
or
190197
preconditionBranchEdge(this, bb1, bb2, branch)

‎java/ql/test/library-tests/guards12/PrintAst.expected

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ Test.java:
66
#-----| 4: (Parameters)
77
# 2| 0: [Parameter] s
88
# 2| 0: [TypeAccess] String
9+
# 2| 1: [Parameter] unknown
10+
# 2| 0: [TypeAccess] boolean
911
# 2| 5: [BlockStmt] { ... }
1012
# 3| 0: [LocalVariableDeclStmt] var ...;
1113
# 3| 0: [TypeAccess] int
@@ -58,3 +60,25 @@ Test.java:
5860
# 18| 0: [StringLiteral] "e"
5961
# 19| 2: [DefaultCase] default
6062
# 19| -1: [BlockStmt] { ... }
63+
# 21| 4: [SwitchStmt] switch (...)
64+
# 21| -1: [ConditionalExpr] ...?...:...
65+
# 21| 0: [VarAccess] unknown
66+
# 21| 1: [VarAccess] s
67+
# 21| 2: [MethodCall] toLowerCase(...)
68+
# 21| -1: [VarAccess] s
69+
# 22| 0: [ConstCase] case ...
70+
# 22| -1: [BlockStmt] { ... }
71+
# 22| 0: [StringLiteral] "f"
72+
# 23| 1: [PatternCase] case <Pattern>
73+
# 23| -3: [EQExpr] ... == ...
74+
# 23| 0: [VarAccess] len
75+
# 23| 1: [IntegerLiteral] 4
76+
# 23| -1: [BlockStmt] { ... }
77+
#-----| 0: (Single Local Variable Declaration)
78+
# 23| 0: [TypeAccess] String
79+
# 23| 1: [LocalVariableDeclExpr] s2
80+
# 24| 2: [ConstCase] case ...
81+
# 24| -1: [BlockStmt] { ... }
82+
# 24| 0: [StringLiteral] "g"
83+
# 25| 3: [DefaultCase] default
84+
# 25| -1: [BlockStmt] { ... }

‎java/ql/test/library-tests/guards12/Test.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
class Test {
2-
void foo(String s) {
2+
void foo(String s, boolean unknown) {
33
int x = switch(s) {
44
case "a", "b" -> 1;
55
case "c" -> 2;
@@ -18,5 +18,11 @@ void foo(String s) {
1818
case "e" -> { }
1919
default -> { }
2020
}
21+
switch (unknown ? s : s.toLowerCase()) {
22+
case "f" -> { }
23+
case String s2 when len == 4 -> { }
24+
case "g" -> { }
25+
default -> { }
26+
}
2127
}
2228
}

‎java/ql/test/library-tests/guards12/guard.expected

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,37 @@
1+
hasBranchEdge
2+
| Test.java:4:7:4:22 | case ... | Test.java:2:39:27:3 | { ... } | Test.java:4:7:4:22 | case ... | true |
3+
| Test.java:5:7:5:17 | case ... | Test.java:2:39:27:3 | { ... } | Test.java:5:7:5:17 | case ... | true |
4+
| Test.java:6:7:6:17 | case ... | Test.java:2:39:27:3 | { ... } | Test.java:6:7:6:17 | case ... | true |
5+
| Test.java:7:7:7:16 | default | Test.java:2:39:27:3 | { ... } | Test.java:7:7:7:16 | default | true |
6+
| Test.java:10:7:10:22 | case ... | Test.java:3:9:3:21 | x | Test.java:10:7:10:22 | case ... | true |
7+
| Test.java:11:7:11:17 | case ... | Test.java:3:9:3:21 | x | Test.java:11:7:11:17 | case ... | true |
8+
| Test.java:12:7:12:17 | case ... | Test.java:3:9:3:21 | x | Test.java:12:7:12:17 | case ... | true |
9+
| Test.java:13:7:13:16 | default | Test.java:3:9:3:21 | x | Test.java:13:7:13:16 | default | true |
10+
| Test.java:17:7:17:37 | case <Pattern> | Test.java:15:5:15:25 | var ...; | Test.java:17:19:17:20 | s2 | true |
11+
| Test.java:17:7:17:37 | case <Pattern> | Test.java:15:5:15:25 | var ...; | Test.java:18:7:18:17 | case ... | false |
12+
| Test.java:17:7:17:37 | case <Pattern> | Test.java:15:5:15:25 | var ...; | Test.java:19:7:19:16 | default | false |
13+
| Test.java:17:27:17:34 | ... == ... | Test.java:17:19:17:20 | s2 | Test.java:17:39:17:41 | { ... } | true |
14+
| Test.java:17:27:17:34 | ... == ... | Test.java:17:19:17:20 | s2 | Test.java:18:7:18:17 | case ... | false |
15+
| Test.java:17:27:17:34 | ... == ... | Test.java:17:19:17:20 | s2 | Test.java:19:7:19:16 | default | false |
16+
| Test.java:18:7:18:17 | case ... | Test.java:15:5:15:25 | var ...; | Test.java:18:7:18:17 | case ... | true |
17+
| Test.java:18:7:18:17 | case ... | Test.java:17:19:17:20 | s2 | Test.java:18:7:18:17 | case ... | true |
18+
| Test.java:19:7:19:16 | default | Test.java:15:5:15:25 | var ...; | Test.java:19:7:19:16 | default | true |
19+
| Test.java:19:7:19:16 | default | Test.java:17:19:17:20 | s2 | Test.java:19:7:19:16 | default | true |
20+
| Test.java:21:13:21:19 | unknown | Test.java:21:5:21:42 | switch (...) | Test.java:21:23:21:23 | s | true |
21+
| Test.java:21:13:21:19 | unknown | Test.java:21:5:21:42 | switch (...) | Test.java:21:27:21:27 | s | false |
22+
| Test.java:22:7:22:17 | case ... | Test.java:21:23:21:23 | s | Test.java:22:7:22:17 | case ... | true |
23+
| Test.java:22:7:22:17 | case ... | Test.java:21:27:21:27 | s | Test.java:22:7:22:17 | case ... | true |
24+
| Test.java:23:7:23:37 | case <Pattern> | Test.java:23:7:23:37 | case <Pattern> | Test.java:23:19:23:20 | s2 | true |
25+
| Test.java:23:7:23:37 | case <Pattern> | Test.java:23:7:23:37 | case <Pattern> | Test.java:24:7:24:17 | case ... | false |
26+
| Test.java:23:7:23:37 | case <Pattern> | Test.java:23:7:23:37 | case <Pattern> | Test.java:25:7:25:16 | default | false |
27+
| Test.java:23:27:23:34 | ... == ... | Test.java:23:19:23:20 | s2 | Test.java:23:39:23:41 | { ... } | true |
28+
| Test.java:23:27:23:34 | ... == ... | Test.java:23:19:23:20 | s2 | Test.java:24:7:24:17 | case ... | false |
29+
| Test.java:23:27:23:34 | ... == ... | Test.java:23:19:23:20 | s2 | Test.java:25:7:25:16 | default | false |
30+
| Test.java:24:7:24:17 | case ... | Test.java:23:7:23:37 | case <Pattern> | Test.java:24:7:24:17 | case ... | true |
31+
| Test.java:24:7:24:17 | case ... | Test.java:23:19:23:20 | s2 | Test.java:24:7:24:17 | case ... | true |
32+
| Test.java:25:7:25:16 | default | Test.java:23:7:23:37 | case <Pattern> | Test.java:25:7:25:16 | default | true |
33+
| Test.java:25:7:25:16 | default | Test.java:23:19:23:20 | s2 | Test.java:25:7:25:16 | default | true |
34+
#select
135
| Test.java:5:7:5:17 | case ... | Test.java:3:20:3:20 | s | Test.java:5:12:5:14 | "c" | true | false | Test.java:7:7:7:16 | default |
236
| Test.java:5:7:5:17 | case ... | Test.java:3:20:3:20 | s | Test.java:5:12:5:14 | "c" | true | true | Test.java:5:7:5:17 | case ... |
337
| Test.java:6:7:6:17 | case ... | Test.java:3:20:3:20 | s | Test.java:6:12:6:14 | "d" | true | false | Test.java:7:7:7:16 | default |
@@ -9,3 +43,8 @@
943
| Test.java:17:27:17:34 | ... == ... | Test.java:17:27:17:29 | len | Test.java:17:34:17:34 | 4 | true | true | Test.java:17:39:17:41 | { ... } |
1044
| Test.java:18:7:18:17 | case ... | Test.java:16:13:16:13 | s | Test.java:18:12:18:14 | "e" | true | false | Test.java:19:7:19:16 | default |
1145
| Test.java:18:7:18:17 | case ... | Test.java:16:13:16:13 | s | Test.java:18:12:18:14 | "e" | true | true | Test.java:18:7:18:17 | case ... |
46+
| Test.java:22:7:22:17 | case ... | Test.java:21:13:21:41 | ...?...:... | Test.java:22:12:22:14 | "f" | true | false | Test.java:25:7:25:16 | default |
47+
| Test.java:22:7:22:17 | case ... | Test.java:21:13:21:41 | ...?...:... | Test.java:22:12:22:14 | "f" | true | true | Test.java:22:7:22:17 | case ... |
48+
| Test.java:23:27:23:34 | ... == ... | Test.java:23:27:23:29 | len | Test.java:23:34:23:34 | 4 | true | true | Test.java:23:39:23:41 | { ... } |
49+
| Test.java:24:7:24:17 | case ... | Test.java:21:13:21:41 | ...?...:... | Test.java:24:12:24:14 | "g" | true | false | Test.java:25:7:25:16 | default |
50+
| Test.java:24:7:24:17 | case ... | Test.java:21:13:21:41 | ...?...:... | Test.java:24:12:24:14 | "g" | true | true | Test.java:24:7:24:17 | case ... |

‎java/ql/test/library-tests/guards12/guard.ql

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
import java
22
import semmle.code.java.controlflow.Guards
33

4-
from Guard g, BasicBlock bb, boolean branch, VarAccess e1, Expr e2, boolean pol
4+
query predicate hasBranchEdge(Guard g, BasicBlock bb1, BasicBlock bb2, boolean branch) {
5+
g.hasBranchEdge(bb1, bb2, branch)
6+
}
7+
8+
from Guard g, BasicBlock bb, boolean branch, Expr e1, Expr e2, boolean pol
59
where
610
g.controls(bb, branch) and
7-
g.isEquality(e1, e2, pol)
11+
g.isEquality(e1, e2, pol) and
12+
not e1 instanceof Literal
813
select g, e1, e2, pol, branch, bb

0 commit comments

Comments
 (0)