Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
paldepind merged 4 commits into from
Dec 8, 2025
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
59ce721
Rust: Add global data flow example
paldepind Dec 5, 2025
5888ed3
Rust: Do not dispatch to all implementations when trait target is acc…
paldepind Dec 5, 2025
fa4b212
Rust: Fix grammar
paldepind Dec 5, 2025
f200dba
Rust: Add change note
paldepind Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,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() }
Expand All @@ -79,9 +79,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))
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion rust/ql/lib/codeql/rust/elements/internal/InvocationExprImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, _) }
}
}
27 changes: 21 additions & 6 deletions rust/ql/lib/codeql/rust/internal/TypeInference.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 the 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)
)
Copy link
Contributor

@geoffw0 geoffw0 Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For method calls, is it always resolved by type inference?

Copy link
Contributor

@hvitved hvitved Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Contributor Author

@paldepind paldepind Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, "method call" is a bit of an overloaded term now 😅

Syntactic method calls from MethodCallExpr are always resolved by type inference.

Semantic methods calls (any expression that ends up calling a method, such as Trait::method(ting)) are mostly resolved by type inference. I don't have all of the details in my mind right now. But, for instance the simple case like Vec::new is resolved by path resolution.

}

/**
Expand Down Expand Up @@ -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(
Expand Down
51 changes: 51 additions & 0 deletions rust/ql/test/library-tests/dataflow/global/inline-flow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(...) |
Expand Down Expand Up @@ -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() |
Expand Down Expand Up @@ -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(...) |
63 changes: 63 additions & 0 deletions rust/ql/test/library-tests/dataflow/global/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
38 changes: 27 additions & 11 deletions rust/ql/test/library-tests/dataflow/global/viableCallable.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Loading