From 5abaea37211426f88a7e846b62ae4b32afd6e920 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 2 Oct 2024 15:02:26 +0200 Subject: [PATCH] Rust: Simplify break/continue CFG labels --- .../rust/controlflow/ControlFlowGraph.qll | 4 +- .../rust/controlflow/internal/Completion.qll | 52 +++++-------------- .../internal/ControlFlowGraphImpl.qll | 9 ++-- .../controlflow/internal/SuccessorType.qll | 32 ++++-------- .../library-tests/controlflow/Cfg.expected | 22 ++++---- 5 files changed, 42 insertions(+), 77 deletions(-) diff --git a/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll b/rust/ql/lib/codeql/rust/controlflow/ControlFlowGraph.qll index 18159ccea748..20f1af4a4afb 100644 --- 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 index 4034db5fe288..15619f06a2fe 100644 --- 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 index ea181764cefd..17add12b9404 100644 --- 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 index 6bdd54c263dd..ba76888bd303 100644 --- 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 index e2f3464800a0..39fc6b96b621 100644 --- 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 | |