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] Improve widening for changing offsets #1467

Merged
merged 10 commits into from
Oct 14, 2021

Conversation

m-yac
Copy link
Contributor

@m-yac m-yac commented Sep 23, 2021

This PR improves the way widening works in Heapster to be able to handle examples where a pointer into a block of memory is modified (e.g. incremented) over the course of a loop. For example:

uint64_t sum(const uint8_t *arr, size_t len) {
  uint64_t sum = 0;
  while (len--) {
    sum += arr[0];
    arr += 1;
  }
  return sum;
}

This is in service of being able to handle this sha512 example which this PR adds to the heapster-saw/examples directory – though currently, only the function above (which mirrors the loop structure of sha512_block_data_order) has been verified.

@m-yac m-yac requested a review from eddywestbrook September 23, 2021 21:24
@eddywestbrook
Copy link
Contributor

eddywestbrook commented Oct 4, 2021

The coq proof for this seems to be stuck in an infinite loop. The CI says that it blew the stack.

@m-yac
Copy link
Contributor Author

m-yac commented Oct 12, 2021

@eddywestbrook looks like the heapster-saw CI passes now. It was Coq type-checking that got stuck in an infinite loop 🤦‍♂️

@m-yac m-yac changed the title [Heapster] Improve widening for changing offsets, add WIP sha512 example [Heapster] Improve widening for changing offsets Oct 12, 2021
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.

Overall this looks great, good job! Please address the one change I suggested.


-- FIXME: add cases to prove struct(es1)=struct(es2)

-- Otherwise give up!
_ -> proveEqFail e mb_e

where -- If there is exactly one 'BVFactor' in a list of 'BVFactor's which is
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put these two helper functions into Permissions.hs and make them top-level functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I initially added them to the bitvector section but they depend on the definition of PartialSubst, so I added them in a new section below that definition.

-- Prove e = L_1*y_1 + ... + L_k*y_k + N*z + M where z is an unset variable,
-- each y_i is either a set variable with value e_i or an unbound variable
-- with e_i = y_i, and e - (L_1*e_1 + ... + L_k*e_k + M) is divisible by N,
-- by setting z = (e - (L_1*e_1 + ... + L_k*e_k + M))/N
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, I like this. At first I thought there were multiple potential solutions for z, which would be problematic if we chose the wrong one, but you are right, this is a most general solution

@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 labels Oct 14, 2021
@mergify mergify bot merged commit 610cbe3 into master Oct 14, 2021
@mergify mergify bot deleted the heapster/widening-offsets branch October 14, 2021 17:37
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants