Skip to content

Rust: Make SummarizedCallable extend Function instead of string #19268

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
merged 1 commit into from
Jun 18, 2025
Merged

Rust: Make SummarizedCallable extend Function instead of string #19268

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
27 changes: 1 addition & 26 deletions rust/ql/lib/codeql/rust/dataflow/FlowSummary.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,17 @@

private import rust
private import internal.FlowSummaryImpl as Impl
private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprBaseImpl

// import all instances below
private module Summaries {
private import codeql.rust.Frameworks
private import codeql.rust.dataflow.internal.ModelsAsData
}

/** Provides the `Range` class used to define the extent of `LibraryCallable`. */
module LibraryCallable {
/** A callable defined in library code, identified by a unique string. */
abstract class Range extends string {
bindingset[this]
Range() { any() }

/** Gets a call to this library callable. */
CallExprBase getACall() {
exists(Resolvable r, string crate |
r = CallExprBaseImpl::getCallResolvable(result) and
this = crate + r.getResolvedPath()
|
crate = r.getResolvedCrateOrigin() + "::_::"
or
not r.hasResolvedCrateOrigin() and
crate = ""
)
}
}
}

final class LibraryCallable = LibraryCallable::Range;

/** Provides the `Range` class used to define the extent of `SummarizedCallable`. */
module SummarizedCallable {
/** A callable with a flow summary, identified by a unique string. */
abstract class Range extends LibraryCallable::Range, Impl::Public::SummarizedCallable {
abstract class Range extends Impl::Public::SummarizedCallable {
bindingset[this]
Range() { any() }

Expand Down
25 changes: 13 additions & 12 deletions rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,12 @@ final class DataFlowCallable extends TDataFlowCallable {
/**
* Gets the underlying library callable, if any.
*/
LibraryCallable asLibraryCallable() { this = TLibraryCallable(result) }
SummarizedCallable asSummarizedCallable() { this = TSummarizedCallable(result) }

/** Gets a textual representation of this callable. */
string toString() { result = [this.asCfgScope().toString(), this.asLibraryCallable().toString()] }
string toString() {
result = [this.asCfgScope().toString(), this.asSummarizedCallable().toString()]
}

/** Gets the location of this callable. */
Location getLocation() { result = this.asCfgScope().getLocation() }
Expand All @@ -65,12 +67,9 @@ final class DataFlowCall extends TDataFlowCall {
}

DataFlowCallable getEnclosingCallable() {
result = TCfgScope(this.asCallCfgNode().getExpr().getEnclosingCfgScope())
result.asCfgScope() = this.asCallCfgNode().getExpr().getEnclosingCfgScope()
or
exists(FlowSummaryImpl::Public::SummarizedCallable c |
this.isSummaryCall(c, _) and
result = TLibraryCallable(c)
)
this.isSummaryCall(result.asSummarizedCallable(), _)
}

string toString() {
Expand Down Expand Up @@ -401,9 +400,11 @@ module RustDataFlow implements InputSig<Location> {

/** Gets a viable implementation of the target of the given `Call`. */
DataFlowCallable viableCallable(DataFlowCall call) {
result.asCfgScope() = call.asCallCfgNode().getCall().getStaticTarget()
or
result.asLibraryCallable().getACall() = call.asCallCfgNode().getCall()
exists(Callable target | target = call.asCallCfgNode().getCall().getStaticTarget() |
target = result.asCfgScope()
or
target = result.asSummarizedCallable()
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is a key change? We now use getStaticTarget target in both cases, whereas we previously used getACall in the later case, which used extractor provided canonical paths internally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely correct.

}

/**
Expand Down Expand Up @@ -757,7 +758,7 @@ module RustDataFlow implements InputSig<Location> {
predicate allowParameterReturnInSelf(ParameterNode p) {
exists(DataFlowCallable c, ParameterPosition pos |
p.isParameterOf(c, pos) and
FlowSummaryImpl::Private::summaryAllowParameterReturnInSelf(c.asLibraryCallable(), pos)
FlowSummaryImpl::Private::summaryAllowParameterReturnInSelf(c.asSummarizedCallable(), pos)
)
or
VariableCapture::Flow::heuristicAllowInstanceParameterReturnInSelf(p.(ClosureParameterNode)
Expand Down Expand Up @@ -968,7 +969,7 @@ private module Cached {
cached
newtype TDataFlowCallable =
TCfgScope(CfgScope scope) or
TLibraryCallable(LibraryCallable c)
TSummarizedCallable(SummarizedCallable c)

/** This is the local flow predicate that is exposed. */
cached
Expand Down
4 changes: 2 additions & 2 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 @@ -13,7 +13,7 @@ module Input implements InputSig<Location, RustDataFlow> {
private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprBaseImpl
private import codeql.rust.frameworks.stdlib.Stdlib

class SummarizedCallableBase = string;
class SummarizedCallableBase = Function;

abstract private class SourceSinkBase extends AstNode {
/** Gets the associated call. */
Expand Down Expand Up @@ -138,7 +138,7 @@ private import Make<Location, RustDataFlow, Input> as Impl

private module StepsInput implements Impl::Private::StepsInputSig {
DataFlowCall getACall(Public::SummarizedCallable sc) {
result.asCallCfgNode().getCall() = sc.(LibraryCallable).getACall()
result.asCallCfgNode().getCall().getStaticTarget() = sc
}

RustDataFlow::Node getSourceNode(Input::SourceBase source, Impl::Private::SummaryComponent sc) {
Expand Down
8 changes: 7 additions & 1 deletion rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ private import rust
private import codeql.rust.dataflow.FlowSummary
private import codeql.rust.dataflow.FlowSource
private import codeql.rust.dataflow.FlowSink
private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprBaseImpl

/**
* Holds if in a call to the function with canonical path `path`, defined in the
Expand Down Expand Up @@ -120,7 +121,12 @@ private class SummarizedCallableFromModel extends SummarizedCallable::Range {

SummarizedCallableFromModel() {
summaryModel(crate, path, _, _, _, _, _) and
this = crate + "::_::" + path
exists(CallExprBase call, Resolvable r |
call.getStaticTarget() = this and
r = CallExprBaseImpl::getCallResolvable(call) and
r.getResolvedPath() = path and
r.getResolvedCrateOrigin() = crate
)
}

override predicate propagatesFlow(
Expand Down
8 changes: 4 additions & 4 deletions rust/ql/lib/codeql/rust/dataflow/internal/Node.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ abstract class NodePublic extends TNode {

abstract class Node extends NodePublic {
/** Gets the enclosing callable. */
DataFlowCallable getEnclosingCallable() { result = TCfgScope(this.getCfgScope()) }
DataFlowCallable getEnclosingCallable() { result.asCfgScope() = this.getCfgScope() }

/** Do not call: use `getEnclosingCallable()` instead. */
abstract CfgScope getCfgScope();
Expand Down Expand Up @@ -102,9 +102,9 @@ class FlowSummaryNode extends Node, TFlowSummaryNode {
}

override DataFlowCallable getEnclosingCallable() {
result.asLibraryCallable() = this.getSummarizedCallable()
or
result.asCfgScope() = this.getCfgScope()
or
result.asSummarizedCallable() = this.getSummarizedCallable()
}

override Location getLocation() {
Expand Down Expand Up @@ -195,7 +195,7 @@ final class SummaryParameterNode extends ParameterNode, FlowSummaryNode {
}

override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
this.getSummarizedCallable() = c.asLibraryCallable() and pos = pos_
this.getSummarizedCallable() = c.asSummarizedCallable() and pos = pos_
}
}

Expand Down
10 changes: 3 additions & 7 deletions rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,9 @@ private import codeql.rust.dataflow.FlowSummary
/** A `clone` method. */
final class CloneCallable extends SummarizedCallable::Range {
CloneCallable() {
// NOTE: The function target may not exist in the database, so we base this
// on method calls.
exists(MethodCallExpr c |
c.getIdentifier().getText() = "clone" and
c.getArgList().getNumberOfArgs() = 0 and
this = c.getResolvedCrateOrigin() + "::_::" + c.getResolvedPath()
)
this.getParamList().hasSelfParam() and
this.getParamList().getNumberOfParams() = 0 and
this.getName().getText() = "clone"
}

final override predicate propagatesFlow(
Expand Down
4 changes: 2 additions & 2 deletions rust/ql/lib/codeql/rust/internal/PathResolution.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1522,8 +1522,8 @@ private module Debug {
private Locatable getRelevantLocatable() {
exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
filepath.matches("%/test.rs") and
startline = 74
filepath.matches("%/main.rs") and
startline = 52
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice not to commit these changes. For instance, when looking at the history of a file it's easier if the commits that show up actually make meaningful changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll avoid that going forward.

)
}

Expand Down
5 changes: 3 additions & 2 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 @@ -1170,6 +1170,7 @@ final class MethodCall extends Call {
* Holds if a method for `type` with the name `name` and the arity `arity`
* exists in `impl`.
*/
pragma[nomagic]
private predicate methodCandidate(Type type, string name, int arity, Impl impl) {
type = impl.getSelfTy().(TypeMention).resolveType() and
exists(Function f |
Expand Down Expand Up @@ -1579,8 +1580,8 @@ private module Debug {
private Locatable getRelevantLocatable() {
exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |

Check warning

Code scanning / CodeQL

Omittable 'exists' variable Warning

This exists variable can be omitted by using a don't-care expression
in this argument
Loading
.

Check warning

Code scanning / CodeQL

Omittable 'exists' variable Warning

This exists variable can be omitted by using a don't-care expression
in this argument
Loading
.

Check warning

Code scanning / CodeQL

Omittable 'exists' variable Warning

This exists variable can be omitted by using a don't-care expression
in this argument
Loading
.
result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
filepath.matches("%/main.rs") and
startline = 1718
filepath.matches("%/sqlx.rs") and
startline = [56 .. 60]
Copy link
Contributor

Choose a reason for hiding this comment

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

Save as above.

)
}

Expand Down
4 changes: 2 additions & 2 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 @@ -75,10 +75,10 @@
| main.rs:279:17:279:25 | source(...) | main.rs:1:1:3:1 | fn source |
| main.rs:280:9:280:15 | sink(...) | main.rs:5:1:7:1 | fn sink |
| main.rs:283:5:283:17 | sink(...) | main.rs:5:1:7:1 | fn sink |
| main.rs:287:13:287:55 | ...::block_on(...) | file://:0:0:0:0 | repo:https://github.com/rust-lang/futures-rs:futures-executor::_::crate::local_pool::block_on |
| main.rs:287:13:287:55 | ...::block_on(...) | file://:0:0:0:0 | fn block_on |
| main.rs:287:41:287:54 | async_source(...) | main.rs:268:1:272:1 | fn async_source |
| main.rs:288:5:288:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
| main.rs:290:5:290:62 | ...::block_on(...) | file://:0:0:0:0 | repo:https://github.com/rust-lang/futures-rs:futures-executor::_::crate::local_pool::block_on |
| main.rs:290:5:290:62 | ...::block_on(...) | file://:0:0:0:0 | fn block_on |
| main.rs:290:33:290:61 | test_async_await_async_part(...) | main.rs:274:1:284:1 | fn test_async_await_async_part |
| main.rs:294:5:294:22 | data_out_of_call(...) | main.rs:16:1:19:1 | fn data_out_of_call |
| main.rs:295:5:295:35 | data_out_of_call_side_effect1(...) | main.rs:35:1:40:1 | fn data_out_of_call_side_effect1 |
Expand Down
Loading