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

crucible-mir: Properly parse ArrayToPointer casts #1225

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

RyanGlScott
Copy link
Contributor

@RyanGlScott RyanGlScott commented Jul 18, 2024

Although crucible-mir had support for translating these sorts of casts (ArrayToPointer casts), it did not parse them properly. This patch ensures that ArrayToPointer casts are parsed as Misc, which allows them to be handled by the code in Mir.Trans.

Fixes #1224.

Currently, `crucible-mir`'s implementation of `CastKind` lags behind the
`rustc` API's `CastKind` enum. Let's leave a link to the relevant issue (#1223)
as breadcrumbs.
@RyanGlScott RyanGlScott added crucible MIR Issues relating to Rust/MIR support labels Jul 18, 2024
@RyanGlScott
Copy link
Contributor Author

After my changes, the maybe_uninit_array_cast test case now fails:

      mem
        maybe_uninit_array_cast: FAIL (1.39s)
          Compiling and running oracle program                           (0.09s)
          Oracle output: [1, 2]                                          (1.30s)
          Crux output: 
          failures:
          
          ---- maybe_uninit_array_cast/984deb76::crux_test[0] counterexamples ----
          [Crux] Found counterexample for verification goal
          [Crux]   test/conc_eval/mem/maybe_uninit_array_cast.rs:7:19: 7:45: error: in maybe_uninit_array_cast/984deb76::crux_test[0]
          [Crux]   Translation error in maybe_uninit_array_cast/984deb76::crux_test[0]: unimplemented cast: Misc
          [Crux]     ty: TyRawPtr (TyArray (TyInt B32) 2) Mut
          [Crux]     as: TyRawPtr (TyInt B32) Mut
          
          [Crux] Overall status: Invalid.
          
            test/Test.hs:123:
            crux doesn't match oracle

Fair enough, since I changed the relevant code which casts from an array reference (or, in this case, an array pointer) to a pointer to look for ArrayToPointer instead of PtrToPtr, the latter of which is lumped under the Misc umbrella. I could add a dedicated PtrToPtr constructor, but the problem is that there are many different evalCast' cases that could now equally apply to both Misc and PtrToPtr. It would be nice to clean up the code to make this sort of thing easier to express (#1223), but I am inclined not to that just now, since this is intended to be a simple bugfix.

As such, I think I will do the simplest possible thing and map ArrayToPointer to Misc. This is a bit ugly, but it avoids the possibility of introducing regressions like the one seen in the maybe_uninit_array_cast test case. I'll defer the larger task of getting rid of Misc to #1223.

Although `crucible-mir` had support for translating these sorts of casts
(`ArrayToPointer` casts), it did not parse them properly. This patch ensures
that `ArrayToPointer` casts are parsed as `Misc`, which allows them to be
handled by the code in `Mir.Trans`.

Fixes #1224.
@RyanGlScott RyanGlScott force-pushed the T1224-ArrayToPointer-CastKind branch from b07ad45 to edddf4b Compare July 18, 2024 19:23
Copy link
Contributor

@mccleeary-galois mccleeary-galois left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable option for a quick fix.

@RyanGlScott RyanGlScott merged commit 8cc5e17 into master Jul 18, 2024
32 checks passed
@RyanGlScott RyanGlScott deleted the T1224-ArrayToPointer-CastKind branch July 18, 2024 20:31
RyanGlScott added a commit to GaloisInc/saw-script that referenced this pull request Jul 18, 2024
This bumps the `crucible` submodule to bring in the changes from
GaloisInc/crucible#1225, which fixes GaloisInc/crucible#1224 on the SAW side.
This also requires bumping the `cryptol`, `macaw`, and `what4` submodules in
order to construct a consistent build plan with the latest `crucible`.
RyanGlScott added a commit to GaloisInc/saw-script that referenced this pull request Jul 19, 2024
This bumps the `crucible` submodule to bring in the changes from
GaloisInc/crucible#1225, which fixes GaloisInc/crucible#1224 on the SAW side.
This also requires bumping the `cryptol`, `macaw`, and `what4` submodules in
order to construct a consistent build plan with the latest `crucible`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crucible MIR Issues relating to Rust/MIR support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crucible-mir fails to parse MIR JSON that casts array reference to pointer
2 participants