Skip to content

Rust: Assume callbacks will be invoked in library functions - #22287

Merged
hvitved merged 4 commits into
github:mainfrom
hvitved:rust/default-callback-models
Aug 10, 2026
Merged

hvitved merged 4 commits into
github:mainfrom
hvitved:rust/default-callback-models

Conversation

@hvitved

@hvitved hvitved commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

Flow through captured variables relies on being able to see calls to the capturing closures. For calls that happen inside library functions we hence need a flow model, which this PR adds, similar to other languages (e.g. Java).

@github-actions github-actions Bot added the Rust Pull requests that update Rust code label Aug 6, 2026
Comment thread rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds heuristic Rust data-flow modeling for callbacks invoked by external library functions.

Changes:

  • Models external Fn/FnOnce callback invocation.
  • Aligns closure-self access paths and excludes external stubs from CFG scopes.
  • Adds callback flow tests and expected results.
Show a summary per file
File Description
ModelsAsData.qll Adds heuristic callback summaries.
DataFlowImpl.qll Standardizes the closure-self label.
Scope.qll Excludes external stubs from callable CFG scopes.
main.rs Adds callback flow assertions.
external_file.rs Defines external callback fixtures.
inline-flow.ext.yml Marks the fixture as external.
inline-flow.expected Records expected flow results.

Review details

  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll Outdated
@hvitved
hvitved force-pushed the rust/default-callback-models branch from 4c49096 to e9bd698 Compare August 6, 2026 08:28

@geoffw0 geoffw0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, pending DCA outcome (I'd be a little surprised if we don't get some new flow there).

@geoffw0 geoffw0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The performance regression is mostly fixed, I think it's good enough (if we can't get it down to zero).

👍

@hvitved hvitved added the no-change-note-required This PR does not need a change note label Aug 10, 2026
@hvitved
hvitved marked this pull request as ready for review August 10, 2026 14:32
@hvitved
hvitved requested a review from a team as a code owner August 10, 2026 14:32
@hvitved
hvitved merged commit e481c01 into github:main Aug 10, 2026
22 checks passed
@hvitved
hvitved deleted the rust/default-callback-models branch August 10, 2026 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants