Rust: Model pointer read and write functions#18896
Conversation
c6f56fc to
c4773c4
Compare
There was a problem hiding this comment.
PR Overview
This PR extends the Rust models by incorporating pointer read and write functions and updating the custom option model to use a dedicated replace function.
- Added a new module in the modeled tests to simulate pointer operations using unsafe read/write.
- Updated the custom MyOption implementations to use a new replace function instead of mem::replace.
- Extended the Rust standard library model to include pointer operations.
Reviewed Changes
| File | Description |
|---|---|
| rust/ql/test/library-tests/dataflow/modeled/main.rs | Adds a new module for pointer read/write tests and adjusts import ordering and punctuation. |
| rust/ql/test/utils-tests/modelgenerator/option.rs | Introduces a custom replace function and updates MyOption methods to use it. |
| rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml | Augments the core model with pointer operation summaries. |
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
rust/ql/test/utils-tests/modelgenerator/option.rs:13
- [nitpick] The custom function name 'replace' may be confused with std::mem::replace. Consider renaming it to something more distinctive, such as 'custom_replace'.
pub fn replace<T>(dest: &mut T, src: T) -> T {
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
f5c436f to
77e1161
Compare
77e1161 to
c1ee20b
Compare
| | Macro calls - total | 2 | | ||
| | Macro calls - unresolved | 0 | | ||
| | Taint edges - number of edges | 1471 | | ||
| | Taint edges - number of edges | 1670 | |
There was a problem hiding this comment.
I wonder if this row of output is something we should keep, or is it more bother than it's worth?
There was a problem hiding this comment.
It's nice to see the number go up. On the other hand, I haven't figured out how to run these locally so I've been updating them manually from the CIs output. Is there a way to run them locally?
There was a problem hiding this comment.
I think you can run the integration tests just by clicking within VSCode.
But this has been frustrating me as I never remember to run them, and sometimes they generate merge conflicts. I've started a conversation (elsewhere) about reducing the amount of stats we generate for these projects.
There was a problem hiding this comment.
Update: you're right, the integration tests are hard to run locally. :(
| - ["lang:alloc", "<crate::borrow::Cow as crate::ops::deref::Deref>::deref", "Argument[self].Reference.Field[crate::borrow::Cow::Borrowed(0)]", "ReturnValue", "value", "dfc-generated"] | ||
| - ["lang:alloc", "<crate::borrow::Cow>::into_owned", "Argument[self].Field[crate::borrow::Cow::Owned(0)]", "ReturnValue", "value", "dfc-generated"] | ||
| - ["lang:alloc", "<crate::borrow::Cow>::to_mut", "Argument[self].Reference.Field[crate::borrow::Cow::Owned(0)]", "ReturnValue", "value", "dfc-generated"] | ||
| - ["lang:alloc", "<crate::borrow::Cow>::to_mut", "Argument[self].Reference.Field[crate::borrow::Cow::Owned(0)]", "ReturnValue.Reference", "value", "dfc-generated"] |
There was a problem hiding this comment.
When should the qualifier be Argument[self].Reference vs just Argument[self]? I thought self is always a reference?
There was a problem hiding this comment.
When a method takes self without & or &mut (for instance Option::unwrap) then self is not a reference but passed by value.
So roughly, Argument[self].Reference corresponds to &self and &mut self, and Argument[self] corresponds to Argument[self].
Here Cow::to_mut takes &mut self and dereferences the self pointer which gives rise to Argument[self].Reference.
Adds models for
ptr::readandptr::write. Since these are rather low-level, modelling them allows us to generate new models formem::replace,mem::take,Option::take,Option::replace, and a number of other things (that I haven't paid much attention to).