-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rust: Support Argument[x]
MaD source definitions
#19107
Conversation
There was a problem hiding this 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 adds support for interpreting post-update nodes of the first argument in calls to arg_source by introducing a new MaD source definition.
- Adds a new MaD source definition for "Argument[0]" in the YAML models file.
- Introduces an arg_source function and an associated test (test_arg_source) in main.rs that demonstrates the new source flow.
Reviewed Changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
rust/ql/test/library-tests/dataflow/models/models.ext.yml | Added a new row defining a source model for arg_source with "Argument[0]" |
rust/ql/test/library-tests/dataflow/models/main.rs | Added the arg_source function and a corresponding test to invoke it |
Files not reviewed (2)
- rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll: Language not supported
- rust/ql/test/library-tests/dataflow/models/models.expected: Language not supported
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
fn test_arg_source() { | ||
let i = 19; | ||
arg_source(i); | ||
sink(i) // $ hasValueFlow=i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a semicolon after the sink(i) call for consistency with other test cases and to prevent potential compilation issues.
sink(i) // $ hasValueFlow=i | |
sink(i); // $ hasValueFlow=i |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
result = call.getArgList().getArg(pos.getPosition()) | ||
or | ||
result = call.(MethodCallExpr).getReceiver() and pos.isSelf() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could make this a member predicate on ParameterPosition
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for addressing this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Adds support for writing MaD sources like
which interprets post-update nodes of first arguments in calls to
arg_source
as sources.