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

crux-mir: Support nightly-2023-01-23 #1096

Merged
merged 114 commits into from
Jul 14, 2023

Conversation

RyanGlScott
Copy link
Contributor

@RyanGlScott RyanGlScott commented Jul 5, 2023

This is a fairly large overhaul of crux-mir's and crucible-mir's internals to support a more recent Rust nightly (nightly-2023-01-23). The work is split between myself, @m10f, and @spernsteiner.

There is a lot happening here, so if you want to learn specifics, I'd encourage you to look at individual commits. Some of the broad themes in the patchset include:

  • Propertly support for Rust's new constant forms
  • Better support for zero-sized constants
  • Encoding enum discriminant types so that crucible-mir can know about non-isize discriminant types (e.g., Ordering, which uses an i8 discriminant)
  • Being much more systematic about which patches we must apply to the Rust standard libraries for Crucible's needs (see the new crux-mir/lib/Patches.md file for explanations of each patch)
  • A more intelligent way of computing crate disambiguators for looking up known types such as MaybeUninit and Option
  • Re-enabling the macOS CI for crux-mir, which was previously disabled due to the nightly not working on recent macOSes

This should fix several issues:

m10f and others added 30 commits July 3, 2023 15:08
- updated libs
- changed crux_mir to crux::mir in tests
- new lib build script (build.sh)
- changes to crux-mir to support changes to rustc (nightly-2023-01-22)
This is in preparation for permitting various translation functions to be
called from `transStatics`.
If a static is initialized with a constant value, we now detect and handle
this in `translateStatics`.
Adapted from the original patch in 0374310
Previously, only constant arrays of bytes were translatable. This patch lifts
that restriction so that constant arrays of arbitrary types can be translated,
which is necessary to support some constant arrays used in a newer Rust
nightly.
This took us a while to debug before we realized what was going on. Let's make
the failure obvious.
RyanGlScott added a commit to GaloisInc/saw-script that referenced this pull request Jul 7, 2023
This patch bumps the `crucible` submodule to bring in the changes from
GaloisInc/crucible#1096 and updates the code in `crucible-mir-comp` and
`crux-mir-comp` accordingly. Some highlights:

* All of the `crux-mir-comp` test cases now use `crux::test` instead of
  `crux_test`.
* All of the `crux-mir-comp` test cases now scrub out the values of crate hash
  disambiguators to make their output more stable.
* The overrides in `crux-mir-comp` no longer depend on the specific
  disambiguator values being used, which makes them work with the sometimes
  unpredictable hash values used for crate disambiguators.
* I have added a `tyToShape` case for `TyStr` to handle new kinds of static
  values that arise in the new Rust nightly (mostly coming from constant values
  in the `fmt` crate). The code for this case is nearly identical to that in the
  `TySlice` case.
* There are now static values of type `TyFnPtr` in the new Rust nightly (mostly
  coming from constant values in the `fmt` crate), so in order to handle them
  in `clobberGlobals`, I needed to add a `FnPtrShape` data constructor to
  `TypeShape`. This is mostly a quick hack, since I implemented all other
  functionality for `FnPtrShape` with calls to `error`. Nevertheless, that is
  enough to make all of the tests pass. We can always fill out the calls to
  `error` later if need be.
Copy link
Contributor

@spernsteiner spernsteiner left a comment

Choose a reason for hiding this comment

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

Mostly looks good, aside from a possible correctness issue with the handling of StatementKind::Intrinsic.

crucible-mir/src/Mir/DefId.hs Outdated Show resolved Hide resolved
crucible-mir/src/Mir/Trans.hs Outdated Show resolved Hide resolved
crucible-mir/src/Mir/Trans.hs Outdated Show resolved Hide resolved
crucible-mir/src/Mir/Trans.hs Outdated Show resolved Hide resolved
crucible-mir/src/Mir/Trans.hs Outdated Show resolved Hide resolved
This require a fair bit of refactoring to avoid problematic uses of `textId`
that were relying on `defaultDisambiguator`.
Because `Box<T>` no longer uses the same `TypeRep` as `mut *T`, it is not sound
to translate `ShallowInitBox` as though it were an identity function.
Thankfully, nothing in the test suite actually uses `ShallowInitBox`, so we
simply leave it unimplemented for now.
There's not much of a point in checking for the `crux` configuration attribute,
since one wouldn't be able to compile these tests with bare `rustc` anyway due
to their use of the `crucible` crate.
RyanGlScott added a commit to GaloisInc/saw-script that referenced this pull request Jul 13, 2023
This patch bumps the `crucible` submodule to bring in the changes from
GaloisInc/crucible#1096 and updates the code in `crucible-mir-comp` and
`crux-mir-comp` accordingly. Some highlights:

* All of the `crux-mir-comp` test cases now use `crux::test` instead of
  `crux_test`.
* All of the `crux-mir-comp` test cases now scrub out the values of crate hash
  disambiguators to make their output more stable.
* The overrides in `crux-mir-comp` no longer depend on the specific
  disambiguator values being used, which makes them work with the sometimes
  unpredictable hash values used for crate disambiguators.
* I have added a `tyToShape` case for `TyStr` to handle new kinds of static
  values that arise in the new Rust nightly (mostly coming from constant values
  in the `fmt` crate). The code for this case is nearly identical to that in the
  `TySlice` case.
* There are now static values of type `TyFnPtr` in the new Rust nightly (mostly
  coming from constant values in the `fmt` crate), so in order to handle them
  in `clobberGlobals`, I needed to add a `FnPtrShape` data constructor to
  `TypeShape`. This is mostly a quick hack, since I implemented all other
  functionality for `FnPtrShape` with calls to `error`. Nevertheless, that is
  enough to make all of the tests pass. We can always fill out the calls to
  `error` later if need be.
@RyanGlScott RyanGlScott marked this pull request as ready for review July 14, 2023 18:11
@RyanGlScott RyanGlScott merged commit b4546ce into master Jul 14, 2023
@RyanGlScott RyanGlScott deleted the crux-mir-nightly-2023-01-23-merge branch July 14, 2023 18:11
@RyanGlScott
Copy link
Contributor Author

It turns out that this broke the nightly crux-mir Docker job, as seen here. Thankfully, I was notified very quickly about this! I've submitted a fix in #1099.

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.

3 participants