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

Update crucible and macaw submodules, changes to x86 initial stack #1110

Merged
merged 7 commits into from
Mar 11, 2021

Conversation

chameco
Copy link
Contributor

@chameco chameco commented Mar 4, 2021

(This PR bundles two somewhat-related changes for the next https://github.com/GaloisInc/blst-verification release, in the interest of expediency. GaloisInc/what4#105 is also relevant in that context, and I plan to bump that submodule here as well if there are no strong objections.)

The changes to x86 verification amount to increasing the alignment of the base of RSP while shrinking the alignment of the initial offset. The actual alignment of the initial RSP remains 16 bytes. This makes it more viable to verify code that performs bitwise operations on pointers derived from RSP (given GaloisInc/macaw#192).

@chameco chameco requested a review from andreistefanescu March 4, 2021 06:10
@@ -235,11 +237,13 @@ llvm_verify_x86 (Some (llvmModule :: LLVMModule x)) path nm globsyms checkSat se
opts <- getOptions
basic_ss <- getBasicSS
sym <- liftIO $ newSAWCoreBackend sc
rw <- getTopLevelRW
cacheTermsSetting <- liftIO $ W4.getOptionSetting W4.B.cacheTerms $ W4.getConfiguration sym
_ <- liftIO $ W4.setOpt cacheTermsSetting $ rwWhat4HashConsing rw
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes enable_what4_hash_consing do the expected thing

@@ -304,7 +308,7 @@ llvm_verify_x86 (Some (llvmModule :: LLVMModule x)) path nm globsyms checkSat se
pure
( C.cfgHandle funcCFG
, st & C.stateContext . C.functionBindings
%~ C.insertHandleMap (C.cfgHandle funcCFG) (C.UseCFG funcCFG $ C.postdomInfo funcCFG)
%~ C.FnBindings . C.insertHandleMap (C.cfgHandle funcCFG) (C.UseCFG funcCFG $ C.postdomInfo funcCFG) . C.fnBindings
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -322,7 +326,7 @@ llvm_verify_x86 (Some (llvmModule :: LLVMModule x)) path nm globsyms checkSat se
Macaw.macawExtensions (Macaw.x86_64MacawEvalFn sfs) mvar
(mkGlobalMap . Map.singleton 0 $ preState ^. x86GlobalBase)
funcLookup noExtraValidityPred
, C._functionBindings = C.insertHandleMap (C.cfgHandle cfg) (C.UseCFG cfg $ C.postdomInfo cfg) C.emptyHandleMap
, C._functionBindings = C.FnBindings $ C.insertHandleMap (C.cfgHandle cfg) (C.UseCFG cfg $ C.postdomInfo cfg) C.emptyHandleMap
Copy link
Contributor Author

Choose a reason for hiding this comment

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

insertHandleMap (cfgHandle cfg) (UseCFG cfg (postdomInfo cfg)) $
emptyHandleMap
, _functionBindings = FnBindings $
insertHandleMap (cfgHandle cfg) (UseCFG cfg (postdomInfo cfg)) emptyHandleMap
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -1082,7 +1082,7 @@ setupGlobals opts gs fs s
let halloc = simHandleAllocator (st ^. stateContext)
h <- mkHandle halloc fname
let addBinding = over (stateContext . functionBindings)
(insertHandleMap h (UseOverride o))
(FnBindings . insertHandleMap h (UseOverride o) . fnBindings)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chameco
Copy link
Contributor Author

chameco commented Mar 10, 2021

I think the problem with the awslc test is as follows:

  • Changes in this PR cause crucible_llvm_verify_x86 to respect enable_what4_hash_consing (during simulation),
  • The AES-GCM proof in awslc uses enable_what4_hash_consing to simplify some goals (during a w4_unint tactic); however,
  • Enabling hash consing during simulation breaks some rewrite rule.

I'm going to add a distinct option for What4 hash consing during simulation and see if that works out. It's unfortunately quite ugly and cumbersome to pile up more options in TopLevelRW along with getters/setters, but I'm not sure if there's a better solution short-term.

Copy link
Contributor

@andreistefanescu andreistefanescu 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, please take a look at the inline comments

src/SAWScript/Crucible/LLVM/X86.hs Outdated Show resolved Hide resolved
src/SAWScript/Interpreter.hs Show resolved Hide resolved
src/SAWScript/Crucible/LLVM/X86.hs Outdated Show resolved Hide resolved
src/SAWScript/Interpreter.hs Show resolved Hide resolved
@chameco chameco added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Mar 11, 2021
@mergify mergify bot merged commit a9e13c2 into master Mar 11, 2021
@mergify mergify bot deleted the x86-stack-align branch March 11, 2021 10:17
@RyanGlScott RyanGlScott added the subsystem: x86 Issues related to verifying x86 binaries via Macaw label Dec 6, 2021
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: x86 Issues related to verifying x86 binaries via Macaw
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants