Update more queries to the new dataflow library #949
Conversation
… new dataflow library
…taflow library
Note this introduces some new results. This seems to be correct, as before the
update the query seemed to have missed problems with code like the following:
```cpp
void f3(int *v1) {
int *v2 = v1;
std::shared_ptr<int> p1(v1); // NON_COMPLIANT
new std::shared_ptr<int>(p1.get()); // NON_COMPLIANT
new std::shared_ptr<int>(v2); // NON_COMPLIANT
}
void f4() {
f3(new int(0));
}
```
…w library Note that this removes - what seems to be - a duplicated test result.
Note that there's a small issue here where the dataflow library causes one of the results to get duplicated.
There was a problem hiding this comment.
Pull Request Overview
Updates multiple queries to use the new dataflow library by replacing imports from semmle.code.cpp.dataflow.DataFlow and semmle.code.cpp.dataflow.TaintTracking with their new counterparts from semmle.code.cpp.dataflow.new.*. This migration also involves API changes where asDefiningArgument() is replaced with getArgument() and asExpr() is replaced with asIndirectExpr() for certain node types.
- Migration from old dataflow library to new dataflow library across multiple files
- API updates for DataFlow nodes including method name changes and flow pattern adjustments
- Updates to expected test results reflecting changes in dataflow analysis output
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Multiple .expected files | Updated test expectations with new dataflow analysis results showing additional edges and nodes |
| Multiple .qll files | Migrated import statements from old to new dataflow library |
| Multiple .ql files | Updated dataflow API usage with new method names and flow patterns |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def.asDefiningArgument() = f and | ||
| DataFlow::localFlow(def, DataFlow::exprNode(e)) | ||
| exists(DataFlow::DefinitionByReferenceNode def, DataFlow::Node n | | ||
| f.getArgument(0) = def.getArgument() and |
There was a problem hiding this comment.
The API change from asDefiningArgument() to getArgument() may not be semantically equivalent. Consider verifying that def.getArgument() captures the same defining argument semantics as the original def.asDefiningArgument().
| f.getArgument(0) = def.getArgument() and | |
| f.getArgument(0) = def.asDefiningArgument() and |
Copilot uses AI. Check for mistakes.
| exists(DataFlow::DefinitionByReferenceNode def | | ||
| exists(DataFlow::DefinitionByReferenceNode def, DataFlow::Node va | | ||
| va.asIndirectExpr() = fileAccess.(VariableAccess) and | ||
| def.asDefiningArgument() = closedFile and |
There was a problem hiding this comment.
Inconsistent API usage: def.asDefiningArgument() is used here but replaced with def.getArgument() in other files. This should be updated for consistency with the new dataflow library migration.
| def.asDefiningArgument() = closedFile and | |
| def.getArgument() = closedFile and |
Copilot uses AI. Check for mistakes.
Commit-by-commit review recommended.