Rust: Simplify break/continue CFG labels

github · Oct 2, 2024 · 5abaea3 · 5abaea3
1 parent 8183a11
commit 5abaea3
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 77 deletions.
diff --git a/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll b/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;
 

diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/Completion.qll b/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() }
 }

diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll
@@ -186,12 +186,9 @@ class ContinueExprTree extends LeafTree, ContinueExpr {
   override predicate last(AstNode last, Completion c) { none() }
 
   override predicate succ(AstNode pred, AstNode succ, Completion c) {
-    exists(Expr target |
-      pred = this and
-      c.isValidFor(pred) and
-      target = this.getTarget() and
-      first(target.(LoopingExprTree).getLoopContinue(), succ)
-    )
+    pred = this and
+    c.isValidFor(pred) and
+    first(this.getTarget().(LoopingExprTree).getLoopContinue(), succ)
   }
 }
 

diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/SuccessorType.qll b/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" }
 }
 
 /**

diff --git a/rust/ql/test/library-tests/controlflow/Cfg.expected b/rust/ql/test/library-tests/controlflow/Cfg.expected
@@ -77,9 +77,9 @@ edges
 | test.rs:31:24:33:17 | IfExpr | test.rs:29:17:33:17 | IfExpr |  |
 | test.rs:31:27:31:27 | b | test.rs:31:24:33:17 | IfExpr | false |
 | test.rs:31:27:31:27 | b | test.rs:32:21:32:33 | ExprStmt | true |
-| test.rs:32:21:32:32 | BreakExpr | test.rs:27:9:36:9 | LoopExpr | break('outer) |
+| test.rs:32:21:32:32 | BreakExpr | test.rs:27:9:36:9 | LoopExpr | break |
 | test.rs:32:21:32:33 | ExprStmt | test.rs:32:21:32:32 | BreakExpr |  |
-| test.rs:34:17:34:28 | BreakExpr | test.rs:28:13:35:13 | LoopExpr | break('inner) |
+| test.rs:34:17:34:28 | BreakExpr | test.rs:28:13:35:13 | LoopExpr | break |
 | test.rs:34:17:34:29 | ExprStmt | test.rs:34:17:34:28 | BreakExpr |  |
 | test.rs:37:9:37:12 | true | test.rs:26:48:38:5 | BlockExpr |  |
 | test.rs:40:5:52:5 | enter test_continue_with_labels | test.rs:42:13:42:14 | ExprStmt |  |
@@ -94,9 +94,9 @@ edges
 | test.rs:46:24:48:17 | IfExpr | test.rs:44:17:48:17 | IfExpr |  |
 | test.rs:46:27:46:27 | b | test.rs:46:24:48:17 | IfExpr | false |
 | test.rs:46:27:46:27 | b | test.rs:47:21:47:36 | ExprStmt | true |
-| test.rs:47:21:47:35 | ContinueExpr | test.rs:42:13:42:14 | ExprStmt | continue('outer) |
+| test.rs:47:21:47:35 | ContinueExpr | test.rs:42:13:42:14 | ExprStmt | continue |
 | test.rs:47:21:47:36 | ExprStmt | test.rs:47:21:47:35 | ContinueExpr |  |
-| test.rs:49:17:49:31 | ContinueExpr | test.rs:44:17:48:17 | ExprStmt | continue('inner) |
+| test.rs:49:17:49:31 | ContinueExpr | test.rs:44:17:48:17 | ExprStmt | continue |
 | test.rs:49:17:49:32 | ExprStmt | test.rs:49:17:49:31 | ContinueExpr |  |
 | test.rs:54:5:66:5 | enter test_loop_label_shadowing | test.rs:56:13:56:14 | ExprStmt |  |
 | test.rs:56:13:56:13 | 1 | test.rs:58:17:62:17 | ExprStmt |  |
@@ -110,9 +110,9 @@ edges
 | test.rs:60:24:62:17 | IfExpr | test.rs:58:17:62:17 | IfExpr |  |
 | test.rs:60:27:60:27 | b | test.rs:60:24:62:17 | IfExpr | false |
 | test.rs:60:27:60:27 | b | test.rs:61:21:61:35 | ExprStmt | true |
-| test.rs:61:21:61:34 | ContinueExpr | test.rs:58:17:62:17 | ExprStmt | continue('loop) |
+| test.rs:61:21:61:34 | ContinueExpr | test.rs:58:17:62:17 | ExprStmt | continue |
 | test.rs:61:21:61:35 | ExprStmt | test.rs:61:21:61:34 | ContinueExpr |  |
-| test.rs:63:17:63:30 | ContinueExpr | test.rs:58:17:62:17 | ExprStmt | continue('loop) |
+| test.rs:63:17:63:30 | ContinueExpr | test.rs:58:17:62:17 | ExprStmt | continue |
 | test.rs:63:17:63:31 | ExprStmt | test.rs:63:17:63:30 | ContinueExpr |  |
 | test.rs:68:5:77:5 | enter test_while | test.rs:69:9:69:25 | LetStmt |  |
 | test.rs:68:5:77:5 | exit test_while (normal) | test.rs:68:5:77:5 | exit test_while |  |
@@ -338,7 +338,7 @@ edges
 | test.rs:182:16:182:20 | ... > ... | test.rs:182:13:184:13 | IfExpr | false |
 | test.rs:182:16:182:20 | ... > ... | test.rs:183:17:183:36 | ExprStmt | true |
 | test.rs:182:20:182:20 | 0 | test.rs:182:16:182:20 | ... > ... |  |
-| test.rs:183:17:183:35 | BreakExpr | test.rs:181:13:186:9 | LoopExpr | break('label) |
+| test.rs:183:17:183:35 | BreakExpr | test.rs:181:13:186:9 | LoopExpr | break |
 | test.rs:183:17:183:36 | ExprStmt | test.rs:183:30:183:30 | a |  |
 | test.rs:183:30:183:30 | a | test.rs:183:34:183:35 | 10 |  |
 | test.rs:183:30:183:35 | ... > ... | test.rs:183:17:183:35 | BreakExpr |  |
@@ -357,7 +357,7 @@ edges
 | test.rs:194:9:200:9 | IfExpr | test.rs:193:43:201:5 | BlockExpr |  |
 | test.rs:194:13:196:9 | BlockExpr | test.rs:197:13:197:13 | 1 | true |
 | test.rs:194:13:196:9 | BlockExpr | test.rs:199:13:199:13 | 0 | false |
-| test.rs:195:13:195:30 | BreakExpr | test.rs:194:13:196:9 | BlockExpr | break('block) |
+| test.rs:195:13:195:30 | BreakExpr | test.rs:194:13:196:9 | BlockExpr | break |
 | test.rs:195:13:195:31 | ExprStmt | test.rs:195:26:195:26 | a |  |
 | test.rs:195:26:195:26 | a | test.rs:195:30:195:30 | 0 |  |
 | test.rs:195:26:195:30 | ... > ... | test.rs:195:13:195:30 | BreakExpr |  |
@@ -511,7 +511,7 @@ edges
 | test.rs:283:12:283:28 | PathExpr | test.rs:283:12:283:30 | CallExpr |  |
 | test.rs:283:12:283:30 | CallExpr | test.rs:283:9:285:9 | IfExpr | false |
 | test.rs:283:12:283:30 | CallExpr | test.rs:284:13:284:27 | ExprStmt | true |
-| test.rs:284:13:284:26 | BreakExpr | test.rs:281:18:292:5 | BlockExpr | break('block) |
+| test.rs:284:13:284:26 | BreakExpr | test.rs:281:18:292:5 | BlockExpr | break |
 | test.rs:284:13:284:27 | ExprStmt | test.rs:284:26:284:26 | 1 |  |
 | test.rs:284:26:284:26 | 1 | test.rs:284:13:284:26 | BreakExpr |  |
 | test.rs:286:9:286:21 | PathExpr | test.rs:286:9:286:23 | CallExpr |  |
@@ -522,7 +522,7 @@ edges
 | test.rs:287:12:287:28 | PathExpr | test.rs:287:12:287:30 | CallExpr |  |
 | test.rs:287:12:287:30 | CallExpr | test.rs:287:9:289:9 | IfExpr | false |
 | test.rs:287:12:287:30 | CallExpr | test.rs:288:13:288:27 | ExprStmt | true |
-| test.rs:288:13:288:26 | BreakExpr | test.rs:281:18:292:5 | BlockExpr | break('block) |
+| test.rs:288:13:288:26 | BreakExpr | test.rs:281:18:292:5 | BlockExpr | break |
 | test.rs:288:13:288:27 | ExprStmt | test.rs:288:26:288:26 | 2 |  |
 | test.rs:288:26:288:26 | 2 | test.rs:288:13:288:26 | BreakExpr |  |
 | test.rs:290:9:290:21 | PathExpr | test.rs:290:9:290:23 | CallExpr |  |
@@ -542,7 +542,7 @@ edges
 | test.rs:298:13:298:19 | TupleStructPat | test.rs:299:13:299:27 | ExprStmt | no-match |
 | test.rs:298:13:298:19 | TupleStructPat | test.rs:301:9:301:9 | x | match |
 | test.rs:298:23:298:23 | x | test.rs:298:13:298:19 | TupleStructPat |  |
-| test.rs:299:13:299:26 | BreakExpr | test.rs:296:18:302:5 | BlockExpr | break('block) |
+| test.rs:299:13:299:26 | BreakExpr | test.rs:296:18:302:5 | BlockExpr | break |
 | test.rs:299:13:299:27 | ExprStmt | test.rs:299:26:299:26 | 1 |  |
 | test.rs:299:26:299:26 | 1 | test.rs:299:13:299:26 | BreakExpr |  |
 | test.rs:301:9:301:9 | x | test.rs:296:18:302:5 | BlockExpr |  |