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 ghost values in all language backends #1958

Merged
merged 3 commits into from
Nov 28, 2023
Merged

Support ghost values in all language backends #1958

merged 3 commits into from
Nov 28, 2023

Conversation

RyanGlScott
Copy link
Contributor

@RyanGlScott RyanGlScott commented Oct 13, 2023

This patch:

  • Factors out the machinery needed to track ghost state into the SAWScript.Crucible.Common.Override and SAWScript.Builtins modules. Nothing about ghost state is specific to any language backend, so it deserves to live in a non-LLVM–specific location.
  • Adds jvm_ghost_value and mir_ghost_value commands, which behave exactly like the LLVM backend's llvm_ghost_value command does, but for the JVM and MIR backends, respectively.
  • Adds a test_mir_ghost test case in SAWScript and the remote API to ensure that everything works as expected.

Fixes #1929.

@RyanGlScott RyanGlScott force-pushed the T1929 branch 3 times, most recently from c6ca304 to 4232a42 Compare October 16, 2023 23:00
RyanGlScott added a commit that referenced this pull request Nov 2, 2023
This also adds some very rudimentary test cases. A more impressive test case
would involve ghost state, but we will need #1958 before we can write such a
test case.

Fixes #1970.
RyanGlScott added a commit that referenced this pull request Nov 5, 2023
This also adds some very rudimentary test cases. A more impressive test case
would involve ghost state, but we will need #1958 before we can write such a
test case.

Fixes #1970.
@RyanGlScott RyanGlScott changed the title Draft: Support ghost values in all language backends Support ghost values in all language backends Nov 9, 2023
@RyanGlScott RyanGlScott marked this pull request as ready for review November 9, 2023 17:37
Comment on lines -3247 to +3556
* `llvm_declare_ghost_state : String -> TopLevel Ghost`
* `declare_ghost_state : String -> TopLevel Ghost`

Ghost state variables do not initially have any particluar type, and can
store data of any type. Given an existing ghost variable the following
function can be used to specify its value:
functions can be used to specify its value:

* `llvm_ghost_value : Ghost -> Term -> LLVMSetup ()`
* `jvm_ghost_value : Ghost -> Term -> JVMSetup ()`
* `mir_ghost_value : Ghost -> Term -> MIRSetup ()`

Currently, this function can only be used for LLVM verification, though
that will likely be generalized in the future. It can be used in either
the pre state or the post state, to specify the value of ghost state
either before or after the execution of the function, respectively.
These can be used in either the pre state or the post state, to specify the
value of ghost state either before or after the execution of the function,
respectively.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documentation-related changes involving ghost state.

@@ -0,0 +1,28 @@
enable_experimental;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A test case involving ghost state in the SAW MIR backend.

RyanGlScott added a commit that referenced this pull request Nov 9, 2023
This also adds some very rudimentary test cases. A more impressive test case
would involve ghost state, but we will need #1958 before we can write such a
test case.

Fixes #1970.
RyanGlScott added a commit that referenced this pull request Nov 9, 2023
This also adds some very rudimentary test cases. A more impressive test case
would involve ghost state, but we will need #1958 before we can write such a
test case.

Fixes #1970.
@RyanGlScott
Copy link
Contributor Author

@qsctr discovered that SAW will panic when it attempts to combine MIR ghost state with symbolic branching. Here is a small example which triggers the issue:

// Meant to be overridden
pub fn f(_x: u32) {}

pub fn g(b: bool) {
    if b {
        f(27)
    } else {
        f(42)
    }
}
enable_experimental;

m <- mir_load_module "test.linked-mir.json";

g <- declare_ghost_state "g";

let f_spec = do {
    x <- mir_fresh_var "x" mir_u32;

    mir_execute_func [mir_term x];

    mir_ghost_value g x;
};

let g_spec = do {
    b <- mir_fresh_var "b" mir_bool;

    mir_execute_func [mir_term b];

    mir_ghost_value g ({{ if b then 27 else 42 : [32] }});
};

f_ov <- mir_unsafe_assume_spec m "test::f" f_spec;
mir_verify m "test::g" [f_ov] false g_spec z3;
$ ./bin/saw test.saw



[13:41:31.524] Loading file "/home/ryanscott/Documents/Hacking/Haskell/saw-script/test.saw"
[13:41:31.552] Verifying test/40fac751::g[0] ...
[13:41:31.561] Simulating test/40fac751::g[0] ...
[13:41:31.561] Matching 1 overrides of  test/40fac751::f[0] ...
[13:41:31.561] Branching on 1 override variants of test/40fac751::f[0] ...
[13:41:31.561] Applied override! test/40fac751::f[0]
[13:41:31.562] Matching 1 overrides of  test/40fac751::f[0] ...
[13:41:31.562] Branching on 1 override variants of test/40fac751::f[0] ...
[13:41:31.562] Applied override! test/40fac751::f[0]
[13:41:31.562] You have encountered a bug in Crucible's implementation.
*** Please create an issue at https://github.com/GaloisInc/crucible/issues

%< --------------------------------------------------- 
  Revision:  c19fc2c53851cdd162ae1ccca14032467e3c2fd1
  Branch:    HEAD
  Location:  RegMap.muxRegForType
  Message:   Unknown intrinsic type:
             *** Name: GhostValue
CallStack (from HasCallStack):
  panic, called at src/Lang/Crucible/Panic.hs:11:9 in crucible-0.6-inplace:Lang.Crucible.Panic
  panic, called at src/Lang/Crucible/Simulator/RegMap.hs:252:12 in crucible-0.6-inplace:Lang.Crucible.Simulator.RegMap
%< ---------------------------------------------------

The reason that this does not happen in the LLVM backend is because we add the GhostValue intrinsic on top of the default LLVM intrinsics here:

intrinsics :: MapF.MapF Crucible.SymbolRepr (Crucible.IntrinsicMuxFn Sym)
intrinsics =
MapF.insert
(Crucible.knownSymbol :: Crucible.SymbolRepr MS.GhostValue)
Crucible.IntrinsicMuxFn
CL.llvmIntrinsicTypes

We need to do something similar for the JVM and MIR backends.

@RyanGlScott
Copy link
Contributor Author

The issue in #1958 (comment) is fixed as of 276a54b.

RyanGlScott added a commit that referenced this pull request Nov 22, 2023
This also adds some very rudimentary test cases. A more impressive test case
would involve ghost state, but we will need #1958 before we can write such a
test case.

Fixes #1970.
We want to be able to declare ghost state in all SAW language backends, so we
now use a more general `declare_ghost_state` name for the command to reflect
this. The old `llvm_declare_ghost_state` command is now a legacy command whose
use is discouraged in favor of `declare_ghost_state`.

This is one part of an eventual fix for #1929.
This patch:

* Factors out the machinery needed to track ghost state into
  the `SAWScript.Crucible.Common.Override` and `SAWScript.Builtins` modules.
  Nothing about ghost state is specific to any language backend, so it deserves
  to live in a non-LLVM–specific location.
* Adds `jvm_ghost_value` and `mir_ghost_value` commands, which behave exactly
  like the LLVM backend's `llvm_ghost_value` command does, but for the JVM and
  MIR backends, respectively.
* Adds a `test_mir_ghost` test case in SAWScript and the remote API to ensure
  that everything works as expected.

Fixes #1929.
This is not entirely straightforward to do for the JVM backend, as the
`jvmSimContext` function (`crucible-jvm`'s API function for constructing a
`SimContext`) does not support adding additional intrinsic types, including the
intrinsic type needed to support SAW ghost state. For the time being, I have
simply inlined the implementation of `jvmSimContext` as needed, and I have
opened GaloisInc/crucible#1128 as a reminder to
generalize `jvmSimContext` upstream.
Copy link
Contributor

@qsctr qsctr 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, thanks. It's a bit unfortunate that we have to duplicate some more code across the 3 backends, hopefully one day that can be addressed with a solution to #379.

@RyanGlScott RyanGlScott 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 Nov 28, 2023
@mergify mergify bot merged commit 6ff9711 into master Nov 28, 2023
40 checks passed
@mergify mergify bot deleted the T1929 branch November 28, 2023 10:17
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement ghost state in all language backends
2 participants