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

[Heapster] Reduce catchpoints and add more Mbox proofs #1413

Merged
merged 6 commits into from
Aug 9, 2021

Conversation

m-yac
Copy link
Contributor

@m-yac m-yac commented Aug 9, 2021

This PR has two parts:

  1. Change how the catch rule in the Heapster implication checker is translated, so that branches that could fail are pruned in favor of branches that are guaranteed not to, and update the generated files and proofs in the heapster examples directory to take into account this change.
  2. Add no-errors and spec refinement proofs for mbox_drop, mbox_concat_chains, and mbox_len.

NB: The changes to arrays.v, clearbufs,v, iter_linked_list.v, mbox.v, rust_lifetimes.v, and string_set.v are all auto-generated. In a future PR, these auto-generated files are going to be added to the .gitignore.

@m-yac m-yac requested review from glguy and eddywestbrook August 9, 2021 22:06
Copy link
Contributor

@eddywestbrook eddywestbrook left a comment

Choose a reason for hiding this comment

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

The Coq proofs look good! The Haskell code is my own, though, so I'm waiting for someone else (possibly @m-yac himself, who authored this PR) to review.

Copy link
Contributor Author

@m-yac m-yac left a comment

Choose a reason for hiding this comment

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

This all looks good to me. I did comment one readability nitpick, but if you don't care I'd be happy to merge this as-is.

Comment on lines 3110 to 3112
mbCombine RL.typeCtxProxies mb_impl1) >>= \trans_pair1 ->
pitmCatching (translatePermImpl prx $
mbCombine RL.typeCtxProxies mb_impl2) >>= \trans_pair2 ->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not use a pattern matching lambda here (i.e. ... >>= \(mtrans1,hasf1) ->) like you do below (lies 3150 and 3152)?

@eddywestbrook eddywestbrook added the subsystem: heapster Issues specifically related to memory verification using Heapster label Aug 9, 2021
@eddywestbrook
Copy link
Contributor

The CI tests all passed before the latest commit, which was just a small tweak and also I tested thoroughly on my local machine, so I am squashing without waiting for the CI. May I be struck down by your angry words if I break the build.

@eddywestbrook eddywestbrook merged commit 3e2578a into master Aug 9, 2021
@mergify mergify bot deleted the heapster-reduce-catchpoints branch August 9, 2021 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subsystem: heapster Issues specifically related to memory verification using Heapster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants