Rust: Remove borrow logic from Call classes

Author: hvitved

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

@@ -21,7 +21,7 @@ private module Input implements InputSig<Location, RustDataFlow> {
     or
     // We allow flow into post-update node for receiver expressions (from the
     // synthetic post receiever node).
-    n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = any(Node::ReceiverNode r).getReceiver()
+    n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = any(Node::ArgBorrowNode r).getArg()
     or
     n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = getPostUpdateReverseStep(_, _)
     or

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

@@ -136,29 +136,31 @@ final class ParameterPosition extends TParameterPosition {
  */
 final class ArgumentPosition extends TArgumentPosition {
   /** Gets the argument of `call` at this position, if any. */
-  Expr getArgument(Call call) {
+  ExprCfgNode getArgument(CallCfgNode call) {
     this.isReceiver() and
     (
-      result = call.(MethodCallExpr).getReceiver()
+      result = call.(MethodCallExprCfgNode).getReceiver()
       or
-      result = call.(Operation).getOperand(0)
+      result = call.(BinaryExprCfgNode).getLhs()
       or
-      result = call.(IndexExpr).getBase()
+      result = call.(PrefixExprCfgNode).getExpr()
+      or
+      result = call.(IndexExprCfgNode).getBase()
     )
     or
     exists(boolean inMethodCall, int pos | this.getPosition(inMethodCall) = pos |
-      result = call.(CallExpr).getArg(pos) and
+      result = call.(CallExprCfgNode).getArgument(pos) and
       inMethodCall = false
       or
-      result = call.(MethodCallExpr).getArg(pos) and
+      result = call.(MethodCallExprCfgNode).getArgument(pos) and
       inMethodCall = true
       or
-      result = call.(Operation).getOperand(pos + 1) and
-      pos >= 0 and
+      result = call.(BinaryExprCfgNode).getRhs() and
+      pos = 0 and
       inMethodCall = true
       or
-      result = call.(IndexExpr).getIndex() and
-      pos = 1 and
+      result = call.(IndexExprCfgNode).getIndex() and
+      pos = 0 and
       inMethodCall = true
     )
   }
@@ -202,8 +204,7 @@ final class ArgumentPosition extends TArgumentPosition {
 predicate isArgumentForCall(ExprCfgNode arg, CallCfgNode call, ArgumentPosition pos) {
   // TODO: Handle index expressions as calls in data flow.
   not call.getCall() instanceof IndexExpr and
-  arg.getExpr() = pos.getArgument(call.getCall()) and
-  not (pos.isReceiver() and call.getCall().receiverImplicitlyBorrowed())
+  arg = pos.getArgument(call)
 }
 
 /** Provides logic related to SSA. */
@@ -329,14 +330,6 @@ module LocalFlow {
     or
     nodeFrom.asPat().(OrPatCfgNode).getAPat() = nodeTo.asPat()
     or
-    // Simple value step from receiver expression to receiver node, in case
-    // there is no implicit deref or borrow operation.
-    nodeFrom.asExpr() = nodeTo.(ReceiverNode).getReceiver()
-    or
-    // The dual step of the above, for the post-update nodes.
-    nodeFrom.(PostUpdateNode).getPreUpdateNode().(ReceiverNode).getReceiver() =
-      nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr()
-    or
     nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr() =
       getPostUpdateReverseStep(nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr(), true)
   }
@@ -428,8 +421,8 @@ module RustDataFlow implements InputSig<Location> {
     node.(FlowSummaryNode).getSummaryNode().isHidden() or
     node instanceof CaptureNode or
     node instanceof ClosureParameterNode or
-    node instanceof ReceiverNode or
-    node instanceof ReceiverPostUpdateNode
+    node instanceof ArgBorrowNode or
+    node instanceof ArgBorrowPostUpdateNode
   }
 
   predicate neverSkipInPathGraph(Node node) {
@@ -574,16 +567,16 @@ module RustDataFlow implements InputSig<Location> {
   }
 
   pragma[nomagic]
-  private predicate implicitDerefToReceiver(Node node1, ReceiverNode node2, ReferenceContent c) {
+  private predicate implicitDerefToReceiver(Node node1, ArgBorrowNode node2, ReferenceContent c) {
     TypeInference::receiverHasImplicitDeref(node1.asExpr().getExpr()) and
-    node1.asExpr() = node2.getReceiver() and
+    node1.asExpr() = node2.getArg() and
     exists(c)
   }
 
   pragma[nomagic]
-  private predicate implicitBorrowToReceiver(Node node1, ReceiverNode node2, ReferenceContent c) {
-    TypeInference::receiverHasImplicitBorrow(node1.asExpr().getExpr()) and
-    node1.asExpr() = node2.getReceiver() and
+  private predicate implicitBorrowToReceiver(Node node1, ArgBorrowNode node2, ReferenceContent c) {
+    TypeInference::argumentHasImplicitBorrow(node1.asExpr().getExpr()) and
+    node1.asExpr() = node2.getArg() and
     exists(c)
   }
 

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

@@ -28,8 +28,9 @@ module Input implements InputSig<Location, RustDataFlow> {
     }
 
     Expr getArgument(ParameterPosition pos) {
-      exists(ParameterPosition pos0, ArgumentPosition apos |
-        result = apos.getArgument(this.getCall()) and
+      exists(ParameterPosition pos0, ArgumentPosition apos, CallExprBaseCfgNode call |
+        call.getCallExprBase() = this.getCall() and
+        result = apos.getArgument(call).getExpr() and
         RustDataFlow::parameterMatch(pos0, apos)
       |
         not pos.hasPosition(_) and

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

@@ -14,6 +14,7 @@ private import codeql.rust.controlflow.ControlFlowGraph
 private import codeql.rust.controlflow.CfgNodes
 private import codeql.rust.dataflow.Ssa
 private import codeql.rust.dataflow.FlowSummary
+private import codeql.rust.internal.TypeInference as TypeInference
 private import Node as Node
 private import DataFlowImpl
 private import FlowSummaryImpl as FlowSummaryImpl
@@ -238,35 +239,37 @@ final class ExprArgumentNode extends ArgumentNode, ExprNode {
   private CallCfgNode call_;
   private RustDataFlow::ArgumentPosition pos_;
 
-  ExprArgumentNode() { isArgumentForCall(n, call_, pos_) }
+  ExprArgumentNode() {
+    isArgumentForCall(n, call_, pos_) and
+    not TypeInference::argumentHasImplicitBorrow(n.getExpr()) and
+    not TypeInference::receiverHasImplicitDeref(n.getExpr())
+  }
 
   override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
     call.asCallCfgNode() = call_ and pos = pos_
   }
 }
 
 /**
- * The receiver of a method call _after_ any implicit borrow or dereferencing
+ * The argument of a call _after_ any implicit borrow or dereferencing
  * has taken place.
  */
-final class ReceiverNode extends ArgumentNode, TReceiverNode {
-  private CallCfgNode n;
-
-  ReceiverNode() { this = TReceiverNode(n, false) }
+final class ArgBorrowNode extends ArgumentNode, TArgBorrowNode {
+  private ExprCfgNode arg;
 
-  ExprCfgNode getReceiver() { result = n.getReceiver() }
+  ArgBorrowNode() { this = TArgBorrowNode(arg, false) }
 
-  MethodCallExprCfgNode getMethodCall() { result = n }
+  ExprCfgNode getArg() { result = arg }
 
   override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
-    call.asCallCfgNode() = n and pos = TReceiverArgumentPosition()
+    isArgumentForCall(arg, call.asCallCfgNode(), pos)
   }
 
-  override CfgScope getCfgScope() { result = n.getAstNode().getEnclosingCfgScope() }
+  override CfgScope getCfgScope() { result = arg.getAstNode().getEnclosingCfgScope() }
 
-  override Location getLocation() { result = this.getReceiver().getLocation() }
+  override Location getLocation() { result = arg.getLocation() }
 
-  override string toString() { result = "receiver for " + this.getReceiver() }
+  override string toString() { result = "receiver for " + arg }
 }
 
 final class SummaryArgumentNode extends FlowSummaryNode, ArgumentNode {
@@ -414,16 +417,16 @@ final class ExprPostUpdateNode extends PostUpdateNode, TExprPostUpdateNode {
   override Location getLocation() { result = n.getLocation() }
 }
 
-final class ReceiverPostUpdateNode extends PostUpdateNode, TReceiverNode {
-  private CallCfgNode n;
+final class ArgBorrowPostUpdateNode extends PostUpdateNode, TArgBorrowNode {
+  private ExprCfgNode arg;
 
-  ReceiverPostUpdateNode() { this = TReceiverNode(n, true) }
+  ArgBorrowPostUpdateNode() { this = TArgBorrowNode(arg, true) }
 
-  override Node getPreUpdateNode() { result = TReceiverNode(n, false) }
+  override Node getPreUpdateNode() { result = TArgBorrowNode(arg, false) }
 
-  override CfgScope getCfgScope() { result = n.getAstNode().getEnclosingCfgScope() }
+  override CfgScope getCfgScope() { result = arg.getAstNode().getEnclosingCfgScope() }
 
-  override Location getLocation() { result = n.getReceiver().getLocation() }
+  override Location getLocation() { result = arg.getLocation() }
 }
 
 final class SummaryPostUpdateNode extends FlowSummaryNode, PostUpdateNode {
@@ -483,10 +486,15 @@ newtype TNode =
         getPostUpdateReverseStep(any(PostUpdateNode n).getPreUpdateNode().asExpr(), _)
       ]
   } or
-  TReceiverNode(CallCfgNode mc, Boolean isPost) {
-    mc.getCall().receiverImplicitlyBorrowed() and
-    // TODO: Handle index expressions as calls in data flow.
-    not mc.getCall() instanceof IndexExpr
+  TArgBorrowNode(ExprCfgNode arg, Boolean isPost) {
+    exists(Expr e |
+      e = arg.getExpr() and
+      // TODO: Handle index expressions as calls in data flow.
+      not e = any(IndexExpr ie).getBase()
+    |
+      TypeInference::argumentHasImplicitBorrow(e) or
+      TypeInference::receiverHasImplicitDeref(e)
+    )
   } or
   TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
   TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or

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

@@ -34,9 +34,6 @@ module Impl {
    * called in Rust.
    */
   abstract class Call extends ExprImpl::Expr {
-    /** Holds if the receiver of this call is implicitly borrowed. */
-    predicate receiverImplicitlyBorrowed() { this.implicitBorrowAt(TSelfArgumentPosition(), _) }
-
     /** Gets the trait targeted by this call, if any. */
     abstract Trait getTrait();
 
@@ -49,9 +46,6 @@ module Impl {
     /** Gets the argument at the given position, if any. */
     abstract Expr getArgument(ArgumentPosition pos);
 
-    /** Holds if the argument at `pos` might be implicitly borrowed. */
-    abstract predicate implicitBorrowAt(ArgumentPosition pos, boolean certain);
-
     /** Gets the number of arguments _excluding_ any `self` argument. */
     int getNumberOfArguments() { result = count(this.getArgument(TPositionalArgumentPosition(_))) }
 
@@ -115,8 +109,6 @@ module Impl {
 
     override Trait getTrait() { callHasTraitQualifier(this, result) }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) { none() }
-
     override Expr getArgument(ArgumentPosition pos) {
       result = super.getArgList().getArg(pos.asPosition())
     }
@@ -139,8 +131,6 @@ module Impl {
 
     override Trait getTrait() { callHasTraitQualifier(this, result) }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) { none() }
-
     override Expr getArgument(ArgumentPosition pos) {
       pos.isSelf() and result = super.getArgList().getArg(0)
       or
@@ -153,10 +143,6 @@ module Impl {
 
     override Trait getTrait() { none() }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
-      pos.isSelf() and certain = false
-    }
-
     override Expr getArgument(ArgumentPosition pos) {
       pos.isSelf() and result = this.(MethodCallExpr).getReceiver()
       or
@@ -175,15 +161,6 @@ module Impl {
 
     override Trait getTrait() { result = trait }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
-      (
-        pos.isSelf() and borrows >= 1
-        or
-        pos.asPosition() = 0 and borrows = 2
-      ) and
-      certain = true
-    }
-
     override Expr getArgument(ArgumentPosition pos) {
       pos.isSelf() and result = super.getOperand(0)
       or
@@ -196,10 +173,6 @@ module Impl {
 
     override Trait getTrait() { result.getCanonicalPath() = "core::ops::index::Index" }
 
-    override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
-      pos.isSelf() and certain = true
-    }
-
     override Expr getArgument(ArgumentPosition pos) {
       pos.isSelf() and result = super.getBase()
       or

rust/ql/lib/codeql/rust/internal/TypeInference.qll

@@ -1610,9 +1610,9 @@ private module MethodResolution {
       receiver = this.getArgument(CallImpl::TSelfArgumentPosition())
     }
 
-    predicate receiverHasImplicitBorrow(AstNode receiver) {
+    predicate argumentHasImplicitBorrow(AstNode arg) {
       exists(this.resolveCallTarget(_, "", true)) and
-      receiver = this.getArgument(CallImpl::TSelfArgumentPosition())
+      arg = this.getArgument(CallImpl::TSelfArgumentPosition())
     }
   }
 
@@ -1703,8 +1703,16 @@ private module MethodResolution {
 
     override Expr getArgument(ArgumentPosition pos) { result = this.(Call).getArgument(pos) }
 
+    private predicate implicitBorrowAt(ArgumentPosition pos) {
+      exists(int borrows | this.isOverloaded(_, _, borrows) |
+        pos.isSelf() and borrows >= 1
+        or
+        pos.asPosition() = 0 and borrows = 2
+      )
+    }
+
     override Type getArgumentTypeAt(ArgumentPosition pos, TypePath path) {
-      if this.(Call).implicitBorrowAt(pos, true)
+      if this.implicitBorrowAt(pos)
       then
         result = TRefType() and
         path.isEmpty()
@@ -1716,10 +1724,10 @@ private module MethodResolution {
       else result = inferType(this.getArgument(pos), path)
     }
 
-    override predicate receiverHasImplicitBorrow(AstNode receiver) {
+    override predicate argumentHasImplicitBorrow(AstNode arg) {
       exists(ArgumentPosition pos |
-        this.(Call).implicitBorrowAt(pos, true) and
-        receiver = this.getArgument(pos)
+        this.implicitBorrowAt(pos) and
+        arg = this.getArgument(pos)
       )
     }
 
@@ -3466,10 +3474,10 @@ private module Cached {
     any(MethodResolution::MethodCall mc).receiverHasImplicitDeref(receiver)
   }
 
-  /** Holds if `receiver` is the receiver of a method call with an implicit borrow. */
+  /** Holds if `arg` is the argument of a call with an implicit borrow. */
   cached
-  predicate receiverHasImplicitBorrow(AstNode receiver) {
-    any(MethodResolution::MethodCall mc).receiverHasImplicitBorrow(receiver)
+  predicate argumentHasImplicitBorrow(AstNode arg) {
+    any(MethodResolution::MethodCall mc).argumentHasImplicitBorrow(arg)
   }
 
   /** Gets an item (function or tuple struct/variant) that `call` resolves to, if any. */