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`
This commit is contained in:
Andrew Brown
2022-11-10 13:54:52 -08:00
committed by GitHub
parent 0548952319
commit 7717d8fa55
11 changed files with 446 additions and 148 deletions

View File

@@ -80,8 +80,11 @@ impl<'a> WasiEphemeralNn for WasiNnCtx {
out_buffer: &GuestPtr<'_, u8>,
out_buffer_max_size: u32,
) -> Result<u32> {
let mut destination = out_buffer.as_array(out_buffer_max_size).as_slice_mut()?;
if let Some(exec_context) = self.executions.get_mut(exec_context_id) {
let mut destination = out_buffer
.as_array(out_buffer_max_size)
.as_slice_mut()?
.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
Ok(exec_context.get_output(index, &mut destination)?)
} else {
Err(UsageError::InvalidGraphHandle.into())

View File

@@ -31,8 +31,15 @@ impl Backend for OpenvinoBackend {
// Read the guest array.
let builders = builders.as_ptr();
let xml = builders.read()?.as_slice()?;
let weights = builders.add(1)?.read()?.as_slice()?;
let xml = builders
.read()?
.as_slice()?
.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let weights = builders
.add(1)?
.read()?
.as_slice()?
.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
// Construct OpenVINO graph structures: `cnn_network` contains the graph
// structure, `exec_network` can perform inference.
@@ -78,6 +85,7 @@ impl BackendExecutionContext for OpenvinoExecutionContext {
let dimensions = tensor
.dimensions
.as_slice()?
.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)")
.iter()
.map(|d| *d as usize)
.collect::<Vec<_>>();
@@ -86,7 +94,10 @@ impl BackendExecutionContext for OpenvinoExecutionContext {
// TODO There must be some good way to discover the layout here; this
// should not have to default to NHWC.
let desc = TensorDesc::new(Layout::NHWC, &dimensions, precision);
let data = tensor.data.as_slice()?;
let data = tensor
.data
.as_slice()?
.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let blob = openvino::Blob::new(&desc, &data)?;
// Actually assign the blob to the request.