Skip to content

Rust: Handle closures/lambdas in data flow#18160

Merged
paldepind merged 6 commits into
github:mainfrom
paldepind:rust-df-closure
Dec 4, 2024
Merged

Rust: Handle closures/lambdas in data flow#18160
paldepind merged 6 commits into
github:mainfrom
paldepind:rust-df-closure

Conversation

@paldepind
Copy link
Copy Markdown
Contributor

This implements lambdaCreation and lambdaCall, enabling data flow through lambdas.

Note that if we extend the QL variables implementation to handle local function definitions then we need to adapt lambdaCall.

Also, this doesn't handle .await calling async blocks, but I think that is of less importance.

@github-actions github-actions Bot added the Rust Pull requests that update Rust code label Nov 29, 2024
@paldepind paldepind marked this pull request as ready for review November 29, 2024 11:01
Copy link
Copy Markdown
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM, one question.

predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) { none() }
predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) {
exists(Expr cl | cl = creation.asExpr().getExpr() and cl = c.asCfgScope() |
cl instanceof ClosureExpr or cl instanceof AsyncBlockExpr
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.

I don't think we should include AsyncBlockExpr here, otherwise I guess we would want to have different LambdaCallKind for closures and async blocks?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense to treat them as lambdas (they capture variables, are "called" by .await). But you're right that we should also have a lambda kind for them. That's what I was alluding to in the PR description, that the async block handling is half baked. I can take AsyncBlockExpr for now and leave it until we implement proper support?

Comment thread rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll Outdated
Co-authored-by: Tom Hvitved <hvitved@github.com>
@paldepind paldepind merged commit 4371433 into github:main Dec 4, 2024
@paldepind paldepind deleted the rust-df-closure branch December 4, 2024 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants