Address review comments

Author: hvitved

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

@@ -258,7 +258,8 @@ final class CallExprCfgNode extends Nodes::CallExprCfgNode {
 
   /** Gets the `i`th argument of this call. */
   ExprCfgNode getSyntacticArgument(int i) {
-    any(ChildMapping mapping).hasCfgChild(node, node.getSyntacticArgument(i), this, result)
+    any(ChildMapping mapping)
+        .hasCfgChild(node, node.getSyntacticPositionalArgument(i), this, result)
   }
 }
 

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

@@ -210,13 +210,18 @@ module ExprTrees {
     override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() }
   }
 
-  class ArgsExprTree extends StandardPostOrderTree instanceof ArgsExpr {
-    ArgsExprTree() {
+  class InvocationExprTree extends StandardPostOrderTree instanceof InvocationExpr {
+    InvocationExprTree() {
       not this instanceof CallExpr and
       not this instanceof BinaryLogicalOperation
     }
 
-    override AstNode getChildNode(int i) { result = super.getSyntacticArgument(i) }
+    override AstNode getChildNode(int i) {
+      i = 0 and
+      result = super.getSyntacticReceiver()
+      or
+      result = super.getSyntacticPositionalArgument(i - 1)
+    }
   }
 
   class LogicalOrExprTree extends PostOrderTree, LogicalOrExpr {
@@ -295,7 +300,7 @@ module ExprTrees {
     override AstNode getChildNode(int i) {
       i = 0 and result = super.getFunction()
       or
-      result = super.getSyntacticArgument(i - 1)
+      result = super.getSyntacticPositionalArgument(i - 1)
     }
   }
 
@@ -533,10 +538,6 @@ module ExprTrees {
     }
   }
 
-  class RefExprTree extends StandardPostOrderTree instanceof RefExpr {
-    override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() }
-  }
-
   class ReturnExprTree extends StandardPostOrderTree instanceof ReturnExpr {
     override AstNode getChildNode(int i) { i = 0 and result = super.getExpr() }
   }

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

@@ -5,6 +5,7 @@
 private import rust
 private import codeql.rust.frameworks.stdlib.Builtins
 private import DataFlowImpl
+private import codeql.rust.elements.internal.CallExprImpl::Impl as CallExprImpl
 
 /**
  * A path to a value contained in an object. For example a field name of a struct.
@@ -168,10 +169,10 @@ final class TuplePositionContent extends FieldContent, TTuplePositionContent {
  * Used by the model generator to create flow summaries for higher-order
  * functions.
  */
-final class ClosureCallArgumentContent extends Content, TClosureCallArgumentContent {
+final class FunctionCallArgumentContent extends Content, TFunctionCallArgumentContent {
   private int pos;
 
-  ClosureCallArgumentContent() { this = TClosureCallArgumentContent(pos) }
+  FunctionCallArgumentContent() { this = TFunctionCallArgumentContent(pos) }
 
   int getPosition() { result = pos }
 
@@ -269,8 +270,8 @@ newtype TContent =
         )]
   } or
   TFunctionCallReturnContent() or
-  TClosureCallArgumentContent(int pos) {
-    pos in [0 .. any(ClosureCallExpr c).getNumberOfPositionalArguments()]
+  TFunctionCallArgumentContent(int pos) {
+    pos in [0 .. any(CallExprImpl::DynamicCallExpr c).getNumberOfPositionalArguments()]
   } or
   TCapturedVariableContent(VariableCapture::CapturedVariable v) or
   TReferenceContent()

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

@@ -10,6 +10,7 @@ private import codeql.dataflow.internal.DataFlowImpl
 private import rust
 private import SsaImpl as SsaImpl
 private import codeql.rust.controlflow.internal.Scope as Scope
+private import codeql.rust.elements.internal.CallExprImpl::Impl as CallExprImpl
 private import codeql.rust.internal.PathResolution
 private import codeql.rust.controlflow.ControlFlowGraph
 private import codeql.rust.dataflow.Ssa
@@ -85,62 +86,10 @@ final class DataFlowCall extends TDataFlowCall {
   Location getLocation() { result = this.asCall().getLocation() }
 }
 
-/**
- * The position of a parameter in a function.
- *
- * In Rust there is a 1-to-1 correspondence between parameter positions and
- * arguments positions, so we use the same underlying type for both.
- */
-final class ParameterPosition extends TParameterPosition {
-  /** Gets the underlying integer position, if any. */
-  int getPosition() { this = TPositionalParameterPosition(result) }
-
-  predicate hasPosition() { exists(this.getPosition()) }
-
-  /** Holds if this position represents the `self` position. */
-  predicate isSelf() { this = TSelfParameterPosition() }
-
-  /**
-   * Holds if this position represents a reference to a closure itself. Only
-   * used for tracking flow through captured variables.
-   */
-  predicate isClosureSelf() { this = TClosureSelfParameterPosition() }
-
-  /** Gets a textual representation of this position. */
-  string toString() {
-    result = this.getPosition().toString()
-    or
-    result = "self" and this.isSelf()
-    or
-    result = "closure self" and this.isClosureSelf()
-  }
-
-  ParamBase getParameterIn(ParamList ps) {
-    result = ps.getParam(this.getPosition())
-    or
-    result = ps.getSelfParam() and this.isSelf()
-  }
-}
-
-/**
- * The position of an argument in a call.
- *
- * In Rust there is a 1-to-1 correspondence between parameter positions and
- * arguments positions, so we use the same underlying type for both.
- */
-final class ArgumentPosition extends ParameterPosition {
-  /** Gets the argument of `call` at this position, if any. */
-  Expr getArgument(Call call) {
-    result = call.getPositionalArgument(this.getPosition())
-    or
-    result = call.(MethodCall).getReceiver() and this.isSelf()
-  }
-}
-
 /**
  * Holds if `arg` is an argument of `call` at the position `pos`.
  */
-predicate isArgumentForCall(Expr arg, Call call, ArgumentPosition pos) {
+predicate isArgumentForCall(Expr arg, Call call, RustDataFlow::ArgumentPosition pos) {
   // TODO: Handle index expressions as calls in data flow.
   not call instanceof IndexExpr and
   arg = pos.getArgument(call)
@@ -292,8 +241,8 @@ predicate lambdaCreationExpr(Expr creation) {
  * Holds if `call` is a lambda call of kind `kind` where `receiver` is the
  * invoked expression.
  */
-predicate lambdaCallExpr(ClosureCallExpr call, LambdaCallKind kind, Expr receiver) {
-  receiver = call.getClosureExpr() and
+predicate lambdaCallExpr(CallExprImpl::DynamicCallExpr call, LambdaCallKind kind, Expr receiver) {
+  receiver = call.getFunction() and
   exists(kind)
 }
 
@@ -305,10 +254,6 @@ private module Aliases {
 
   class DataFlowCallAlias = DataFlowCall;
 
-  class ParameterPositionAlias = ParameterPosition;
-
-  class ArgumentPositionAlias = ArgumentPosition;
-
   class ContentAlias = Content;
 
   class ContentSetAlias = ContentSet;
@@ -340,6 +285,58 @@ module RustDataFlow implements InputSig<Location> {
 
   final class CastNode = Node::CastNode;
 
+  /**
+   * The position of a parameter in a function.
+   *
+   * In Rust there is a 1-to-1 correspondence between parameter positions and
+   * arguments positions, so we use the same underlying type for both.
+   */
+  final class ParameterPosition extends TParameterPosition {
+    /** Gets the underlying integer position, if any. */
+    int getPosition() { this = TPositionalParameterPosition(result) }
+
+    predicate hasPosition() { exists(this.getPosition()) }
+
+    /** Holds if this position represents the `self` position. */
+    predicate isSelf() { this = TSelfParameterPosition() }
+
+    /**
+     * Holds if this position represents a reference to a closure itself. Only
+     * used for tracking flow through captured variables.
+     */
+    predicate isClosureSelf() { this = TClosureSelfParameterPosition() }
+
+    /** Gets a textual representation of this position. */
+    string toString() {
+      result = this.getPosition().toString()
+      or
+      result = "self" and this.isSelf()
+      or
+      result = "closure self" and this.isClosureSelf()
+    }
+
+    ParamBase getParameterIn(ParamList ps) {
+      result = ps.getParam(this.getPosition())
+      or
+      result = ps.getSelfParam() and this.isSelf()
+    }
+  }
+
+  /**
+   * The position of an argument in a call.
+   *
+   * In Rust there is a 1-to-1 correspondence between parameter positions and
+   * arguments positions, so we use the same underlying type for both.
+   */
+  final class ArgumentPosition extends ParameterPosition {
+    /** Gets the argument of `call` at this position, if any. */
+    Expr getArgument(Call call) {
+      result = call.getPositionalArgument(this.getPosition())
+      or
+      result = call.(MethodCall).getReceiver() and this.isSelf()
+    }
+  }
+
   /** Holds if `p` is a parameter of `c` at the position `pos`. */
   predicate isParameterNode(ParameterNode p, DataFlowCallable c, ParameterPosition pos) {
     p.isParameterOf(c, pos)
@@ -449,10 +446,6 @@ module RustDataFlow implements InputSig<Location> {
 
   ContentApprox getContentApprox(Content c) { result = c }
 
-  class ParameterPosition = ParameterPositionAlias;
-
-  class ArgumentPosition = ArgumentPositionAlias;
-
   /**
    * Holds if the parameter position `ppos` matches the argument position
    * `apos`.
@@ -664,7 +657,7 @@ module RustDataFlow implements InputSig<Location> {
   pragma[nomagic]
   additional predicate storeContentStep(Node node1, Content c, Node node2) {
     exists(CallExpr ce, TupleField tf, int pos |
-      node1.asExpr() = ce.getSyntacticArgument(pos) and
+      node1.asExpr() = ce.getSyntacticPositionalArgument(pos) and
       node2.asExpr() = ce and
       c = TTupleFieldContent(tf)
     |
@@ -716,7 +709,7 @@ module RustDataFlow implements InputSig<Location> {
     exists(DataFlowCall call, int i |
       isArgumentNode(node1, call, TPositionalParameterPosition(i)) and
       lambdaCall(call, _, node2.(PostUpdateNode).getPreUpdateNode()) and
-      c.(ClosureCallArgumentContent).getPosition() = i
+      c.(FunctionCallArgumentContent).getPosition() = i
     )
     or
     VariableCapture::storeStep(node1, c, node2)
@@ -825,7 +818,7 @@ module RustDataFlow implements InputSig<Location> {
    */
   predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) {
     (
-      receiver.asExpr() = call.asCall().(ClosureCallExpr).getClosureExpr()
+      receiver.asExpr() = call.asCall().(CallExprImpl::DynamicCallExpr).getFunction()
       or
       call.isSummaryCall(_, receiver.(FlowSummaryNode).getSummaryNode())
     ) and

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

@@ -50,7 +50,7 @@ module Input implements InputSig<Location, RustDataFlow> {
 
   ReturnKind getStandardReturnValueKind() { result = TNormalReturnKind() }
 
-  string encodeParameterPosition(ParameterPosition pos) { result = pos.toString() }
+  string encodeParameterPosition(RustDataFlow::ParameterPosition pos) { result = pos.toString() }
 
   string encodeArgumentPosition(RustDataFlow::ArgumentPosition pos) { result = pos.toString() }
 
@@ -105,7 +105,9 @@ module Input implements InputSig<Location, RustDataFlow> {
   string encodeWithContent(ContentSet c, string arg) { result = "With" + encodeContent(c, arg) }
 
   bindingset[token]
-  ParameterPosition decodeUnknownParameterPosition(AccessPath::AccessPathTokenBase token) {
+  RustDataFlow::ParameterPosition decodeUnknownParameterPosition(
+    AccessPath::AccessPathTokenBase token
+  ) {
     // needed to support `Argument[x..y]` ranges
     token.getName() = "Argument" and
     result.getPosition() = AccessPath::parseInt(token.getAnArgument())
@@ -132,7 +134,7 @@ private module StepsInput implements Impl::Private::StepsInputSig {
 
   /** Gets the argument of `source` described by `sc`, if any. */
   private Expr getSourceNodeArgument(Input::SourceBase source, Impl::Private::SummaryComponent sc) {
-    exists(ArgumentPosition pos |
+    exists(RustDataFlow::ArgumentPosition pos |
       sc = Impl::Private::SummaryComponent::argument(pos) and
       result = pos.getArgument(source.getCall())
     )
@@ -159,7 +161,7 @@ private module StepsInput implements Impl::Private::StepsInputSig {
     s.head() = Impl::Private::SummaryComponent::return(_) and
     result.asExpr() = source.getCall()
     or
-    exists(ArgumentPosition pos, Expr arg |
+    exists(RustDataFlow::ArgumentPosition pos, Expr arg |
       s.head() = Impl::Private::SummaryComponent::parameter(pos) and
       arg = getSourceNodeArgument(source, s.tail().headOfSingleton()) and
       result.asParameter() = getCallable(arg).getParam(pos.getPosition())
@@ -170,7 +172,7 @@ private module StepsInput implements Impl::Private::StepsInputSig {
   }
 
   RustDataFlow::Node getSinkNode(Input::SinkBase sink, Impl::Private::SummaryComponent sc) {
-    exists(ArgsExpr call, Expr arg, ArgumentPosition pos |
+    exists(InvocationExpr call, Expr arg, RustDataFlow::ArgumentPosition pos |
       result.asExpr() = arg and
       sc = Impl::Private::SummaryComponent::argument(pos) and
       call = sink.getCall() and

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

@@ -167,34 +167,34 @@ final class NameNode extends AstNodeNode, TNameNode {
  */
 abstract class ParameterNode extends Node {
   /** Holds if this node is a parameter of `c` at position `pos`. */
-  abstract predicate isParameterOf(DataFlowCallable c, ParameterPosition pos);
+  abstract predicate isParameterOf(DataFlowCallable c, RustDataFlow::ParameterPosition pos);
 }
 
 final class SourceParameterNode extends AstNodeNode, ParameterNode, TSourceParameterNode {
   override ParamBase n;
 
   SourceParameterNode() { this = TSourceParameterNode(n) }
 
-  override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
+  override predicate isParameterOf(DataFlowCallable c, RustDataFlow::ParameterPosition pos) {
     n = pos.getParameterIn(c.asCfgScope().(Callable).getParamList())
   }
 
   /** Get the parameter position of this parameter. */
-  ParameterPosition getPosition() { this.isParameterOf(_, result) }
+  RustDataFlow::ParameterPosition getPosition() { this.isParameterOf(_, result) }
 
   /** Gets the parameter in the CFG that this node corresponds to. */
   ParamBase getParameter() { result = n }
 }
 
 /** A parameter for a library callable with a flow summary. */
 final class SummaryParameterNode extends ParameterNode, FlowSummaryNode {
-  private ParameterPosition pos_;
+  private RustDataFlow::ParameterPosition pos_;
 
   SummaryParameterNode() {
     FlowSummaryImpl::Private::summaryParameterNode(this.getSummaryNode(), pos_)
   }
 
-  override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
+  override predicate isParameterOf(DataFlowCallable c, RustDataFlow::ParameterPosition pos) {
     this.getSummarizedCallable() = c.asSummarizedCallable() and pos = pos_
   }
 }
@@ -210,7 +210,7 @@ final class ClosureParameterNode extends ParameterNode, TClosureSelfReferenceNod
 
   final override CfgScope getCfgScope() { result = cfgScope }
 
-  override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
+  override predicate isParameterOf(DataFlowCallable c, RustDataFlow::ParameterPosition pos) {
     cfgScope = c.asCfgScope() and pos.isClosureSelf()
   }