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

Unexpected failure when modifies attribute points to a ZST #3181

Closed
celinval opened this issue May 14, 2024 · 4 comments · Fixed by #3417
Closed

Unexpected failure when modifies attribute points to a ZST #3181

celinval opened this issue May 14, 2024 · 4 comments · Fixed by #3417
Assignees
Labels
[C] Bug This is a bug. Something isn't working. Z-Contracts Issue related to code contracts

Comments

@celinval
Copy link
Contributor

celinval commented May 14, 2024

I tried this code:

    #[requires(assert_valid_ptr(dst) && has_valid_value(dst))]
    #[modifies(dst)]
    pub unsafe fn replace<T>(dst: *mut T, src: T) -> T {
        std::ptr::replace(dst, src)
    }

    /// This one fails in the builtin-library
    #[kani::proof_for_contract(contracts::replace)]
    pub fn check_replace_unit() {
        check_replace_impl::<()>();
    }

    /// This one succeeds
    #[kani::proof_for_contract(contracts::replace)]
    pub fn check_replace_char() {
        check_replace_impl::<char>();
    }

    fn check_replace_impl<T: kani::Arbitrary + Eq + Clone>() {
        let mut dst = T::any();
        let orig = dst.clone();
        let src = T::any();
        let ret = unsafe { contracts::replace(&mut dst, src.clone()) };
        assert_eq!(ret, orig);
        assert_eq!(dst, src);
    }

using the following command line invocation:

kani contracts.rs

with Kani version: 0.51.0-dev (#3107)

I expected to see this happen: verification succeeds

Instead, this happened: Verification failed with the following failure:

SUMMARY:
 ** 1 of 1485 failed (3 unreachable)
Failed Checks: ptr NULL or writable up to size
 File: "<builtin-library-__CPROVER_contracts_library>", line 162, in __CPROVER_contracts_car_set_insert
@celinval celinval added the [C] Bug This is a bug. Something isn't working. label May 14, 2024
@remi-delmas-3000
Copy link
Contributor

#[modifies(dst)]
Does this mean that

  1. the function modifies the contents of the location pointed to by dst or
  2. the contents of dst ?

Assuming 1), this should be translated to __CPROVER_assigns(*dst) and CBMC will check that *dst is a valid location of size 0. Which may not be the case depending on how pointers to ZSTs are initialised in Rust.

@celinval
Copy link
Contributor Author

In Rust, a 0 sized access is valid for any pointer, and the address of a ZST variable can be anything. #3134 changed Kani to implement that.

I believe CBMC still expects the pointer to point to a valid allocation, which is likely the reason why this is failing. If that's the case, we need to omit the assigns clause.

@feliperodri feliperodri added this to the Function Contracts MVP milestone Jun 6, 2024
@tautschnig tautschnig added the Z-Contracts Issue related to code contracts label Aug 1, 2024
@carolynzech carolynzech self-assigned this Aug 1, 2024
@carolynzech
Copy link
Contributor

@celinval For this line:
#[requires(assert_valid_ptr(dst) && has_valid_value(dst))]
where are assert_valid_ptr and has_valid_value defined? I'm trying to reproduce the failure and it won't compile because the functions aren't in scope.
For now, I just commented out this requires clause, since it's not required to reproduce the issue anyway, but just curious.

@celinval
Copy link
Contributor Author

celinval commented Aug 2, 2024

I think those were old names

github-merge-queue bot pushed a commit that referenced this issue Aug 5, 2024
This PR filters out ZST pointee types when generating CMBC assigns
clauses for contracts. This prevents CMBC from complaining that the
pointer doesn't point to a valid allocation.

Resolves #3181

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
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. Z-Contracts Issue related to code contracts
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants