remove duplicated code, wrap WasmPtr on wasi side

lib/emscripten/src/lib.rs

@@ -42,7 +42,6 @@ mod lock;
 mod math;
 mod memory;
 mod process;
-mod ptr;
 mod signal;
 mod storage;
 mod syscalls;

lib/emscripten/src/syscalls/mod.rs

@@ -10,7 +10,6 @@ pub use self::unix::*;
 #[cfg(windows)]
 pub use self::windows::*;
 
-use crate::ptr::{Array, WasmPtr};
 use crate::utils::{copy_stat_into_wasm, get_cstr_path, get_current_directory};
 
 use super::varargs::VarArgs;
@@ -42,7 +41,10 @@ use libc::{
     write,
     // readlink,
 };
-use wasmer_runtime_core::vm::Ctx;
+use wasmer_runtime_core::{
+    memory::ptr::{Array, WasmPtr},
+    vm::Ctx,
+};
 
 use super::env;
 use std::cell::Cell;
@@ -282,7 +284,7 @@ pub fn ___syscall183(ctx: &mut Ctx, _which: c_int, mut varargs: VarArgs) -> i32
         buf_writer[i].set(byte as i8);
     }
     buf_writer[len].set(0);
-    buf_offset.offset()
+    buf_offset.offset() as i32
 }
 
 // mmap2

lib/runtime-core/src/memory/mod.rs

@@ -10,7 +10,7 @@ use crate::{
 };
 use std::{
     cell::{Cell, RefCell},
-    fmt, mem, ptr,
+    fmt, mem,
     rc::Rc,
 };
 
@@ -21,6 +21,7 @@ pub use self::view::{Atomically, MemoryView};
 
 mod atomic;
 mod dynamic;
+pub mod ptr;
 mod static_;
 mod view;
 
@@ -221,9 +222,9 @@ struct UnsharedMemoryInternal {
 impl UnsharedMemory {
     pub fn new(desc: MemoryDescriptor) -> Result<Self, CreationError> {
         let mut local = vm::LocalMemory {
-            base: ptr::null_mut(),
+            base: std::ptr::null_mut(),
             bound: 0,
-            memory: ptr::null_mut(),
+            memory: std::ptr::null_mut(),
         };
 
         let storage = match desc.memory_type() {

lib/runtime-core/src/memory/ptr.rs

@@ -1,15 +1,20 @@
-use std::{cell::Cell, fmt, marker::PhantomData, mem};
-use wasmer_runtime_core::{
+//! A reusable pointer abstraction for getting memory from the guest's memory.
+//!
+//! This abstraction is safe: it ensures the memory is in bounds and that the pointer
+//! is aligned (avoiding undefined behavior).
+//!
+//! Therefore, you should use this abstraction whenever possible to avoid memory
+//! related bugs when implementing an ABI.
+
+use crate::{
     memory::Memory,
     types::{ValueType, WasmExternType},
 };
+use std::{cell::Cell, fmt, marker::PhantomData, mem};
 
 pub struct Array;
 pub struct Item;
 
-// TODO: before shipping, refactor this and the wasi code into a common crate
-// and wrap it when used in the context of wasi to return the proper error codes
-
 #[repr(transparent)]
 pub struct WasmPtr<T: Copy, Ty = Item> {
     offset: u32,
@@ -26,8 +31,8 @@ impl<T: Copy, Ty> WasmPtr<T, Ty> {
     }
 
     #[inline]
-    pub fn offset(self) -> i32 {
-        self.offset as i32
+    pub fn offset(self) -> u32 {
+        self.offset
     }
 }
 
@@ -72,7 +77,7 @@ impl<T: Copy + ValueType> WasmPtr<T, Array> {
                 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 as usize];
+                [index as usize..slice_full_len];
             Some(cell_ptrs)
         }
     }

lib/wasi/src/ptr.rs

@@ -1,123 +1,78 @@
+//! This is a wrapper around the `WasmPtr` abstraction that returns __WASI_EFAULT
+//! if memory access failed
+
 use crate::syscalls::types::{__wasi_errno_t, __WASI_EFAULT};
-use std::{cell::Cell, fmt, marker::PhantomData, mem};
+use std::{cell::Cell, fmt};
+pub use wasmer_runtime_core::memory::ptr::Array;
 use wasmer_runtime_core::{
-    memory::Memory,
+    memory::{ptr, Memory},
     types::{ValueType, WasmExternType},
 };
 
-pub struct Array;
-pub struct Item;
-
 #[repr(transparent)]
-pub struct WasmPtr<T: Copy, Ty = Item> {
-    offset: u32,
-    _phantom: PhantomData<(T, Ty)>,
+pub struct WasmPtr<T: Copy, Ty = ptr::Item>(ptr::WasmPtr<T, Ty>);
+
+unsafe impl<T: Copy, Ty> ValueType for WasmPtr<T, Ty> {}
+impl<T: Copy, Ty> Copy for WasmPtr<T, Ty> {}
+
+impl<T: Copy, Ty> Clone for WasmPtr<T, Ty> {
+    fn clone(&self) -> Self {
+        Self(self.0.clone())
+    }
 }
 
+impl<T: Copy, Ty> fmt::Debug for WasmPtr<T, Ty> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{:?}", self.0)
+    }
+}
+
+unsafe impl<T: Copy, Ty> WasmExternType for WasmPtr<T, Ty> {
+    type Native = <ptr::WasmPtr<T, Ty> as WasmExternType>::Native;
+
+    fn to_native(self) -> Self::Native {
+        self.0.to_native()
+    }
+    fn from_native(n: Self::Native) -> Self {
+        Self(ptr::WasmPtr::from_native(n))
+    }
+}
+
+impl<T: Copy, Ty> PartialEq for WasmPtr<T, Ty> {
+    fn eq(&self, other: &Self) -> bool {
+        self.0 == other.0
+    }
+}
+
+impl<T: Copy, Ty> Eq for WasmPtr<T, Ty> {}
+
 impl<T: Copy, Ty> WasmPtr<T, Ty> {
-    #[inline]
+    #[inline(always)]
     pub fn new(offset: u32) -> Self {
-        Self {
-            offset,
-            _phantom: PhantomData,
-        }
+        Self(ptr::WasmPtr::new(offset))
     }
 
-    #[inline]
+    #[inline(always)]
     pub fn offset(self) -> u32 {
-        self.offset
+        self.0.offset()
     }
 }
 
-#[inline(always)]
-fn align_pointer(ptr: usize, align: usize) -> usize {
-    // clears bits below aligment amount (assumes power of 2) to align pointer
-    debug_assert!(align.count_ones() == 1);
-    ptr & !(align - 1)
-}
-
-impl<T: Copy + ValueType> WasmPtr<T, Item> {
-    #[inline]
+impl<T: Copy + ValueType> WasmPtr<T, ptr::Item> {
+    #[inline(always)]
     pub fn deref<'a>(self, memory: &'a Memory) -> Result<&'a Cell<T>, __wasi_errno_t> {
-        if (self.offset as usize) + mem::size_of::<T>() >= memory.size().bytes().0 {
-            return Err(__WASI_EFAULT);
-        }
-        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>;
-            Ok(&*cell_ptr)
-        }
+        self.0.deref(memory).ok_or(__WASI_EFAULT)
     }
 }
 
-impl<T: Copy + ValueType> WasmPtr<T, Array> {
-    #[inline]
+impl<T: Copy + ValueType> WasmPtr<T, ptr::Array> {
+    #[inline(always)]
     pub fn deref<'a>(
         self,
         memory: &'a Memory,
         index: u32,
         length: u32,
     ) -> Result<&'a [Cell<T>], __wasi_errno_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>() + (mem::size_of::<T>() % mem::align_of::<T>());
-        let slice_full_len = index as usize + length as usize;
-
-        if (self.offset as usize) + (item_size * slice_full_len) >= memory.size().bytes().0 {
-            return Err(__WASI_EFAULT);
-        }
-
-        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];
-            Ok(cell_ptrs)
-        }
-    }
-}
-
-unsafe impl<T: Copy, Ty> WasmExternType for WasmPtr<T, Ty> {
-    type Native = i32;
-
-    fn to_native(self) -> Self::Native {
-        self.offset as i32
-    }
-    fn from_native(n: Self::Native) -> Self {
-        Self {
-            offset: n as u32,
-            _phantom: PhantomData,
-        }
-    }
-}
-
-unsafe impl<T: Copy, Ty> ValueType for WasmPtr<T, Ty> {}
-
-impl<T: Copy, Ty> Clone for WasmPtr<T, Ty> {
-    fn clone(&self) -> Self {
-        Self {
-            offset: self.offset,
-            _phantom: PhantomData,
-        }
-    }
-}
-
-impl<T: Copy, Ty> Copy for WasmPtr<T, Ty> {}
-
-impl<T: Copy, Ty> PartialEq for WasmPtr<T, Ty> {
-    fn eq(&self, other: &Self) -> bool {
-        self.offset == other.offset
-    }
-}
-
-impl<T: Copy, Ty> Eq for WasmPtr<T, Ty> {}
-
-impl<T: Copy, Ty> fmt::Debug for WasmPtr<T, Ty> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "WasmPtr({:#x})", self.offset)
+        self.0.deref(memory, index, length).ok_or(__WASI_EFAULT)
     }
 }