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

Contracts & Harnesses for byte_add, byte_offset, and byte_offset_from #103

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
bcdedb8
verification for byte_add, byte_offset, and byte_offset_from
danielhumanmod Oct 5, 2024
009c561
remove redundant requires and add TODO
danielhumanmod Oct 23, 2024
a8974ed
Merge branch 'main' into daniel/byte_operation
danielhumanmod Oct 23, 2024
3a986c5
fix error for byte_offset()
danielhumanmod Oct 24, 2024
fff0e6a
add contracts with same_allocation
danielhumanmod Nov 1, 2024
7ce2d89
simplify contract for byte_offset
danielhumanmod Nov 5, 2024
459a692
refactor proof harness with pointer generator for byte_offset_from
danielhumanmod Nov 5, 2024
d95ad3e
Merge branch 'main' into daniel/byte_operation
danielhumanmod Nov 5, 2024
670adef
update code based on comments
danielhumanmod Nov 8, 2024
505f90b
Merge branch 'daniel/byte_operation' of https://github.com/danielhuma…
danielhumanmod Nov 8, 2024
e6c6795
update contracts
danielhumanmod Nov 13, 2024
293d9c8
remove unnecessary preconditions
danielhumanmod Nov 13, 2024
40b3833
fix comment issues
danielhumanmod Nov 14, 2024
42cb421
Merge remote-tracking branch 'origin' into daniel/byte_operation
danielhumanmod Nov 15, 2024
16bf419
adjust the usage of pointer generator
danielhumanmod Nov 15, 2024
8451bcb
unify contracts
danielhumanmod Nov 15, 2024
98a196a
fix overflow fail case
danielhumanmod Nov 16, 2024
fbccaa2
Merge branch 'main' of https://github.com/danielhumanmod/verify-rust-…
danielhumanmod Nov 27, 2024
c1b4488
Merge branch 'main' of https://github.com/danielhumanmod/verify-rust-…
danielhumanmod Nov 27, 2024
48b30e5
Merge branch 'main' into daniel/byte_operation
danielhumanmod Nov 28, 2024
0e11499
Merge branch 'main' into daniel/byte_operation
danielhumanmod Nov 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions library/core/src/ptr/non_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,11 @@ impl<T: ?Sized> NonNull<T> {
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
#[stable(feature = "non_null_convenience", since = "1.80.0")]
#[rustc_const_stable(feature = "non_null_convenience", since = "1.80.0")]
#[requires(

Choose a reason for hiding this comment

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

I believe it's safe to call this function with count == 0 even if self.pointer is dangling.

Copy link
Author

@danielhumanmod danielhumanmod Nov 20, 2024

Choose a reason for hiding this comment

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

I believe it's safe to call this function with count == 0 even if self.pointer is dangling.

Appreciate the suggestion @celinval! But there is some failure with the my dangling proof:

Proof harness:

#[kani::proof_for_contract(NonNull::byte_add)]
    pub fn non_null_byte_add_dangling_proof() {
        let ptr = NonNull::<i32>::dangling();
        let count: usize = 0;
        unsafe {
            let _ = ptr.byte_add(count);
        }
    }

Error:

VERIFICATION RESULT:
 ** 1 of 178 failed
Failed Checks: Kani does not support reasoning about pointer to unallocated memory
 File: "/Users/danieltu/Developer/verify-rust-std/library/core/src/lib.rs", line 423, in kani::mem::cbmc::same_allocation::<i32>

VERIFICATION:- FAILED
Verification Time: 0.26384938s

I am not sure whether it is because I didn't create a proper dangling scenario, could you provide some suggestion on this? Thanks!

Choose a reason for hiding this comment

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

Did you extend the requires clause?

Copy link
Author

Choose a reason for hiding this comment

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

Did you extend the requires clause?

Hi @celinval, do you mind being more specific about it? Based on my understanding, we need to add a proof harness where ptr is dangling and count = 0. Any additional contract need to be added into the API?

(self.as_ptr().addr() as isize).checked_add(count).is_some() &&
kani::mem::same_allocation(self.as_ptr(), self.as_ptr().wrapping_byte_offset(count))
)]
#[ensures(|result: &Self| result.as_ptr() == self.as_ptr().wrapping_byte_offset(count))]
pub const unsafe fn byte_offset(self, count: isize) -> Self {
// SAFETY: the caller must uphold the safety contract for `offset` and `byte_offset` has
// the same safety contract.
Expand Down Expand Up @@ -613,6 +618,10 @@ impl<T: ?Sized> NonNull<T> {
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
#[stable(feature = "non_null_convenience", since = "1.80.0")]
#[rustc_const_stable(feature = "non_null_convenience", since = "1.80.0")]
#[requires(
count <= (isize::MAX as usize) - self.as_ptr().addr() && // Ensure the offset doesn't overflow
kani::mem::same_allocation(self.as_ptr(),self.as_ptr().wrapping_byte_add(count)) // Ensure the offset is within the same allocation
danielhumanmod marked this conversation as resolved.
Show resolved Hide resolved
)]
pub const unsafe fn byte_add(self, count: usize) -> Self {
// SAFETY: the caller must uphold the safety contract for `add` and `byte_add` has the same
// safety contract.
Expand Down Expand Up @@ -826,6 +835,11 @@ impl<T: ?Sized> NonNull<T> {
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
#[stable(feature = "non_null_convenience", since = "1.80.0")]
#[rustc_const_stable(feature = "non_null_convenience", since = "1.80.0")]
#[requires(kani::mem::same_allocation(self.as_ptr() as *const(), origin.as_ptr() as *const()))]

Choose a reason for hiding this comment

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

This should also be safe if the pointers hold the same address, even if they are dangling

Copy link
Author

Choose a reason for hiding this comment

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

This should also be safe if the pointers hold the same address, even if they are dangling

Same as above, there is a failure:

Proof harness

#[kani::proof_for_contract(NonNull::byte_offset_from)]
    pub fn non_null_byte_offset_from_dangling_proof() {
        let origin = NonNull::<i32>::dangling();
        let ptr = origin;
        unsafe {
            let _ = ptr.byte_offset_from(origin);
        }
    }

Error:

VERIFICATION RESULT:
 ** 1 of 184 failed (1 unreachable)
Failed Checks: Kani does not support reasoning about pointer to unallocated memory
 File: "/Users/danieltu/Developer/verify-rust-std/library/core/src/lib.rs", line 423, in kani::mem::cbmc::same_allocation::<()>

VERIFICATION:- FAILED
Verification Time: 0.24433088s

Choose a reason for hiding this comment

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

Please see my question above.

#[ensures(
|result: &isize|
*result == (self.as_ptr() as *const u8).offset_from(origin.as_ptr() as *const u8)
)]
pub const unsafe fn byte_offset_from<U: ?Sized>(self, origin: NonNull<U>) -> isize {
// SAFETY: the caller must uphold the safety contract for `byte_offset_from`.
unsafe { self.pointer.byte_offset_from(origin.pointer) }
Expand Down Expand Up @@ -1907,6 +1921,7 @@ impl<T: ?Sized> From<&T> for NonNull<T> {
mod verify {
use super::*;
use crate::ptr::null_mut;
use crate::mem;
use kani::PointerGenerator;

trait SampleTrait {
Expand Down Expand Up @@ -2473,4 +2488,54 @@ mod verify {
let distance = ptr_nonnull.offset_from(origin_nonnull);
}
}

#[kani::proof_for_contract(NonNull::byte_add)]
pub fn non_null_byte_add_proof() {
const ARR_SIZE: usize = mem::size_of::<i32>() * 1000;
let mut generator = PointerGenerator::<ARR_SIZE>::new();

let count: usize = kani::any();
let raw_ptr: *mut i32 = generator.any_in_bounds().ptr as *mut i32;

unsafe {
let ptr = NonNull::new(raw_ptr).unwrap();
let result = ptr.byte_add(count);
}
}

#[kani::proof_for_contract(NonNull::byte_offset)]
pub fn non_null_byte_offset_proof() {
const ARR_SIZE: usize = mem::size_of::<i32>() * 1000;
let mut generator = PointerGenerator::<ARR_SIZE>::new();

let count: isize = kani::any();
let raw_ptr: *mut i32 = generator.any_in_bounds().ptr as *mut i32;

unsafe {
let ptr = NonNull::new(raw_ptr).unwrap();
let result = ptr.byte_offset(count);
}
}

#[kani::proof_for_contract(NonNull::byte_offset_from)]
pub fn non_null_byte_offset_from_proof() {
const SIZE: usize = mem::size_of::<i32>() * 1000;
let mut generator1 = PointerGenerator::<SIZE>::new();
danielhumanmod marked this conversation as resolved.
Show resolved Hide resolved
let mut generator2 = PointerGenerator::<SIZE>::new();

let ptr: *mut i32 = generator1.any_in_bounds().ptr as *mut i32;

let origin: *mut i32 = if kani::any() {
generator1.any_in_bounds().ptr as *mut i32
} else {
generator2.any_in_bounds().ptr as *mut i32
};

let ptr_nonnull = unsafe { NonNull::new(ptr).unwrap() };
let origin_nonnull = unsafe { NonNull::new(origin).unwrap() };

unsafe {
let result = ptr_nonnull.byte_offset_from(origin_nonnull);
}
}
}