wip4

Author: hvitved

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

@@ -1123,17 +1123,27 @@ private Function getMethodSuccessor(ItemNode item, string name, int arity) {
   arity = result.getParamList().getNumberOfParams()
 }
 
+/**
+ * Holds if the type path `path` pointing to `type` is either empty, in which
+ * case `type` is not `&`, or `path` points to a type being referenced by `&`.
+ */
+bindingset[path, type]
+predicate isRefStrippedRoot(TypePath path, Type type) {
+  path.isEmpty() and
+  type != TRefType()
+  or
+  path = TypePath::singleton(TRefTypeParameter()) and
+  exists(type)
+}
+
 /** Provides logic for resolving method calls. */
 private module MethodCallResolution {
   /**
    * Holds if method `f` with the name `name` and the arity `arity` exists in
    * `i`, and the type of the `self` parameter is `selfType`.
    *
-   * TODO
-   * `rootType` is the root type of `selfType`, and `selfRootPath` points to
-   * `selfRootType` inside `selfType`, where `selfRootType` is either the type
-   * being implemented, when `i` is an `impl`, or the trait itself when `i` is
-   * a trait.
+   * `rootTypePath` points to the type `rootType` inside `selfType`, which is
+   * the (possibly `&`-stripped) root type of `selfType`.
    */
   pragma[nomagic]
   predicate methodInfo(
@@ -1144,12 +1154,7 @@ private module MethodCallResolution {
       f = i.getASuccessor(name) and
       arity = f.getParamList().getNumberOfParams() and
       rootType = selfType.getTypeAt(rootTypePath) and
-      (
-        rootTypePath.isEmpty() and
-        rootType != TRefType()
-        or
-        rootTypePath = TypePath::singleton(TRefTypeParameter())
-      ) and
+      isRefStrippedRoot(rootTypePath, rootType) and
       selfType.appliesTo(f, pos, i) and
       pos.isSelf() and
       not i.(ImplItemNode).isBlanket()
@@ -1275,15 +1280,24 @@ private module MethodCallResolution {
     }
 
     /**
-     * Holds if the method inside `i` with matching name and arity can be ruled out
-     * as a candidate for the receiver type represented by `derefChainBorrow`.
+     * Holds if the candidate method inside `i` with matching name and arity can
+     * be ruled out as a target of this call, because the receiver type represented
+     * by `derefChainBorrow` is incompatible with the `self` parameter type.
      */
     pragma[nomagic]
-    private predicate isNotCandidate(ImplOrTraitItemNode i, string derefChainBorrow) {
+    private predicate isIncompatibleCandidate(ImplOrTraitItemNode i, string derefChainBorrow) {
       IsInstantiationOf<MethodCallCand, FunctionType, MethodCallReceiverIsInstantiationOfInput>::isNotInstantiationOf(MkMethodCallCand(this,
           derefChainBorrow), i, _)
     }
 
+    pragma[nomagic]
+    private Type getACandidateReceiverRootTypeAtSubstituteTraitBounds(
+      TypePath rootTypePath, string derefChainBorrow
+    ) {
+      result = this.getACandidateReceiverTypeAtSubstituteTraitBounds(rootTypePath, derefChainBorrow) and
+      isRefStrippedRoot(rootTypePath, result)
+    }
+
     /**
      * Holds if the candidate receiver type represented by `derefChain` does not
      * have a matching method target.
@@ -1294,16 +1308,10 @@ private module MethodCallResolution {
         derefChainBorrow = derefChain + ";" and
         not derefChain.matches("%.ref") and // no need to try a borrow if the last thing we did was a deref
         rootType =
-          this.getACandidateReceiverTypeAtSubstituteTraitBounds(rootTypePath, derefChainBorrow) and
-        (
-          rootTypePath.isEmpty() and
-          rootType != TRefType()
-          or
-          rootTypePath = TypePath::singleton(TRefTypeParameter())
-        )
+          this.getACandidateReceiverRootTypeAtSubstituteTraitBounds(rootTypePath, derefChainBorrow)
       |
         forall(ImplOrTraitItemNode i | methodCallCandidate(this, i, _, rootTypePath, rootType) |
-          this.isNotCandidate(i, derefChainBorrow)
+          this.isIncompatibleCandidate(i, derefChainBorrow)
         )
       )
     }
@@ -1318,11 +1326,10 @@ private module MethodCallResolution {
         derefChainBorrow = derefChain + ";borrow" and
         this.noCandidateReceiverTypeAtNoBorrow(derefChain) and
         rootType =
-          this.getACandidateReceiverTypeAtSubstituteTraitBounds(rootTypePath, derefChainBorrow) and
-        rootTypePath = TypePath::singleton(TRefTypeParameter())
+          this.getACandidateReceiverRootTypeAtSubstituteTraitBounds(rootTypePath, derefChainBorrow)
       |
         forall(ImplOrTraitItemNode i | methodCallCandidate(this, i, _, rootTypePath, rootType) |
-          this.isNotCandidate(i, derefChainBorrow)
+          this.isIncompatibleCandidate(i, derefChainBorrow)
         )
       )
     }
@@ -1463,12 +1470,7 @@ private module MethodCallResolution {
     private predicate hasNoInherentTarget() {
       exists(TypePath rootTypePath, Type rootType, string name, int arity |
         this.isMethodCall(_, rootTypePath, rootType, name, arity) and
-        (
-          rootTypePath.isEmpty() and
-          rootType != TRefType()
-          or
-          rootTypePath = TypePath::singleton(TRefTypeParameter())
-        ) and
+        isRefStrippedRoot(rootTypePath, rootType) and
         forall(Impl i |
           methodInfo(_, name, arity, i, _, rootTypePath, rootType) and
           not i.hasTrait()
@@ -1663,8 +1665,8 @@ private module MethodCallMatchingInput implements MatchingWithEnvironmentInputSi
     }
 
     pragma[nomagic]
-    private Type getInferredSelfType(AccessEnvironment state, TypePath path) {
-      result = this.getACandidateReceiverTypeAt(path, state)
+    private Type getInferredSelfType(AccessEnvironment env, TypePath path) {
+      result = this.getACandidateReceiverTypeAt(path, env)
     }
 
     pragma[nomagic]
@@ -1689,16 +1691,16 @@ private module MethodCallMatchingInput implements MatchingWithEnvironmentInputSi
     }
 
     pragma[nomagic]
-    Type getInferredType(AccessEnvironment state, AccessPosition apos, TypePath path) {
+    Type getInferredType(AccessEnvironment env, AccessPosition apos, TypePath path) {
       apos.asArgumentPosition().isSelf() and
-      result = this.getInferredSelfType(state, path)
+      result = this.getInferredSelfType(env, path)
       or
       result = this.getInferredNonSelfType(apos, path) and
-      exists(this.getTarget(state))
+      exists(this.getTarget(env))
     }
 
-    Declaration getTarget(AccessEnvironment state) {
-      result = this.resolveCallTarget(state) // mutual recursion; resolving method calls requires resolving types and vice versa
+    Declaration getTarget(AccessEnvironment env) {
+      result = this.resolveCallTarget(env) // mutual recursion; resolving method calls requires resolving types and vice versa
     }
   }
 
@@ -1712,11 +1714,11 @@ private module MethodCallMatching = MatchingWithEnvironment<MethodCallMatchingIn
 pragma[nomagic]
 private Type inferMethodCallExprType0(
   MethodCallMatchingInput::Access a, MethodCallMatchingInput::AccessPosition apos, AstNode n,
-  string state, TypePath path
+  string env, TypePath path
 ) {
   exists(TypePath path0 |
     n = a.getNodeAt(apos) and
-    result = MethodCallMatching::inferAccessType(a, state, apos, path0)
+    result = MethodCallMatching::inferAccessType(a, env, apos, path0)
   |
     if
       // index expression `x[i]` desugars to `*x.index(i)`, so we must account for
@@ -1735,26 +1737,26 @@ private Type inferMethodCallExprType0(
 pragma[nomagic]
 private Type inferMethodCallExprType(AstNode n, TypePath path) {
   exists(
-    MethodCallMatchingInput::Access a, MethodCallMatchingInput::AccessPosition apos, string state,
+    MethodCallMatchingInput::Access a, MethodCallMatchingInput::AccessPosition apos, string env,
     TypePath path0
   |
-    result = inferMethodCallExprType0(a, apos, n, state, path0)
+    result = inferMethodCallExprType0(a, apos, n, env, path0)
   |
     (
       not apos.asArgumentPosition().isSelf()
       or
-      state = ";"
+      env = ";"
     ) and
     path = path0
     or
     // adjust for implicit deref
     apos.asArgumentPosition().isSelf() and
-    state = ".ref;" and
+    env = ".ref;" and
     path = TypePath::cons(TRefTypeParameter(), path0)
     or
     // adjust for implicit borrow
     apos.asArgumentPosition().isSelf() and
-    state = ";borrow" and
+    env = ";borrow" and
     path0.isCons(TRefTypeParameter(), path)
   )
 }
@@ -2314,18 +2316,13 @@ private Type inferOperationType(AstNode n, TypePath path) {
   )
 }
 
-pragma[inline]
-private Type inferRootTypeDeref(AstNode n) {
-  result = inferType(n) and
-  result != TRefType()
-  or
-  // for reference types, lookup members in the type being referenced
-  result = inferType(n, TypePath::singleton(TRefTypeParameter()))
-}
-
 pragma[nomagic]
 private Type getFieldExprLookupType(FieldExpr fe, string name) {
-  result = inferRootTypeDeref(fe.getContainer()) and name = fe.getIdentifier().getText()
+  exists(TypePath path |
+    result = inferType(fe.getContainer(), path) and
+    name = fe.getIdentifier().getText() and
+    isRefStrippedRoot(path, result)
+  )
 }
 
 pragma[nomagic]

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

@@ -1,4 +1,2 @@
 multipleCallTargets
-| main.rs:322:14:322:33 | ... .cmp(...) |
-| main.rs:334:9:334:28 | ... .cmp(...) |
 | main.rs:362:14:362:30 | ... .lt(...) |

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

@@ -11,8 +11,6 @@ multipleCallTargets
 | test.rs:179:30:179:68 | ...::_print(...) |
 | test.rs:188:26:188:105 | ...::_print(...) |
 | test.rs:229:22:229:72 | ... .read_to_string(...) |
-| test.rs:639:26:639:43 | file1.chain(...) |
-| test.rs:647:26:647:40 | file1.take(...) |
 | test.rs:697:18:697:38 | ...::_print(...) |
 | test.rs:702:18:702:45 | ...::_print(...) |
 | test.rs:720:38:720:42 | ...::_print(...) |

rust/ql/test/library-tests/dataflow/sources/test.rs

@@ -638,15 +638,15 @@ async fn test_tokio_file() -> std::io::Result<()> {
         let file2 = tokio::fs::File::open("another_file.txt").await?; // $ Alert[rust/summary/taint-sources]
         let mut reader = file1.chain(file2);
         reader.read_to_string(&mut buffer).await?;
-        sink(&buffer); // $ hasTaintFlow="file.txt" hasTaintFlow="another_file.txt"
+        sink(&buffer); // $ MISSING: hasTaintFlow="file.txt" hasTaintFlow="another_file.txt" -- we cannot resolve the `chain` and `read_to_string` calls above, which comes from `impl<R: AsyncRead + ?Sized> AsyncReadExt for R {}` in `async_read_ext.rs`
     }
 
     {
         let mut buffer = String::new();
         let file1 = tokio::fs::File::open("file.txt").await?; // $ Alert[rust/summary/taint-sources]
         let mut reader = file1.take(100);
         reader.read_to_string(&mut buffer).await?;
-        sink(&buffer); // $ hasTaintFlow="file.txt"
+        sink(&buffer); // $ MISSING: hasTaintFlow="file.txt" -- we cannot resolve the `take` and `read_to_string` calls above, which comes from `impl<R: AsyncRead + ?Sized> AsyncReadExt for R {}` in `async_read_ext.rs`
     }
 
     Ok(())

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

@@ -5,7 +5,6 @@ multipleCallTargets
 | dereference.rs:184:17:184:30 | ... .foo() |
 | dereference.rs:186:17:186:25 | S.bar(...) |
 | dereference.rs:187:17:187:29 | S.bar(...) |
-| main.rs:1803:13:1803:63 | ... .partial_cmp(...) |
 | main.rs:2318:9:2318:34 | ...::my_from2(...) |
 | main.rs:2319:9:2319:33 | ...::my_from2(...) |
 | main.rs:2320:9:2320:38 | ...::my_from2(...) |