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()
@@ -1284,6 +1289,14 @@ private module MethodCallResolution {
           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,13 +1307,7 @@ 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)
@@ -1318,8 +1325,7 @@ 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)
@@ -1463,12 +1469,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()
@@ -2314,18 +2315,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(...) |