Rust: Handle functions as data flow lambdas #20547
Conversation
| creation instanceof Scope::AsyncBlockScope | ||
| ) and | ||
| exists(kind) | ||
| predicate lambdaCreationExpr(Expr creation) { |
There was a problem hiding this comment.
Dropped the kind parameter as it was only a nuisance for most callers.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances Rust data flow analysis by enabling proper handling of functions as data flow lambdas, specifically when functions are passed as function pointers. The change allows function references to be treated as lambda creations in the data flow graph, similar to how closures are handled.
Key Changes
- Modified lambda creation logic to handle function references as lambda creations
- Added test cases demonstrating data flow through function pointers
- Updated expected test results to reflect new function pointer data flow capabilities
Reviewed Changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
rust/ql/test/library-tests/dataflow/lambdas/main.rs |
Added comprehensive test cases for function pointer data flow scenarios |
rust/ql/test/library-tests/dataflow/lambdas/inline-flow.expected |
New expected test results for lambda data flow including function pointers |
rust/ql/test/library-tests/dataflow/closures/inline-flow.expected |
Cleared previous closure test expectations (likely moved to lambdas) |
rust/ql/lib/codeql/rust/dataflow/internal/Node.qll |
Simplified lambda creation predicate signature |
rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll |
Enhanced lambda creation logic to handle function references |
rust/ql/lib/change-notes/2025-09-29-data-flow-function-pointer.md |
Documentation of the improvement |
| // A path expression, that resolves to a function, evaluates to a function | ||
| // pointer. Except if the path occurs directly in a call, then it's just a | ||
| // call to the function and not a function being passed as data. |
There was a problem hiding this comment.
The comment should clarify what resolvePath returns or link to its definition, as it's not immediately clear what entities it resolves to or how it relates to c.asCfgScope().
| // A path expression, that resolves to a function, evaluates to a function | |
| // pointer. Except if the path occurs directly in a call, then it's just a | |
| // call to the function and not a function being passed as data. | |
| // A path expression that resolves to a function evaluates to a function | |
| // pointer. Except if the path occurs directly in a call, then it's just a | |
| // call to the function and not a function being passed as data. | |
| // `resolvePath` returns the entity (such as a function or variable) to which | |
| // the path expression refers. Here, we check if the resolved entity is the same | |
| // as `c.asCfgScope()`, which represents the callable's scope. See the definition | |
| // of `resolvePath` in `codeql.rust.internal.PathResolution` for details. |
Copilot uses AI. Check for mistakes.
| let f = |cond, data| { | ||
| if cond { | ||
| sink(data); // $ hasValueFlow=87 | ||
| } else { | ||
| sink(0) | ||
| }; | ||
| } | ||
| }; |
There was a problem hiding this comment.
[nitpick] The closure formatting is inconsistent with the rest of the file. Consider using the same single-line format as closure_flow_through() for consistency, or apply consistent multi-line formatting throughout.
| let f = |cond, data| { | |
| if cond { | |
| sink(data); // $ hasValueFlow=87 | |
| } else { | |
| sink(0) | |
| }; | |
| } | |
| }; | |
| let f = |cond, data| sink(if cond { data } else { 0 }); // $ hasValueFlow=87 |
Copilot uses AI. Check for mistakes.
| let f = |cond, data| { | ||
| if cond { | ||
| sink(data); // $ hasValueFlow=87 | ||
| } else { | ||
| sink(0) | ||
| }; | ||
| } | ||
| }; |
There was a problem hiding this comment.
[nitpick] The closure formatting is inconsistent with the rest of the file. Consider using the same single-line format as closure_flow_through() for consistency, or apply consistent multi-line formatting throughout.
| let f = |cond, data| { | |
| if cond { | |
| sink(data); // $ hasValueFlow=87 | |
| } else { | |
| sink(0) | |
| }; | |
| } | |
| }; | |
| let f = |cond, data| if cond { sink(data); /* $ hasValueFlow=87 */ } else { sink(0) }; |
Copilot uses AI. Check for mistakes.
| @@ -0,0 +1,50 @@ | |||
| models | |||
There was a problem hiding this comment.
Why did you change the dir name to lambdas? I thought closure was more standard terminology in Rust.
Fixes the problem pointed out at #20452 (comment).
The DCA report is mega boring.
I noticed that in JavaScript
lambdaCreationholds for the function declaration itself. With how we're doing things in Rust (functions don't have DF nodes) letting the path that refers to the function be the creation seemed simpler.