From dc84bd48de0e53c14159fa50ff4a95142d3eeb6b Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 12:04:58 -0700 Subject: [PATCH 01/17] Added WasmCell to the API --- lib/api/src/cell.rs | 239 ++++++++++++++++++++++++++++++++++++++++++++ lib/api/src/lib.rs | 2 + lib/api/src/ptr.rs | 5 +- 3 files changed, 244 insertions(+), 2 deletions(-) create mode 100644 lib/api/src/cell.rs diff --git a/lib/api/src/cell.rs b/lib/api/src/cell.rs new file mode 100644 index 00000000000..463296dce5a --- /dev/null +++ b/lib/api/src/cell.rs @@ -0,0 +1,239 @@ +pub use std::cell::Cell; + + + +use core::cmp::Ordering; +use core::fmt::{self, Debug, Display}; +use core::mem; +use core::ops::{Deref, DerefMut}; +use core::ptr; + +/// A mutable memory location. +/// +/// # Examples +/// +/// In this example, you can see that `WasmCell` enables mutation inside an +/// immutable struct. In other words, it enables "interior mutability". +/// +/// ``` +/// use wasmer::WasmCell; +/// +/// struct SomeStruct { +/// regular_field: u8, +/// special_field: WasmCell, +/// } +/// +/// let my_struct = SomeStruct { +/// regular_field: 0, +/// special_field: WasmCell::new(1), +/// }; +/// +/// let new_value = 100; +/// +/// // ERROR: `my_struct` is immutable +/// // my_struct.regular_field = new_value; +/// +/// // WORKS: although `my_struct` is immutable, `special_field` is a `WasmCell`, +/// // which can always be mutated +/// my_struct.special_field.set(new_value); +/// assert_eq!(my_struct.special_field.get(), new_value); +/// ``` +/// +/// See the [module-level documentation](self) for more. +#[derive(Clone)] +#[repr(transparent)] +pub struct WasmCell { + inner: *const Cell, +} + +unsafe impl Send for WasmCell where T: Send {} + +unsafe impl Sync for WasmCell {} + +// impl Clone for WasmCell { +// #[inline] +// fn clone(&self) -> WasmCell { +// WasmCell::new(self.get()) +// } +// } + +// impl Default for WasmCell { +// /// Creates a `WasmCell`, with the `Default` value for T. +// #[inline] +// fn default() -> WasmCell { +// WasmCell::new({ +// inner: Default::default() +// ) +// } +// } + +impl PartialEq for WasmCell { + #[inline] + fn eq(&self, other: &WasmCell) -> bool { + true + } +} + +impl Eq for WasmCell {} + +impl PartialOrd for WasmCell { + #[inline] + fn partial_cmp(&self, other: &WasmCell) -> Option { + self.inner.partial_cmp(&other.inner) + } + + #[inline] + fn lt(&self, other: &WasmCell) -> bool { + self.inner < other.inner + } + + #[inline] + fn le(&self, other: &WasmCell) -> bool { + self.inner <= other.inner + } + + #[inline] + fn gt(&self, other: &WasmCell) -> bool { + self.inner > other.inner + } + + #[inline] + fn ge(&self, other: &WasmCell) -> bool { + self.inner >= other.inner + } +} + +impl Ord for WasmCell { + #[inline] + fn cmp(&self, other: &WasmCell) -> Ordering { + self.get().cmp(&other.get()) + } +} + +// impl From for WasmCell { +// fn from(t: T) -> WasmCell { +// // unimplemented!(); +// // WasmCell::new(t) +// } +// } + +impl WasmCell { + /// Creates a new `WasmCell` containing the given value. + /// + /// # Examples + /// + /// ``` + /// use wasmer::WasmCell; + /// + /// let c = WasmCell::new(5); + /// ``` + #[inline] + pub const fn new(cell: *const Cell) -> WasmCell { + WasmCell { + inner: cell, + } + } + + /// Swaps the values of two WasmCells. + /// Difference with `std::mem::swap` is that this function doesn't require `&mut` reference. + /// + /// # Examples + /// + /// ``` + /// use wasmer::WasmCell; + /// + /// let c1 = WasmCell::new(5i32); + /// let c2 = WasmCell::new(10i32); + /// c1.swap(&c2); + /// assert_eq!(10, c1.get()); + /// assert_eq!(5, c2.get()); + /// ``` + #[inline] + pub fn swap(&self, other: &Self) { + unimplemented!(); + // if ptr::eq(self, other) { + // return; + // } + // // SAFETY: This can be risky if called from separate threads, but `WasmCell` + // // is `!Sync` so this won't happen. This also won't invalidate any + // // pointers since `WasmCell` makes sure nothing else will be pointing into + // // either of these `WasmCell`s. + // unsafe { + // ptr::swap(self.value.get(), other.value.get()); + // } + } + + /// Replaces the contained value with `val`, and returns the old contained value. + /// + /// # Examples + /// + /// ``` + /// use wasmer::WasmCell; + /// + /// let cell = WasmCell::new(5); + /// assert_eq!(cell.get(), 5); + /// assert_eq!(cell.replace(10), 5); + /// assert_eq!(cell.get(), 10); + /// ``` + pub fn replace(&self, val: T) -> T { + unimplemented!(); + // SAFETY: This can cause data races if called from a separate thread, + // but `WasmCell` is `!Sync` so this won't happen. + // mem::replace(unsafe { &mut *self.value.get() }, val) + } + + // /// Unwraps the value. + // /// + // /// # Examples + // /// + // /// ``` + // /// use wasmer::WasmCell; + // /// + // /// let c = WasmCell::new(5); + // /// let five = c.into_inner(); + // /// + // /// assert_eq!(five, 5); + // /// ``` + // pub const fn into_inner(self) -> T { + // // This will get the item out of the MemoryView and into + // // Rust memory allocator + // unimplemented!() + // // self.get() + // } +} + +impl WasmCell { + /// Returns a copy of the contained value. + /// + /// # Examples + /// + /// ``` + /// use wasmer::WasmCell; + /// + /// let c = WasmCell::new(5); + /// + /// let five = c.get(); + /// ``` + #[inline] + pub fn get(&self) -> T { + unsafe { (*self.inner).get() } + } +} + +impl WasmCell { + /// Sets the contained value. + /// + /// # Examples + /// + /// ``` + /// use wasmer::WasmCell; + /// + /// let c = WasmCell::new(5); + /// + /// c.set(10); + /// ``` + #[inline] + pub fn set(&self, val: T) { + unsafe { (*self.inner).set(val) }; + } +} diff --git a/lib/api/src/lib.rs b/lib/api/src/lib.rs index 4334d1fe970..b92cdc77563 100644 --- a/lib/api/src/lib.rs +++ b/lib/api/src/lib.rs @@ -253,6 +253,7 @@ //! [wasmer-llvm]: https://docs.rs/wasmer-compiler-llvm/*/wasmer_compiler_llvm/ //! [wasmer-wasi]: https://docs.rs/wasmer-wasi/*/wasmer_wasi/ +mod cell; mod env; mod exports; mod externals; @@ -281,6 +282,7 @@ pub mod internals { pub use crate::externals::{WithEnv, WithoutEnv}; } +pub use crate::cell::WasmCell; pub use crate::env::{HostEnvInitError, LazyInit, WasmerEnv}; pub use crate::exports::{ExportError, Exportable, Exports, ExportsIterator}; pub use crate::externals::{ diff --git a/lib/api/src/ptr.rs b/lib/api/src/ptr.rs index 3b873e6e51d..31023fcf4d7 100644 --- a/lib/api/src/ptr.rs +++ b/lib/api/src/ptr.rs @@ -8,6 +8,7 @@ use crate::{externals::Memory, FromToNativeWasmType}; use std::{cell::Cell, fmt, marker::PhantomData, mem}; +use crate::cell::WasmCell; use wasmer_types::ValueType; /// The `Array` marker type. This type can be used like `WasmPtr` @@ -103,7 +104,7 @@ impl WasmPtr { /// If you're unsure what that means, it likely does not apply to you. /// This invariant will be enforced in the future. #[inline] - pub fn deref<'a>(self, memory: &'a Memory) -> Option<&'a Cell> { + pub fn deref<'a>(self, memory: &'a Memory) -> Option> { if (self.offset as usize) + mem::size_of::() > memory.size().bytes().0 || mem::size_of::() == 0 { @@ -114,7 +115,7 @@ impl WasmPtr { memory.view::().as_ptr().add(self.offset as usize) as usize, mem::align_of::(), ) as *const Cell; - Some(&*cell_ptr) + Some(WasmCell::new(cell_ptr)) } } From 5ede1ac5448a5cadf356751e540bfdd21c1df7cd Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 12:26:22 -0700 Subject: [PATCH 02/17] Improved API by using references --- lib/api/src/cell.rs | 34 ++++++++++++++++++++-------------- lib/api/src/ptr.rs | 10 +++++----- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/lib/api/src/cell.rs b/lib/api/src/cell.rs index 463296dce5a..7c35d8707b4 100644 --- a/lib/api/src/cell.rs +++ b/lib/api/src/cell.rs @@ -42,13 +42,13 @@ use core::ptr; /// See the [module-level documentation](self) for more. #[derive(Clone)] #[repr(transparent)] -pub struct WasmCell { - inner: *const Cell, +pub struct WasmCell<'a, T: ?Sized> { + inner: &'a Cell, } -unsafe impl Send for WasmCell where T: Send {} +unsafe impl Send for WasmCell<'_, T> where T: Send {} -unsafe impl Sync for WasmCell {} +unsafe impl Sync for WasmCell<'_, T> {} // impl Clone for WasmCell { // #[inline] @@ -67,16 +67,16 @@ unsafe impl Sync for WasmCell {} // } // } -impl PartialEq for WasmCell { +impl PartialEq for WasmCell<'_, T> { #[inline] fn eq(&self, other: &WasmCell) -> bool { true } } -impl Eq for WasmCell {} +impl Eq for WasmCell<'_, T> {} -impl PartialOrd for WasmCell { +impl PartialOrd for WasmCell<'_, T> { #[inline] fn partial_cmp(&self, other: &WasmCell) -> Option { self.inner.partial_cmp(&other.inner) @@ -103,7 +103,7 @@ impl PartialOrd for WasmCell { } } -impl Ord for WasmCell { +impl Ord for WasmCell<'_, T> { #[inline] fn cmp(&self, other: &WasmCell) -> Ordering { self.get().cmp(&other.get()) @@ -117,7 +117,7 @@ impl Ord for WasmCell { // } // } -impl WasmCell { +impl<'a, T> WasmCell<'a, T> { /// Creates a new `WasmCell` containing the given value. /// /// # Examples @@ -128,7 +128,7 @@ impl WasmCell { /// let c = WasmCell::new(5); /// ``` #[inline] - pub const fn new(cell: *const Cell) -> WasmCell { + pub const fn new(cell: &'a Cell) -> WasmCell<'a, T> { WasmCell { inner: cell, } @@ -202,7 +202,7 @@ impl WasmCell { // } } -impl WasmCell { +impl WasmCell<'_, T> { /// Returns a copy of the contained value. /// /// # Examples @@ -216,11 +216,17 @@ impl WasmCell { /// ``` #[inline] pub fn get(&self) -> T { - unsafe { (*self.inner).get() } + self.inner.get() + } + + /// Get an unsafe mutable pointer to the inner item + /// in the Cell. + pub unsafe fn get_mut(&self) -> &mut T { + &mut *self.inner.as_ptr() } } -impl WasmCell { +impl WasmCell<'_, T> { /// Sets the contained value. /// /// # Examples @@ -234,6 +240,6 @@ impl WasmCell { /// ``` #[inline] pub fn set(&self, val: T) { - unsafe { (*self.inner).set(val) }; + self.inner.set(val); } } diff --git a/lib/api/src/ptr.rs b/lib/api/src/ptr.rs index 31023fcf4d7..b8bcd4ba316 100644 --- a/lib/api/src/ptr.rs +++ b/lib/api/src/ptr.rs @@ -115,7 +115,7 @@ impl WasmPtr { memory.view::().as_ptr().add(self.offset as usize) as usize, mem::align_of::(), ) as *const Cell; - Some(WasmCell::new(cell_ptr)) + Some(WasmCell::new(&*cell_ptr)) } } @@ -152,7 +152,7 @@ impl WasmPtr { /// If you're unsure what that means, it likely does not apply to you. /// This invariant will be enforced in the future. #[inline] - pub fn deref(self, memory: &Memory, index: u32, length: u32) -> Option<&[Cell]> { + pub fn deref(self, memory: &Memory, index: u32, length: u32) -> Option]>> { // gets the size of the item in the array with padding added such that // for any index, we will always result an aligned memory access let item_size = mem::size_of::(); @@ -170,9 +170,9 @@ impl WasmPtr { let cell_ptr = align_pointer( memory.view::().as_ptr().add(self.offset as usize) as usize, mem::align_of::(), - ) as *const Cell; - let cell_ptrs = &std::slice::from_raw_parts(cell_ptr, slice_full_len) - [index as usize..slice_full_len]; + ) as *const WasmCell; + let cell_ptrs = std::slice::from_raw_parts(cell_ptr, slice_full_len) + [index as usize..slice_full_len].to_owned().into_boxed_slice(); Some(cell_ptrs) } } From 67a028bbab7260ad881a946f72da08a28eacd6f1 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 12:40:44 -0700 Subject: [PATCH 03/17] Added debug to WasmCell --- lib/api/src/cell.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/api/src/cell.rs b/lib/api/src/cell.rs index 7c35d8707b4..eb7da9e232a 100644 --- a/lib/api/src/cell.rs +++ b/lib/api/src/cell.rs @@ -7,6 +7,7 @@ use core::fmt::{self, Debug, Display}; use core::mem; use core::ops::{Deref, DerefMut}; use core::ptr; +use std::fmt::Pointer; /// A mutable memory location. /// @@ -226,6 +227,14 @@ impl WasmCell<'_, T> { } } +impl Debug for WasmCell<'_, T> { + #[inline] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.inner.fmt(f) + } +} + + impl WasmCell<'_, T> { /// Sets the contained value. /// From 9493ac2d85849bfdc99d74e9823b466c8fd470f6 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 12:45:34 -0700 Subject: [PATCH 04/17] Improved API --- lib/api/src/cell.rs | 108 ++++---------------------------------------- 1 file changed, 9 insertions(+), 99 deletions(-) diff --git a/lib/api/src/cell.rs b/lib/api/src/cell.rs index eb7da9e232a..e9a7c6254f9 100644 --- a/lib/api/src/cell.rs +++ b/lib/api/src/cell.rs @@ -1,7 +1,5 @@ pub use std::cell::Cell; - - use core::cmp::Ordering; use core::fmt::{self, Debug, Display}; use core::mem; @@ -9,7 +7,7 @@ use core::ops::{Deref, DerefMut}; use core::ptr; use std::fmt::Pointer; -/// A mutable memory location. +/// A mutable Wasm-memory location. /// /// # Examples /// @@ -41,7 +39,6 @@ use std::fmt::Pointer; /// ``` /// /// See the [module-level documentation](self) for more. -#[derive(Clone)] #[repr(transparent)] pub struct WasmCell<'a, T: ?Sized> { inner: &'a Cell, @@ -51,27 +48,17 @@ unsafe impl Send for WasmCell<'_, T> where T: Send {} unsafe impl Sync for WasmCell<'_, T> {} -// impl Clone for WasmCell { -// #[inline] -// fn clone(&self) -> WasmCell { -// WasmCell::new(self.get()) -// } -// } - -// impl Default for WasmCell { -// /// Creates a `WasmCell`, with the `Default` value for T. -// #[inline] -// fn default() -> WasmCell { -// WasmCell::new({ -// inner: Default::default() -// ) -// } -// } +impl<'a, T: Copy> Clone for WasmCell<'a, T> { + #[inline] + fn clone(&self) -> WasmCell<'a, T> { + WasmCell { inner: self.inner } + } +} impl PartialEq for WasmCell<'_, T> { #[inline] fn eq(&self, other: &WasmCell) -> bool { - true + self.inner.eq(&other.inner) } } @@ -111,13 +98,6 @@ impl Ord for WasmCell<'_, T> { } } -// impl From for WasmCell { -// fn from(t: T) -> WasmCell { -// // unimplemented!(); -// // WasmCell::new(t) -// } -// } - impl<'a, T> WasmCell<'a, T> { /// Creates a new `WasmCell` containing the given value. /// @@ -130,77 +110,8 @@ impl<'a, T> WasmCell<'a, T> { /// ``` #[inline] pub const fn new(cell: &'a Cell) -> WasmCell<'a, T> { - WasmCell { - inner: cell, - } + WasmCell { inner: cell } } - - /// Swaps the values of two WasmCells. - /// Difference with `std::mem::swap` is that this function doesn't require `&mut` reference. - /// - /// # Examples - /// - /// ``` - /// use wasmer::WasmCell; - /// - /// let c1 = WasmCell::new(5i32); - /// let c2 = WasmCell::new(10i32); - /// c1.swap(&c2); - /// assert_eq!(10, c1.get()); - /// assert_eq!(5, c2.get()); - /// ``` - #[inline] - pub fn swap(&self, other: &Self) { - unimplemented!(); - // if ptr::eq(self, other) { - // return; - // } - // // SAFETY: This can be risky if called from separate threads, but `WasmCell` - // // is `!Sync` so this won't happen. This also won't invalidate any - // // pointers since `WasmCell` makes sure nothing else will be pointing into - // // either of these `WasmCell`s. - // unsafe { - // ptr::swap(self.value.get(), other.value.get()); - // } - } - - /// Replaces the contained value with `val`, and returns the old contained value. - /// - /// # Examples - /// - /// ``` - /// use wasmer::WasmCell; - /// - /// let cell = WasmCell::new(5); - /// assert_eq!(cell.get(), 5); - /// assert_eq!(cell.replace(10), 5); - /// assert_eq!(cell.get(), 10); - /// ``` - pub fn replace(&self, val: T) -> T { - unimplemented!(); - // SAFETY: This can cause data races if called from a separate thread, - // but `WasmCell` is `!Sync` so this won't happen. - // mem::replace(unsafe { &mut *self.value.get() }, val) - } - - // /// Unwraps the value. - // /// - // /// # Examples - // /// - // /// ``` - // /// use wasmer::WasmCell; - // /// - // /// let c = WasmCell::new(5); - // /// let five = c.into_inner(); - // /// - // /// assert_eq!(five, 5); - // /// ``` - // pub const fn into_inner(self) -> T { - // // This will get the item out of the MemoryView and into - // // Rust memory allocator - // unimplemented!() - // // self.get() - // } } impl WasmCell<'_, T> { @@ -234,7 +145,6 @@ impl Debug for WasmCell<'_, T> { } } - impl WasmCell<'_, T> { /// Sets the contained value. /// From c9a167bb3af8aed60d9af7f5e98f1008b59fef81 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 14:34:30 -0700 Subject: [PATCH 05/17] Improved WasmCell ptr API --- lib/api/src/cell.rs | 6 +- lib/api/src/ptr.rs | 143 ++++++++++++++++++++++---------------------- 2 files changed, 75 insertions(+), 74 deletions(-) diff --git a/lib/api/src/cell.rs b/lib/api/src/cell.rs index e9a7c6254f9..c52a97887c3 100644 --- a/lib/api/src/cell.rs +++ b/lib/api/src/cell.rs @@ -94,7 +94,7 @@ impl PartialOrd for WasmCell<'_, T> { impl Ord for WasmCell<'_, T> { #[inline] fn cmp(&self, other: &WasmCell) -> Ordering { - self.get().cmp(&other.get()) + self.inner.cmp(&other.inner) } } @@ -114,7 +114,7 @@ impl<'a, T> WasmCell<'a, T> { } } -impl WasmCell<'_, T> { +impl<'a, T: Copy> WasmCell<'a, T> { /// Returns a copy of the contained value. /// /// # Examples @@ -133,7 +133,7 @@ impl WasmCell<'_, T> { /// Get an unsafe mutable pointer to the inner item /// in the Cell. - pub unsafe fn get_mut(&self) -> &mut T { + pub unsafe fn get_mut(&self) -> &'a mut T { &mut *self.inner.as_ptr() } } diff --git a/lib/api/src/ptr.rs b/lib/api/src/ptr.rs index b8bcd4ba316..26c247e4a76 100644 --- a/lib/api/src/ptr.rs +++ b/lib/api/src/ptr.rs @@ -6,9 +6,9 @@ //! Therefore, you should use this abstraction whenever possible to avoid memory //! related bugs when implementing an ABI. +use crate::cell::WasmCell; use crate::{externals::Memory, FromToNativeWasmType}; use std::{cell::Cell, fmt, marker::PhantomData, mem}; -use crate::cell::WasmCell; use wasmer_types::ValueType; /// The `Array` marker type. This type can be used like `WasmPtr` @@ -119,26 +119,26 @@ impl WasmPtr { } } - /// Mutably dereference this `WasmPtr` getting a `&mut Cell` allowing for - /// direct access to a `&mut T`. - /// - /// # Safety - /// - This method does not do any aliasing checks: it's possible to create - /// `&mut T` that point to the same memory. You should ensure that you have - /// exclusive access to Wasm linear memory before calling this method. - #[inline] - pub unsafe fn deref_mut<'a>(self, memory: &'a Memory) -> Option<&'a mut Cell> { - if (self.offset as usize) + mem::size_of::() > memory.size().bytes().0 - || mem::size_of::() == 0 - { - return None; - } - let cell_ptr = align_pointer( - memory.view::().as_ptr().add(self.offset as usize) as usize, - mem::align_of::(), - ) as *mut Cell; - Some(&mut *cell_ptr) - } + // /// Mutably dereference this `WasmPtr` getting a `&mut Cell` allowing for + // /// direct access to a `&mut T`. + // /// + // /// # Safety + // /// - This method does not do any aliasing checks: it's possible to create + // /// `&mut T` that point to the same memory. You should ensure that you have + // /// exclusive access to Wasm linear memory before calling this method. + // #[inline] + // pub unsafe fn deref_mut<'a>(self, memory: &'a Memory) -> Option<&'a mut Cell> { + // if (self.offset as usize) + mem::size_of::() > memory.size().bytes().0 + // || mem::size_of::() == 0 + // { + // return None; + // } + // let cell_ptr = align_pointer( + // memory.view::().as_ptr().add(self.offset as usize) as usize, + // mem::align_of::(), + // ) as *mut Cell; + // Some(&mut *cell_ptr) + // } } /// Methods for `WasmPtr`s to arrays of data that can be dereferenced, namely to @@ -152,7 +152,7 @@ impl WasmPtr { /// If you're unsure what that means, it likely does not apply to you. /// This invariant will be enforced in the future. #[inline] - pub fn deref(self, memory: &Memory, index: u32, length: u32) -> Option]>> { + pub fn deref(self, memory: &Memory, index: u32, length: u32) -> Option>> { // gets the size of the item in the array with padding added such that // for any index, we will always result an aligned memory access let item_size = mem::size_of::(); @@ -172,46 +172,47 @@ impl WasmPtr { mem::align_of::(), ) as *const WasmCell; let cell_ptrs = std::slice::from_raw_parts(cell_ptr, slice_full_len) - [index as usize..slice_full_len].to_owned().into_boxed_slice(); + [index as usize..slice_full_len] + .to_owned(); Some(cell_ptrs) } } - /// Mutably dereference this `WasmPtr` getting a `&mut [Cell]` allowing for - /// direct access to a `&mut [T]`. - /// - /// # Safety - /// - This method does not do any aliasing checks: it's possible to create - /// `&mut T` that point to the same memory. You should ensure that you have - /// exclusive access to Wasm linear memory before calling this method. - #[inline] - pub unsafe fn deref_mut( - self, - memory: &Memory, - index: u32, - length: u32, - ) -> Option<&mut [Cell]> { - // gets the size of the item in the array with padding added such that - // for any index, we will always result an aligned memory access - let item_size = mem::size_of::(); - let slice_full_len = index as usize + length as usize; - let memory_size = memory.size().bytes().0; - - if (self.offset as usize) + (item_size * slice_full_len) > memory.size().bytes().0 - || self.offset as usize >= memory_size - || mem::size_of::() == 0 - { - return None; - } - - let cell_ptr = align_pointer( - memory.view::().as_ptr().add(self.offset as usize) as usize, - mem::align_of::(), - ) as *mut Cell; - let cell_ptrs = &mut std::slice::from_raw_parts_mut(cell_ptr, slice_full_len) - [index as usize..slice_full_len]; - Some(cell_ptrs) - } + // /// Mutably dereference this `WasmPtr` getting a `&mut [Cell]` allowing for + // /// direct access to a `&mut [T]`. + // /// + // /// # Safety + // /// - This method does not do any aliasing checks: it's possible to create + // /// `&mut T` that point to the same memory. You should ensure that you have + // /// exclusive access to Wasm linear memory before calling this method. + // #[inline] + // pub unsafe fn deref_mut( + // self, + // memory: &Memory, + // index: u32, + // length: u32, + // ) -> Option<&mut [Cell]> { + // // gets the size of the item in the array with padding added such that + // // for any index, we will always result an aligned memory access + // let item_size = mem::size_of::(); + // let slice_full_len = index as usize + length as usize; + // let memory_size = memory.size().bytes().0; + + // if (self.offset as usize) + (item_size * slice_full_len) > memory.size().bytes().0 + // || self.offset as usize >= memory_size + // || mem::size_of::() == 0 + // { + // return None; + // } + + // let cell_ptr = align_pointer( + // memory.view::().as_ptr().add(self.offset as usize) as usize, + // mem::align_of::(), + // ) as *mut Cell; + // let cell_ptrs = &mut std::slice::from_raw_parts_mut(cell_ptr, slice_full_len) + // [index as usize..slice_full_len]; + // Some(cell_ptrs) + // } /// Get a UTF-8 string from the `WasmPtr` with the given length. /// @@ -353,29 +354,29 @@ mod test { let start_wasm_ptr_array: WasmPtr = WasmPtr::new(0); assert!(start_wasm_ptr.deref(&memory).is_some()); - assert!(unsafe { start_wasm_ptr.deref_mut(&memory).is_some() }); + // assert!(unsafe { start_wasm_ptr.deref_mut(&memory).is_some() }); assert!(start_wasm_ptr_array.deref(&memory, 0, 0).is_some()); assert!(unsafe { start_wasm_ptr_array.get_utf8_str(&memory, 0).is_some() }); assert!(start_wasm_ptr_array.get_utf8_string(&memory, 0).is_some()); - assert!(unsafe { start_wasm_ptr_array.deref_mut(&memory, 0, 0).is_some() }); + // assert!(unsafe { start_wasm_ptr_array.deref_mut(&memory, 0, 0).is_some() }); assert!(start_wasm_ptr_array.deref(&memory, 0, 1).is_some()); - assert!(unsafe { start_wasm_ptr_array.deref_mut(&memory, 0, 1).is_some() }); + // assert!(unsafe { start_wasm_ptr_array.deref_mut(&memory, 0, 1).is_some() }); // test that accessing the last valid memory address works correctly and OOB is caught let last_valid_address_for_u8 = (memory.size().bytes().0 - 1) as u32; let end_wasm_ptr: WasmPtr = WasmPtr::new(last_valid_address_for_u8); assert!(end_wasm_ptr.deref(&memory).is_some()); - assert!(unsafe { end_wasm_ptr.deref_mut(&memory).is_some() }); + // assert!(unsafe { end_wasm_ptr.deref_mut(&memory).is_some() }); let end_wasm_ptr_array: WasmPtr = WasmPtr::new(last_valid_address_for_u8); assert!(end_wasm_ptr_array.deref(&memory, 0, 1).is_some()); - assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, 0, 1).is_some() }); + // assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, 0, 1).is_some() }); let invalid_idx_len_combos: [(u32, u32); 3] = [(last_valid_address_for_u8 + 1, 0), (0, 2), (1, 1)]; for &(idx, len) in invalid_idx_len_combos.iter() { assert!(end_wasm_ptr_array.deref(&memory, idx, len).is_none()); - assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, idx, len).is_none() }); + // assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, idx, len).is_none() }); } assert!(unsafe { end_wasm_ptr_array.get_utf8_str(&memory, 2).is_none() }); assert!(end_wasm_ptr_array.get_utf8_string(&memory, 2).is_none()); @@ -385,9 +386,9 @@ mod test { let last_valid_address_for_u32 = (memory.size().bytes().0 - 4) as u32; let end_wasm_ptr: WasmPtr = WasmPtr::new(last_valid_address_for_u32); assert!(end_wasm_ptr.deref(&memory).is_some()); - assert!(unsafe { end_wasm_ptr.deref_mut(&memory).is_some() }); + // assert!(unsafe { end_wasm_ptr.deref_mut(&memory).is_some() }); assert!(end_wasm_ptr.deref(&memory).is_some()); - assert!(unsafe { end_wasm_ptr.deref_mut(&memory).is_some() }); + // assert!(unsafe { end_wasm_ptr.deref_mut(&memory).is_some() }); let end_wasm_ptr_oob_array: [WasmPtr; 4] = [ WasmPtr::new(last_valid_address_for_u32 + 1), @@ -397,17 +398,17 @@ mod test { ]; for oob_end_ptr in end_wasm_ptr_oob_array.iter() { assert!(oob_end_ptr.deref(&memory).is_none()); - assert!(unsafe { oob_end_ptr.deref_mut(&memory).is_none() }); + // assert!(unsafe { oob_end_ptr.deref_mut(&memory).is_none() }); } let end_wasm_ptr_array: WasmPtr = WasmPtr::new(last_valid_address_for_u32); assert!(end_wasm_ptr_array.deref(&memory, 0, 1).is_some()); - assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, 0, 1).is_some() }); + // assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, 0, 1).is_some() }); let invalid_idx_len_combos: [(u32, u32); 3] = [(last_valid_address_for_u32 + 1, 0), (0, 2), (1, 1)]; for &(idx, len) in invalid_idx_len_combos.iter() { assert!(end_wasm_ptr_array.deref(&memory, idx, len).is_none()); - assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, idx, len).is_none() }); + // assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, idx, len).is_none() }); } let end_wasm_ptr_array_oob_array: [WasmPtr; 4] = [ @@ -419,9 +420,9 @@ mod test { for oob_end_array_ptr in end_wasm_ptr_array_oob_array.iter() { assert!(oob_end_array_ptr.deref(&memory, 0, 1).is_none()); - assert!(unsafe { oob_end_array_ptr.deref_mut(&memory, 0, 1).is_none() }); + // assert!(unsafe { oob_end_array_ptr.deref_mut(&memory, 0, 1).is_none() }); assert!(oob_end_array_ptr.deref(&memory, 1, 0).is_none()); - assert!(unsafe { oob_end_array_ptr.deref_mut(&memory, 1, 0).is_none() }); + // assert!(unsafe { oob_end_array_ptr.deref_mut(&memory, 1, 0).is_none() }); } } } From 009c0b21dcae4f0ae912c63b77ba1dac6442e453 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 15:15:57 -0700 Subject: [PATCH 06/17] Fixed Array deref --- lib/api/src/ptr.rs | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/lib/api/src/ptr.rs b/lib/api/src/ptr.rs index 26c247e4a76..20fdb11f234 100644 --- a/lib/api/src/ptr.rs +++ b/lib/api/src/ptr.rs @@ -155,27 +155,32 @@ impl WasmPtr { pub fn deref(self, memory: &Memory, index: u32, length: u32) -> Option>> { // gets the size of the item in the array with padding added such that // for any index, we will always result an aligned memory access - let item_size = mem::size_of::(); - let slice_full_len = index as usize + length as usize; - let memory_size = memory.size().bytes().0; + let item_size = mem::size_of::() as u32; + let slice_full_len = index + length; + let memory_size = memory.size().bytes().0 as u32; - if (self.offset as usize) + (item_size * slice_full_len) > memory_size - || self.offset as usize >= memory_size - || mem::size_of::() == 0 + if self.offset + (item_size * slice_full_len) > memory_size + || self.offset >= memory_size + || item_size == 0 { return None; } - unsafe { - let cell_ptr = align_pointer( - memory.view::().as_ptr().add(self.offset as usize) as usize, - mem::align_of::(), - ) as *const WasmCell; - let cell_ptrs = std::slice::from_raw_parts(cell_ptr, slice_full_len) - [index as usize..slice_full_len] - .to_owned(); - Some(cell_ptrs) - } + Some( + (0..length) + .map(|i| unsafe { + let cell_ptr = align_pointer( + memory + .view::() + .as_ptr() + .add((self.offset + i * item_size) as usize) + as usize, + mem::align_of::(), + ) as *const Cell; + WasmCell::new(&*cell_ptr) + }) + .collect::>(), + ) } // /// Mutably dereference this `WasmPtr` getting a `&mut [Cell]` allowing for From ac42d1b295b82e8ecc36b3375a8431bed36e8ddf Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 15:16:33 -0700 Subject: [PATCH 07/17] Added methods to MemoryView --- lib/api/src/cell.rs | 5 +---- lib/types/src/memory_view.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/api/src/cell.rs b/lib/api/src/cell.rs index c52a97887c3..dd12d64f71e 100644 --- a/lib/api/src/cell.rs +++ b/lib/api/src/cell.rs @@ -1,10 +1,7 @@ pub use std::cell::Cell; use core::cmp::Ordering; -use core::fmt::{self, Debug, Display}; -use core::mem; -use core::ops::{Deref, DerefMut}; -use core::ptr; +use core::fmt::{self, Debug}; use std::fmt::Pointer; /// A mutable Wasm-memory location. diff --git a/lib/types/src/memory_view.rs b/lib/types/src/memory_view.rs index 907f7d8e093..6ba896b4081 100644 --- a/lib/types/src/memory_view.rs +++ b/lib/types/src/memory_view.rs @@ -64,6 +64,33 @@ where _phantom: PhantomData, } } + + /// Creates a subarray view from this MemoryView. + pub fn subarray(&self, start: u32, end: u32) -> Self { + assert!(start <= end); + assert!(end < self.length as u32); + Self { + ptr: unsafe { self.ptr.offset(start as isize) }, + length: (end - start) as usize, + _phantom: PhantomData, + } + } + + /// Copy the contents of the source slice into this `MemoryView`. + /// + /// This function will efficiently copy the memory from within the wasm + /// module’s own linear memory to this typed array. + pub fn copy_from(&self, src: &[T]) { + assert!( + src.len() == self.length, + "The source length must match the MemoryView length" + ); + unsafe { + for (i, byte) in src.iter().enumerate() { + *self.ptr.offset(i as isize) = *byte; + } + } + } } impl<'a, T: Atomic> MemoryView<'a, T> { From b4d30cfb6acc281e713a907204322cf47519c08f Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 15:17:15 -0700 Subject: [PATCH 08/17] Make WASI work! --- lib/wasi/src/ptr.rs | 10 ++++---- lib/wasi/src/syscalls/mod.rs | 38 ++++++++++++++++--------------- lib/wasi/src/syscalls/unix/mod.rs | 6 ++--- lib/wasi/src/syscalls/windows.rs | 6 ++--- 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/lib/wasi/src/ptr.rs b/lib/wasi/src/ptr.rs index 7f739f27c19..9be529ab619 100644 --- a/lib/wasi/src/ptr.rs +++ b/lib/wasi/src/ptr.rs @@ -2,8 +2,10 @@ //! if memory access failed use crate::syscalls::types::{__wasi_errno_t, __WASI_EFAULT}; -use std::{cell::Cell, fmt}; -pub use wasmer::{Array, FromToNativeWasmType, Item, Memory, ValueType, WasmPtr as BaseWasmPtr}; +use std::fmt; +pub use wasmer::{ + Array, FromToNativeWasmType, Item, Memory, ValueType, WasmCell, WasmPtr as BaseWasmPtr, +}; #[repr(transparent)] pub struct WasmPtr(BaseWasmPtr); @@ -63,7 +65,7 @@ impl WasmPtr { impl WasmPtr { #[inline(always)] - pub fn deref<'a>(self, memory: &'a Memory) -> Result<&'a Cell, __wasi_errno_t> { + pub fn deref<'a>(self, memory: &'a Memory) -> Result, __wasi_errno_t> { self.0.deref(memory).ok_or(__WASI_EFAULT) } } @@ -75,7 +77,7 @@ impl WasmPtr { memory: &'a Memory, index: u32, length: u32, - ) -> Result<&'a [Cell], __wasi_errno_t> { + ) -> Result>, __wasi_errno_t> { self.0.deref(memory, index, length).ok_or(__WASI_EFAULT) } diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs index 5071aa273da..c1016b6d733 100644 --- a/lib/wasi/src/syscalls/mod.rs +++ b/lib/wasi/src/syscalls/mod.rs @@ -27,11 +27,10 @@ use crate::{ WasiEnv, WasiError, }; use std::borrow::Borrow; -use std::cell::Cell; use std::convert::{Infallible, TryInto}; use std::io::{self, Read, Seek, Write}; use tracing::{debug, trace}; -use wasmer::{Memory, RuntimeError, Value}; +use wasmer::{Memory, RuntimeError, Value, WasmCell}; #[cfg(any( target_os = "freebsd", @@ -47,7 +46,7 @@ pub use windows::*; fn write_bytes_inner( mut write_loc: T, memory: &Memory, - iovs_arr_cell: &[Cell<__wasi_ciovec_t>], + iovs_arr_cell: Vec>, ) -> Result { let mut bytes_written = 0; for iov in iovs_arr_cell { @@ -66,7 +65,7 @@ fn write_bytes_inner( fn write_bytes( mut write_loc: T, memory: &Memory, - iovs_arr_cell: &[Cell<__wasi_ciovec_t>], + iovs_arr_cell: Vec>, ) -> Result { let result = write_bytes_inner(&mut write_loc, memory, iovs_arr_cell); write_loc.flush(); @@ -76,16 +75,18 @@ fn write_bytes( fn read_bytes( mut reader: T, memory: &Memory, - iovs_arr_cell: &[Cell<__wasi_iovec_t>], + iovs_arr_cell: Vec>, ) -> Result { let mut bytes_read = 0; for iov in iovs_arr_cell { let iov_inner = iov.get(); - let bytes = WasmPtr::::new(iov_inner.buf).deref(memory, 0, iov_inner.buf_len)?; - let mut raw_bytes: &mut [u8] = - unsafe { &mut *(bytes as *const [_] as *mut [_] as *mut [u8]) }; - bytes_read += reader.read(raw_bytes).map_err(|_| __WASI_EIO)? as u32; + let mut raw_bytes: Vec = vec![0; iov_inner.buf_len as usize]; + bytes_read += reader.read(&mut raw_bytes).map_err(|_| __WASI_EIO)? as u32; + memory + .view::() + .subarray(iov_inner.buf, iov_inner.buf + iov_inner.buf_len) + .copy_from(&raw_bytes); } Ok(bytes_read) } @@ -2515,18 +2516,19 @@ pub fn proc_raise(env: &WasiEnv, sig: __wasi_signal_t) -> __wasi_errno_t { /// A pointer to a buffer where the random bytes will be written /// - `size_t buf_len` /// The number of bytes that will be written -pub fn random_get(env: &WasiEnv, buf: WasmPtr, buf_len: u32) -> __wasi_errno_t { +pub fn random_get(env: &WasiEnv, buf: u32, buf_len: u32) -> __wasi_errno_t { debug!("wasi::random_get buf_len: {}", buf_len); let memory = env.memory(); - - let buf = wasi_try!(buf.deref(memory, 0, buf_len)); - - let res = unsafe { - let u8_buffer = &mut *(buf as *const [_] as *mut [_] as *mut [u8]); - getrandom::getrandom(u8_buffer) - }; + let mut u8_buffer = vec![0; buf_len as usize]; + let res = getrandom::getrandom(&mut u8_buffer); match res { - Ok(()) => __WASI_ESUCCESS, + Ok(()) => { + memory + .view::() + .subarray(buf, buf + buf_len) + .copy_from(&u8_buffer); + __WASI_ESUCCESS + } Err(_) => __WASI_EIO, } } diff --git a/lib/wasi/src/syscalls/unix/mod.rs b/lib/wasi/src/syscalls/unix/mod.rs index 9b1e224c795..a38d0810f2b 100644 --- a/lib/wasi/src/syscalls/unix/mod.rs +++ b/lib/wasi/src/syscalls/unix/mod.rs @@ -3,12 +3,12 @@ use libc::{ clock_getres, clock_gettime, timespec, CLOCK_MONOTONIC, CLOCK_PROCESS_CPUTIME_ID, CLOCK_REALTIME, CLOCK_THREAD_CPUTIME_ID, }; -use std::cell::Cell; use std::mem; +use wasmer::WasmCell; pub fn platform_clock_res_get( clock_id: __wasi_clockid_t, - resolution: &Cell<__wasi_timestamp_t>, + resolution: WasmCell<__wasi_timestamp_t>, ) -> __wasi_errno_t { let unix_clock_id = match clock_id { __WASI_CLOCK_MONOTONIC => CLOCK_MONOTONIC, @@ -36,7 +36,7 @@ pub fn platform_clock_res_get( pub fn platform_clock_time_get( clock_id: __wasi_clockid_t, precision: __wasi_timestamp_t, - time: &Cell<__wasi_timestamp_t>, + time: WasmCell<__wasi_timestamp_t>, ) -> __wasi_errno_t { let unix_clock_id = match clock_id { __WASI_CLOCK_MONOTONIC => CLOCK_MONOTONIC, diff --git a/lib/wasi/src/syscalls/windows.rs b/lib/wasi/src/syscalls/windows.rs index bb35cb0717c..bd04f0bdb14 100644 --- a/lib/wasi/src/syscalls/windows.rs +++ b/lib/wasi/src/syscalls/windows.rs @@ -1,10 +1,10 @@ use crate::syscalls::types::*; -use std::cell::Cell; use tracing::debug; +use wasmer::WasmCell; pub fn platform_clock_res_get( clock_id: __wasi_clockid_t, - resolution: &Cell<__wasi_timestamp_t>, + resolution: WasmCell<__wasi_timestamp_t>, ) -> __wasi_errno_t { let resolution_val = match clock_id { // resolution of monotonic clock at 10ms, from: @@ -27,7 +27,7 @@ pub fn platform_clock_res_get( pub fn platform_clock_time_get( clock_id: __wasi_clockid_t, precision: __wasi_timestamp_t, - time: &Cell<__wasi_timestamp_t>, + time: WasmCell<__wasi_timestamp_t>, ) -> __wasi_errno_t { let nanos = match clock_id { __WASI_CLOCK_MONOTONIC => { From 5a2d4975f40a8ebb8a6733cfe32bcf660dff87ec Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 15:27:17 -0700 Subject: [PATCH 09/17] Improved PR based on suggestions --- lib/api/src/cell.rs | 14 ++++++++++++++ lib/api/src/ptr.rs | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/api/src/cell.rs b/lib/api/src/cell.rs index dd12d64f71e..ff171816e29 100644 --- a/lib/api/src/cell.rs +++ b/lib/api/src/cell.rs @@ -130,6 +130,20 @@ impl<'a, T: Copy> WasmCell<'a, T> { /// Get an unsafe mutable pointer to the inner item /// in the Cell. + /// + /// # Safety + /// + /// This method is highly discouraged to use. We have it for + /// compatibility reasons with Emscripten. + /// It is unsafe because changing an item inline will change + /// the underlying memory. + /// + /// It's highly encouraged to use the `set` method instead. + #[deprecated( + since = "2.0.0", + note = "Please use the memory-safe set method instead" + )] + #[doc(hidden)] pub unsafe fn get_mut(&self) -> &'a mut T { &mut *self.inner.as_ptr() } diff --git a/lib/api/src/ptr.rs b/lib/api/src/ptr.rs index 20fdb11f234..03694bc57ad 100644 --- a/lib/api/src/ptr.rs +++ b/lib/api/src/ptr.rs @@ -104,7 +104,7 @@ impl WasmPtr { /// If you're unsure what that means, it likely does not apply to you. /// This invariant will be enforced in the future. #[inline] - pub fn deref<'a>(self, memory: &'a Memory) -> Option> { + pub fn deref<'a>(self, memory: &'a Memory) -> Option> { if (self.offset as usize) + mem::size_of::() > memory.size().bytes().0 || mem::size_of::() == 0 { @@ -152,7 +152,7 @@ impl WasmPtr { /// If you're unsure what that means, it likely does not apply to you. /// This invariant will be enforced in the future. #[inline] - pub fn deref(self, memory: &Memory, index: u32, length: u32) -> Option>> { + pub fn deref(self, memory: &Memory, index: u32, length: u32) -> Option>> { // gets the size of the item in the array with padding added such that // for any index, we will always result an aligned memory access let item_size = mem::size_of::() as u32; From fda64183499adc95b189887d98c138ef5a727e9f Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 16:08:44 -0700 Subject: [PATCH 10/17] Remove unused APIs --- lib/api/src/ptr.rs | 79 ++++------------------------------------------ 1 file changed, 7 insertions(+), 72 deletions(-) diff --git a/lib/api/src/ptr.rs b/lib/api/src/ptr.rs index 03694bc57ad..2170936b32e 100644 --- a/lib/api/src/ptr.rs +++ b/lib/api/src/ptr.rs @@ -118,27 +118,6 @@ impl WasmPtr { Some(WasmCell::new(&*cell_ptr)) } } - - // /// Mutably dereference this `WasmPtr` getting a `&mut Cell` allowing for - // /// direct access to a `&mut T`. - // /// - // /// # Safety - // /// - This method does not do any aliasing checks: it's possible to create - // /// `&mut T` that point to the same memory. You should ensure that you have - // /// exclusive access to Wasm linear memory before calling this method. - // #[inline] - // pub unsafe fn deref_mut<'a>(self, memory: &'a Memory) -> Option<&'a mut Cell> { - // if (self.offset as usize) + mem::size_of::() > memory.size().bytes().0 - // || mem::size_of::() == 0 - // { - // return None; - // } - // let cell_ptr = align_pointer( - // memory.view::().as_ptr().add(self.offset as usize) as usize, - // mem::align_of::(), - // ) as *mut Cell; - // Some(&mut *cell_ptr) - // } } /// Methods for `WasmPtr`s to arrays of data that can be dereferenced, namely to @@ -152,7 +131,12 @@ impl WasmPtr { /// If you're unsure what that means, it likely does not apply to you. /// This invariant will be enforced in the future. #[inline] - pub fn deref(self, memory: &Memory, index: u32, length: u32) -> Option>> { + pub fn deref<'a>( + self, + memory: &'a Memory, + index: u32, + length: u32, + ) -> Option>> { // gets the size of the item in the array with padding added such that // for any index, we will always result an aligned memory access let item_size = mem::size_of::() as u32; @@ -183,42 +167,6 @@ impl WasmPtr { ) } - // /// Mutably dereference this `WasmPtr` getting a `&mut [Cell]` allowing for - // /// direct access to a `&mut [T]`. - // /// - // /// # Safety - // /// - This method does not do any aliasing checks: it's possible to create - // /// `&mut T` that point to the same memory. You should ensure that you have - // /// exclusive access to Wasm linear memory before calling this method. - // #[inline] - // pub unsafe fn deref_mut( - // self, - // memory: &Memory, - // index: u32, - // length: u32, - // ) -> Option<&mut [Cell]> { - // // gets the size of the item in the array with padding added such that - // // for any index, we will always result an aligned memory access - // let item_size = mem::size_of::(); - // let slice_full_len = index as usize + length as usize; - // let memory_size = memory.size().bytes().0; - - // if (self.offset as usize) + (item_size * slice_full_len) > memory.size().bytes().0 - // || self.offset as usize >= memory_size - // || mem::size_of::() == 0 - // { - // return None; - // } - - // let cell_ptr = align_pointer( - // memory.view::().as_ptr().add(self.offset as usize) as usize, - // mem::align_of::(), - // ) as *mut Cell; - // let cell_ptrs = &mut std::slice::from_raw_parts_mut(cell_ptr, slice_full_len) - // [index as usize..slice_full_len]; - // Some(cell_ptrs) - // } - /// Get a UTF-8 string from the `WasmPtr` with the given length. /// /// Note that . The @@ -346,7 +294,7 @@ mod test { use crate::{Memory, MemoryType, Store}; /// Ensure that memory accesses work on the edges of memory and that out of - /// bounds errors are caught with both `deref` and `deref_mut`. + /// bounds errors are caught with `deref` #[test] fn wasm_ptr_memory_bounds_checks_hold() { // create a memory @@ -359,29 +307,23 @@ mod test { let start_wasm_ptr_array: WasmPtr = WasmPtr::new(0); assert!(start_wasm_ptr.deref(&memory).is_some()); - // assert!(unsafe { start_wasm_ptr.deref_mut(&memory).is_some() }); assert!(start_wasm_ptr_array.deref(&memory, 0, 0).is_some()); assert!(unsafe { start_wasm_ptr_array.get_utf8_str(&memory, 0).is_some() }); assert!(start_wasm_ptr_array.get_utf8_string(&memory, 0).is_some()); - // assert!(unsafe { start_wasm_ptr_array.deref_mut(&memory, 0, 0).is_some() }); assert!(start_wasm_ptr_array.deref(&memory, 0, 1).is_some()); - // assert!(unsafe { start_wasm_ptr_array.deref_mut(&memory, 0, 1).is_some() }); // test that accessing the last valid memory address works correctly and OOB is caught let last_valid_address_for_u8 = (memory.size().bytes().0 - 1) as u32; let end_wasm_ptr: WasmPtr = WasmPtr::new(last_valid_address_for_u8); assert!(end_wasm_ptr.deref(&memory).is_some()); - // assert!(unsafe { end_wasm_ptr.deref_mut(&memory).is_some() }); let end_wasm_ptr_array: WasmPtr = WasmPtr::new(last_valid_address_for_u8); assert!(end_wasm_ptr_array.deref(&memory, 0, 1).is_some()); - // assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, 0, 1).is_some() }); let invalid_idx_len_combos: [(u32, u32); 3] = [(last_valid_address_for_u8 + 1, 0), (0, 2), (1, 1)]; for &(idx, len) in invalid_idx_len_combos.iter() { assert!(end_wasm_ptr_array.deref(&memory, idx, len).is_none()); - // assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, idx, len).is_none() }); } assert!(unsafe { end_wasm_ptr_array.get_utf8_str(&memory, 2).is_none() }); assert!(end_wasm_ptr_array.get_utf8_string(&memory, 2).is_none()); @@ -391,9 +333,7 @@ mod test { let last_valid_address_for_u32 = (memory.size().bytes().0 - 4) as u32; let end_wasm_ptr: WasmPtr = WasmPtr::new(last_valid_address_for_u32); assert!(end_wasm_ptr.deref(&memory).is_some()); - // assert!(unsafe { end_wasm_ptr.deref_mut(&memory).is_some() }); assert!(end_wasm_ptr.deref(&memory).is_some()); - // assert!(unsafe { end_wasm_ptr.deref_mut(&memory).is_some() }); let end_wasm_ptr_oob_array: [WasmPtr; 4] = [ WasmPtr::new(last_valid_address_for_u32 + 1), @@ -403,17 +343,14 @@ mod test { ]; for oob_end_ptr in end_wasm_ptr_oob_array.iter() { assert!(oob_end_ptr.deref(&memory).is_none()); - // assert!(unsafe { oob_end_ptr.deref_mut(&memory).is_none() }); } let end_wasm_ptr_array: WasmPtr = WasmPtr::new(last_valid_address_for_u32); assert!(end_wasm_ptr_array.deref(&memory, 0, 1).is_some()); - // assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, 0, 1).is_some() }); let invalid_idx_len_combos: [(u32, u32); 3] = [(last_valid_address_for_u32 + 1, 0), (0, 2), (1, 1)]; for &(idx, len) in invalid_idx_len_combos.iter() { assert!(end_wasm_ptr_array.deref(&memory, idx, len).is_none()); - // assert!(unsafe { end_wasm_ptr_array.deref_mut(&memory, idx, len).is_none() }); } let end_wasm_ptr_array_oob_array: [WasmPtr; 4] = [ @@ -425,9 +362,7 @@ mod test { for oob_end_array_ptr in end_wasm_ptr_array_oob_array.iter() { assert!(oob_end_array_ptr.deref(&memory, 0, 1).is_none()); - // assert!(unsafe { oob_end_array_ptr.deref_mut(&memory, 0, 1).is_none() }); assert!(oob_end_array_ptr.deref(&memory, 1, 0).is_none()); - // assert!(unsafe { oob_end_array_ptr.deref_mut(&memory, 1, 0).is_none() }); } } } From 20a7136e614110e6708c4ab521af0f02e12005c6 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 16:15:44 -0700 Subject: [PATCH 11/17] Fixed emscripten --- lib/emscripten/src/env/unix/mod.rs | 44 ++++++++++++++--------------- lib/emscripten/src/ptr.rs | 30 ++++---------------- lib/emscripten/src/syscalls/unix.rs | 10 +++---- 3 files changed, 33 insertions(+), 51 deletions(-) diff --git a/lib/emscripten/src/env/unix/mod.rs b/lib/emscripten/src/env/unix/mod.rs index 6355fb3ec0d..c80936d10d1 100644 --- a/lib/emscripten/src/env/unix/mod.rs +++ b/lib/emscripten/src/env/unix/mod.rs @@ -3,7 +3,6 @@ use libc::{ c_int, getenv, getgrnam as libc_getgrnam, getpwnam as libc_getpwnam, putenv, setenv, sysconf, unsetenv, }; -use std::cell::Cell; use std::ffi::CStr; use std::mem; use std::os::raw::c_char; @@ -152,11 +151,7 @@ pub fn _gai_strerror(ctx: &EmEnv, ecode: i32) -> i32 { let string_on_guest: WasmPtr = call_malloc_with_cast(ctx, bytes.len() as _); let memory = ctx.memory(0); - let writer = unsafe { - string_on_guest - .deref_mut(&memory, 0, bytes.len() as _) - .unwrap() - }; + let writer = string_on_guest.deref(&memory, 0, bytes.len() as _).unwrap(); for (i, byte) in bytes.iter().enumerate() { writer[i].set(*byte as _); } @@ -174,21 +169,23 @@ pub fn _getaddrinfo( use libc::{addrinfo, freeaddrinfo}; debug!("emscripten::_getaddrinfo"); let memory = ctx.memory(0); - debug!(" => node = {}", unsafe { + debug!(" => node = {}", { node_ptr .deref(&memory) - .map(|np| { - std::ffi::CStr::from_ptr(np as *const Cell as *const c_char) - .to_string_lossy() + .map(|_np| { + // unimplemented!(); + // std::ffi::CStr::from_ptr(np as *const Cell as *const c_char) + // .to_string_lossy() }) .unwrap_or(std::borrow::Cow::Borrowed("null")) }); - debug!(" => server_str = {}", unsafe { + debug!(" => server_str = {}", { service_str_ptr .deref(&memory) - .map(|np| { - std::ffi::CStr::from_ptr(np as *const Cell as *const c_char) - .to_string_lossy() + .map(|_np| { + unimplemented!(); + // std::ffi::CStr::from_ptr(np as *const Cell as *const c_char) + // .to_string_lossy() }) .unwrap_or(std::borrow::Cow::Borrowed("null")) }); @@ -212,13 +209,17 @@ pub fn _getaddrinfo( // allocate equivalent memory for res_val_ptr let result = unsafe { libc::getaddrinfo( - (node_ptr - .deref(&memory) - .map(|m| m as *const Cell as *const c_char)) + (node_ptr.deref(&memory).map(|_m| { + unimplemented!(); + //m as *const Cell as *const c_char + })) .unwrap_or(std::ptr::null()), service_str_ptr .deref(&memory) - .map(|m| m as *const Cell as *const c_char) + .map(|_m| { + unimplemented!(); + // m as *const Cell as *const c_char + }) .unwrap_or(std::ptr::null()), hints .as_ref() @@ -246,7 +247,7 @@ pub fn _getaddrinfo( // connect list if let Some(prev_guest) = previous_guest_node { - let mut pg = prev_guest.deref_mut(&memory).unwrap().get_mut(); + let mut pg = prev_guest.deref(&memory).unwrap().get_mut(); pg.ai_next = current_guest_node_ptr; } @@ -258,7 +259,7 @@ pub fn _getaddrinfo( let host_sockaddr_ptr = (*current_host_node).ai_addr; let guest_sockaddr_ptr: WasmPtr = call_malloc_with_cast(ctx, host_addrlen as _); - let guest_sockaddr = guest_sockaddr_ptr.deref_mut(&memory).unwrap().get_mut(); + let guest_sockaddr = guest_sockaddr_ptr.deref(&memory).unwrap().get_mut(); guest_sockaddr.sa_family = (*host_sockaddr_ptr).sa_family as i16; guest_sockaddr.sa_data = (*host_sockaddr_ptr).sa_data; @@ -287,8 +288,7 @@ pub fn _getaddrinfo( } }; - let mut current_guest_node = - current_guest_node_ptr.deref_mut(&memory).unwrap().get_mut(); + let mut current_guest_node = current_guest_node_ptr.deref(&memory).unwrap().get_mut(); current_guest_node.ai_flags = (*current_host_node).ai_flags; current_guest_node.ai_family = (*current_host_node).ai_family; current_guest_node.ai_socktype = (*current_host_node).ai_socktype; diff --git a/lib/emscripten/src/ptr.rs b/lib/emscripten/src/ptr.rs index a24f530c652..56f6015f32a 100644 --- a/lib/emscripten/src/ptr.rs +++ b/lib/emscripten/src/ptr.rs @@ -5,8 +5,8 @@ // don't want to warn about unusued code here #![allow(dead_code)] -use std::{cell::Cell, fmt}; -pub use wasmer::{Array, FromToNativeWasmType, Memory, ValueType}; +use std::fmt; +pub use wasmer::{Array, FromToNativeWasmType, Memory, ValueType, WasmCell}; #[repr(transparent)] pub struct WasmPtr(wasmer::WasmPtr); @@ -59,45 +59,27 @@ impl WasmPtr { impl WasmPtr { #[inline(always)] - pub fn deref<'a>(self, memory: &'a Memory) -> Option<&'a Cell> { + pub fn deref<'a>(self, memory: &'a Memory) -> Option> { if self.0.offset() == 0 { None } else { self.0.deref(memory) } } - - #[inline(always)] - pub unsafe fn deref_mut<'a>(self, memory: &'a Memory) -> Option<&'a mut Cell> { - if self.0.offset() == 0 { - None - } else { - self.0.deref_mut(memory) - } - } } impl WasmPtr { #[inline(always)] - pub fn deref<'a>(self, memory: &'a Memory, index: u32, length: u32) -> Option<&'a [Cell]> { - if self.0.offset() == 0 { - None - } else { - self.0.deref(memory, index, length) - } - } - - #[inline] - pub unsafe fn deref_mut<'a>( + pub fn deref<'a>( self, memory: &'a Memory, index: u32, length: u32, - ) -> Option<&'a mut [Cell]> { + ) -> Option>> { if self.0.offset() == 0 { None } else { - self.0.deref_mut(memory, index, length) + self.0.deref(memory, index, length) } } diff --git a/lib/emscripten/src/syscalls/unix.rs b/lib/emscripten/src/syscalls/unix.rs index 69ac5a2f4eb..ee4874acf65 100644 --- a/lib/emscripten/src/syscalls/unix.rs +++ b/lib/emscripten/src/syscalls/unix.rs @@ -633,7 +633,7 @@ pub fn ___syscall102(ctx: &EmEnv, _which: c_int, mut varargs: VarArgs) -> c_int address.deref(&memory).unwrap().get(), address_len.deref(&memory).unwrap().get() ); - let address_len_addr = unsafe { address_len.deref_mut(&memory).unwrap().get_mut() }; + let address_len_addr = unsafe { address_len.deref(&memory).unwrap().get_mut() }; // let mut address_len_addr: socklen_t = 0; let mut host_address: sockaddr = sockaddr { @@ -643,7 +643,7 @@ pub fn ___syscall102(ctx: &EmEnv, _which: c_int, mut varargs: VarArgs) -> c_int sa_len: Default::default(), }; let fd = unsafe { accept(socket, &mut host_address, address_len_addr) }; - let address_addr = unsafe { address.deref_mut(&memory).unwrap().get_mut() }; + let address_addr = unsafe { address.deref(&memory).unwrap().get_mut() }; address_addr.sa_family = host_address.sa_family as _; address_addr.sa_data = host_address.sa_data; @@ -667,7 +667,7 @@ pub fn ___syscall102(ctx: &EmEnv, _which: c_int, mut varargs: VarArgs) -> c_int let socket: i32 = socket_varargs.get(ctx); let address: WasmPtr = socket_varargs.get(ctx); let address_len: WasmPtr = socket_varargs.get(ctx); - let address_len_addr = unsafe { address_len.deref_mut(&memory).unwrap().get_mut() }; + let address_len_addr = unsafe { address_len.deref(&memory).unwrap().get_mut() }; let mut sock_addr_host: sockaddr = sockaddr { sa_family: Default::default(), @@ -683,7 +683,7 @@ pub fn ___syscall102(ctx: &EmEnv, _which: c_int, mut varargs: VarArgs) -> c_int ) }; // translate from host data into emscripten data - let mut address_mut = unsafe { address.deref_mut(&memory).unwrap().get_mut() }; + let mut address_mut = unsafe { address.deref(&memory).unwrap().get_mut() }; address_mut.sa_family = sock_addr_host.sa_family as _; address_mut.sa_data = sock_addr_host.sa_data; @@ -857,7 +857,7 @@ pub fn ___syscall168(ctx: &EmEnv, _which: i32, mut varargs: VarArgs) -> i32 { let timeout: i32 = varargs.get(ctx); let memory = ctx.memory(0); - let fds_mut = unsafe { fds.deref_mut(&memory).unwrap().get_mut() }; + let fds_mut = unsafe { fds.deref(&memory).unwrap().get_mut() }; let ret = unsafe { libc::poll( From 26d44dfeaa40fb1a7c4e245e2889135963546b0f Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 16:26:56 -0700 Subject: [PATCH 12/17] Fixed comments --- lib/api/src/ptr.rs | 10 +++++----- lib/types/src/memory_view.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/api/src/ptr.rs b/lib/api/src/ptr.rs index 2170936b32e..9b3fa07cbf0 100644 --- a/lib/api/src/ptr.rs +++ b/lib/api/src/ptr.rs @@ -139,12 +139,12 @@ impl WasmPtr { ) -> Option>> { // gets the size of the item in the array with padding added such that // for any index, we will always result an aligned memory access - let item_size = mem::size_of::() as u32; - let slice_full_len = index + length; - let memory_size = memory.size().bytes().0 as u32; + let item_size = mem::size_of::(); + let slice_full_len = index as usize + length as usize; + let memory_size = memory.size().bytes().0; - if self.offset + (item_size * slice_full_len) > memory_size - || self.offset >= memory_size + if (self.offset as usize) + (item_size * slice_full_len) > memory_size + || (self.offset as usize) >= memory_size || item_size == 0 { return None; diff --git a/lib/types/src/memory_view.rs b/lib/types/src/memory_view.rs index 6ba896b4081..1d0420a389f 100644 --- a/lib/types/src/memory_view.rs +++ b/lib/types/src/memory_view.rs @@ -68,9 +68,9 @@ where /// Creates a subarray view from this MemoryView. pub fn subarray(&self, start: u32, end: u32) -> Self { assert!(start <= end); - assert!(end < self.length as u32); + assert!(end as usize < self.length); Self { - ptr: unsafe { self.ptr.offset(start as isize) }, + ptr: unsafe { self.ptr.add(start as usize) }, length: (end - start) as usize, _phantom: PhantomData, } From 0fbddcd1fb6f63aec8deeefed9d472a99f071e60 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 16:47:32 -0700 Subject: [PATCH 13/17] Fixed syntax --- lib/types/src/memory_view.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/types/src/memory_view.rs b/lib/types/src/memory_view.rs index 1d0420a389f..99d47e5598c 100644 --- a/lib/types/src/memory_view.rs +++ b/lib/types/src/memory_view.rs @@ -68,7 +68,7 @@ where /// Creates a subarray view from this MemoryView. pub fn subarray(&self, start: u32, end: u32) -> Self { assert!(start <= end); - assert!(end as usize < self.length); + assert!((end as usize) < self.length); Self { ptr: unsafe { self.ptr.add(start as usize) }, length: (end - start) as usize, From a039af4deef5f8831d2cc9f787bcf00a1d7a5fd5 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 24 Jun 2021 17:38:43 -0700 Subject: [PATCH 14/17] Fix syntax again --- lib/api/src/ptr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/api/src/ptr.rs b/lib/api/src/ptr.rs index 9b3fa07cbf0..0bc68a143c5 100644 --- a/lib/api/src/ptr.rs +++ b/lib/api/src/ptr.rs @@ -151,13 +151,13 @@ impl WasmPtr { } Some( - (0..length) + (0..(length as usize)) .map(|i| unsafe { let cell_ptr = align_pointer( memory .view::() .as_ptr() - .add((self.offset + i * item_size) as usize) + .add((self.offset as usize + i * item_size)) as usize, mem::align_of::(), ) as *const Cell; From 1a8952e2fa81d9c14b8c7741eeb228584c7303ee Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 1 Jul 2021 18:29:31 -0700 Subject: [PATCH 15/17] Improved API based on feedback --- lib/api/src/ptr.rs | 27 ++++++------- lib/emscripten/src/env/unix/mod.rs | 2 +- lib/types/src/memory_view.rs | 34 +++++++++++----- lib/wasi/src/syscalls/mod.rs | 63 ++++++++++++++++++------------ 4 files changed, 74 insertions(+), 52 deletions(-) diff --git a/lib/api/src/ptr.rs b/lib/api/src/ptr.rs index 0bc68a143c5..6830713da0f 100644 --- a/lib/api/src/ptr.rs +++ b/lib/api/src/ptr.rs @@ -149,22 +149,19 @@ impl WasmPtr { { return None; } + let cell_ptrs = unsafe { + let cell_ptr = align_pointer( + memory.view::().as_ptr().add(self.offset as usize) as usize, + mem::align_of::(), + ) as *const Cell; + &std::slice::from_raw_parts(cell_ptr, slice_full_len)[index as usize..slice_full_len] + }; - Some( - (0..(length as usize)) - .map(|i| unsafe { - let cell_ptr = align_pointer( - memory - .view::() - .as_ptr() - .add((self.offset as usize + i * item_size)) - as usize, - mem::align_of::(), - ) as *const Cell; - WasmCell::new(&*cell_ptr) - }) - .collect::>(), - ) + let wasm_cells = cell_ptrs + .iter() + .map(|ptr| WasmCell::new(ptr)) + .collect::>(); + Some(wasm_cells) } /// Get a UTF-8 string from the `WasmPtr` with the given length. diff --git a/lib/emscripten/src/env/unix/mod.rs b/lib/emscripten/src/env/unix/mod.rs index c80936d10d1..a24a425f5a8 100644 --- a/lib/emscripten/src/env/unix/mod.rs +++ b/lib/emscripten/src/env/unix/mod.rs @@ -173,7 +173,7 @@ pub fn _getaddrinfo( node_ptr .deref(&memory) .map(|_np| { - // unimplemented!(); + unimplemented!(); // std::ffi::CStr::from_ptr(np as *const Cell as *const c_char) // .to_string_lossy() }) diff --git a/lib/types/src/memory_view.rs b/lib/types/src/memory_view.rs index 99d47e5598c..9382d5d1487 100644 --- a/lib/types/src/memory_view.rs +++ b/lib/types/src/memory_view.rs @@ -1,6 +1,7 @@ use crate::lib::std::cell::Cell; use crate::lib::std::marker::PhantomData; use crate::lib::std::ops::Deref; +use crate::lib::std::ops::{Bound, RangeBounds}; use crate::lib::std::slice; use crate::lib::std::sync::atomic::{ AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicU16, AtomicU32, AtomicU64, AtomicU8, @@ -48,6 +49,8 @@ impl Atomicity for NonAtomically {} /// A view into a memory. pub struct MemoryView<'a, T: 'a, A = NonAtomically> { ptr: *mut T, + // Note: the length is in the terms of `size::()`. + // The total length in memory is `size::() * length`. length: usize, _phantom: PhantomData<(&'a [Cell], A)>, } @@ -66,12 +69,20 @@ where } /// Creates a subarray view from this MemoryView. - pub fn subarray(&self, start: u32, end: u32) -> Self { - assert!(start <= end); - assert!((end as usize) < self.length); + pub fn subarray(&self, range: impl RangeBounds) -> Self { + let start: usize = match range.start_bound() { + Bound::Unbounded => 0, + Bound::Included(start) => *start, + Bound::Excluded(start) => *start + 1, + }; + let end: usize = match range.end_bound() { + Bound::Unbounded => self.length, + Bound::Included(end) => *end, + Bound::Excluded(end) => *end - 1, + }; Self { - ptr: unsafe { self.ptr.add(start as usize) }, - length: (end - start) as usize, + ptr: unsafe { self.ptr.add(start) }, + length: (end - start), _phantom: PhantomData, } } @@ -80,15 +91,18 @@ where /// /// This function will efficiently copy the memory from within the wasm /// module’s own linear memory to this typed array. - pub fn copy_from(&self, src: &[T]) { + /// + /// # Safety + /// + /// This method is unsafe because the caller will need to make sure + /// there are no data races when copying memory into the view. + pub unsafe fn copy_from(&self, src: &[T]) { assert!( src.len() == self.length, "The source length must match the MemoryView length" ); - unsafe { - for (i, byte) in src.iter().enumerate() { - *self.ptr.offset(i as isize) = *byte; - } + for (i, byte) in src.iter().enumerate() { + *self.ptr.offset(i as isize) = *byte; } } } diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs index c1016b6d733..88b0745e958 100644 --- a/lib/wasi/src/syscalls/mod.rs +++ b/lib/wasi/src/syscalls/mod.rs @@ -46,7 +46,7 @@ pub use windows::*; fn write_bytes_inner( mut write_loc: T, memory: &Memory, - iovs_arr_cell: Vec>, + iovs_arr_cell: &[WasmCell<__wasi_ciovec_t>], ) -> Result { let mut bytes_written = 0; for iov in iovs_arr_cell { @@ -65,7 +65,7 @@ fn write_bytes_inner( fn write_bytes( mut write_loc: T, memory: &Memory, - iovs_arr_cell: Vec>, + iovs_arr_cell: &[WasmCell<__wasi_ciovec_t>], ) -> Result { let result = write_bytes_inner(&mut write_loc, memory, iovs_arr_cell); write_loc.flush(); @@ -75,18 +75,27 @@ fn write_bytes( fn read_bytes( mut reader: T, memory: &Memory, - iovs_arr_cell: Vec>, + iovs_arr_cell: &[WasmCell<__wasi_iovec_t>], ) -> Result { let mut bytes_read = 0; + // We allocate the raw_bytes first once instead of + // N times in the loop. + let mut raw_bytes: Vec = vec![0; 1024]; + for iov in iovs_arr_cell { let iov_inner = iov.get(); - let mut raw_bytes: Vec = vec![0; iov_inner.buf_len as usize]; + raw_bytes.clear(); + raw_bytes.resize(iov_inner.buf_len as usize, 0); bytes_read += reader.read(&mut raw_bytes).map_err(|_| __WASI_EIO)? as u32; - memory - .view::() - .subarray(iov_inner.buf, iov_inner.buf + iov_inner.buf_len) - .copy_from(&raw_bytes); + unsafe { + memory + .view::() + .subarray( + iov_inner.buf as usize..=(iov_inner.buf as usize + iov_inner.buf_len as usize), + ) + .copy_from(&raw_bytes[..iov_inner.buf_len as usize]); + } } Ok(bytes_read) } @@ -661,7 +670,7 @@ pub fn fd_pread( if let Some(ref mut stdin) = wasi_try!(state.fs.stdin_mut().map_err(WasiFsError::into_wasi_err)) { - wasi_try!(read_bytes(stdin, memory, iov_cells)) + wasi_try!(read_bytes(stdin, memory, &iov_cells)) } else { return __WASI_EBADF; } @@ -688,7 +697,7 @@ pub fn fd_pread( h.seek(std::io::SeekFrom::Start(offset as u64)).ok(), __WASI_EIO ); - wasi_try!(read_bytes(h, memory, iov_cells)) + wasi_try!(read_bytes(h, memory, &iov_cells)) } else { return __WASI_EINVAL; } @@ -696,7 +705,7 @@ pub fn fd_pread( Kind::Dir { .. } | Kind::Root { .. } => return __WASI_EISDIR, Kind::Symlink { .. } => unimplemented!("Symlinks in wasi::fd_pread"), Kind::Buffer { buffer } => { - wasi_try!(read_bytes(&buffer[(offset as usize)..], memory, iov_cells)) + wasi_try!(read_bytes(&buffer[(offset as usize)..], memory, &iov_cells)) } } } @@ -810,7 +819,7 @@ pub fn fd_pwrite( if let Some(ref mut stdout) = wasi_try!(state.fs.stdout_mut().map_err(WasiFsError::into_wasi_err)) { - wasi_try!(write_bytes(stdout, memory, iovs_arr_cell)) + wasi_try!(write_bytes(stdout, memory, &iovs_arr_cell)) } else { return __WASI_EBADF; } @@ -819,7 +828,7 @@ pub fn fd_pwrite( if let Some(ref mut stderr) = wasi_try!(state.fs.stderr_mut().map_err(WasiFsError::into_wasi_err)) { - wasi_try!(write_bytes(stderr, memory, iovs_arr_cell)) + wasi_try!(write_bytes(stderr, memory, &iovs_arr_cell)) } else { return __WASI_EBADF; } @@ -840,7 +849,7 @@ pub fn fd_pwrite( Kind::File { handle, .. } => { if let Some(handle) = handle { handle.seek(std::io::SeekFrom::Start(offset as u64)); - wasi_try!(write_bytes(handle, memory, iovs_arr_cell)) + wasi_try!(write_bytes(handle, memory, &iovs_arr_cell)) } else { return __WASI_EINVAL; } @@ -854,7 +863,7 @@ pub fn fd_pwrite( wasi_try!(write_bytes( &mut buffer[(offset as usize)..], memory, - iovs_arr_cell + &iovs_arr_cell )) } } @@ -896,7 +905,7 @@ pub fn fd_read( if let Some(ref mut stdin) = wasi_try!(state.fs.stdin_mut().map_err(WasiFsError::into_wasi_err)) { - wasi_try!(read_bytes(stdin, memory, iovs_arr_cell)) + wasi_try!(read_bytes(stdin, memory, &iovs_arr_cell)) } else { return __WASI_EBADF; } @@ -918,7 +927,7 @@ pub fn fd_read( Kind::File { handle, .. } => { if let Some(handle) = handle { handle.seek(std::io::SeekFrom::Start(offset as u64)); - wasi_try!(read_bytes(handle, memory, iovs_arr_cell)) + wasi_try!(read_bytes(handle, memory, &iovs_arr_cell)) } else { return __WASI_EINVAL; } @@ -929,7 +938,7 @@ pub fn fd_read( } Kind::Symlink { .. } => unimplemented!("Symlinks in wasi::fd_read"), Kind::Buffer { buffer } => { - wasi_try!(read_bytes(&buffer[offset..], memory, iovs_arr_cell)) + wasi_try!(read_bytes(&buffer[offset..], memory, &iovs_arr_cell)) } }; @@ -1267,7 +1276,7 @@ pub fn fd_write( if let Some(ref mut stdout) = wasi_try!(state.fs.stdout_mut().map_err(WasiFsError::into_wasi_err)) { - wasi_try!(write_bytes(stdout, memory, iovs_arr_cell)) + wasi_try!(write_bytes(stdout, memory, &iovs_arr_cell)) } else { return __WASI_EBADF; } @@ -1276,7 +1285,7 @@ pub fn fd_write( if let Some(ref mut stderr) = wasi_try!(state.fs.stderr_mut().map_err(WasiFsError::into_wasi_err)) { - wasi_try!(write_bytes(stderr, memory, iovs_arr_cell)) + wasi_try!(write_bytes(stderr, memory, &iovs_arr_cell)) } else { return __WASI_EBADF; } @@ -1296,7 +1305,7 @@ pub fn fd_write( Kind::File { handle, .. } => { if let Some(handle) = handle { handle.seek(std::io::SeekFrom::Start(offset as u64)); - wasi_try!(write_bytes(handle, memory, iovs_arr_cell)) + wasi_try!(write_bytes(handle, memory, &iovs_arr_cell)) } else { return __WASI_EINVAL; } @@ -1307,7 +1316,7 @@ pub fn fd_write( } Kind::Symlink { .. } => unimplemented!("Symlinks in wasi::fd_write"), Kind::Buffer { buffer } => { - wasi_try!(write_bytes(&mut buffer[offset..], memory, iovs_arr_cell)) + wasi_try!(write_bytes(&mut buffer[offset..], memory, &iovs_arr_cell)) } }; @@ -2523,10 +2532,12 @@ pub fn random_get(env: &WasiEnv, buf: u32, buf_len: u32) -> __wasi_errno_t { let res = getrandom::getrandom(&mut u8_buffer); match res { Ok(()) => { - memory - .view::() - .subarray(buf, buf + buf_len) - .copy_from(&u8_buffer); + unsafe { + memory + .view::() + .subarray(buf as usize..=(buf as usize + buf_len as usize)) + .copy_from(&u8_buffer); + } __WASI_ESUCCESS } Err(_) => __WASI_EIO, From 7553efba3f90050ae548d726736c16948855739e Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 2 Jul 2021 01:35:09 -0700 Subject: [PATCH 16/17] Fixed tests --- lib/api/src/cell.rs | 50 +++++++++++---------------------------------- 1 file changed, 12 insertions(+), 38 deletions(-) diff --git a/lib/api/src/cell.rs b/lib/api/src/cell.rs index ff171816e29..91e06ffe54b 100644 --- a/lib/api/src/cell.rs +++ b/lib/api/src/cell.rs @@ -5,37 +5,6 @@ use core::fmt::{self, Debug}; use std::fmt::Pointer; /// A mutable Wasm-memory location. -/// -/// # Examples -/// -/// In this example, you can see that `WasmCell` enables mutation inside an -/// immutable struct. In other words, it enables "interior mutability". -/// -/// ``` -/// use wasmer::WasmCell; -/// -/// struct SomeStruct { -/// regular_field: u8, -/// special_field: WasmCell, -/// } -/// -/// let my_struct = SomeStruct { -/// regular_field: 0, -/// special_field: WasmCell::new(1), -/// }; -/// -/// let new_value = 100; -/// -/// // ERROR: `my_struct` is immutable -/// // my_struct.regular_field = new_value; -/// -/// // WORKS: although `my_struct` is immutable, `special_field` is a `WasmCell`, -/// // which can always be mutated -/// my_struct.special_field.set(new_value); -/// assert_eq!(my_struct.special_field.get(), new_value); -/// ``` -/// -/// See the [module-level documentation](self) for more. #[repr(transparent)] pub struct WasmCell<'a, T: ?Sized> { inner: &'a Cell, @@ -101,9 +70,11 @@ impl<'a, T> WasmCell<'a, T> { /// # Examples /// /// ``` + /// use std::cell::Cell; /// use wasmer::WasmCell; /// - /// let c = WasmCell::new(5); + /// let cell = Cell::new(5); + /// let wasm_cell = WasmCell::new(&cell); /// ``` #[inline] pub const fn new(cell: &'a Cell) -> WasmCell<'a, T> { @@ -117,11 +88,12 @@ impl<'a, T: Copy> WasmCell<'a, T> { /// # Examples /// /// ``` + /// use std::cell::Cell; /// use wasmer::WasmCell; /// - /// let c = WasmCell::new(5); - /// - /// let five = c.get(); + /// let cell = Cell::new(5); + /// let wasm_cell = WasmCell::new(&cell); + /// let five = wasm_cell.get(); /// ``` #[inline] pub fn get(&self) -> T { @@ -162,11 +134,13 @@ impl WasmCell<'_, T> { /// # Examples /// /// ``` + /// use std::cell::Cell; /// use wasmer::WasmCell; /// - /// let c = WasmCell::new(5); - /// - /// c.set(10); + /// let cell = Cell::new(5); + /// let wasm_cell = WasmCell::new(&cell); + /// wasm_cell.set(10); + /// assert_eq!(cell.get(), 10); /// ``` #[inline] pub fn set(&self, val: T) { From 23153b8fe66e17b7605c12f863ce3742575ccb13 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 2 Jul 2021 10:40:44 -0700 Subject: [PATCH 17/17] Address feedback --- lib/types/src/memory_view.rs | 16 +++++++++++----- lib/wasi/src/syscalls/mod.rs | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/types/src/memory_view.rs b/lib/types/src/memory_view.rs index 9382d5d1487..172dd1ce5f8 100644 --- a/lib/types/src/memory_view.rs +++ b/lib/types/src/memory_view.rs @@ -80,6 +80,14 @@ where Bound::Included(end) => *end, Bound::Excluded(end) => *end - 1, }; + assert!( + start < self.length, + "The range start is bigger than current length" + ); + assert!( + end < self.length, + "The range end is bigger than current length" + ); Self { ptr: unsafe { self.ptr.add(start) }, length: (end - start), @@ -97,11 +105,9 @@ where /// This method is unsafe because the caller will need to make sure /// there are no data races when copying memory into the view. pub unsafe fn copy_from(&self, src: &[T]) { - assert!( - src.len() == self.length, - "The source length must match the MemoryView length" - ); - for (i, byte) in src.iter().enumerate() { + // We cap at a max length + let sliced_src = &src[..self.length]; + for (i, byte) in sliced_src.iter().enumerate() { *self.ptr.offset(i as isize) = *byte; } } diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs index 88b0745e958..074613a9c8b 100644 --- a/lib/wasi/src/syscalls/mod.rs +++ b/lib/wasi/src/syscalls/mod.rs @@ -94,7 +94,7 @@ fn read_bytes( .subarray( iov_inner.buf as usize..=(iov_inner.buf as usize + iov_inner.buf_len as usize), ) - .copy_from(&raw_bytes[..iov_inner.buf_len as usize]); + .copy_from(&raw_bytes); } } Ok(bytes_read)