Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Rust: SpeedupAccessAfterLifetime.ql#21071

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.

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

Already on GitHub?Sign in to your account

Open
hvitved wants to merge1 commit intogithub:main
base:main
Choose a base branch
Loading
fromhvitved:rust/access-after-lifetime-perf

Conversation

@hvitved
Copy link
Contributor

@hvitvedhvitved commentedDec 19, 2025
edited
Loading

Manually pushes magic context into transitive closure calculation, and replaces the recursive calculation withdoubleBoundedFastTC.

Before

Pipeline standard for AccessAfterLifetimeExtensions::AccessAfterLifetime::mayEncloseOnStack/2#3cdefece#bf@61cb32j5 was evaluated in 30 iterations totaling 44856ms (delta sizes total: 241646328).         241404616     ~1%    {2} r1 = SCAN `AccessAfterLifetimeExtensions::AccessAfterLifetime::mayEncloseOnStack/2#3cdefece#bf#prev_delta` OUTPUT In.1, In.0        7379161442  ~1080%    {2}    | JOIN WITH `_AstNode::AstNode.getEnclosingBlock/0#5c38e65a_AstNode::AstNode.getEnclosingCallable/0#5a548913_Bloc__#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1         333897324    ~40%    {2}    | AND NOT `AccessAfterLifetimeExtensions::AccessAfterLifetime::mayEncloseOnStack/2#3cdefece#bf#prev`(FIRST 2)         297961888    ~24%    {2}    | JOIN WITH `project#AccessAfterLifetimeExtensions::AccessAfterLifetime::sourceValueScope/3#d065ba16#2` ON FIRST 1 OUTPUT Lhs.0, Lhs.1                              return r1

The query started regressing with108db75, which effectively computed the transitive closure over the entire virtual dispatch graph.

DCA is great; a 30 % reduction intotal analysis time onrust.

@github-actionsgithub-actionsbot added the RustPull requests that update Rust code labelDec 19, 2025
Before```Pipeline standard for AccessAfterLifetimeExtensions::AccessAfterLifetime::mayEncloseOnStack/2#3cdefece#bf@61cb32j5 was evaluated in 30 iterations totaling 44856ms (delta sizes total: 241646328).         241404616     ~1%    {2} r1 = SCAN `AccessAfterLifetimeExtensions::AccessAfterLifetime::mayEncloseOnStack/2#3cdefece#bf#prev_delta` OUTPUT In.1, In.0        7379161442  ~1080%    {2}    | JOIN WITH `_AstNode::AstNode.getEnclosingBlock/0#5c38e65a_AstNode::AstNode.getEnclosingCallable/0#5a548913_Bloc__#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1         333897324    ~40%    {2}    | AND NOT `AccessAfterLifetimeExtensions::AccessAfterLifetime::mayEncloseOnStack/2#3cdefece#bf#prev`(FIRST 2)         297961888    ~24%    {2}    | JOIN WITH `project#AccessAfterLifetimeExtensions::AccessAfterLifetime::sourceValueScope/3#d065ba16#2` ON FIRST 1 OUTPUT Lhs.0, Lhs.1                              return r1```
@hvitvedhvitvedforce-pushed therust/access-after-lifetime-perf branch frombfcbf51 to2f04451CompareDecember 19, 2025 12:10
@hvitvedhvitved reopened thisDec 19, 2025
@hvitvedhvitved added the no-change-note-requiredThis PR does not need a change note labelDec 19, 2025
@hvitvedhvitved marked this pull request as ready for reviewDecember 19, 2025 18:15
@hvitvedhvitved requested a review froma team as acode ownerDecember 19, 2025 18:15
CopilotAI review requested due to automatic review settingsDecember 19, 2025 18:15
Copy link
Contributor

CopilotAI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes theAccessAfterLifetime.ql query to significantly improve performance by replacing a recursive transitive closure calculation withdoubleBoundedFastTC. The optimization was motivated by a 30% reduction in total analysis time on Rust code after a previous change (PR#20966) caused performance regression by computing transitive closure over the entire virtual dispatch graph.

Key Changes

  • Introduces a parameterized module pattern using a signature for dataflow candidate pairs
  • Replaces recursivemayEncloseOnStack predicate with optimized transitive closure usingdoubleBoundedFastTC
  • Refactors the logic into aDereferenceAfterLifetime module that takes a flow predicate as a parameter

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.qlUpdates the call todereferenceAfterLifetime to use the new parameterized module, passingAccessAfterLifetimeFlow::flow/2 as the flow predicate
rust/ql/lib/codeql/rust/security/AccessAfterLifetimeExtensions.qllRefactorsdereferenceAfterLifetime into a parameterized module with optimized transitive closure usingdoubleBoundedFastTC, introducesTcNode type to unify sources, sinks, and block expressions for efficient graph traversal

💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.

* Holds if block `a` contains block `b`, in the sense that a stack allocated variable in
* `a` may still be on the stack during execution of `b`. This is interprocedural,
* but is an overapproximation that doesn't accurately track call contexts
* (for example if `f` and `g` both call `b`, then then depending on the

Choose a reason for hiding this comment

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

The comment contains a duplicate word 'then'. The phrase should read "for example iff andg both callb, then depending on the caller..." with only one 'then'.

Suggested change
*(forexampleif `f`and `g`bothcall `b`,thenthendependingon the
*(forexampleif `f`and `g`bothcall `b`,thendependingon the

Copilot uses AI. Check for mistakes.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

Copilot code reviewCopilotCopilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

no-change-note-requiredThis PR does not need a change noteRustPull requests that update Rust code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@hvitved

[8]ページ先頭

©2009-2025 Movatter.jp