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 f{32,64}::to_int_unchecked #134

Conversation

Yenyun035
Copy link

@Yenyun035 Yenyun035 commented Oct 24, 2024

Towards #59

(Resolved) Depends on this Kani Issue and this PR, as discussed in this thread in #59

(Resolved) Depends on this Kani Issue and this PR

(Resolved) Waiting for Kani PR#3742 merged into feature/verify-rust-std

f16 and f128 are in #163

Changes

  • Added contracts for f{32,64}::to_int_unchecked (located in library/core/src/num/f{32,64}.rs)
  • Added a macro for generating to_int_unchecked harnesses
  • Added harnesses for f{32,64}to_int_unchecked of each integer type
    • i8, i16, i32, i64, i128, isize, u8, u16, u32, u64, u128, usize --- 12 harnesses in total.

Verification Results

To compile, we need to add the -Z float-lib flag.

Checking harness num::verify::checked_f32_to_int_unchecked_usize...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.424911s

Checking harness num::verify::checked_f64_to_int_unchecked_u128...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.8557353s

Checking harness num::verify::checked_f32_to_int_unchecked_u16...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.195041s

Checking harness num::verify::checked_f32_to_int_unchecked_i8...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.2361426s

Checking harness num::verify::checked_f64_to_int_unchecked_i32...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.3952055s

Checking harness num::verify::checked_f64_to_int_unchecked_i128...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 7.5295496s

Checking harness num::verify::checked_f64_to_int_unchecked_u16...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.2897367s

Checking harness num::verify::checked_f32_to_int_unchecked_i64...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.58576s

Checking harness num::verify::checked_f64_to_int_unchecked_i16...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.2046432s

Checking harness num::verify::checked_f32_to_int_unchecked_i128...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.8473463s

Checking harness num::verify::checked_f32_to_int_unchecked_u8...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.131122s

Checking harness num::verify::checked_f32_to_int_unchecked_i16...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.436728s

Checking harness num::verify::checked_f32_to_int_unchecked_u128...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.666422s

Checking harness num::verify::checked_f64_to_int_unchecked_u8...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.17829s

Checking harness num::verify::checked_f32_to_int_unchecked_i32...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.6507607s

Checking harness num::verify::checked_f64_to_int_unchecked_i64...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 7.3081775s

Checking harness num::verify::checked_f64_to_int_unchecked_u64...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 7.0912967s

Checking harness num::verify::checked_f64_to_int_unchecked_i8...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.4602604s

Checking harness num::verify::checked_f64_to_int_unchecked_usize...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.9098988s

Checking harness num::verify::checked_f64_to_int_unchecked_u32...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.557031s

Checking harness num::verify::checked_f64_to_int_unchecked_isize...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 7.1193557s

Checking harness num::verify::checked_f32_to_int_unchecked_u64...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.7919626s

Checking harness num::verify::checked_f32_to_int_unchecked_u32...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.557074s

Checking harness num::verify::checked_f32_to_int_unchecked_isize...

VERIFICATION RESULT:
 ** 0 of 136 failed

VERIFICATION:- SUCCESSFUL
Verification Time: 6.710118s

Complete - 24 successfully verified harnesses, 0 failures, 24 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 changed the title Contracts & Harnesses for f32::to_int_unchecked Contracts & Harnesses for to_int_unchecked Oct 30, 2024
@Yenyun035
Copy link
Author

Yenyun035 commented Nov 8, 2024

@celinval @carolynzech @zhassan-aws @feliperodri I was able to run f32->i8 harness. One of the checks failed, and I guess it is because the contract is incorrect. Specifically, I think the Self::MAX and Self::MIN in the contract is referring to the max and min of the float type instead of the resulting integer type. Therefore, I tried to change the contract to use <Int>::MIN/MAX (because the doc mentioned "Be representable in the return type Int"), and it gave me "associated item not found in Int" error. I only found FloatToInt and did not find any docs related to Int.

My confusion is, how can I refer to the max/min of the resulting integer type given that the type parameters accessible from the context of the contract are Self and Int?

Check 14: <f32 as convert::num::FloatToInt>::to_int_unchecked.arithmetic_overflow.2
- Status: FAILURE
- Description: "float_to_int_unchecked: attempt to convert a value out of range of the target integer"
- Location: library/core/src/convert/num.rs:30:30 in function <f32 as convert::num::FloatToInt>::to_int_unchecked

@Yenyun035
Copy link
Author

My confusion is, how can I refer to the max/min of the resulting integer type given that the type parameters accessible from the context of the contract are Self and Int?

I found this workaround, inspired by this TypeChecker, and was able to run the f32->i8 harness (see here for unreachables). It took a while (~1.5 minutes) after compilation (Finished dev profile...) to start running the harness (Checking harness num::verify::checked_f32_to_int_unchecked_i8...), which is generally slower than previous harnesses.

// Tested for f32->i8
#[requires(self.is_finite() && ((type_name::<Int>().contains("i8") && self >= i8::MIN as Self) && (self <= i8::MAX as Self)))]

SUMMARY:
 ** 0 of 1283 failed (95 unreachable)

VERIFICATION:- SUCCESSFUL
Verification Time: 10.796687s

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

While this workaround might work, I think it would be too messy in the end. I'm wondering if it's possible to have a macro generating functions that check if the float value is in the bound of an integer type, as shown in the example below. However, I think it might cause circular dependency. If there is no better way to make the contract cleaner, maybe we could still proceed with the aforementioned approach.

use core::any::type_name;

// Not sure where to place; Not sure if this works.
macro_rules! generate_is_in_range_check {
    ($floatType:ty, $($intType:ty, $intStr:expr),+) => {
        $(
            pub fn is_in_range<Int>(num: $floatType) -> bool {
                type_name::<$intType>().contains($intStr) && num >= <$intType>::MIN as $floatType && num <= <$intType>::MAX as $floatType
            }
        )+
    }
}

generate_is_in_range_check!(f32, i8, "i8");

Then the contract becomes:
#[requires(self.is_finite() && is_in_range::<Int>(self))]

By the way, I also tried the TypeId::of comparison but got this error:

Contract:
#[requires(self.is_finite() && ((TypeId::of::<Int>() == TypeId::of::<i8> && self >= i8::MIN as Self) && (self <= i8::MAX as Self)))]

Result:
error[E0310]: the parameter type `Int` may not live long enough
    --> /Users/yew005/Docs/Academic/CMU/Fall24/practicum/vrs/library/core/src/num/f32.rs:1078:38
     |
1078 |     #[requires(self.is_finite() && ((TypeId::of::<Int>() == TypeId::of::<i8>() && self >= i8::MIN as Self) && (self <= i8::MA...
     |                                      ^^^^^^^^^^^^^^^^^^^
     |                                      |
     |                                      the parameter type `Int` must be valid for the static lifetime...
     |                                      ...so that the type `Int` will meet its required lifetime bounds
     |
help: consider adding an explicit lifetime bound
     |
1079 |     pub unsafe fn to_int_unchecked<Int: 'static>(self) -> Int
     |                                       +++++++++

@celinval @carolynzech @zhassan-aws @feliperodri I would appreciate any ideas and advice!

@zhassan-aws
Copy link

I think you should be able to define a macro that matches on the Int type, e.g.

macro_rules! foo {
    (u8, ...) => { ... }
    (u16, ...) => { ... }
    ....
}

and use it in the contract.

@Yenyun035
Copy link
Author

Yenyun035 commented Nov 11, 2024

I think you should be able to define a macro that matches on the Int type, ... and use it in the contract.

@feliperodri Thank you for your response! I tried this approach by adding the macro in mod verify in mod.rs and calling it in the function contract. However, as shown below, it gave me the error "no rules expected Int." I expected that if Int is type i8, it should match the first, i.e., (i8, ...) case. From the output, it looks like the Int and i8 etc primitive types are different. I'm. wondering why it happened. Appreciate any insights!

// mod.rs
pub mod verify {
    macro_rules! is_in_range {
        (i8, $floatType:ty, $num:ident) => { $num >= i8::MIN as $floatType && $num <= i8::MAX as $floatType };
        (i16, $floatType:ty, $num:ident) => { $num >= i16::MIN as $floatType && $num <= i16::MAX as $floatType };
        (i32, $floatType:ty, $num:ident) => { $num >= i32::MIN as $floatType && $num <= i32::MAX as $floatType };
        (i64, $floatType:ty, $num:ident) => { $num >= i64::MIN as $floatType && $num <= i64::MAX as $floatType };
        (i128, $floatType:ty, $num:ident) => { $num >= i128::MIN as $floatType && $num <= i128::MAX as $floatType };
        (isize, $floatType:ty, $num:ident) => { $num >= isize::MIN as $floatType && $num <= isize::MAX as $floatType };
        (u8, $floatType:ty, $num:ident) => { $num >= u8::MIN as $floatType && $num <= u8::MAX as $floatType };
        (u16, $floatType:ty, $num:ident) => { $num >= u16::MIN as $floatType && $num <= u16::MAX as $floatType };
        (u32, $floatType:ty, $num:ident) => { $num >= u32::MIN as $floatType && $num <= u32::MAX as $floatType };
        (u64, $floatType:ty, $num:ident) => { $num >= u64::MIN as $floatType && $num <= u64::MAX as $floatType };
        (u128, $floatType:ty, $num:ident) => { $num >= u128::MIN as $floatType && $num <= u128::MAX as $floatType };
        (usize, $floatType:ty, $num:ident) => { $num >= usize::MIN as $floatType && $num <= usize_isize_to_xe_bytes_doc::MAX as $floatType };
    }
    pub(crate) use is_in_range;
}


// Call in to_int_unchecked contract (f32.rs)
use super::num::verify::is_in_range;
#[requires(self.is_finite() && is_in_range!(Int, Self, self))]

// Result
error: no rules expected `Int`
    --> /path/to/repo/vrs/library/core/src/num/f32.rs:1076:49
     |
1076 |     #[requires(self.is_finite() && is_in_range!(Int, Self, self))]
     |                                                 ^^^ no rules expected this token in macro call
     |
    ::: /path/to/repo/vrs/library/core/src/num/mod.rs:1693:5
     |
1693 |     macro_rules! is_in_range {
     |     ------------------------ when calling this macro
     |
note: while trying to match `i8`
    --> /path/to/repo/vrs/library/core/src/num/mod.rs:1694:10
     |
1694 |         (i8, $floatType:ty, $num:ident) => { $num >= i8::MIN as $floatType && $num <= i8::MAX as $floatType };
     |  

@zhassan-aws
Copy link

I see. The other alternative I could think of is to define a trait that provides min and max methods and implement it for each of the Int types. I'm not sure if there's a clean way to require Int to implement this trait though without affecting the non-Kani build.

@zhassan-aws
Copy link

There's also the issue of imprecision when casting an int value to a float. For instance, i32::MAX as f32 is 2147483647 as f32 which is 2147483648.0 (see #134). This means that the contract pre-condition is valid for the f32 value 2147483648.0 even though it's out of the range of an i32.

Not sure what the best approach here is. Both MIRI and (now) Kani have their own internal methods to check if the values are in range (e.g. see rust-lang/miri#1325). I wonder whether those can be somehow exposed in the library. Ideally, there should be a float_to_int_checked method, in which case we can just check if it returns Some.

@Yenyun035
Copy link
Author

There's also the issue of imprecision when casting an int value to a float. ...

Thank you for pointing this out! We will pay attention to it.

  1. The other alternative I could think of is to define a trait ...
  2. Both MIRI and (now) Kani have their own internal methods.... I wonder whether those can be somehow exposed in the library...

Thank you for sharing this. Is it possible to have Kani support this? E.g. expose an in_range(float, floatType, IntType) API that we can directly call. We saw that the Kani internally has codegen_in_range_expr which we think useful.

Thank you! @zhassan-aws

@zhassan-aws
Copy link

Thank you for sharing this. Is it possible to have Kani support this? E.g. expose an in_range(float, floatType, IntType) API that we can directly call. We saw that the Kani internally has codegen_in_range_expr which we think useful.

Thank you! @zhassan-aws

Yes, I think it would be possible. This would likely need to be done through providing a trait and its implementation for different float types so that it can be used with the generic Int. Can you file a feature request in the Kani repo?

@Yenyun035
Copy link
Author

@zhassan-aws I just filed this Feature Request. Thank you!

@celinval
Copy link

Hi @Yenyun035, we have updated the Kani version in this repo. You should be able to use the new API

@Yenyun035 Yenyun035 marked this pull request as ready for review November 30, 2024 05:26
@Yenyun035 Yenyun035 requested a review from a team as a code owner November 30, 2024 05:26
@Yenyun035
Copy link
Author

Yenyun035 commented Nov 30, 2024

@celinval Thank you! I have run Kani on all harnesses and updated the results in the above. It's ready for review now 👍

To compile, we need to add the -Z float-lib flag.

@celinval
Copy link

celinval commented Dec 2, 2024

@celinval Thank you! I have run Kani on all harnesses and updated the results in the above. It's ready for review now 👍

To compile, we need to add the -Z float-lib flag.

Can you please add that to run-kani.sh around here:

-Z function-contracts \
-Z mem-predicates \
-Z loop-contracts \

@Yenyun035 Yenyun035 requested a review from celinval December 3, 2024 00:29
@Yenyun035
Copy link
Author

Yenyun035 commented Dec 3, 2024

@celinval @tautschnig @carolynzech @feliperodri Could you please review this PR and #163 as well? Thank you!

library/core/src/ub_checks.rs Outdated Show resolved Hide resolved
library/core/src/ub_checks.rs Outdated Show resolved Hide resolved
@Yenyun035
Copy link
Author

@zhassan-aws Thank you. Fixed.

@Yenyun035
Copy link
Author

unused import: use crate::ub_checks::float_to_int_in_range;

I found one of the checks failed due to the above error, but the function is indeed used in the contract:

#[requires(self.is_finite() && float_to_int_in_range::<Self, Int>(self))]

@zhassan-aws
Copy link

Try putting the import under #[cfg(kani)]

@zhassan-aws zhassan-aws removed their assignment Dec 4, 2024
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.

Thanks!!

library/core/src/num/f32.rs Outdated Show resolved Hide resolved
library/core/src/num/f64.rs Outdated Show resolved Hide resolved
@Yenyun035
Copy link
Author

@celinval Thank you. I fixed the import for this and #163.

@zhassan-aws zhassan-aws enabled auto-merge (squash) December 4, 2024 15:37
@zhassan-aws zhassan-aws merged commit 72323e4 into model-checking:main Dec 4, 2024
9 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.

6 participants