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

List Subcommand (Implementation) #3523

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

carolynzech
Copy link
Contributor

@carolynzech carolynzech commented Sep 17, 2024

Implementation of the list subcommand (and updates to the RFC).

As a larger test, I ran on the standard library (kani list -Z list -Z function-contracts -Z mem-predicates ./library --std) and manually verified that the results were correct. I pasted the output below. mem::swap only has modifies clauses, so we list zero contracts (see the "Modifies Clauses" section of the RFC for rationale).

Contracts:
Each function in the table below either has contracts or is the target of a contract harness (#[kani::proof_for_contract]).
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | Function                                         | # of Contracts | Contract Harnesses                                     |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | alloc::layout::Layout::from_size_align_unchecked | 1              | alloc::layout::verify::check_from_size_align_unchecked |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ascii::ascii_char::AsciiChar::from_u8            | 1              | ascii::ascii_char::verify::check_from_u8               |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ascii::ascii_char::AsciiChar::from_u8_unchecked  | 2              | ascii::ascii_char::verify::check_from_u8_unchecked     |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | char::convert::from_u32_unchecked                | 2              | char::convert::verify::check_from_u32_unchecked        |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | char::methods::verify::as_ascii_clone            | 1              | char::methods::verify::check_as_ascii_ascii_char       |
|       |                                                  |                | char::methods::verify::check_as_ascii_non_ascii_char   |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | intrinsics::typed_swap                           | 4              | intrinsics::verify::check_typed_swap_char              |
|       |                                                  |                | intrinsics::verify::check_typed_swap_non_zero          |
|       |                                                  |                | intrinsics::verify::check_typed_swap_u8                |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | mem::swap                                        | 0              | mem::verify::check_swap_adt_no_drop                    |
|       |                                                  |                | mem::verify::check_swap_primitive                      |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::align_offset                                | 2              | ptr::verify::check_align_offset_17                     |
|       |                                                  |                | ptr::verify::check_align_offset_4096                   |
|       |                                                  |                | ptr::verify::check_align_offset_u128                   |
|       |                                                  |                | ptr::verify::check_align_offset_u16                    |
|       |                                                  |                | ptr::verify::check_align_offset_u32                    |
|       |                                                  |                | ptr::verify::check_align_offset_u64                    |
|       |                                                  |                | ptr::verify::check_align_offset_u8                     |
|       |                                                  |                | ptr::verify::check_align_offset_zst                    |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::alignment::Alignment::as_nonzero            | 2              | ptr::alignment::verify::check_as_nonzero               |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::alignment::Alignment::as_usize              | 1              | ptr::alignment::verify::check_as_usize                 |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::alignment::Alignment::log2                  | 3              | ptr::alignment::verify::check_log2                     |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::alignment::Alignment::mask                  | 3              | ptr::alignment::verify::check_mask                     |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::alignment::Alignment::new                   | 2              | ptr::alignment::verify::check_new                      |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::alignment::Alignment::new_unchecked         | 3              | ptr::alignment::verify::check_new_unchecked            |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::alignment::Alignment::of                    | 2              | ptr::alignment::verify::check_of_i32                   |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::read_volatile                               | 1              | ptr::verify::check_read_u128                           |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::unique::Unique::<T>::as_non_null_ptr        | 1              | ptr::unique::verify::check_as_non_null_ptr             |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::unique::Unique::<T>::as_ptr                 | 1              | ptr::unique::verify::check_as_ptr                      |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::unique::Unique::<T>::new                    | 2              | ptr::unique::verify::check_new                         |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::unique::Unique::<T>::new_unchecked          | 2              | ptr::unique::verify::check_new_unchecked               |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::verify::mod_inv_copy                        | 2              | ptr::verify::check_mod_inv                             |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
|       | ptr::write_volatile                              | 1              | NONE                                                   |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+
| Total | 22                                               | 39             | 32                                                     |
+-------+--------------------------------------------------+----------------+--------------------------------------------------------+

Standard Harnesses (#[kani::proof]):
1. ptr::unique::verify::check_as_mut
2. ptr::unique::verify::check_cast
3. ptr::unique::verify::check_as_ref

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

@carolynzech carolynzech requested a review from a team as a code owner September 17, 2024 20:53
@github-actions github-actions bot added the Z-BenchCI Tag a PR to run benchmark CI label Sep 17, 2024
Copy link
Contributor

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

I would prefer if the compiler is not aware that this is a list command.

@@ -47,6 +47,9 @@ pub struct Arguments {
pub reachability_analysis: ReachabilityType,
#[clap(long = "enable-stubbing")]
pub stubbing_enabled: bool,
/// Option name used to tell the compiler to execute the list subcommand
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the compiler need to be aware of the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per slack, no -- will fix. Going to move to draft until then, since this will take me a bit of time to do and I don't want anyone wasting time reviewing in the meantime.

Comment on lines 544 to 548
// Only emit an error if we are trying to actually verify the contract.
// (If we are running the list subcommand, we just report later that there are no contracts for this harness).
if is_list_enabled {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I think this should emit a compilation error.

Copy link
Contributor Author

@carolynzech carolynzech Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People may want to write harnesses before they write the contracts. For instance, if I'm planning to write a contract for a function, I could write:

fn foo(x: usize) -> usize { ... }

#[kani::proof_for_contract(foo)]
fn harness() {
  let x: usize = kani::any();
  foo(x);
}

before I write the contracts for foo.

It makes sense to error during verification, since we can't verify a contract that doesn't exist. But I didn't think it made sense to punish people for being halfway done; they may want to use the list command to see what work they have left to do.

That being said, I'm not even sure I could make this distinction if we're making the compiler unaware of the list command, so we may have to make it a compiler error.

@carolynzech carolynzech marked this pull request as draft September 19, 2024 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-BenchCI Tag a PR to run benchmark CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants