From 52e8300f01e890b407f12ecc4cd669111fc14909 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 18 May 2020 11:45:12 -0700 Subject: [PATCH 01/19] wiggle: automate borrow checking, explicitly passing borrow checker throughout --- crates/wiggle/generate/src/funcs.rs | 14 +-- crates/wiggle/src/borrow.rs | 136 ++++++++++++++++------------ crates/wiggle/src/guest_type.rs | 2 +- crates/wiggle/src/lib.rs | 130 +++++++++++++++++++------- crates/wiggle/tests/arrays.rs | 38 ++++---- crates/wiggle/tests/atoms.rs | 9 +- crates/wiggle/tests/flags.rs | 8 +- crates/wiggle/tests/handles.rs | 11 ++- crates/wiggle/tests/ints.rs | 6 +- crates/wiggle/tests/pointers.rs | 16 ++-- crates/wiggle/tests/strings.rs | 45 +++++---- crates/wiggle/tests/structs.rs | 45 +++++---- crates/wiggle/tests/union.rs | 26 +++--- crates/wiggle/tests/wasi.rs | 18 ++-- 14 files changed, 313 insertions(+), 191 deletions(-) diff --git a/crates/wiggle/generate/src/funcs.rs b/crates/wiggle/generate/src/funcs.rs index 6bb5beed40..2a11a3d495 100644 --- a/crates/wiggle/generate/src/funcs.rs +++ b/crates/wiggle/generate/src/funcs.rs @@ -24,7 +24,9 @@ pub fn define_func( }); let abi_args = quote!( - ctx: &#ctx_type, memory: &dyn #rt::GuestMemory, + ctx: &#ctx_type, + memory: &dyn #rt::GuestMemory, + bc: &#rt::BorrowChecker, #(#params),* ); let abi_ret = if let Some(ret) = &coretype.ret { @@ -210,7 +212,7 @@ fn marshal_arg( let arg_name = names.func_ptr_binding(¶m.name); let name = names.func_param(¶m.name); quote! { - let #name = match #rt::GuestPtr::<#pointee_type>::new(memory, #arg_name as u32).read() { + let #name = match #rt::GuestPtr::<#pointee_type>::new(memory, bc, #arg_name as u32).read() { Ok(r) => r, Err(e) => { #error_handling @@ -256,7 +258,7 @@ fn marshal_arg( let len_name = names.func_len_binding(¶m.name); let name = names.func_param(¶m.name); quote! { - let #name = #rt::GuestPtr::<#lifetime, str>::new(memory, (#ptr_name as u32, #len_name as u32)); + let #name = #rt::GuestPtr::<#lifetime, str>::new(memory, bc, (#ptr_name as u32, #len_name as u32)); } } }, @@ -264,7 +266,7 @@ fn marshal_arg( let pointee_type = names.type_ref(pointee, anon_lifetime()); let name = names.func_param(¶m.name); quote! { - let #name = #rt::GuestPtr::<#pointee_type>::new(memory, #name as u32); + let #name = #rt::GuestPtr::<#pointee_type>::new(memory, bc, #name as u32); } } witx::Type::Struct(_) => read_conversion, @@ -274,7 +276,7 @@ fn marshal_arg( let len_name = names.func_len_binding(¶m.name); let name = names.func_param(¶m.name); quote! { - let #name = #rt::GuestPtr::<[#pointee_type]>::new(memory, (#ptr_name as u32, #len_name as u32)); + let #name = #rt::GuestPtr::<[#pointee_type]>::new(memory, bc, (#ptr_name as u32, #len_name as u32)); } } witx::Type::Union(_u) => read_conversion, @@ -303,7 +305,7 @@ where let ptr_name = names.func_ptr_binding(&result.name); let ptr_err_handling = error_handling(&format!("{}:result_ptr_mut", result.name.as_str())); let pre = quote! { - let #ptr_name = #rt::GuestPtr::<#pointee_type>::new(memory, #ptr_name as u32); + let #ptr_name = #rt::GuestPtr::<#pointee_type>::new(memory, bc, #ptr_name as u32); }; // trait binding returns func_param name. let val_name = names.func_param(&result.name); diff --git a/crates/wiggle/src/borrow.rs b/crates/wiggle/src/borrow.rs index 8d5d81c01a..dddf86391c 100644 --- a/crates/wiggle/src/borrow.rs +++ b/crates/wiggle/src/borrow.rs @@ -1,64 +1,67 @@ +use crate::error::GuestError; use crate::region::Region; -use crate::{GuestError, GuestPtr, GuestType}; +use std::cell::RefCell; +use std::collections::HashMap; -#[derive(Debug)] -pub struct GuestBorrows { - borrows: Vec, +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct BorrowHandle(usize); + +pub struct BorrowChecker { + bc: RefCell, } -impl GuestBorrows { +impl BorrowChecker { pub fn new() -> Self { - Self { - borrows: Vec::new(), + BorrowChecker { + bc: RefCell::new(InnerBorrowChecker::new()), + } + } + pub fn borrow(&self, r: Region) -> Result { + self.bc + .borrow_mut() + .borrow(r) + .ok_or_else(|| GuestError::PtrBorrowed(r)) + } + pub fn unborrow(&self, h: BorrowHandle) { + self.bc.borrow_mut().unborrow(h) + } +} + +#[derive(Debug)] +struct InnerBorrowChecker { + borrows: HashMap, + next_handle: BorrowHandle, +} + +impl InnerBorrowChecker { + fn new() -> Self { + InnerBorrowChecker { + borrows: HashMap::new(), + next_handle: BorrowHandle(0), } } fn is_borrowed(&self, r: Region) -> bool { - !self.borrows.iter().all(|b| !b.overlaps(r)) + !self.borrows.values().all(|b| !b.overlaps(r)) } - pub(crate) fn borrow(&mut self, r: Region) -> Result<(), GuestError> { + fn new_handle(&mut self) -> BorrowHandle { + let h = self.next_handle; + self.next_handle = BorrowHandle(h.0 + 1); + h + } + + fn borrow(&mut self, r: Region) -> Option { if self.is_borrowed(r) { - Err(GuestError::PtrBorrowed(r)) - } else { - self.borrows.push(r); - Ok(()) + return None; } + let h = self.new_handle(); + self.borrows.insert(h, r); + Some(h) } - /// Borrow the region of memory pointed to by a `GuestPtr`. This is required for safety if - /// you are dereferencing `GuestPtr`s while holding a reference to a slice via - /// `GuestPtr::as_raw`. - pub fn borrow_pointee<'a, T>(&mut self, p: &GuestPtr<'a, T>) -> Result<(), GuestError> - where - T: GuestType<'a>, - { - self.borrow(Region { - start: p.offset(), - len: T::guest_size(), - }) - } - - /// Borrow the slice of memory pointed to by a `GuestPtr<[T]>`. This is required for safety if - /// you are dereferencing the `GuestPtr`s while holding a reference to another slice via - /// `GuestPtr::as_raw`. Not required if using `GuestPtr::as_raw` on this pointer. - pub fn borrow_slice<'a, T>(&mut self, p: &GuestPtr<'a, [T]>) -> Result<(), GuestError> - where - T: GuestType<'a>, - { - let (start, elems) = p.offset(); - let len = T::guest_size() - .checked_mul(elems) - .ok_or_else(|| GuestError::PtrOverflow)?; - self.borrow(Region { start, len }) - } - - /// Borrow the slice of memory pointed to by a `GuestPtr`. This is required for safety if - /// you are dereferencing the `GuestPtr`s while holding a reference to another slice via - /// `GuestPtr::as_raw`. Not required if using `GuestPtr::as_raw` on this pointer. - pub fn borrow_str(&mut self, p: &GuestPtr) -> Result<(), GuestError> { - let (start, len) = p.offset(); - self.borrow(Region { start, len }) + fn unborrow(&mut self, h: BorrowHandle) { + let _ = self.borrows.remove(&h); } } @@ -67,14 +70,14 @@ mod test { use super::*; #[test] fn nonoverlapping() { - let mut bs = GuestBorrows::new(); + let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(0, 10); let r2 = Region::new(10, 10); assert!(!r1.overlaps(r2)); bs.borrow(r1).expect("can borrow r1"); bs.borrow(r2).expect("can borrow r2"); - let mut bs = GuestBorrows::new(); + let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(10, 10); let r2 = Region::new(0, 10); assert!(!r1.overlaps(r2)); @@ -84,35 +87,35 @@ mod test { #[test] fn overlapping() { - let mut bs = GuestBorrows::new(); + let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(0, 10); let r2 = Region::new(9, 10); assert!(r1.overlaps(r2)); bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_err(), "cant borrow r2"); + assert!(bs.borrow(r2).is_none(), "cant borrow r2"); - let mut bs = GuestBorrows::new(); + let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(0, 10); let r2 = Region::new(2, 5); assert!(r1.overlaps(r2)); bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_err(), "cant borrow r2"); + assert!(bs.borrow(r2).is_none(), "cant borrow r2"); - let mut bs = GuestBorrows::new(); + let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(9, 10); let r2 = Region::new(0, 10); assert!(r1.overlaps(r2)); bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_err(), "cant borrow r2"); + assert!(bs.borrow(r2).is_none(), "cant borrow r2"); - let mut bs = GuestBorrows::new(); + let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(2, 5); let r2 = Region::new(0, 10); assert!(r1.overlaps(r2)); bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_err(), "cant borrow r2"); + assert!(bs.borrow(r2).is_none(), "cant borrow r2"); - let mut bs = GuestBorrows::new(); + let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(2, 5); let r2 = Region::new(10, 5); let r3 = Region::new(15, 5); @@ -121,6 +124,23 @@ mod test { bs.borrow(r1).expect("can borrow r1"); bs.borrow(r2).expect("can borrow r2"); bs.borrow(r3).expect("can borrow r3"); - assert!(bs.borrow(r4).is_err(), "cant borrow r4"); + assert!(bs.borrow(r4).is_none(), "cant borrow r4"); + } + + #[test] + fn unborrowing() { + let mut bs = InnerBorrowChecker::new(); + let r1 = Region::new(0, 10); + let r2 = Region::new(10, 10); + assert!(!r1.overlaps(r2)); + let _h1 = bs.borrow(r1).expect("can borrow r1"); + let h2 = bs.borrow(r2).expect("can borrow r2"); + + assert!(bs.borrow(r2).is_none(), "can't borrow r2 twice"); + bs.unborrow(h2); + + let _h3 = bs + .borrow(r2) + .expect("can borrow r2 again now that its been unborrowed"); } } diff --git a/crates/wiggle/src/guest_type.rs b/crates/wiggle/src/guest_type.rs index 51e003c7f6..6cc1a877a1 100644 --- a/crates/wiggle/src/guest_type.rs +++ b/crates/wiggle/src/guest_type.rs @@ -131,7 +131,7 @@ impl<'a, T> GuestType<'a> for GuestPtr<'a, T> { fn read(ptr: &GuestPtr<'a, Self>) -> Result { let offset = ptr.cast::().read()?; - Ok(GuestPtr::new(ptr.mem(), offset)) + Ok(GuestPtr::new(ptr.mem(), ptr.borrow_checker(), offset)) } fn write(ptr: &GuestPtr<'_, Self>, val: Self) -> Result<(), GuestError> { diff --git a/crates/wiggle/src/lib.rs b/crates/wiggle/src/lib.rs index 2f7d6c7773..1901bfa9dc 100644 --- a/crates/wiggle/src/lib.rs +++ b/crates/wiggle/src/lib.rs @@ -16,7 +16,8 @@ mod error; mod guest_type; mod region; -pub use borrow::GuestBorrows; +pub use borrow::BorrowChecker; +use borrow::BorrowHandle; pub use error::GuestError; pub use guest_type::{GuestErrorType, GuestType, GuestTypeTransparent}; pub use region::Region; @@ -150,12 +151,12 @@ pub unsafe trait GuestMemory { /// Note that `T` can be almost any type, and typically `offset` is a `u32`. /// The exception is slices and strings, in which case `offset` is a `(u32, /// u32)` of `(offset, length)`. - fn ptr<'a, T>(&'a self, offset: T::Pointer) -> GuestPtr<'a, T> + fn ptr<'a, T>(&'a self, bc: &'a BorrowChecker, offset: T::Pointer) -> GuestPtr<'a, T> where Self: Sized, T: ?Sized + Pointee, { - GuestPtr::new(self, offset) + GuestPtr::new(self, bc, offset) } } @@ -237,6 +238,7 @@ unsafe impl GuestMemory for Arc { /// already-attached helper methods. pub struct GuestPtr<'a, T: ?Sized + Pointee> { mem: &'a (dyn GuestMemory + 'a), + bc: &'a BorrowChecker, pointer: T::Pointer, _marker: marker::PhantomData<&'a Cell>, } @@ -247,9 +249,14 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { /// Note that for sized types like `u32`, `GuestPtr`, etc, the `pointer` /// vlue is a `u32` offset into guest memory. For slices and strings, /// `pointer` is a `(u32, u32)` offset/length pair. - pub fn new(mem: &'a (dyn GuestMemory + 'a), pointer: T::Pointer) -> GuestPtr<'_, T> { + pub fn new( + mem: &'a (dyn GuestMemory + 'a), + bc: &'a BorrowChecker, + pointer: T::Pointer, + ) -> GuestPtr<'a, T> { GuestPtr { mem, + bc, pointer, _marker: marker::PhantomData, } @@ -268,6 +275,11 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { self.mem } + /// Returns the borrow checker that this pointer uses + pub fn borrow_checker(&self) -> &'a BorrowChecker { + self.bc + } + /// Casts this `GuestPtr` type to a different type. /// /// This is a safe method which is useful for simply reinterpreting the type @@ -278,7 +290,7 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { where T: Pointee, { - GuestPtr::new(self.mem, self.pointer) + GuestPtr::new(self.mem, self.bc, self.pointer) } /// Safely read a value from this pointer. @@ -345,7 +357,7 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { Some(o) => o, None => return Err(GuestError::PtrOverflow), }; - Ok(GuestPtr::new(self.mem, offset)) + Ok(GuestPtr::new(self.mem, self.bc, offset)) } /// Returns a `GuestPtr` for an array of `T`s using this pointer as the @@ -354,7 +366,7 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { where T: GuestType<'a> + Pointee, { - GuestPtr::new(self.mem, (self.pointer, elems)) + GuestPtr::new(self.mem, self.bc, (self.pointer, elems)) } } @@ -403,7 +415,7 @@ impl<'a, T> GuestPtr<'a, [T]> { /// For safety against overlapping mutable borrows, the user must use the /// same `GuestBorrows` to create all `*mut str` or `*mut [T]` that are alive /// at the same time. - pub fn as_raw(&self, bc: &mut GuestBorrows) -> Result<*mut [T], GuestError> + pub fn as_slice(&self) -> Result, GuestError> where T: GuestTypeTransparent<'a>, { @@ -415,7 +427,7 @@ impl<'a, T> GuestPtr<'a, [T]> { self.mem .validate_size_align(self.pointer.0, T::guest_align(), len)? as *mut T; - bc.borrow(Region { + let borrow = self.bc.borrow(Region { start: self.pointer.0, len, })?; @@ -428,10 +440,16 @@ impl<'a, T> GuestPtr<'a, [T]> { // SAFETY: iff there are no overlapping borrows (all uses of as_raw use this same // GuestBorrows), its valid to construct a *mut [T] - unsafe { + let ptr = unsafe { let s = slice::from_raw_parts_mut(ptr, self.pointer.1 as usize); - Ok(s as *mut [T]) - } + s as *mut [T] + }; + + Ok(GuestSlice { + ptr, + bc: self.bc, + borrow, + }) } /// Copies the data pointed to by `slice` into this guest region. @@ -451,22 +469,20 @@ impl<'a, T> GuestPtr<'a, [T]> { T: GuestTypeTransparent<'a> + Copy, { // bounds check ... - let raw = self.as_raw(&mut GuestBorrows::new())?; - unsafe { - // ... length check ... - if (*raw).len() != slice.len() { - return Err(GuestError::SliceLengthsDiffer); - } - // ... and copy! - (*raw).copy_from_slice(slice); - Ok(()) + let mut self_slice = self.as_slice()?; + // ... length check ... + if self_slice.len() != slice.len() { + return Err(GuestError::SliceLengthsDiffer); } + // ... and copy! + self_slice.copy_from_slice(slice); + Ok(()) } /// Returns a `GuestPtr` pointing to the base of the array for the interior /// type `T`. pub fn as_ptr(&self) -> GuestPtr<'a, T> { - GuestPtr::new(self.mem, self.offset_base()) + GuestPtr::new(self.mem, self.bc, self.offset_base()) } } @@ -485,7 +501,7 @@ impl<'a> GuestPtr<'a, str> { /// Returns a raw pointer for the underlying slice of bytes that this /// pointer points to. pub fn as_bytes(&self) -> GuestPtr<'a, [u8]> { - GuestPtr::new(self.mem, self.pointer) + GuestPtr::new(self.mem, self.bc, self.pointer) } /// Attempts to read a raw `*mut str` pointer from this pointer, performing @@ -505,24 +521,26 @@ impl<'a> GuestPtr<'a, str> { /// For safety against overlapping mutable borrows, the user must use the /// same `GuestBorrows` to create all `*mut str` or `*mut [T]` that are /// alive at the same time. - pub fn as_raw(&self, bc: &mut GuestBorrows) -> Result<*mut str, GuestError> { + pub fn as_str(&self) -> Result, GuestError> { let ptr = self .mem .validate_size_align(self.pointer.0, 1, self.pointer.1)?; - bc.borrow(Region { + let borrow = self.bc.borrow(Region { start: self.pointer.0, len: self.pointer.1, })?; // SAFETY: iff there are no overlapping borrows (all uses of as_raw use this same // GuestBorrows), its valid to construct a *mut str - unsafe { - let s = slice::from_raw_parts_mut(ptr, self.pointer.1 as usize); - match str::from_utf8_mut(s) { - Ok(s) => Ok(s), - Err(e) => Err(GuestError::InvalidUtf8(e)), - } + let ptr = unsafe { slice::from_raw_parts_mut(ptr, self.pointer.1 as usize) }; + match str::from_utf8_mut(ptr) { + Ok(ptr) => Ok(GuestStr { + ptr, + bc: self.bc, + borrow, + }), + Err(e) => Err(GuestError::InvalidUtf8(e)), } } } @@ -541,6 +559,56 @@ impl fmt::Debug for GuestPtr<'_, T> { } } +pub struct GuestSlice<'a, T> { + ptr: *mut [T], + bc: &'a BorrowChecker, + borrow: BorrowHandle, +} + +impl<'a, T> std::ops::Deref for GuestSlice<'a, T> { + type Target = [T]; + fn deref(&self) -> &Self::Target { + unsafe { self.ptr.as_ref().expect("ptr guaranteed to be non-null") } + } +} + +impl<'a, T> std::ops::DerefMut for GuestSlice<'a, T> { + fn deref_mut(&mut self) -> &mut Self::Target { + unsafe { self.ptr.as_mut().expect("ptr guaranteed to be non-null") } + } +} + +impl<'a, T> Drop for GuestSlice<'a, T> { + fn drop(&mut self) { + self.bc.unborrow(self.borrow) + } +} + +pub struct GuestStr<'a> { + ptr: *mut str, + bc: &'a BorrowChecker, + borrow: BorrowHandle, +} + +impl<'a> std::ops::Deref for GuestStr<'a> { + type Target = str; + fn deref(&self) -> &Self::Target { + unsafe { self.ptr.as_ref().expect("ptr guaranteed to be non-null") } + } +} + +impl<'a> std::ops::DerefMut for GuestStr<'a> { + fn deref_mut(&mut self) -> &mut Self::Target { + unsafe { self.ptr.as_mut().expect("ptr guaranteed to be non-null") } + } +} + +impl<'a> Drop for GuestStr<'a> { + fn drop(&mut self) { + self.bc.unborrow(self.borrow) + } +} + mod private { pub trait Sealed {} impl Sealed for T {} diff --git a/crates/wiggle/tests/arrays.rs b/crates/wiggle/tests/arrays.rs index 52d6c4b345..61b47ba416 100644 --- a/crates/wiggle/tests/arrays.rs +++ b/crates/wiggle/tests/arrays.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{GuestMemory, GuestPtr}; +use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -75,31 +75,35 @@ impl ReduceExcusesExcercise { } pub fn test(&self) { - let mut ctx = WasiCtx::new(); - let mut host_memory = HostMemory::new(); + let ctx = WasiCtx::new(); + let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); // Populate memory with pointers to generated Excuse values for (&excuse, ptr) in self.excuse_values.iter().zip(self.excuse_ptr_locs.iter()) { host_memory - .ptr(ptr.ptr) + .ptr(&bc, ptr.ptr) .write(excuse) .expect("deref ptr mut to Excuse value"); } // Populate the array with pointers to generated Excuse values { - let array: GuestPtr<'_, [GuestPtr]> = - host_memory.ptr((self.array_ptr_loc.ptr, self.excuse_ptr_locs.len() as u32)); + let array: GuestPtr<'_, [GuestPtr]> = host_memory.ptr( + &bc, + (self.array_ptr_loc.ptr, self.excuse_ptr_locs.len() as u32), + ); for (slot, ptr) in array.iter().zip(&self.excuse_ptr_locs) { let slot = slot.expect("array should be in bounds"); - slot.write(host_memory.ptr(ptr.ptr)) + slot.write(host_memory.ptr(&bc, ptr.ptr)) .expect("should succeed in writing array"); } } let res = arrays::reduce_excuses( - &mut ctx, - &mut host_memory, + &ctx, + &host_memory, + &bc, self.array_ptr_loc.ptr as i32, self.excuse_ptr_locs.len() as i32, self.return_ptr_loc.ptr as i32, @@ -112,7 +116,7 @@ impl ReduceExcusesExcercise { .last() .expect("generated vec of excuses should be non-empty"); let given: types::Excuse = host_memory - .ptr(self.return_ptr_loc.ptr) + .ptr(&bc, self.return_ptr_loc.ptr) .read() .expect("deref ptr to returned value"); assert_eq!(expected, given, "reduce excuses return val"); @@ -165,28 +169,30 @@ impl PopulateExcusesExcercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); // Populate array with valid pointers to Excuse type in memory - let ptr = host_memory.ptr::<[GuestPtr<'_, types::Excuse>]>(( - self.array_ptr_loc.ptr, - self.elements.len() as u32, - )); + let ptr = host_memory.ptr::<[GuestPtr<'_, types::Excuse>]>( + &bc, + (self.array_ptr_loc.ptr, self.elements.len() as u32), + ); for (ptr, val) in ptr.iter().zip(&self.elements) { ptr.expect("should be valid pointer") - .write(host_memory.ptr(val.ptr)) + .write(host_memory.ptr(&bc, val.ptr)) .expect("failed to write value"); } let res = arrays::populate_excuses( &ctx, &host_memory, + &bc, self.array_ptr_loc.ptr as i32, self.elements.len() as i32, ); assert_eq!(res, types::Errno::Ok.into(), "populate excuses errno"); let arr: GuestPtr<'_, [GuestPtr<'_, types::Excuse>]> = - host_memory.ptr((self.array_ptr_loc.ptr, self.elements.len() as u32)); + host_memory.ptr(&bc, (self.array_ptr_loc.ptr, self.elements.len() as u32)); for el in arr.iter() { let ptr_to_ptr = el .expect("valid ptr to ptr") diff --git a/crates/wiggle/tests/atoms.rs b/crates/wiggle/tests/atoms.rs index 5407828fa9..674fdaa5d5 100644 --- a/crates/wiggle/tests/atoms.rs +++ b/crates/wiggle/tests/atoms.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::GuestMemory; +use wiggle::{BorrowChecker, GuestMemory}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -31,8 +31,9 @@ impl IntFloatExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); - let e = atoms::int_float_args(&ctx, &host_memory, self.an_int as i32, self.an_float); + let e = atoms::int_float_args(&ctx, &host_memory, &bc, self.an_int as i32, self.an_float); assert_eq!(e, types::Errno::Ok.into(), "int_float_args error"); } @@ -60,16 +61,18 @@ impl DoubleIntExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); let e = atoms::double_int_return_float( &ctx, &host_memory, + &bc, self.input as i32, self.return_loc.ptr as i32, ); let return_val = host_memory - .ptr::(self.return_loc.ptr) + .ptr::(&bc, self.return_loc.ptr) .read() .expect("failed to read return"); assert_eq!(e, types::Errno::Ok.into(), "errno"); diff --git a/crates/wiggle/tests/flags.rs b/crates/wiggle/tests/flags.rs index fcb07e7a7d..33f47c8ccc 100644 --- a/crates/wiggle/tests/flags.rs +++ b/crates/wiggle/tests/flags.rs @@ -1,6 +1,6 @@ use proptest::prelude::*; use std::convert::TryFrom; -use wiggle::{GuestMemory, GuestPtr}; +use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -65,16 +65,18 @@ impl ConfigureCarExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); // Populate input ptr host_memory - .ptr(self.other_config_by_ptr.ptr) + .ptr(&bc, self.other_config_by_ptr.ptr) .write(self.other_config) .expect("deref ptr mut to CarConfig"); let res = flags::configure_car( &ctx, &host_memory, + &bc, self.old_config.into(), self.other_config_by_ptr.ptr as i32, self.return_ptr_loc.ptr as i32, @@ -82,7 +84,7 @@ impl ConfigureCarExercise { assert_eq!(res, types::Errno::Ok.into(), "configure car errno"); let res_config = host_memory - .ptr::(self.return_ptr_loc.ptr) + .ptr::(&bc, self.return_ptr_loc.ptr) .read() .expect("deref to CarConfig value"); diff --git a/crates/wiggle/tests/handles.rs b/crates/wiggle/tests/handles.rs index 93e3b54d18..292e689eb4 100644 --- a/crates/wiggle/tests/handles.rs +++ b/crates/wiggle/tests/handles.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{GuestMemory, GuestType}; +use wiggle::{BorrowChecker, GuestMemory, GuestType}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; const FD_VAL: u32 = 123; @@ -34,23 +34,24 @@ impl HandleExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); - let e = handle_examples::fd_create(&ctx, &host_memory, self.return_loc.ptr as i32); + let e = handle_examples::fd_create(&ctx, &host_memory, &bc, self.return_loc.ptr as i32); assert_eq!(e, types::Errno::Ok.into(), "fd_create error"); let h_got: u32 = host_memory - .ptr(self.return_loc.ptr) + .ptr(&bc, self.return_loc.ptr) .read() .expect("return ref_mut"); assert_eq!(h_got, 123, "fd_create return val"); - let e = handle_examples::fd_consume(&ctx, &host_memory, h_got as i32); + let e = handle_examples::fd_consume(&ctx, &host_memory, &bc, h_got as i32); assert_eq!(e, types::Errno::Ok.into(), "fd_consume error"); - let e = handle_examples::fd_consume(&ctx, &host_memory, h_got as i32 + 1); + let e = handle_examples::fd_consume(&ctx, &host_memory, &bc, h_got as i32 + 1); assert_eq!( e, diff --git a/crates/wiggle/tests/ints.rs b/crates/wiggle/tests/ints.rs index 36d7933e62..20d30ff86d 100644 --- a/crates/wiggle/tests/ints.rs +++ b/crates/wiggle/tests/ints.rs @@ -1,6 +1,6 @@ use proptest::prelude::*; use std::convert::TryFrom; -use wiggle::GuestMemory; +use wiggle::{BorrowChecker, GuestMemory}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -46,17 +46,19 @@ impl CookieCutterExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); let res = ints::cookie_cutter( &ctx, &host_memory, + &bc, self.cookie.into(), self.return_ptr_loc.ptr as i32, ); assert_eq!(res, types::Errno::Ok.into(), "cookie cutter errno"); let is_cookie_start = host_memory - .ptr::(self.return_ptr_loc.ptr) + .ptr::(&bc, self.return_ptr_loc.ptr) .read() .expect("deref to Bool value"); diff --git a/crates/wiggle/tests/pointers.rs b/crates/wiggle/tests/pointers.rs index c39988d115..2a63507900 100644 --- a/crates/wiggle/tests/pointers.rs +++ b/crates/wiggle/tests/pointers.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{GuestMemory, GuestPtr}; +use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -128,30 +128,32 @@ impl PointersAndEnumsExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); host_memory - .ptr(self.input2_loc.ptr) + .ptr(&bc, self.input2_loc.ptr) .write(self.input2) .expect("input2 ref_mut"); host_memory - .ptr(self.input3_loc.ptr) + .ptr(&bc, self.input3_loc.ptr) .write(self.input3) .expect("input3 ref_mut"); host_memory - .ptr(self.input4_loc.ptr) + .ptr(&bc, self.input4_loc.ptr) .write(self.input4) .expect("input4 ref_mut"); host_memory - .ptr(self.input4_ptr_loc.ptr) + .ptr(&bc, self.input4_ptr_loc.ptr) .write(self.input4_loc.ptr) .expect("input4 ptr ref_mut"); let e = pointers::pointers_and_enums( &ctx, &host_memory, + &bc, self.input1.into(), self.input2_loc.ptr as i32, self.input3_loc.ptr as i32, @@ -161,7 +163,7 @@ impl PointersAndEnumsExercise { // Implementation of pointers_and_enums writes input3 to the input2_loc: let written_to_input2_loc: i32 = host_memory - .ptr(self.input2_loc.ptr) + .ptr(&bc, self.input2_loc.ptr) .read() .expect("input2 ref"); @@ -173,7 +175,7 @@ impl PointersAndEnumsExercise { // Implementation of pointers_and_enums writes input2_loc to input4_ptr_loc: let written_to_input4_ptr: u32 = host_memory - .ptr(self.input4_ptr_loc.ptr) + .ptr(&bc, self.input4_ptr_loc.ptr) .read() .expect("input4_ptr_loc ref"); diff --git a/crates/wiggle/tests/strings.rs b/crates/wiggle/tests/strings.rs index f14920ca75..51c5b4fde2 100644 --- a/crates/wiggle/tests/strings.rs +++ b/crates/wiggle/tests/strings.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{GuestBorrows, GuestMemory, GuestPtr}; +use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, MemAreas, WasiCtx}; wiggle::from_witx!({ @@ -11,12 +11,9 @@ impl_errno!(types::Errno, types::GuestErrorConversion); impl<'a> strings::Strings for WasiCtx<'a> { fn hello_string(&self, a_string: &GuestPtr) -> Result { - let mut bc = GuestBorrows::new(); - let s = a_string.as_raw(&mut bc).expect("should be valid string"); - unsafe { - println!("a_string='{}'", &*s); - Ok((*s).len() as u32) - } + let s = a_string.as_str().expect("should be valid string"); + println!("a_string='{}'", &*s); + Ok(s.len() as u32) } fn multi_string( @@ -25,18 +22,15 @@ impl<'a> strings::Strings for WasiCtx<'a> { b: &GuestPtr, c: &GuestPtr, ) -> Result { - let mut bc = GuestBorrows::new(); - let sa = a.as_raw(&mut bc).expect("A should be valid string"); - let sb = b.as_raw(&mut bc).expect("B should be valid string"); - let sc = c.as_raw(&mut bc).expect("C should be valid string"); - unsafe { - let total_len = (&*sa).len() + (&*sb).len() + (&*sc).len(); - println!( - "len={}, a='{}', b='{}', c='{}'", - total_len, &*sa, &*sb, &*sc - ); - Ok(total_len as u32) - } + let sa = a.as_str().expect("A should be valid string"); + let sb = b.as_str().expect("B should be valid string"); + let sc = c.as_str().expect("C should be valid string"); + let total_len = sa.len() + sb.len() + sc.len(); + println!( + "len={}, a='{}', b='{}', c='{}'", + total_len, &*sa, &*sb, &*sc + ); + Ok(total_len as u32) } } @@ -75,9 +69,11 @@ impl HelloStringExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); // Populate string in guest's memory - let ptr = host_memory.ptr::((self.string_ptr_loc.ptr, self.test_word.len() as u32)); + let ptr = + host_memory.ptr::(&bc, (self.string_ptr_loc.ptr, self.test_word.len() as u32)); for (slot, byte) in ptr.as_bytes().iter().zip(self.test_word.bytes()) { slot.expect("should be valid pointer") .write(byte) @@ -87,6 +83,7 @@ impl HelloStringExercise { let res = strings::hello_string( &ctx, &host_memory, + &bc, self.string_ptr_loc.ptr as i32, self.test_word.len() as i32, self.return_ptr_loc.ptr as i32, @@ -94,7 +91,7 @@ impl HelloStringExercise { assert_eq!(res, types::Errno::Ok.into(), "hello string errno"); let given = host_memory - .ptr::(self.return_ptr_loc.ptr) + .ptr::(&bc, self.return_ptr_loc.ptr) .read() .expect("deref ptr to return value"); assert_eq!(self.test_word.len() as u32, given); @@ -181,9 +178,10 @@ impl MultiStringExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); let write_string = |val: &str, loc: MemArea| { - let ptr = host_memory.ptr::((loc.ptr, val.len() as u32)); + let ptr = host_memory.ptr::(&bc, (loc.ptr, val.len() as u32)); for (slot, byte) in ptr.as_bytes().iter().zip(val.bytes()) { slot.expect("should be valid pointer") .write(byte) @@ -198,6 +196,7 @@ impl MultiStringExercise { let res = strings::multi_string( &ctx, &host_memory, + &bc, self.sa_ptr_loc.ptr as i32, self.a.len() as i32, self.sb_ptr_loc.ptr as i32, @@ -209,7 +208,7 @@ impl MultiStringExercise { assert_eq!(res, types::Errno::Ok.into(), "multi string errno"); let given = host_memory - .ptr::(self.return_ptr_loc.ptr) + .ptr::(&bc, self.return_ptr_loc.ptr) .read() .expect("deref ptr to return value"); assert_eq!((self.a.len() + self.b.len() + self.c.len()) as u32, given); diff --git a/crates/wiggle/tests/structs.rs b/crates/wiggle/tests/structs.rs index b98cf66163..accba503f7 100644 --- a/crates/wiggle/tests/structs.rs +++ b/crates/wiggle/tests/structs.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{GuestMemory, GuestPtr}; +use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -83,18 +83,20 @@ impl SumOfPairExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); host_memory - .ptr(self.input_loc.ptr) + .ptr(&bc, self.input_loc.ptr) .write(self.input.first) .expect("input ref_mut"); host_memory - .ptr(self.input_loc.ptr + 4) + .ptr(&bc, self.input_loc.ptr + 4) .write(self.input.second) .expect("input ref_mut"); let sum_err = structs::sum_of_pair( &ctx, &host_memory, + &bc, self.input_loc.ptr as i32, self.return_loc.ptr as i32, ); @@ -102,7 +104,7 @@ impl SumOfPairExercise { assert_eq!(sum_err, types::Errno::Ok.into(), "sum errno"); let return_val: i64 = host_memory - .ptr(self.return_loc.ptr) + .ptr(&bc, self.return_loc.ptr) .read() .expect("return ref"); @@ -171,28 +173,30 @@ impl SumPairPtrsExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); host_memory - .ptr(self.input_first_loc.ptr) + .ptr(&bc, self.input_first_loc.ptr) .write(self.input_first) .expect("input_first ref"); host_memory - .ptr(self.input_second_loc.ptr) + .ptr(&bc, self.input_second_loc.ptr) .write(self.input_second) .expect("input_second ref"); host_memory - .ptr(self.input_struct_loc.ptr) + .ptr(&bc, self.input_struct_loc.ptr) .write(self.input_first_loc.ptr) .expect("input_struct ref"); host_memory - .ptr(self.input_struct_loc.ptr + 4) + .ptr(&bc, self.input_struct_loc.ptr + 4) .write(self.input_second_loc.ptr) .expect("input_struct ref"); let res = structs::sum_of_pair_of_ptrs( &ctx, &host_memory, + &bc, self.input_struct_loc.ptr as i32, self.return_loc.ptr as i32, ); @@ -200,7 +204,7 @@ impl SumPairPtrsExercise { assert_eq!(res, types::Errno::Ok.into(), "sum of pair of ptrs errno"); let doubled: i64 = host_memory - .ptr(self.return_loc.ptr) + .ptr(&bc, self.return_loc.ptr) .read() .expect("return ref"); @@ -255,23 +259,25 @@ impl SumIntAndPtrExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); host_memory - .ptr(self.input_first_loc.ptr) + .ptr(&bc, self.input_first_loc.ptr) .write(self.input_first) .expect("input_first ref"); host_memory - .ptr(self.input_struct_loc.ptr) + .ptr(&bc, self.input_struct_loc.ptr) .write(self.input_first_loc.ptr) .expect("input_struct ref"); host_memory - .ptr(self.input_struct_loc.ptr + 4) + .ptr(&bc, self.input_struct_loc.ptr + 4) .write(self.input_second) .expect("input_struct ref"); let res = structs::sum_of_int_and_ptr( &ctx, &host_memory, + &bc, self.input_struct_loc.ptr as i32, self.return_loc.ptr as i32, ); @@ -279,7 +285,7 @@ impl SumIntAndPtrExercise { assert_eq!(res, types::Errno::Ok.into(), "sum of int and ptr errno"); let doubled: i64 = host_memory - .ptr(self.return_loc.ptr) + .ptr(&bc, self.return_loc.ptr) .read() .expect("return ref"); @@ -312,13 +318,14 @@ impl ReturnPairInts { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); - let err = structs::return_pair_ints(&ctx, &host_memory, self.return_loc.ptr as i32); + let err = structs::return_pair_ints(&ctx, &host_memory, &bc, self.return_loc.ptr as i32); assert_eq!(err, types::Errno::Ok.into(), "return struct errno"); let return_struct: types::PairInts = host_memory - .ptr(self.return_loc.ptr) + .ptr(&bc, self.return_loc.ptr) .read() .expect("return ref"); @@ -377,19 +384,21 @@ impl ReturnPairPtrsExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); host_memory - .ptr(self.input_first_loc.ptr) + .ptr(&bc, self.input_first_loc.ptr) .write(self.input_first) .expect("input_first ref"); host_memory - .ptr(self.input_second_loc.ptr) + .ptr(&bc, self.input_second_loc.ptr) .write(self.input_second) .expect("input_second ref"); let res = structs::return_pair_of_ptrs( &ctx, &host_memory, + &bc, self.input_first_loc.ptr as i32, self.input_second_loc.ptr as i32, self.return_loc.ptr as i32, @@ -398,7 +407,7 @@ impl ReturnPairPtrsExercise { assert_eq!(res, types::Errno::Ok.into(), "return pair of ptrs errno"); let ptr_pair_int_ptrs: types::PairIntPtrs<'_> = host_memory - .ptr(self.return_loc.ptr) + .ptr(&bc, self.return_loc.ptr) .read() .expect("failed to read return location"); let ret_first_ptr = ptr_pair_int_ptrs.first; diff --git a/crates/wiggle/tests/union.rs b/crates/wiggle/tests/union.rs index ab3dada99c..5a6e2b3a42 100644 --- a/crates/wiggle/tests/union.rs +++ b/crates/wiggle/tests/union.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{GuestMemory, GuestType}; +use wiggle::{BorrowChecker, GuestMemory, GuestType}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -107,21 +107,22 @@ impl GetTagExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); let discriminant: u8 = reason_tag(&self.input).into(); host_memory - .ptr(self.input_loc.ptr) + .ptr(&bc, self.input_loc.ptr) .write(discriminant) .expect("input discriminant ptr"); match self.input { types::Reason::DogAte(f) => { host_memory - .ptr(self.input_loc.ptr + 4) + .ptr(&bc, self.input_loc.ptr + 4) .write(f) .expect("input contents ref_mut"); } types::Reason::Traffic(v) => host_memory - .ptr(self.input_loc.ptr + 4) + .ptr(&bc, self.input_loc.ptr + 4) .write(v) .expect("input contents ref_mut"), types::Reason::Sleeping => {} // Do nothing @@ -129,6 +130,7 @@ impl GetTagExercise { let e = union_example::get_tag( &ctx, &host_memory, + &bc, self.input_loc.ptr as i32, self.return_loc.ptr as i32, ); @@ -136,7 +138,7 @@ impl GetTagExercise { assert_eq!(e, types::Errno::Ok.into(), "get_tag errno"); let return_val: types::Excuse = host_memory - .ptr(self.return_loc.ptr) + .ptr(&bc, self.return_loc.ptr) .read() .expect("return ref"); @@ -184,27 +186,28 @@ impl ReasonMultExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); + let bc = BorrowChecker::new(); let discriminant: u8 = reason_tag(&self.input).into(); host_memory - .ptr(self.input_loc.ptr) + .ptr(&bc, self.input_loc.ptr) .write(discriminant) .expect("input discriminant ref_mut"); host_memory - .ptr(self.input_loc.ptr + 4) + .ptr(&bc, self.input_loc.ptr + 4) .write(self.input_pointee_loc.ptr) .expect("input pointer ref_mut"); match self.input { types::Reason::DogAte(f) => { host_memory - .ptr(self.input_pointee_loc.ptr) + .ptr(&bc, self.input_pointee_loc.ptr) .write(f) .expect("input contents ref_mut"); } types::Reason::Traffic(v) => { host_memory - .ptr(self.input_pointee_loc.ptr) + .ptr(&bc, self.input_pointee_loc.ptr) .write(v) .expect("input contents ref_mut"); } @@ -213,6 +216,7 @@ impl ReasonMultExercise { let e = union_example::reason_mult( &ctx, &host_memory, + &bc, self.input_loc.ptr as i32, self.multiply_by as i32, ); @@ -222,7 +226,7 @@ impl ReasonMultExercise { match self.input { types::Reason::DogAte(f) => { let f_result: f32 = host_memory - .ptr(self.input_pointee_loc.ptr) + .ptr(&bc, self.input_pointee_loc.ptr) .read() .expect("input contents ref_mut"); assert_eq!( @@ -233,7 +237,7 @@ impl ReasonMultExercise { } types::Reason::Traffic(v) => { let v_result: i32 = host_memory - .ptr(self.input_pointee_loc.ptr) + .ptr(&bc, self.input_pointee_loc.ptr) .read() .expect("input contents ref_mut"); assert_eq!( diff --git a/crates/wiggle/tests/wasi.rs b/crates/wiggle/tests/wasi.rs index 5985d4b16b..6e04428627 100644 --- a/crates/wiggle/tests/wasi.rs +++ b/crates/wiggle/tests/wasi.rs @@ -1,4 +1,4 @@ -use wiggle::{GuestBorrows, GuestError, GuestErrorType, GuestPtr}; +use wiggle::{GuestError, GuestErrorType, GuestPtr, GuestSlice}; use wiggle_test::WasiCtx; // This test file exists to make sure that the entire `wasi.witx` file can be @@ -141,11 +141,11 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { // that we can use the wiggle API to create the datastructures we want // for efficient implementation of this function elsewhere. - let mut bc = GuestBorrows::new(); - let mut slices: Vec<&'_ mut [u8]> = Vec::new(); + let mut slices: Vec> = Vec::new(); + // XXX FIXME make sure this property still holds: // Mark the iov elements as borrowed, to ensure that they does not // overlap with any of the as_raw regions. - bc.borrow_slice(&iovs).expect("borrow iovec array"); + // bc.borrow_slice(&iovs).expect("borrow iovec array"); for iov_ptr in iovs.iter() { let iov_ptr = iov_ptr.expect("iovec element pointer is valid"); @@ -153,10 +153,14 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { let base: GuestPtr = iov.buf; let len: u32 = iov.buf_len; let buf: GuestPtr<[u8]> = base.as_array(len); - let slice = buf.as_raw(&mut bc).expect("borrow slice from iovec"); - slices.push(unsafe { &mut *slice }); + let slice = buf.as_slice().expect("borrow slice from iovec"); + slices.push(slice); } - println!("iovec slices: {:?}", slices); + println!("iovec slices: ["); + for slice in slices { + println!(" {:?},", &*slice); + } + println!("]"); unimplemented!("fd_pread") } From 73602c6bfe2922052e436a208e276e1f4ef8ead3 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 18 May 2020 18:56:30 -0700 Subject: [PATCH 02/19] borrow checker: reset index when empty, handle oom --- crates/wiggle/src/borrow.rs | 38 ++++++++++++++++++++----------------- crates/wiggle/src/error.rs | 2 ++ 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/crates/wiggle/src/borrow.rs b/crates/wiggle/src/borrow.rs index dddf86391c..c08ffea178 100644 --- a/crates/wiggle/src/borrow.rs +++ b/crates/wiggle/src/borrow.rs @@ -17,10 +17,7 @@ impl BorrowChecker { } } pub fn borrow(&self, r: Region) -> Result { - self.bc - .borrow_mut() - .borrow(r) - .ok_or_else(|| GuestError::PtrBorrowed(r)) + self.bc.borrow_mut().borrow(r) } pub fn unborrow(&self, h: BorrowHandle) { self.bc.borrow_mut().unborrow(h) @@ -45,19 +42,26 @@ impl InnerBorrowChecker { !self.borrows.values().all(|b| !b.overlaps(r)) } - fn new_handle(&mut self) -> BorrowHandle { + fn new_handle(&mut self) -> Result { + // Reset handles to 0 if all handles have been returned. + if self.borrows.is_empty() { + self.next_handle = BorrowHandle(0); + } let h = self.next_handle; - self.next_handle = BorrowHandle(h.0 + 1); - h + self.next_handle = BorrowHandle( + h.0.checked_add(1) + .ok_or_else(|| GuestError::BorrowCheckerOOM)?, + ); + Ok(h) } - fn borrow(&mut self, r: Region) -> Option { + fn borrow(&mut self, r: Region) -> Result { if self.is_borrowed(r) { - return None; + return Err(GuestError::PtrBorrowed(r)); } - let h = self.new_handle(); + let h = self.new_handle()?; self.borrows.insert(h, r); - Some(h) + Ok(h) } fn unborrow(&mut self, h: BorrowHandle) { @@ -92,28 +96,28 @@ mod test { let r2 = Region::new(9, 10); assert!(r1.overlaps(r2)); bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_none(), "cant borrow r2"); + assert!(bs.borrow(r2).is_err(), "cant borrow r2"); let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(0, 10); let r2 = Region::new(2, 5); assert!(r1.overlaps(r2)); bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_none(), "cant borrow r2"); + assert!(bs.borrow(r2).is_err(), "cant borrow r2"); let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(9, 10); let r2 = Region::new(0, 10); assert!(r1.overlaps(r2)); bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_none(), "cant borrow r2"); + assert!(bs.borrow(r2).is_err(), "cant borrow r2"); let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(2, 5); let r2 = Region::new(0, 10); assert!(r1.overlaps(r2)); bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_none(), "cant borrow r2"); + assert!(bs.borrow(r2).is_err(), "cant borrow r2"); let mut bs = InnerBorrowChecker::new(); let r1 = Region::new(2, 5); @@ -124,7 +128,7 @@ mod test { bs.borrow(r1).expect("can borrow r1"); bs.borrow(r2).expect("can borrow r2"); bs.borrow(r3).expect("can borrow r3"); - assert!(bs.borrow(r4).is_none(), "cant borrow r4"); + assert!(bs.borrow(r4).is_err(), "cant borrow r4"); } #[test] @@ -136,7 +140,7 @@ mod test { let _h1 = bs.borrow(r1).expect("can borrow r1"); let h2 = bs.borrow(r2).expect("can borrow r2"); - assert!(bs.borrow(r2).is_none(), "can't borrow r2 twice"); + assert!(bs.borrow(r2).is_err(), "can't borrow r2 twice"); bs.unborrow(h2); let _h3 = bs diff --git a/crates/wiggle/src/error.rs b/crates/wiggle/src/error.rs index a7bb43fa72..558f0c380a 100644 --- a/crates/wiggle/src/error.rs +++ b/crates/wiggle/src/error.rs @@ -15,6 +15,8 @@ pub enum GuestError { PtrNotAligned(Region, u32), #[error("Pointer already borrowed: {0:?}")] PtrBorrowed(Region), + #[error("Borrow checker out of memory")] + BorrowCheckerOOM, #[error("Slice length mismatch")] SliceLengthsDiffer, #[error("In func {funcname}:{location}:")] From 478cc68082ae4f69581315e129637d3f95ad1327 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 18 May 2020 19:03:51 -0700 Subject: [PATCH 03/19] wiggle: GuestType read and write must borrow --- crates/wiggle/src/guest_type.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/crates/wiggle/src/guest_type.rs b/crates/wiggle/src/guest_type.rs index 6cc1a877a1..a4a497c020 100644 --- a/crates/wiggle/src/guest_type.rs +++ b/crates/wiggle/src/guest_type.rs @@ -1,4 +1,4 @@ -use crate::{GuestError, GuestPtr}; +use crate::{region::Region, GuestError, GuestPtr}; use std::mem; /// A trait for types which are used to report errors. Each type used in the @@ -74,27 +74,39 @@ macro_rules! primitives { // size of our type as well as properly aligned. Consequently we // should be able to safely ready the pointer just after we // validated it, returning it along here. + let offset = ptr.offset(); + let size = Self::guest_size(); let host_ptr = ptr.mem().validate_size_align( - ptr.offset(), + offset, Self::guest_align(), - Self::guest_size(), + size, )?; - Ok(unsafe { *host_ptr.cast::() }) + let borrow_handle = ptr.borrow_checker().borrow( Region { + start: offset, + len: size, + })?; + let v = unsafe { *host_ptr.cast::() }; + ptr.borrow_checker().unborrow(borrow_handle); + Ok(v) } #[inline] fn write(ptr: &GuestPtr<'_, Self>, val: Self) -> Result<(), GuestError> { + let offset = ptr.offset(); + let size = Self::guest_size(); let host_ptr = ptr.mem().validate_size_align( - ptr.offset(), + offset, Self::guest_align(), - Self::guest_size(), + size, )?; - // Similar to above `as_raw` will do a lot of validation, and - // then afterwards we can safely write our value into the - // memory location. + let borrow_handle = ptr.borrow_checker().borrow( Region { + start: offset, + len: size, + })?; unsafe { *host_ptr.cast::() = val; } + ptr.borrow_checker().unborrow(borrow_handle); Ok(()) } } From be1df80c1b95dbbf2facfaa963b87e0d0a03c213 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 18 May 2020 19:08:23 -0700 Subject: [PATCH 04/19] wasi test: update explanation of safety --- crates/wiggle/tests/wasi.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/wiggle/tests/wasi.rs b/crates/wiggle/tests/wasi.rs index 6e04428627..823d5f84da 100644 --- a/crates/wiggle/tests/wasi.rs +++ b/crates/wiggle/tests/wasi.rs @@ -142,17 +142,16 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { // for efficient implementation of this function elsewhere. let mut slices: Vec> = Vec::new(); - // XXX FIXME make sure this property still holds: - // Mark the iov elements as borrowed, to ensure that they does not - // overlap with any of the as_raw regions. - // bc.borrow_slice(&iovs).expect("borrow iovec array"); for iov_ptr in iovs.iter() { let iov_ptr = iov_ptr.expect("iovec element pointer is valid"); + // Borrow checker will make sure the pointee of this read() doesn't overlap with any + // existing borrows: let iov: types::Iovec = iov_ptr.read().expect("read iovec element"); let base: GuestPtr = iov.buf; let len: u32 = iov.buf_len; let buf: GuestPtr<[u8]> = base.as_array(len); + // GuestSlice will remain borrowed until dropped: let slice = buf.as_slice().expect("borrow slice from iovec"); slices.push(slice); } From d221a3a3469ba7a4776ebd4e0ba6144e550d6943 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 18 May 2020 19:12:55 -0700 Subject: [PATCH 05/19] faster path for borrow-checking GuestPtr::{read, write} --- crates/wiggle/src/borrow.rs | 3 +++ crates/wiggle/src/guest_type.rs | 19 +++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/wiggle/src/borrow.rs b/crates/wiggle/src/borrow.rs index c08ffea178..61851e8f57 100644 --- a/crates/wiggle/src/borrow.rs +++ b/crates/wiggle/src/borrow.rs @@ -22,6 +22,9 @@ impl BorrowChecker { pub fn unborrow(&self, h: BorrowHandle) { self.bc.borrow_mut().unborrow(h) } + pub fn is_borrowed(&self, r: Region) -> bool { + self.bc.borrow().is_borrowed(r) + } } #[derive(Debug)] diff --git a/crates/wiggle/src/guest_type.rs b/crates/wiggle/src/guest_type.rs index a4a497c020..47a5a80e09 100644 --- a/crates/wiggle/src/guest_type.rs +++ b/crates/wiggle/src/guest_type.rs @@ -81,13 +81,14 @@ macro_rules! primitives { Self::guest_align(), size, )?; - let borrow_handle = ptr.borrow_checker().borrow( Region { + let region = Region { start: offset, len: size, - })?; - let v = unsafe { *host_ptr.cast::() }; - ptr.borrow_checker().unborrow(borrow_handle); - Ok(v) + }; + if ptr.borrow_checker().is_borrowed(region) { + return Err(GuestError::PtrBorrowed(region)); + } + Ok(unsafe { *host_ptr.cast::() }) } #[inline] @@ -99,14 +100,16 @@ macro_rules! primitives { Self::guest_align(), size, )?; - let borrow_handle = ptr.borrow_checker().borrow( Region { + let region = Region { start: offset, len: size, - })?; + }; + if ptr.borrow_checker().is_borrowed(region) { + return Err(GuestError::PtrBorrowed(region)); + } unsafe { *host_ptr.cast::() = val; } - ptr.borrow_checker().unborrow(borrow_handle); Ok(()) } } From a4c1079b50c7a66aac1e4bb03d4c652365774549 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 18 May 2020 19:15:04 -0700 Subject: [PATCH 06/19] borrow checker: add method to check that its empty --- crates/wiggle/src/borrow.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/wiggle/src/borrow.rs b/crates/wiggle/src/borrow.rs index 61851e8f57..992ecddeb5 100644 --- a/crates/wiggle/src/borrow.rs +++ b/crates/wiggle/src/borrow.rs @@ -25,6 +25,9 @@ impl BorrowChecker { pub fn is_borrowed(&self, r: Region) -> bool { self.bc.borrow().is_borrowed(r) } + pub fn is_empty(&self) -> bool { + self.bc.borrow().is_empty() + } } #[derive(Debug)] @@ -41,6 +44,10 @@ impl InnerBorrowChecker { } } + fn is_empty(&self) -> bool { + self.borrows.is_empty() + } + fn is_borrowed(&self, r: Region) -> bool { !self.borrows.values().all(|b| !b.overlaps(r)) } From c30194dfa1f49036fe9b95517b85bf0062bdba55 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 19 May 2020 13:27:43 -0700 Subject: [PATCH 07/19] document BorrowChecker, make creation unsafe --- crates/wiggle/src/borrow.rs | 42 +++++++++++++++++++++++++-------- crates/wiggle/tests/arrays.rs | 4 ++-- crates/wiggle/tests/atoms.rs | 4 ++-- crates/wiggle/tests/flags.rs | 2 +- crates/wiggle/tests/handles.rs | 2 +- crates/wiggle/tests/ints.rs | 2 +- crates/wiggle/tests/pointers.rs | 2 +- crates/wiggle/tests/strings.rs | 4 ++-- crates/wiggle/tests/structs.rs | 10 ++++---- crates/wiggle/tests/union.rs | 4 ++-- 10 files changed, 49 insertions(+), 27 deletions(-) diff --git a/crates/wiggle/src/borrow.rs b/crates/wiggle/src/borrow.rs index 992ecddeb5..2519f44dae 100644 --- a/crates/wiggle/src/borrow.rs +++ b/crates/wiggle/src/borrow.rs @@ -11,23 +11,36 @@ pub struct BorrowChecker { } impl BorrowChecker { - pub fn new() -> Self { + /// A `BorrowChecker` manages run-time validation of borrows from a `GuestMemory`. It keeps + /// track of regions of guest memory which are possible to alias with Rust references (via the + /// `GuestSlice` and `GuestStr` structs, which implement `std::ops::Deref` and + /// `std::ops::DerefMut`. It also enforces that `GuestPtr::read` and `GuestPtr::write` do not + /// access memory with an outstanding borrow. + /// The safety of this mechanism depends on creating exactly one `BorrowChecker` per + /// WebAssembly memory. There must be no other reads or writes of WebAssembly the memory by + /// either Rust or WebAssembly code while there are any outstanding borrows, as given by + /// `BorrowChecker::has_outstanding_borrows()`. + pub unsafe fn new() -> Self { BorrowChecker { bc: RefCell::new(InnerBorrowChecker::new()), } } - pub fn borrow(&self, r: Region) -> Result { + /// Indicates whether any outstanding borrows are known to the `BorrowChecker`. This function + /// must be `false` in order for it to be safe to recursively call into a WebAssembly module, + /// or to manipulate the WebAssembly memory by any other means. + pub fn has_outstanding_borrows(&self) -> bool { + self.bc.borrow().has_outstanding_borrows() + } + + pub(crate) fn borrow(&self, r: Region) -> Result { self.bc.borrow_mut().borrow(r) } - pub fn unborrow(&self, h: BorrowHandle) { + pub(crate) fn unborrow(&self, h: BorrowHandle) { self.bc.borrow_mut().unborrow(h) } - pub fn is_borrowed(&self, r: Region) -> bool { + pub(crate) fn is_borrowed(&self, r: Region) -> bool { self.bc.borrow().is_borrowed(r) } - pub fn is_empty(&self) -> bool { - self.bc.borrow().is_empty() - } } #[derive(Debug)] @@ -44,8 +57,8 @@ impl InnerBorrowChecker { } } - fn is_empty(&self) -> bool { - self.borrows.is_empty() + fn has_outstanding_borrows(&self) -> bool { + !self.borrows.is_empty() } fn is_borrowed(&self, r: Region) -> bool { @@ -147,11 +160,20 @@ mod test { let r1 = Region::new(0, 10); let r2 = Region::new(10, 10); assert!(!r1.overlaps(r2)); - let _h1 = bs.borrow(r1).expect("can borrow r1"); + assert_eq!(bs.has_outstanding_borrows(), false, "start with no borrows"); + let h1 = bs.borrow(r1).expect("can borrow r1"); + assert_eq!(bs.has_outstanding_borrows(), true, "h1 is outstanding"); let h2 = bs.borrow(r2).expect("can borrow r2"); assert!(bs.borrow(r2).is_err(), "can't borrow r2 twice"); bs.unborrow(h2); + assert_eq!( + bs.has_outstanding_borrows(), + true, + "h1 is still outstanding" + ); + bs.unborrow(h1); + assert_eq!(bs.has_outstanding_borrows(), false, "no remaining borrows"); let _h3 = bs .borrow(r2) diff --git a/crates/wiggle/tests/arrays.rs b/crates/wiggle/tests/arrays.rs index 61b47ba416..4fba074667 100644 --- a/crates/wiggle/tests/arrays.rs +++ b/crates/wiggle/tests/arrays.rs @@ -77,7 +77,7 @@ impl ReduceExcusesExcercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; // Populate memory with pointers to generated Excuse values for (&excuse, ptr) in self.excuse_values.iter().zip(self.excuse_ptr_locs.iter()) { @@ -169,7 +169,7 @@ impl PopulateExcusesExcercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; // Populate array with valid pointers to Excuse type in memory let ptr = host_memory.ptr::<[GuestPtr<'_, types::Excuse>]>( diff --git a/crates/wiggle/tests/atoms.rs b/crates/wiggle/tests/atoms.rs index 674fdaa5d5..3497215f0c 100644 --- a/crates/wiggle/tests/atoms.rs +++ b/crates/wiggle/tests/atoms.rs @@ -31,7 +31,7 @@ impl IntFloatExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; let e = atoms::int_float_args(&ctx, &host_memory, &bc, self.an_int as i32, self.an_float); @@ -61,7 +61,7 @@ impl DoubleIntExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; let e = atoms::double_int_return_float( &ctx, diff --git a/crates/wiggle/tests/flags.rs b/crates/wiggle/tests/flags.rs index 33f47c8ccc..727a9262bf 100644 --- a/crates/wiggle/tests/flags.rs +++ b/crates/wiggle/tests/flags.rs @@ -65,7 +65,7 @@ impl ConfigureCarExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; // Populate input ptr host_memory diff --git a/crates/wiggle/tests/handles.rs b/crates/wiggle/tests/handles.rs index 292e689eb4..8b066c7aa5 100644 --- a/crates/wiggle/tests/handles.rs +++ b/crates/wiggle/tests/handles.rs @@ -34,7 +34,7 @@ impl HandleExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; let e = handle_examples::fd_create(&ctx, &host_memory, &bc, self.return_loc.ptr as i32); diff --git a/crates/wiggle/tests/ints.rs b/crates/wiggle/tests/ints.rs index 20d30ff86d..db47ea469d 100644 --- a/crates/wiggle/tests/ints.rs +++ b/crates/wiggle/tests/ints.rs @@ -46,7 +46,7 @@ impl CookieCutterExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; let res = ints::cookie_cutter( &ctx, diff --git a/crates/wiggle/tests/pointers.rs b/crates/wiggle/tests/pointers.rs index 2a63507900..74ca3a1fa2 100644 --- a/crates/wiggle/tests/pointers.rs +++ b/crates/wiggle/tests/pointers.rs @@ -128,7 +128,7 @@ impl PointersAndEnumsExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; host_memory .ptr(&bc, self.input2_loc.ptr) diff --git a/crates/wiggle/tests/strings.rs b/crates/wiggle/tests/strings.rs index 51c5b4fde2..aa34312a86 100644 --- a/crates/wiggle/tests/strings.rs +++ b/crates/wiggle/tests/strings.rs @@ -69,7 +69,7 @@ impl HelloStringExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; // Populate string in guest's memory let ptr = @@ -178,7 +178,7 @@ impl MultiStringExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; let write_string = |val: &str, loc: MemArea| { let ptr = host_memory.ptr::(&bc, (loc.ptr, val.len() as u32)); diff --git a/crates/wiggle/tests/structs.rs b/crates/wiggle/tests/structs.rs index accba503f7..4861239ac2 100644 --- a/crates/wiggle/tests/structs.rs +++ b/crates/wiggle/tests/structs.rs @@ -83,7 +83,7 @@ impl SumOfPairExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; host_memory .ptr(&bc, self.input_loc.ptr) @@ -173,7 +173,7 @@ impl SumPairPtrsExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; host_memory .ptr(&bc, self.input_first_loc.ptr) @@ -259,7 +259,7 @@ impl SumIntAndPtrExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; host_memory .ptr(&bc, self.input_first_loc.ptr) @@ -318,7 +318,7 @@ impl ReturnPairInts { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; let err = structs::return_pair_ints(&ctx, &host_memory, &bc, self.return_loc.ptr as i32); @@ -384,7 +384,7 @@ impl ReturnPairPtrsExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; host_memory .ptr(&bc, self.input_first_loc.ptr) diff --git a/crates/wiggle/tests/union.rs b/crates/wiggle/tests/union.rs index 5a6e2b3a42..55894ba49a 100644 --- a/crates/wiggle/tests/union.rs +++ b/crates/wiggle/tests/union.rs @@ -107,7 +107,7 @@ impl GetTagExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; let discriminant: u8 = reason_tag(&self.input).into(); host_memory @@ -186,7 +186,7 @@ impl ReasonMultExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = BorrowChecker::new(); + let bc = unsafe { BorrowChecker::new() }; let discriminant: u8 = reason_tag(&self.input).into(); host_memory From 056a7d07298f85121a57f78fbe028e816ad97801 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 19 May 2020 17:35:16 -0700 Subject: [PATCH 08/19] wiggle: redo docs for auto borrow checking --- crates/wiggle/src/lib.rs | 124 ++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 73 deletions(-) diff --git a/crates/wiggle/src/lib.rs b/crates/wiggle/src/lib.rs index 1901bfa9dc..45479c8644 100644 --- a/crates/wiggle/src/lib.rs +++ b/crates/wiggle/src/lib.rs @@ -53,37 +53,24 @@ pub use region::Region; /// must be "somehow nonzero in length" to allow users of `GuestMemory` and /// `GuestPtr` to safely read and write interior data. /// -/// # Using Raw Pointers /// -/// Methods like [`GuestMemory::base`] or [`GuestPtr::as_raw`] will return raw -/// pointers to use. Returning raw pointers is significant because it shows -/// there are hazards with using the returned pointers, and they can't blanket -/// be used in a safe fashion. It is possible to use these pointers safely, but -/// any usage needs to uphold a few guarantees. +/// # Using References /// -/// * Whenever a `*mut T` is accessed or modified, it must be guaranteed that -/// since the pointer was originally obtained the guest memory wasn't -/// relocated in any way. This means you can't call back into the guest, call -/// other arbitrary functions which might call into the guest, etc. The -/// problem here is that the guest could execute instructions like -/// `memory.grow` which would invalidate the raw pointer. If, however, after -/// you acquire `*mut T` you only execute your own code and it doesn't touch -/// the guest, then `*mut T` is still guaranteed to point to valid code. +/// See the safety guarantees of [`BorrowChecker`], which asserts that exactly +/// one `BorrowChecker` may be constructed for each WebAssembly memory. /// -/// * Furthermore, Rust's aliasing rules must still be upheld. For example you -/// can't have two `&mut T` types that point to the area or overlap in any -/// way. This in particular becomes an issue when you're dealing with multiple -/// `GuestPtr` types. If you want to simultaneously work with them then you -/// need to dynamically validate that you're either working with them all in a -/// shared fashion (e.g. as if they were `&T`) or you must verify that they do -/// not overlap to work with them as `&mut T`. +/// The [`GuestMemory::as_slice`] or [`GuestPtr::as_str`] will return smart +/// pointers [`GuestSlice`] and [`GuestStr`]. These types, which implement +/// [`std::ops::Deref`] and [`std::ops::DerefMut`], provide mutable references +/// into the memory region given by a `GuestMemory`. /// -/// Note that safely using the raw pointers is relatively difficult. This crate -/// strives to provide utilities to safely work with guest pointers so long as -/// the previous guarantees are all upheld. If advanced operations are done with -/// guest pointers it's recommended to be extremely cautious and thoroughly -/// consider possible ramifications with respect to this API before codifying -/// implementation details. +/// These smart pointers are dynamically borrow-checked by the `BorrowChecker` +/// passed to the wiggle-generated ABI-level functions. While a `GuestSlice` +/// or a `GuestStr` are live, the [`BorrowChecker::has_outstanding_borrows()`] +/// method will always return `true`. If you need to re-enter the guest or +/// otherwise read or write to the contents of a WebAssembly memory, all +/// `GuestSlice`s and `GuestStr`s for the memory must be dropped, at which +/// point `BorrowChecker::has_outstanding_borrows()` will return `false`. pub unsafe trait GuestMemory { /// Returns the base allocation of this guest memory, located in host /// memory. @@ -208,8 +195,12 @@ unsafe impl GuestMemory for Arc { /// Note that the type parameter does not need to implement the `Sized` trait, /// so you can implement types such as this: /// -/// * `GuestPtr<'_, str>` - a pointer to a guest string -/// * `GuestPtr<'_, [T]>` - a pointer to a guest array +/// * `GuestPtr<'_, str>` - a pointer to a guest string. Has the method +/// [`GuestPtr::as_str`], which gives a dynamically borrow-checked +/// `GuestStr<'_>`, which `DerefMut`s to a `&mut str`. +/// * `GuestPtr<'_, [T]>` - a pointer to a guest array. Has the method +/// [`GuestPtr::as_slice`], which gives a dynamically borrow-checked +/// `GuestSlice<'_, T>`, which `DerefMut`s to a `&mut [T]`. /// /// Unsized types such as this may have extra methods and won't have methods /// like [`GuestPtr::read`] or [`GuestPtr::write`]. @@ -398,23 +389,15 @@ impl<'a, T> GuestPtr<'a, [T]> { (0..self.len()).map(move |i| base.add(i)) } - /// Attempts to read a raw `*mut [T]` pointer from this pointer, performing - /// bounds checks and type validation. - /// The resulting `*mut [T]` can be used as a `&mut [t]` as long as the - /// reference is dropped before any Wasm code is re-entered. + /// Attempts to create a [`GuestSlice<'_, T>`] from this pointer, performing + /// bounds checks and type validation. The `GuestSlice` is a smart pointer + /// that can be used as a `&[T]` or a `&mut [T]` via the `Deref` and `DerefMut` + /// traits. The region of memory backing the slice will be marked as borrowed + /// by the [`BorrowChecker`] until the `GuestSlice` is dropped. /// - /// This function will return a raw pointer into host memory if all checks - /// succeed (valid utf-8, valid pointers, etc). If any checks fail then - /// `GuestError` will be returned. - /// - /// Note that the `*mut [T]` pointer is still unsafe to use in general, but - /// there are specific situations that it is safe to use. For more - /// information about using the raw pointer, consult the [`GuestMemory`] - /// trait documentation. - /// - /// For safety against overlapping mutable borrows, the user must use the - /// same `GuestBorrows` to create all `*mut str` or `*mut [T]` that are alive - /// at the same time. + /// This function will return a `GuestSlice` into host memory if all checks + /// succeed (valid utf-8, valid pointers, memory is not borrowed, etc). If + /// any checks fail then `GuestError` will be returned. pub fn as_slice(&self) -> Result, GuestError> where T: GuestTypeTransparent<'a>, @@ -438,12 +421,8 @@ impl<'a, T> GuestPtr<'a, [T]> { T::validate(unsafe { ptr.add(offs as usize) })?; } - // SAFETY: iff there are no overlapping borrows (all uses of as_raw use this same - // GuestBorrows), its valid to construct a *mut [T] - let ptr = unsafe { - let s = slice::from_raw_parts_mut(ptr, self.pointer.1 as usize); - s as *mut [T] - }; + // SAFETY: iff there are no overlapping borrows it is valid to construct a &mut [T] + let ptr = unsafe { slice::from_raw_parts_mut(ptr, self.pointer.1 as usize) }; Ok(GuestSlice { ptr, @@ -504,23 +483,15 @@ impl<'a> GuestPtr<'a, str> { GuestPtr::new(self.mem, self.bc, self.pointer) } - /// Attempts to read a raw `*mut str` pointer from this pointer, performing - /// bounds checks and utf-8 checks. - /// The resulting `*mut str` can be used as a `&mut str` as long as the - /// reference is dropped before any Wasm code is re-entered. + /// Attempts to create a [`GuestStr<'_>`] from this pointer, performing + /// bounds checks and utf-8 checks. The resulting `GuestStr` can be used + /// as a `&str` or `&mut str` via the `Deref` and `DerefMut` traits. The + /// region of memory backing the `str` will be marked as borrowed by the + /// [`BorrowChecker`] until the `GuestStr` is dropped. /// - /// This function will return a raw pointer into host memory if all checks + /// This function will return `GuestStr` into host memory if all checks /// succeed (valid utf-8, valid pointers, etc). If any checks fail then /// `GuestError` will be returned. - /// - /// Note that the `*mut str` pointer is still unsafe to use in general, but - /// there are specific situations that it is safe to use. For more - /// information about using the raw pointer, consult the [`GuestMemory`] - /// trait documentation. - /// - /// For safety against overlapping mutable borrows, the user must use the - /// same `GuestBorrows` to create all `*mut str` or `*mut [T]` that are - /// alive at the same time. pub fn as_str(&self) -> Result, GuestError> { let ptr = self .mem @@ -531,9 +502,10 @@ impl<'a> GuestPtr<'a, str> { len: self.pointer.1, })?; - // SAFETY: iff there are no overlapping borrows (all uses of as_raw use this same - // GuestBorrows), its valid to construct a *mut str + // SAFETY: iff there are no overlapping borrows it is ok to construct + // a &mut str. let ptr = unsafe { slice::from_raw_parts_mut(ptr, self.pointer.1 as usize) }; + // Validate that contents are utf-8: match str::from_utf8_mut(ptr) { Ok(ptr) => Ok(GuestStr { ptr, @@ -559,8 +531,11 @@ impl fmt::Debug for GuestPtr<'_, T> { } } +/// A smart pointer to a mutable slice in guest memory. +/// Usable as a `&'a [T]` via [`std::ops::Deref`] and as a `&'a mut [T]` via +/// [`std::ops::DerefMut`]. pub struct GuestSlice<'a, T> { - ptr: *mut [T], + ptr: &'a mut [T], bc: &'a BorrowChecker, borrow: BorrowHandle, } @@ -568,13 +543,13 @@ pub struct GuestSlice<'a, T> { impl<'a, T> std::ops::Deref for GuestSlice<'a, T> { type Target = [T]; fn deref(&self) -> &Self::Target { - unsafe { self.ptr.as_ref().expect("ptr guaranteed to be non-null") } + self.ptr } } impl<'a, T> std::ops::DerefMut for GuestSlice<'a, T> { fn deref_mut(&mut self) -> &mut Self::Target { - unsafe { self.ptr.as_mut().expect("ptr guaranteed to be non-null") } + self.ptr } } @@ -584,8 +559,11 @@ impl<'a, T> Drop for GuestSlice<'a, T> { } } +/// A smart pointer to a mutable `str` in guest memory. +/// Usable as a `&'a str` via [`std::ops::Deref`] and as a `&'a mut str` via +/// [`std::ops::DerefMut`]. pub struct GuestStr<'a> { - ptr: *mut str, + ptr: &'a mut str, bc: &'a BorrowChecker, borrow: BorrowHandle, } @@ -593,13 +571,13 @@ pub struct GuestStr<'a> { impl<'a> std::ops::Deref for GuestStr<'a> { type Target = str; fn deref(&self) -> &Self::Target { - unsafe { self.ptr.as_ref().expect("ptr guaranteed to be non-null") } + self.ptr } } impl<'a> std::ops::DerefMut for GuestStr<'a> { fn deref_mut(&mut self) -> &mut Self::Target { - unsafe { self.ptr.as_mut().expect("ptr guaranteed to be non-null") } + self.ptr } } From 3643f2d164ff56ed5ff514e781771152cbe58448 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 19 May 2020 18:13:02 -0700 Subject: [PATCH 09/19] wig: create a BorrowChecker just to get things compiling. Needs fixing. --- crates/wasi-common/wig/src/wasi.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/wasi-common/wig/src/wasi.rs b/crates/wasi-common/wig/src/wasi.rs index 39e263c7a6..0a0acf60f1 100644 --- a/crates/wasi-common/wig/src/wasi.rs +++ b/crates/wasi-common/wig/src/wasi.rs @@ -468,9 +468,17 @@ pub fn define_struct_for_wiggle(args: TokenStream) -> TokenStream { } }; let mem: WasiMemory = mem.into(); + // FIXME: + // Currently, none of the wasi-common functions will re-enter the guest, so + // creating a BorrowChecker here ensures there is just one per the + // WebAssembly memory. I'd like a wasmtime expert to show me how to + // create a BorrowChecker once per Store, and access it in this + // context, though. + let bc = wiggle::BorrowChecker::new(); wasi_common::wasi::#module_id::#name_ident( &mut my_cx.borrow_mut(), &mem, + &bc, #(#hostcall_args),* ) #cvt_ret } From 04fb4acc6a0850ab51fc6d1b4d7d1623ce078092 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 19 May 2020 18:13:20 -0700 Subject: [PATCH 10/19] wasi-common: WIP translating to use automated borrow checking --- crates/wasi-common/src/path.rs | 15 ++-- .../src/snapshots/wasi_snapshot_preview1.rs | 86 ++++++------------- 2 files changed, 34 insertions(+), 67 deletions(-) diff --git a/crates/wasi-common/src/path.rs b/crates/wasi-common/src/path.rs index f5399a7b7e..1f27b7a205 100644 --- a/crates/wasi-common/src/path.rs +++ b/crates/wasi-common/src/path.rs @@ -3,7 +3,7 @@ use crate::handle::{Handle, HandleRights}; use crate::wasi::{types, Errno, Result}; use std::path::{Component, Path}; use std::str; -use wiggle::{GuestBorrows, GuestPtr}; +use wiggle::GuestPtr; pub(crate) use crate::sys::path::{from_host, open_rights}; @@ -14,19 +14,15 @@ pub(crate) fn get( entry: &Entry, required_rights: &HandleRights, dirflags: types::Lookupflags, - path: &GuestPtr<'_, str>, + path_ptr: &GuestPtr<'_, str>, needs_final_component: bool, ) -> Result<(Box, String)> { const MAX_SYMLINK_EXPANSIONS: usize = 128; // Extract path as &str from guest's memory. - let path = unsafe { - let mut bc = GuestBorrows::new(); - let raw = path.as_raw(&mut bc)?; - &*raw - }; + let path = path_ptr.as_str()?; - log::trace!(" | (path_ptr,path_len)='{}'", path); + log::trace!(" | (path_ptr,path_len)='{}'", &*path); if path.contains('\0') { // if contains NUL, return Ilseq @@ -51,6 +47,9 @@ pub(crate) fn get( // any symlinks we encounter are processed by pushing them on the stack. let mut path_stack = vec![path.to_owned()]; + // No longer need a borrow into the guest memory for `path`: + drop(path); + // Track the number of symlinks we've expanded, so we can return `ELOOP` after too many. let mut symlink_expansions = 0; diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index 31528beb39..8de648de52 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -8,7 +8,7 @@ use crate::{path, poll}; use log::{debug, error, trace}; use std::convert::TryInto; use std::io::{self, SeekFrom}; -use wiggle::{GuestBorrows, GuestPtr}; +use wiggle::{GuestPtr, GuestSlice}; impl<'a> WasiSnapshotPreview1 for WasiCtx { fn args_get<'b>( @@ -199,18 +199,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { iovs: &types::IovecArray<'_>, offset: types::Filesize, ) -> Result { - let mut buf = Vec::new(); - let mut bc = GuestBorrows::new(); - bc.borrow_slice(iovs)?; + let mut guest_slices: Vec> = Vec::new(); for iov_ptr in iovs.iter() { let iov_ptr = iov_ptr?; let iov: types::Iovec = iov_ptr.read()?; - let slice = unsafe { - let buf = iov.buf.as_array(iov.buf_len); - let raw = buf.as_raw(&mut bc)?; - &mut *raw - }; - buf.push(io::IoSliceMut::new(slice)); + guest_slices.push(iov.buf.as_array(iov.buf_len).as_slice()?); } let required_rights = @@ -219,10 +212,18 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { if offset > i64::max_value() as u64 { return Err(Errno::Io); } - let host_nread = entry - .as_handle(&required_rights)? - .preadv(&mut buf, offset)? - .try_into()?; + + let host_nread = { + let mut buf = guest_slices + .iter() + .map(|s| io::IoSliceMut::new(&mut *s)) + .collect::>>(); + entry + .as_handle(&required_rights)? + .preadv(&mut buf, offset)? + .try_into()? + }; + drop(guest_slices); Ok(host_nread) } @@ -276,16 +277,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { offset: types::Filesize, ) -> Result { let mut buf = Vec::new(); - let mut bc = GuestBorrows::new(); - bc.borrow_slice(ciovs)?; for ciov_ptr in ciovs.iter() { let ciov_ptr = ciov_ptr?; let ciov: types::Ciovec = ciov_ptr.read()?; - let slice = unsafe { - let buf = ciov.buf.as_array(ciov.buf_len); - let raw = buf.as_raw(&mut bc)?; - &*raw - }; + let guest_slice: GuestSlice = ciov.buf.as_array(ciov.buf_len).as_slice()?; + let slice: &[u8] = &*guest_slice; buf.push(io::IoSlice::new(slice)); } @@ -305,17 +301,12 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } fn fd_read(&self, fd: types::Fd, iovs: &types::IovecArray<'_>) -> Result { - let mut bc = GuestBorrows::new(); let mut slices = Vec::new(); - bc.borrow_slice(&iovs)?; for iov_ptr in iovs.iter() { let iov_ptr = iov_ptr?; let iov: types::Iovec = iov_ptr.read()?; - let slice = unsafe { - let buf = iov.buf.as_array(iov.buf_len); - let raw = buf.as_raw(&mut bc)?; - &mut *raw - }; + let guest_slice = iov.buf.as_array(iov.buf_len).as_slice()?; + let slice = &mut *guest_slice; slices.push(io::IoSliceMut::new(slice)); } @@ -423,18 +414,12 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } fn fd_write(&self, fd: types::Fd, ciovs: &types::CiovecArray<'_>) -> Result { - let mut bc = GuestBorrows::new(); let mut slices = Vec::new(); - bc.borrow_slice(&ciovs)?; for ciov_ptr in ciovs.iter() { let ciov_ptr = ciov_ptr?; let ciov: types::Ciovec = ciov_ptr.read()?; - let slice = unsafe { - let buf = ciov.buf.as_array(ciov.buf_len); - let raw = buf.as_raw(&mut bc)?; - &*raw - }; - slices.push(io::IoSlice::new(slice)); + let slice = ciov.buf.as_array(ciov.buf_len).as_slice()?; + slices.push(io::IoSlice::new(&*slice)); } let required_rights = HandleRights::from_base(types::Rights::FD_WRITE); let entry = self.get_entry(fd)?; @@ -596,13 +581,8 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { path, false, )?; - let slice = unsafe { - let mut bc = GuestBorrows::new(); - let buf = buf.as_array(buf_len); - let raw = buf.as_raw(&mut bc)?; - &mut *raw - }; - let host_bufused = dirfd.readlink(&path, slice)?.try_into()?; + let slice = buf.as_array(buf_len).as_slice()?; + let host_bufused = dirfd.readlink(&path, &mut *slice)?.try_into()?; Ok(host_bufused) } @@ -662,12 +642,8 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { new_path, true, )?; - let old_path = unsafe { - let mut bc = GuestBorrows::new(); - let raw = old_path.as_raw(&mut bc)?; - &*raw - }; - trace!(" | old_path='{}'", old_path); + let old_path = old_path.as_str()?; + trace!(" | old_path='{}'", &*old_path); new_fd.symlink(&old_path, &new_path) } @@ -696,9 +672,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } let mut subscriptions = Vec::new(); - let mut bc = GuestBorrows::new(); let subs = in_.as_array(nsubscriptions); - bc.borrow_slice(&subs)?; for sub_ptr in subs.iter() { let sub_ptr = sub_ptr?; let sub: types::Subscription = sub_ptr.read()?; @@ -793,7 +767,6 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { let nevents = events.len().try_into()?; let out_events = out.as_array(nevents); - bc.borrow_slice(&out_events)?; for (event, event_ptr) in events.into_iter().zip(out_events.iter()) { let event_ptr = event_ptr?; event_ptr.write(event)?; @@ -820,13 +793,8 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } fn random_get(&self, buf: &GuestPtr, buf_len: types::Size) -> Result<()> { - let slice = unsafe { - let mut bc = GuestBorrows::new(); - let buf = buf.as_array(buf_len); - let raw = buf.as_raw(&mut bc)?; - &mut *raw - }; - getrandom::getrandom(slice).map_err(|err| { + let slice = buf.as_array(buf_len).as_slice()?; + getrandom::getrandom(&mut *slice).map_err(|err| { error!("getrandom failure: {:?}", err); Errno::Io }) From b130a64d195e2d2970766aecd279fc6dfdb327a5 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 20 May 2020 09:43:49 -0700 Subject: [PATCH 11/19] wasi-common fixes --- .../src/snapshots/wasi_snapshot_preview1.rs | 61 +++++++++++-------- crates/wasi-common/src/wasi.rs | 1 + 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index 8de648de52..49ae8270e0 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -215,7 +215,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { let host_nread = { let mut buf = guest_slices - .iter() + .iter_mut() .map(|s| io::IoSliceMut::new(&mut *s)) .collect::>>(); entry @@ -276,13 +276,11 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { ciovs: &types::CiovecArray<'_>, offset: types::Filesize, ) -> Result { - let mut buf = Vec::new(); + let mut guest_slices = Vec::new(); for ciov_ptr in ciovs.iter() { let ciov_ptr = ciov_ptr?; let ciov: types::Ciovec = ciov_ptr.read()?; - let guest_slice: GuestSlice = ciov.buf.as_array(ciov.buf_len).as_slice()?; - let slice: &[u8] = &*guest_slice; - buf.push(io::IoSlice::new(slice)); + guest_slices.push(ciov.buf.as_array(ciov.buf_len).as_slice()?); } let required_rights = @@ -293,29 +291,37 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { return Err(Errno::Io); } - let host_nwritten = entry - .as_handle(&required_rights)? - .pwritev(&buf, offset)? - .try_into()?; + let host_nwritten = { + let buf: Vec = + guest_slices.iter().map(|s| io::IoSlice::new(&*s)).collect(); + entry + .as_handle(&required_rights)? + .pwritev(&buf, offset)? + .try_into()? + }; Ok(host_nwritten) } fn fd_read(&self, fd: types::Fd, iovs: &types::IovecArray<'_>) -> Result { - let mut slices = Vec::new(); + let mut guest_slices = Vec::new(); for iov_ptr in iovs.iter() { let iov_ptr = iov_ptr?; let iov: types::Iovec = iov_ptr.read()?; - let guest_slice = iov.buf.as_array(iov.buf_len).as_slice()?; - let slice = &mut *guest_slice; - slices.push(io::IoSliceMut::new(slice)); + guest_slices.push(iov.buf.as_array(iov.buf_len).as_slice()?); } let required_rights = HandleRights::from_base(types::Rights::FD_READ); let entry = self.get_entry(fd)?; - let host_nread = entry - .as_handle(&required_rights)? - .read_vectored(&mut slices)? - .try_into()?; + let host_nread = { + let mut slices: Vec = guest_slices + .iter_mut() + .map(|s| io::IoSliceMut::new(&mut *s)) + .collect(); + entry + .as_handle(&required_rights)? + .read_vectored(&mut slices)? + .try_into()? + }; Ok(host_nread) } @@ -414,19 +420,22 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } fn fd_write(&self, fd: types::Fd, ciovs: &types::CiovecArray<'_>) -> Result { - let mut slices = Vec::new(); + let mut guest_slices = Vec::new(); for ciov_ptr in ciovs.iter() { let ciov_ptr = ciov_ptr?; let ciov: types::Ciovec = ciov_ptr.read()?; - let slice = ciov.buf.as_array(ciov.buf_len).as_slice()?; - slices.push(io::IoSlice::new(&*slice)); + guest_slices.push(ciov.buf.as_array(ciov.buf_len).as_slice()?); } let required_rights = HandleRights::from_base(types::Rights::FD_WRITE); let entry = self.get_entry(fd)?; - let host_nwritten = entry - .as_handle(&required_rights)? - .write_vectored(&slices)? - .try_into()?; + let host_nwritten = { + let slices: Vec = + guest_slices.iter().map(|s| io::IoSlice::new(&*s)).collect(); + entry + .as_handle(&required_rights)? + .write_vectored(&slices)? + .try_into()? + }; Ok(host_nwritten) } @@ -581,7 +590,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { path, false, )?; - let slice = buf.as_array(buf_len).as_slice()?; + let mut slice = buf.as_array(buf_len).as_slice()?; let host_bufused = dirfd.readlink(&path, &mut *slice)?.try_into()?; Ok(host_bufused) } @@ -793,7 +802,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { } fn random_get(&self, buf: &GuestPtr, buf_len: types::Size) -> Result<()> { - let slice = buf.as_array(buf_len).as_slice()?; + let mut slice = buf.as_array(buf_len).as_slice()?; getrandom::getrandom(&mut *slice).map_err(|err| { error!("getrandom failure: {:?}", err); Errno::Io diff --git a/crates/wasi-common/src/wasi.rs b/crates/wasi-common/src/wasi.rs index 53dbd5476b..4ab77ec3b2 100644 --- a/crates/wasi-common/src/wasi.rs +++ b/crates/wasi-common/src/wasi.rs @@ -40,6 +40,7 @@ impl From for Errno { InFunc { .. } => Self::Inval, InDataField { .. } => Self::Inval, SliceLengthsDiffer { .. } => Self::Fault, + BorrowCheckerOOM { .. } => Self::Fault, } } } From 37514b730f154d1ff48d00d458a42956fa22b3ba Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 20 May 2020 12:29:00 -0700 Subject: [PATCH 12/19] wig: alex has convinced me that this BorrowChecker creation is correct --- crates/wasi-common/wig/src/wasi.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/wasi-common/wig/src/wasi.rs b/crates/wasi-common/wig/src/wasi.rs index 0a0acf60f1..243334c5be 100644 --- a/crates/wasi-common/wig/src/wasi.rs +++ b/crates/wasi-common/wig/src/wasi.rs @@ -468,12 +468,10 @@ pub fn define_struct_for_wiggle(args: TokenStream) -> TokenStream { } }; let mem: WasiMemory = mem.into(); - // FIXME: - // Currently, none of the wasi-common functions will re-enter the guest, so - // creating a BorrowChecker here ensures there is just one per the - // WebAssembly memory. I'd like a wasmtime expert to show me how to - // create a BorrowChecker once per Store, and access it in this - // context, though. + // Wiggle does not expose any methods for functions to re-enter the + // WebAssembly module, or expose the memory via non-wiggle mechanisms. + // Therefore, creating a new BorrowChecker at the root of each function + // invocation is correct. let bc = wiggle::BorrowChecker::new(); wasi_common::wasi::#module_id::#name_ident( &mut my_cx.borrow_mut(), From ba82ddcf379012f804f377958526bcb2912b617a Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 May 2020 12:14:53 -0700 Subject: [PATCH 13/19] borrow out of handles: change error name and describe behavior in comment --- crates/wiggle/src/borrow.rs | 16 +++++++++++++++- crates/wiggle/src/error.rs | 4 ++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/wiggle/src/borrow.rs b/crates/wiggle/src/borrow.rs index 2519f44dae..83d96c3448 100644 --- a/crates/wiggle/src/borrow.rs +++ b/crates/wiggle/src/borrow.rs @@ -44,8 +44,17 @@ impl BorrowChecker { } #[derive(Debug)] +/// This is a pretty naive way to account for borrows. This datastructure +/// could be made a lot more efficient with some effort. struct InnerBorrowChecker { + /// Map from handle to region borrowed. A HashMap is probably not ideal + /// for this but it works. It would be more efficient if we could + /// check `is_borrowed` without an O(n) iteration, by organizing borrows + /// by an ordering of Region. borrows: HashMap, + /// Handle to give out for the next borrow. This is the bare minimum of + /// bookkeeping of free handles, and in a pathological case we could run + /// out, hence [`GuestError::BorrowCheckerOutOfHandles`] next_handle: BorrowHandle, } @@ -71,9 +80,14 @@ impl InnerBorrowChecker { self.next_handle = BorrowHandle(0); } let h = self.next_handle; + // Get the next handle. Since we don't recycle handles until all of + // them have been returned, there is a pathological case where a user + // may make a Very Large (usize::MAX) number of valid borrows and + // unborrows while always keeping at least one borrow outstanding, and + // we will run out of borrow handles. self.next_handle = BorrowHandle( h.0.checked_add(1) - .ok_or_else(|| GuestError::BorrowCheckerOOM)?, + .ok_or_else(|| GuestError::BorrowCheckerOutOfHandles)?, ); Ok(h) } diff --git a/crates/wiggle/src/error.rs b/crates/wiggle/src/error.rs index 558f0c380a..5cfab66675 100644 --- a/crates/wiggle/src/error.rs +++ b/crates/wiggle/src/error.rs @@ -15,8 +15,8 @@ pub enum GuestError { PtrNotAligned(Region, u32), #[error("Pointer already borrowed: {0:?}")] PtrBorrowed(Region), - #[error("Borrow checker out of memory")] - BorrowCheckerOOM, + #[error("Borrow checker out of handles")] + BorrowCheckerOutOfHandles, #[error("Slice length mismatch")] SliceLengthsDiffer, #[error("In func {funcname}:{location}:")] From 3920d8c1f3b414884179eae5fc4295d409b02d87 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 May 2020 12:22:47 -0700 Subject: [PATCH 14/19] code review fix --- crates/wasi-common/src/path.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/wasi-common/src/path.rs b/crates/wasi-common/src/path.rs index 1f27b7a205..fdce7b481a 100644 --- a/crates/wasi-common/src/path.rs +++ b/crates/wasi-common/src/path.rs @@ -47,9 +47,6 @@ pub(crate) fn get( // any symlinks we encounter are processed by pushing them on the stack. let mut path_stack = vec![path.to_owned()]; - // No longer need a borrow into the guest memory for `path`: - drop(path); - // Track the number of symlinks we've expanded, so we can return `ELOOP` after too many. let mut symlink_expansions = 0; From 96d6884d33e39858aa72f275a20c1a6e066c51f8 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 May 2020 12:37:14 -0700 Subject: [PATCH 15/19] wiggle: get BorrowChecker from GuestMemory method --- crates/wiggle/generate/src/funcs.rs | 11 +++--- crates/wiggle/src/guest_type.rs | 2 +- crates/wiggle/src/lib.rs | 54 +++++++++++++++++---------- crates/wiggle/test-helpers/src/lib.rs | 7 +++- crates/wiggle/tests/arrays.rs | 30 ++++++--------- crates/wiggle/tests/atoms.rs | 9 ++--- crates/wiggle/tests/flags.rs | 8 ++-- crates/wiggle/tests/handles.rs | 11 +++--- crates/wiggle/tests/ints.rs | 6 +-- crates/wiggle/tests/pointers.rs | 16 ++++---- crates/wiggle/tests/strings.rs | 15 +++----- crates/wiggle/tests/structs.rs | 45 +++++++++------------- crates/wiggle/tests/union.rs | 26 ++++++------- 13 files changed, 112 insertions(+), 128 deletions(-) diff --git a/crates/wiggle/generate/src/funcs.rs b/crates/wiggle/generate/src/funcs.rs index 2a11a3d495..690ced5512 100644 --- a/crates/wiggle/generate/src/funcs.rs +++ b/crates/wiggle/generate/src/funcs.rs @@ -26,7 +26,6 @@ pub fn define_func( let abi_args = quote!( ctx: &#ctx_type, memory: &dyn #rt::GuestMemory, - bc: &#rt::BorrowChecker, #(#params),* ); let abi_ret = if let Some(ret) = &coretype.ret { @@ -212,7 +211,7 @@ fn marshal_arg( let arg_name = names.func_ptr_binding(¶m.name); let name = names.func_param(¶m.name); quote! { - let #name = match #rt::GuestPtr::<#pointee_type>::new(memory, bc, #arg_name as u32).read() { + let #name = match #rt::GuestPtr::<#pointee_type>::new(memory, #arg_name as u32).read() { Ok(r) => r, Err(e) => { #error_handling @@ -258,7 +257,7 @@ fn marshal_arg( let len_name = names.func_len_binding(¶m.name); let name = names.func_param(¶m.name); quote! { - let #name = #rt::GuestPtr::<#lifetime, str>::new(memory, bc, (#ptr_name as u32, #len_name as u32)); + let #name = #rt::GuestPtr::<#lifetime, str>::new(memory, (#ptr_name as u32, #len_name as u32)); } } }, @@ -266,7 +265,7 @@ fn marshal_arg( let pointee_type = names.type_ref(pointee, anon_lifetime()); let name = names.func_param(¶m.name); quote! { - let #name = #rt::GuestPtr::<#pointee_type>::new(memory, bc, #name as u32); + let #name = #rt::GuestPtr::<#pointee_type>::new(memory, #name as u32); } } witx::Type::Struct(_) => read_conversion, @@ -276,7 +275,7 @@ fn marshal_arg( let len_name = names.func_len_binding(¶m.name); let name = names.func_param(¶m.name); quote! { - let #name = #rt::GuestPtr::<[#pointee_type]>::new(memory, bc, (#ptr_name as u32, #len_name as u32)); + let #name = #rt::GuestPtr::<[#pointee_type]>::new(memory, (#ptr_name as u32, #len_name as u32)); } } witx::Type::Union(_u) => read_conversion, @@ -305,7 +304,7 @@ where let ptr_name = names.func_ptr_binding(&result.name); let ptr_err_handling = error_handling(&format!("{}:result_ptr_mut", result.name.as_str())); let pre = quote! { - let #ptr_name = #rt::GuestPtr::<#pointee_type>::new(memory, bc, #ptr_name as u32); + let #ptr_name = #rt::GuestPtr::<#pointee_type>::new(memory, #ptr_name as u32); }; // trait binding returns func_param name. let val_name = names.func_param(&result.name); diff --git a/crates/wiggle/src/guest_type.rs b/crates/wiggle/src/guest_type.rs index 47a5a80e09..4359d3745f 100644 --- a/crates/wiggle/src/guest_type.rs +++ b/crates/wiggle/src/guest_type.rs @@ -146,7 +146,7 @@ impl<'a, T> GuestType<'a> for GuestPtr<'a, T> { fn read(ptr: &GuestPtr<'a, Self>) -> Result { let offset = ptr.cast::().read()?; - Ok(GuestPtr::new(ptr.mem(), ptr.borrow_checker(), offset)) + Ok(GuestPtr::new(ptr.mem(), offset)) } fn write(ptr: &GuestPtr<'_, Self>, val: Self) -> Result<(), GuestError> { diff --git a/crates/wiggle/src/lib.rs b/crates/wiggle/src/lib.rs index 45479c8644..000080ad86 100644 --- a/crates/wiggle/src/lib.rs +++ b/crates/wiggle/src/lib.rs @@ -65,7 +65,7 @@ pub use region::Region; /// into the memory region given by a `GuestMemory`. /// /// These smart pointers are dynamically borrow-checked by the `BorrowChecker` -/// passed to the wiggle-generated ABI-level functions. While a `GuestSlice` +/// given by [`GuestMemory::borrow_checker()`]. While a `GuestSlice` /// or a `GuestStr` are live, the [`BorrowChecker::has_outstanding_borrows()`] /// method will always return `true`. If you need to re-enter the guest or /// otherwise read or write to the contents of a WebAssembly memory, all @@ -84,6 +84,11 @@ pub unsafe trait GuestMemory { /// [`GuestMemory`] documentation. fn base(&self) -> (*mut u8, u32); + /// Gives a reference to the [`BorrowChecker`] used to keep track of each + /// outstanding borrow of the memory region. [`BorrowChecker::new`] safety + /// rules require that exactly one checker exist for each memory region. + fn borrow_checker(&self) -> &BorrowChecker; + /// Validates a guest-relative pointer given various attributes, and returns /// the corresponding host pointer. /// @@ -138,12 +143,12 @@ pub unsafe trait GuestMemory { /// Note that `T` can be almost any type, and typically `offset` is a `u32`. /// The exception is slices and strings, in which case `offset` is a `(u32, /// u32)` of `(offset, length)`. - fn ptr<'a, T>(&'a self, bc: &'a BorrowChecker, offset: T::Pointer) -> GuestPtr<'a, T> + fn ptr<'a, T>(&'a self, offset: T::Pointer) -> GuestPtr<'a, T> where Self: Sized, T: ?Sized + Pointee, { - GuestPtr::new(self, bc, offset) + GuestPtr::new(self, offset) } } @@ -153,30 +158,45 @@ unsafe impl<'a, T: ?Sized + GuestMemory> GuestMemory for &'a T { fn base(&self) -> (*mut u8, u32) { T::base(self) } + fn borrow_checker(&self) -> &BorrowChecker { + T::borrow_checker(self) + } } unsafe impl<'a, T: ?Sized + GuestMemory> GuestMemory for &'a mut T { fn base(&self) -> (*mut u8, u32) { T::base(self) } + fn borrow_checker(&self) -> &BorrowChecker { + T::borrow_checker(self) + } } unsafe impl GuestMemory for Box { fn base(&self) -> (*mut u8, u32) { T::base(self) } + fn borrow_checker(&self) -> &BorrowChecker { + T::borrow_checker(self) + } } unsafe impl GuestMemory for Rc { fn base(&self) -> (*mut u8, u32) { T::base(self) } + fn borrow_checker(&self) -> &BorrowChecker { + T::borrow_checker(self) + } } unsafe impl GuestMemory for Arc { fn base(&self) -> (*mut u8, u32) { T::base(self) } + fn borrow_checker(&self) -> &BorrowChecker { + T::borrow_checker(self) + } } /// A *guest* pointer into host memory. @@ -229,7 +249,6 @@ unsafe impl GuestMemory for Arc { /// already-attached helper methods. pub struct GuestPtr<'a, T: ?Sized + Pointee> { mem: &'a (dyn GuestMemory + 'a), - bc: &'a BorrowChecker, pointer: T::Pointer, _marker: marker::PhantomData<&'a Cell>, } @@ -240,14 +259,9 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { /// Note that for sized types like `u32`, `GuestPtr`, etc, the `pointer` /// vlue is a `u32` offset into guest memory. For slices and strings, /// `pointer` is a `(u32, u32)` offset/length pair. - pub fn new( - mem: &'a (dyn GuestMemory + 'a), - bc: &'a BorrowChecker, - pointer: T::Pointer, - ) -> GuestPtr<'a, T> { + pub fn new(mem: &'a (dyn GuestMemory + 'a), pointer: T::Pointer) -> GuestPtr<'a, T> { GuestPtr { mem, - bc, pointer, _marker: marker::PhantomData, } @@ -268,7 +282,7 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { /// Returns the borrow checker that this pointer uses pub fn borrow_checker(&self) -> &'a BorrowChecker { - self.bc + self.mem.borrow_checker() } /// Casts this `GuestPtr` type to a different type. @@ -281,7 +295,7 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { where T: Pointee, { - GuestPtr::new(self.mem, self.bc, self.pointer) + GuestPtr::new(self.mem, self.pointer) } /// Safely read a value from this pointer. @@ -348,7 +362,7 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { Some(o) => o, None => return Err(GuestError::PtrOverflow), }; - Ok(GuestPtr::new(self.mem, self.bc, offset)) + Ok(GuestPtr::new(self.mem, offset)) } /// Returns a `GuestPtr` for an array of `T`s using this pointer as the @@ -357,7 +371,7 @@ impl<'a, T: ?Sized + Pointee> GuestPtr<'a, T> { where T: GuestType<'a> + Pointee, { - GuestPtr::new(self.mem, self.bc, (self.pointer, elems)) + GuestPtr::new(self.mem, (self.pointer, elems)) } } @@ -410,7 +424,7 @@ impl<'a, T> GuestPtr<'a, [T]> { self.mem .validate_size_align(self.pointer.0, T::guest_align(), len)? as *mut T; - let borrow = self.bc.borrow(Region { + let borrow = self.mem.borrow_checker().borrow(Region { start: self.pointer.0, len, })?; @@ -426,7 +440,7 @@ impl<'a, T> GuestPtr<'a, [T]> { Ok(GuestSlice { ptr, - bc: self.bc, + bc: self.mem.borrow_checker(), borrow, }) } @@ -461,7 +475,7 @@ impl<'a, T> GuestPtr<'a, [T]> { /// Returns a `GuestPtr` pointing to the base of the array for the interior /// type `T`. pub fn as_ptr(&self) -> GuestPtr<'a, T> { - GuestPtr::new(self.mem, self.bc, self.offset_base()) + GuestPtr::new(self.mem, self.offset_base()) } } @@ -480,7 +494,7 @@ impl<'a> GuestPtr<'a, str> { /// Returns a raw pointer for the underlying slice of bytes that this /// pointer points to. pub fn as_bytes(&self) -> GuestPtr<'a, [u8]> { - GuestPtr::new(self.mem, self.bc, self.pointer) + GuestPtr::new(self.mem, self.pointer) } /// Attempts to create a [`GuestStr<'_>`] from this pointer, performing @@ -497,7 +511,7 @@ impl<'a> GuestPtr<'a, str> { .mem .validate_size_align(self.pointer.0, 1, self.pointer.1)?; - let borrow = self.bc.borrow(Region { + let borrow = self.mem.borrow_checker().borrow(Region { start: self.pointer.0, len: self.pointer.1, })?; @@ -509,7 +523,7 @@ impl<'a> GuestPtr<'a, str> { match str::from_utf8_mut(ptr) { Ok(ptr) => Ok(GuestStr { ptr, - bc: self.bc, + bc: self.mem.borrow_checker(), borrow, }), Err(e) => Err(GuestError::InvalidUtf8(e)), diff --git a/crates/wiggle/test-helpers/src/lib.rs b/crates/wiggle/test-helpers/src/lib.rs index 42719672b2..dfb20f799d 100644 --- a/crates/wiggle/test-helpers/src/lib.rs +++ b/crates/wiggle/test-helpers/src/lib.rs @@ -1,7 +1,7 @@ use proptest::prelude::*; use std::cell::UnsafeCell; use std::marker; -use wiggle::GuestMemory; +use wiggle::{BorrowChecker, GuestMemory}; #[derive(Debug, Clone)] pub struct MemAreas(Vec); @@ -45,11 +45,13 @@ impl Into> for MemAreas { #[repr(align(4096))] pub struct HostMemory { buffer: UnsafeCell<[u8; 4096]>, + bc: BorrowChecker, } impl HostMemory { pub fn new() -> Self { HostMemory { buffer: UnsafeCell::new([0; 4096]), + bc: unsafe { BorrowChecker::new() }, } } @@ -111,6 +113,9 @@ unsafe impl GuestMemory for HostMemory { ((*ptr).as_mut_ptr(), (*ptr).len() as u32) } } + fn borrow_checker(&self) -> &BorrowChecker { + &self.bc + } } #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] diff --git a/crates/wiggle/tests/arrays.rs b/crates/wiggle/tests/arrays.rs index 4fba074667..9142e9f01e 100644 --- a/crates/wiggle/tests/arrays.rs +++ b/crates/wiggle/tests/arrays.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; +use wiggle::{GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -77,25 +77,22 @@ impl ReduceExcusesExcercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; // Populate memory with pointers to generated Excuse values for (&excuse, ptr) in self.excuse_values.iter().zip(self.excuse_ptr_locs.iter()) { host_memory - .ptr(&bc, ptr.ptr) + .ptr(ptr.ptr) .write(excuse) .expect("deref ptr mut to Excuse value"); } // Populate the array with pointers to generated Excuse values { - let array: GuestPtr<'_, [GuestPtr]> = host_memory.ptr( - &bc, - (self.array_ptr_loc.ptr, self.excuse_ptr_locs.len() as u32), - ); + let array: GuestPtr<'_, [GuestPtr]> = + host_memory.ptr((self.array_ptr_loc.ptr, self.excuse_ptr_locs.len() as u32)); for (slot, ptr) in array.iter().zip(&self.excuse_ptr_locs) { let slot = slot.expect("array should be in bounds"); - slot.write(host_memory.ptr(&bc, ptr.ptr)) + slot.write(host_memory.ptr(ptr.ptr)) .expect("should succeed in writing array"); } } @@ -103,7 +100,6 @@ impl ReduceExcusesExcercise { let res = arrays::reduce_excuses( &ctx, &host_memory, - &bc, self.array_ptr_loc.ptr as i32, self.excuse_ptr_locs.len() as i32, self.return_ptr_loc.ptr as i32, @@ -116,7 +112,7 @@ impl ReduceExcusesExcercise { .last() .expect("generated vec of excuses should be non-empty"); let given: types::Excuse = host_memory - .ptr(&bc, self.return_ptr_loc.ptr) + .ptr(self.return_ptr_loc.ptr) .read() .expect("deref ptr to returned value"); assert_eq!(expected, given, "reduce excuses return val"); @@ -169,30 +165,28 @@ impl PopulateExcusesExcercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; // Populate array with valid pointers to Excuse type in memory - let ptr = host_memory.ptr::<[GuestPtr<'_, types::Excuse>]>( - &bc, - (self.array_ptr_loc.ptr, self.elements.len() as u32), - ); + let ptr = host_memory.ptr::<[GuestPtr<'_, types::Excuse>]>(( + self.array_ptr_loc.ptr, + self.elements.len() as u32, + )); for (ptr, val) in ptr.iter().zip(&self.elements) { ptr.expect("should be valid pointer") - .write(host_memory.ptr(&bc, val.ptr)) + .write(host_memory.ptr(val.ptr)) .expect("failed to write value"); } let res = arrays::populate_excuses( &ctx, &host_memory, - &bc, self.array_ptr_loc.ptr as i32, self.elements.len() as i32, ); assert_eq!(res, types::Errno::Ok.into(), "populate excuses errno"); let arr: GuestPtr<'_, [GuestPtr<'_, types::Excuse>]> = - host_memory.ptr(&bc, (self.array_ptr_loc.ptr, self.elements.len() as u32)); + host_memory.ptr((self.array_ptr_loc.ptr, self.elements.len() as u32)); for el in arr.iter() { let ptr_to_ptr = el .expect("valid ptr to ptr") diff --git a/crates/wiggle/tests/atoms.rs b/crates/wiggle/tests/atoms.rs index 3497215f0c..5407828fa9 100644 --- a/crates/wiggle/tests/atoms.rs +++ b/crates/wiggle/tests/atoms.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{BorrowChecker, GuestMemory}; +use wiggle::GuestMemory; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -31,9 +31,8 @@ impl IntFloatExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; - let e = atoms::int_float_args(&ctx, &host_memory, &bc, self.an_int as i32, self.an_float); + let e = atoms::int_float_args(&ctx, &host_memory, self.an_int as i32, self.an_float); assert_eq!(e, types::Errno::Ok.into(), "int_float_args error"); } @@ -61,18 +60,16 @@ impl DoubleIntExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; let e = atoms::double_int_return_float( &ctx, &host_memory, - &bc, self.input as i32, self.return_loc.ptr as i32, ); let return_val = host_memory - .ptr::(&bc, self.return_loc.ptr) + .ptr::(self.return_loc.ptr) .read() .expect("failed to read return"); assert_eq!(e, types::Errno::Ok.into(), "errno"); diff --git a/crates/wiggle/tests/flags.rs b/crates/wiggle/tests/flags.rs index 727a9262bf..fcb07e7a7d 100644 --- a/crates/wiggle/tests/flags.rs +++ b/crates/wiggle/tests/flags.rs @@ -1,6 +1,6 @@ use proptest::prelude::*; use std::convert::TryFrom; -use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; +use wiggle::{GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -65,18 +65,16 @@ impl ConfigureCarExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; // Populate input ptr host_memory - .ptr(&bc, self.other_config_by_ptr.ptr) + .ptr(self.other_config_by_ptr.ptr) .write(self.other_config) .expect("deref ptr mut to CarConfig"); let res = flags::configure_car( &ctx, &host_memory, - &bc, self.old_config.into(), self.other_config_by_ptr.ptr as i32, self.return_ptr_loc.ptr as i32, @@ -84,7 +82,7 @@ impl ConfigureCarExercise { assert_eq!(res, types::Errno::Ok.into(), "configure car errno"); let res_config = host_memory - .ptr::(&bc, self.return_ptr_loc.ptr) + .ptr::(self.return_ptr_loc.ptr) .read() .expect("deref to CarConfig value"); diff --git a/crates/wiggle/tests/handles.rs b/crates/wiggle/tests/handles.rs index 8b066c7aa5..93e3b54d18 100644 --- a/crates/wiggle/tests/handles.rs +++ b/crates/wiggle/tests/handles.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{BorrowChecker, GuestMemory, GuestType}; +use wiggle::{GuestMemory, GuestType}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; const FD_VAL: u32 = 123; @@ -34,24 +34,23 @@ impl HandleExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; - let e = handle_examples::fd_create(&ctx, &host_memory, &bc, self.return_loc.ptr as i32); + let e = handle_examples::fd_create(&ctx, &host_memory, self.return_loc.ptr as i32); assert_eq!(e, types::Errno::Ok.into(), "fd_create error"); let h_got: u32 = host_memory - .ptr(&bc, self.return_loc.ptr) + .ptr(self.return_loc.ptr) .read() .expect("return ref_mut"); assert_eq!(h_got, 123, "fd_create return val"); - let e = handle_examples::fd_consume(&ctx, &host_memory, &bc, h_got as i32); + let e = handle_examples::fd_consume(&ctx, &host_memory, h_got as i32); assert_eq!(e, types::Errno::Ok.into(), "fd_consume error"); - let e = handle_examples::fd_consume(&ctx, &host_memory, &bc, h_got as i32 + 1); + let e = handle_examples::fd_consume(&ctx, &host_memory, h_got as i32 + 1); assert_eq!( e, diff --git a/crates/wiggle/tests/ints.rs b/crates/wiggle/tests/ints.rs index db47ea469d..36d7933e62 100644 --- a/crates/wiggle/tests/ints.rs +++ b/crates/wiggle/tests/ints.rs @@ -1,6 +1,6 @@ use proptest::prelude::*; use std::convert::TryFrom; -use wiggle::{BorrowChecker, GuestMemory}; +use wiggle::GuestMemory; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -46,19 +46,17 @@ impl CookieCutterExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; let res = ints::cookie_cutter( &ctx, &host_memory, - &bc, self.cookie.into(), self.return_ptr_loc.ptr as i32, ); assert_eq!(res, types::Errno::Ok.into(), "cookie cutter errno"); let is_cookie_start = host_memory - .ptr::(&bc, self.return_ptr_loc.ptr) + .ptr::(self.return_ptr_loc.ptr) .read() .expect("deref to Bool value"); diff --git a/crates/wiggle/tests/pointers.rs b/crates/wiggle/tests/pointers.rs index 74ca3a1fa2..c39988d115 100644 --- a/crates/wiggle/tests/pointers.rs +++ b/crates/wiggle/tests/pointers.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; +use wiggle::{GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -128,32 +128,30 @@ impl PointersAndEnumsExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; host_memory - .ptr(&bc, self.input2_loc.ptr) + .ptr(self.input2_loc.ptr) .write(self.input2) .expect("input2 ref_mut"); host_memory - .ptr(&bc, self.input3_loc.ptr) + .ptr(self.input3_loc.ptr) .write(self.input3) .expect("input3 ref_mut"); host_memory - .ptr(&bc, self.input4_loc.ptr) + .ptr(self.input4_loc.ptr) .write(self.input4) .expect("input4 ref_mut"); host_memory - .ptr(&bc, self.input4_ptr_loc.ptr) + .ptr(self.input4_ptr_loc.ptr) .write(self.input4_loc.ptr) .expect("input4 ptr ref_mut"); let e = pointers::pointers_and_enums( &ctx, &host_memory, - &bc, self.input1.into(), self.input2_loc.ptr as i32, self.input3_loc.ptr as i32, @@ -163,7 +161,7 @@ impl PointersAndEnumsExercise { // Implementation of pointers_and_enums writes input3 to the input2_loc: let written_to_input2_loc: i32 = host_memory - .ptr(&bc, self.input2_loc.ptr) + .ptr(self.input2_loc.ptr) .read() .expect("input2 ref"); @@ -175,7 +173,7 @@ impl PointersAndEnumsExercise { // Implementation of pointers_and_enums writes input2_loc to input4_ptr_loc: let written_to_input4_ptr: u32 = host_memory - .ptr(&bc, self.input4_ptr_loc.ptr) + .ptr(self.input4_ptr_loc.ptr) .read() .expect("input4_ptr_loc ref"); diff --git a/crates/wiggle/tests/strings.rs b/crates/wiggle/tests/strings.rs index aa34312a86..3546fd1276 100644 --- a/crates/wiggle/tests/strings.rs +++ b/crates/wiggle/tests/strings.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; +use wiggle::{GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, MemAreas, WasiCtx}; wiggle::from_witx!({ @@ -69,11 +69,9 @@ impl HelloStringExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; // Populate string in guest's memory - let ptr = - host_memory.ptr::(&bc, (self.string_ptr_loc.ptr, self.test_word.len() as u32)); + let ptr = host_memory.ptr::((self.string_ptr_loc.ptr, self.test_word.len() as u32)); for (slot, byte) in ptr.as_bytes().iter().zip(self.test_word.bytes()) { slot.expect("should be valid pointer") .write(byte) @@ -83,7 +81,6 @@ impl HelloStringExercise { let res = strings::hello_string( &ctx, &host_memory, - &bc, self.string_ptr_loc.ptr as i32, self.test_word.len() as i32, self.return_ptr_loc.ptr as i32, @@ -91,7 +88,7 @@ impl HelloStringExercise { assert_eq!(res, types::Errno::Ok.into(), "hello string errno"); let given = host_memory - .ptr::(&bc, self.return_ptr_loc.ptr) + .ptr::(self.return_ptr_loc.ptr) .read() .expect("deref ptr to return value"); assert_eq!(self.test_word.len() as u32, given); @@ -178,10 +175,9 @@ impl MultiStringExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; let write_string = |val: &str, loc: MemArea| { - let ptr = host_memory.ptr::(&bc, (loc.ptr, val.len() as u32)); + let ptr = host_memory.ptr::((loc.ptr, val.len() as u32)); for (slot, byte) in ptr.as_bytes().iter().zip(val.bytes()) { slot.expect("should be valid pointer") .write(byte) @@ -196,7 +192,6 @@ impl MultiStringExercise { let res = strings::multi_string( &ctx, &host_memory, - &bc, self.sa_ptr_loc.ptr as i32, self.a.len() as i32, self.sb_ptr_loc.ptr as i32, @@ -208,7 +203,7 @@ impl MultiStringExercise { assert_eq!(res, types::Errno::Ok.into(), "multi string errno"); let given = host_memory - .ptr::(&bc, self.return_ptr_loc.ptr) + .ptr::(self.return_ptr_loc.ptr) .read() .expect("deref ptr to return value"); assert_eq!((self.a.len() + self.b.len() + self.c.len()) as u32, given); diff --git a/crates/wiggle/tests/structs.rs b/crates/wiggle/tests/structs.rs index 4861239ac2..b98cf66163 100644 --- a/crates/wiggle/tests/structs.rs +++ b/crates/wiggle/tests/structs.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{BorrowChecker, GuestMemory, GuestPtr}; +use wiggle::{GuestMemory, GuestPtr}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -83,20 +83,18 @@ impl SumOfPairExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; host_memory - .ptr(&bc, self.input_loc.ptr) + .ptr(self.input_loc.ptr) .write(self.input.first) .expect("input ref_mut"); host_memory - .ptr(&bc, self.input_loc.ptr + 4) + .ptr(self.input_loc.ptr + 4) .write(self.input.second) .expect("input ref_mut"); let sum_err = structs::sum_of_pair( &ctx, &host_memory, - &bc, self.input_loc.ptr as i32, self.return_loc.ptr as i32, ); @@ -104,7 +102,7 @@ impl SumOfPairExercise { assert_eq!(sum_err, types::Errno::Ok.into(), "sum errno"); let return_val: i64 = host_memory - .ptr(&bc, self.return_loc.ptr) + .ptr(self.return_loc.ptr) .read() .expect("return ref"); @@ -173,30 +171,28 @@ impl SumPairPtrsExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; host_memory - .ptr(&bc, self.input_first_loc.ptr) + .ptr(self.input_first_loc.ptr) .write(self.input_first) .expect("input_first ref"); host_memory - .ptr(&bc, self.input_second_loc.ptr) + .ptr(self.input_second_loc.ptr) .write(self.input_second) .expect("input_second ref"); host_memory - .ptr(&bc, self.input_struct_loc.ptr) + .ptr(self.input_struct_loc.ptr) .write(self.input_first_loc.ptr) .expect("input_struct ref"); host_memory - .ptr(&bc, self.input_struct_loc.ptr + 4) + .ptr(self.input_struct_loc.ptr + 4) .write(self.input_second_loc.ptr) .expect("input_struct ref"); let res = structs::sum_of_pair_of_ptrs( &ctx, &host_memory, - &bc, self.input_struct_loc.ptr as i32, self.return_loc.ptr as i32, ); @@ -204,7 +200,7 @@ impl SumPairPtrsExercise { assert_eq!(res, types::Errno::Ok.into(), "sum of pair of ptrs errno"); let doubled: i64 = host_memory - .ptr(&bc, self.return_loc.ptr) + .ptr(self.return_loc.ptr) .read() .expect("return ref"); @@ -259,25 +255,23 @@ impl SumIntAndPtrExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; host_memory - .ptr(&bc, self.input_first_loc.ptr) + .ptr(self.input_first_loc.ptr) .write(self.input_first) .expect("input_first ref"); host_memory - .ptr(&bc, self.input_struct_loc.ptr) + .ptr(self.input_struct_loc.ptr) .write(self.input_first_loc.ptr) .expect("input_struct ref"); host_memory - .ptr(&bc, self.input_struct_loc.ptr + 4) + .ptr(self.input_struct_loc.ptr + 4) .write(self.input_second) .expect("input_struct ref"); let res = structs::sum_of_int_and_ptr( &ctx, &host_memory, - &bc, self.input_struct_loc.ptr as i32, self.return_loc.ptr as i32, ); @@ -285,7 +279,7 @@ impl SumIntAndPtrExercise { assert_eq!(res, types::Errno::Ok.into(), "sum of int and ptr errno"); let doubled: i64 = host_memory - .ptr(&bc, self.return_loc.ptr) + .ptr(self.return_loc.ptr) .read() .expect("return ref"); @@ -318,14 +312,13 @@ impl ReturnPairInts { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; - let err = structs::return_pair_ints(&ctx, &host_memory, &bc, self.return_loc.ptr as i32); + let err = structs::return_pair_ints(&ctx, &host_memory, self.return_loc.ptr as i32); assert_eq!(err, types::Errno::Ok.into(), "return struct errno"); let return_struct: types::PairInts = host_memory - .ptr(&bc, self.return_loc.ptr) + .ptr(self.return_loc.ptr) .read() .expect("return ref"); @@ -384,21 +377,19 @@ impl ReturnPairPtrsExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; host_memory - .ptr(&bc, self.input_first_loc.ptr) + .ptr(self.input_first_loc.ptr) .write(self.input_first) .expect("input_first ref"); host_memory - .ptr(&bc, self.input_second_loc.ptr) + .ptr(self.input_second_loc.ptr) .write(self.input_second) .expect("input_second ref"); let res = structs::return_pair_of_ptrs( &ctx, &host_memory, - &bc, self.input_first_loc.ptr as i32, self.input_second_loc.ptr as i32, self.return_loc.ptr as i32, @@ -407,7 +398,7 @@ impl ReturnPairPtrsExercise { assert_eq!(res, types::Errno::Ok.into(), "return pair of ptrs errno"); let ptr_pair_int_ptrs: types::PairIntPtrs<'_> = host_memory - .ptr(&bc, self.return_loc.ptr) + .ptr(self.return_loc.ptr) .read() .expect("failed to read return location"); let ret_first_ptr = ptr_pair_int_ptrs.first; diff --git a/crates/wiggle/tests/union.rs b/crates/wiggle/tests/union.rs index 55894ba49a..ab3dada99c 100644 --- a/crates/wiggle/tests/union.rs +++ b/crates/wiggle/tests/union.rs @@ -1,5 +1,5 @@ use proptest::prelude::*; -use wiggle::{BorrowChecker, GuestMemory, GuestType}; +use wiggle::{GuestMemory, GuestType}; use wiggle_test::{impl_errno, HostMemory, MemArea, WasiCtx}; wiggle::from_witx!({ @@ -107,22 +107,21 @@ impl GetTagExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; let discriminant: u8 = reason_tag(&self.input).into(); host_memory - .ptr(&bc, self.input_loc.ptr) + .ptr(self.input_loc.ptr) .write(discriminant) .expect("input discriminant ptr"); match self.input { types::Reason::DogAte(f) => { host_memory - .ptr(&bc, self.input_loc.ptr + 4) + .ptr(self.input_loc.ptr + 4) .write(f) .expect("input contents ref_mut"); } types::Reason::Traffic(v) => host_memory - .ptr(&bc, self.input_loc.ptr + 4) + .ptr(self.input_loc.ptr + 4) .write(v) .expect("input contents ref_mut"), types::Reason::Sleeping => {} // Do nothing @@ -130,7 +129,6 @@ impl GetTagExercise { let e = union_example::get_tag( &ctx, &host_memory, - &bc, self.input_loc.ptr as i32, self.return_loc.ptr as i32, ); @@ -138,7 +136,7 @@ impl GetTagExercise { assert_eq!(e, types::Errno::Ok.into(), "get_tag errno"); let return_val: types::Excuse = host_memory - .ptr(&bc, self.return_loc.ptr) + .ptr(self.return_loc.ptr) .read() .expect("return ref"); @@ -186,28 +184,27 @@ impl ReasonMultExercise { pub fn test(&self) { let ctx = WasiCtx::new(); let host_memory = HostMemory::new(); - let bc = unsafe { BorrowChecker::new() }; let discriminant: u8 = reason_tag(&self.input).into(); host_memory - .ptr(&bc, self.input_loc.ptr) + .ptr(self.input_loc.ptr) .write(discriminant) .expect("input discriminant ref_mut"); host_memory - .ptr(&bc, self.input_loc.ptr + 4) + .ptr(self.input_loc.ptr + 4) .write(self.input_pointee_loc.ptr) .expect("input pointer ref_mut"); match self.input { types::Reason::DogAte(f) => { host_memory - .ptr(&bc, self.input_pointee_loc.ptr) + .ptr(self.input_pointee_loc.ptr) .write(f) .expect("input contents ref_mut"); } types::Reason::Traffic(v) => { host_memory - .ptr(&bc, self.input_pointee_loc.ptr) + .ptr(self.input_pointee_loc.ptr) .write(v) .expect("input contents ref_mut"); } @@ -216,7 +213,6 @@ impl ReasonMultExercise { let e = union_example::reason_mult( &ctx, &host_memory, - &bc, self.input_loc.ptr as i32, self.multiply_by as i32, ); @@ -226,7 +222,7 @@ impl ReasonMultExercise { match self.input { types::Reason::DogAte(f) => { let f_result: f32 = host_memory - .ptr(&bc, self.input_pointee_loc.ptr) + .ptr(self.input_pointee_loc.ptr) .read() .expect("input contents ref_mut"); assert_eq!( @@ -237,7 +233,7 @@ impl ReasonMultExercise { } types::Reason::Traffic(v) => { let v_result: i32 = host_memory - .ptr(&bc, self.input_pointee_loc.ptr) + .ptr(self.input_pointee_loc.ptr) .read() .expect("input contents ref_mut"); assert_eq!( From 9f763375deb6b81df81b71f64f50f6bf02853a61 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 May 2020 12:38:06 -0700 Subject: [PATCH 16/19] error name change fixup --- crates/wasi-common/src/wasi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasi-common/src/wasi.rs b/crates/wasi-common/src/wasi.rs index 4ab77ec3b2..f35055ffa8 100644 --- a/crates/wasi-common/src/wasi.rs +++ b/crates/wasi-common/src/wasi.rs @@ -40,7 +40,7 @@ impl From for Errno { InFunc { .. } => Self::Inval, InDataField { .. } => Self::Inval, SliceLengthsDiffer { .. } => Self::Fault, - BorrowCheckerOOM { .. } => Self::Fault, + BorrowCheckerOutOfHandles { .. } => Self::Fault, } } } From d19a09a4be7a244297f5264340812a76f9e73593 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 May 2020 12:40:39 -0700 Subject: [PATCH 17/19] wig: wiggle now puts BorrowChecker inside GuestMemory --- crates/wasi-common/wig/src/wasi.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/wasi-common/wig/src/wasi.rs b/crates/wasi-common/wig/src/wasi.rs index 243334c5be..ca113250e1 100644 --- a/crates/wasi-common/wig/src/wasi.rs +++ b/crates/wasi-common/wig/src/wasi.rs @@ -467,16 +467,16 @@ pub fn define_struct_for_wiggle(args: TokenStream) -> TokenStream { #handle_early_error } }; - let mem: WasiMemory = mem.into(); - // Wiggle does not expose any methods for functions to re-enter the - // WebAssembly module, or expose the memory via non-wiggle mechanisms. - // Therefore, creating a new BorrowChecker at the root of each function - // invocation is correct. + // Wiggle does not expose any methods for + // functions to re-enter the WebAssembly module, + // or expose the memory via non-wiggle mechanisms. + // Therefore, creating a new BorrowChecker at the + // root of each function invocation is correct. let bc = wiggle::BorrowChecker::new(); + let mem: WasiMemory { mem, bc }; wasi_common::wasi::#module_id::#name_ident( &mut my_cx.borrow_mut(), &mem, - &bc, #(#hostcall_args),* ) #cvt_ret } @@ -490,18 +490,18 @@ pub fn define_struct_for_wiggle(args: TokenStream) -> TokenStream { /// Lightweight `wasmtime::Memory` wrapper so that we can /// implement `wiggle::GuestMemory` trait on it which is /// now required to interface with `wasi-common`. - struct WasiMemory(wasmtime::Memory); - - impl From for WasiMemory { - fn from(mem: wasmtime::Memory) -> Self { - Self(mem) - } - } + struct WasiMemory { + mem: wasmtime::Memory, + bc: wiggle::BorrowChecker, + }; unsafe impl wiggle::GuestMemory for WasiMemory { fn base(&self) -> (*mut u8, u32) { (self.0.data_ptr(), self.0.data_size() as _) } + fn borrow_checker(&self) -> &wiggle::BorrowChecker { + &self.bc + } } /// An instantiated instance of the wasi exports. From 561f9e084a01395f84b534c44f98ddf70affcae2 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 May 2020 14:21:47 -0700 Subject: [PATCH 18/19] wig: bugfixes (sorry, thought i had tested before committimg) --- crates/wasi-common/wig/src/wasi.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/wasi-common/wig/src/wasi.rs b/crates/wasi-common/wig/src/wasi.rs index ca113250e1..579e6f3ca1 100644 --- a/crates/wasi-common/wig/src/wasi.rs +++ b/crates/wasi-common/wig/src/wasi.rs @@ -473,7 +473,7 @@ pub fn define_struct_for_wiggle(args: TokenStream) -> TokenStream { // Therefore, creating a new BorrowChecker at the // root of each function invocation is correct. let bc = wiggle::BorrowChecker::new(); - let mem: WasiMemory { mem, bc }; + let mem = WasiMemory { mem, bc }; wasi_common::wasi::#module_id::#name_ident( &mut my_cx.borrow_mut(), &mem, @@ -493,11 +493,11 @@ pub fn define_struct_for_wiggle(args: TokenStream) -> TokenStream { struct WasiMemory { mem: wasmtime::Memory, bc: wiggle::BorrowChecker, - }; + } unsafe impl wiggle::GuestMemory for WasiMemory { fn base(&self) -> (*mut u8, u32) { - (self.0.data_ptr(), self.0.data_size() as _) + (self.mem.data_ptr(), self.mem.data_size() as _) } fn borrow_checker(&self) -> &wiggle::BorrowChecker { &self.bc From bc1f538385aa1f910e22cdd6ead4542ae423fb28 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 21 May 2020 15:47:48 -0700 Subject: [PATCH 19/19] wiggle: fix tests --- crates/wiggle/test-helpers/src/lib.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/wiggle/test-helpers/src/lib.rs b/crates/wiggle/test-helpers/src/lib.rs index dfb20f799d..40d468a435 100644 --- a/crates/wiggle/test-helpers/src/lib.rs +++ b/crates/wiggle/test-helpers/src/lib.rs @@ -43,14 +43,20 @@ impl Into> for MemAreas { } #[repr(align(4096))] +struct HostBuffer { + cell: UnsafeCell<[u8; 4096]>, +} + pub struct HostMemory { - buffer: UnsafeCell<[u8; 4096]>, + buffer: HostBuffer, bc: BorrowChecker, } impl HostMemory { pub fn new() -> Self { HostMemory { - buffer: UnsafeCell::new([0; 4096]), + buffer: HostBuffer { + cell: UnsafeCell::new([0; 4096]), + }, bc: unsafe { BorrowChecker::new() }, } } @@ -109,7 +115,7 @@ impl HostMemory { unsafe impl GuestMemory for HostMemory { fn base(&self) -> (*mut u8, u32) { unsafe { - let ptr = self.buffer.get(); + let ptr = self.buffer.cell.get(); ((*ptr).as_mut_ptr(), (*ptr).len() as u32) } }