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

Low performance verification caused by conditional jump #1046

Closed
adpaco-aws opened this issue Apr 14, 2022 · 0 comments · Fixed by #1117
Closed

Low performance verification caused by conditional jump #1046

adpaco-aws opened this issue Apr 14, 2022 · 0 comments · Fixed by #1117
Labels
[C] Bug This is a bug. Something isn't working. [E] Performance Track performance improvement (Time / Memory / CPU)

Comments

@adpaco-aws
Copy link
Contributor

While writing a test case for the ctpop intrinsic, I wrote something similar to this code:

fn my_ctpop_u32(x: u32) -> u32 {
    let mut count = 0;
    let num_bits = u32::BITS;
    for i in 0..num_bits {
        // Get value at index `i`
        let bitmask = 1 << i;
        let bit = x & bitmask;
        if bit != 0 {
            count += 1;
        }
        // assert!(count >= 0);
    }
    count
}

#[kani::proof]
fn main() {
    let var: u32 = kani::any();
    assert!(my_ctpop_u32(var) == ctpop(var));
}

Running kani on this example (without arguments) does not terminate (at least for a few minutes). We can pass --unwind up to 8 or so in order to get a result, but it will fail the unwinding assertion unless we pass 33 (which gets us back to non-termination).

What is surprising about this example is that adding back the assertion that is commented out will allow CBMC to successfully verify the code in a few seconds. Apparently, this is because the conditional jump in the goto program following from the if bit != 0 { ... } complicates the unwinding and verification of this example.

@tautschnig proposed using the --ensure-one-backedge-per-target option from goto-instrument in order to fix this, but it is not clear if this transformation preserves soundness in some cases (see the regression failure in #900).

@adpaco-aws adpaco-aws added [C] Bug This is a bug. Something isn't working. Area: verification [E] Performance Track performance improvement (Time / Memory / CPU) labels Apr 14, 2022
@adpaco-aws adpaco-aws mentioned this issue Apr 14, 2022
4 tasks
tautschnig added a commit to tedinski/rmc that referenced this issue Apr 28, 2022
rustc reasonably translates if statements at end of a loop body to
backwards jumps to the loop head. This, however, causes CBMC's symbolic
execution to spuriously treat these as nested loops. This commit now
enables a rewrite step that had previously been built for exactly such
cases.

This treatment as nested loops implied that seemingly lower unwinding
bounds were sufficient (as the loop unwinding counter got reset). Two of
the tests exhibited this behaviour, which now makes larger unwinding
bounds necessary.

Fixes: model-checking#1046

Co-authored-by: Daniel Schwartz-Narbonne <dsn@amazon.com>
tautschnig added a commit to tedinski/rmc that referenced this issue Apr 28, 2022
rustc reasonably translates if statements at end of a loop body to
backwards jumps to the loop head. This, however, causes CBMC's symbolic
execution to spuriously treat these as nested loops. This commit now
enables a rewrite step that had previously been built for exactly such
cases.

This treatment as nested loops implied that seemingly lower unwinding
bounds were sufficient (as the loop unwinding counter got reset). Three
of the tests exhibited this behaviour, which now makes larger unwinding
bounds necessary.

Fixes: model-checking#1046

Co-authored-by: Daniel Schwartz-Narbonne <dsn@amazon.com>
danielsn added a commit that referenced this issue Apr 28, 2022
…nly (#1117)

rustc reasonably translates if statements at end of a loop body to
backwards jumps to the loop head. This, however, causes CBMC's symbolic
execution to spuriously treat these as nested loops. This commit now
enables a rewrite step that had previously been built for exactly such
cases.

This treatment as nested loops implied that seemingly lower unwinding
bounds were sufficient (as the loop unwinding counter got reset). Three
of the tests exhibited this behaviour, which now makes larger unwinding
bounds necessary.

Fixes: #1046

Co-authored-by: Daniel Schwartz-Narbonne <dsn@amazon.com>

Co-authored-by: Michael Tautschnig <tautschn@amazon.com>
Co-authored-by: Daniel Schwartz-Narbonne <dsn@amazon.com>
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. [E] Performance Track performance improvement (Time / Memory / CPU)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant