Skip to content

Commit 8838104

Browse files
authored
Merge pull request #19733 from aschackmull/java/assert-cfg
Java: Update the CFG for assert statements to make them proper guards.
2 parents e04dea1 + 6131c68 commit 8838104

File tree

4 files changed

+55
-17
lines changed

4 files changed

+55
-17
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Java `assert` statements are now assumed to be executed for the purpose of analysing control flow. This improves precision for a number of queries.

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

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,18 @@ private module ControlFlowGraphImpl {
327327
)
328328
}
329329

330+
private ThrowableType assertionError() { result.hasQualifiedName("java.lang", "AssertionError") }
331+
330332
/**
331333
* Gets an exception type that may be thrown during execution of the
332334
* body or the resources (if any) of `try`.
333335
*/
334336
private ThrowableType thrownInBody(TryStmt try) {
335-
exists(AstNode n | mayThrow(n, result) |
337+
exists(AstNode n |
338+
mayThrow(n, result)
339+
or
340+
n instanceof AssertStmt and result = assertionError()
341+
|
336342
n.getEnclosingStmt().getEnclosingStmt+() = try.getBlock() or
337343
n.(Expr).getParent*() = try.getAResource()
338344
)
@@ -394,10 +400,7 @@ private module ControlFlowGraphImpl {
394400
exists(LogicExpr logexpr |
395401
logexpr.(BinaryExpr).getLeftOperand() = b
396402
or
397-
// Cannot use LogicExpr.getAnOperand or BinaryExpr.getAnOperand as they remove parentheses.
398-
logexpr.(BinaryExpr).getRightOperand() = b and inBooleanContext(logexpr)
399-
or
400-
logexpr.(UnaryExpr).getExpr() = b and inBooleanContext(logexpr)
403+
logexpr.getAnOperand() = b and inBooleanContext(logexpr)
401404
)
402405
or
403406
exists(ConditionalExpr condexpr |
@@ -407,6 +410,8 @@ private module ControlFlowGraphImpl {
407410
inBooleanContext(condexpr)
408411
)
409412
or
413+
exists(AssertStmt assertstmt | assertstmt.getExpr() = b)
414+
or
410415
exists(SwitchExpr switch |
411416
inBooleanContext(switch) and
412417
switch.getAResult() = b
@@ -672,8 +677,6 @@ private module ControlFlowGraphImpl {
672677
this instanceof EmptyStmt
673678
or
674679
this instanceof LocalTypeDeclStmt
675-
or
676-
this instanceof AssertStmt
677680
}
678681

679682
/** Gets child nodes in their order of execution. Indexing starts at either -1 or 0. */
@@ -744,8 +747,6 @@ private module ControlFlowGraphImpl {
744747
or
745748
index = 0 and result = this.(ThrowStmt).getExpr()
746749
or
747-
index = 0 and result = this.(AssertStmt).getExpr()
748-
or
749750
result = this.(RecordPatternExpr).getSubPattern(index)
750751
}
751752

@@ -807,9 +808,12 @@ private module ControlFlowGraphImpl {
807808
or
808809
result = first(n.(SynchronizedStmt).getExpr())
809810
or
811+
result = first(n.(AssertStmt).getExpr())
812+
or
810813
result.asStmt() = n and
811814
not n instanceof PostOrderNode and
812-
not n instanceof SynchronizedStmt
815+
not n instanceof SynchronizedStmt and
816+
not n instanceof AssertStmt
813817
or
814818
result.asExpr() = n and n instanceof SwitchExpr
815819
}
@@ -1112,7 +1116,22 @@ private module ControlFlowGraphImpl {
11121116
// `return` statements give rise to a `Return` completion
11131117
last.asStmt() = n.(ReturnStmt) and completion = ReturnCompletion()
11141118
or
1115-
// `throw` statements or throwing calls give rise to ` Throw` completion
1119+
exists(AssertStmt assertstmt | assertstmt = n |
1120+
// `assert` statements may complete normally - we use the `AssertStmt` itself
1121+
// to represent this outcome
1122+
last.asStmt() = assertstmt and completion = NormalCompletion()
1123+
or
1124+
// `assert` statements may throw
1125+
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+
)
1132+
)
1133+
or
1134+
// `throw` statements or throwing calls give rise to `Throw` completion
11161135
exists(ThrowableType tt | mayThrow(n, tt) |
11171136
last = n.getCfgNode() and completion = ThrowCompletion(tt)
11181137
)
@@ -1520,6 +1539,17 @@ private module ControlFlowGraphImpl {
15201539
exists(int i | last(s.getVariable(i), n, completion) and result = first(s.getVariable(i + 1)))
15211540
)
15221541
or
1542+
// Assert statements:
1543+
exists(AssertStmt assertstmt |
1544+
last(assertstmt.getExpr(), n, completion) and
1545+
completion = BooleanCompletion(true, _) and
1546+
result.asStmt() = assertstmt
1547+
or
1548+
last(assertstmt.getExpr(), n, completion) and
1549+
completion = BooleanCompletion(false, _) and
1550+
result = first(assertstmt.getMessage())
1551+
)
1552+
or
15231553
// When expressions:
15241554
exists(WhenExpr whenexpr |
15251555
n.asExpr() = whenexpr and

java/ql/lib/semmle/code/java/dataflow/Nullness.qll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,6 @@ private ControlFlowNode varDereference(SsaVariable v, VarAccess va) {
141141
private ControlFlowNode ensureNotNull(SsaVariable v) {
142142
result = varDereference(v, _)
143143
or
144-
result.asStmt().(AssertStmt).getExpr() = nullGuard(v, true, false)
145-
or
146144
exists(AssertTrueMethod m | result.asCall() = m.getACheck(nullGuard(v, true, false)))
147145
or
148146
exists(AssertFalseMethod m | result.asCall() = m.getACheck(nullGuard(v, false, false)))

java/ql/lib/semmle/code/java/frameworks/Assertions.qll

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,17 @@ predicate assertFail(BasicBlock bb, ControlFlowNode n) {
110110
(
111111
exists(AssertTrueMethod m |
112112
n.asExpr() = m.getACheck(any(BooleanLiteral b | b.getBooleanValue() = false))
113-
) or
113+
)
114+
or
114115
exists(AssertFalseMethod m |
115116
n.asExpr() = m.getACheck(any(BooleanLiteral b | b.getBooleanValue() = true))
116-
) or
117-
exists(AssertFailMethod m | n.asExpr() = m.getACheck()) or
118-
n.asStmt().(AssertStmt).getExpr().(BooleanLiteral).getBooleanValue() = false
117+
)
118+
or
119+
exists(AssertFailMethod m | n.asExpr() = m.getACheck())
120+
or
121+
exists(AssertStmt a |
122+
n.asExpr() = a.getExpr() and
123+
a.getExpr().(BooleanLiteral).getBooleanValue() = false
124+
)
119125
)
120126
}

0 commit comments

Comments
 (0)