Skip to content

Do not turn trivially diverging loops into assume(false)#3223

Merged
tautschnig merged 7 commits intomodel-checking:mainfrom
tautschnig:fix-2909
Jun 8, 2024
Merged

Do not turn trivially diverging loops into assume(false)#3223
tautschnig merged 7 commits intomodel-checking:mainfrom
tautschnig:fix-2909

Conversation

@tautschnig
Copy link
Member

@tautschnig tautschnig commented Jun 4, 2024

CBMC's symbolic execution by default turns while(true) {} loops into assume(false) to counter trivial non-termination of symbolic execution. When unwinding assertions are enabled, however, we should report the non-termination of such loops.

Resolves: #2909

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

CBMC's symbolic execution by default turns `while(true) {}` loops into
`assume(false)` to counter trivial non-termination of symbolic
execution. When unwinding assertions are enabled, however, we should
report the non-termination of such loops.
@tautschnig tautschnig requested a review from a team as a code owner June 4, 2024 09:47
@zhassan-aws
Copy link
Contributor

Thanks @tautschnig for the quick fix! Should we consider this a soundness issue in CBMC and fix it on the CBMC side instead? i.e., when unwinding assertions are enabled, self loops should not be replaced with assume(false)?

@tautschnig
Copy link
Member Author

Thanks @tautschnig for the quick fix! Should we consider this a soundness issue in CBMC and fix it on the CBMC side instead? i.e., when unwinding assertions are enabled, self loops should not be replaced with assume(false)?

I think we should indeed seek to make this consistent on the CBMC side for version 6. I'll come up with a PR.

@qinheping
Copy link
Contributor

Yes, when applying loop contracts, lack of decreases may lead to vacuous verification, which is the case of Zyad's comment. Now the synthesizer for Kani doesn't synthesize decrease clauses, and should does before it became stable.

@feliperodri
Copy link
Contributor

Thanks @tautschnig for the quick fix! Should we consider this a soundness issue in CBMC and fix it on the CBMC side instead? i.e., when unwinding assertions are enabled, self loops should not be replaced with assume(false)?

I think we should indeed seek to make this consistent on the CBMC side for version 6. I'll come up with a PR.

@tautschnig if we fix this on version 6, we do not need to merge this PR, correct? Shall we close it in favor of the new update?

@tautschnig
Copy link
Member Author

Thanks @tautschnig for the quick fix! Should we consider this a soundness issue in CBMC and fix it on the CBMC side instead? i.e., when unwinding assertions are enabled, self loops should not be replaced with assume(false)?

I think we should indeed seek to make this consistent on the CBMC side for version 6. I'll come up with a PR.

@tautschnig if we fix this on version 6, we do not need to merge this PR, correct? Shall we close it in favor of the new update?

Merging this PR would help disentangle a Kani issue from the CBMC release. Once CBMC v6 is released and Kani starts using it we can then remove this, but then we will anyway end up removing several options from the command line being used to invoke CBMC.

@tautschnig tautschnig merged commit adc1f0d into model-checking:main Jun 8, 2024
@tautschnig tautschnig deleted the fix-2909 branch June 8, 2024 15:02
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.

Diverging modifies clauses cause unsound vacuity

4 participants