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

Open
paldepind wants to merge 4 commits into
base: main
Choose a base branch
from
+170 −21
Open

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)
)
}

/**
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