diff --git a/lib/emscripten/src/lib.rs b/lib/emscripten/src/lib.rs index d260351b9..70d62d950 100644 --- a/lib/emscripten/src/lib.rs +++ b/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; diff --git a/lib/emscripten/src/syscalls/mod.rs b/lib/emscripten/src/syscalls/mod.rs index 26f3e8d72..be8531376 100644 --- a/lib/emscripten/src/syscalls/mod.rs +++ b/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 diff --git a/lib/runtime-core/src/memory/mod.rs b/lib/runtime-core/src/memory/mod.rs index 6f532b6d4..04ffaa384 100644 --- a/lib/runtime-core/src/memory/mod.rs +++ b/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 { 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() { diff --git a/lib/emscripten/src/ptr.rs b/lib/runtime-core/src/memory/ptr.rs similarity index 86% rename from lib/emscripten/src/ptr.rs rename to lib/runtime-core/src/memory/ptr.rs index 079158f94..892668ce5 100644 --- a/lib/emscripten/src/ptr.rs +++ b/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 { offset: u32, @@ -26,8 +31,8 @@ impl WasmPtr { } #[inline] - pub fn offset(self) -> i32 { - self.offset as i32 + pub fn offset(self) -> u32 { + self.offset } } @@ -72,7 +77,7 @@ impl WasmPtr { 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 usize]; + [index as usize..slice_full_len]; Some(cell_ptrs) } } diff --git a/lib/wasi/src/ptr.rs b/lib/wasi/src/ptr.rs index 021c0925f..0af012eac 100644 --- a/lib/wasi/src/ptr.rs +++ b/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 { - offset: u32, - _phantom: PhantomData<(T, Ty)>, +pub struct WasmPtr(ptr::WasmPtr); + +unsafe impl ValueType for WasmPtr {} +impl Copy for WasmPtr {} + +impl Clone for WasmPtr { + fn clone(&self) -> Self { + Self(self.0.clone()) + } } +impl fmt::Debug for WasmPtr { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{:?}", self.0) + } +} + +unsafe impl WasmExternType for WasmPtr { + type Native = 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 PartialEq for WasmPtr { + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 + } +} + +impl Eq for WasmPtr {} + impl WasmPtr { - #[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 WasmPtr { - #[inline] +impl WasmPtr { + #[inline(always)] pub fn deref<'a>(self, memory: &'a Memory) -> Result<&'a Cell, __wasi_errno_t> { - if (self.offset as usize) + mem::size_of::() >= memory.size().bytes().0 { - return Err(__WASI_EFAULT); - } - unsafe { - let cell_ptr = align_pointer( - memory.view::().as_ptr().add(self.offset as usize) as usize, - mem::align_of::(), - ) as *const Cell; - Ok(&*cell_ptr) - } + self.0.deref(memory).ok_or(__WASI_EFAULT) } } -impl WasmPtr { - #[inline] +impl WasmPtr { + #[inline(always)] pub fn deref<'a>( self, memory: &'a Memory, index: u32, length: u32, ) -> Result<&'a [Cell], __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::() + (mem::size_of::() % mem::align_of::()); - 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::().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]; - Ok(cell_ptrs) - } - } -} - -unsafe impl WasmExternType for WasmPtr { - 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 ValueType for WasmPtr {} - -impl Clone for WasmPtr { - fn clone(&self) -> Self { - Self { - offset: self.offset, - _phantom: PhantomData, - } - } -} - -impl Copy for WasmPtr {} - -impl PartialEq for WasmPtr { - fn eq(&self, other: &Self) -> bool { - self.offset == other.offset - } -} - -impl Eq for WasmPtr {} - -impl fmt::Debug for WasmPtr { - 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) } }