Skip to content

Conversation

@qinheping
Copy link
Contributor

Description of changes:

There are unreachable loops in the code snippet compiled from generator resume. For example, in the goto program compiled from the benchmark moved_locals.rs:

     8: ASSERT false // KANI_CHECK_ID_moved_locals.c1d95029::moved_locals_3
        // 716 file /Users/qinhh/Repos/kani/qinheping/tests/kani/Generator/rustc-generator-tests/moved-locals.rs line 71 column 5 function overlap_x_and_y::{closure#0}
        ASSERT 0 ≠ 0 // [KANI_CHECK_ID_moved_locals.c1d95029::moved_locals_3] generator resumed after completion
        // 717 file /Users/qinhh/Repos/kani/qinheping/tests/kani/Generator/rustc-generator-tests/moved-locals.rs line 71 column 5 function overlap_x_and_y::{closure#0}
        ASSUME 0 ≠ 0
        // 718 file /Users/qinhh/Repos/kani/qinheping/tests/kani/Generator/rustc-generator-tests/moved-locals.rs line 71 column 5 function overlap_x_and_y::{closure#0}
        GOTO 8

The jump-back instruction GOTO 8 is unreachable as it is after ASSUME 0 ≠ 0.

However, such unreachable loops can actually be correctly handle by CBMC. So we can remove the unwinding numbers from the benchmarks modified in this PR.

Resolved issues:

Resolves #ISSUE-NUMBER

Related RFC:

Optional #ISSUE-NUMBER.

Call-outs:

Testing:

  • How is this change tested?

  • Is this a refactor change?

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

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

@qinheping qinheping requested a review from a team as a code owner May 4, 2023 21:54
Copy link
Contributor

@celinval celinval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why you decided to remove the unwind in these tests. Did you try other tests as well? I'm assuming we have a lot of unwind annotations that are not quite needed anymore.

@qinheping
Copy link
Contributor Author

I'm curious why you decided to remove the unwind in these tests. Did you try other tests as well? I'm assuming we have a lot of unwind annotations that are not quite needed anymore.

I tried the synthesizer on all benchmarks with unwinding numbers and investigated what kind of loops are in the benchmarks. Some of those benchmarks are actually unbounded even though we specify unwinding number for them. So, to better track which benchmarks should be the target of the synthesizer, I decided to remove unwinding numbers from those unbounded benchmarks (assuming the input become unbounded).

Yes, there are more unbounded benchmarks in which we can remove the unwinding numbers. This PR now only covers loops caused by generators. Let me also add other kind of loops into this PR.

@qinheping qinheping changed the title Remove unneeded unwinds Remove unneeded unwinds from unbounded benchmarks May 5, 2023
qinheping added 2 commits May 5, 2023 16:03
There are unreachable loops in the code snippet compiled from generator resume.
Such unreachable loops can actually be correctly handled by CBMC. So we can remove the unwinding numbers from the benchmarks modified in this PR.
@qinheping qinheping force-pushed the remove-unneeded-unwinds branch from 756f7a5 to c9ce04b Compare May 5, 2023 21:53
@qinheping qinheping enabled auto-merge (squash) May 5, 2023 21:53
@qinheping qinheping merged commit 708bc35 into model-checking:main May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants