Merge pull request #17558 from hvitved/rust/cfg-consistency-queries

Rust: Enable CFG consistency checks

csharp/ql/consistency-queries/CfgConsistency.ql

@@ -61,8 +61,3 @@ query predicate preBasicBlockConsistency(ControlFlowElement cfe1, ControlFlowEle
   bbIntraSuccInconsistency(cfe1, cfe2) and
   s = "intra succ inconsistency"
 }
-
-query predicate multipleToString(Node n, string s) {
-  s = strictconcat(n.toString(), ",") and
-  strictcount(n.toString()) > 1
-}

ruby/ql/consistency-queries/CfgConsistency.ql

@@ -19,8 +19,3 @@ query predicate nonPostOrderExpr(Expr e, string cls) {
     c instanceof NormalCompletion
   )
 }
-
-query predicate multipleToString(CfgNode n, string s) {
-  s = strictconcat(n.toString(), ",") and
-  strictcount(n.toString()) > 1
-}

rust/ql/consistency-queries/CfgConsistency.ql

@@ -0,0 +1,19 @@
+import rust
+import codeql.rust.controlflow.internal.ControlFlowGraphImpl::Consistency
+import codeql.rust.controlflow.internal.ControlFlowGraphImpl as CfgImpl
+import codeql.rust.controlflow.internal.Completion
+
+/**
+ * All `Expr` nodes are `PostOrderTree`s
+ */
+query predicate nonPostOrderExpr(Expr e, string cls) {
+  cls = e.getPrimaryQlClasses() and
+  not e instanceof LetExpr and
+  not e instanceof LogicalAndExpr and // todo
+  not e instanceof LogicalOrExpr and
+  exists(AstNode last, Completion c |
+    CfgImpl::last(e, last, c) and
+    last != e and
+    c instanceof NormalCompletion
+  )
+}

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

@@ -73,7 +73,7 @@ class BecomeExprTree extends StandardPostOrderTree instanceof BecomeExpr {
 }
 
 class BinaryOpExprTree extends StandardPostOrderTree instanceof BinaryExpr {
-  BinaryOpExprTree() { super.getOperatorName() != "&&" and super.getOperatorName() != "||" }
+  BinaryOpExprTree() { not this instanceof BinaryLogicalOperation }
 
   override ControlFlowTree getChildNode(int i) {
     i = 0 and result = super.getLhs()
@@ -82,61 +82,53 @@ class BinaryOpExprTree extends StandardPostOrderTree instanceof BinaryExpr {
   }
 }
 
-class LogicalOrBinaryOpExprTree extends PreOrderTree instanceof BinaryExpr {
-  LogicalOrBinaryOpExprTree() { super.getOperatorName() = "||" }
-
-  final override predicate propagatesAbnormal(AstNode child) {
-    child = [super.getRhs(), super.getLhs()]
-  }
+class LogicalOrBinaryOpExprTree extends PreOrderTree, LogicalOrExpr {
+  final override predicate propagatesAbnormal(AstNode child) { child = this.getAnOperand() }
 
   override predicate succ(AstNode pred, AstNode succ, Completion c) {
     // Edge to the first node in the lhs
     pred = this and
-    first(super.getLhs(), succ) and
+    first(this.getLhs(), succ) and
     completionIsSimple(c)
     or
     // Edge from the last node in the lhs to the first node in the rhs
-    last(super.getLhs(), pred, c) and
-    first(super.getRhs(), succ) and
+    last(this.getLhs(), pred, c) and
+    first(this.getRhs(), succ) and
     c.(BooleanCompletion).failed()
   }
 
   override predicate last(AstNode node, Completion c) {
     // Lhs. as the last node
-    last(super.getLhs(), node, c) and
+    last(this.getLhs(), node, c) and
     c.(BooleanCompletion).succeeded()
     or
     // Rhs. as the last node
-    last(super.getRhs(), node, c)
+    last(this.getRhs(), node, c)
   }
 }
 
-class LogicalAndBinaryOpExprTree extends PreOrderTree instanceof BinaryExpr {
-  LogicalAndBinaryOpExprTree() { super.getOperatorName() = "&&" }
-
-  final override predicate propagatesAbnormal(AstNode child) {
-    child = [super.getRhs(), super.getLhs()]
-  }
+class LogicalAndBinaryOpExprTree extends PreOrderTree, LogicalAndExpr {
+  final override predicate propagatesAbnormal(AstNode child) { child = this.getAnOperand() }
 
   override predicate succ(AstNode pred, AstNode succ, Completion c) {
     // Edge to the first node in the lhs
     pred = this and
-    first(super.getLhs(), succ) and
+    first(this.getLhs(), succ) and
     completionIsSimple(c)
     or
     // Edge from the last node in the lhs to the first node in the rhs
-    last(super.getLhs(), pred, c) and
-    first(super.getRhs(), succ) and
+    last(this.getLhs(), pred, c) and
+    first(this.getRhs(), succ) and
     c.(BooleanCompletion).succeeded()
   }
 
   override predicate last(AstNode node, Completion c) {
     // Lhs. as the last node
-    last(super.getLhs(), node, c) and
+    last(this.getLhs(), node, c) and
     c.(BooleanCompletion).failed()
     or
     // Rhs. as the last node
-    last(super.getRhs(), node, c)
+    last(this.getRhs(), node, c)
   }
 }
 

rust/ql/lib/codeql/rust/elements/LogicalOperation.qll

@@ -0,0 +1,33 @@
+private import codeql.rust.elements.Expr
+private import codeql.rust.elements.BinaryExpr
+private import codeql.rust.elements.PrefixExpr
+
+abstract private class LogicalOperationImpl extends Expr {
+  abstract Expr getAnOperand();
+}
+
+final class LogicalOperation = LogicalOperationImpl;
+
+abstract private class BinaryLogicalOperationImpl extends BinaryExpr, LogicalOperationImpl {
+  override Expr getAnOperand() { result = [this.getLhs(), this.getRhs()] }
+}
+
+final class BinaryLogicalOperation = BinaryLogicalOperationImpl;
+
+final class LogicalAndExpr extends BinaryLogicalOperationImpl, BinaryExpr {
+  LogicalAndExpr() { this.getOperatorName() = "&&" }
+}
+
+final class LogicalOrExpr extends BinaryLogicalOperationImpl {
+  LogicalOrExpr() { this.getOperatorName() = "||" }
+}
+
+abstract private class UnaryLogicalOperationImpl extends PrefixExpr, LogicalOperationImpl { }
+
+final class UnaryLogicalOperation = UnaryLogicalOperationImpl;
+
+final class LogicalNotExpr extends UnaryLogicalOperationImpl {
+  LogicalNotExpr() { this.getOperatorName() = "!" }
+
+  override Expr getAnOperand() { result = this.getExpr() }
+}

rust/ql/lib/codeql/rust/elements/internal/BinaryExprImpl.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 `BinaryExpr`.
  *
@@ -12,6 +11,7 @@ private import codeql.rust.elements.internal.generated.BinaryExpr
  * be referenced directly.
  */
 module Impl {
+  // the following QLdoc is generated: if you need to edit it, do it in the schema file
   /**
    * A binary operation expression. For example:
    * ```rust
@@ -22,5 +22,7 @@ module Impl {
    * x += y;
    * ```
    */
-  class BinaryExpr extends Generated::BinaryExpr { }
+  class BinaryExpr extends Generated::BinaryExpr {
+    override string toString() { result = "... " + this.getOperatorName() + " ..." }
+  }
 }

rust/ql/lib/codeql/rust/elements/internal/PrefixExprImpl.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 `PrefixExpr`.
  *
@@ -12,6 +11,7 @@ private import codeql.rust.elements.internal.generated.PrefixExpr
  * be referenced directly.
  */
 module Impl {
+  // the following QLdoc is generated: if you need to edit it, do it in the schema file
   /**
    * A unary operation expression. For example:
    * ```rust
@@ -20,5 +20,7 @@ module Impl {
    * let z = *ptr
    * ```
    */
-  class PrefixExpr extends Generated::PrefixExpr { }
+  class PrefixExpr extends Generated::PrefixExpr {
+    override string toString() { result = this.getOperatorName() + " ..." }
+  }
 }

rust/ql/lib/rust.qll

@@ -3,3 +3,4 @@
 import codeql.rust.elements
 import codeql.Locations
 import codeql.files.FileSystem
+import codeql.rust.elements.LogicalOperation

rust/ql/test/extractor-tests/generated/BinaryExpr/BinaryExpr.expected

@@ -1,5 +1,5 @@
-| gen_binary_expr.rs:5:5:5:9 | BinaryExpr | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |
-| gen_binary_expr.rs:6:5:6:10 | BinaryExpr | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |
-| gen_binary_expr.rs:7:5:7:10 | BinaryExpr | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |
-| gen_binary_expr.rs:8:5:8:9 | BinaryExpr | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |
-| gen_binary_expr.rs:9:5:9:10 | BinaryExpr | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |
+| gen_binary_expr.rs:5:5:5:9 | ... + ... | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |
+| gen_binary_expr.rs:6:5:6:10 | ... && ... | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |
+| gen_binary_expr.rs:7:5:7:10 | ... <= ... | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |
+| gen_binary_expr.rs:8:5:8:9 | ... = ... | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |
+| gen_binary_expr.rs:9:5:9:10 | ... += ... | getNumberOfAttrs: | 0 | hasLhs: | yes | hasOperatorName: | yes | hasRhs: | yes |

rust/ql/test/extractor-tests/generated/BinaryExpr/BinaryExpr_getLhs.expected

@@ -1,5 +1,5 @@
-| gen_binary_expr.rs:5:5:5:9 | BinaryExpr | gen_binary_expr.rs:5:5:5:5 | PathExpr |
-| gen_binary_expr.rs:6:5:6:10 | BinaryExpr | gen_binary_expr.rs:6:5:6:5 | PathExpr |
-| gen_binary_expr.rs:7:5:7:10 | BinaryExpr | gen_binary_expr.rs:7:5:7:5 | PathExpr |
-| gen_binary_expr.rs:8:5:8:9 | BinaryExpr | gen_binary_expr.rs:8:5:8:5 | PathExpr |
-| gen_binary_expr.rs:9:5:9:10 | BinaryExpr | gen_binary_expr.rs:9:5:9:5 | PathExpr |
+| gen_binary_expr.rs:5:5:5:9 | ... + ... | gen_binary_expr.rs:5:5:5:5 | PathExpr |
+| gen_binary_expr.rs:6:5:6:10 | ... && ... | gen_binary_expr.rs:6:5:6:5 | PathExpr |
+| gen_binary_expr.rs:7:5:7:10 | ... <= ... | gen_binary_expr.rs:7:5:7:5 | PathExpr |
+| gen_binary_expr.rs:8:5:8:9 | ... = ... | gen_binary_expr.rs:8:5:8:5 | PathExpr |
+| gen_binary_expr.rs:9:5:9:10 | ... += ... | gen_binary_expr.rs:9:5:9:5 | PathExpr |

rust/ql/test/extractor-tests/generated/BinaryExpr/BinaryExpr_getOperatorName.expected

@@ -1,5 +1,5 @@
-| gen_binary_expr.rs:5:5:5:9 | BinaryExpr | + |
-| gen_binary_expr.rs:6:5:6:10 | BinaryExpr | && |
-| gen_binary_expr.rs:7:5:7:10 | BinaryExpr | <= |
-| gen_binary_expr.rs:8:5:8:9 | BinaryExpr | = |
-| gen_binary_expr.rs:9:5:9:10 | BinaryExpr | += |
+| gen_binary_expr.rs:5:5:5:9 | ... + ... | + |
+| gen_binary_expr.rs:6:5:6:10 | ... && ... | && |
+| gen_binary_expr.rs:7:5:7:10 | ... <= ... | <= |
+| gen_binary_expr.rs:8:5:8:9 | ... = ... | = |
+| gen_binary_expr.rs:9:5:9:10 | ... += ... | += |

rust/ql/test/extractor-tests/generated/BinaryExpr/BinaryExpr_getRhs.expected

@@ -1,5 +1,5 @@
-| gen_binary_expr.rs:5:5:5:9 | BinaryExpr | gen_binary_expr.rs:5:9:5:9 | PathExpr |
-| gen_binary_expr.rs:6:5:6:10 | BinaryExpr | gen_binary_expr.rs:6:10:6:10 | PathExpr |
-| gen_binary_expr.rs:7:5:7:10 | BinaryExpr | gen_binary_expr.rs:7:10:7:10 | PathExpr |
-| gen_binary_expr.rs:8:5:8:9 | BinaryExpr | gen_binary_expr.rs:8:9:8:9 | PathExpr |
-| gen_binary_expr.rs:9:5:9:10 | BinaryExpr | gen_binary_expr.rs:9:10:9:10 | PathExpr |
+| gen_binary_expr.rs:5:5:5:9 | ... + ... | gen_binary_expr.rs:5:9:5:9 | PathExpr |
+| gen_binary_expr.rs:6:5:6:10 | ... && ... | gen_binary_expr.rs:6:10:6:10 | PathExpr |
+| gen_binary_expr.rs:7:5:7:10 | ... <= ... | gen_binary_expr.rs:7:10:7:10 | PathExpr |
+| gen_binary_expr.rs:8:5:8:9 | ... = ... | gen_binary_expr.rs:8:9:8:9 | PathExpr |
+| gen_binary_expr.rs:9:5:9:10 | ... += ... | gen_binary_expr.rs:9:10:9:10 | PathExpr |

rust/ql/test/extractor-tests/generated/ClosureExpr/ClosureExpr_getBody.expected

@@ -1,5 +1,5 @@
-| gen_closure_expr.rs:5:5:5:13 | ClosureExpr | gen_closure_expr.rs:5:9:5:13 | BinaryExpr |
+| gen_closure_expr.rs:5:5:5:13 | ClosureExpr | gen_closure_expr.rs:5:9:5:13 | ... + ... |
 | gen_closure_expr.rs:6:5:6:34 | ClosureExpr | gen_closure_expr.rs:6:26:6:34 | BlockExpr |
-| gen_closure_expr.rs:7:5:7:27 | ClosureExpr | gen_closure_expr.rs:7:23:7:27 | BinaryExpr |
+| gen_closure_expr.rs:7:5:7:27 | ClosureExpr | gen_closure_expr.rs:7:23:7:27 | ... + ... |
 | gen_closure_expr.rs:8:6:9:15 | ClosureExpr | gen_closure_expr.rs:9:9:9:15 | YieldExpr |
 | gen_closure_expr.rs:10:6:11:23 | ClosureExpr | gen_closure_expr.rs:11:17:11:23 | YieldExpr |

rust/ql/test/extractor-tests/generated/IfExpr/IfExpr_getCondition.expected

@@ -1,2 +1,2 @@
-| gen_if_expr.rs:5:5:7:5 | IfExpr | gen_if_expr.rs:5:8:5:14 | BinaryExpr |
-| gen_if_expr.rs:8:13:12:5 | IfExpr | gen_if_expr.rs:8:16:8:20 | BinaryExpr |
+| gen_if_expr.rs:5:5:7:5 | IfExpr | gen_if_expr.rs:5:8:5:14 | ... == ... |
+| gen_if_expr.rs:8:13:12:5 | IfExpr | gen_if_expr.rs:8:16:8:20 | ... > ... |

rust/ql/test/extractor-tests/generated/MatchArm/CONSISTENCY/CfgConsistency.expected

@@ -0,0 +1,2 @@
+deadEnd
+| gen_match_arm.rs:10:20:10:25 | ... != ... |

rust/ql/test/extractor-tests/generated/MatchArm/MatchArm_getExpr.expected

@@ -1,4 +1,4 @@
 | gen_match_arm.rs:6:9:6:29 | MatchArm | gen_match_arm.rs:6:28:6:28 | PathExpr |
 | gen_match_arm.rs:7:9:7:26 | MatchArm | gen_match_arm.rs:7:25:7:25 | LiteralExpr |
-| gen_match_arm.rs:10:9:10:35 | MatchArm | gen_match_arm.rs:10:30:10:34 | BinaryExpr |
+| gen_match_arm.rs:10:9:10:35 | MatchArm | gen_match_arm.rs:10:30:10:34 | ... / ... |
 | gen_match_arm.rs:11:9:11:15 | MatchArm | gen_match_arm.rs:11:14:11:14 | LiteralExpr |

rust/ql/test/extractor-tests/generated/MatchExpr/CONSISTENCY/CfgConsistency.expected

@@ -0,0 +1,2 @@
+deadEnd
+| gen_match_expr.rs:10:20:10:25 | ... != ... |

rust/ql/test/extractor-tests/generated/PrefixExpr/PrefixExpr.expected

@@ -1,3 +1,3 @@
-| gen_prefix_expr.rs:5:13:5:15 | PrefixExpr | getNumberOfAttrs: | 0 | hasExpr: | yes | hasOperatorName: | yes |
-| gen_prefix_expr.rs:6:13:6:17 | PrefixExpr | getNumberOfAttrs: | 0 | hasExpr: | yes | hasOperatorName: | yes |
-| gen_prefix_expr.rs:7:13:7:16 | PrefixExpr | getNumberOfAttrs: | 0 | hasExpr: | yes | hasOperatorName: | yes |
+| gen_prefix_expr.rs:5:13:5:15 | - ... | getNumberOfAttrs: | 0 | hasExpr: | yes | hasOperatorName: | yes |
+| gen_prefix_expr.rs:6:13:6:17 | ! ... | getNumberOfAttrs: | 0 | hasExpr: | yes | hasOperatorName: | yes |
+| gen_prefix_expr.rs:7:13:7:16 | * ... | getNumberOfAttrs: | 0 | hasExpr: | yes | hasOperatorName: | yes |

rust/ql/test/extractor-tests/generated/PrefixExpr/PrefixExpr_getExpr.expected

@@ -1,3 +1,3 @@
-| gen_prefix_expr.rs:5:13:5:15 | PrefixExpr | gen_prefix_expr.rs:5:14:5:15 | LiteralExpr |
-| gen_prefix_expr.rs:6:13:6:17 | PrefixExpr | gen_prefix_expr.rs:6:14:6:17 | LiteralExpr |
-| gen_prefix_expr.rs:7:13:7:16 | PrefixExpr | gen_prefix_expr.rs:7:14:7:16 | PathExpr |
+| gen_prefix_expr.rs:5:13:5:15 | - ... | gen_prefix_expr.rs:5:14:5:15 | LiteralExpr |
+| gen_prefix_expr.rs:6:13:6:17 | ! ... | gen_prefix_expr.rs:6:14:6:17 | LiteralExpr |
+| gen_prefix_expr.rs:7:13:7:16 | * ... | gen_prefix_expr.rs:7:14:7:16 | PathExpr |

rust/ql/test/extractor-tests/generated/PrefixExpr/PrefixExpr_getOperatorName.expected

@@ -1,3 +1,3 @@
-| gen_prefix_expr.rs:5:13:5:15 | PrefixExpr | - |
-| gen_prefix_expr.rs:6:13:6:17 | PrefixExpr | ! |
-| gen_prefix_expr.rs:7:13:7:16 | PrefixExpr | * |
+| gen_prefix_expr.rs:5:13:5:15 | - ... | - |
+| gen_prefix_expr.rs:6:13:6:17 | ! ... | ! |
+| gen_prefix_expr.rs:7:13:7:16 | * ... | * |

rust/ql/test/library-tests/controlflow/CONSISTENCY/CfgConsistency.expected

@@ -0,0 +1,3 @@
+deadEnd
+| test.rs:124:28:124:33 | ... < ... |
+| test.rs:139:30:141:9 | BlockExpr |