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

Error when annotating an associated function with a contract #3206

Closed
zhassan-aws opened this issue May 28, 2024 · 4 comments · Fixed by #3363
Closed

Error when annotating an associated function with a contract #3206

zhassan-aws opened this issue May 28, 2024 · 4 comments · Fixed by #3363
Assignees
Labels
[C] Bug This is a bug. Something isn't working. T-High Priority Tag issues that have high priority Z-Contracts Issue related to code contracts

Comments

@zhassan-aws
Copy link
Contributor

I tried this code:

extern crate kani;

#[derive(PartialEq, Eq)]
enum Foo {
    A,
    B,
}

impl Foo {
    #[kani::ensures(result == Foo::A)]
    pub fn new() -> Foo {
        Foo::A
    }
}

#[kani::proof_for_contract(Foo::new)]
fn check() {
    let _ = Foo::new();
}

using the following command line invocation:

kani enum_ensures.rs -Zfunction-contracts

with Kani version: f10e61c

I expected to see this happen: Verification succeeds

Instead, this happened:

$ kani enum_ensures.rs -Zfunction-contracts
Kani Rust Verifier 0.51.0 (standalone)
error[E0425]: cannot find function `new_wrapper_faac06` in this scope
  --> enum_ensures.rs:10:5
   |
10 |     #[kani::ensures(result == Foo::A)]
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope
   |
   = note: this error originates in the attribute macro `kani::ensures` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider using the associated function on `Self`
   |
