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

Challenge 11: Safety of Methods for Numeric Primitive Types #59

Open
carolynzech opened this issue Aug 20, 2024 · 11 comments · Fixed by #91, #102 or #112 · May be fixed by #134
Open

Challenge 11: Safety of Methods for Numeric Primitive Types #59

carolynzech opened this issue Aug 20, 2024 · 11 comments · Fixed by #91, #102 or #112 · May be fixed by #134
Assignees
Labels
Challenge Used to tag a challenge

Comments

@carolynzech
Copy link

carolynzech commented Aug 20, 2024

Link to challenge: https://model-checking.github.io/verify-rust-std/challenges/0011-floats-ints.html

@carolynzech carolynzech added the Challenge Used to tag a challenge label Aug 20, 2024
@carolynzech carolynzech changed the title Tracking Issue for Verification of Floats and Integers Tracking Issue for Verification of Numeric Primitives Aug 23, 2024
@feliperodri feliperodri changed the title Tracking Issue for Verification of Numeric Primitives Challenge 11: Safety of Methods for Numeric Primitive Types Sep 5, 2024
@Yenyun035
Copy link

Hello! We are CMU Team 2 and will be working on this challenge :)

Our team:

@Yenyun035
Copy link

Hello, this is Team 2 :) We have a question regarding how to write proofs.

Per Part 1 of the challenge, we will need to write proofs for unchecked_* methods. We referred to other harnesses in the repo and found that all of them use the kani::proof_for_contract() attribute instead of kani::proof. According to our understanding from this function contract doc, we can use kani::proof_for_contract() when verifying functions that have requires() and/or ensures(). However, in our case, unchecked_* methods in int_macros.rs and uint_macros.rs do not have any contracts. Therefore, we came up with two approaches and are wondering which to proceed with:

  1. kani::proof_for_contract() + a wrapper with contracts around unchecked_* methods. For example:
#[kani::requires(!num1.overflowing_add(num2).1)]
#[kani::ensures(|ret| *ret >= i8::MIN && *ret <= i8::MAX)]
fn i8_unchecked_add_wrapper(num1: i8, num2: i8) -> i8 {
    unsafe { num1.unchecked_add(num2) }
}

// pub const unsafe fn unchecked_add(self, rhs: Self) -> Self
#[kani::proof_for_contract(i8_unchecked_add_wrapper)]
pub fn check_unchecked_add_i8() {
    let num1: i8 = kani::any::<i8>();
    let num2: i8 = kani::any::<i8>();

    unsafe {
        // Kani ensures the inputs satisfy preconditions
        i8_unchecked_add_wrapper(num1, num2);
        // Kani ensures the output satisfies postconditions
    }
}
  1. kani::proof + kani::assume() for preconditions and assert() for postconditions. For example:
// pub const unsafe fn unchecked_add(self, rhs: Self) -> Self
#[kani::proof]
pub fn check_unchecked_add_i16() {
    let num1: i16 = kani::any::<i16>();
    let num2: i16 = kani::any::<i16>();
        
    // overflowing_add return (result, bool) where bool is if
    // add will overflow. This check takes the boolean. 
    kani::assume(!num1.overflowing_add(num2).1);

    unsafe {
        let result = num1.unchecked_add(num2);

        // Either check result
        assert_eq!(Some(result), num1.checked_add(num2));
            
        // Or check the range of result
        assert!(result >= i16::MIN && result <= i16::MAX);
    }
}

Thank you for any guidance and clarifications! @rahulku @feliperodri

@carolynzech
Copy link
Author

@Yenyun035 Good question! We recommend writing contracts and applying them directly to the functions. So, close to what you have for option 1, but omitting the wrapper. For example:

// your contracts go here!
pub const unsafe fn unchecked_add(self, rhs: Self) -> Self { ... }

You can then go ahead and write harnesses for the methods directly, e.g.

#[kani::proof_for_contract(i8::unchecked_add)]
fn harness() {...}

It seems like you have a good handle on how contracts work, but if you are looking for more resources, we recommend looking at the contracts section of our tutorial.

@Yenyun035
Copy link

@carolynzech Appreciate it for your response! I just tried to add contracts and write a corresponding harness as you stated. However, I encountered this error:

Failed to resolve checking function i8::unchecked_add because Kani currently cannot resolve path including primitive types

// originated from #[kani::proof_for_contract(i8::unchecked_add)] 

How could this be resolved?

@carolynzech
Copy link
Author

carolynzech commented Sep 18, 2024

@carolynzech Appreciate it for your response! I just tried to add contracts and write a corresponding harness as you stated. However, I encountered this error:

Failed to resolve checking function i8::unchecked_add because Kani currently cannot resolve path including primitive types

// originated from #[kani::proof_for_contract(i8::unchecked_add)] 

How could this be resolved?

