wip

Author: hvitved

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

@@ -28,11 +28,7 @@ module Impl {
    */
   class CallExprBase extends Generated::CallExprBase {
     /** Gets the static target of this call, if any. */
-    final Callable getStaticTarget() {
-      result = TypeInference::resolveMethodCallTarget(this)
-      or
-      result = TypeInference::resolveCallTarget(this)
-    }
+    final Function getStaticTarget() { result = TypeInference::resolveCallTarget(this) }
 
     override Expr getArg(int index) { result = this.getArgList().getArg(index) }
   }

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

@@ -59,11 +59,7 @@ module Impl {
     Expr getReceiver() { result = this.getArgument(TSelfArgumentPosition()) }
 
     /** Gets the static target of this call, if any. */
-    Function getStaticTarget() {
-      result = TypeInference::resolveMethodCallTarget(this)
-      or
-      result = this.(CallExpr).getStaticTarget()
-    }
+    Function getStaticTarget() { result = TypeInference::resolveCallTarget(this) }
 
     /** Gets a runtime target of this call, if any. */
     pragma[nomagic]
@@ -78,18 +74,21 @@ module Impl {
   }
 
   /** Holds if the call expression dispatches to a method. */
-  private predicate callIsMethodCall(CallExpr call, Path qualifier, string methodName) {
+  private predicate callIsMethodCall(CallExpr call, Path qualifier, string methodName, boolean isRef) {
     exists(Path path, Function f |
       path = call.getFunction().(PathExpr).getPath() and
       f = resolvePath(path) and
-      f.getParamList().hasSelfParam() and
       qualifier = path.getQualifier() and
-      path.getSegment().getIdentifier().getText() = methodName
+      path.getSegment().getIdentifier().getText() = methodName and
+      exists(SelfParam self |
+        self = f.getParamList().getSelfParam() and
+        if self.isRef() then isRef = true else isRef = false
+      )
     )
   }
 
   private class CallExprCall extends Call instanceof CallExpr {
-    CallExprCall() { not callIsMethodCall(this, _, _) }
+    CallExprCall() { not callIsMethodCall(this, _, _, _) }
 
     override string getMethodName() { none() }
 
@@ -102,11 +101,19 @@ module Impl {
     }
   }
 
-  private class CallExprMethodCall extends Call instanceof CallExpr {
+  class CallExprMethodCall extends Call instanceof CallExpr {
     Path qualifier;
     string methodName;
+    boolean isRef;
+
+    CallExprMethodCall() { callIsMethodCall(this, qualifier, methodName, isRef) }
 
-    CallExprMethodCall() { callIsMethodCall(this, qualifier, methodName) }
+    /**
+     * Holds if this call has an explicit borrow for the `self` argument,
+     * which is needed for `&self` parameters when method are called using
+     * function call syntax.
+     */
+    predicate hasExplicitSelfBorrow() { isRef = true }
 
     override string getMethodName() { result = methodName }
 

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

@@ -770,9 +770,7 @@ private module CallExprBaseMatchingInput implements MatchingInputSig {
     Declaration getTarget() {
       result = resolveMethodCallTarget(this) // mutual recursion; resolving method calls requires resolving types and vice versa
       or
-      result = resolveCallTargetSimple(this)
-      or
-      result = resolveCallTargetComplex(this) // mutual recursion
+      result = resolveFunctionCallTarget(this) // potential mutual recursion; resolving some associated function calls requires resolving types
     }
   }
 
@@ -1224,12 +1222,22 @@ private Type inferForLoopExprType(AstNode n, TypePath path) {
 final class MethodCall extends Call {
   MethodCall() { exists(this.getReceiver()) }
 
+  private Type getReceiverTypeAt(TypePath path) {
+    result = inferType(super.getReceiver(), path)
+    or
+    exists(PathExpr pe, TypeMention tm |
+      pe = this.(CallExpr).getFunction() and
+      tm = pe.getPath().getQualifier() and
+      result = tm.resolveTypeAt(path)
+    )
+  }
+
   /** Gets the type of the receiver of the method call at `path`. */
   Type getTypeAt(TypePath path) {
     if this.receiverImplicitlyBorrowed()
     then
       exists(TypePath path0, Type t0 |
-        t0 = inferType(super.getReceiver(), path0) and
+        t0 = this.getReceiverTypeAt(path0) and
         (
           path0.isCons(TRefTypeParameter(), path)
           or
@@ -1257,10 +1265,21 @@ final class MethodCall extends Call {
         t0.(StructType).asItemNode() instanceof StringStruct and
         result.(StructType).asItemNode() instanceof Builtins::Str
       )
-    else result = inferType(super.getReceiver(), path)
+    else
+      if this.(CallImpl::CallExprMethodCall).hasExplicitSelfBorrow()
+      then
+        exists(TypePath path0 |
+          result = this.getReceiverTypeAt(path0) and
+          path0.isCons(TRefTypeParameter(), path)
+        )
+      else result = this.getReceiverTypeAt(path)
   }
 }
 
+final private class FunctionCallExpr extends CallExpr {
+  FunctionCallExpr() { not this instanceof MethodCall }
+}
+
 /**
  * Holds if a method for `type` with the name `name` and the arity `arity`
  * exists in `impl`.
@@ -1500,15 +1519,28 @@ private Function getTraitMethod(ImplTraitReturnType trait, string name) {
   result = getMethodSuccessor(trait.getImplTraitTypeRepr(), name)
 }
 
+pragma[nomagic]
+private Function resolveMethodCallTarget(MethodCall mc) {
+  // The method comes from an `impl` block targeting the type of the receiver.
+  result = getMethodFromImpl(mc)
+  or
+  // The type of the receiver is a type parameter and the method comes from a
+  // trait bound on the type parameter.
+  result = getTypeParameterMethod(mc.getTypeAt(TypePath::nil()), mc.getMethodName())
+  or
+  // The type of the receiver is an `impl Trait` type.
+  result = getTraitMethod(mc.getTypeAt(TypePath::nil()), mc.getMethodName())
+}
+
 pragma[nomagic]
 private predicate assocFuncResolutionDependsOnArgument(Function f, Impl impl, int pos) {
   methodResolutionDependsOnArgument(impl, _, f, pos, _, _)
 }
 
-private class AssocFuncCallExpr extends CallExpr {
+private class AssocFunctionCallExpr extends FunctionCallExpr {
   private int pos;
 
-  AssocFuncCallExpr() {
+  AssocFunctionCallExpr() {
     assocFuncResolutionDependsOnArgument(CallExprImpl::getResolvedFunction(this), _, pos)
   }
 
@@ -1524,11 +1556,11 @@ private class AssocFuncCallExpr extends CallExpr {
 }
 
 private module AssocFuncIsInstantiationOfInput implements
-  IsInstantiationOfInputSig<AssocFuncCallExpr>
+  IsInstantiationOfInputSig<AssocFunctionCallExpr>
 {
   pragma[nomagic]
   predicate potentialInstantiationOf(
-    AssocFuncCallExpr ce, TypeAbstraction impl, TypeMention constraint
+    AssocFunctionCallExpr ce, TypeAbstraction impl, TypeMention constraint
   ) {
     exists(Function cand |
       cand = ce.getACandidate(impl) and
@@ -1537,21 +1569,34 @@ private module AssocFuncIsInstantiationOfInput implements
   }
 }
 
+/**
+ * Gets the target of `call`, where resolution does not rely on type inference.
+ */
 pragma[nomagic]
-ItemNode resolveCallTargetSimple(CallExpr ce) {
-  result = CallExprImpl::getResolvedFunction(ce) and
+private ItemNode resolveFunctionCallTargetSimple(FunctionCallExpr call) {
+  result = CallExprImpl::getResolvedFunction(call) and
   not assocFuncResolutionDependsOnArgument(result, _, _)
 }
 
+/**
+ * Gets the target of `call`, where resolution relies on type inference.
+ */
 pragma[nomagic]
-Function resolveCallTargetComplex(AssocFuncCallExpr ce) {
+private Function resolveFunctionCallTargetComplex(AssocFunctionCallExpr call) {
   exists(Impl impl |
-    IsInstantiationOf<AssocFuncCallExpr, AssocFuncIsInstantiationOfInput>::isInstantiationOf(ce,
+    IsInstantiationOf<AssocFunctionCallExpr, AssocFuncIsInstantiationOfInput>::isInstantiationOf(call,
       impl, _) and
-    result = getMethodSuccessor(impl, ce.getACandidate(_).getName().getText())
+    result = getMethodSuccessor(impl, call.getACandidate(_).getName().getText())
   )
 }
 
+pragma[inline]
+private ItemNode resolveFunctionCallTarget(FunctionCallExpr call) {
+  result = resolveFunctionCallTargetSimple(call)
+  or
+  result = resolveFunctionCallTargetComplex(call)
+}
+
 cached
 private module Cached {
   private import codeql.rust.internal.CachedStages
@@ -1580,26 +1625,12 @@ private module Cached {
     )
   }
 
-  /** Gets a method that the method call `mc` resolves to, if any. */
-  cached
-  Function resolveMethodCallTarget(MethodCall mc) {
-    // The method comes from an `impl` block targeting the type of the receiver.
-    result = getMethodFromImpl(mc)
-    or
-    // The type of the receiver is a type parameter and the method comes from a
-    // trait bound on the type parameter.
-    result = getTypeParameterMethod(mc.getTypeAt(TypePath::nil()), mc.getMethodName())
-    or
-    // The type of the receiver is an `impl Trait` type.
-    result = getTraitMethod(mc.getTypeAt(TypePath::nil()), mc.getMethodName())
-  }
-
-  /** Gets a method that the method call `mc` resolves to, if any. */
+  /** Gets a function that `call` resolves to, if any. */
   cached
-  Function resolveCallTarget(CallExpr ce) {
-    result = resolveCallTargetSimple(ce)
+  Function resolveCallTarget(Call call) {
+    result = resolveMethodCallTarget(call)
     or
-    result = resolveCallTargetComplex(ce)
+    result = resolveFunctionCallTarget(call)
   }
 
   pragma[inline]
@@ -1736,8 +1767,8 @@ private module Debug {
   private Locatable getRelevantLocatable() {
     exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
       result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
-      filepath.matches("%/sqlx.rs") and
-      startline = [56 .. 60]
+      filepath.matches("%/main.rs") and
+      startline = 120
     )
   }
 

rust/ql/test/extractor-tests/macro-expansion/CONSISTENCY/PathResolutionConsistency.expected

@@ -1,15 +1,10 @@
 multipleCallTargets
 | proc_macro.rs:15:5:17:5 | ...::new(...) |
-| proc_macro.rs:16:12:16:16 | ...::to_tokens(...) |
 | proc_macro.rs:25:5:28:5 | ...::new(...) |
-| proc_macro.rs:26:10:26:12 | ...::to_tokens(...) |
-| proc_macro.rs:27:10:27:16 | ...::to_tokens(...) |
 | proc_macro.rs:41:5:49:5 | ...::new(...) |
 | proc_macro.rs:41:5:49:5 | ...::new(...) |
 | proc_macro.rs:41:5:49:5 | ...::new(...) |
 | proc_macro.rs:41:5:49:5 | ...::new(...) |
-| proc_macro.rs:42:16:42:26 | ...::to_tokens(...) |
 | proc_macro.rs:44:27:44:30 | ...::to_tokens(...) |
-| proc_macro.rs:46:18:46:28 | ...::to_tokens(...) |
 multiplePathResolutions
 | macro_expansion.rs:1:5:1:14 | proc_macro |

rust/ql/test/library-tests/dataflow/global/CONSISTENCY/PathResolutionConsistency.expected

@@ -1,6 +1,8 @@
 multipleCallTargets
 | test.rs:98:14:98:43 | ...::_print(...) |
 | test.rs:109:14:109:33 | ...::_print(...) |
+| test.rs:112:62:112:77 | ...::from(...) |
+| test.rs:119:58:119:73 | ...::from(...) |
 | test.rs:135:22:135:43 | ...::_print(...) |
 | test.rs:140:22:140:43 | ...::_print(...) |
 | test.rs:144:22:144:44 | ...::_print(...) |
@@ -30,6 +32,8 @@ multipleCallTargets
 | test.rs:737:30:737:43 | ...::_print(...) |
 | test.rs:752:14:752:43 | ...::_print(...) |
 | test.rs:766:14:766:34 | ...::_print(...) |
+| test.rs:806:50:806:66 | ...::from(...) |
+| test.rs:806:50:806:66 | ...::from(...) |
 | test.rs:808:14:808:31 | ...::_print(...) |
 | test.rs:811:14:811:31 | ...::_print(...) |
 | test.rs:814:14:814:31 | ...::_print(...) |
@@ -75,13 +79,5 @@ multipleCallTargets
 | test_futures_io.rs:93:26:93:63 | pinned.poll_read(...) |
 | test_futures_io.rs:116:22:116:50 | pinned.poll_fill_buf(...) |
 | test_futures_io.rs:145:26:145:49 | ...::with_capacity(...) |
-| web_frameworks.rs:40:5:40:26 | ...::next_key::<...>(...) |
-| web_frameworks.rs:40:5:40:26 | ...::next_value::<...>(...) |
-| web_frameworks.rs:40:5:40:26 | ...::write_str(...) |
-| web_frameworks.rs:40:5:40:26 | ...::write_str(...) |
-| web_frameworks.rs:42:9:42:17 | ...::next_element::<...>(...) |
-| web_frameworks.rs:42:9:42:17 | ...::next_value::<...>(...) |
-| web_frameworks.rs:43:9:43:17 | ...::next_element::<...>(...) |
-| web_frameworks.rs:43:9:43:17 | ...::next_value::<...>(...) |
 | web_frameworks.rs:101:14:101:23 | a.as_str() |
 | web_frameworks.rs:102:14:102:25 | a.as_bytes() |

rust/ql/test/library-tests/dataflow/sources/CONSISTENCY/PathResolutionConsistency.expected

@@ -0,0 +1,2 @@
+| test_futures_io.rs:46:40:46:60 | //... | Missing result: hasTaintFlow=url |
+| test_futures_io.rs:104:40:104:60 | //... | Missing result: hasTaintFlow=url |

rust/ql/test/library-tests/dataflow/sources/InlineFlow.expected

@@ -1,7 +1,6 @@
 multipleCallTargets
 | main.rs:118:9:118:11 | f(...) |
 | proc_macro.rs:9:5:11:5 | ...::new(...) |
-| proc_macro.rs:10:10:10:12 | ...::to_tokens(...) |
 multiplePathResolutions
 | main.rs:626:3:626:12 | proc_macro |
 | main.rs:632:7:632:16 | proc_macro |

rust/ql/test/library-tests/path-resolution/CONSISTENCY/PathResolutionConsistency.expected

@@ -1,3 +1,8 @@
 multipleCallTargets
 | dereference.rs:61:15:61:24 | e1.deref() |
-| main.rs:120:17:120:40 | ...::trait_method(...) |
+| main.rs:2145:13:2145:31 | ...::from(...) |
+| main.rs:2146:13:2146:31 | ...::from(...) |
+| main.rs:2147:13:2147:31 | ...::from(...) |
+| main.rs:2153:13:2153:31 | ...::from(...) |
+| main.rs:2154:13:2154:31 | ...::from(...) |
+| main.rs:2155:13:2155:31 | ...::from(...) |

rust/ql/test/library-tests/type-inference/CONSISTENCY/PathResolutionConsistency.expected

@@ -117,7 +117,7 @@ mod trait_impl {
         let a = x.trait_method(); // $ type=a:bool method=MyThing::trait_method
 
         let y = MyThing { field: false };
-        let b = MyTrait::trait_method(y); // $ type=b:bool method=MyThing::trait_method $ SPURIOUS: method=trait_method
+        let b = MyTrait::trait_method(y); // $ type=b:bool method=MyThing::trait_method
     }
 }