Merge pull request #17644 from hvitved/rust/break-continue-target

rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll

@@ -19,7 +19,9 @@ final class BooleanSuccessor = BooleanSuccessorImpl;
 
 final class MatchSuccessor = MatchSuccessorImpl;
 
-final class LoopJumpSuccessor = LoopJumpSuccessorImpl;
+final class BreakSuccessor = BreakSuccessorImpl;
+
+final class ContinueSuccessor = ContinueSuccessorImpl;
 
 final class ReturnSuccessor = ReturnSuccessorImpl;
 

rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll

@@ -7,13 +7,8 @@ newtype TCompletion =
   TSimpleCompletion() or
   TBooleanCompletion(Boolean b) or
   TMatchCompletion(Boolean isMatch) or
-  TLoopCompletion(TLoopJumpType kind, TLabelType label) {
-    label = TNoLabel()
-    or
-    kind = TBreakJump() and label = TLabel(any(BreakExpr b).getLifetime().getText())
-    or
-    kind = TContinueJump() and label = TLabel(any(ContinueExpr b).getLifetime().getText())
-  } or
+  TBreakCompletion() or
+  TContinueCompletion() or
   TReturnCompletion()
 
 /** A completion of a statement or an expression. */
@@ -148,42 +143,23 @@ class MatchCompletion extends TMatchCompletion, ConditionalCompletion {
 }
 
 /**
- * A completion that represents a break or a continue.
+ * A completion that represents a `break`.
  */
-class LoopJumpCompletion extends TLoopCompletion, Completion {
-  override LoopJumpSuccessor getAMatchingSuccessorType() {
-    result = TLoopSuccessor(this.getKind(), this.getLabelType())
-  }
-
-  final TLoopJumpType getKind() { this = TLoopCompletion(result, _) }
-
-  final TLabelType getLabelType() { this = TLoopCompletion(_, result) }
+class BreakCompletion extends TBreakCompletion, Completion {
+  override BreakSuccessor getAMatchingSuccessorType() { any() }
 
-  final predicate hasLabel() { this.getLabelType() = TLabel(_) }
+  override predicate isValidForSpecific(AstNode e) { e instanceof BreakExpr }
 
-  final string getLabelName() { TLabel(result) = this.getLabelType() }
-
-  final predicate isContinue() { this.getKind() = TContinueJump() }
+  override string toString() { result = this.getAMatchingSuccessorType().toString() }
+}
 
-  final predicate isBreak() { this.getKind() = TBreakJump() }
+/**
+ * A completion that represents a `continue`.
+ */
+class ContinueCompletion extends TContinueCompletion, Completion {
+  override ContinueSuccessor getAMatchingSuccessorType() { any() }
 
-  override predicate isValidForSpecific(AstNode e) {
-    this.isBreak() and
-    e instanceof BreakExpr and
-    (
-      not e.(BreakExpr).hasLifetime() and not this.hasLabel()
-      or
-      e.(BreakExpr).getLifetime().getText() = this.getLabelName()
-    )
-    or
-    this.isContinue() and
-    e instanceof ContinueExpr and
-    (
-      not e.(ContinueExpr).hasLifetime() and not this.hasLabel()
-      or
-      e.(ContinueExpr).getLifetime().getText() = this.getLabelName()
-    )
-  }
+  override predicate isValidForSpecific(AstNode e) { e instanceof ContinueExpr }
 
   override string toString() { result = this.getAMatchingSuccessorType().toString() }
 }

rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll

@@ -145,44 +145,26 @@ class BlockExprTree extends StandardPostOrderTree, BlockExpr {
     result = super.getStmtList().getTailExpr()
   }
 
-  override predicate propagatesAbnormal(AstNode child) { none() }
-
-  /** Holds if this block captures the break completion `c`. */
-  private predicate capturesBreakCompletion(LoopJumpCompletion c) {
-    c.isBreak() and
-    c.getLabelName() = this.getLabel().getLifetime().getText()
-  }
-
-  override predicate succ(AstNode pred, AstNode succ, Completion c) {
-    super.succ(pred, succ, c)
-    or
-    // Edge for exiting the block with a break expressions
-    last(this.getChildNode(_), pred, c) and
-    this.capturesBreakCompletion(c) and
-    succ = this
-  }
-
-  override predicate last(AstNode last, Completion c) {
-    super.last(last, c)
-    or
-    // Any abnormal completions that this block does not capture should propagate
-    last(this.getChildNode(_), last, c) and
-    not completionIsNormal(c) and
-    not this.capturesBreakCompletion(c)
-  }
+  override predicate propagatesAbnormal(AstNode child) { child = this.getChildNode(_) }
 }
 
-class BreakExprTree extends PostOrderTree instanceof BreakExpr {
-  override predicate propagatesAbnormal(AstNode child) { child = super.getExpr() }
+class BreakExprTree extends PostOrderTree, BreakExpr {
+  override predicate propagatesAbnormal(AstNode child) { child = this.getExpr() }
 
   override predicate first(AstNode node) {
-    first(super.getExpr(), node)
+    first(this.getExpr(), node)
     or
-    not super.hasExpr() and node = this
+    not this.hasExpr() and node = this
   }
 
+  override predicate last(AstNode last, Completion c) { none() }
+
   override predicate succ(AstNode pred, AstNode succ, Completion c) {
     last(super.getExpr(), pred, c) and succ = this
+    or
+    pred = this and
+    c.isValidFor(pred) and
+    succ = this.getTarget()
   }
 }
 
@@ -200,7 +182,15 @@ class CastExprTree extends StandardPostOrderTree instanceof CastExpr {
 
 class ClosureExprTree extends LeafTree instanceof ClosureExpr { }
 
-class ContinueExprTree extends LeafTree instanceof ContinueExpr { }
+class ContinueExprTree extends LeafTree, ContinueExpr {
+  override predicate last(AstNode last, Completion c) { none() }
+
+  override predicate succ(AstNode pred, AstNode succ, Completion c) {
+    pred = this and
+    c.isValidFor(pred) and
+    first(this.getTarget().(LoopingExprTree).getLoopContinue(), succ)
+  }
+}
 
 class ExprStmtTree extends StandardPreOrderTree instanceof ExprStmt {
   override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() }
@@ -310,57 +300,27 @@ class LetStmtTree extends PreOrderTree instanceof LetStmt {
 class LiteralExprTree extends LeafTree instanceof LiteralExpr { }
 
 abstract class LoopingExprTree extends PostOrderTree {
-  override predicate propagatesAbnormal(AstNode child) { none() }
+  override predicate propagatesAbnormal(AstNode child) { child = this.getLoopBody() }
 
   abstract BlockExpr getLoopBody();
 
-  abstract Label getLabel();
-
   /**
    * Gets the node to execute when continuing the loop; either after
    * executing the last node in the body or after an explicit `continue`.
    */
   abstract AstNode getLoopContinue();
 
-  /** Holds if this loop captures the `c` completion. */
-  private predicate capturesLoopJumpCompletion(LoopJumpCompletion c) {
-    not c.hasLabel()
-    or
-    c.getLabelName() = this.getLabel().getLifetime().getText()
-  }
-
   override predicate succ(AstNode pred, AstNode succ, Completion c) {
-    // Edge for exiting the loop with a break expressions
-    last(this.getLoopBody(), pred, c) and
-    c.(LoopJumpCompletion).isBreak() and
-    this.capturesLoopJumpCompletion(c) and
-    succ = this
-    or
     // Edge back to the start for final expression and continue expressions
     last(this.getLoopBody(), pred, c) and
-    (
-      completionIsNormal(c)
-      or
-      c.(LoopJumpCompletion).isContinue() and this.capturesLoopJumpCompletion(c)
-    ) and
+    completionIsNormal(c) and
     first(this.getLoopContinue(), succ)
   }
-
-  override predicate last(AstNode last, Completion c) {
-    super.last(last, c)
-    or
-    // Any abnormal completions that this loop does not capture should propagate
-    last(this.getLoopBody(), last, c) and
-    not completionIsNormal(c) and
-    not this.capturesLoopJumpCompletion(c)
-  }
 }
 
 class LoopExprTree extends LoopingExprTree instanceof LoopExpr {
   override BlockExpr getLoopBody() { result = LoopExpr.super.getLoopBody() }
 
-  override Label getLabel() { result = LoopExpr.super.getLabel() }
-
   override AstNode getLoopContinue() { result = this.getLoopBody() }
 
   override predicate first(AstNode node) { first(this.getLoopBody(), node) }
@@ -369,11 +329,13 @@ class LoopExprTree extends LoopingExprTree instanceof LoopExpr {
 class WhileExprTree extends LoopingExprTree instanceof WhileExpr {
   override BlockExpr getLoopBody() { result = WhileExpr.super.getLoopBody() }
 
-  override Label getLabel() { result = WhileExpr.super.getLabel() }
-
   override AstNode getLoopContinue() { result = super.getCondition() }
 
-  override predicate propagatesAbnormal(AstNode child) { child = super.getCondition() }
+  override predicate propagatesAbnormal(AstNode child) {
+    super.propagatesAbnormal(child)
+    or
+    child = super.getCondition()
+  }
 
   override predicate first(AstNode node) { first(super.getCondition(), node) }
 
@@ -399,11 +361,13 @@ class WhileExprTree extends LoopingExprTree instanceof WhileExpr {
 class ForExprTree extends LoopingExprTree instanceof ForExpr {
   override BlockExpr getLoopBody() { result = ForExpr.super.getLoopBody() }
 
-  override Label getLabel() { result = ForExpr.super.getLabel() }
-
   override AstNode getLoopContinue() { result = super.getPat() }
 
-  override predicate propagatesAbnormal(AstNode child) { child = super.getIterable() }
+  override predicate propagatesAbnormal(AstNode child) {
+    super.propagatesAbnormal(child)
+    or
+    child = super.getIterable()
+  }
 
   override predicate first(AstNode node) { first(super.getIterable(), node) }
 

rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll

@@ -15,7 +15,8 @@ newtype TSuccessorType =
   TSuccessorSuccessor() or
   TBooleanSuccessor(Boolean b) or
   TMatchSuccessor(Boolean b) or
-  TLoopSuccessor(TLoopJumpType kind, TLabelType label) { exists(TLoopCompletion(kind, label)) } or
+  TBreakSuccessor() or
+  TContinueSuccessor() or
   TReturnSuccessor()
 
 /** The type of a control flow successor. */
@@ -59,28 +60,17 @@ class MatchSuccessorImpl extends ConditionalSuccessorImpl, TMatchSuccessor {
 }
 
 /**
- * A control flow successor of a loop control flow expression, `continue` or `break`.
+ * A control flow successor of a `break` expression.
  */
-class LoopJumpSuccessorImpl extends SuccessorTypeImpl, TLoopSuccessor {
-  private TLoopJumpType getKind() { this = TLoopSuccessor(result, _) }
-
-  private TLabelType getLabelType() { this = TLoopSuccessor(_, result) }
-
-  predicate hasLabel() { this.getLabelType() = TLabel(_) }
-
-  string getLabelName() { this = TLoopSuccessor(_, TLabel(result)) }
-
-  predicate isContinue() { this.getKind() = TContinueJump() }
-
-  predicate isBreak() { this.getKind() = TBreakJump() }
+class BreakSuccessorImpl extends SuccessorTypeImpl, TBreakSuccessor {
+  override string toString() { result = "break" }
+}
 
-  override string toString() {
-    exists(string kind, string label |
-      (if this.isContinue() then kind = "continue" else kind = "break") and
-      (if this.hasLabel() then label = "(" + this.getLabelName() + ")" else label = "") and
-      result = kind + label
-    )
-  }
+/**
+ * A control flow successor of a `continue` expression.
+ */
+class ContinueSuccessorImpl extends SuccessorTypeImpl, TContinueSuccessor {
+  override string toString() { result = "continue" }
 }
 
 /**