@Yenyun035 Ensure that you're running Kani from the features/verify-std branch--we fixed that issue in this PR. See the instructions to build from source.

@Yenyun035
Copy link

@carolynzech Hello! Recently our team has been writing proofs for unchecked_{sub, mul, shl, shr}. Each of us has run the proofs on our end, and we ended up passing a different number of checks. For example:

What could be the reason? @Yenyun035 and @lanfeima seemed to be using the same toolchain version (nightly-09-08).

@carolynzech
Copy link
Author

carolynzech commented Sep 25, 2024

What could be the reason? @Yenyun035 and @lanfeima seemed to be using the same toolchain version (nightly-09-08).

@Yenyun035 It could be because you're running on different machines--different architectures can perhaps produce different checks. Could you check which versions of Kani and CBMC you're using?

@Yenyun035
Copy link

Yenyun035 commented Sep 25, 2024

@carolynzech

@Yenyun035 - Mac Pro M1; Kani 0.55.0 and CBMC 6.3.1.
@lanfeima - Mac Pro M3; Kani 0.55.0 and CBMC 6.3.1.

@rajathkotyal
Copy link

rajathkotyal commented Sep 26, 2024

Hi @carolynzech ,
I ran the same i8::unchecked_sub proof on a : Ubuntu 22.04 Docker : Intel x86 architecture. And got only a 119 checks. With the same Kani, CBMC and Rustup toolchain versions as others.

Our very trivial speculation right now for this is the machine architecture and amount of cpu cores / RAM that is allowed by the system for the verifier to utilize, which in turn could have made cbmc perform lesser checks to accommodate the respective proofs.

To test this out, we are planning to create a Ubuntu docker image with all modules pre installed and run it on different architectures to see if we obtain different results. But if you think we are missing something, or if you have faced a similar kind of issue prior to this with other proofs, please advice. Thanks!

Versions :

Screen Shot 2024-09-25 at 8 39 59 PM

@carolynzech
Copy link
Author

@rajathkotyal I suspect that this is just caused by architecture differences. As long as you're all running the same versions of Kani and CBMC (and the same harnesses), I wouldn't worry about the different number of checks. I'd recommend focusing on writing other harnesses or contracts for now.

feliperodri pushed a commit that referenced this issue Oct 4, 2024
Towards #59 

* 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.

---------

Co-authored-by: yew005 <yew005@ucsd.edu>
Co-authored-by: Rajath Kotyal <53811196+rajathkotyal@users.noreply.github.com>
Co-authored-by: rajathmCMU <rajathkotyal@gmail.com>
carolynzech pushed a commit that referenced this issue 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>
zhassan-aws pushed a commit that referenced this issue Oct 8, 2024
Towards #59 

### Changes
* Added contracts for `unchecked_neg` (located in
`library/core/src/num/int_macros.rs`)
* Added a harness for `unchecked_neg` of each signed integer type
  * `i8`, `i16`, `i32`, `i64`, `i128`, `isize` --- 6 harnesses in total.
* Fixed comments.

### Revalidation
1. Per the discussion in #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_neg_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
```

All default harnesses should pass.

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: yew005 <yew005@ucsd.edu>
Co-authored-by: Rajath Kotyal <53811196+rajathkotyal@users.noreply.github.com>
Co-authored-by: rajathmCMU <rajathkotyal@gmail.com>
carolynzech pushed a commit that referenced this issue Oct 14, 2024
Towards : issue #59

Parent branch : main 

Revalidation : 
Per the discussion in
#59, we have to
build and run Kani from feature/verify-rust-std branch.
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. widening_mul_u16_small) to run a
specific harness.
```
kani verify-std  "path/to/library" \
    --harness <harness_to_run> \
    -Z unstable-options \
    -Z function-contracts \
    -Z mem-predicates
```
carolynzech pushed a commit that referenced this issue Oct 17, 2024
Towards #59 

### Changes
* Added contracts for `wrapping_shl` (located in
`library/core/src/num/int_macros.rs` and `uint_macros.rs`)
* Added a macro for generating wrapping_{shl, shr} harnesses
* Added harnesses for `wrapping_shl` 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 #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. `checked_wrapping_shl_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
```

All harnesses should pass the default checks (1251 checks where 1
unreachable).
```
SUMMARY:
 ** 0 of 1251 failed (1 unreachable)

VERIFICATION:- SUCCESSFUL
Verification Time: 2.4682913s

Complete - 1 successfully verified harnesses, 0 failures, 1 total.
```
Example of the unreachable check:
```
Check 123: num::<impl i8>::wrapping_shl.assertion.1
         - Status: UNREACHABLE
         - Description: "attempt to subtract with overflow"
         - Location: library/core/src/num/int_macros.rs:2172:42 in function num::<impl i8>::wrapping_shl
```

### Questions
1. Should we add `requires` (and `ensures`) for `wrapping_shl` given
that `unchecked_shl` already has a `requires`?

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
celinval pushed a commit that referenced this issue Oct 22, 2024
Towards #59

Changes
Added contracts for wrapping_shr (located in
library/core/src/num/int_macros.rs and uint_macros.rs)
Added harnesses for wrapping_shr of each integer type
i8, i16, i32, i64, i128, isize, u8, u16, u32, u64, u128, usize --- 12
harnesses in total.
Revalidation
Per the discussion in
#59, we have to
build and run Kani from feature/verify-rust-std branch.
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. checked_wrapping_shl_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
```
All harnesses should pass the default checks (1251 checks where 1
unreachable).

```
SUMMARY:
 ** 0 of 161 failed (1 unreachable)

VERIFICATION:- SUCCESSFUL
Verification Time: 0.32086188s

Complete - 12 successfully verified harnesses, 0 failures, 12 total.
```

Example of the unreachable check:
```
Check 9: num::<impl i8>::wrapping_shr.assertion.1
	 - Status: UNREACHABLE
	 - Description: "attempt to subtract with overflow"
	 - Location: library/core/src/num/int_macros.rs:2199:42 in function num::<impl i8>::wrapping_shr
```

---------

Co-authored-by: Yenyun035 <yew005eng@gmail.com>
@Yenyun035
Copy link

Yenyun035 commented Oct 22, 2024

Hi @carolynzech @celinval @feliperodri @zhassan-aws

When I was testing my harness for f32::to_int_unchecked, I encountered the error indicating that float_to_int_unchecked is not currently supported by Kani, as shown below. Is it possible to support it?

SUMMARY:
 ** 1 of 1277 failed (1276 undetermined)
Failed Checks: float_to_int_unchecked is not currently supported by Kani. Please post your example at https://github.com/model-checking/kani/issues/new/choose
 File: "/Users/yew005/Docs/Academic/CMU/Fall24/practicum/verify-rust-std/library/core/src/convert/num.rs", line 30, in <f32 as convert::num::FloatToInt<i32>>::to_int_unchecked

VERIFICATION:- FAILED
** WARNING: A Rust construct that is not currently supported by Kani was found to be reachable. Check the results for more details.
Verification Time: 5.8839436s

Summary:
Verification failed for - num::verify::checked_to_int_unchecked_f32
Complete - 0 successfully verified harnesses, 1 failures, 1 total.

library/core/src/convert/num.rs Line 20 to 35:

20 macro_rules! impl_float_to_int {
21     ($Float:ty => $($Int:ty),+) => {
22         #[unstable(feature = "convert_float_to_int", issue = "67057")]
23         impl private::Sealed for $Float {}
24         $(
25             #[unstable(feature = "convert_float_to_int", issue = "67057")]
26             impl FloatToInt<$Int> for $Float {
27                 #[inline]
28                 unsafe fn to_int_unchecked(self) -> $Int {
29                     // SAFETY: the safety contract must be upheld by the caller.
30                     unsafe { crate::intrinsics::float_to_int_unchecked(self) }
31                 }
32            }
33         )+
34     }
35 }

Test harness:

#[kani::proof_for_contract(f32::to_int_unchecked)]
pub fn checked_to_int_unchecked_f32() {
    let num1: f32 = kani::any::<f32>();

    let result = unsafe { num1.to_int_unchecked::<i32>() };

    assert_eq!(result, num1 as i32);
}

Contracts added to f32::to_int_unchecked (in library/core/src/num/f32.rs):

/// # Safety
///
/// The value must:
///
/// * Not be `NaN`
/// * Not be infinite
/// * Be representable in the return type `Int`, after truncating off its fractional part
/// ...
#[requires(!self.is_nan() && !self.is_infinite())]
#[requires(self >= Self::MIN && self <= Self::MAX)]
pub unsafe fn to_int_unchecked<Int>(self) -> Int
where
    Self: FloatToInt<Int>,
{ ... }

Thank you very much!

@Yenyun035 Yenyun035 linked a pull request Oct 24, 2024 that will close this issue
celinval pushed a commit that referenced this issue Oct 26, 2024
## Towards: Issue #59

### Parent branch: main

---

### Changes


- Added macros for generating `carrying_mul` harnesses
- Added harnesses for `carrying_mul` for the following integer types:
  - `u8`, `u16`, `u32`, `u64`

---------

Co-authored-by: yew005 <yew005@ucsd.edu>
Co-authored-by: MWDZ <jinjunfeng721@gmail.com>
Co-authored-by: Yenyun035 <yenyunw@andrew.cmu.edu>
Co-authored-by: Rajath <rajathkotyal@gmail.com>
Co-authored-by: Rajath Kotyal <53811196+rajathkotyal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment