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

Proofs for String::remove, Vec::swap_remove, Option::as_slice, and VecDeque::swap #212

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

Conversation

stogaru
Copy link

@stogaru stogaru commented Dec 6, 2024

Closes #76

Changes

  • Adds proofs for the following functions using raw pointer operations:
    • String::remove
    • Vec::swap_remove
    • Option::as_slice
    • VecDeque::swap
  • ideally the usages should have been verified by stubbing the contracts for reaw pointer operations like byte_add, add and offset, but stubbing cannot be done for these functions at this time due to Compilation Error When Stubbing Functions That Return Pointers Using Function Contracts kani#3732
  • Marks Challenge 3 as Resolved and changes its end date.
  • Adds contributors.

PoCs:

PRs that need to be merged before this PR:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

xsxszab and others added 30 commits October 7, 2024 12:44
implemented integer type proof for contract for fn add, sub and offset
Combines macros for different types.
…allocation api, modified their proof for harness accordingly
@stogaru stogaru requested a review from a team as a code owner December 6, 2024 09:07
@feliperodri
Copy link

@stogaru Update the description of the PR to close the issue, update the goal's document stating the challenge is completed, and list the previous PRs that need to be merged before we can evaluate this PR and complete the goal.

@stogaru
Copy link
Author

stogaru commented Dec 8, 2024

@feliperodri, I made the required changes.

library/core/src/option.rs Outdated Show resolved Hide resolved
library/core/src/option.rs Outdated Show resolved Hide resolved
library/alloc/src/collections/vec_deque/mod.rs Outdated Show resolved Hide resolved
Comment on lines +3117 to +3122
// Ensure other elements remain unchanged
for k in 0..len {
if k != i && k != j {
assert_eq!(deque[k], arr[k]);
}
}

Choose a reason for hiding this comment

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

We don't need a loop here, this assertion could be done symbolically.

Suggested change
// Ensure other elements remain unchanged
for k in 0..len {
if k != i && k != j {
assert_eq!(deque[k], arr[k]);
}
}
// Ensure other elements remain unchanged
let k = kani::any_where(|&x: &usize| x < len);
assert!(implies!(k != i && k !=j, deque[k] == arr[k]));

Copy link
Author

Choose a reason for hiding this comment

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

@feliperodri , how do I import implies macro?

use kani::{ensures, implies};

The above code gives me the following error:

error[E0432]: unresolved import `kani::implies`
    --> /Users/user/Documents/Education/CMU/Academics/2024Fall/14798/verify-rust-std/library/alloc/src/collections/vec_deque/mod.rs:3091:25
     |
3091 |     use kani::{ensures, implies};
     |                         ^^^^^^^ no `implies` in `kani`

Choose a reason for hiding this comment

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

I made a mistake when I wrote the implies! macro previously.

use kani::{implies}
...
assert!(implies!(k != i && k !=j => deque[k] == arr[k]));
...

However, you don't need the implication here at all if that doesn't work. If it makes it more clear, you can just use an if statement. The main point here is to avoid the unnecessary loop.

///
/// The harness checks:
/// 1. The string length decreases by one after the `remove` operation.
/// 2. The removed character is valid ASCII.

Choose a reason for hiding this comment

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

Shouldn't it be a valid UTF-8?

/// 2. The removed character is valid ASCII.
/// 3. The removed character matches the character at the specified index in the original string.
///
/// This ensures the `remove` function behaves as expected for constrained inputs.

Choose a reason for hiding this comment

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

What do you mean by that? This line might not be necessary.

///
/// This ensures the `remove` function behaves as expected for constrained inputs.
fn check_remove() {
// array size is chosen because it is small enough to be feasible to check exhaustively

Choose a reason for hiding this comment

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

Could you give us a insight of the verification performance here?

Choose a reason for hiding this comment

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

@qinheping would we be possible to use your loop invariants for valid UTF-8 chars in this harness?

Comment on lines +3247 to +3248
// Convert byte array to a String directly (safe since all are ASCII)
let mut s = unsafe { String::from_utf8_unchecked(arr.to_vec()) };

Choose a reason for hiding this comment

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

https://doc.rust-lang.org/book/ch08-02-strings.html

It should be UTF-8 encoded, not only ASCII.

Comment on lines +4073 to +4078
// Check that all other unaffected elements remain unchanged
for i in 0..vec.len() {
if i != index && i < original_len - 1 {
assert!(vec[i] == original_vec[i], "Unaffected elements should remain unchanged");
}
}

Choose a reason for hiding this comment

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

This can be done symbolically. See previous comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Challenge 3: Verifying Raw Pointer Arithmetic Operations
6 participants