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

fix: Count proof obligations of loop headers #3244

Merged
merged 3 commits into from
Dec 22, 2022

Conversation

RustanLeino
Copy link
Collaborator

This PR changes the verifier to increment the assertion count when it encounters a loop with non-free invariants. This increment was previously missing, which caused certain "loop invariant on entry" checks to be omitted (see #3243).

Fixes #3243

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@RustanLeino RustanLeino marked this pull request as ready for review December 21, 2022 18:38
@RustanLeino RustanLeino enabled auto-merge (squash) December 21, 2022 18:38
Copy link
Member

@MikaelMayer MikaelMayer left a comment

Choose a reason for hiding this comment

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

This solves this soundness bug, but now I realize that the mechanism for computing the number of assertions (and emitting nothing if there is no assertion) is stateful, which makes me wonder whether we are missing other cases as well. Isn't there a way that we wouldn't need to manually increment a variable?

@RustanLeino RustanLeino merged commit b054243 into dafny-lang:master Dec 22, 2022
@RustanLeino
Copy link
Collaborator Author

Yes, it could be stateless. One way to do that would be to generate the entire Boogie procedure implementation and then apply a visitor that checks if there are any proof obligations. Indeed, Boogie could even do that instead of Dafny. It is alarming that the bug that this PR fixes has been around unnoticed for several years.

@cpitclaudel
Copy link
Member

It appears that the bug was introduced by 2d4176b1 , but this PR isn't reverting the relevant part of that commit (instead it uses a more localized fix). @RustanLeino could you have a second look to check if we might have introduced any other issue not covered by this PR with the original faulty commit?

RustanLeino added a commit to RustanLeino/dafny that referenced this pull request Dec 29, 2022
This PR changes the verifier to increment the assertion count when it
encounters a loop with non-free invariants. This increment was
previously missing, which caused certain "loop invariant on entry"
checks to be omitted (see dafny-lang#3243).

Fixes dafny-lang#3243


<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
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.

Loop invariant check omitted in absence of other proof obligations
3 participants