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

Performance regression with new rust toolchain #2191

Closed
celinval opened this issue Feb 6, 2023 · 4 comments · Fixed by #2301
Closed

Performance regression with new rust toolchain #2191

celinval opened this issue Feb 6, 2023 · 4 comments · Fixed by #2301
Assignees
Labels
[C] Bug This is a bug. Something isn't working. [F] Spurious Failure Issues that cause Kani verification to fail despite the code being correct. T-CBMC Issue related to an existing CBMC issue

Comments

@celinval
Copy link
Contributor

celinval commented Feb 6, 2023

Our recent efforts to upgrade the rust toolchain to nightly-2023-01-23 #2149 is currently blocked due to a significant performance regression.

2 out of 3 tests added to measure the performance of Kani with Vectors seems to be highly affected. Here the analysis we added to the PR for tests/perf/vec/box_dyn.

Running the failing test in my local machine with cargo kani, I got the following:

Configuration Time Memory
main (CBMC 5.75) 5.74s 165MB
this PR (CBMC 5.75) 206s 16GB
main (CBMC 5.70) 200.s 16GB

Note that the time in this PR is very similar to the one I obtained when running main with an older CBMC version (before diffblue/cbmc#7230).

I wonder if whatever has changed neutralized the optimization that @tautschnig introduced. 😕

Originally posted by @celinval in #2149 (comment)

@celinval celinval added [C] Bug This is a bug. Something isn't working. [F] Spurious Failure Issues that cause Kani verification to fail despite the code being correct. labels Feb 6, 2023
@celinval
Copy link
Contributor Author

celinval commented Feb 6, 2023

@tautschnig do you think you can take a look at this? Thanks!

@tautschnig
Copy link
Member

It seems that the way null-ness is tested has been changed, and now CBMC no longer manages to simplify this. The following C snippet demonstrates the same problem (which we'll have to make CBMC solve solely via constant propagation and simplification):

union U
{
  void * ptr;
  __CPROVER_size_t n;
};

int main()
{
  int *p = __CPROVER_allocate(sizeof(int), 0);
  union U u = { .ptr = p };
  __CPROVER_assert(u.n != 0, "is not null");
}

Working on a fix.

tautschnig added a commit to tautschnig/cbmc that referenced this issue Feb 22, 2023
Injecting casts to integer types should not hamper our ability to
simplify tests for null-ness. Such expressions emerge from Rust code,
see model-checking/kani#2191.
tautschnig added a commit to tautschnig/cbmc that referenced this issue Feb 22, 2023
Injecting casts to integer types should not hamper our ability to
simplify tests for null-ness. Such expressions emerge from Rust code,
see model-checking/kani#2191.
tautschnig added a commit to tautschnig/cbmc that referenced this issue Feb 22, 2023
Injecting casts to integer types should not hamper our ability to
simplify tests for null-ness. Such expressions emerge from Rust code,
see model-checking/kani#2191.
@tautschnig
Copy link
Member

Fixed in diffblue/cbmc#7556.

tautschnig added a commit to tautschnig/cbmc that referenced this issue Feb 23, 2023
Injecting casts to integer types should not hamper our ability to
simplify tests for null-ness. Such expressions emerge from Rust code,
see model-checking/kani#2191.
@celinval
Copy link
Contributor Author

Thank you so much!! @tautschnig

@tautschnig tautschnig added the T-CBMC Issue related to an existing CBMC issue label Mar 1, 2023
tautschnig added a commit to tautschnig/kani that referenced this issue Mar 17, 2023
- Re-enable tests that had to be disabled with the toolchain upgrade in
  model-checking#2149. Fixes model-checking#2286, fixes model-checking#2191.
- Do not generate non-NULL pointer constants. Together with the CBMC
  version update this avoids the need for an unwinding annotation in the
  mir-linker test. Fixes model-checking#1978.
- CBMC 5.79.0 ships simplifier improvements that enable constant
  propagation to avoid slow-down with the Display trait. Fixes model-checking#1996.
- CBMC 5.79.0 ships SMT back-end fixes. Fixes model-checking#2002.

Co-authored-by: Zyad Hassan <88045115+zhassan-aws@users.noreply.github.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. [F] Spurious Failure Issues that cause Kani verification to fail despite the code being correct. T-CBMC Issue related to an existing CBMC issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants