Rust: Fix minor issues from PR feedback

Author: paldepind

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

@@ -9,8 +9,6 @@ private import BasicBlocks
 
 final class CfgScope = Scope::CfgScope;
 
-predicate getEnclosingCfgScope = Scope::getEnclosingCfgScope/1;
-
 final class SuccessorType = SuccessorTypeImpl;
 
 final class NormalSuccessor = NormalSuccessorImpl;
@@ -54,6 +52,3 @@ final class CfgNode extends Node {
   /** Gets the basic block that this control flow node belongs to. */
   BasicBlock getBasicBlock() { result.getANode() = this }
 }
-
-/** Holds if evaluating `e` jumps to the evaluation of a different CFG scope. */
-predicate isControlFlowJump(Expr e) { e instanceof CallExprBase or e instanceof AwaitExpr }

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

@@ -23,7 +23,7 @@ private module CfgInput implements InputSig<Location> {
   class CfgScope = Scope::CfgScope;
 
   CfgScope getCfgScope(AstNode n) {
-    result = Scope::getEnclosingCfgScope(n) and
+    result = n.getEnclosingCfgScope() and
     Stages::CfgStage::ref()
   }
 

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

@@ -16,13 +16,12 @@ abstract private class CfgScopeImpl extends AstNode {
 
 final class CfgScope = CfgScopeImpl;
 
-final class AsyncBlockScope extends CfgScopeImpl, AsyncBlockExpr {
-  override predicate scopeFirst(AstNode first) {
-    first(this.(ExprTrees::AsyncBlockExprTree).getFirstChildNode(), first)
-  }
+final class AsyncBlockScope extends CfgScopeImpl, AsyncBlockExpr instanceof ExprTrees::AsyncBlockExprTree
+{
+  override predicate scopeFirst(AstNode first) { first(super.getFirstChildNode(), first) }
 
   override predicate scopeLast(AstNode last, Completion c) {
-    last(this.(ExprTrees::AsyncBlockExprTree).getLastChildElement(), last, c)
+    last(super.getLastChildElement(), last, c)
   }
 }
 
@@ -52,13 +51,3 @@ final class CallableScope extends CfgScopeImpl, Callable {
   /** Holds if `scope` is exited when `last` finishes with completion `c`. */
   override predicate scopeLast(AstNode last, Completion c) { last(this.getBody(), last, c) }
 }
-
-/** Gets the CFG scope that encloses `node`, if any. */
-CfgScope getEnclosingCfgScope(AstNode node) {
-  exists(AstNode p | p = node.getParentNode() |
-    result = p
-    or
-    not p instanceof CfgScope and
-    result = getEnclosingCfgScope(p)
-  )
-}

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -134,7 +134,7 @@ module Node {
 
     ExprNode() { this = TExprNode(n) }
 
-    override CfgScope getCfgScope() { result = getEnclosingCfgScope(this.asExpr()) }
+    override CfgScope getCfgScope() { result = this.asExpr().getEnclosingCfgScope() }
 
     override Location getLocation() { result = n.getExpr().getLocation() }
 
@@ -154,7 +154,7 @@ module Node {
 
     ParameterNode() { this = TParameterNode(parameter) }
 
-    override CfgScope getCfgScope() { result = getEnclosingCfgScope(parameter) }
+    override CfgScope getCfgScope() { result = parameter.getEnclosingCfgScope() }
 
     override Location getLocation() { result = parameter.getLocation() }
 

rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll

@@ -156,7 +156,7 @@ private predicate variableReadActual(BasicBlock bb, int i, Variable v) {
  */
 pragma[noinline]
 private predicate hasCapturedWrite(Variable v, Cfg::CfgScope scope) {
-  any(VariableWriteAccess write | write.getVariable() = v and scope = getEnclosingCfgScope+(write))
+  any(VariableWriteAccess write | write.getVariable() = v and scope = write.getEnclosingCfgScope+())
       .isCapture()
 }
 
@@ -168,7 +168,7 @@ pragma[noinline]
 private predicate variableReadActualInOuterScope(
   BasicBlock bb, int i, Variable v, Cfg::CfgScope scope
 ) {
-  variableReadActual(bb, i, v) and bb.getScope() = getEnclosingCfgScope(scope)
+  variableReadActual(bb, i, v) and bb.getScope() = scope.getEnclosingCfgScope()
 }
 
 pragma[noinline]
@@ -263,7 +263,7 @@ private predicate readsCapturedVariable(BasicBlock bb, Variable v) {
  */
 pragma[noinline]
 private predicate hasCapturedRead(Variable v, Cfg::CfgScope scope) {
-  any(VariableReadAccess read | read.getVariable() = v and scope = getEnclosingCfgScope+(read))
+  any(VariableReadAccess read | read.getVariable() = v and scope = read.getEnclosingCfgScope+())
       .isCapture()
 }
 
@@ -273,15 +273,18 @@ private predicate hasCapturedRead(Variable v, Cfg::CfgScope scope) {
  */
 pragma[noinline]
 private predicate variableWriteInOuterScope(BasicBlock bb, int i, Variable v, Cfg::CfgScope scope) {
-  SsaInput::variableWrite(bb, i, v, _) and getEnclosingCfgScope(scope) = bb.getScope()
+  SsaInput::variableWrite(bb, i, v, _) and scope.getEnclosingCfgScope() = bb.getScope()
 }
 
+/** Holds if evaluating `e` jumps to the evaluation of a different CFG scope. */
+private predicate isControlFlowJump(Expr e) { e instanceof CallExprBase or e instanceof AwaitExpr }
+
 /**
  * Holds if the call `call` at index `i` in basic block `bb` may reach
  * a callable that reads captured variable `v`.
  */
 private predicate capturedCallRead(Expr call, BasicBlock bb, int i, Variable v) {
-  Cfg::isControlFlowJump(call) and
+  isControlFlowJump(call) and
   exists(Cfg::CfgScope scope |
     hasCapturedRead(v, scope) and
     (
@@ -297,7 +300,7 @@ private predicate capturedCallRead(Expr call, BasicBlock bb, int i, Variable v)
  * that writes captured variable `v`.
  */
 predicate capturedCallWrite(Expr call, BasicBlock bb, int i, Variable v) {
-  Cfg::isControlFlowJump(call) and
+  isControlFlowJump(call) and
   call = bb.getNode(i).getAstNode() and
   exists(Cfg::CfgScope scope |
     hasVariableReadWithCapturedWrite(bb, any(int j | j > i), v, scope)

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

@@ -5,6 +5,7 @@
  */
 
 private import codeql.rust.elements.internal.generated.AstNode
+private import codeql.rust.controlflow.ControlFlowGraph
 
 /**
  * INTERNAL: This module contains the customizable definition of `AstNode` and should not
@@ -44,6 +45,16 @@ module Impl {
       )
     }
 
+    /** Gets the CFG scope that encloses this node, if any. */
+    CfgScope getEnclosingCfgScope() {
+      exists(AstNode p | p = this.getParentNode() |
+        result = p
+        or
+        not p instanceof CfgScope and
+        result = p.getEnclosingCfgScope()
+      )
+    }
+
     /** Holds if this node is inside a macro expansion. */
     predicate isInMacroExpansion() {
       this = any(MacroCall mc).getExpanded()

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

@@ -446,7 +446,7 @@ module Impl {
     Variable getVariable() { result = v }
 
     /** Holds if this access is a capture. */
-    predicate isCapture() { getEnclosingCfgScope(this) != getEnclosingCfgScope(v.getPat()) }
+    predicate isCapture() { this.getEnclosingCfgScope() != v.getPat().getEnclosingCfgScope() }
 
     override string toString() { result = name }
 

rust/ql/test/library-tests/variables/variables.expected

@@ -1,6 +1,4 @@
 testFailures
-| variables.rs:438:9:438:9 | i | Unexpected result: write_access=i |
-| variables.rs:438:16:438:33 | Comment | Missing result: read_access=i |
 failures
 variable
 | variables.rs:3:14:3:14 | s |

rust/ql/test/library-tests/variables/variables.rs

@@ -435,7 +435,7 @@ fn capture_mut() {
 async fn async_block_capture() {
     let mut i: i64 = 0; // i
     let block = async {
-        i = 1; // $ read_access=i
+        i = 1; // $ write_access=i
     };
     // The await below causes write to `i`
     block.await; // $ read_access=block