Skip to content

Commit

Permalink
Try #2442:
Browse files Browse the repository at this point in the history
  • Loading branch information
bors[bot] committed Jul 2, 2021
2 parents d292c24 + 1a8952e commit 449734c
Show file tree
Hide file tree
Showing 11 changed files with 328 additions and 174 deletions.
175 changes: 175 additions & 0 deletions lib/api/src/cell.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
pub use std::cell::Cell;

use core::cmp::Ordering;
use core::fmt::{self, Debug};
use std::fmt::Pointer;

/// A mutable Wasm-memory location.
///
/// # Examples
///
/// In this example, you can see that `WasmCell<T>` enables mutation inside an
/// immutable struct. In other words, it enables "interior mutability".
///
/// ```
/// use wasmer::WasmCell;
///
/// struct SomeStruct {
/// regular_field: u8,
/// special_field: WasmCell<u8>,
/// }
///
/// 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<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 wasmer::WasmCell;
///
/// let c = WasmCell::new(5);
/// ```
#[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 wasmer::WasmCell;
///
/// let c = WasmCell::new(5);
///
/// let five = c.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 wasmer::WasmCell;
///
/// let c = WasmCell::new(5);
///
/// c.set(10);
/// ```
#[inline]
pub fn set(&self, val: T) {
self.inner.set(val);
}
}
2 changes: 2 additions & 0 deletions lib/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::{
Expand Down
101 changes: 20 additions & 81 deletions lib/api/src/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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());
Expand All @@ -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),
Expand All @@ -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] = [
Expand All @@ -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() });
}
}
}

0 comments on commit 449734c

Please sign in to comment.