Skip to content

Rust: Remove references to getResolvedPath and getExtendedCanonicalPath #20224

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
hvitved merged 1 commit into from
Aug 14, 2025
Merged

Rust: Remove references to getResolvedPath and getExtendedCanonicalPath #20224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 0 additions & 13 deletions rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,6 @@ module Input implements InputSig<Location, RustDataFlow> {
/** Gets the associated call. */
abstract CallExprBase getCall();

/** Holds if the associated call resolves to `crate, path`. */
final predicate callResolvesTo(string crate, string path) {
exists(Resolvable r |
r = CallExprBaseImpl::getCallResolvable(this.getCall()) and
path = r.getResolvedPath()
|
crate = r.getResolvedCrateOrigin()
or
not r.hasResolvedCrateOrigin() and
crate = ""
)
}

/** Holds if the associated call resolves to `path`. */
final predicate callResolvesTo(string path) {
path = this.getCall().getStaticTarget().(Addressable).getCanonicalPath()
Expand Down
2 changes: 1 addition & 1 deletion rust/ql/lib/codeql/rust/frameworks/stdlib/Stdlib.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ private import codeql.rust.internal.PathResolution
*/
private class StartswithCall extends Path::SafeAccessCheck::Range, CfgNodes::MethodCallExprCfgNode {
StartswithCall() {
this.getAstNode().(Resolvable).getResolvedPath() = "<crate::path::Path>::starts_with"
this.getMethodCallExpr().getStaticTarget().getCanonicalPath() = "<std::path::Path>::starts_with"
}

override predicate checks(Cfg::CfgNode e, boolean branch) {
Expand Down
23 changes: 14 additions & 9 deletions rust/ql/src/queries/telemetry/RustAnalyzerComparison.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ import rust

pragma[nomagic]
private predicate resolvesAsItem(Resolvable r, Item i) {
r.getResolvedPath() = i.getExtendedCanonicalPath() and
(
r.getResolvedCrateOrigin() = i.getCrateOrigin()
or
not r.hasResolvedCrateOrigin() and not i.hasCrateOrigin()
)
none()
// r.getResolvedPath() = i.getExtendedCanonicalPath() and
// (
// r.getResolvedCrateOrigin() = i.getCrateOrigin()
// or
// not r.hasResolvedCrateOrigin() and not i.hasCrateOrigin()
// )
Copy link
Contributor

@geoffw0 geoffw0 Aug 14, 2025

Choose a reason for hiding this comment

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

What is RustAnalyzerComparison.qll? Is there a reason for leaving these predicates in (as none()) for the time being?

Copy link
Contributor Author

@hvitved hvitved Aug 14, 2025

Choose a reason for hiding this comment

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

It was used to compare data in DCA, but those reports have now been disabled.

Copy link
Contributor

@geoffw0 geoffw0 Aug 14, 2025

Choose a reason for hiding this comment

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

Great.

}

private signature module ResolvableSig {
Expand Down Expand Up @@ -102,7 +103,10 @@ private module PathResolution implements ResolvableSig {
}

private module RustAnalyzerPathResolution implements CompareSig<PathResolution> {
predicate isResolvable(PathResolution::Source s) { s.hasResolvedPath() }
predicate isResolvable(PathResolution::Source s) {
none()
//s.hasResolvedPath()
}

Item resolve(PathResolution::Source s) { resolvesAsItem(s, result) }
}
Expand Down Expand Up @@ -157,6 +161,7 @@ private module QlCallGraph implements CompareSig<CallGraph> {
module CallGraphCompare = Compare<CallGraph, RustAnalyzerCallGraph, QlCallGraph>;

predicate qlMissingCanonicalPath(Addressable a, string path) {
path = a.getExtendedCanonicalPath() and
not exists(a.getCanonicalPath(_))
none()
// path = a.getExtendedCanonicalPath() and
// not exists(a.getCanonicalPath(_))
}
84 changes: 0 additions & 84 deletions rust/ql/test/extractor-tests/canonical_path/canonical_paths.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
canonicalPath
| anonymous.rs:3:1:32:1 | fn canonicals | test::anonymous::canonicals |
| anonymous.rs:34:1:36:1 | fn other | test::anonymous::other |
| {EXTERNAL LOCATION} | fn trim | <core::str>::trim |
Expand Down Expand Up @@ -31,86 +30,3 @@ canonicalPath
| regular.rs:74:5:74:25 | fn abs | <_ as test::regular::Abs>::abs |
| regular.rs:77:1:85:1 | impl Abs for i32 { ... } | <core::i32 as test::regular::Abs> |
| regular.rs:78:5:84:5 | fn abs | <core::i32 as test::regular::Abs>::abs |
canonicalPaths
| anonymous.rs:1:1:1:26 | use ...::Trait | None | None |
| anonymous.rs:3:1:32:1 | fn canonicals | repo::test | crate::anonymous::canonicals |
| anonymous.rs:4:5:4:23 | struct OtherStruct | None | None |
| anonymous.rs:6:5:8:5 | trait OtherTrait | None | None |
| anonymous.rs:7:9:7:20 | fn g | None | None |
| anonymous.rs:10:5:12:5 | impl OtherTrait for OtherStruct { ... } | None | None |
| anonymous.rs:11:9:11:22 | fn g | None | None |
| anonymous.rs:14:5:16:5 | impl OtherTrait for ...::Struct { ... } | None | None |
| anonymous.rs:15:9:15:22 | fn g | None | None |
| anonymous.rs:18:5:20:5 | impl ...::Trait for OtherStruct { ... } | None | None |
| anonymous.rs:19:9:19:22 | fn f | None | None |
| anonymous.rs:22:5:24:5 | fn nested | None | None |
| anonymous.rs:23:9:23:27 | struct OtherStruct | None | None |
| anonymous.rs:26:5:31:5 | fn usage | None | None |
| anonymous.rs:34:1:36:1 | fn other | repo::test | crate::anonymous::other |
| anonymous.rs:35:5:35:23 | struct OtherStruct | None | None |
| lib.rs:1:1:1:18 | mod anonymous | repo::test | crate::anonymous |
| lib.rs:2:1:2:16 | mod regular | repo::test | crate::regular |
| regular.rs:1:1:2:18 | struct Struct | repo::test | crate::regular::Struct |
| regular.rs:2:12:2:17 | fn eq | repo::test | <crate::regular::Struct as crate::cmp::PartialEq>::eq |
| regular.rs:2:12:2:17 | impl ...::Eq for Struct::<...> { ... } | None | None |
| regular.rs:2:12:2:17 | impl ...::PartialEq for Struct::<...> { ... } | None | None |
| regular.rs:4:1:6:1 | trait Trait | repo::test | crate::regular::Trait |
| regular.rs:5:5:5:16 | fn f | repo::test | crate::regular::Trait::f |
| regular.rs:8:1:10:1 | impl Trait for Struct { ... } | None | None |
| regular.rs:9:5:9:18 | fn f | repo::test | <crate::regular::Struct as crate::regular::Trait>::f |
| regular.rs:12:1:14:1 | impl Struct { ... } | None | None |
| regular.rs:13:5:13:18 | fn g | repo::test | <crate::regular::Struct>::g |
| regular.rs:16:1:18:1 | trait TraitWithBlanketImpl | repo::test | crate::regular::TraitWithBlanketImpl |
| regular.rs:17:5:17:16 | fn h | repo::test | crate::regular::TraitWithBlanketImpl::h |
| regular.rs:20:1:22:1 | impl TraitWithBlanketImpl for T { ... } | None | None |
| regular.rs:21:5:21:18 | fn h | repo::test | <_ as crate::regular::TraitWithBlanketImpl>::h |
| regular.rs:24:1:24:12 | fn free | repo::test | crate::regular::free |
| regular.rs:26:1:32:1 | fn usage | repo::test | crate::regular::usage |
| regular.rs:34:1:38:1 | enum MyEnum | repo::test | crate::regular::MyEnum |
| regular.rs:40:1:46:1 | fn enum_qualified_usage | repo::test | crate::regular::enum_qualified_usage |
| regular.rs:48:1:55:1 | fn enum_unqualified_usage | repo::test | crate::regular::enum_unqualified_usage |
| regular.rs:51:5:51:18 | use MyEnum::* | None | None |
| regular.rs:57:1:63:1 | fn enum_match | repo::test | crate::regular::enum_match |
| regular.rs:65:1:67:1 | ExternBlock | None | None |
| regular.rs:66:5:66:40 | fn is_alphanum | repo::test | ::is_alphanum |
| regular.rs:69:1:71:1 | fn is_number_or_letter | repo::test | crate::regular::is_number_or_letter |
| regular.rs:73:1:75:1 | trait Abs | repo::test | crate::regular::Abs |
| regular.rs:74:5:74:25 | fn abs | repo::test | crate::regular::Abs::abs |
| regular.rs:77:1:85:1 | impl Abs for i32 { ... } | None | None |
| regular.rs:78:5:84:5 | fn abs | repo::test | <i32 as crate::regular::Abs>::abs |
resolvedPaths
| anonymous.rs:27:17:27:30 | OtherStruct {...} | None | None |
| anonymous.rs:28:9:28:9 | s | None | None |
| anonymous.rs:28:9:28:13 | s.f() | None | None |
| anonymous.rs:29:9:29:9 | s | None | None |
| anonymous.rs:29:9:29:13 | s.g() | None | None |
| anonymous.rs:30:9:30:14 | nested | None | None |
| regular.rs:1:1:1:24 | other | None | None |
| regular.rs:1:1:1:24 | self | None | None |
| regular.rs:27:13:27:21 | Struct {...} | repo::test | crate::regular::Struct |
| regular.rs:28:5:28:5 | s | None | None |
| regular.rs:28:5:28:9 | s.f() | repo::test | <crate::regular::Struct as crate::regular::Trait>::f |
| regular.rs:29:5:29:5 | s | None | None |
| regular.rs:29:5:29:9 | s.g() | repo::test | <crate::regular::Struct>::g |
| regular.rs:30:5:30:5 | s | None | None |
| regular.rs:30:5:30:9 | s.h() | repo::test | <_ as crate::regular::TraitWithBlanketImpl>::h |
| regular.rs:31:5:31:8 | free | repo::test | crate::regular::free |
| regular.rs:41:9:41:26 | ...::None::<...> | lang:core | crate::option::Option::None |
| regular.rs:42:9:42:20 | ...::Some | lang:core | crate::option::Option::Some |
| regular.rs:43:9:43:24 | ...::Variant1 | repo::test | crate::regular::MyEnum::Variant1 |
| regular.rs:44:9:44:24 | ...::Variant2 | repo::test | crate::regular::MyEnum::Variant2 |
| regular.rs:45:9:45:33 | ...::Variant3 {...} | repo::test | crate::regular::MyEnum::Variant3 |
| regular.rs:49:9:49:18 | None::<...> | lang:core | crate::option::Option::None |
| regular.rs:50:9:50:12 | Some | lang:core | crate::option::Option::Some |
| regular.rs:52:9:52:16 | Variant1 | repo::test | crate::regular::MyEnum::Variant1 |
| regular.rs:53:9:53:16 | Variant2 | repo::test | crate::regular::MyEnum::Variant2 |
| regular.rs:54:9:54:25 | Variant3 {...} | repo::test | crate::regular::MyEnum::Variant3 |
| regular.rs:58:11:58:11 | e | None | None |
| regular.rs:59:9:59:24 | ...::Variant1 | repo::test | crate::regular::MyEnum::Variant1 |
| regular.rs:60:9:60:27 | ...::Variant2(...) | repo::test | crate::regular::MyEnum::Variant2 |
| regular.rs:61:9:61:31 | ...::Variant3 {...} | repo::test | crate::regular::MyEnum::Variant3 |
| regular.rs:70:14:70:24 | is_alphanum | repo::test | ::is_alphanum |
| regular.rs:70:26:70:28 | chr | None | None |
| regular.rs:79:12:79:15 | self | None | None |
| regular.rs:80:14:80:17 | self | None | None |
| regular.rs:82:13:82:16 | self | None | None |
28 changes: 0 additions & 28 deletions rust/ql/test/extractor-tests/canonical_path/canonical_paths.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,3 @@ query predicate canonicalPath(Addressable a, string path) {
) and
path = a.getCanonicalPath(_)
}

query predicate canonicalPaths(Item i, string origin, string path) {
toBeTested(i) and
(
origin = i.getCrateOrigin()
or
not i.hasCrateOrigin() and origin = "None"
) and
(
path = i.getExtendedCanonicalPath()
or
not i.hasExtendedCanonicalPath() and path = "None"
)
}

query predicate resolvedPaths(Resolvable e, string origin, string path) {
toBeTested(e) and
(
origin = e.getResolvedCrateOrigin()
or
not e.hasResolvedCrateOrigin() and origin = "None"
) and
(
path = e.getResolvedPath()
or
not e.hasResolvedPath() and path = "None"
)
}
72 changes: 0 additions & 72 deletions rust/ql/test/extractor-tests/canonical_path_disabled/canonical_paths.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
canonicalPath
| anonymous.rs:6:1:35:1 | fn canonicals | test::anonymous::canonicals |
| anonymous.rs:37:1:39:1 | fn other | test::anonymous::other |
| {EXTERNAL LOCATION} | fn trim | <core::str>::trim |
Expand All @@ -25,74 +24,3 @@ canonicalPath
| regular.rs:43:1:49:1 | fn enum_qualified_usage | test::regular::enum_qualified_usage |
| regular.rs:51:1:58:1 | fn enum_unqualified_usage | test::regular::enum_unqualified_usage |
| regular.rs:60:1:66:1 | fn enum_match | test::regular::enum_match |
canonicalPaths
| anonymous.rs:4:1:4:26 | use ...::Trait | None | None |
| anonymous.rs:6:1:35:1 | fn canonicals | None | None |
| anonymous.rs:7:5:7:23 | struct OtherStruct | None | None |
| anonymous.rs:9:5:11:5 | trait OtherTrait | None | None |
| anonymous.rs:10:9:10:20 | fn g | None | None |
| anonymous.rs:13:5:15:5 | impl OtherTrait for OtherStruct { ... } | None | None |
| anonymous.rs:14:9:14:22 | fn g | None | None |
| anonymous.rs:17:5:19:5 | impl OtherTrait for ...::Struct { ... } | None | None |
| anonymous.rs:18:9:18:22 | fn g | None | None |
| anonymous.rs:21:5:23:5 | impl ...::Trait for OtherStruct { ... } | None | None |
| anonymous.rs:22:9:22:22 | fn f | None | None |
| anonymous.rs:25:5:27:5 | fn nested | None | None |
| anonymous.rs:26:9:26:27 | struct OtherStruct | None | None |
| anonymous.rs:29:5:34:5 | fn usage | None | None |
| anonymous.rs:37:1:39:1 | fn other | None | None |
| anonymous.rs:38:5:38:23 | struct OtherStruct | None | None |
| lib.rs:1:1:1:18 | mod anonymous | None | None |
| lib.rs:2:1:2:16 | mod regular | None | None |
| regular.rs:4:1:5:18 | struct Struct | None | None |
| regular.rs:5:12:5:17 | fn eq | None | None |
| regular.rs:5:12:5:17 | impl ...::Eq for Struct::<...> { ... } | None | None |
| regular.rs:5:12:5:17 | impl ...::PartialEq for Struct::<...> { ... } | None | None |
| regular.rs:7:1:9:1 | trait Trait | None | None |
| regular.rs:8:5:8:16 | fn f | None | None |
| regular.rs:11:1:13:1 | impl Trait for Struct { ... } | None | None |
| regular.rs:12:5:12:18 | fn f | None | None |
| regular.rs:15:1:17:1 | impl Struct { ... } | None | None |
| regular.rs:16:5:16:18 | fn g | None | None |
| regular.rs:19:1:21:1 | trait TraitWithBlanketImpl | None | None |
| regular.rs:20:5:20:16 | fn h | None | None |
| regular.rs:23:1:25:1 | impl TraitWithBlanketImpl for T { ... } | None | None |
| regular.rs:24:5:24:18 | fn h | None | None |
| regular.rs:27:1:27:12 | fn free | None | None |
| regular.rs:29:1:35:1 | fn usage | None | None |
| regular.rs:37:1:41:1 | enum MyEnum | None | None |
| regular.rs:43:1:49:1 | fn enum_qualified_usage | None | None |
| regular.rs:51:1:58:1 | fn enum_unqualified_usage | None | None |
| regular.rs:54:5:54:18 | use MyEnum::* | None | None |
| regular.rs:60:1:66:1 | fn enum_match | None | None |
resolvedPaths
| anonymous.rs:30:17:30:30 | OtherStruct {...} | None | None |
| anonymous.rs:31:9:31:9 | s | None | None |
| anonymous.rs:31:9:31:13 | s.f() | None | None |
| anonymous.rs:32:9:32:9 | s | None | None |
| anonymous.rs:32:9:32:13 | s.g() | None | None |
| anonymous.rs:33:9:33:14 | nested | None | None |
| regular.rs:4:1:4:24 | other | None | None |
| regular.rs:4:1:4:24 | self | None | None |
| regular.rs:30:13:30:21 | Struct {...} | None | None |
| regular.rs:31:5:31:5 | s | None | None |
| regular.rs:31:5:31:9 | s.f() | None | None |
| regular.rs:32:5:32:5 | s | None | None |
| regular.rs:32:5:32:9 | s.g() | None | None |
| regular.rs:33:5:33:5 | s | None | None |
| regular.rs:33:5:33:9 | s.h() | None | None |
| regular.rs:34:5:34:8 | free | None | None |
| regular.rs:44:9:44:26 | ...::None::<...> | None | None |
| regular.rs:45:9:45:20 | ...::Some | None | None |
| regular.rs:46:9:46:24 | ...::Variant1 | None | None |
| regular.rs:47:9:47:24 | ...::Variant2 | None | None |
| regular.rs:48:9:48:33 | ...::Variant3 {...} | None | None |
| regular.rs:52:9:52:18 | None::<...> | None | None |
| regular.rs:53:9:53:12 | Some | None | None |
| regular.rs:55:9:55:16 | Variant1 | None | None |
| regular.rs:56:9:56:16 | Variant2 | None | None |
| regular.rs:57:9:57:25 | Variant3 {...} | None | None |
| regular.rs:61:11:61:11 | e | None | None |
| regular.rs:62:9:62:24 | ...::Variant1 | None | None |
| regular.rs:63:9:63:27 | ...::Variant2(...) | None | None |
| regular.rs:64:9:64:31 | ...::Variant3 {...} | None | None |
6 changes: 3 additions & 3 deletions rust/ql/test/library-tests/dataflow/sources/InlineFlow.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ module MyFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof ThreatModelSource }

predicate isSink(DataFlow::Node sink) {
any(CallExpr call | call.getFunction().(PathExpr).getResolvedPath().matches("%::sink"))
.getArgList()
.getAnArg() = sink.asExpr().getExpr()
any(CallExpr call |
call.getFunction().(PathExpr).getPath().getSegment().getIdentifier().getText() = "sink"
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

The new path resolution approach is fragile as it only checks the final segment identifier. This will match any function named 'sink' regardless of module/crate. The original code used pattern matching with '%::sink' which was more specific. Consider using a more robust matching approach.

Suggested change
call.getFunction().(PathExpr).getPath().getSegment().getIdentifier().getText() = "sink"
call.getFunction().(PathExpr).getPath().toString().matches("%::sink")

Copilot uses AI. Check for mistakes.
).getArgList().getAnArg() = sink.asExpr().getExpr()
}

predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
Expand Down
6 changes: 3 additions & 3 deletions rust/ql/test/library-tests/sensitivedata/SensitiveData.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ module SensitiveDataConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof SensitiveData }

predicate isSink(DataFlow::Node sink) {
any(CallExpr call | call.getFunction().(PathExpr).getResolvedPath() = "crate::test::sink")
.getArgList()
.getAnArg() = sink.asExpr().getExpr()
any(CallExpr call |
call.getFunction().(PathExpr).getPath().getSegment().getIdentifier().getText() = "sink"
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

The new path resolution approach is fragile as it only checks the final segment identifier. This will match any function named 'sink' regardless of module/crate. Consider using a more specific matching approach or add additional checks to ensure the correct function is matched.

Suggested change
call.getFunction().(PathExpr).getPath().getSegment().getIdentifier().getText() = "sink"
call.getFunction().(PathExpr).getPath().toString() = "sink"

Copilot uses AI. Check for mistakes.
).getArgList().getAnArg() = sink.asExpr().getExpr()
}
}

Expand Down