GaloisInc / saw-script

The SAW scripting language.
BSD 3-Clause "New" or "Revised" License
442 stars 63 forks source link

MIR `bool` arrays are cumbersome to use in SAW #2147

Open RyanGlScott opened 1 week ago

RyanGlScott commented 1 week ago

Consider the following Rust function, which uses an array of bools:

// test.rs
pub fn bit_array(a : [bool; 8]) -> [bool; 8] {
    return a;
}

You would expect that SAW would be able to verify f against the following specification:

// test.saw
enable_experimental;

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

let spec = do {
    a <- mir_fresh_var "a" (mir_array 8 mir_bool);

    mir_execute_func [mir_term a];

    mir_return (mir_term a);
};

mir_verify m "test::bit_array" [] false spec z3;

Surprisingly, however, it can't:

$ ~/Software/saw-1.2/bin/saw test.saw
[14:32:11.105] Loading file "/home/ryanscott/Documents/Hacking/Haskell/saw-script/test.saw"
[14:32:11.123] Stack trace:
"mir_verify" (/home/ryanscott/Documents/Hacking/Haskell/saw-script/test.saw:14:1-14:11)
"spec" (/home/ryanscott/Documents/Hacking/Haskell/saw-script/test.saw:14:41-14:45)
mir_execute_func: Argument type mismatch
Argument position: 0
Expected type: [bool; 8]
Given type:    u8

The reason this happens is rather unfortunate:

The key step here is in picking a Crucible type for [8]. This choice is somewhat arbitrary, as we could just as well pick BaseBVRepr (knownNat @8) or MirVectorRepr BoolRepr. Picking the former is usually the more sensible option, as it happens to correspond to the same Crucible type that is assigned to types such as u8 (see here), and these integer types are far more common that bool arrays. Unfortunately, this choice comes at the expense of making it impossible to write SAW specifications like the one above.

A handful of scattered observations:

Ultimately, I suspect that fixing this issue will require us to make SAW more flexible about how we embed types into Cryptol, as proposed in https://github.com/GaloisInc/saw-script/issues/1976. This is because the types u8 and [bool; 8] have the same representation in Cryptol, but different Crucible representations. Until then, the mir_fresh_expanded_value trick mentioned above is a serviceable workaround.

Lastly, one radical idea: could we fix this issue by changing the Crucible representation of bool from BoolRepr to BaseBVRepr (knownNat @1), similarly to how LLVM's i1 type is handled? Perhaps so, but this would require changing a fair bit of code in crucible-mir to accommodate this, and it's unclear to me whether that is a good idea. In any case, we want to fix https://github.com/GaloisInc/saw-script/issues/1976 anyway, so it's likely best to pursue a solution there.

RyanGlScott commented 1 week ago

@Isweet suggests that we could make this work by treating BaseBVRepr and MirVectorRepr BoolRepr as belonging to the same equivalence class, and when we try to equate one with the other, performing explicit conversions where necessary. I am somewhat on the fence about needing to sprinkle conversions in various places, since this feels like we are sidestepping a design issue to some extent, but that might be one way to unblock this.

Isweet commented 1 week ago

@RyanGlScott, I suspect your intuition about how to work around this issue is better, so I'd recommend going with your gut rather than mine.

I'd only recommend documenting the purpose of each Crucible type (with some explanation of why they must be different).

sauclovian-g commented 1 week ago

We ultimately need #1976 anyway, I think.