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

Function contract doesn't propagate const function bodies correctly #3905

Open
thanhnguyen-aws opened this issue Feb 24, 2025 · 5 comments
Open
Labels
[C] Bug This is a bug. Something isn't working.

Comments

@thanhnguyen-aws
Copy link
Contributor

thanhnguyen-aws commented Feb 24, 2025

I tried this code:

#[ensures(|result| result.as_usize().is_power_of_two())]
pub const fn Align_of<T>() -> Self {
    // This can't actually panic since type alignment is always a power of two.
    const { Alignment::new(mem::align_of::<T>()).unwrap() }
}

#[kani::proof_for_contract(Alignment::of)]
pub fn check_of_i32() {
    let _ = Align_of::<i32>();
}

and

#[kani::proof]
pub fn check_of_i32() {
    let a = Align_of::<i32>();
    assert!(a.as_usize().is_power_of_two());
}

using the following command line invocation:

kani src/main.rs  -Z function-contracts

with Kani version: 0.59.0

I expected to see this happen: The same results for both harnesses.

Instead, this happened: The kani::proof_for_contract failed but the kani::proof succeed

@thanhnguyen-aws thanhnguyen-aws added the [C] Bug This is a bug. Something isn't working. label Feb 24, 2025
@thanhnguyen-aws
Copy link
Contributor Author

Another example:
This harness fails:

#[kani::ensures(|result : &NonNull<T>| !result.as_ptr().is_null())]
pub const fn from_raw_parts_test<T>(
    data_pointer: NonNull<impl ptr::Thin>,
    metadata: <T as ptr::Pointee>::Metadata,
) -> NonNull<T> {
    // SAFETY: The result of `ptr::from::raw_parts_mut` is non-null because `data_pointer` is.
    unsafe {
        NonNull::new_unchecked(ptr::from_raw_parts_mut(data_pointer.as_ptr(), metadata))
    }
}

#[kani::proof_for_contract(from_raw_parts_test)]
pub fn non_null_check_from_raw_parts() {
    let dangling_ptr = NonNull::<i8>::dangling();
    let zero_length = from_raw_parts_test::<()> (dangling_ptr, ());
}

But it will succeed if I add this precondition:

#[kani::requires(!data_pointer.as_ptr().is_null())]

However, this precondition is not necessary because this harness succeeds:

#[kani::proof]
pub fn non_null_check_from_raw_parts() {
    let dangling_ptr = NonNull::<i8>::dangling();
    assert!(!dangling_ptr.as_ptr().is_null());
    let zero_length = from_raw_parts_test::<()> (dangling_ptr, ());
}

And this harness succeeds too:

#[kani::proof]
pub fn non_null_check_from_raw_parts() {
    let dangling_ptr = NonNull::<i8>::dangling();
    let zero_length = from_raw_parts_test::<()> (dangling_ptr, ());
    assert!(!zero_length.as_ptr().is_null());
}

@tautschnig
Copy link
Member

For the record, here is a full reproducer of the const vs no-const disparity:

#![feature(ptr_alignment_type)]

use std::ptr::Alignment;
use std::mem;

#[kani::ensures(|result| result.as_usize().is_power_of_two())]
pub const fn Align_of<T>() -> Alignment {
    // This can't actually panic since type alignment is always a power of two.
    const { Alignment::new(mem::align_of::<T>()).unwrap() }
}

#[kani::ensures(|result| result.as_usize().is_power_of_two())]
pub const fn Align_of_no_const<T>() -> Alignment {
    // This can't actually panic since type alignment is always a power of two.
    Alignment::new(mem::align_of::<T>()).unwrap()
}

#[kani::proof_for_contract(Align_of)]
pub fn check_of_i32() {
    let _ = Align_of::<i32>();
}

#[kani::proof_for_contract(Align_of_no_const)]
pub fn check_of_i32_no_const() {
    let _ = Align_of_no_const::<i32>();
}

@tautschnig
Copy link
Member

Further note: we already have #3864 and I wonder whether the second example provided above is the same problem?

@thanhnguyen-aws
Copy link
Contributor Author

thanhnguyen-aws commented Feb 25, 2025

Another example

#[kani::requires(true)]
pub const unsafe fn byte_add_n<T>(s : NonNull<T>, count: usize) -> NonNull<T> {
    unsafe { NonNull::new_unchecked( s.as_ptr().byte_add(count) ) }
}

#[kani::proof_for_contract(byte_add_n)]
pub fn non_null_byte_add_dangling_proof() {
    let ptr = NonNull::<i32>::dangling();
    assert!(ptr.as_ptr().addr() == 4);
    assert!(ptr.as_ptr().addr() <= (isize::MAX as usize) );
    unsafe {
        let _ = byte_add_n(ptr,0);
    }
}

Output: (note that it fail at the assertion before the function call)

Failed Checks: assertion failed: ptr.as_ptr().addr() == 4
 File: "src/main.rs", line 234, in non_null_byte_add_dangling_proof

But this one succeed:

#[kani::proof]
pub fn dangling_addr() {
    let ptr = NonNull::<i32>::dangling();
    assert!(ptr.as_ptr().addr() == 4);
    assert!(ptr.as_ptr().addr() <= (isize::MAX as usize) );
    unsafe {
        let _ = byte_add_n(ptr,0);
    }
}

@carolynzech carolynzech changed the title Function contract fail Function contract doesn't propagate const function bodies correctly Feb 25, 2025
@carolynzech
Copy link
Contributor

I'm inclined to keep this issue focused on fixing the const body issue for alignment, i.e. fixing #3905 (comment). It's possible that each of these examples trace back to the same bug (I hope so!) but we don't have enough evidence of that at this point.

github-merge-queue bot pushed a commit to model-checking/verify-rust-std that referenced this issue Mar 3, 2025
**Do not merge this PR** -- it is just meant for reviews and a CI check,
and then we'll push to main directly instead.

Some callouts:

- The changes to `const_ptr`, `mut_ptr`, and `nonnull` are necessary
because model-checking/kani#3755 introduced a
Kani model for `offset`, which expanded the range of UB that Kani can
catch. It also introduced these failures under the "safety check" label,
which is not affected by `#{should_panic]`. These updates exposed issues
with the existing contracts and proofs (most notably for ZSTs) which are
fixed here.
- A few of the proofs are still failing because of bug(s) in Kani's
function contracts. See
model-checking/kani#3905 for a tracking issue.
For now, they're commented out with a FIXME tag.

Note that the VeriFast failures are expected, since part of its CI job
checks if its proofs are up-to-date with the most recent version of the
library, and the subtree update makes it so that's not the case.

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

---------

Co-authored-by: gitbot <git@bot>
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.
Projects
None yet
Development

No branches or pull requests

3 participants