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 non_null::new and non_null::new_unchecked #88

Merged
merged 13 commits into from
Sep 27, 2024

Conversation

QinyuanWu
Copy link

@QinyuanWu QinyuanWu commented Sep 18, 2024

Towards #53

Changes

  • added contract and harness for non_null::new
  • added contract and harness for non_null::new_unchecked

The difference between the two APIs is that non_null::new can handle null pointers while non_null::new_unchecked does not. Therefore the contract for non_null::new does not require a nonnull pointer.

Revalidation

To revalidate the verification results, run kani verify-std -Z unstable-options "path/to/library" -Z function-contracts -Z mem-predicates --harness ptr::non_null::verify::non_null_check_new. This will run both harnesses. All default checks should pass:

Kani Rust Verifier 0.55.0 (standalone)
Checking harness ptr::non_null::verify::non_null_check_new...
...
SUMMARY:
 ** 0 of 1849 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 3.5815656s

Checking harness ptr::non_null::verify::non_null_check_new_unchecked...
...
SUMMARY:
 ** 0 of 1559 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 0.25408816s

Complete - 2 successfully verified harnesses, 0 failures, 2 total.

@QinyuanWu QinyuanWu requested a review from a team as a code owner September 18, 2024 18:44
Copy link

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

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

Nice work! A few comments.

library/core/src/ptr/non_null.rs Outdated Show resolved Hide resolved
library/core/src/ptr/non_null.rs Outdated Show resolved Hide resolved
library/core/src/ptr/non_null.rs Outdated Show resolved Hide resolved
library/core/src/ptr/non_null.rs Outdated Show resolved Hide resolved
library/core/src/ptr/non_null.rs Outdated Show resolved Hide resolved
library/core/src/ptr/non_null.rs Outdated Show resolved Hide resolved
library/core/src/ptr/non_null.rs Outdated Show resolved Hide resolved
@QinyuanWu QinyuanWu changed the title Contracts & Harnesses for non_null::new and non_null::new_unchecked AWS Team 4 Contracts & Harnesses for non_null::new and non_null::new_unchecked Sep 19, 2024
@QinyuanWu QinyuanWu changed the title AWS Team 4 Contracts & Harnesses for non_null::new and non_null::new_unchecked AWS-Team-4 Contracts & Harnesses for non_null::new and non_null::new_unchecked Sep 19, 2024
@QinyuanWu
Copy link
Author

@zhassan-aws if everything else looks good I will squash the commits.

Copy link

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

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

Thanks!

@zhassan-aws
Copy link

@zhassan-aws if everything else looks good I will squash the commits.

No need to. We squash on merge.

@QinyuanWu
Copy link
Author

@zhassan-aws Thank you! Do we need another reviewer for approval to merge?

@zhassan-aws
Copy link

Yes.

Copy link

@celinval celinval left a comment

Choose a reason for hiding this comment

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

I think it would be great to have harnesses that covers all possible values of the raw pointer. Something like:

let ptr = kani::any::<usize>() as *mut i32;
let _ = NonNull::new_unchecked(ptr);

Note that we cannot use the same strategy for functions where we need to ensure pointer validity.

Copy link

@celinval celinval left a comment

Choose a reason for hiding this comment

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

Awesome! thanks

@feliperodri feliperodri changed the title AWS-Team-4 Contracts & Harnesses for non_null::new and non_null::new_unchecked Contracts & Harnesses for non_null::new and non_null::new_unchecked Sep 27, 2024
@feliperodri feliperodri enabled auto-merge (squash) September 27, 2024 17:21
@feliperodri feliperodri merged commit 024e117 into model-checking:main Sep 27, 2024
7 of 8 checks passed
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.

4 participants