Merge #820

820: Remove null pointer checks generally, re-add them in Emscripten r=MarkMcCaskey a=MarkMcCaskey

Resolves #818 

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <[email protected]>

CHANGELOG.md

@@ -6,6 +6,7 @@ Blocks of changes will separated by version increments.
 
 ## **[Unreleased]**
 
+- [#820](https://github.com/wasmerio/wasmer/issues/820) Remove null-pointer checks in `WasmPtr` from runtime-core, re-add them in Emscripten
 - [#803](https://github.com/wasmerio/wasmer/issues/803) Add method to `Ctx` to invoke functions by their `TableIndex`
 - [#790](https://github.com/wasmerio/wasmer/pull/790) Fix flaky test failure with LLVM, switch to large code model.
 - [#788](https://github.com/wasmerio/wasmer/pull/788) Use union merge on the changelog file.

lib/emscripten/src/env/mod.rs

@@ -12,14 +12,14 @@ pub use self::windows::*;
 
 use libc::c_char;
 
-use crate::{allocate_on_stack, EmscriptenData};
+use crate::{
+    allocate_on_stack,
+    ptr::{Array, WasmPtr},
+    EmscriptenData,
+};
 
 use std::os::raw::c_int;
-use wasmer_runtime_core::{
-    memory::ptr::{Array, WasmPtr},
-    types::ValueType,
-    vm::Ctx,
-};
+use wasmer_runtime_core::{types::ValueType, vm::Ctx};
 
 pub fn call_malloc(ctx: &mut Ctx, size: u32) -> u32 {
     get_emscripten_data(ctx)

lib/emscripten/src/env/unix/mod.rs

@@ -9,11 +9,9 @@ use std::mem;
 use std::os::raw::c_char;
 
 use crate::env::{call_malloc, call_malloc_with_cast, EmAddrInfo, EmSockAddr};
+use crate::ptr::{Array, WasmPtr};
 use crate::utils::{copy_cstr_into_wasm, copy_terminated_array_of_cstrs};
-use wasmer_runtime_core::{
-    memory::ptr::{Array, WasmPtr},
-    vm::Ctx,
-};
+use wasmer_runtime_core::vm::Ctx;
 
 // #[no_mangle]
 /// emscripten: _getenv // (name: *const char) -> *const c_char;

lib/emscripten/src/env/windows/mod.rs

@@ -6,8 +6,9 @@ use std::mem;
 use std::os::raw::c_char;
 
 use crate::env::{call_malloc, EmAddrInfo};
+use crate::ptr::WasmPtr;
 use crate::utils::{copy_cstr_into_wasm, read_string_from_wasm};
-use wasmer_runtime_core::{memory::ptr::WasmPtr, vm::Ctx};
+use wasmer_runtime_core::vm::Ctx;
 
 extern "C" {
     #[link_name = "_putenv"]

lib/emscripten/src/lib.rs

@@ -62,6 +62,7 @@ mod math;
 mod memory;
 mod process;
 mod pthread;
+mod ptr;
 mod signal;
 mod storage;
 mod syscalls;

lib/emscripten/src/ptr.rs

@@ -0,0 +1,116 @@
+//! This is a wrapper around the `WasmPtr` abstraction that does not allow deref of address 0
+//! This is a common assumption in Emscripten code
+
+// this is a wrapper with extra logic around the runtime-core `WasmPtr`, so we
+// don't want to warn about unusued code here
+#![allow(dead_code)]
+
+use std::{cell::Cell, fmt};
+pub use wasmer_runtime_core::memory::ptr::Array;
+use wasmer_runtime_core::{
+    memory::{ptr, Memory},
+    types::{ValueType, WasmExternType},
+};
+
+#[repr(transparent)]
+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(always)]
+    pub fn new(offset: u32) -> Self {
+        Self(ptr::WasmPtr::new(offset))
+    }
+
+    #[inline(always)]
+    pub fn offset(self) -> u32 {
+        self.0.offset()
+    }
+}
+
+impl<T: Copy + ValueType> WasmPtr<T, ptr::Item> {
+    #[inline(always)]
+    pub fn deref<'a>(self, memory: &'a Memory) -> Option<&'a Cell<T>> {
+        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<T>> {
+        if self.0.offset() == 0 {
+            None
+        } else {
+            self.0.deref_mut(memory)
+        }
+    }
+}
+
+impl<T: Copy + ValueType> WasmPtr<T, ptr::Array> {
+    #[inline(always)]
+    pub fn deref<'a>(self, memory: &'a Memory, index: u32, length: u32) -> Option<&'a [Cell<T>]> {
+        if self.0.offset() == 0 {
+            None
+        } else {
+            self.0.deref(memory, index, length)
+        }
+    }
+
+    #[inline]
+    pub unsafe fn deref_mut<'a>(
+        self,
+        memory: &'a Memory,
+        index: u32,
+        length: u32,
+    ) -> Option<&'a mut [Cell<T>]> {
+        if self.0.offset() == 0 {
+            None
+        } else {
+            self.0.deref_mut(memory, index, length)
+        }
+    }
+
+    #[inline(always)]
+    pub fn get_utf8_string<'a>(self, memory: &'a Memory, str_len: u32) -> Option<&'a str> {
+        if self.0.offset() == 0 {
+            None
+        } else {
+            self.0.get_utf8_string(memory, str_len)
+        }
+    }
+}

