Commit Graph

19 Commits

Author SHA1 Message Date
Andrew Brown
7717d8fa55 wiggle: adapt Wiggle guest slices for unsafe shared use (#5229)
* wiggle: adapt Wiggle guest slices for `unsafe` shared use

When multiple threads can concurrently modify a WebAssembly shared
memory, the underlying data for a Wiggle `GuestSlice` and
`GuestSliceMut` could change due to access from other threads. This
breaks Rust guarantees when `&[T]` and `&mut [T]` slices are handed out.
This change modifies `GuestPtr` to make `as_slice` and `as_slice_mut`
return an `Option` which is `None` when the underlying WebAssembly
memory is shared.

But WASI implementations still need access to the underlying WebAssembly
memory, both to read to it and write from it. This change adds new APIs:
- `GuestPtr::to_vec` copies the  bytes from WebAssembly memory (from
  which we can safely take a `&[T]`)
- `GuestPtr::as_unsafe_slice_mut` returns a wrapper `struct` from which
  we can  `unsafe`-ly return a mutable slice (users must accept the
  unsafety of concurrently modifying a `&mut [T]`)

This approach allows us to maintain Wiggle's borrow-checking
infrastructure, which enforces the guarantee that Wiggle will not modify
overlapping regions, e.g. This is important because the underlying
system calls may expect this. Though other threads may modify the same
underlying region, this is impossible to prevent; at least Wiggle will
not be able to do so.

Finally, the changes to Wiggle's API are propagated to all WASI
implementations in Wasmtime. For now, code locations that attempt to get
a guest slice will panic if the underlying memory is shared. Note that
Wiggle is not enabled for shared memory (that will come later in
something like #5054), but when it is, these panics will be clear
indicators of locations that must be re-implemented in a thread-safe
way.

* review: remove double cast

* review: refactor to include more logic in 'UnsafeGuestSlice'

* review: add reference to #4203

* review: link all thread-safe WASI fixups to #5235

* fix: consume 'UnsafeGuestSlice' during conversion to safe versions

* review: remove 'as_slice' and 'as_slice_mut'

* review: use 'as_unsafe_slice_mut' in 'to_vec'

* review: add `UnsafeBorrowResult`
2022-11-10 21:54:52 +00:00
Alex Crichton
2afaac5181 Return anyhow::Error from host functions instead of Trap, redesign Trap (#5149)
* Return `anyhow::Error` from host functions instead of `Trap`

This commit refactors how errors are modeled when returned from host
functions and additionally refactors how custom errors work with `Trap`.
At a high level functions in Wasmtime that previously worked with
`Result<T, Trap>` now work with `Result<T>` instead where the error is
`anyhow::Error`. This includes functions such as:

* Host-defined functions in a `Linker<T>`
* `TypedFunc::call`
* Host-related callbacks like call hooks

Errors are now modeled primarily as `anyhow::Error` throughout Wasmtime.
This subsequently removes the need for `Trap` to have the ability to
represent all host-defined errors as it previously did. Consequently the
`From` implementations for any error into a `Trap` have been removed
here and the only embedder-defined way to create a `Trap` is to use
`Trap::new` with a custom string.

After this commit the distinction between a `Trap` and a host error is
the wasm backtrace that it contains. Previously all errors in host
functions would flow through a `Trap` and get a wasm backtrace attached
to them, but now this only happens if a `Trap` itself is created meaning
that arbitrary host-defined errors flowing from a host import to the
other side won't get backtraces attached. Some internals of Wasmtime
itself were updated or preserved to use `Trap::new` to capture a
backtrace where it seemed useful, such as when fuel runs out.

The main motivation for this commit is that it now enables hosts to
thread a concrete error type from a host function all the way through to
where a wasm function was invoked. Previously this could not be done
since the host error was wrapped in a `Trap` that didn't provide the
ability to get at the internals.

A consequence of this commit is that when a host error is returned that
isn't a `Trap` we'll capture a backtrace and then won't have a `Trap` to
attach it to. To avoid losing the contextual information this commit
uses the `Error::context` method to attach the backtrace as contextual
information to ensure that the backtrace is itself not lost.

This is a breaking change for likely all users of Wasmtime, but it's
hoped to be a relatively minor change to workaround. Most use cases can
likely change `-> Result<T, Trap>` to `-> Result<T>` and otherwise
explicit creation of a `Trap` is largely no longer necessary.

* Fix some doc links

* add some tests and make a backtrace type public (#55)

* Trap: avoid a trailing newline in the Display impl

which in turn ends up with three newlines between the end of the
backtrace and the `Caused by` in the anyhow Debug impl

* make BacktraceContext pub, and add tests showing downcasting behavior of anyhow::Error to traps or backtraces

* Remove now-unnecesary `Trap` downcasts in `Linker::module`

* Fix test output expectations

* Remove `Trap::i32_exit`

This commit removes special-handling in the `wasmtime::Trap` type for
the i32 exit code required by WASI. This is now instead modeled as a
specific `I32Exit` error type in the `wasmtime-wasi` crate which is
returned by the `proc_exit` hostcall. Embedders which previously tested
for i32 exits now downcast to the `I32Exit` value.

* Remove the `Trap::new` constructor

This commit removes the ability to create a trap with an arbitrary error
message. The purpose of this commit is to continue the prior trend of
leaning into the `anyhow::Error` type instead of trying to recreate it
with `Trap`. A subsequent simplification to `Trap` after this commit is
that `Trap` will simply be an `enum` of trap codes with no extra
information. This commit is doubly-motivated by the desire to always use
the new `BacktraceContext` type instead of sometimes using that and
sometimes using `Trap`.

Most of the changes here were around updating `Trap::new` calls to
`bail!` calls instead. Tests which assert particular error messages
additionally often needed to use the `:?` formatter instead of the `{}`
formatter because the prior formats the whole `anyhow::Error` and the
latter only formats the top-most error, which now contains the
backtrace.

* Merge `Trap` and `TrapCode`

With prior refactorings there's no more need for `Trap` to be opaque or
otherwise contain a backtrace. This commit parse down `Trap` to simply
an `enum` which was the old `TrapCode`. All various tests and such were
updated to handle this.

The main consequence of this commit is that all errors have a
`BacktraceContext` context attached to them. This unfortunately means
that the backtrace is printed first before the error message or trap
code, but given all the prior simplifications that seems worth it at
this time.

* Rename `BacktraceContext` to `WasmBacktrace`

This feels like a better name given how this has turned out, and
additionally this commit removes having both `WasmBacktrace` and
`BacktraceContext`.

* Soup up documentation for errors and traps

* Fix build of the C API

Co-authored-by: Pat Hickey <pat@moreproductive.org>
2022-11-02 16:29:31 +00:00
Matthew Tamayo-Rios
f082756643 Make WASI-NN classes send and/or sync (#5077)
* Make send and remove wrapper around WasiNnCtx·

This removes the wrapper around WasiNnCtx and no longer requires borrow_mut(). Once send/sync
changes in OpenVINO crate are merged in it will allow·use by frameworks that requires this trait.

* Bump openvino to compatible version.

* BackendExecutionContext should be Send and Sync

* Fix rust format issues.

* Update Cargo.lock for openvino

* Audit changes to openvino crates.
2022-10-27 15:52:23 -07:00
Pat Hickey
0290a83502 wiggle: make wasmtime a mandatory dep, get rid of own Trap enum (#5137)
* wiggle: no longer need to guard wasmtime integration behind a feature

this existed so we could use wiggle in lucet, but lucet is long EOL

* replace wiggle::Trap with wiggle::wasmtime_crate::Trap

* wiggle tests: unwrap traps because we cant assert_eq on them

* wasi-common: emit a wasmtime::Trap instead of a wiggle::Trap

formally add a dependency on wasmtime here to make it obvious, though
we do now have a transitive one via wiggle no matter what (and therefore
can get rid of the default-features=false on the wiggle dep)

* wasi-nn: use wasmtime::Trap instead of wiggle::Trap

there's no way the implementation of this func is actually
a good idea, it will panic the host process on any error,
but I'll ask @mtr to fix that

* wiggle test-helpers examples: fixes

* wasi-common cant cross compile to wasm32-unknown-emscripten anymore

this was originally for the WASI polyfill for web targets. Those days
are way behind us now.

* wasmtime wont compile for armv7-unknown-linux-gnueabihf either
2022-10-27 09:28:10 -07:00
Andrew Brown
558a9273e9 wasi-nn: update openvino crate, use external CI action (#4383)
* ci: replace OpenVINO installer action

To test wasi-nn, we currently use an OpenVINO backend. The Wasmtime CI
must install OpenVINO using a custom GitHub action. This CI action has
not been updated in some time and in the meantime OpenVINO (and the
OpenVINO crates) have released several new versions.
https://github.com/abrown/install-openvino-action is an external action
that we plan to keep up to date with the latest releases. This change
replaces the current CI action with that one.

* wasi-nn: upgrade openvino dependency to v0.4.1

This eliminates a `lazy_static` dependency and changes a few parameters
to pass by reference. Importantly, it enables support for the latest
versions of OpenVINO (v2022.*) in wasi-nn.

* ci: update wasi-nn script to source correct env script

* ci: really use the correct path for the env script

Also, clarify which directory OpenVINO is installed in (the symlink may
not be present).
2022-07-05 18:50:50 +00:00
Brian Jones
65b443ad23 Fix for issue #3948 (#3956)
Updates the OpenVINO backend for wasi-nn to pre-configure all inputs as `NHWC`. This is not a long-term fix but no worse than the status quo, which configures all input tensors to the same layout. This change updates the CI script to use the latest version of OpenVINO. Closes #3948.

Co-authored-by: Andrew Brown <andrew.brown@intel.com>
2022-03-24 13:32:52 -07:00
Andrew Brown
2e95d4e7c6 wasi-nn: refactor wasi-nn context to use multiple backends 2021-08-10 10:05:52 -07:00
Andrew Brown
c3bbdead7c wasi-nn: add backend abstraction 2021-08-10 10:05:52 -07:00
Alex Crichton
7a1b7cdf92 Implement RFC 11: Redesigning Wasmtime's APIs (#2897)
Implement Wasmtime's new API as designed by RFC 11. This is quite a large commit which has had lots of discussion externally, so for more information it's best to read the RFC thread and the PR thread.
2021-06-03 09:10:53 -05:00
Andrew Brown
92e0b6b9e8 wasi-nn: turn it on by default (#2859)
* wasi-nn: turn it on by default

This change makes the wasi-nn Cargo feature a default feature. Previously, a wasi-nn user would have to build a separate Wasmtime binary (e.g. `cargo build --features wasi-nn ...`) to use wasi-nn and the resulting binary would require OpenVINO shared libraries to be present in the environment in order to run (otherwise it would fail immediately with linking errors). With recent changes to the `openvino` crate, the wasi-nn implementation can defer the loading of the OpenVINO shared libraries until runtime (i.e., when the user Wasm program calls `wasi_ephemeral_nn::load`) and display a user-level error if anything goes wrong (e.g., the OpenVINO libraries are not present on the system). This runtime-linking addition allows the wasi-nn feature to be turned on by default and shipped with upcoming releases of Wasmtime. This change should be transparent for users who do not use wasi-nn: the `openvino` crate is small and the newly-available wasi-nn imports only affect programs in which they are used.

For those interested in reviewing the runtime linking approach added to the `openvino` crate, see https://github.com/intel/openvino-rs/pull/19.

* wasi-nn spec path: don't use canonicalize

* Allow dependencies using the ISC license

The ISC license should be [just as permissive](https://choosealicense.com/licenses/isc) as MIT, e.g., with no additional limitations.

* Add a `--wasi-modules` flag

This flag controls which WASI modules are made available to the Wasm program. This initial commit enables `wasi-common` by default (equivalent to `--wasi-modules=all`) and allows `wasi-nn` and `wasi-crypto` to be added in either individually (e.g., `--wasi-modules=wasi-nn`) or as a group (e.g., `--wasi-modules=all-experimental`).

* wasi-crypto: fix unused dependency

Co-authored-by: Pat Hickey <pat@moreproductive.org>
2021-04-29 15:03:28 -05:00
Pat Hickey
e6c7e00a52 wiggle-using crates: delete GuestErrorConversion 2021-03-24 10:39:06 -07:00
Pat Hickey
bcebdd43ef wiggle use sites: remove ctx argument 2021-03-04 18:16:37 -08:00
Alex Crichton
250f11c572 Fix build of wasi-nn 2021-02-18 14:45:20 -08:00
Pat Hickey
2483ad4c1c wasi-nn: UserErrorConversion error is now a wiggle::Trap 2021-01-07 11:45:11 -08:00
Pat Hickey
d4aaae3e86 wasi-nn: remove missing_memory from wasmtime_integration 2021-01-06 10:59:29 -08:00
Tanya L. Crenshaw
b06ed39c1e Fixes #2418: Enhance wiggle to generate its UserErrorConverstion trait with a function that returns Result<abi_err, String> (#2419)
* Enhance wiggle to generate its UserErrorConverstion trait with a function that returns
a Result<abi_err, String>.  This enhancement allows hostcall implementations using wiggle
to return an actionable error to the instance (the abi_err) or to terminate the instance
using the String as fatal error information.

* Enhance wiggle to generate its UserErrorConverstion trait with a function that returns
a Result<abi_err, String>.  This enhancement allows hostcall implementations using wiggle
to return an actionable error to the instance (the abi_err) or to terminate the instance
using the String as fatal error information.

* Enhance the wiggle/wasmtime integration to leverage new work in ab7e9c6.  Hostcall
implementations generated by wiggle now return an Result<abi_error, Trap>.  As a
result, hostcalls experiencing fatal errors may trap, thereby terminating the
wasmtime instance.  This enhancement has been performed for both wasi snapshot1
and wasi snapshot0.

* Update wasi-nn crate to reflect enhancement in issue #2418.

* Update wiggle test-helpers for wiggle enhancement made in issue #2418.

* Address PR feedback; omit verbose return statement.

* Address PR feedback; manually format within a proc macro.

* Address PR feedback; manually format proc macro.

* Restore return statements to wasi.rs.

* Restore return statements in funcs.rs.

* Address PR feedback; omit TODO and fix formatting.

* Ok-wrap error type in assert statement.
2020-11-24 14:06:57 -06:00
Pat Hickey
224e8b0e88 wasi-nn: fix mutable guestslice borrow 2020-11-19 15:28:53 -08:00
Andrew Brown
3d606a01e5 wasi-nn: remove unused functions (#2427) 2020-11-18 09:21:51 -06:00
Andrew Brown
a61f068c64 Add an initial wasi-nn implementation for Wasmtime (#2208)
* Add an initial wasi-nn implementation for Wasmtime

This change adds a crate, `wasmtime-wasi-nn`, that uses `wiggle` to expose the current state of the wasi-nn API and `openvino` to implement the exposed functions. It includes an end-to-end test demonstrating how to do classification using wasi-nn:
 - `crates/wasi-nn/tests/classification-example` contains Rust code that is compiled to the `wasm32-wasi` target and run with a Wasmtime embedding that exposes the wasi-nn calls
 - the example uses Rust bindings for wasi-nn contained in `crates/wasi-nn/tests/wasi-nn-rust-bindings`; this crate contains code generated by `witx-bindgen` and eventually should be its own standalone crate

* Test wasi-nn as a CI step

This change adds:
 - a GitHub action for installing OpenVINO
 - a script, `ci/run-wasi-nn-example.sh`, to run the classification example
2020-11-16 12:54:00 -06:00