Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Precondition Invariant: Disjunct over matching state invariants #786

Merged
merged 18 commits into from
Aug 4, 2022

Conversation

jerhard
Copy link
Member

@jerhard jerhard commented Jul 18, 2022

This PR builds on #752.
As already discussed, one must take into account that we may discover multiple contexts that fulfill a precondition we generate for a particular context. Thus, the postcondition of a precondition_loop_invariant has to be a disjunction over invariants obtained for "matching" unknowns.

An alternative implementation may use Union-Find data structures to group preconditions together that do no exclude each other. While this alternative approach would reduce the number of precondition invariants generated, it may also lose some information.

jerhard added 6 commits July 14, 2022 12:27
In this example, among others, the following precondition_loop_invariants are generated:

loop_invariant:
    string: result == 1
    [...]
  precondition:
    string: '*ptr1 == 5 && *ptr2 == 5'

and:

loop_invariant:
    string: result == 0
    [...]
  precondition:
    string: '*ptr1 == 5 && *ptr2 == 5'

To fix this issue, grouping of precondition_loop_invariants is required.
@jerhard jerhard added the pr-dependency Depends or builds on another PR, which should be merged before label Jul 18, 2022
@jerhard jerhard requested a review from sim642 July 18, 2022 13:55
@jerhard
Copy link
Member Author

jerhard commented Jul 18, 2022

An example where the invariants generated were previously inconsistent was added here.

jerhard added 2 commits July 18, 2022 17:30
…ed for one state.

In case that an invariant for a state  could not be generated, no precondition_loop_invariant is generated. Reason: The invariant (that failed to be generated) must be part of the postcondition.
@jerhard
Copy link
Member Author

jerhard commented Jul 18, 2022

Now, for the example, instead of generating inconsistent/wrong precondition_loop_invariants (see the commit message),
we generate precondition invariants as follows:

  loop_invariant:
    string: result == 0 || result == 1
    [...]
  precondition:
    string: '*ptr1 == 5 && *ptr2 == 5'
    [...]

However, due to the way these precondition invariants are created, we still create one precondition invariant per context per program point. For example in this case, we generate this precondition invariant twice.

@michael-schwarz michael-schwarz self-requested a review July 19, 2022 08:04
@sim642 sim642 added the unsound label Jul 25, 2022
} else {
result = 1;
}
// Look at the generated witness.yml to check whether there are contradictory precondition_loop_invariant[s]
Copy link
Member

Choose a reason for hiding this comment

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

This isn't great as a test, but without something like #787 we cannot do any better either.

@sim642
Copy link
Member

sim642 commented Jul 25, 2022

However, due to the way these precondition invariants are created, we still create one precondition invariant per context per program point. For example in this case, we generate this precondition invariant twice.

Given the groupings, couldn't we avoid the duplication? Or would that not work to the possible non-symmetry of the relation?

@jerhard
Copy link
Member Author

jerhard commented Jul 27, 2022

Given the groupings, couldn't we avoid the duplication? Or would that not work to the possible non-symmetry of the relation?

As the grouping is implemented now, the problem with deduplicating entries is indeed the possibility of non-symmetry.
However, it should be possible to first group start-states according their invariant-representation: One would put start-states that lead to the same invariant in the same bucket; then, for each such bucket one would only have to generate on precondition-invariant. There one would still have to iterate over all other start states of the function though, to check whether they may make the precondition true.

For this bucket-per-invariant approach, the invariants should best be usable as keys in some hashtable.

Copy link
Member

@sim642 sim642 left a comment

Choose a reason for hiding this comment

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

Looks good! For the order of merging things I would propose the following:

  1. Get Add experimental generation and validation of YAML witness invariants with preconditions #752 approved itself, because then its diff is easier to look at.
  2. Then merge this into Add experimental generation and validation of YAML witness invariants with preconditions #752, so it becomes a complete implementation on its own.
  3. Finally merge Add experimental generation and validation of YAML witness invariants with preconditions #752 into master, with changes from both included.

@jerhard
Copy link
Member Author

jerhard commented Aug 3, 2022

Get #752 approved itself, because then its diff is easier to look at.

What do you mean by that? How is the diff presented differently when the PR is approved? Can we not just merge this PR into #752?

@sim642
Copy link
Member

sim642 commented Aug 3, 2022

What do you mean by that? How is the diff presented differently when the PR is approved? Can we not just merge this PR into #752?

I mean if this is merged into #752, then when reviewing #752 the diff also contains all the changes of this PR, as if they were made in #752. It just makes it more difficult to distinguish, which changes we already approved here and thus needn't be re-reviewed in #752.

But you can merge this into #752 right away as well, if you want. It doesn't really matter for me.

@sim642 sim642 removed the pr-dependency Depends or builds on another PR, which should be merged before label Aug 4, 2022
@sim642 sim642 merged commit 9e07541 into yaml-witness-precondition Aug 4, 2022
@sim642 sim642 deleted the yaml-witness-precondition-groups branch August 4, 2022 07:20
@sim642 sim642 added this to the v2.0.0 milestone Aug 12, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants