Improved WasmPtr, added WasmCell

lib/api/src/cell.rs

@@ -0,0 +1,149 @@
+pub use std::cell::Cell;
+
+use core::cmp::Ordering;
+use core::fmt::{self, Debug};
+use std::fmt::Pointer;
+
+/// A mutable Wasm-memory location.
+#[repr(transparent)]
+pub struct WasmCell<'a, T: ?Sized> {
+    inner: &'a Cell<T>,
+}
+
+unsafe impl<T: ?Sized> Send for WasmCell<'_, T> where T: Send {}
+
+unsafe impl<T: ?Sized> Sync for WasmCell<'_, T> {}
+
+impl<'a, T: Copy> Clone for WasmCell<'a, T> {
+    #[inline]
+    fn clone(&self) -> WasmCell<'a, T> {
+        WasmCell { inner: self.inner }
+    }
+}
+
+impl<T: PartialEq + Copy> PartialEq for WasmCell<'_, T> {
+    #[inline]
+    fn eq(&self, other: &WasmCell<T>) -> bool {
+        self.inner.eq(&other.inner)
+    }
+}
+
+impl<T: Eq + Copy> Eq for WasmCell<'_, T> {}
+
+impl<T: PartialOrd + Copy> PartialOrd for WasmCell<'_, T> {
+    #[inline]
+    fn partial_cmp(&self, other: &WasmCell<T>) -> Option<Ordering> {
+        self.inner.partial_cmp(&other.inner)
+    }
+
+    #[inline]
+    fn lt(&self, other: &WasmCell<T>) -> bool {
+        self.inner < other.inner
+    }
+
+    #[inline]
+    fn le(&self, other: &WasmCell<T>) -> bool {
+        self.inner <= other.inner
+    }
+
+    #[inline]
+    fn gt(&self, other: &WasmCell<T>) -> bool {
+        self.inner > other.inner
+    }
+
+    #[inline]
+    fn ge(&self, other: &WasmCell<T>) -> bool {
+        self.inner >= other.inner
+    }
+}
+
+impl<T: Ord + Copy> Ord for WasmCell<'_, T> {
+    #[inline]
+    fn cmp(&self, other: &WasmCell<T>) -> Ordering {
+        self.inner.cmp(&other.inner)
+    }
+}
+
+impl<'a, T> WasmCell<'a, T> {
+    /// Creates a new `WasmCell` containing the given value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use std::cell::Cell;
+    /// use wasmer::WasmCell;
+    ///
+    /// let cell = Cell::new(5);
+    /// let wasm_cell = WasmCell::new(&cell);
+    /// ```
+    #[inline]
+    pub const fn new(cell: &'a Cell<T>) -> WasmCell<'a, T> {
+        WasmCell { inner: cell }
+    }
+}
+
+impl<'a, T: Copy> WasmCell<'a, T> {
+    /// Returns a copy of the contained value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use std::cell::Cell;
+    /// use wasmer::WasmCell;
+    ///
+    /// let cell = Cell::new(5);
+    /// let wasm_cell = WasmCell::new(&cell);
+    /// let five = wasm_cell.get();
+    /// ```
+    #[inline]
+    pub fn get(&self) -> T {
+        self.inner.get()
+    }
+
+    /// 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()
+    }
+}
+
+impl<T: Debug> Debug for WasmCell<'_, T> {
+    #[inline]
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.inner.fmt(f)
+    }
+}
+
+impl<T: Sized> WasmCell<'_, T> {
+    /// Sets the contained value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use std::cell::Cell;
+    /// use wasmer::WasmCell;
+    ///
+    /// 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) {
+        self.inner.set(val);
+    }
+}

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::{

lib/api/src/ptr.rs

@@ -6,6 +6,7 @@
 //! 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 wasmer_types::ValueType;
@@ -103,7 +104,7 @@ impl<T: Copy + ValueType> WasmPtr<T, Item> {
     /// 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<T>> {
+    pub fn deref<'a>(self, memory: &'a Memory) -> Option<WasmCell<'a, T>> {
         if (self.offset as usize) + mem::size_of::<T>() > memory.size().bytes().0
             || mem::size_of::<T>() == 0
         {
@@ -114,30 +115,9 @@ impl<T: Copy + ValueType> WasmPtr<T, Item> {
                 memory.view::<u8>().as_ptr().add(self.offset as usize) as usize,
                 mem::align_of::<T>(),
             ) as *const Cell<T>;
-            Some(&*cell_ptr)
+            Some(WasmCell::new(&*cell_ptr))
         }
     }
-
-    /// Mutably dereference this `WasmPtr` getting a `&mut Cell<T>` 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<T>> {
-        if (self.offset as usize) + mem::size_of::<T>() > memory.size().bytes().0
-            || mem::size_of::<T>() == 0
-        {
-            return None;
-        }
-        let cell_ptr = align_pointer(
-            memory.view::<u8>().as_ptr().add(self.offset as usize) as usize,
-            mem::align_of::<T>(),
-        ) as *mut Cell<T>;
-        Some(&mut *cell_ptr)
-    }
 }
 
 /// Methods for `WasmPtr`s to arrays of data that can be dereferenced, namely to
@@ -151,65 +131,37 @@ impl<T: Copy + ValueType> WasmPtr<T, Array> {
     /// 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<T>]> {
+    pub fn deref<'a>(
+        self,
+        memory: &'a Memory,
+        index: u32,
+        length: u32,
+    ) -> Option<Vec<WasmCell<'a, T>>> {
         // 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::<T>();
         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
-            || self.offset as usize >= memory_size
-            || mem::size_of::<T>() == 0
+            || (self.offset as usize) >= memory_size
+            || item_size == 0
         {
             return None;
         }
-
-        unsafe {
+        let cell_ptrs = unsafe {
             let cell_ptr = align_pointer(
                 memory.view::<u8>().as_ptr().add(self.offset as usize) as usize,
                 mem::align_of::<T>(),
             ) as *const Cell<T>;
-            let cell_ptrs = &std::slice::from_raw_parts(cell_ptr, slice_full_len)
-                [index as usize..slice_full_len];
-            Some(cell_ptrs)
-        }
-    }
+            &std::slice::from_raw_parts(cell_ptr, slice_full_len)[index as usize..slice_full_len]
+        };
 
-    /// Mutably dereference this `WasmPtr` getting a `&mut [Cell<T>]` 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<T>]> {
-        // 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::<T>();
-        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::<T>() == 0
-        {
-            return None;
-        }
-
-        let cell_ptr = align_pointer(
-            memory.view::<u8>().as_ptr().add(self.offset as usize) as usize,
-            mem::align_of::<T>(),
-        ) as *mut Cell<T>;
-        let cell_ptrs = &mut std::slice::from_raw_parts_mut(cell_ptr, slice_full_len)
-            [index as usize..slice_full_len];
-        Some(cell_ptrs)
+        let wasm_cells = cell_ptrs
+            .iter()
+            .map(|ptr| WasmCell::new(ptr))
+            .collect::<Vec<_>>();
+        Some(wasm_cells)
     }
 
     /// Get a UTF-8 string from the `WasmPtr` with the given length.
@@ -339,7 +291,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
@@ -352,29 +304,23 @@ mod test {
         let start_wasm_ptr_array: WasmPtr<u8, Array> = 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<u8> = 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<u8, Array> = 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());
@@ -384,9 +330,7 @@ mod test {
         let last_valid_address_for_u32 = (memory.size().bytes().0 - 4) as u32;
         let end_wasm_ptr: WasmPtr<u32> = 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<u32>; 4] = [
             WasmPtr::new(last_valid_address_for_u32 + 1),
@@ -396,17 +340,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<u32, Array> = 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<u32, Array>; 4] = [
@@ -418,9 +359,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() });
         }
     }
 }