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

Niche optimization encoding is broken #1533

Closed
celinval opened this issue Aug 17, 2022 · 4 comments · Fixed by #1542
Closed

Niche optimization encoding is broken #1533

celinval opened this issue Aug 17, 2022 · 4 comments · Fixed by #1542
Assignees
Labels
[C] Bug This is a bug. Something isn't working. [F] Soundness Kani failed to detect an issue

Comments

@celinval
Copy link
Contributor

I tried this code:

#[kani::proof]
fn check_i8() {
    let v1: i8 = -10;
    let n1 = NonZeroI8::new(v1);
    assert!(n1.is_some());
}

using the following command line invocation:

kani test.rs

with Kani version:

I expected to see this happen: Harness should succeed.

Instead, this happened: Verification failed.

SUMMARY:
 ** 1 of 8 failed
Failed Checks: assertion failed: n1.is_some()
File: "/kani/tests/kani/Invariants/check_nonzero.rs", line 25, in check_i8

VERIFICATION:- FAILED

Note that we do have a test that should be checking this (tests/kani/Invariants/check_nonzero.rs), and if you run it manually, you can see that the test is failing. However, the test uses kani::expect_fail() which is also broken. So the tests actually succeeds in our CI.

@celinval celinval added the [C] Bug This is a bug. Something isn't working. label Aug 17, 2022
@celinval celinval self-assigned this Aug 17, 2022
@celinval
Copy link
Contributor Author

It looks like we are failing when the value is negative. I wonder if this regression was introduced by: d0b74ca

@celinval celinval added the [F] Soundness Kani failed to detect an issue label Aug 17, 2022
@celinval
Copy link
Contributor Author

That said, the following harness only fails in the second assert:

#[kani::proof]
fn check_i8() {
    let v1: i8 = -10;
    let n1 = unsafe { NonZeroI8::new_unchecked(v1) };

    assert_eq!(n1.get(), v1);
    assert!(Some(n1).is_some());
}

@celinval
Copy link
Contributor Author

d0b74ca

It predates this commit.

@danielsn
Copy link
Contributor

I figured out what's wrong: we're doing a signed comparison against 0, when we need unsigned.

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. [F] Soundness Kani failed to detect an issue
Projects
No open projects
Status: Done
2 participants