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

Failing verification for proptest example using format! #576

Closed
nchong-at-aws opened this issue Oct 20, 2021 · 5 comments
Closed

Failing verification for proptest example using format! #576

nchong-at-aws opened this issue Oct 20, 2021 · 5 comments
Assignees
Labels
[C] Bug This is a bug. Something isn't working.

Comments

@nchong-at-aws
Copy link
Contributor

I tried this code, which is a slightly modified example from the proptest book (https://altsysrq.github.io/proptest-book/proptest/getting-started.html):

#![allow(unused)]

fn __nondet<T>() -> T {
    unimplemented!()
}

fn __VERIFIER_assume(cond: bool) {
    unimplemented!()
}

fn parse_date(s: &str) -> Option<(u32, u32, u32)> {
    if 10 != s.len() {
        return None;
    }

    if "-" != &s[4..5] || "-" != &s[7..8] {
        return None;
    }

    let year = &s[0..4];
    let month = &s[5..7]; //< fixed from example that can be found using proptest
    let day = &s[8..10];

    year.parse::<u32>().ok().and_then(|y| {
        month
            .parse::<u32>()
            .ok()
            .and_then(|m| day.parse::<u32>().ok().map(|d| (y, m, d)))
    })
}

pub fn main() {
    let y: u32 = __nondet();
    let m: u32 = __nondet();
    let d: u32 = __nondet();
    __VERIFIER_assume(0 <= y && y < 10000);
    __VERIFIER_assume(1 <= m && m < 13);
    __VERIFIER_assume(1 <= d && d < 32);
    let (y2, m2, d2) = parse_date(&format!("{:04}-{:02}-{:02}", y, m, d)).unwrap();
    assert!(y == y2);
    assert!(m == m2);
    assert!(d == d2);
}

using the following command line invocation:

rmc example.rs

with RMC version: a728d8d

I expected to see this happen: VERIFICATION SUCCESSFUL

Instead, this happened: VERIFICATION FAILED

@nchong-at-aws nchong-at-aws added the [C] Bug This is a bug. Something isn't working. label Oct 20, 2021
@nchong-at-aws
Copy link
Contributor Author

This also fails:

pub fn main() {
  assert!("2021".parse::<u32>().unwrap() == 2021);
}

@zhassan-aws
Copy link
Contributor

zhassan-aws commented Mar 18, 2022

Experiment with switching to running goto-instrument with --generate-function-body assert-false-assume-false.

We should also replace the call to the function with codegen_unimplemented so that we get a proper error message on what is unsupported.

celinval added a commit to celinval/kani-dev that referenced this issue Mar 19, 2022
 - Support to --lib-c is not well tested. Keep it as experiemental.
 - Change how CBMC behaves when a function that is not defined is
   reachable. This mitigates issues model-checking#576. We should still add an
   unimplemented assertion so we can flip the results of other checks to
   undetermined.
celinval added a commit to celinval/kani-dev that referenced this issue Mar 19, 2022
 - Support to --lib-c is not well tested. Keep it as experiemental.
 - Change how CBMC behaves when a function that is not defined is
   reachable. This mitigates issues model-checking#576. We should still add an
   unimplemented assertion so we can flip the results of other checks to
   undetermined.
celinval added a commit to celinval/kani-dev that referenced this issue Mar 22, 2022
 - Support to --lib-c is not well tested. Keep it as experiemental.
 - Change how CBMC behaves when a function that is not defined is
   reachable. This mitigates issues model-checking#576. We should still add an
   unimplemented assertion so we can flip the results of other checks to
   undetermined.
celinval added a commit to celinval/kani-dev that referenced this issue Mar 22, 2022
 - Support to --lib-c is not well tested. Keep it as experiemental.
 - Change how CBMC behaves when a function that is not defined is
   reachable. This mitigates issues model-checking#576. We should still add an
   unimplemented assertion so we can flip the results of other checks to
   undetermined.
@celinval
Copy link
Contributor

@zhassan-aws Do you mind adding taking a look at this one while you are looking into adding compiler warnings to codegen_unimplemented? Thanks

celinval added a commit that referenced this issue Mar 22, 2022
 - Support to --lib-c is not well tested. Keep it as experiemental.
 - Change how CBMC behaves when a function that is not defined is
   reachable. This mitigates issues #576. We should still add an
   unimplemented assertion so we can flip the results of other checks to
   undetermined.
@zhassan-aws
Copy link
Contributor

Summarizing a discussion from Friday 4/8:

  • For the program involving "2021".parse::<u32>(), the failure is due to a missing definition for an std function, which is caused by Kani not linking the standard library.
  • The reported failure is not user friendly as it involves the mangled name of the missing function as well as an unclear description ("assertion false"):
Check 3:  _ZN4core3num60_$LT$impl$u20$core..str..traits..FromStr$u20$for$u20$u32$GT$8from_str17hbdfd7c3b5dc0b1b5E.assertion.1
         - Status: FAILURE
         - Description: "assertion false"
         - Location: Unknown File in function _ZN4core3num60_$LT$impl$u20$core..str..traits..FromStr$u20$for$u20$u32$GT$8from_str17hbdfd7c3b5dc0b1b5E
  • The longer term fix is for Kani to link the standard library so that the definitions of those functions are brought in. This however requires:
    1. Changing the Kani flow so that it codegens the std crate when compiling a file/package, or including the symtab/goto binary of the std crate in the Kani release package
    2. Performing some sort of slicing (e.g. via CBMC's --reachability-slice or via implementing our own linker/slicer) to aggressively prune the code that gets analyzed (much more than what is pruned by CBMC's --drop-unused-functions).
  • This requires some effort, so in the short term, we will just clean up the reported failure using the result parser/post-processor by:
    1. Showing the original (unmangled) name of the missing function
    2. Improving the description of the failure

@zhassan-aws zhassan-aws assigned jaisnan and unassigned zhassan-aws Apr 19, 2022
tedinski pushed a commit to tedinski/rmc that referenced this issue Apr 22, 2022
 - Support to --lib-c is not well tested. Keep it as experiemental.
 - Change how CBMC behaves when a function that is not defined is
   reachable. This mitigates issues model-checking#576. We should still add an
   unimplemented assertion so we can flip the results of other checks to
   undetermined.
tedinski pushed a commit to tedinski/rmc that referenced this issue Apr 25, 2022
 - Support to --lib-c is not well tested. Keep it as experiemental.
 - Change how CBMC behaves when a function that is not defined is
   reachable. This mitigates issues model-checking#576. We should still add an
   unimplemented assertion so we can flip the results of other checks to
   undetermined.
tedinski pushed a commit to tedinski/rmc that referenced this issue Apr 26, 2022
 - Support to --lib-c is not well tested. Keep it as experiemental.
 - Change how CBMC behaves when a function that is not defined is
   reachable. This mitigates issues model-checking#576. We should still add an
   unimplemented assertion so we can flip the results of other checks to
   undetermined.
tedinski pushed a commit that referenced this issue Apr 27, 2022
 - Support to --lib-c is not well tested. Keep it as experiemental.
 - Change how CBMC behaves when a function that is not defined is
   reachable. This mitigates issues #576. We should still add an
   unimplemented assertion so we can flip the results of other checks to
   undetermined.
@zhassan-aws zhassan-aws mentioned this issue May 25, 2022
4 tasks
@celinval
Copy link
Contributor

I believe this issue has been fixed by the MIR Linker (#1588). The following test has been added to our regression to ensure we can correctly handle parse function.

Unfortunately, I wasn't able to run the original test from this issue in a reasonable time. The test no longer fail, but symbolic execution is just taking too long. Maybe we should create a different issue to track the performance part of it.

@celinval celinval self-assigned this Oct 24, 2022
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

4 participants