Skip to content

Commit 1d4c819

Browse files
committed
Java: Fix assert CFG by properly tagging the false successor.
1 parent 4d2c678 commit 1d4c819

File tree

3 files changed

+32
-12
lines changed

3 files changed

+32
-12
lines changed

java/ql/lib/semmle/code/java/ControlFlowGraph.qll

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ module ControlFlow {
100100
private newtype TNode =
101101
TExprNode(Expr e) { hasControlFlow(e) } or
102102
TStmtNode(Stmt s) or
103-
TExitNode(Callable c) { exists(c.getBody()) }
103+
TExitNode(Callable c) { exists(c.getBody()) } or
104+
TAssertThrowNode(AssertStmt s)
104105

105106
/** A node in the expression-level control-flow graph. */
106107
class Node extends TNode {
@@ -204,6 +205,25 @@ module ControlFlow {
204205
/** Gets the source location for this element. */
205206
override Location getLocation() { result = c.getLocation() }
206207
}
208+
209+
/** A control flow node indicating a failing assertion. */
210+
class AssertThrowNode extends Node, TAssertThrowNode {
211+
AssertStmt s;
212+
213+
AssertThrowNode() { this = TAssertThrowNode(s) }
214+
215+
override Stmt getEnclosingStmt() { result = s }
216+
217+
override Callable getEnclosingCallable() { result = s.getEnclosingCallable() }
218+
219+
override ExprParent getAstNode() { result = s }
220+
221+
/** Gets a textual representation of this element. */
222+
override string toString() { result = "Assert Throw" }
223+
224+
/** Gets the source location for this element. */
225+
override Location getLocation() { result = s.getLocation() }
226+
}
207227
}
208228

209229
class ControlFlowNode = ControlFlow::Node;
@@ -1123,12 +1143,7 @@ private module ControlFlowGraphImpl {
11231143
or
11241144
// `assert` statements may throw
11251145
completion = ThrowCompletion(assertionError()) and
1126-
(
1127-
last(assertstmt.getMessage(), last, NormalCompletion())
1128-
or
1129-
not exists(assertstmt.getMessage()) and
1130-
last(assertstmt.getExpr(), last, BooleanCompletion(false, _))
1131-
)
1146+
last.(AssertThrowNode).getAstNode() = assertstmt
11321147
)
11331148
or
11341149
// `throw` statements or throwing calls give rise to `Throw` completion
@@ -1547,7 +1562,15 @@ private module ControlFlowGraphImpl {
15471562
or
15481563
last(assertstmt.getExpr(), n, completion) and
15491564
completion = BooleanCompletion(false, _) and
1550-
result = first(assertstmt.getMessage())
1565+
(
1566+
result = first(assertstmt.getMessage())
1567+
or
1568+
not exists(assertstmt.getMessage()) and
1569+
result.(AssertThrowNode).getAstNode() = assertstmt
1570+
)
1571+
or
1572+
last(assertstmt.getMessage(), n, NormalCompletion()) and
1573+
result.(AssertThrowNode).getAstNode() = assertstmt
15511574
)
15521575
or
15531576
// When expressions:

java/ql/test/query-tests/Nullness/C.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ public void ex18(boolean b, int[] xs, Object related) {
251251
(b && xs == null && related == null);
252252
if (b) {
253253
if (related == null) { return; }
254-
xs[0] = 42; // FP - correlated conditions fails to recognize assert edges
254+
xs[0] = 42; // OK
255255
}
256256
}
257257
}

java/ql/test/query-tests/Nullness/NullMaybe.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@
3232
| C.java:207:9:207:11 | obj | Variable $@ may be null at this access because of $@ assignment. | C.java:201:5:201:22 | Object obj | obj | C.java:201:12:201:21 | obj | this |
3333
| C.java:219:9:219:10 | o1 | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:212:20:212:28 | o1 | o1 | C.java:213:9:213:18 | ... == ... | this |
3434
| C.java:233:7:233:8 | xs | Variable $@ may be null at this access because of $@ assignment. | C.java:231:5:231:56 | int[] xs | xs | C.java:231:11:231:55 | xs | this |
35-
| C.java:254:7:254:8 | xs | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:248:31:248:38 | xs | xs | C.java:249:19:249:28 | ... == ... | this |
36-
| C.java:254:7:254:8 | xs | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:248:31:248:38 | xs | xs | C.java:250:18:250:27 | ... != ... | this |
37-
| C.java:254:7:254:8 | xs | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:248:31:248:38 | xs | xs | C.java:251:18:251:27 | ... == ... | this |
3835
| F.java:11:5:11:7 | obj | Variable $@ may be null at this access as suggested by $@ null guard. | F.java:8:18:8:27 | obj | obj | F.java:9:9:9:19 | ... == ... | this |
3936
| F.java:17:5:17:7 | obj | Variable $@ may be null at this access as suggested by $@ null guard. | F.java:14:18:14:27 | obj | obj | F.java:15:9:15:19 | ... == ... | this |
4037
| G.java:20:12:20:12 | s | Variable $@ may be null at this access as suggested by $@ null guard. | G.java:3:27:3:34 | s | s | G.java:5:9:5:17 | ... == ... | this |

0 commit comments

Comments
 (0)