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

[saw-core-coq] Check bitvector lemmas for < 4 bits #1495

Merged
merged 4 commits into from
Nov 6, 2021

Conversation

m-yac
Copy link
Contributor

@m-yac m-yac commented Nov 3, 2021

Inspired by @robdockins' comment in #1236, this PR adds the axiom:

Axiom holds_up_to_4 : forall (P : nat -> Prop),
                      P 0 -> P 1 -> P 2 -> P 3 -> P 4 ->
                      forall n, P n.

and uses it instead of Admitted to ensure that the bitvector propositions in SAWCoreBitvectors.v at least work for inputs with less than four bits. In the process, this PR fixes a few incorrect bitvector propositions (namely, Preorder_isBvslt, Preorder_isBvult, isBvsle_suc_r, and bvNeg_msb).

This is quick way to gain some confidence in the existing lemmas, not a long-term solution. To reinforce that, the tactic holds_for_bits_up_to_4, which tries to prove a lemma using the above axiom, prints out a warning each time it is used.

This PR also fixes the definition of shiftR, as noticed in #1236.

This is a draft because I still need to either fix or remove bvslt_bvSub_r and bvslt_bvSub_l, and have yet to update the heapster example proofs.

m-yac added 2 commits November 2, 2021 16:38
- replace `Preorder_isBvslt`, `Preorder_isBvult` with `Transitive_isBvslt`, `Transitive_isBvult`
- fix precondition of `isBvsle_suc_r`
- add preconditions to `bvNeg_msb`
@robdockins
Copy link
Contributor

Yeah, that's more or less what I had in mind. It's a touch disappointing it starts to slow down too much beyond 4 bits, but that is probably enough to catch most of the problematic corner cases and such.

Copy link
Contributor

@jpaykin jpaykin 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! I think size-3 or 4 bitvectors should be fine for the types of lemmas you have here. If you really did want to scale to larger dimensions, I could suggest 2 things... (1) instead of dependent destruction, just enumerate all the n-digit bitvectors. I'm not sure if this would be faster but it's a possibility; (2) use something like quickchick (https://github.com/QuickChick/QuickChick) to randomly test inputs at larger dimensions, instead of enumerating all of them.

@m-yac m-yac marked this pull request as ready for review November 5, 2021 23:16
@m-yac
Copy link
Contributor Author

m-yac commented Nov 5, 2021

Thanks Rob and Jen. I got the Heapster examples working again with these new lemmas, so this should be ready to merge now.

Note that I updated all the checks to only go up to 3 bits, for the sake of performance (checking SAWCoreBitvectors.v now takes 35s instead of 72s on my machine). I did this after realizing that every time either the saw-core prelude or the proof automation changes, this file would need to be recompiled – so it would be nice if it didn't take over a minute to check every time.

If this idea ends up being more permanent, we should definitely look into implementing your suggestions @jpaykin.

@m-yac m-yac force-pushed the saw-core-coq/partially-check-bv-props branch from 7665e7f to 91825b0 Compare November 5, 2021 23:40
@m-yac m-yac added subsystem: heapster Issues specifically related to memory verification using Heapster PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run subsystem: saw-core-coq Issues related to converting saw-core to Gallina for use with the Coq/Rocq theorem prover labels Nov 5, 2021
@mergify mergify bot merged commit 3537f97 into master Nov 6, 2021
@mergify mergify bot deleted the saw-core-coq/partially-check-bv-props branch November 6, 2021 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run subsystem: heapster Issues specifically related to memory verification using Heapster subsystem: saw-core-coq Issues related to converting saw-core to Gallina for use with the Coq/Rocq theorem prover
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants