WIP: demonstrate copying bytes at the Wasm-WASI boundary

The fundamental concern with shared Wasm memories is that passing around
a `&[T]` or `&mut [T]` (or `&str` or `&mut str`, though these are less
common) to the Rust code implementing a WASI call would be prone to
having "the ground shift under its feet." There is no way to guarantee
that multiple threads are not reading or writing to the same Wasm
address, but somehow we want to prevent the Rust code from being
affected since it may have been written with Rust assumptions about
`&[T]` and `&mut [T]` in mind. This problem is difficult due to Wasm's
unrestricted access to linear memory; the best we can do is add a layer
of protection at the Wasm-WASI boundary by copying the bytes.

This change modifies `GuestSlice` (only this one for now to get early
feedback) to copy the slice bytes out of linear memory into an
allocation owned and later deallocated on the Wiggle side. There is a
lot that can be improved but I am looking for feedback on whether this
is the right approach (modulo some clean up).

crates/wiggle/generate/src/wasmtime.rs

@@ -112,22 +112,22 @@ fn generate_func(
  };
 
  let body = quote! {
- let (mem, ctx) = match caller.get_export("memory") {
+ let (mem, ctx, shared) = match caller.get_export("memory") {
  Some(#rt::wasmtime_crate::Extern::Memory(m)) => {
  let (mem, ctx) = m.data_and_store_mut(&mut caller);
  let ctx = get_cx(ctx);
- (mem, ctx)
+ (mem, ctx, false)
  }
  Some(#rt::wasmtime_crate::Extern::SharedMemory(m)) => {
  let mem = unsafe { std::slice::from_raw_parts_mut(m.data() as *mut u8, m.data_size()) };
  let ctx = get_cx(caller.data_mut());
- (mem, ctx)
+ (mem, ctx, true)
  }
  _ => {
  return Err(#rt::wasmtime_crate::Trap::new("missing required memory export"));
  }
  };
- let mem = #rt::wasmtime::WasmtimeGuestMemory::new(mem);
+ let mem = #rt::wasmtime::WasmtimeGuestMemory::new(mem, shared);
  Ok(<#ret_ty>::from(#abi_func(ctx, &mem #(, #arg_names)*) #await_ ?))
  };
 

crates/wiggle/src/lib.rs

@@ -191,6 +191,11 @@ pub unsafe trait GuestMemory: Send + Sync {
  /// `GuestStr` are implemented correctly, a shared `BorrowHandle` should only be
  /// unborrowed once.
  fn shared_unborrow(&self, h: BorrowHandle);
+
+ /// TODO
+ fn is_shared_concurrently(&self) -> bool {
+ false
+ }
 }
 
 /// A handle to a borrow on linear memory. It is produced by `{mut, shared}_borrow` and
@@ -538,14 +543,41 @@ impl<'a, T> GuestPtr<'a, [T]> {
  T::validate(unsafe { ptr.add(offs as usize) })?;
  }
 
- // SAFETY: iff there are no overlapping mut borrows it is valid to construct a &[T]
- let ptr = unsafe { slice::from_raw_parts(ptr, self.pointer.1 as usize) };
+ if self.mem.is_shared_concurrently() {
+ // SAFETY: iff there are no overlapping mut borrows it is valid to construct a &[T]
+ let ptr = unsafe { slice::from_raw_parts(ptr, self.pointer.1 as usize) };
 
- Ok(GuestSlice {
- ptr,
- mem: self.mem,
- borrow,
- })
+ Ok(GuestSlice {
+ ptr,
+ mem: self.mem,
+ borrow,
+ copy: None,
+ })
+ } else {
+ // SAFETY: to prevent concurrent access to the underlying bytes, we
+ // copy the bytes and use a pointer to that instead of the original
+ // linear memory. (It would have been nice to use `Box` but the
+ // `Global` allocator interface is still in nightly, see
+ // https://doc.rust-lang.org/std/boxed/index.html#memory-layout.)
+ let layout =
+ std::alloc::Layout::from_size_align(self.pointer.1 as usize, T::guest_align())
+ .unwrap(); // TODO
+ let copy = unsafe { std::alloc::alloc(layout) };
+ let copy = std::ptr::NonNull::new(copy).unwrap_or_else(|| {
+ std::alloc::handle_alloc_error(layout);
+ });
+ let copy = unsafe { copy.cast::<T>().as_mut() };
+ unsafe { std::ptr::copy(ptr, copy, self.pointer.1 as usize) }
+
+ let ptr = unsafe { slice::from_raw_parts(copy, self.pointer.1 as usize) };
+
+ Ok(GuestSlice {
+ ptr,
+ mem: self.mem,
+ borrow,
+ copy: Some(copy),
+ })
+ }
  }
 
  /// Attempts to create a [`GuestSliceMut<'_, T>`] from this pointer, performing
@@ -778,6 +810,7 @@ pub struct GuestSlice<'a, T> {
  ptr: &'a [T],
  mem: &'a dyn GuestMemory,
  borrow: BorrowHandle,
+ copy: Option<*mut T>,
 }
 
 impl<'a, T> std::ops::Deref for GuestSlice<'a, T> {
@@ -789,7 +822,10 @@ impl<'a, T> std::ops::Deref for GuestSlice<'a, T> {
 
 impl<'a, T> Drop for GuestSlice<'a, T> {
  fn drop(&mut self) {
- self.mem.shared_unborrow(self.borrow)
+ self.mem.shared_unborrow(self.borrow);
+ if let Some(copy) = self.copy {
+ unsafe { std::alloc::dealloc(copy.cast::<u8>(), std::alloc::Layout::for_value(&*copy)) }
+ }
  }
 }
 

crates/wiggle/src/wasmtime.rs

@@ -6,10 +6,11 @@ use crate::{BorrowHandle, GuestError, GuestMemory, Region};
 pub struct WasmtimeGuestMemory<'a> {
  mem: &'a mut [u8],
  bc: BorrowChecker,
+ shared: bool,
 }
 
 impl<'a> WasmtimeGuestMemory<'a> {
- pub fn new(mem: &'a mut [u8]) -> Self {
+ pub fn new(mem: &'a mut [u8], shared: bool) -> Self {
  Self {
  mem,
  // Wiggle does not expose any methods for functions to re-enter
@@ -22,6 +23,7 @@ impl<'a> WasmtimeGuestMemory<'a> {
  // integrated fully with wasmtime:
  // https://github.com/bytecodealliance/wasmtime/issues/1917
  bc: BorrowChecker::new(),
+ shared,
  }
  }
 }
@@ -51,4 +53,7 @@ unsafe impl GuestMemory for WasmtimeGuestMemory<'_> {
  fn mut_unborrow(&self, h: BorrowHandle) {
  self.bc.mut_unborrow(h)
  }
+ fn is_shared_concurrently(&self) -> bool {
+ self.shared
+ }
 }