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

Fancy assumptions #769

Merged
merged 26 commits into from
Jun 29, 2021
Merged

Fancy assumptions #769

merged 26 commits into from
Jun 29, 2021

Conversation

robdockins
Copy link
Contributor

Refactor various aspects of assumption handling. The main aim here is to allow "events" to be tracked alongside the logical assumptions that are already tracked. This will allow us to concretely reconstruct a linear sequence of events when counterexamples to proof goals are found so that we can replay the creation of symbolic variables and identify sequences of program points the run passed through.

This abstracts more fully over the types of assumptions and
assertions and delays the specialization to @LabeledPred@.
It removes some intermedidate type aliaes that seemed mostly
to be confusing.  This sets the stage for modifying the
representation of assumptions and assertions.
Instead of a flat sequence of assumptions, we now have a
tree structure based on mux values.
This allows us to track program events alongside the existing
assumption tracking. Using this, we can reconstruct a linear sequences
of events that occured along the program run when we find
counterexamples to proof goals.

Initially, the only event we track is the creation of user-requested
symbolic variables, but others are planned.  By tracking symbolic
variables this way, we can accurately reconstruct a linear sequence
of values to replay, which allows us to fix #587.
These are no longer necessary.
In crux-llvm, we need to make sure that declarations are emitted for each of
the types so the counterexample code will compile.  Previously,
this was handed by prepopulating the Model map, but this is fragile
and mixes crux-llvm concerns into crux proper.  Here, we split this
concern out, and push the handling of this issue into crux-llvm,
where it belongs.
It is better to use the `ProcessedGoals` record instead.
It will be unnecessary when we properly implement
event-based program location tracking.
Use this feature in crux to record a detailed sequence of program
points that can later be used to reconstruct a concrete program run.
@robdockins robdockins requested review from atomb and RyanGlScott June 27, 2021 21:24
@robdockins robdockins marked this pull request as ready for review June 27, 2021 21:26
Copy link
Contributor

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

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

Ooh, exciting! Does this fix any bugs on its own, or is this a preparatory refactoring for future efforts? I see quite a bit of refactoring around data types in Crux.Types (e.g., NotProvedGoal now has a list of CrucibleAssumptions)—is there a way to test out this new code by using crux itself?

I've done a quick pass over the code, mostly commenting on typos and minor stylistic nits. If there is a part of the patch that you feel deserves closer scrutiny, let me know.

crucible-concurrency/crucibles/Main.hs Outdated Show resolved Hide resolved
crucible-concurrency/src/Crucibles/Explore.hs Outdated Show resolved Hide resolved
crucible-llvm/src/Lang/Crucible/LLVM/Translation/Monad.hs Outdated Show resolved Hide resolved
crucible/src/Lang/Crucible/Backend/AssumptionStack.hs Outdated Show resolved Hide resolved
crucible/src/Lang/Crucible/Backend/AssumptionStack.hs Outdated Show resolved Hide resolved
crucible/src/Lang/Crucible/Backend.hs Show resolved Hide resolved
crucible/src/Lang/Crucible/Backend/AssumptionStack.hs Outdated Show resolved Hide resolved
crux/src/Crux/Goal.hs Outdated Show resolved Hide resolved
crux/src/Crux/Goal.hs Show resolved Hide resolved
crucible/src/Lang/Crucible/Backend.hs Show resolved Hide resolved
@robdockins
Copy link
Contributor Author

@RyanGlScott This PR fixes #587, which is related to incorrect counterexample generation, and is a crux-specific bug. I intend to add a test case, but we'll need to add something to the test harness to be able to observe the concrete model generated, which is why I haven't done it yet.

But more generally, I think this should help a lot with the counterexample witness generation you've been looking into.

@RyanGlScott
Copy link
Contributor

I intend to add a test case, but we'll need to add something to the test harness to be able to observe the concrete model generated, which is why I haven't done it yet.

I see. In that case, I'd be fine with just opening another issue to serve as a reminder to augment the test suite later.

But more generally, I think this should help a lot with the counterexample witness generation you've been looking into.

Indeed, I'm currently looking into trying to debug-print these CrucibleAssumptions to see if it will work for my needs. (I tried rebasing #760 on top of this PR, but alas, giving the new version of ProvedGoals a derived Show instance proves non-trivial.)

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.

2 participants