-
Notifications
You must be signed in to change notification settings - Fork 48
NonZero (unchecked_mul & unchecked_add) Proof for Contracts #338
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
base: main
Are you sure you want to change the base?
Conversation
Added contract and placeholder harness
Corrected issues from Pull Request
Restored previous cargo files
Co-authored-by: Carolyn Zech <cmzech@amazon.com>
Co-authored-by: Carolyn Zech <cmzech@amazon.com>
Co-authored-by: Carolyn Zech <cmzech@amazon.com>
Co-authored-by: Carolyn Zech <cmzech@amazon.com>
Co-authored-by: Zyad Hassan <88045115+zhassan-aws@users.noreply.github.com>
library/core/src/num/nonzero.rs
Outdated
|
||
macro_rules! nonzero_check_add { | ||
($t:ty, $nonzero_type:ty, $nonzero_check_unchecked_add_for:ident) => { | ||
#[kani::proof_for_contract(<$t>::unchecked_add)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#[kani::proof_for_contract(<$t>::unchecked_add)] | |
#[kani::proof_for_contract(<$nonzero_type>::unchecked_add)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that would not produce the right type. We want something like <i8>::unchecked_add
, not <NonZeroI8>::unchecked_add
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But taking u8
as my example, won't that produce a proof for https://doc.rust-lang.org/std/primitive.u8.html#method.unchecked_add instead of https://doc.rust-lang.org/std/num/struct.NonZero.html#method.unchecked_add?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to use your proposed patch, but then I get:
error: Failed to resolve checking function < core::num::NonZeroU8 > :: unchecked_add because unable to find `core` inside module `num::nonzero::verify`
--> /verify-rust-std.git/library/core/src/num/nonzero.rs:2875:13
|
2875 | #[kani::proof_for_contract(<$nonzero_type>::unchecked_add)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
2887 | nonzero_check_add!(u8, core::num::NonZeroU8, nonzero_check_unchecked_add_for_u8);
| -------------------------------------------------------------------------------- in this macro invocation
|
= note: this error originates in the attribute macro `kani::proof_for_contract` which comes from the expansion of the macro `nonzero_check_add` (in Nightly builds, run with -Z macro-backtrace for more info)
Maybe I'm doing something wrong, but the one with $t
is really the only one I was able to make work so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were not doing anything wrong, this is a Kani bug. I've verified that with the changes from model-checking/kani#4051, this:
#[kani::proof_for_contract(NonZero::<$t>::unchecked_add)]
compiles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in the meantime, this PR can change all the targets of the proof_for_contract
to be correct, and then we'll have to wait for Kani to update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Argh, thank you so much for getting this right! I have updated the PR, so expecting CI to fail for the time being.
Co-authored-by: Carolyn Zech <carolynzech@gmail.com>
Co-authored-by: Carolyn Zech <carolynzech@gmail.com>
Co-authored-by: Carolyn Zech <carolynzech@gmail.com>
Co-authored-by: Carolyn Zech <carolynzech@gmail.com>
Co-authored-by: Carolyn Zech <carolynzech@gmail.com>
Towards #71
These are the changes contributed by SahithiMV in #184 plus the proposed changes in that PR.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.