lib/emscripten/src/syscalls/mod.rs

@@ -10,7 +10,10 @@ pub use self::unix::*;
 #[cfg(windows)]
 pub use self::windows::*;
 
-use crate::utils::{copy_stat_into_wasm, get_cstr_path, get_current_directory};
+use crate::{
+    ptr::{Array, WasmPtr},
+    utils::{copy_stat_into_wasm, get_cstr_path, get_current_directory},
+};
 
 use super::varargs::VarArgs;
 use byteorder::{ByteOrder, LittleEndian};
@@ -40,10 +43,7 @@ use libc::{
     write,
     // ENOTTY,
 };
-use wasmer_runtime_core::{
-    memory::ptr::{Array, WasmPtr},
-    vm::Ctx,
-};
+use wasmer_runtime_core::vm::Ctx;
 
 use super::env;
 use std::cell::Cell;

lib/emscripten/src/syscalls/unix.rs

@@ -1,4 +1,4 @@
-use crate::varargs::VarArgs;
+use crate::{ptr::WasmPtr, varargs::VarArgs};
 #[cfg(target_os = "macos")]
 use libc::size_t;
 /// NOTE: TODO: These syscalls only support wasm_32 for now because they assume offsets are u32
@@ -111,7 +111,7 @@ fn translate_ioctl(wasm_ioctl: u32) -> c_ulong {
 
 #[allow(unused_imports)]
 use std::ffi::CStr;
-use wasmer_runtime_core::{memory::ptr::WasmPtr, vm::Ctx};
+use wasmer_runtime_core::vm::Ctx;
 
 use crate::env::EmSockAddr;
 use crate::utils::{self, get_cstr_path};

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

@@ -46,9 +46,7 @@ fn align_pointer(ptr: usize, align: usize) -> usize {
 impl<T: Copy + ValueType> WasmPtr<T, Item> {
     #[inline]
     pub fn deref<'a>(self, memory: &'a Memory) -> Option<&'a Cell<T>> {
-        if self.offset == 0
-            || (self.offset as usize) + mem::size_of::<T>() >= memory.size().bytes().0
-        {
+        if (self.offset as usize) + mem::size_of::<T>() >= memory.size().bytes().0 {
             return None;
         }
         unsafe {
@@ -62,9 +60,7 @@ impl<T: Copy + ValueType> WasmPtr<T, Item> {
 
     #[inline]
     pub unsafe fn deref_mut<'a>(self, memory: &'a Memory) -> Option<&'a mut Cell<T>> {
-        if self.offset == 0
-            || (self.offset as usize) + mem::size_of::<T>() >= memory.size().bytes().0
-        {
+        if (self.offset as usize) + mem::size_of::<T>() >= memory.size().bytes().0 {
             return None;
         }
         let cell_ptr = align_pointer(
@@ -83,9 +79,7 @@ impl<T: Copy + ValueType> WasmPtr<T, Array> {
         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 == 0
-            || (self.offset as usize) + (item_size * slice_full_len) >= memory.size().bytes().0
-        {
+        if (self.offset as usize) + (item_size * slice_full_len) >= memory.size().bytes().0 {
             return None;
         }
 
@@ -112,9 +106,7 @@ impl<T: Copy + ValueType> WasmPtr<T, Array> {
         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 == 0
-            || (self.offset as usize) + (item_size * slice_full_len) >= memory.size().bytes().0
-        {
+        if (self.offset as usize) + (item_size * slice_full_len) >= memory.size().bytes().0 {
             return None;
         }