Skip to content
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: Model pointer read and write functions #18896

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

paldepind
Copy link
Contributor

@paldepind paldepind commented Feb 28, 2025

Adds models for ptr::read and ptr::write. Since these are rather low-level, modelling them allows us to generate new models for mem::replace, mem::take, Option::take, Option::replace, and a number of other things (that I haven't paid much attention to).

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Feb 28, 2025
@paldepind paldepind marked this pull request as ready for review February 28, 2025 11:47
@Copilot Copilot bot review requested due to automatic review settings February 28, 2025 11:47
@paldepind paldepind assigned paldepind and unassigned paldepind Feb 28, 2025
@paldepind paldepind requested a review from geoffw0 February 28, 2025 11:47

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@@ -14,7 +14,7 @@
| Macro calls - resolved | 2 |
| Macro calls - total | 2 |
| Macro calls - unresolved | 0 |
| Taint edges - number of edges | 1471 |
| Taint edges - number of edges | 1670 |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this row of output is something we should keep, or is it more bother than it's worth?

@@ -18,14 +18,15 @@ extensions:
- ["lang:alloc", "<crate::borrow::Cow as crate::ops::arith::AddAssign>::add_assign", "Argument[0]", "Argument[self].Reference", "value", "dfc-generated"]
- ["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"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When should the qualifier be Argument[self].Reference vs just Argument[self]? I thought self is always a reference?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants