You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.

53 lines
1.3 KiB

Run some tests in MIRI on CI (#6332) * 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 * Update alignment comment
2 years ago
#![cfg(not(miri))]
use wasmtime::*;
#[test]
fn same_import_names_still_distinct() -> anyhow::Result<()> {
const WAT: &str = r#"
(module
(import "" "" (func $a (result i32)))
(import "" "" (func $b (result f32)))
(func (export "foo") (result i32)
call $a
call $b
i32.trunc_f32_u
i32.add)
)
"#;
let mut store = Store::<()>::default();
let module = Module::new(store.engine(), WAT)?;
let imports = [
Remove `HostRef` from the `wasmtime` public API (#788) * Remove `HostRef` from the `wasmtime` public API This commit removes all remaining usages of `HostRef` in the public API of the `wasmtime` crate. This involved a number of API decisions such as: * None of `Func`, `Global`, `Table`, or `Memory` are wrapped in `HostRef` * All of `Func`, `Global`, `Table`, and `Memory` implement `Clone` now. * Methods called `type` are renamed to `ty` to avoid typing `r#type`. * Methods requiring mutability for external items now no longer require mutability. The mutable reference here is sort of a lie anyway since the internals are aliased by the underlying module anyway. This affects: * `Table::set` * `Table::grow` * `Memory::grow` * `Instance::set_signal_handler` * The `Val::FuncRef` type is now no longer automatically coerced to `AnyRef`. This is technically a breaking change which is pretty bad, but I&#39;m hoping that we can live with this interim state while we sort out the `AnyRef` story in general. * The implementation of the C API was refactored and updated in a few locations to account for these changes: * Accessing the exports of an instance are now cached to ensure we always hand out the same `HostRef` values. * `wasm_*_t` for external values no longer have internal cache, instead they all wrap `wasm_external_t` and have an unchecked accessor for the underlying variant (since the type is proof that it&#39;s there). This makes casting back and forth much more trivial. This is all related to #708 and while there&#39;s still more work to be done in terms of documentation, this is the major bulk of the rest of the implementation work on #708 I believe. * More API updates * Run rustfmt * Fix a doc test * More test updates
5 years ago
Func::new(
&mut store,
FuncType::new(None, Some(ValType::I32)),
|_, params, results| {
assert!(params.is_empty());
assert_eq!(results.len(), 1);
results[0] = 1i32.into();
Ok(())
},
Remove `HostRef` from the `wasmtime` public API (#788) * Remove `HostRef` from the `wasmtime` public API This commit removes all remaining usages of `HostRef` in the public API of the `wasmtime` crate. This involved a number of API decisions such as: * None of `Func`, `Global`, `Table`, or `Memory` are wrapped in `HostRef` * All of `Func`, `Global`, `Table`, and `Memory` implement `Clone` now. * Methods called `type` are renamed to `ty` to avoid typing `r#type`. * Methods requiring mutability for external items now no longer require mutability. The mutable reference here is sort of a lie anyway since the internals are aliased by the underlying module anyway. This affects: * `Table::set` * `Table::grow` * `Memory::grow` * `Instance::set_signal_handler` * The `Val::FuncRef` type is now no longer automatically coerced to `AnyRef`. This is technically a breaking change which is pretty bad, but I&#39;m hoping that we can live with this interim state while we sort out the `AnyRef` story in general. * The implementation of the C API was refactored and updated in a few locations to account for these changes: * Accessing the exports of an instance are now cached to ensure we always hand out the same `HostRef` values. * `wasm_*_t` for external values no longer have internal cache, instead they all wrap `wasm_external_t` and have an unchecked accessor for the underlying variant (since the type is proof that it&#39;s there). This makes casting back and forth much more trivial. This is all related to #708 and while there&#39;s still more work to be done in terms of documentation, this is the major bulk of the rest of the implementation work on #708 I believe. * More API updates * Run rustfmt * Fix a doc test * More test updates
5 years ago
)
.into(),
Remove `HostRef` from the `wasmtime` public API (#788) * Remove `HostRef` from the `wasmtime` public API This commit removes all remaining usages of `HostRef` in the public API of the `wasmtime` crate. This involved a number of API decisions such as: * None of `Func`, `Global`, `Table`, or `Memory` are wrapped in `HostRef` * All of `Func`, `Global`, `Table`, and `Memory` implement `Clone` now. * Methods called `type` are renamed to `ty` to avoid typing `r#type`. * Methods requiring mutability for external items now no longer require mutability. The mutable reference here is sort of a lie anyway since the internals are aliased by the underlying module anyway. This affects: * `Table::set` * `Table::grow` * `Memory::grow` * `Instance::set_signal_handler` * The `Val::FuncRef` type is now no longer automatically coerced to `AnyRef`. This is technically a breaking change which is pretty bad, but I&#39;m hoping that we can live with this interim state while we sort out the `AnyRef` story in general. * The implementation of the C API was refactored and updated in a few locations to account for these changes: * Accessing the exports of an instance are now cached to ensure we always hand out the same `HostRef` values. * `wasm_*_t` for external values no longer have internal cache, instead they all wrap `wasm_external_t` and have an unchecked accessor for the underlying variant (since the type is proof that it&#39;s there). This makes casting back and forth much more trivial. This is all related to #708 and while there&#39;s still more work to be done in terms of documentation, this is the major bulk of the rest of the implementation work on #708 I believe. * More API updates * Run rustfmt * Fix a doc test * More test updates
5 years ago
Func::new(
&mut store,
FuncType::new(None, Some(ValType::F32)),
|_, params, results| {
assert!(params.is_empty());
assert_eq!(results.len(), 1);
results[0] = 2.0f32.into();
Ok(())
},
Remove `HostRef` from the `wasmtime` public API (#788) * Remove `HostRef` from the `wasmtime` public API This commit removes all remaining usages of `HostRef` in the public API of the `wasmtime` crate. This involved a number of API decisions such as: * None of `Func`, `Global`, `Table`, or `Memory` are wrapped in `HostRef` * All of `Func`, `Global`, `Table`, and `Memory` implement `Clone` now. * Methods called `type` are renamed to `ty` to avoid typing `r#type`. * Methods requiring mutability for external items now no longer require mutability. The mutable reference here is sort of a lie anyway since the internals are aliased by the underlying module anyway. This affects: * `Table::set` * `Table::grow` * `Memory::grow` * `Instance::set_signal_handler` * The `Val::FuncRef` type is now no longer automatically coerced to `AnyRef`. This is technically a breaking change which is pretty bad, but I&#39;m hoping that we can live with this interim state while we sort out the `AnyRef` story in general. * The implementation of the C API was refactored and updated in a few locations to account for these changes: * Accessing the exports of an instance are now cached to ensure we always hand out the same `HostRef` values. * `wasm_*_t` for external values no longer have internal cache, instead they all wrap `wasm_external_t` and have an unchecked accessor for the underlying variant (since the type is proof that it&#39;s there). This makes casting back and forth much more trivial. This is all related to #708 and while there&#39;s still more work to be done in terms of documentation, this is the major bulk of the rest of the implementation work on #708 I believe. * More API updates * Run rustfmt * Fix a doc test * More test updates
5 years ago
)
.into(),
];
let instance = Instance::new(&mut store, &module, &imports)?;
let func = instance.get_typed_func::<(), i32>(&mut store, "foo")?;
let result = func.call(&mut store, ())?;
Redo the statically typed `Func` API (#2719) * Redo the statically typed `Func` API This commit reimplements the `Func` API with respect to statically typed dispatch. Previously `Func` had a `getN` and `getN_async` family of methods which were implemented for 0 to 16 parameters. The return value of these functions was an `impl Fn(..)` closure with the appropriate parameters and return values. There are a number of downsides with this approach that have become apparent over time: * The addition of `*_async` doubled the API surface area (which is quite large here due to one-method-per-number-of-parameters). * The [documentation of `Func`][old-docs] are quite verbose and feel &#34;polluted&#34; with all these getters, making it harder to understand the other methods that can be used to interact with a `Func`. * These methods unconditionally pay the cost of returning an owned `impl Fn` with a `&#39;static` lifetime. While cheap, this is still paying the cost for cloning the `Store` effectively and moving data into the closed-over environment. * Storage of the return value into a struct, for example, always requires `Box`-ing the returned closure since it otherwise cannot be named. * Recently I had the desire to implement an &#34;unchecked&#34; path for invoking wasm where you unsafely assert the type signature of a wasm function. Doing this with today&#39;s scheme would require doubling (again) the API surface area for both async and synchronous calls, further polluting the documentation. The main benefit of the previous scheme is that by returning a `impl Fn` it was quite easy and ergonomic to actually invoke the function. In practice, though, examples would often have something akin to `.get0::&lt;()&gt;()?()?` which is a lot of things to interpret all at once. Note that `get0` means &#34;0 parameters&#34; yet a type parameter is passed. There&#39;s also a double function invocation which looks like a lot of characters all lined up in a row. Overall, I think that the previous design is starting to show too many cracks and deserves a rewrite. This commit is that rewrite. The new design in this commit is to delete the `getN{,_async}` family of functions and instead have a new API: impl Func { fn typed&lt;P, R&gt;(&amp;self) -&gt; Result&lt;&amp;Typed&lt;P, R&gt;&gt;; } impl Typed&lt;P, R&gt; { fn call(&amp;self, params: P) -&gt; Result&lt;R, Trap&gt;; async fn call_async(&amp;self, params: P) -&gt; Result&lt;R, Trap&gt;; } This should entirely replace the current scheme, albeit by slightly losing ergonomics use cases. The idea behind the API is that the existence of `Typed&lt;P, R&gt;` is a &#34;proof&#34; that the underlying function takes `P` and returns `R`. The `Func::typed` method peforms a runtime type-check to ensure that types all match up, and if successful you get a `Typed` value. Otherwise an error is returned. Once you have a `Typed` then, like `Func`, you can either `call` or `call_async`. The difference with a `Typed`, however, is that the params/results are statically known and hence these calls can be much more efficient. This is a much smaller API surface area from before and should greatly simplify the `Func` documentation. There&#39;s still a problem where `Func::wrapN_async` produces a lot of functions to document, but that&#39;s now the sole offender. It&#39;s a nice benefit that the statically-typed-async verisons are now expressed with an `async` function rather than a function-returning-a-future which makes it both more efficient and easier to understand. The type `P` and `R` are intended to either be bare types (e.g. `i32`) or tuples of any length (including 0). At this time `R` is only allowed to be `()` or a bare `i32`-style type because multi-value is not supported with a native ABI (yet). The `P`, however, can be any size of tuples of parameters. This is also where some ergonomics are lost because instead of `f(1, 2)` you now have to write `f.call((1, 2))` (note the double-parens). Similarly `f()` becomes `f.call(())`. Overall I feel that this is a better tradeoff than before. While not universally better due to the loss in ergonomics I feel that this design is much more flexible in terms of what you can do with the return value and also understanding the API surface area (just less to take in). [old-docs]: https://docs.rs/wasmtime/0.24.0/wasmtime/struct.Func.html#method.get0 * Rename Typed to TypedFunc * Implement multi-value returns through `Func::typed` * Fix examples in docs * Fix some more errors * More test fixes * Rebasing and adding `get_typed_func` * Updating tests * Fix typo * More doc tweaks * Tweak visibility on `Func::invoke` * Fix tests again
4 years ago
assert_eq!(result, 3);
Ok(())
}