Merge pull request #17557 from hvitved/rust/cfg-improvements

Rust: CFG improvements

rust/ql/.generated.list

@@ -287,7 +287,6 @@ lib/codeql/rust/elements/internal/MatchArmImpl.qll 065dff16fc70b51924eb4db57be12
 lib/codeql/rust/elements/internal/MatchArmListConstructor.qll 8bc5ac978fe1158ef70d0ac06bdad9e02aadd657decb64abcc4ea03f6715a87a 4604ab0e524d0de6e19c16711b713f2090c95a8708909816a2b046f1bd83fe24
 lib/codeql/rust/elements/internal/MatchArmListImpl.qll 896c6f1650e7ceb60d0b3d90e2b95fe7f8dc529203ddfec58edb063fa9b2871f a668fed1eb68806abfb021913786168d124de47b25da470e7b57f56bf8556891
 lib/codeql/rust/elements/internal/MatchExprConstructor.qll 0355ca543a0f9ad56697bc2e1e2511fa3f233bc1f6344d9e1c2369106901c696 78622807a1c4bff61b751c715639510146c7a713e0c4f63246e9a2cf302f4875
-lib/codeql/rust/elements/internal/MatchExprImpl.qll 2f933805bbe6f2676501c1c72ce1dbcffb0acff34883fa7b423b1a20eaf58afb 56f1c727f327717afc48c1dc505241d56c46e84094395ca32a33824cf64ebdce
 lib/codeql/rust/elements/internal/MatchGuardConstructor.qll d4cae02d2902fe8d3cb6b9c2796137863f41f55840f6623935a1c99df43f28d8 0c89f2ca71a2fd5a3f365291e784cb779e34ba0542d9285515e1856424cec60d
 lib/codeql/rust/elements/internal/MatchGuardImpl.qll 77453be572769507e6515e622e6c874a875464c2ade8bcd89ef447bdc4649062 86cdf08b0ac5ff9a865ab52eae535d8c4e7d341bc79d422e123af5b8f593ad22
 lib/codeql/rust/elements/internal/MetaConstructor.qll 49ab9aafdcab7785fc5fc9fb8f7c5bb0ae76cf85d0d259c4b3ac4b0eccbbeb56 bc11aef22661077e398b6ca75e3701fd8d0ac94a0e96dc556a6f6de4089d8b8c

rust/ql/.gitattributes

@@ -289,7 +289,6 @@
 /lib/codeql/rust/elements/internal/MatchArmListConstructor.qll linguist-generated
 /lib/codeql/rust/elements/internal/MatchArmListImpl.qll linguist-generated
 /lib/codeql/rust/elements/internal/MatchExprConstructor.qll linguist-generated
-/lib/codeql/rust/elements/internal/MatchExprImpl.qll linguist-generated
 /lib/codeql/rust/elements/internal/MatchGuardConstructor.qll linguist-generated
 /lib/codeql/rust/elements/internal/MatchGuardImpl.qll linguist-generated
 /lib/codeql/rust/elements/internal/MetaConstructor.qll linguist-generated

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

@@ -9,6 +9,20 @@ private import BasicBlocks
 
 final class CfgScope = Scope::CfgScope;
 
+final class SuccessorType = SuccessorTypeImpl;
+
+final class NormalSuccessor = NormalSuccessorImpl;
+
+final class ConditionalSuccessor = ConditionalSuccessorImpl;
+
+final class BooleanSuccessor = BooleanSuccessorImpl;
+
+final class MatchSuccessor = MatchSuccessorImpl;
+
+final class LoopJumpSuccessor = LoopJumpSuccessorImpl;
+
+final class ReturnSuccessor = ReturnSuccessorImpl;
+
 /**
  * A control flow node.
  *

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

@@ -3,7 +3,7 @@ private import codeql.rust.controlflow.ControlFlowGraph
 private import rust
 private import SuccessorType
 
-private newtype TCompletion =
+newtype TCompletion =
   TSimpleCompletion() or
   TBooleanCompletion(Boolean b) or
   TMatchCompletion(Boolean isMatch) or

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

@@ -1,11 +1,10 @@
 private import rust
 import codeql.controlflow.Cfg
 import Completion
-import codeql.controlflow.Cfg
-private import SuccessorType as ST
 private import Scope as Scope
+private import codeql.rust.controlflow.ControlFlowGraph as Cfg
 
-module CfgInput implements InputSig<Location> {
+private module CfgInput implements InputSig<Location> {
   private import rust as Rust
   private import Completion as C
   private import Splitting as S
@@ -29,7 +28,7 @@ module CfgInput implements InputSig<Location> {
 
   class Split = S::Split;
 
-  class SuccessorType = ST::SuccessorType;
+  class SuccessorType = Cfg::SuccessorType;
 
   /** Gets a successor type that matches completion `c`. */
   SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
@@ -37,13 +36,13 @@ module CfgInput implements InputSig<Location> {
   /**
    * Hold if `c` represents simple (normal) evaluation of a statement or an expression.
    */
-  predicate successorTypeIsSimple(SuccessorType t) { t instanceof ST::NormalSuccessor }
+  predicate successorTypeIsSimple(SuccessorType t) { t instanceof Cfg::NormalSuccessor }
 
   /** Holds if `t` is an abnormal exit type out of a CFG scope. */
   predicate isAbnormalExitType(SuccessorType t) { none() }
 
   /** Hold if `t` represents a conditional successor type. */
-  predicate successorTypeIsCondition(SuccessorType t) { t instanceof ST::BooleanSuccessor }
+  predicate successorTypeIsCondition(SuccessorType t) { t instanceof Cfg::BooleanSuccessor }
 
   /** Gets the maximum number of splits allowed for a given node. */
   int maxSplits() { result = 0 }
@@ -357,24 +356,24 @@ class MatchArmTree extends ControlFlowTree instanceof MatchArm {
 
 class MatchExprTree extends PostOrderTree instanceof MatchExpr {
   override predicate propagatesAbnormal(AstNode child) {
-    child = [super.getExpr(), super.getMatchArmList().getAnArm().getExpr()]
+    child = [super.getExpr(), super.getAnArm().getExpr()]
   }
 
   override predicate first(AstNode node) { first(super.getExpr(), node) }
 
   override predicate succ(AstNode pred, AstNode succ, Completion c) {
     // Edge from the scrutinee to the first arm.
-    last(super.getExpr(), pred, c) and succ = super.getMatchArmList().getArm(0).getPat()
+    last(super.getExpr(), pred, c) and succ = super.getArm(0).getPat()
     or
     // Edge from a failed match/guard in one arm to the beginning of the next arm.
     exists(int i |
-      last(super.getMatchArmList().getArm(i), pred, c) and
-      first(super.getMatchArmList().getArm(i + 1), succ) and
+      last(super.getArm(i), pred, c) and
+      first(super.getArm(i + 1), succ) and
       c.(ConditionalCompletion).failed()
     )
     or
     // Edge from the end of each arm to the match expression.
-    last(super.getMatchArmList().getArm(_), pred, c) and succ = this and completionIsNormal(c)
+    last(super.getArm(_).getExpr(), pred, c) and succ = this and completionIsNormal(c)
   }
 }
 
@@ -387,6 +386,10 @@ class MethodCallExprTree extends StandardPostOrderTree instanceof MethodCallExpr
 
 class OffsetOfExprTree extends LeafTree instanceof OffsetOfExpr { }
 
+class ParenExprTree extends StandardPostOrderTree, ParenExpr {
+  override ControlFlowTree getChildNode(int i) { i = 0 and result = super.getExpr() }
+}
+
 // This covers all patterns as they all extend `Pat`
 class PatExprTree extends LeafTree instanceof Pat { }
 

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

@@ -1,5 +1,6 @@
 private import rust
 private import codeql.util.Boolean
+private import Completion
 
 newtype TLoopJumpType =
   TContinueJump() or
@@ -14,36 +15,34 @@ newtype TSuccessorType =
   TSuccessorSuccessor() or
   TBooleanSuccessor(Boolean b) or
   TMatchSuccessor(Boolean b) or
-  TLoopSuccessor(TLoopJumpType kind, TLabelType label) or
+  TLoopSuccessor(TLoopJumpType kind, TLabelType label) { exists(TLoopCompletion(kind, label)) } or
   TReturnSuccessor()
 
 /** The type of a control flow successor. */
-abstract private class SuccessorTypeImpl extends TSuccessorType {
+abstract class SuccessorTypeImpl extends TSuccessorType {
   /** Gets a textual representation of successor type. */
   abstract string toString();
 }
 
-final class SuccessorType = SuccessorTypeImpl;
-
 /** A normal control flow successor. */
-final class NormalSuccessor extends SuccessorTypeImpl, TSuccessorSuccessor {
-  final override string toString() { result = "successor" }
+class NormalSuccessorImpl extends SuccessorTypeImpl, TSuccessorSuccessor {
+  override string toString() { result = "successor" }
 }
 
 /** A conditional control flow successor. */
-abstract private class ConditionalSuccessor extends SuccessorTypeImpl {
+abstract class ConditionalSuccessorImpl extends SuccessorTypeImpl {
   boolean value;
 
   bindingset[value]
-  ConditionalSuccessor() { any() }
+  ConditionalSuccessorImpl() { exists(value) }
 
   /** Gets the Boolean value of this successor. */
-  final boolean getValue() { result = value }
+  boolean getValue() { result = value }
 }
 
 /** A Boolean control flow successor for a boolean conditon. */
-final class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor {
-  BooleanSuccessor() { this = TBooleanSuccessor(value) }
+class BooleanSuccessorImpl extends ConditionalSuccessorImpl, TBooleanSuccessor {
+  BooleanSuccessorImpl() { this = TBooleanSuccessor(value) }
 
   override string toString() { result = this.getValue().toString() }
 }
@@ -51,8 +50,8 @@ final class BooleanSuccessor extends ConditionalSuccessor, TBooleanSuccessor {
 /**
  * A control flow successor of a pattern match.
  */
-final class MatchSuccessor extends ConditionalSuccessor, TMatchSuccessor {
-  MatchSuccessor() { this = TMatchSuccessor(value) }
+class MatchSuccessorImpl extends ConditionalSuccessorImpl, TMatchSuccessor {
+  MatchSuccessorImpl() { this = TMatchSuccessor(value) }
 
   override string toString() {
     if this.getValue() = true then result = "match" else result = "no-match"
@@ -62,20 +61,20 @@ final class MatchSuccessor extends ConditionalSuccessor, TMatchSuccessor {
 /**
  * A control flow successor of a loop control flow expression, `continue` or `break`.
  */
-final class LoopJumpSuccessor extends SuccessorTypeImpl, TLoopSuccessor {
-  final private TLoopJumpType getKind() { this = TLoopSuccessor(result, _) }
+class LoopJumpSuccessorImpl extends SuccessorTypeImpl, TLoopSuccessor {
+  private TLoopJumpType getKind() { this = TLoopSuccessor(result, _) }
 
-  final private TLabelType getLabelType() { this = TLoopSuccessor(_, result) }
+  private TLabelType getLabelType() { this = TLoopSuccessor(_, result) }
 
-  final predicate hasLabel() { this.getLabelType() = TLabel(_) }
+  predicate hasLabel() { this.getLabelType() = TLabel(_) }
 
-  final string getLabelName() { this = TLoopSuccessor(_, TLabel(result)) }
+  string getLabelName() { this = TLoopSuccessor(_, TLabel(result)) }
 
-  final predicate isContinue() { this.getKind() = TContinueJump() }
+  predicate isContinue() { this.getKind() = TContinueJump() }
 
-  final predicate isBreak() { this.getKind() = TBreakJump() }
+  predicate isBreak() { this.getKind() = TBreakJump() }
 
-  final override string toString() {
+  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
@@ -87,6 +86,6 @@ final class LoopJumpSuccessor extends SuccessorTypeImpl, TLoopSuccessor {
 /**
  * A `return` control flow successor.
  */
-final class ReturnSuccessor extends SuccessorTypeImpl, TReturnSuccessor {
-  final override string toString() { result = "return" }
+class ReturnSuccessorImpl extends SuccessorTypeImpl, TReturnSuccessor {
+  override string toString() { result = "return" }
 }

rust/ql/lib/codeql/rust/elements/internal/MatchExprImpl.qll

@@ -1,4 +1,3 @@
-// generated by codegen, remove this comment if you wish to edit this file
 /**
  * This module provides a hand-modifiable wrapper around the generated class `MatchExpr`.
  *
@@ -12,6 +11,7 @@ private import codeql.rust.elements.internal.generated.MatchExpr
  * be referenced directly.
  */
 module Impl {
+  // the following QLdoc is generated: if you need to edit it, do it in the schema file
   /**
    * A match expression. For example:
    * ```rust
@@ -27,5 +27,20 @@ module Impl {
    * }
    * ```
    */
-  class MatchExpr extends Generated::MatchExpr { }
+  class MatchExpr extends Generated::MatchExpr {
+    /**
+     * Gets the `index`th arm of this match expression.
+     */
+    MatchArm getArm(int index) { result = this.getMatchArmList().getArm(index) }
+
+    /**
+     * Gets any of the arms of this match expression.
+     */
+    MatchArm getAnArm() { result = this.getArm(_) }
+
+    /**
+     * Gets the number of arms of this match expression.
+     */
+    int getNumberOfArms() { result = this.getMatchArmList().getNumberOfArms() }
+  }
 }

rust/ql/test/library-tests/controlflow/Cfg.expected

@@ -149,6 +149,47 @@
 | test.rs:79:28:81:9 | BlockExpr | test.rs:79:9:81:9 | IfExpr |  |
 | test.rs:80:13:80:13 | PathExpr | test.rs:79:28:81:9 | BlockExpr |  |
 | test.rs:82:9:82:9 | LiteralExpr | test.rs:78:43:83:5 | BlockExpr |  |
+| test.rs:85:5:91:5 | enter test_nested_if | test.rs:86:16:86:16 | PathExpr |  |
+| test.rs:85:5:91:5 | exit test_nested_if (normal) | test.rs:85:5:91:5 | exit test_nested_if |  |
+| test.rs:85:38:91:5 | BlockExpr | test.rs:85:5:91:5 | exit test_nested_if (normal) |  |
+| test.rs:86:9:90:9 | IfExpr | test.rs:85:38:91:5 | BlockExpr |  |
+| test.rs:86:12:86:48 | ParenExpr | test.rs:87:13:87:13 | LiteralExpr | true |
+| test.rs:86:12:86:48 | ParenExpr | test.rs:89:13:89:13 | LiteralExpr | false |
+| test.rs:86:13:86:47 | IfExpr | test.rs:86:12:86:48 | ParenExpr |  |
+| test.rs:86:16:86:16 | PathExpr | test.rs:86:20:86:20 | LiteralExpr |  |
+| test.rs:86:16:86:20 | BinaryExpr | test.rs:86:24:86:24 | PathExpr | true |
+| test.rs:86:16:86:20 | BinaryExpr | test.rs:86:41:86:41 | PathExpr | false |
+| test.rs:86:20:86:20 | LiteralExpr | test.rs:86:16:86:20 | BinaryExpr |  |
+| test.rs:86:22:86:32 | BlockExpr | test.rs:86:13:86:47 | IfExpr |  |
+| test.rs:86:24:86:24 | PathExpr | test.rs:86:29:86:30 | LiteralExpr |  |
+| test.rs:86:24:86:30 | BinaryExpr | test.rs:86:22:86:32 | BlockExpr |  |
+| test.rs:86:28:86:30 | PrefixExpr | test.rs:86:24:86:30 | BinaryExpr |  |
+| test.rs:86:29:86:30 | LiteralExpr | test.rs:86:28:86:30 | PrefixExpr |  |
+| test.rs:86:39:86:47 | BlockExpr | test.rs:86:13:86:47 | IfExpr |  |
+| test.rs:86:41:86:41 | PathExpr | test.rs:86:45:86:46 | LiteralExpr |  |
+| test.rs:86:41:86:46 | BinaryExpr | test.rs:86:39:86:47 | BlockExpr |  |
+| test.rs:86:45:86:46 | LiteralExpr | test.rs:86:41:86:46 | BinaryExpr |  |
+| test.rs:86:50:88:9 | BlockExpr | test.rs:86:9:90:9 | IfExpr |  |
+| test.rs:87:13:87:13 | LiteralExpr | test.rs:86:50:88:9 | BlockExpr |  |
+| test.rs:88:16:90:9 | BlockExpr | test.rs:86:9:90:9 | IfExpr |  |
+| test.rs:89:13:89:13 | LiteralExpr | test.rs:88:16:90:9 | BlockExpr |  |
+| test.rs:93:5:99:5 | enter test_nested_if_match | test.rs:94:19:94:19 | PathExpr |  |
+| test.rs:93:5:99:5 | exit test_nested_if_match (normal) | test.rs:93:5:99:5 | exit test_nested_if_match |  |
+| test.rs:93:44:99:5 | BlockExpr | test.rs:93:5:99:5 | exit test_nested_if_match (normal) |  |
+| test.rs:94:9:98:9 | IfExpr | test.rs:93:44:99:5 | BlockExpr |  |
+| test.rs:94:12:94:46 | ParenExpr | test.rs:95:13:95:13 | LiteralExpr | true |
+| test.rs:94:12:94:46 | ParenExpr | test.rs:97:13:97:13 | LiteralExpr | false |
+| test.rs:94:13:94:45 | MatchExpr | test.rs:94:12:94:46 | ParenExpr |  |
+| test.rs:94:19:94:19 | PathExpr | test.rs:94:23:94:23 | LiteralPat |  |
+| test.rs:94:23:94:23 | LiteralPat | test.rs:94:28:94:31 | LiteralExpr | match |
+| test.rs:94:23:94:23 | LiteralPat | test.rs:94:34:94:34 | WildcardPat | no-match |
+| test.rs:94:28:94:31 | LiteralExpr | test.rs:94:13:94:45 | MatchExpr |  |
+| test.rs:94:34:94:34 | WildcardPat | test.rs:94:39:94:43 | LiteralExpr | match |
+| test.rs:94:39:94:43 | LiteralExpr | test.rs:94:13:94:45 | MatchExpr |  |
+| test.rs:94:48:96:9 | BlockExpr | test.rs:94:9:98:9 | IfExpr |  |
+| test.rs:95:13:95:13 | LiteralExpr | test.rs:94:48:96:9 | BlockExpr |  |
+| test.rs:96:16:98:9 | BlockExpr | test.rs:94:9:98:9 | IfExpr |  |
+| test.rs:97:13:97:13 | LiteralExpr | test.rs:96:16:98:9 | BlockExpr |  |
 | test.rs:105:5:108:5 | enter test_and_operator | test.rs:106:9:106:28 | LetStmt |  |
 | test.rs:105:5:108:5 | exit test_and_operator (normal) | test.rs:105:5:108:5 | exit test_and_operator |  |
 | test.rs:105:61:108:5 | BlockExpr | test.rs:105:5:108:5 | exit test_and_operator (normal) |  |
@@ -181,24 +222,28 @@
 | test.rs:116:9:116:36 | LetStmt | test.rs:116:17:116:35 | BinaryExpr |  |
 | test.rs:116:13:116:13 | IdentPat | test.rs:117:9:117:9 | PathExpr | match, no-match |
 | test.rs:116:17:116:17 | PathExpr | test.rs:116:13:116:13 | IdentPat | true |
+| test.rs:116:17:116:17 | PathExpr | test.rs:116:23:116:23 | PathExpr | false |
 | test.rs:116:17:116:30 | BinaryExpr | test.rs:116:17:116:17 | PathExpr |  |
 | test.rs:116:17:116:35 | BinaryExpr | test.rs:116:17:116:30 | BinaryExpr |  |
+| test.rs:116:22:116:30 | ParenExpr | test.rs:116:13:116:13 | IdentPat | true |
+| test.rs:116:22:116:30 | ParenExpr | test.rs:116:35:116:35 | PathExpr | false |
+| test.rs:116:23:116:23 | PathExpr | test.rs:116:28:116:29 | LiteralExpr |  |
+| test.rs:116:23:116:29 | BinaryExpr | test.rs:116:22:116:30 | ParenExpr |  |
+| test.rs:116:28:116:29 | LiteralExpr | test.rs:116:23:116:29 | BinaryExpr |  |
+| test.rs:116:35:116:35 | PathExpr | test.rs:116:13:116:13 | IdentPat |  |
 | test.rs:117:9:117:9 | PathExpr | test.rs:115:61:118:5 | BlockExpr |  |
 | test.rs:122:1:128:1 | enter test_match | test.rs:123:11:123:21 | PathExpr |  |
 | test.rs:122:1:128:1 | exit test_match (normal) | test.rs:122:1:128:1 | exit test_match |  |
 | test.rs:122:44:128:1 | BlockExpr | test.rs:122:1:128:1 | exit test_match (normal) |  |
 | test.rs:123:5:127:5 | MatchExpr | test.rs:122:44:128:1 | BlockExpr |  |
 | test.rs:123:11:123:21 | PathExpr | test.rs:124:9:124:23 | TupleStructPat |  |
-| test.rs:124:9:124:23 | TupleStructPat | test.rs:123:5:127:5 | MatchExpr | no-match |
 | test.rs:124:9:124:23 | TupleStructPat | test.rs:124:28:124:28 | PathExpr | match |
 | test.rs:124:9:124:23 | TupleStructPat | test.rs:125:9:125:23 | TupleStructPat | no-match |
 | test.rs:124:28:124:28 | PathExpr | test.rs:124:32:124:33 | LiteralExpr |  |
 | test.rs:124:32:124:33 | LiteralExpr | test.rs:124:28:124:33 | BinaryExpr |  |
-| test.rs:125:9:125:23 | TupleStructPat | test.rs:123:5:127:5 | MatchExpr | no-match |
 | test.rs:125:9:125:23 | TupleStructPat | test.rs:125:28:125:28 | PathExpr | match |
 | test.rs:125:9:125:23 | TupleStructPat | test.rs:126:9:126:20 | PathPat | no-match |
 | test.rs:125:28:125:28 | PathExpr | test.rs:123:5:127:5 | MatchExpr |  |
-| test.rs:126:9:126:20 | PathPat | test.rs:123:5:127:5 | MatchExpr | no-match |
 | test.rs:126:9:126:20 | PathPat | test.rs:126:25:126:25 | LiteralExpr | match |
 | test.rs:126:25:126:25 | LiteralExpr | test.rs:123:5:127:5 | MatchExpr |  |
 | test.rs:131:5:136:5 | enter test_infinite_loop | test.rs:132:9:134:9 | ExprStmt |  |

shared/controlflow/codeql/controlflow/Cfg.qll

@@ -1273,8 +1273,14 @@ module Make<LocationSig Location, InputSig<Location> Input> {
       string getOrderDisambiguation() { result = "" }
     }
 
-    import TestOutput<RelevantNode>
-    import Mermaid
+    private module Output = TestOutput<RelevantNode>;
+
+    import Output::Mermaid
+
+    query predicate edges(RelevantNode pred, RelevantNode succ, string attr, string val) {
+      attr = "semmle.label" and
+      Output::edges(pred, succ, val)
+    }
   }
 
   /** Provides a set of consistency queries. */