Skip to content

Commit c803fbc

Browse files
committed
Cfg: Support short-circuiting compound assignments.
1 parent 64e3dfd commit c803fbc

File tree

2 files changed

+56
-7
lines changed

2 files changed

+56
-7
lines changed

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,24 @@ private module Ast implements AstSig<Location> {
185185

186186
class LogicalNotExpr = LogNotExpr;
187187

188+
class Assignment = J::Assignment;
189+
190+
class AssignExpr = J::AssignExpr;
191+
192+
class CompoundAssignment = J::AssignOp;
193+
194+
class AssignLogicalAndExpr extends CompoundAssignment {
195+
AssignLogicalAndExpr() { none() }
196+
}
197+
198+
class AssignLogicalOrExpr extends CompoundAssignment {
199+
AssignLogicalOrExpr() { none() }
200+
}
201+
202+
class AssignNullCoalescingExpr extends CompoundAssignment {
203+
AssignNullCoalescingExpr() { none() }
204+
}
205+
188206
final private class FinalBooleanLiteral = J::BooleanLiteral;
189207

190208
class BooleanLiteral extends FinalBooleanLiteral {

shared/controlflow/codeql/controlflow/ControlFlowGraph.qll

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,33 @@ signature module AstSig<LocationSig Location> {
309309
/** A logical NOT expression. */
310310
class LogicalNotExpr extends UnaryExpr;
311311

312+
/**
313+
* An assignment expression, either compound or simple.
314+
*
315+
* Examples:
316+
*
317+
* ```
318+
* x = y
319+
* sum += element
320+
* ```
321+
*/
322+
class Assignment extends BinaryExpr;
323+
324+
/** A simple assignment expression, for example `x = y`. */
325+
class AssignExpr extends Assignment;
326+
327+
/** A compound assignment expression, for example `x += y` or `x ??= y`. */
328+
class CompoundAssignment extends Assignment;
329+
330+
/** A short-circuiting logical AND compound assignment expression. */
331+
class AssignLogicalAndExpr extends CompoundAssignment;
332+
333+
/** A short-circuiting logical OR compound assignment expression. */
334+
class AssignLogicalOrExpr extends CompoundAssignment;
335+
336+
/** A short-circuiting null-coalescing compound assignment expression. */
337+
class AssignNullCoalescingExpr extends CompoundAssignment;
338+
312339
/** A boolean literal expression. */
313340
class BooleanLiteral extends Expr {
314341
/** Gets the boolean value of this literal. */
@@ -458,11 +485,14 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
458485
* is the value that causes the short-circuit.
459486
*/
460487
private predicate shortCircuiting(BinaryExpr expr, ConditionalSuccessor shortcircuitValue) {
461-
expr instanceof LogicalAndExpr and shortcircuitValue.(BooleanSuccessor).getValue() = false
488+
(expr instanceof LogicalAndExpr or expr instanceof AssignLogicalAndExpr) and
489+
shortcircuitValue.(BooleanSuccessor).getValue() = false
462490
or
463-
expr instanceof LogicalOrExpr and shortcircuitValue.(BooleanSuccessor).getValue() = true
491+
(expr instanceof LogicalOrExpr or expr instanceof AssignLogicalOrExpr) and
492+
shortcircuitValue.(BooleanSuccessor).getValue() = true
464493
or
465-
expr instanceof NullCoalescingExpr and shortcircuitValue.(NullnessSuccessor).getValue() = true
494+
(expr instanceof NullCoalescingExpr or expr instanceof AssignNullCoalescingExpr) and
495+
shortcircuitValue.(NullnessSuccessor).getValue() = false
466496
}
467497

468498
/**
@@ -472,9 +502,10 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
472502
private predicate propagatesValue(AstNode child, AstNode parent) {
473503
Input1::propagatesValue(child, parent)
474504
or
475-
// For now, the `not postOrInOrder(parent)` is superfluous, as we don't
476-
// have any short-circuiting post-order expressions yet, but this will
477-
// change once we add support for e.g. C#'s `??=`.
505+
// Short-circuiting post-order expressions, i.e. short-circuiting
506+
// compound assignments, e.g. C#'s `??=`, cannot propagate the value of
507+
// the right-hand side to the parent, as the assignment must take place
508+
// in-between, so propagating the value would imply splitting.
478509
shortCircuiting(parent, _) and
479510
not postOrInOrder(parent) and
480511
parent.(BinaryExpr).getRightOperand() = child
@@ -1243,7 +1274,7 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
12431274
n1.isAfterValue(binexpr.getLeftOperand(), shortcircuitValue) and
12441275
n2.isAfterValue(binexpr, shortcircuitValue)
12451276
or
1246-
// short-circuiting operations with side-effects (e.g. `x &&= y`, `x?.Prop = y`) are in post-order:
1277+
// short-circuiting operations with side-effects (e.g. `x &&= y`) are in post-order:
12471278
n1.isAfter(binexpr.getRightOperand()) and
12481279
n2.isIn(binexpr)
12491280
or

0 commit comments

Comments
 (0)