10 |     Self::#[kani::ensures(result == Foo::A)]
   |     ++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0425`.
error: /home/ubuntu/git/kani/target/kani/bin/kani-compiler exited with status exit status: 1
@zhassan-aws zhassan-aws added the [C] Bug This is a bug. Something isn't working. label May 28, 2024
@zhassan-aws
Copy link
Contributor Author

Found a workaround: the error goes away if I use Self as the return type instead of Foo:

$ diff enum_ensures.rs enum_ensures_self.rs 
11c11
<     pub fn new() -> Foo {
---
>     pub fn new() -> Self {

Verification fails though:

$ kani enum_ensures_self.rs -Zfunction-contracts
...
SUMMARY:
 ** 1 of 1336 failed
Failed Checks: result == Foo::A
 File: "enum_ensures_self.rs", line 10, in Foo::new_check_478ff5

VERIFICATION:- FAILED
Verification Time: 1.9594536s

@zhassan-aws
Copy link
Contributor Author

The issue in the previous comment seems to be independent of associated functions, so I filed a separate issue for it (#3228).

@feliperodri feliperodri added this to the Function Contracts milestone Jun 12, 2024
@celinval celinval added the T-High Priority Tag issues that have high priority label Jun 12, 2024
@feliperodri feliperodri self-assigned this Jun 17, 2024
@celinval
Copy link
Contributor

I was talking to @JustusAdam about this. I think the way contract works today won't work with trait implementations, neither associated functions that do not refer to the Self type. See this comment for an explanation on how associated functions are handled today.

One way to overcome these two limitations would be to change the contract functions to be generated as nested functions. This would allow us to attach contracts to associated functions and trait implementations.

However, doing so will remove access to generic parameters defined as part of the impl block, including Self. I.e., the following would not work:

impl<T: Default> Foo<T> {
    pub fn new() -> Foo<T> {
        fn bar() -> T { //<---- T cannot be used in this context.
           // ...
        }
        Foo(bar())
    }
}

See E0401 for more details.

Another possibility would be to convert the functions into closures instead. This will also require changes to how contracts are handled in the compiler.

Let's say we have the following function:

#[kani::requires(can_dereference(ptr))]
unsafe fn read<T>(ptr: *const T) -> T {
    *ptr
}

Ignoring the recursive case for now, this could become something roughly like:

#[kanitool::contract]
unsafe fn read<T>(ptr: *const T) -> T {
    let original_fn = |ptr: *const T| -> T { /* original_body */ };
    let check_fn = |ptr| {
        assert!(can_dereference(ptr));
        original_fn(ptr)
    };
    let replace_fn = |ptr: *const T| -> T {
        assert!(can_dereference(ptr));
        kani::any_modifies()
    };
    kani::contract_pick(original_fn, check_fn, replace_fn);
}

Now the transformation pass would need to pick the correct closure body to do the replacement.

@JustusAdam
Copy link
Contributor

Yes right now the support for associated functions is limited because the macro only has access to the function body itself. It looks at that and basically tries to guess whether it may be an associated function, in this case by looking for an occurrence of self or Self.

I think I agree that closures are the best way to go because they can do generics and avoid this associated function. business. Although we should be careful with actually closing over the arguments, because that can get annoying with lifetimes.
The other problem with closures is that multiple clauses are harder to handle. At the moment such clauses are incrementally expanded on the check and replace functions themselves. On closures there's no stable way to attach attributes to the closure so the incremental expanding may not work.

@celinval celinval self-assigned this Jul 11, 2024
@tautschnig tautschnig added the Z-Contracts Issue related to code contracts label Aug 1, 2024
celinval added a commit that referenced this issue Aug 1, 2024
Contracts could not be used with associated function unless they used
Self. This is because at a proc-macro level, we cannot determine if we
are inside a associated function or not, except in cases where `Self` is
used. In cases where the contract generation could not identify an
associated function, it would generate incorrect call, triggering a
compilation error.

Another problem with our encoding is that users could not annotate trait
implementations with contract attributes. This would try to create new
functions inside the trait implementation context, which is not allowed.

In order to solve these issues, we decided to wrap contract logic using
closures instead of functions. See the discussion in the original issue
(#3206) for more details.

The new solution is still split in two:
1. The proc-macro will now expand the code inside the original function
to encode all possible scenarios (check, replace, recursive check, and
original body).
2. Instead of using stub, we introduced a new transformation pass that
chooses which scenario to pick according to the target harness
configuration, and cleanup unused logic.

The expanded function will have the following structure:
```rust
#[kanitool::recursion_check = "__kani_recursion_check_modify"]
#[kanitool::checked_with = "__kani_check_modify"]
#[kanitool::replaced_with = "__kani_replace_modify"]
#[kanitool::inner_check = "__kani_modifies_modify"]
fn name_fn(ptr: &mut u32) {
    #[kanitool::fn_marker = "kani_register_contract"]
    pub const fn kani_register_contract<T, F: FnOnce() -> T>(f: F) -> T {
        kani::panic("internal error: entered unreachable code: ")
    }
    let kani_contract_mode = kani::internal::mode();
    match kani_contract_mode {
        kani::internal::RECURSION_CHECK => {
            #[kanitool::is_contract_generated(recursion_check)]
            let mut __kani_recursion_check_name_fn = || { /* recursion check body */ };
            kani_register_contract(__kani_recursion_check_modify)
        }
        kani::internal::REPLACE => {
            #[kanitool::is_contract_generated(replace)]
            let mut __kani_replace_name_fn = || { /* replace body */ };
            kani_register_contract(__kani_replace_name_fn)
        }
        kani::internal::SIMPLE_CHECK => {
            #[kanitool::is_contract_generated(check)]
            let mut __kani_check_name_fn = || { /* check body */ };
            kani_register_contract(__kani_check_name_fn)
        }
        _ => { /* original body */ }
    }
}
```
In runtime, `kani::internal::mode()` will return
kani::internal::ORIGINAL, which runs the original body. The
transformation will replace this call by a different assignment in case
the function needs to be replaced. The body of the unused closures will
be replaced by a `UNREACHABLE` statement to avoid unnecessary code to be
analyzed.

This is still fairly hacky, but hopefully we can cleanup this logic once
Rust adds contract support. :crossed_fingers:

Resolves #3206
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C] Bug This is a bug. Something isn't working. T-High Priority Tag issues that have high priority Z-Contracts Issue related to code contracts
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants