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

Run some tests in MIRI on CI #6332

Merged
merged 2 commits into from
May 3, 2023
Merged

Commits on May 2, 2023

  1. Run some tests in MIRI on CI

    This commit is an implementation of getting at least chunks of Wasmtime
    to run in MIRI on CI. The full test suite is not possible to run in MIRI
    because MIRI cannot run Cranelift-produced code at runtime (aka it
    doesn't support JITs). Running MIRI, however, is still quite valuable if
    we can manage it because it would have trivially detected
    GHSA-ch89-5g45-qwc7, our most recent security advisory. The goal of this
    PR is to select a subset of the test suite to execute in CI under MIRI
    and boost our confidence in the copious amount of `unsafe` code in
    Wasmtime's runtime.
    
    Under MIRI's default settings, which is to use the [Stacked
    Borrows][stacked] model, much of the code in `Instance` and `VMContext`
    is considered invalid. Under the optional [Tree Borrows][tree] model,
    however, this same code is accepted. After some [extremely helpful
    discussion][discuss] on the Rust Zulip my current conclusion is that
    what we're doing is not fundamentally un-sound but we need to model it
    in a different way. This PR, however, uses the Tree Borrows model for
    MIRI to get something onto CI sooner rather than later, and I hope to
    follow this up with something that passed Stacked Borrows. Additionally
    that'll hopefully make this diff smaller and easier to digest.
    
    Given all that, the end result of this PR is to get 131 separate unit
    tests executing on CI. These unit tests largely exercise the embedding
    API where wasm function compilation is not involved. Some tests compile
    wasm functions but don't run them, but compiling wasm through Cranelift
    in MIRI is so slow that it doesn't seem worth it at this time. This does
    mean that there's a pretty big hole in MIRI's test coverage, but that's
    to be expected as we're a JIT compiler after all.
    
    To get tests working in MIRI this PR uses a number of strategies:
    
    * When platform-specific code is involved there's now `#[cfg(miri)]` for
      MIRI's version. For example there's a custom-built "mmap" just for
      MIRI now. Many of these are simple noops, some are `unimplemented!()`
      as they shouldn't be reached, and some are slightly nontrivial
      implementations such as mmaps and trap handling (for native-to-native
      function calls).
    
    * Many test modules are simply excluded via `#![cfg(not(miri))]` at the
      top of the file. This excludes the entire module's worth of tests from
      MIRI. Other modules have `#[cfg_attr(miri, ignore)]` annotations to
      ignore tests by default on MIRI. The latter form is used in modules
      where some tests work and some don't. This means that all future test
      additions will need to be effectively annotated whether they work in
      MIRI or not. My hope though is that there's enough precedent in the
      test suite of what to do to not cause too much burden.
    
    * A number of locations are fixed with respect to MIRI's analysis. For
      example `ComponentInstance`, the component equivalent of
      `wasmtime_runtime::Instance`, was actually left out from the fix for
      the CVE by accident. MIRI dutifully highlighted the issues here and
      I've fixed them locally. Some locations fixed for MIRI are changed to
      something that looks similar but is subtly different. For example
      retaining items in a `Store<T>` is now done with a Wasmtime-specific
      `StoreBox<T>` type. This is because, with MIRI's analyses, moving a
      `Box<T>` invalidates all pointers derived from this `Box<T>`. We don't
      want these semantics, so we effectively have a custom `Box<T>` to suit
      our needs in this regard.
    
    * Some default configuration is different under MIRI. For example most
      linear memories are dynamic with no guards and no space reserved for
      growth. Settings such as parallel compilation are disabled. These are
      applied to make MIRI "work by default" in more places ideally. Some
      tests which perform N iterations of something perform fewer iterations
      on MIRI to not take quite so long.
    
    This PR is not intended to be a one-and-done-we-never-look-at-it-again
    kind of thing. Instead this is intended to lay the groundwork to
    continuously run MIRI in CI to catch any soundness issues. This feels,
    to me, overdue given the amount of `unsafe` code inside of Wasmtime. My
    hope is that over time we can figure out how to run Wasm in MIRI but
    that may take quite some time. Regardless this will be adding nontrivial
    maintenance work to contributors to Wasmtime. MIRI will be run on CI for
    merges, MIRI will have test failures when everything else passes,
    MIRI's errors will need to be deciphered by those who have probably
    never run MIRI before, things like that. Despite all this to me it seems
    worth the cost at this time. Just getting this running caught two
    possible soundness bugs in the component implementation that could have
    had a real-world impact in the future!
    
    [stacked]: https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md
    [tree]: https://perso.crans.org/vanille/treebor/
    [discuss]: https://rust-lang.zulipchat.com/#narrow/stream/269128-miri/topic/Tree.20vs.20Stacked.20Borrows.20.26.20a.20debugging.20question
    alexcrichton committed May 2, 2023
    Configuration menu
    Copy the full SHA
    7c6bd2e View commit details
    Browse the repository at this point in the history

Commits on May 3, 2023

  1. Configuration menu
    Copy the full SHA
    1c13b19 View commit details
    Browse the repository at this point in the history