Rust: Add generated models for standard libraries including core #18787
Conversation
paldepind
force-pushed
the
rust-core-std-models
branch
2 times, most recently
from
7bce170 to
bfb716b
Compare
paldepind
force-pushed
the
rust-core-std-models
branch
from
bfb716b to
0c3e8a0
Compare
There was a problem hiding this comment.
PR Overview
This PR adds generated models for standard Rust libraries (core, std, alloc, and proc_macro) and updates the associated tests. The key changes are:
- Updating a test annotation in dataflow/strings/main.rs from "hasTaintFlow" to "hasValueFlow".
- Removing several taint and value model entries in lang-core.model.yml to better scale with the new models.
Changes
| File | Description |
|---|---|
| rust/ql/test/library-tests/dataflow/strings/main.rs | Updated test comment annotation to reflect new model expectations. |
| rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml | Removed several model entries to adjust for increased model volume. |
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
rust/ql/test/library-tests/dataflow/strings/main.rs:53
- Please ensure that changing the annotation from 'hasTaintFlow' to 'hasValueFlow' aligns with the updated test expectations and model semantics.
sink(s2); // $ hasValueFlow=36
rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml:7
- Review the removal of the model entry for 'crate::hint::must_use' to ensure that tests or taint propagation flows are still adequately covered.
- - ["lang:core", "crate::hint::must_use", "Argument[0]", "ReturnValue", "value", "manual"]
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
| input = "Argument[self]" and | ||
| output = "ReturnValue" and | ||
| preservesValue = true and | ||
| model = "generated" |
There was a problem hiding this comment.
Previously this had a model of "" and it seemed to be disabled/overwritten by the generated models. The generated models include a model for clone on i64, which caused the test for this method to fail. Changing the model to generated or manual fixed the problem. I just went with generated without worrying too much as this is temporary anyway.
| @@ -11,6 +11,8 @@ private module Input implements InputSig<Location, RustDataFlow> { | |||
| not exists(n.asExpr().getLocation()) | |||
| } | |||
|
|
|||
| predicate postWithInFlowExclude(RustDataFlow::Node n) { n instanceof Node::FlowSummaryNode } | |||
There was a problem hiding this comment.
This fixes some data flow inconsistencies otherwise introduced by the new models. Ruby and C# have the same, so I think this is appropriate.
| @@ -14,7 +14,7 @@ | |||
| | Macro calls - resolved | 2 | | |||
| | Macro calls - total | 2 | | |||
| | Macro calls - unresolved | 0 | | |||
| | Taint edges - number of edges | 4 | | |||
| | Taint edges - number of edges | 1465 | | |||
There was a problem hiding this comment.
A 366x increase in taint edges 📈 😃
|
DCA shows taint reach going down by 1 on the iced project. That's unexpected, but in the tests things look good, so I don't thing there's much to worry about. |
This adds generated models for some of the standard Rust libraries,
core,std,alloc, andproc_macro.We had some test that created
.expectedoutput growing with the number of models or taint steps caused by models. That didn't scale well to the new amount of models, so I've tweaked those tests.