-
Notifications
You must be signed in to change notification settings - Fork 41
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 len, is_empty, is_aligned, and is_aligned_to #128
Contracts & Harnesses for len, is_empty, is_aligned, and is_aligned_to #128
Conversation
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.
Awesome! I just have a few high level comments:
- Can you please use
safety::ensures
andsafety::requires
instead ofkani
ones? - Can you also check the tests with dangling pointers? For example, you could use
PointerGenerator
, or multiple harnesses.
@Jimmycreative See pointer generator documentation here: https://model-checking.github.io/kani/crates/doc/kani/struct.PointerGenerator.html |
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.
Thank you!
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.
It looks good. Can you please ensure that calling is_aligned_to
with a invalid alignment value (not power of two) will not trigger UB. One way you could do that is to write a regular harness, i.e., a #[kani::proof]
instead of #[kani::proof_for_contract(<func>)]
so Kani does not assume the pre-condition. If the harness fails due to panic, you can add a #[kani::should_panic]
to the harness.
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.
Looks good! Thanks
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.
Nice job! Once you fix the conflicts, please @ me so that I can trigger the merge workflow.
Co-authored-by: Carolyn Zech <cmzech@amazon.com>
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.
trigger merge workflow
Towards #53
Changes
added contract and harness for len, is_empty, is_aligned, and is_aligned_to in the non-null library.