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 unchecked_add #91

Conversation

Yenyun035
Copy link

@Yenyun035 Yenyun035 commented Sep 20, 2024

Resolves #59

Changes

  • Added contracts for unchecked_add (located in library/core/src/num/int_macros.rs and uint_macros.rs)
  • Added a harness for unchecked_add of each integer type
    • i8, i16, i32, i64, i128, isize, u8, u16, u32, u64, u128, usize --- 12 harnesses in total.

Revalidation

  1. Per the discussion in Challenge 11: Safety of Methods for Numeric Primitive Types #59, we have to build and run Kani from feature/verify-rust-std branch.
  2. To revalidate the verification results, run the following command. <harness_to_run> can be either num::verify to run all harnesses or num::verify::<harness_name> (e.g. check_unchecked_add_i8) to run a specific harness.
kani verify-std  "path/to/library" \
    --harness <harness_to_run> \
    -Z unstable-options \
    -Z function-contracts \
    -Z mem-predicates

Except isize and usize harnesses passing 1203 checks, all harnesses should pass the same 1229 checks (1200 checks on 09/24/2024):

SUMMARY:
 ** 0 of 1229 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 2.4755886s

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

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

@Yenyun035 Yenyun035 requested a review from a team as a code owner September 20, 2024 20:47
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.

That's awesome! Thanks

Have you tried using a macro for generating the harness? I think it will be cleaner, but not a requirement.

library/core/src/num/int_macros.rs Outdated Show resolved Hide resolved
library/core/src/num/mod.rs Outdated Show resolved Hide resolved
library/core/src/num/uint_macros.rs Outdated Show resolved Hide resolved
library/core/src/num/mod.rs Outdated Show resolved Hide resolved
@Yenyun035
Copy link
Author

I haven't tried withproc_macro. I'll look into it. Thank you for reviewing this PR :)

@celinval
Copy link

I haven't tried withproc_macro. I'll look into it. Thank you for reviewing this PR :)

Sorry, my bad. I meant just regular macros

@Yenyun035
Copy link
Author

Sorry, my bad. I meant just regular macros

Do you mean "declarative macros"? How could it be used? I guess the macro can take the method to verify and the input type (e.g. i8, i16). I'm not familiar with it. I'll research it more.

@feliperodri feliperodri changed the title AWS-Team 2: Contracts & Harnesses for unchecked_add Contracts & Harnesses for unchecked_add Sep 27, 2024
yew005 and others added 3 commits September 29, 2024 19:47
* Added harnesses for unchecked multiplication (`unchecked_mul`) and shift right (`unchecked_shr`)
* Added a macro and input limits for multiplication proofs
* Reduced duplicity in code by using macros to generate proof harnesses
@Yenyun035 Yenyun035 changed the title Contracts & Harnesses for unchecked_add Contracts & Harnesses for unchecked_add, unchecked_mul, unchecked_shr Sep 30, 2024
@Yenyun035 Yenyun035 changed the title Contracts & Harnesses for unchecked_add, unchecked_mul, unchecked_shr Contracts & Harnesses for unchecked_add Oct 2, 2024
@Yenyun035
Copy link
Author

@feliperodri @celinval Hello! The previous build failed due to an unused import. I fixed that and all looks good now. Could you approve the workflow? Once it passes the checks, we'll get this PR merged. Thank you :)

@feliperodri feliperodri merged commit 1bf01fd into model-checking:main Oct 4, 2024
7 checks passed
@danielhumanmod
Copy link

Hi @Yenyun035, I am the member of AWS team 4, after our team merge the main branch with this commit, there is a compiling error happens, like:

error: Failed to resolve checking function i8 :: unchecked_add because Kani currently cannot resolve path including primitive types
    --> /Users/danieltu/Developer/verify-rust-std/library/core/src/num/mod.rs:1596:13
     |
1596 |             #[kani::proof_for_contract($type::$method)]
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
1645 |     generate_unchecked_math_harness!(i8, unchecked_add, checked_unchecked_add_i8);
     |     ----------------------------------------------------------------------------- in this macro invocation
     |
     = note: this error originates in the attribute macro `kani::proof_for_contract` which comes from the expansion of the macro `generate_unchecked_math_harness` (in Nightly builds, run with -Z macro-backtrace for more info)

I was wondering if this changes using some custom version of Kani?

@QinyuanWu
Copy link

QinyuanWu commented Oct 5, 2024

Hi @Yenyun035, I am the member of AWS team 4, after our team merge the main branch with this commit, there is a compiling error happens, like:

error: Failed to resolve checking function i8 :: unchecked_add because Kani currently cannot resolve path including primitive types
    --> /Users/danieltu/Developer/verify-rust-std/library/core/src/num/mod.rs:1596:13
     |
1596 |             #[kani::proof_for_contract($type::$method)]
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
1645 |     generate_unchecked_math_harness!(i8, unchecked_add, checked_unchecked_add_i8);
     |     ----------------------------------------------------------------------------- in this macro invocation
     |
     = note: this error originates in the attribute macro `kani::proof_for_contract` which comes from the expansion of the macro `generate_unchecked_math_harness` (in Nightly builds, run with -Z macro-backtrace for more info)

I was wondering if this changes using some custom version of Kani?

We tried using the -Z macro-backtrace flag but it's not supported in our installed kani version:

error: invalid value 'macro-backtrace' for '--unstable <UNSTABLE_FEATURE>'
  [possible values: stubbing, gen-c, c-ffi, concrete-playback, async-lib, source-coverage, function-contracts, mem-predicates, valid-value-checks, ghost-state, uninit-checks, unstable-options]
which kani
/Users/owo/.cargo/bin/kani

This compilation error prevents us from running any individual harnesses in the library.
@zhassan-aws @feliperodri

@Yenyun035
Copy link
Author

Yenyun035 commented Oct 5, 2024

@QinyuanWu @danielhumanmod Hi. Regarding your question about kani, yes. In our case, we need to manually build kani from source (features/verify-rust-std branch) to run proofs because the current kani release (installed via cargo install) does not support resolving paths with primitive types. This was discussed in our Challenge Issue #59. Here are our steps to build kani from source:

Build from source code

  1. Clone the repo: git clone https://github.com/model-checking/kani.git
  2. Cd into kani repo
  3. git submodule update --init
  4. Install dependencies: ./scripts/setup/macos/install_deps.sh
    • If failed on XCode CLT: try remove and reinstall OR see system settings > software update
    • If you want to Install dependencies manually on MacOS
      • (latest) brew install cbmc
      • (latest) CBMC-viewer
      • Kissat: Run kani/scripts/setup/install_kissat.sh
  5. source $HOME/.cargo/env
  6. git checkout features/verify-rust-std
  7. cargo build-dev
  8. For all Kani commands, run with {path/to/kani/repo}/scripts/kani

carolynzech pushed a commit that referenced this pull request Oct 7, 2024
…ed_shl` and `unchecked_shr` (#96)

Towards : issue #59 

Parent branch :
[c-0011-core-nums-yenyunw-unsafe-ints](https://github.com/rajathkotyal/verify-rust-std/tree/c-0011-core-nums-yenyunw-unsafe-ints
) - Tracking PR #91

---------

Co-authored-by: yew005 <yew005@ucsd.edu>
Co-authored-by: MWDZ <jinjunfeng721@gmail.com>
Co-authored-by: Lanfei Ma <99311320+lanfeima@users.noreply.github.com>
Co-authored-by: Yenyun035 <yenyunw@andrew.cmu.edu>
Dhvani-Kapadia added a commit to danielhumanmod/verify-rust-std that referenced this pull request Oct 7, 2024
carolynzech added a commit that referenced this pull request Nov 14, 2024
Towards #53

## Changes
Three function contracts & four harnesses:
- added contract and harness for `non_null::add`
- added contract and harness for `non_null::addr`
- added contract and harnesses for `non_null::align_offset`, including
both positive and negative harness that triggers panic. The ensures
clause for `align_offset` is referenced from
[`align_offset`](https://github.com/model-checking/verify-rust-std/pull/69/files)
in `library/core/src/ptr/mod.rs`.


## 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`. This will run all six
harnesses in the module. All default checks should pass:
```
SUMMARY:
 ** 0 of 1556 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 0.28004378s

Complete - 6 successfully verified harnesses, 0 failures, 6 total.
```
### :exclamation: Warning
Running the above command with the default installed cargo kani will
result in compilation error due to the latest merged from
[PR#91](#91).
Detailed errors are commented under that PR. This issue is waiting to be
resolved.

## TODO:

- Use `Layout` to create dynamically sized arrays in place of fixed size
array in harnesses. This approach currently has errors and is documented
in
[discussion](#104).
- Verify multiple data types: these will be added in future PR.
- Add `requires` clause in contract to constrain `count` to be within
object memory size: there is a current
[issue](#99)
with using `ub_checks::can_write` to get the object size. A workaround
is implemented in the harness.

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

---------

Co-authored-by: Carolyn Zech <cmzech@amazon.com>
@Yenyun035 Yenyun035 deleted the c-0011-core-nums-yenyunw-unsafe-ints branch November 27, 2024 08:10
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 11: Safety of Methods for Numeric Primitive Types
6 participants