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

Support memory invalidation in LLVM memory model #312

Merged
merged 5 commits into from
Oct 21, 2019

Conversation

chameco
Copy link
Contributor

@chameco chameco commented Sep 27, 2019

Adds a new WriteSource called MemInvalidate. Reading from invalidated memory yields an error with a user-specified message.

This facilitates a saw-script change that should fix the compositional verification soundness bug described in GaloisInc/saw-script#30.

Copy link
Contributor

@atomb atomb left a comment

Choose a reason for hiding this comment

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

I haven't carefully examined the logic of readMemInvalidate, but I think these changes make sense. It's a simple addition that significantly improves our ability to do sound verification.

@langston-barrett
Copy link
Contributor

This breaks macaw-symbolic, so can't yet be tested against S2N/SAWscript on Fryingpan: http://fryingpan.dev.galois.com/hydra/jobset/s2n/PR-explicit-invalid-write.submodules-ghc865#tabs-jobs

If you create a branch in macaw-symbolic with the same name ("explicit-invalid-write") and update it, the tests will automatically pick up on the changes you make there and build SAW+S2N against both those updates together.

Copy link
Contributor

@robdockins robdockins left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me, except that I think we'll need to expose more memory initialization options. We need to be able to allocate the globals that are mentioned in SAW function preconditions, in addition to "all" globals and just the constant ones.

So, we should probably rename initializeMemory' to just initializeMemory, and rename initializeMemory to initializeAllMemory or something, similar to how we have populateGobals, populateAllGlobals and populateConstantGlobals.

@atomb
Copy link
Contributor

atomb commented Oct 18, 2019

With the naming changes, the crux-llvm build is failing. It should be easy to fix, though: all the references initializeMemory in CruxLLVMMain should become initializeAllMemory. I'd make the change myself, except that the branch is in @chameco's fork of the repo.

Once that's fixed, are there any remaining objections to merging this?

@atomb atomb merged commit ab43acd into GaloisInc:master Oct 21, 2019
robdockins pushed a commit that referenced this pull request Oct 24, 2019
* Support memory invalidation in LLVM memory model
* Support not allocating all globals when initializing LLVM memory
* Change crux-llvm to reflect initializeMemory name change
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.

4 participants