-
Notifications
You must be signed in to change notification settings - Fork 142
Split rmc::nondet into safe and unsafe functions(#607) #657
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
Changes from all commits
36e4f39
ad41274
16e7ba0
460dd98
ca85f7e
9358103
2d9cc0a
2277f34
3f6eadd
502c84e
0f17d25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 OR MIT | ||
|
|
||
| //! This module introduces the Arbitrary trait as well as implementation for the Invariant trait. | ||
| use crate::{any_raw, assume, Invariant}; | ||
|
|
||
| /// This trait should be used to generate symbolic variables that represent any valid value of | ||
| /// its type. | ||
| pub trait Arbitrary { | ||
| fn any() -> Self; | ||
| } | ||
|
|
||
| impl<T> Arbitrary for T | ||
| where | ||
| T: Invariant, | ||
| { | ||
| fn any() -> Self { | ||
| let value = unsafe { any_raw::<T>() }; | ||
| assume(value.is_valid()); | ||
| value | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 OR MIT | ||
|
|
||
| //! This module introduces the Invariant trait as well as implementation for commonly used types. | ||
| use std::num::*; | ||
|
|
||
| /// Types that implement a check to ensure its value is valid and safe to be used. See | ||
| /// https://doc.rust-lang.org/stable/nomicon/what-unsafe-does.html for examples of valid values. | ||
| /// | ||
| /// Implementations of Invariant traits must ensure that the current bit values of the given type | ||
| /// is valid and that all its invariants hold. | ||
| /// | ||
| /// # Safety | ||
| /// | ||
| /// This trait is unsafe since &self might represent an invalid value. The `is_valid()` function | ||
| /// must return `true` if and only if the invariant of its type is held. | ||
| pub unsafe trait Invariant { | ||
| /// Check if `&self` holds a valid value that respect the type invariant. | ||
| /// This function must return `true` if and only if `&self` is valid. | ||
| fn is_valid(&self) -> bool; | ||
| } | ||
|
|
||
| macro_rules! empty_invariant { | ||
| ( $type: ty ) => { | ||
| unsafe impl Invariant for $type { | ||
| #[inline(always)] | ||
| fn is_valid(&self) -> bool { | ||
| true | ||
| } | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| empty_invariant!(u8); | ||
| empty_invariant!(u16); | ||
| empty_invariant!(u32); | ||
| empty_invariant!(u64); | ||
| empty_invariant!(u128); | ||
| empty_invariant!(usize); | ||
|
|
||
| empty_invariant!(i8); | ||
| empty_invariant!(i16); | ||
| empty_invariant!(i32); | ||
| empty_invariant!(i64); | ||
| empty_invariant!(i128); | ||
| empty_invariant!(isize); | ||
|
|
||
| // We do not constraint floating points values per type spec. Users must add assumptions to their | ||
| // verification code if they want to eliminate NaN, infinite, or subnormal. | ||
| empty_invariant!(f32); | ||
| empty_invariant!(f64); | ||
|
|
||
| empty_invariant!(()); | ||
|
|
||
| unsafe impl Invariant for bool { | ||
| #[inline(always)] | ||
| fn is_valid(&self) -> bool { | ||
| let byte = u8::from(*self); | ||
| byte == 0 || byte == 1 | ||
| } | ||
| } | ||
|
|
||
| /// Validate that a char is not outside the ranges [0x0, 0xD7FF] and [0xE000, 0x10FFFF] | ||
| /// Ref: https://doc.rust-lang.org/stable/nomicon/what-unsafe-does.html | ||
| unsafe impl Invariant for char { | ||
| #[inline(always)] | ||
| fn is_valid(&self) -> bool { | ||
| // RMC translates char into i32. | ||
| let val = *self as i32; | ||
| val <= 0xD7FF || (val >= 0xE000 && val <= 0x10FFFF) | ||
| } | ||
| } | ||
|
|
||
| unsafe impl<T: Invariant, const N: usize> Invariant for [T; N] { | ||
| fn is_valid(&self) -> bool { | ||
| self.iter().all(|e| e.is_valid()) | ||
| } | ||
| } | ||
|
|
||
| unsafe impl<T> Invariant for Option<T> | ||
| where | ||
| T: Invariant, | ||
| { | ||
| #[inline(always)] | ||
| fn is_valid(&self) -> bool { | ||
| if let Some(v) = self { v.is_valid() } else { matches!(*self, None) } | ||
| } | ||
| } | ||
|
|
||
| unsafe impl<T, E> Invariant for Result<T, E> | ||
| where | ||
| T: Invariant, | ||
| E: Invariant, | ||
| { | ||
| #[inline(always)] | ||
| fn is_valid(&self) -> bool { | ||
| if let Ok(v) = self { | ||
| v.is_valid() | ||
| } else if let Err(e) = self { | ||
| e.is_valid() | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My two cents: it would be inconvenient if the user needs to annotate all the enums in their code with |
||
|
|
||
| macro_rules! nonzero_invariant { | ||
| ( $type: ty ) => { | ||
| unsafe impl Invariant for $type { | ||
| #[inline(always)] | ||
| fn is_valid(&self) -> bool { | ||
| self.get() != 0 | ||
| } | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| nonzero_invariant!(NonZeroU8); | ||
| nonzero_invariant!(NonZeroU16); | ||
| nonzero_invariant!(NonZeroU32); | ||
| nonzero_invariant!(NonZeroU64); | ||
| nonzero_invariant!(NonZeroU128); | ||
| nonzero_invariant!(NonZeroUsize); | ||
|
|
||
| nonzero_invariant!(NonZeroI8); | ||
| nonzero_invariant!(NonZeroI16); | ||
| nonzero_invariant!(NonZeroI32); | ||
| nonzero_invariant!(NonZeroI64); | ||
| nonzero_invariant!(NonZeroI128); | ||
| nonzero_invariant!(NonZeroIsize); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 OR MIT | ||
| use crate::{assume, nondet}; | ||
| use crate::{any, any_raw, assume}; | ||
| use core::ops::{Deref, DerefMut}; | ||
|
|
||
| /// Given an array `arr` of length `LENGTH`, this function returns a **valid** | ||
|
|
@@ -12,23 +12,23 @@ use core::ops::{Deref, DerefMut}; | |
| /// | ||
| /// ```rust | ||
| /// let arr = [1, 2, 3]; | ||
| /// let slice = rmc::slice::nondet_slice_of_array(&arr); | ||
| /// let slice = rmc::slice::any_slice_of_array(&arr); | ||
| /// foo(slice); // where foo is a function that takes a slice and verifies a property about it | ||
| /// ``` | ||
| pub fn nondet_slice_of_array<T, const LENGTH: usize>(arr: &[T; LENGTH]) -> &[T] { | ||
| let (from, to) = nondet_range::<LENGTH>(); | ||
| pub fn any_slice_of_array<T, const LENGTH: usize>(arr: &[T; LENGTH]) -> &[T] { | ||
| let (from, to) = any_range::<LENGTH>(); | ||
| &arr[from..to] | ||
| } | ||
|
|
||
| /// A mutable version of the previous function | ||
| pub fn nondet_slice_of_array_mut<T, const LENGTH: usize>(arr: &mut [T; LENGTH]) -> &mut [T] { | ||
| let (from, to) = nondet_range::<LENGTH>(); | ||
| pub fn any_slice_of_array_mut<T, const LENGTH: usize>(arr: &mut [T; LENGTH]) -> &mut [T] { | ||
| let (from, to) = any_range::<LENGTH>(); | ||
| &mut arr[from..to] | ||
| } | ||
|
|
||
| fn nondet_range<const LENGTH: usize>() -> (usize, usize) { | ||
| let from: usize = nondet(); | ||
| let to: usize = nondet(); | ||
| fn any_range<const LENGTH: usize>() -> (usize, usize) { | ||
| let from: usize = any(); | ||
| let to: usize = any(); | ||
| assume(to <= LENGTH); | ||
| assume(from <= to); | ||
| (from, to) | ||
|
|
@@ -38,12 +38,12 @@ fn nondet_range<const LENGTH: usize>() -> (usize, usize) { | |
| /// between `0..=MAX_SLICE_LENGTH` and with non-deterministic content. This is | ||
| /// useful in situations where one wants to verify that all slices with any | ||
| /// content and with a length up to `MAX_SLICE_LENGTH` satisfy a certain | ||
| /// property. Use `nondet_slice` to create an instance of this struct. | ||
| /// property. Use `any_slice` to create an instance of this struct. | ||
| /// | ||
| /// # Example: | ||
| /// | ||
| /// ```rust | ||
| /// let slice: rmc::slice::NonDetSlice<u8, 5> = rmc::slice::nondet_slice(); | ||
| /// let slice: rmc::slice::NonDetSlice<u8, 5> = rmc::slice::any_slice(); | ||
| /// foo(&slice); // where foo is a function that takes a slice and verifies a property about it | ||
| /// ``` | ||
| pub struct NonDetSlice<T, const MAX_SLICE_LENGTH: usize> { | ||
|
|
@@ -53,8 +53,8 @@ pub struct NonDetSlice<T, const MAX_SLICE_LENGTH: usize> { | |
|
|
||
| impl<T, const MAX_SLICE_LENGTH: usize> NonDetSlice<T, MAX_SLICE_LENGTH> { | ||
| fn new() -> Self { | ||
| let arr: [T; MAX_SLICE_LENGTH] = nondet(); | ||
| let slice_len: usize = nondet(); | ||
| let arr: [T; MAX_SLICE_LENGTH] = unsafe { any_raw() }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should be able to implement unsafe impl<T, const LEN: usize> Invariant for [T; LEN]
where
T: Invariant,
{
#[inline(always)]
fn is_valid(&self) -> bool {
let valid = true;
for i in *self {
valid &= i.is_valid();
}
valid
}
}?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I am adding a variant of that. Thanks! |
||
| let slice_len: usize = any(); | ||
| assume(slice_len <= MAX_SLICE_LENGTH); | ||
| Self { arr, slice_len } | ||
| } | ||
|
|
@@ -82,6 +82,6 @@ impl<T, const MAX_SLICE_LENGTH: usize> DerefMut for NonDetSlice<T, MAX_SLICE_LEN | |
| } | ||
| } | ||
|
|
||
| pub fn nondet_slice<T, const MAX_SLICE_LENGTH: usize>() -> NonDetSlice<T, MAX_SLICE_LENGTH> { | ||
| pub fn any_slice<T, const MAX_SLICE_LENGTH: usize>() -> NonDetSlice<T, MAX_SLICE_LENGTH> { | ||
| NonDetSlice::<T, MAX_SLICE_LENGTH>::new() | ||
| } | ||
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.
Does this trait itself need to be
unsafe? I agree that it looks like it should be, but is there a way to do this without that?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.
That was a suggestion from @camshaft which I thought made a lot of sense. So I would prefer keeping it.
Uh oh!
There was an error while loading. Please reload this page.
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 was originally thinking it would be something like:
You would then implement that for the primitives.
I'm trying to decide if the unsafe makes sense in its current form, though... My concern is the
&selfargument is completely unvalidated and without knowing the RMC compiler internals, may result in UB when using it.Is there a reason the trait method doesn't return an owned value instead? It would make the trait safe to implement and each implementation may or may not need to use unsafe to generate a value:
Uh oh!
There was an error while loading. Please reload this page.
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.
Another concern is: in its current form, it's very easy to miss validating a field when implementing the trait:
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.
Thanks for clarifying. This is another possibility which uses a builder pattern. I thought the Invariant approach was a bit cleaner and easier to use. It also adds a method that can be used in assert statements which I thought was nice. I do agree that it has the downside of having a potential invalid
Selfand I think the unsafe impl is proper in this case.I think they are somewhat equivalent though. For an arbitrary enum, it would go from:
to
Maybe we should check which one performs better or even offer both.
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 understand the concern, and I debated between the two options for a long time. Maybe we can have a quick sync tomorrow to discuss the options.
Another possibility is to add both methods to the type with default implementations. Something like:
Users can pick and choose which one to implement / use.
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.
Yeah I'm good to sync up whenever works for you!