Merge pull request #20969 from paldepind/rust/dispath-default-trait

Rust: Do not dispatch to all implementations when trait target is accurate

Author: paldepind

rust/ql/lib/change-notes/2025-12-05-trait-function-dispatch.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The call graph is now more precise for calls that target a trait function with a default implemention. This reduces the number of false positives for data flow queries.

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

@@ -63,7 +63,7 @@ module Impl {
     }
 
     /** Gets the resolved target of this call, if any. */
-    Function getStaticTarget() { result = TypeInference::resolveCallTarget(this) }
+    Function getStaticTarget() { result = TypeInference::resolveCallTarget(this, _) }
 
     /** Gets the name of the function called, if any. */
     string getTargetName() { result = this.getStaticTarget().getName().getText() }
@@ -73,9 +73,9 @@ module Impl {
     Function getARuntimeTarget() {
       result.hasImplementation() and
       (
-        result = this.getStaticTarget()
+        result = TypeInference::resolveCallTarget(this, _)
         or
-        result.implements(this.getStaticTarget())
+        result.implements(TypeInference::resolveCallTarget(this, true))
       )
     }
   }

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

@@ -94,6 +94,6 @@ module Impl {
     }
 
     /** Gets the resolved target (function or tuple struct/variant), if any. */
-    Addressable getResolvedTarget() { result = TypeInference::resolveCallTarget(this) }
+    Addressable getResolvedTarget() { result = TypeInference::resolveCallTarget(this, _) }
   }
 }

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

@@ -3522,12 +3522,27 @@ private module Cached {
     any(MethodResolution::MethodCall mc).argumentHasImplicitBorrow(n)
   }
 
-  /** Gets an item (function or tuple struct/variant) that `call` resolves to, if any. */
+  /**
+   * Gets an item (function or tuple struct/variant) that `call` resolves to, if
+   * any.
+   *
+   * The parameter `dispatch` is `true` if and only if the resolved target is a
+   * trait item because a precise target could not be determined from the
+   * types (for instance in the presence of generics or `dyn` types)
+   */
   cached
-  Addressable resolveCallTarget(Expr call) {
-    result = call.(MethodResolution::MethodCall).resolveCallTarget(_, _, _)
+  Addressable resolveCallTarget(InvocationExpr call, boolean dispatch) {
+    dispatch = false and
+    result = call.(NonMethodResolution::NonMethodCall).resolveCallTargetViaPathResolution()
     or
-    result = call.(NonMethodResolution::NonMethodCall).resolveCallTarget()
+    exists(ImplOrTraitItemNode i |
+      i instanceof TraitItemNode and dispatch = true
+      or
+      i instanceof ImplItemNode and dispatch = false
+    |
+      result = call.(MethodResolution::MethodCall).resolveCallTarget(i, _, _) or
+      result = call.(NonMethodResolution::NonMethodCall).resolveCallTargetViaTypeInference(i)
+    )
   }
 
   /**
@@ -3668,9 +3683,9 @@ private module Debug {
     result = inferType(n, path)
   }
 
-  Addressable debugResolveCallTarget(InvocationExpr c) {
+  Addressable debugResolveCallTarget(InvocationExpr c, boolean dispatch) {
     c = getRelevantLocatable() and
-    result = resolveCallTarget(c)
+    result = resolveCallTarget(c, dispatch)
   }
 
   predicate debugConditionSatisfiesConstraint(

rust/ql/test/library-tests/dataflow/global/inline-flow.expected

@@ -187,6 +187,27 @@ edges
 | main.rs:334:9:334:9 | a | main.rs:335:10:335:10 | a | provenance |  |
 | main.rs:334:13:334:55 | ...::block_on(...) | main.rs:334:9:334:9 | a | provenance |  |
 | main.rs:334:41:334:54 | async_source(...) | main.rs:334:13:334:55 | ...::block_on(...) | provenance | MaD:2 |
+| main.rs:346:44:348:9 | { ... } | main.rs:383:18:383:38 | t.get_double_number() | provenance |  |
+| main.rs:346:44:348:9 | { ... } | main.rs:387:18:387:50 | ...::get_double_number(...) | provenance |  |
+| main.rs:347:13:347:29 | self.get_number() | main.rs:346:44:348:9 | { ... } | provenance |  |
+| main.rs:350:33:352:9 | { ... } | main.rs:391:18:391:37 | ...::get_default(...) | provenance |  |
+| main.rs:351:13:351:21 | source(...) | main.rs:350:33:352:9 | { ... } | provenance |  |
+| main.rs:358:37:360:9 | { ... } | main.rs:347:13:347:29 | self.get_number() | provenance |  |
+| main.rs:359:13:359:21 | source(...) | main.rs:358:37:360:9 | { ... } | provenance |  |
+| main.rs:370:44:372:9 | { ... } | main.rs:395:18:395:38 | i.get_double_number() | provenance |  |
+| main.rs:371:13:371:22 | source(...) | main.rs:370:44:372:9 | { ... } | provenance |  |
+| main.rs:374:33:376:9 | { ... } | main.rs:398:18:398:41 | ...::get_default(...) | provenance |  |
+| main.rs:375:13:375:21 | source(...) | main.rs:374:33:376:9 | { ... } | provenance |  |
+| main.rs:383:13:383:14 | n1 | main.rs:384:14:384:15 | n1 | provenance |  |
+| main.rs:383:18:383:38 | t.get_double_number() | main.rs:383:13:383:14 | n1 | provenance |  |
+| main.rs:387:13:387:14 | n2 | main.rs:388:14:388:15 | n2 | provenance |  |
+| main.rs:387:18:387:50 | ...::get_double_number(...) | main.rs:387:13:387:14 | n2 | provenance |  |
+| main.rs:391:13:391:14 | n3 | main.rs:392:14:392:15 | n3 | provenance |  |
+| main.rs:391:18:391:37 | ...::get_default(...) | main.rs:391:13:391:14 | n3 | provenance |  |
+| main.rs:395:13:395:14 | n4 | main.rs:396:14:396:15 | n4 | provenance |  |
+| main.rs:395:18:395:38 | i.get_double_number() | main.rs:395:13:395:14 | n4 | provenance |  |
+| main.rs:398:13:398:14 | n5 | main.rs:399:14:399:15 | n5 | provenance |  |
+| main.rs:398:18:398:41 | ...::get_default(...) | main.rs:398:13:398:14 | n5 | provenance |  |
 nodes
 | main.rs:12:28:14:1 | { ... } | semmle.label | { ... } |
 | main.rs:13:5:13:13 | source(...) | semmle.label | source(...) |
@@ -391,6 +412,31 @@ nodes
 | main.rs:334:13:334:55 | ...::block_on(...) | semmle.label | ...::block_on(...) |
 | main.rs:334:41:334:54 | async_source(...) | semmle.label | async_source(...) |
 | main.rs:335:10:335:10 | a | semmle.label | a |
+| main.rs:346:44:348:9 | { ... } | semmle.label | { ... } |
+| main.rs:347:13:347:29 | self.get_number() | semmle.label | self.get_number() |
+| main.rs:350:33:352:9 | { ... } | semmle.label | { ... } |
+| main.rs:351:13:351:21 | source(...) | semmle.label | source(...) |
+| main.rs:358:37:360:9 | { ... } | semmle.label | { ... } |
+| main.rs:359:13:359:21 | source(...) | semmle.label | source(...) |
+| main.rs:370:44:372:9 | { ... } | semmle.label | { ... } |
+| main.rs:371:13:371:22 | source(...) | semmle.label | source(...) |
+| main.rs:374:33:376:9 | { ... } | semmle.label | { ... } |
+| main.rs:375:13:375:21 | source(...) | semmle.label | source(...) |
+| main.rs:383:13:383:14 | n1 | semmle.label | n1 |
+| main.rs:383:18:383:38 | t.get_double_number() | semmle.label | t.get_double_number() |
+| main.rs:384:14:384:15 | n1 | semmle.label | n1 |
+| main.rs:387:13:387:14 | n2 | semmle.label | n2 |
+| main.rs:387:18:387:50 | ...::get_double_number(...) | semmle.label | ...::get_double_number(...) |
+| main.rs:388:14:388:15 | n2 | semmle.label | n2 |
+| main.rs:391:13:391:14 | n3 | semmle.label | n3 |
+| main.rs:391:18:391:37 | ...::get_default(...) | semmle.label | ...::get_default(...) |
+| main.rs:392:14:392:15 | n3 | semmle.label | n3 |
+| main.rs:395:13:395:14 | n4 | semmle.label | n4 |
+| main.rs:395:18:395:38 | i.get_double_number() | semmle.label | i.get_double_number() |
+| main.rs:396:14:396:15 | n4 | semmle.label | n4 |
+| main.rs:398:13:398:14 | n5 | semmle.label | n5 |
+| main.rs:398:18:398:41 | ...::get_default(...) | semmle.label | ...::get_default(...) |
+| main.rs:399:14:399:15 | n5 | semmle.label | n5 |
 subpaths
 | main.rs:38:23:38:31 | source(...) | main.rs:26:28:26:33 | ...: i64 | main.rs:26:17:26:25 | SelfParam [Return] [&ref, MyStruct] | main.rs:38:6:38:11 | [post] &mut a [&ref, MyStruct] |
 | main.rs:39:10:39:10 | a [MyStruct] | main.rs:30:17:30:21 | SelfParam [&ref, MyStruct] | main.rs:30:31:32:5 | { ... } | main.rs:39:10:39:21 | a.get_data() |
@@ -442,3 +488,8 @@ testFailures
 | main.rs:317:10:317:10 | a | main.rs:316:13:316:21 | source(...) | main.rs:317:10:317:10 | a | $@ | main.rs:316:13:316:21 | source(...) | source(...) |
 | main.rs:327:14:327:14 | c | main.rs:326:17:326:25 | source(...) | main.rs:327:14:327:14 | c | $@ | main.rs:326:17:326:25 | source(...) | source(...) |
 | main.rs:335:10:335:10 | a | main.rs:316:13:316:21 | source(...) | main.rs:335:10:335:10 | a | $@ | main.rs:316:13:316:21 | source(...) | source(...) |
+| main.rs:384:14:384:15 | n1 | main.rs:359:13:359:21 | source(...) | main.rs:384:14:384:15 | n1 | $@ | main.rs:359:13:359:21 | source(...) | source(...) |
+| main.rs:388:14:388:15 | n2 | main.rs:359:13:359:21 | source(...) | main.rs:388:14:388:15 | n2 | $@ | main.rs:359:13:359:21 | source(...) | source(...) |
+| main.rs:392:14:392:15 | n3 | main.rs:351:13:351:21 | source(...) | main.rs:392:14:392:15 | n3 | $@ | main.rs:351:13:351:21 | source(...) | source(...) |
+| main.rs:396:14:396:15 | n4 | main.rs:371:13:371:22 | source(...) | main.rs:396:14:396:15 | n4 | $@ | main.rs:371:13:371:22 | source(...) | source(...) |
+| main.rs:399:14:399:15 | n5 | main.rs:375:13:375:21 | source(...) | main.rs:399:14:399:15 | n5 | $@ | main.rs:375:13:375:21 | source(...) | source(...) |

rust/ql/test/library-tests/dataflow/global/main.rs

@@ -337,6 +337,69 @@ fn test_async_await() {
     futures::executor::block_on(test_async_await_async_part());
 }
 
+mod not_trait_dispatch {
+    use super::{sink, source};
+
+    trait HasNumbers {
+        fn get_number(&self) -> i64;
+
+        fn get_double_number(&self) -> i64 {
+            self.get_number() * 2
+        }
+
+        fn get_default() -> i64 {
+            source(0)
+        }
+    }
+
+    struct Three;
+
+    impl HasNumbers for Three {
+        fn get_number(&self) -> i64 {
+            source(3)
+        }
+    }
+
+    struct TwentyTwo;
+
+    impl HasNumbers for TwentyTwo {
+        fn get_number(&self) -> i64 {
+            22
+        }
+
+        fn get_double_number(&self) -> i64 {
+            source(44)
+        }
+
+        fn get_default() -> i64 {
+            source(1)
+        }
+    }
+
+    fn test_non_trait_dispatch() {
+        let t = Three;
+
+        // This call is to the default method implementation.
+        let n1 = t.get_double_number();
+        sink(n1); // $ hasTaintFlow=3
+
+        // This call is to the default method implementation.
+        let n2 = HasNumbers::get_double_number(&t);
+        sink(n2); // $ hasTaintFlow=3
+
+        // This call is to the default function implementation.
+        let n3 = Three::get_default();
+        sink(n3); // $ hasValueFlow=0
+
+        let i = TwentyTwo;
+        let n4 = i.get_double_number();
+        sink(n4); // $ hasValueFlow=44
+
+        let n5 = TwentyTwo::get_default();
+        sink(n5); // $ hasValueFlow=1
+    }
+}
+
 fn main() {
     data_out_of_call();
     data_out_of_call_side_effect1();

rust/ql/test/library-tests/dataflow/global/viableCallable.expected

@@ -105,14 +105,30 @@
 | main.rs:335:5:335:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
 | main.rs:337:5:337:62 | ...::block_on(...) | {EXTERNAL LOCATION} | fn block_on |
 | main.rs:337:33:337:61 | test_async_await_async_part(...) | main.rs:321:1:331:1 | fn test_async_await_async_part |
-| main.rs:341:5:341:22 | data_out_of_call(...) | main.rs:16:1:19:1 | fn data_out_of_call |
-| main.rs:342:5:342:35 | data_out_of_call_side_effect1(...) | main.rs:35:1:40:1 | fn data_out_of_call_side_effect1 |
-| main.rs:343:5:343:35 | data_out_of_call_side_effect2(...) | main.rs:42:1:50:1 | fn data_out_of_call_side_effect2 |
-| main.rs:344:5:344:21 | data_in_to_call(...) | main.rs:56:1:59:1 | fn data_in_to_call |
-| main.rs:345:5:345:23 | data_through_call(...) | main.rs:65:1:69:1 | fn data_through_call |
-| main.rs:346:5:346:34 | data_through_nested_function(...) | main.rs:79:1:88:1 | fn data_through_nested_function |
-| main.rs:348:5:348:24 | data_out_of_method(...) | main.rs:136:1:146:1 | fn data_out_of_method |
-| main.rs:349:5:349:28 | data_in_to_method_call(...) | main.rs:153:1:163:1 | fn data_in_to_method_call |
-| main.rs:350:5:350:25 | data_through_method(...) | main.rs:171:1:183:1 | fn data_through_method |
-| main.rs:352:5:352:31 | test_operator_overloading(...) | main.rs:240:1:278:1 | fn test_operator_overloading |
-| main.rs:353:5:353:22 | test_async_await(...) | main.rs:333:1:338:1 | fn test_async_await |
+| main.rs:347:13:347:29 | self.get_number() | main.rs:358:9:360:9 | fn get_number |
+| main.rs:347:13:347:29 | self.get_number() | main.rs:366:9:368:9 | fn get_number |
+| main.rs:351:13:351:21 | source(...) | main.rs:1:1:3:1 | fn source |
+| main.rs:359:13:359:21 | source(...) | main.rs:1:1:3:1 | fn source |
+| main.rs:371:13:371:22 | source(...) | main.rs:1:1:3:1 | fn source |
+| main.rs:375:13:375:21 | source(...) | main.rs:1:1:3:1 | fn source |
+| main.rs:383:18:383:38 | t.get_double_number() | main.rs:346:9:348:9 | fn get_double_number |
+| main.rs:384:9:384:16 | sink(...) | main.rs:5:1:7:1 | fn sink |
+| main.rs:387:18:387:50 | ...::get_double_number(...) | main.rs:346:9:348:9 | fn get_double_number |
+| main.rs:388:9:388:16 | sink(...) | main.rs:5:1:7:1 | fn sink |
+| main.rs:391:18:391:37 | ...::get_default(...) | main.rs:350:9:352:9 | fn get_default |
+| main.rs:392:9:392:16 | sink(...) | main.rs:5:1:7:1 | fn sink |
+| main.rs:395:18:395:38 | i.get_double_number() | main.rs:370:9:372:9 | fn get_double_number |
+| main.rs:396:9:396:16 | sink(...) | main.rs:5:1:7:1 | fn sink |
+| main.rs:398:18:398:41 | ...::get_default(...) | main.rs:374:9:376:9 | fn get_default |
+| main.rs:399:9:399:16 | sink(...) | main.rs:5:1:7:1 | fn sink |
+| main.rs:404:5:404:22 | data_out_of_call(...) | main.rs:16:1:19:1 | fn data_out_of_call |
+| main.rs:405:5:405:35 | data_out_of_call_side_effect1(...) | main.rs:35:1:40:1 | fn data_out_of_call_side_effect1 |
+| main.rs:406:5:406:35 | data_out_of_call_side_effect2(...) | main.rs:42:1:50:1 | fn data_out_of_call_side_effect2 |
+| main.rs:407:5:407:21 | data_in_to_call(...) | main.rs:56:1:59:1 | fn data_in_to_call |
+| main.rs:408:5:408:23 | data_through_call(...) | main.rs:65:1:69:1 | fn data_through_call |
+| main.rs:409:5:409:34 | data_through_nested_function(...) | main.rs:79:1:88:1 | fn data_through_nested_function |
+| main.rs:411:5:411:24 | data_out_of_method(...) | main.rs:136:1:146:1 | fn data_out_of_method |
+| main.rs:412:5:412:28 | data_in_to_method_call(...) | main.rs:153:1:163:1 | fn data_in_to_method_call |
+| main.rs:413:5:413:25 | data_through_method(...) | main.rs:171:1:183:1 | fn data_through_method |
+| main.rs:415:5:415:31 | test_operator_overloading(...) | main.rs:240:1:278:1 | fn test_operator_overloading |
+| main.rs:416:5:416:22 | test_async_await(...) | main.rs:333:1:338:1 | fn test_async_await |