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 memblock prover improvements #1406

Merged
merged 11 commits into from
Aug 6, 2021

Conversation

eddywestbrook
Copy link
Contributor

Added a number of improvements to how the implication prover handles memblock permissions. Specifically:

  • Changed implElimLLVMBlock to only perform one step of elimination for memblock permissions

  • Added more comments to implElimLLVMBlock

  • Added implElimAppendIthLLVMBlock, which calls implElimLLVMBlock and then appends its results to the conjunctive permission on top of the stack

  • Added implIntroOrShapeMultiM to allow multiple nested or shapes to be all proved at once

  • Changed proveVarLLVMBlocks to use the new implElimAppendIthLLVMBlock when it eliminates memblock permissions to allow it to call itself again rather than calling implElimPopIthLLVMBlock and then going all the way to the top of proveVarImpl

  • Added the notion of a tagged union shape, which is a multi-way nested or shape of the form sh1 orsh ... orsh shn where each shi starts with an equality permission to a concrete bitvector value that represents a tag for that disjunct. Also added implication prover for recognizing when we are trying to prove a tagged union shape and already know which tag to choose, in which case we avoid doing the search across all n disjuncts and instead choose the right one from the beginning.

  • Added debug output for type-checking return statements

Eddy Westbrook added 8 commits August 2, 2021 17:40
…on before eliminating it and would then go right back to trying to eliminate that same permission again...
…lock permission; changed proveVarLLVMBlocks to focus first on eliminating memblock permissions on the left that overlap with but are not contained in memblock permissions on the right
…s a tagged union type where we already know from something on the left what the tag is, in which case we can avoid searching for proofs of all the disjuncts other than the one that matches that known tag
…ence shape sh;emptysh with the empty shape when necessary
heapster-saw/src/Verifier/SAW/Heapster/Implication.hs Outdated Show resolved Hide resolved
@@ -5423,7 +5572,7 @@ proveVarLLVMBlocks' x ps psubst mb_bps_in mb_ps = case mbMatch mb_bps_in of
-- permission on the left with this exact offset, length, and shape, because
-- it would have matched some previous case, so try to eliminate a memblock
-- and recurse
[nuMP| mb_bp : mb_bps |]
[nuMP| mb_bp : _ |]
| [nuMP| PExpr_NamedShape _ _ nmsh _ |] <- mbMatch $ fmap llvmBlockShape mb_bp
Copy link
Member

Choose a reason for hiding this comment

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

It's not a new change, but while we're looking at this function, it would be nice if we weren't duplicating mbMatch $ fmap llvmBlockShape mb_bp so much. Each of these could be redoing the work of converting to pair representation.

This whole function could potentially benefit from doing a single [nuMP| mb_bp : mb_bps |] pattern and handling the cases under that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I started to look at how to implement this change, and it led to a fair bit of reorganizing proveVarLLVMBlocks. The main changes are that it is now organized into two separate stages, represented by two separate functions, where the second stage / function takes a MatchedMb for the shape as an explicit argument.

Another change is that these functions now take lists of multi-bindings instead of multi-bindings of lists. The justification is that those lists are always going to be converted to pair form and matched against anyway, so we might as well do that to begin with. Plus, there are a number of recursive calls that cons or prepend to those lists, and before they had to do this with fmap, which converts back to function form and then again to pair form, while now this conversion has been removed. Because one of the main concerns with this change is performance, I did in fact time running Heapster on the rust_data example before and after the change, and it seems to improve performance slightly, on the order of .1 - .2 seconds out of 17 seconds for the entire test. So I think that's a win?

heapster-saw/src/Verifier/SAW/Heapster/Permissions.hs Outdated Show resolved Hide resolved
heapster-saw/src/Verifier/SAW/Heapster/Permissions.hs Outdated Show resolved Hide resolved
@eddywestbrook eddywestbrook force-pushed the heapster-memblock-prover-improvements branch from da20fb7 to d03220b Compare August 6, 2021 00:46
@eddywestbrook eddywestbrook added the Heapster Issues specifically related to the Heapster sub-system label Aug 6, 2021
Eddy Westbrook added 3 commits August 6, 2021 09:04
@eddywestbrook eddywestbrook merged commit d031459 into master Aug 6, 2021
@mergify mergify bot deleted the heapster-memblock-prover-improvements branch August 6, 2021 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Heapster Issues specifically related to the Heapster sub-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants