Merge #1401

1401: Make runtime and trap errors well defined r=syrusakbary a=MarkMcCaskey Resolves #1328 This PR goes through and gives explicit types for all the errors instead of using `Box<dyn Any + Send>` where possible. This gives users better insight into what the specific errors are and should help with debugging in the case of mistakes in our code. The remaining uses of `Box<dyn Any>` are due to the structure of our dependency graph -- this is probably solvable but it seems fine as is as all error types are now explicit and the remaining `Box<dyn Any>`s are either fully user controlled or not for end-user consumption. # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Mark McCaskey <[email protected]>
wasmerio · Apr 28, 2020 · d23a3f3 · d23a3f3
2 parents 86a66e2 + 9c5fdd6
commit d23a3f3
Show file tree

Hide file tree

Showing 29 changed files with 405 additions and 215 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## **[Unreleased]**
 
+- [#1401](https://github.com/wasmerio/wasmer/pull/1401) Make breaking change to `RuntimeError`: `RuntimeError` is now more explicit about its possible error values allowing for better insight into why a call into Wasm failed.
 - [#1382](https://github.com/wasmerio/wasmer/pull/1382) Refactored test infranstructure (part 2)
 - [#1380](https://github.com/wasmerio/wasmer/pull/1380) Refactored test infranstructure (part 1)
 - [#1357](https://github.com/wasmerio/wasmer/pull/1357) Refactored bin commands into separate files

diff --git a/examples/callback-guest/callback-guest.wasm b/examples/callback-guest/callback-guest.wasm
diff --git a/lib/clif-backend/src/signal/mod.rs b/lib/clif-backend/src/signal/mod.rs
@@ -4,9 +4,10 @@ use crate::{
     trampoline::Trampolines,
 };
 use libc::c_void;
-use std::{any::Any, cell::Cell, ptr::NonNull, sync::Arc};
+use std::{cell::Cell, ptr::NonNull, sync::Arc};
 use wasmer_runtime_core::{
     backend::RunnableModule,
+    error::RuntimeError,
     module::ModuleInfo,
     typed_func::{Trampoline, Wasm},
     types::{LocalFuncIndex, SigIndex},
@@ -26,11 +27,9 @@ pub use self::unix::*;
 pub use self::windows::*;
 
 thread_local! {
-    pub static TRAP_EARLY_DATA: Cell<Option<Box<dyn Any + Send>>> = Cell::new(None);
+    pub static TRAP_EARLY_DATA: Cell<Option<RuntimeError>> = Cell::new(None);
 }
 
-pub struct CallProtError(pub Box<dyn Any + Send>);
-
 pub struct Caller {
     handler_data: HandlerData,
     trampolines: Arc<Trampolines>,
@@ -63,7 +62,7 @@ impl RunnableModule for Caller {
             func: NonNull<vm::Func>,
             args: *const u64,
             rets: *mut u64,
-            error_out: *mut Option<Box<dyn Any + Send>>,
+            error_out: *mut Option<RuntimeError>,
             invoke_env: Option<NonNull<c_void>>,
         ) -> bool {
             let handler_data = &*invoke_env.unwrap().cast().as_ptr();
@@ -80,7 +79,7 @@ impl RunnableModule for Caller {
 
             match res {
                 Err(err) => {
-                    *error_out = Some(err.0);
+                    *error_out = Some(err.into());
                     false
                 }
                 Ok(()) => true,
@@ -101,7 +100,7 @@ impl RunnableModule for Caller {
         })
     }
 
-    unsafe fn do_early_trap(&self, data: Box<dyn Any + Send>) -> ! {
+    unsafe fn do_early_trap(&self, data: RuntimeError) -> ! {
         TRAP_EARLY_DATA.with(|cell| cell.set(Some(data)));
         trigger_trap()
     }

diff --git a/lib/clif-backend/src/signal/unix.rs b/lib/clif-backend/src/signal/unix.rs
@@ -10,7 +10,7 @@
 //! unless you have memory unsafety elsewhere in your code.
 //!
 use crate::relocation::{TrapCode, TrapData};
-use crate::signal::{CallProtError, HandlerData};
+use crate::signal::HandlerData;
 use libc::{c_int, c_void, siginfo_t};
 use nix::sys::signal::{
     sigaction, SaFlags, SigAction, SigHandler, SigSet, Signal, SIGBUS, SIGFPE, SIGILL, SIGSEGV,
@@ -19,6 +19,7 @@ use std::cell::{Cell, UnsafeCell};
 use std::ptr;
 use std::sync::Once;
 use wasmer_runtime_core::backend::ExceptionCode;
+use wasmer_runtime_core::error::InvokeError;
 
 extern "C" fn signal_trap_handler(
     signum: ::nix::libc::c_int,
@@ -65,7 +66,7 @@ pub unsafe fn trigger_trap() -> ! {
 pub fn call_protected<T>(
     handler_data: &HandlerData,
     f: impl FnOnce() -> T,
-) -> Result<T, CallProtError> {
+) -> Result<T, InvokeError> {
     unsafe {
         let jmp_buf = SETJMP_BUFFER.with(|buf| buf.get());
         let prev_jmp_buf = *jmp_buf;
@@ -79,16 +80,12 @@ pub fn call_protected<T>(
             *jmp_buf = prev_jmp_buf;
 
             if let Some(data) = super::TRAP_EARLY_DATA.with(|cell| cell.replace(None)) {
-                Err(CallProtError(data))
+                Err(InvokeError::EarlyTrap(Box::new(data)))
             } else {
                 let (faulting_addr, inst_ptr) = CAUGHT_ADDRESSES.with(|cell| cell.get());
 
-                if let Some(TrapData {
-                    trapcode,
-                    srcloc: _,
-                }) = handler_data.lookup(inst_ptr)
-                {
-                    Err(CallProtError(Box::new(match Signal::from_c_int(signum) {
+                if let Some(TrapData { trapcode, srcloc }) = handler_data.lookup(inst_ptr) {
+                    let code = match Signal::from_c_int(signum) {
                         Ok(SIGILL) => match trapcode {
                             TrapCode::StackOverflow => ExceptionCode::MemoryOutOfBounds,
                             TrapCode::HeapOutOfBounds => ExceptionCode::MemoryOutOfBounds,
@@ -101,9 +98,10 @@ pub fn call_protected<T>(
                             TrapCode::BadConversionToInteger => ExceptionCode::IllegalArithmetic,
                             TrapCode::UnreachableCodeReached => ExceptionCode::Unreachable,
                             _ => {
-                                return Err(CallProtError(Box::new(
-                                    "unknown clif trap code".to_string(),
-                                )))
+                                return Err(InvokeError::UnknownTrapCode {
+                                    trap_code: format!("{:?}", trapcode),
+                                    srcloc,
+                                })
                             }
                         },
                         Ok(SIGSEGV) | Ok(SIGBUS) => ExceptionCode::MemoryOutOfBounds,
@@ -112,7 +110,8 @@ pub fn call_protected<T>(
                             "ExceptionCode::Unknown signal:{:?}",
                             Signal::from_c_int(signum)
                         ),
-                    })))
+                    };
+                    Err(InvokeError::TrapCode { srcloc, code })
                 } else {
                     let signal = match Signal::from_c_int(signum) {
                         Ok(SIGFPE) => "floating-point exception",
@@ -123,8 +122,10 @@ pub fn call_protected<T>(
                         _ => "unknown trapped signal",
                     };
                     // When the trap-handler is fully implemented, this will return more information.
-                    let s = format!("unknown trap at {:p} - {}", faulting_addr, signal);
-                    Err(CallProtError(Box::new(s)))
+                    Err(InvokeError::UnknownTrap {
+                        address: faulting_addr as usize,
+                        signal,
+                    })
                 }
             }
         } else {

diff --git a/lib/clif-backend/src/signal/windows.rs b/lib/clif-backend/src/signal/windows.rs
@@ -1,6 +1,6 @@
 use crate::{
     relocation::{TrapCode, TrapData},
-    signal::{CallProtError, HandlerData},
+    signal::HandlerData,
 };
 use std::{
     cell::Cell,
@@ -9,6 +9,7 @@ use std::{
 };
 use wasmer_runtime_core::{
     backend::ExceptionCode,
+    error::InvokeError,
     typed_func::Trampoline,
     vm::{Ctx, Func},
 };
@@ -32,14 +33,42 @@ thread_local! {
     pub static CURRENT_EXECUTABLE_BUFFER: Cell<*const c_void> = Cell::new(ptr::null());
 }
 
+fn get_signal_name(code: DWORD) -> &'static str {
+    match code {
+        EXCEPTION_FLT_DENORMAL_OPERAND
+        | EXCEPTION_FLT_DIVIDE_BY_ZERO
+        | EXCEPTION_FLT_INEXACT_RESULT
+        | EXCEPTION_FLT_INVALID_OPERATION
+        | EXCEPTION_FLT_OVERFLOW
+        | EXCEPTION_FLT_STACK_CHECK
+        | EXCEPTION_FLT_UNDERFLOW => "floating-point exception",
+        EXCEPTION_ILLEGAL_INSTRUCTION => "illegal instruction",
+        EXCEPTION_ACCESS_VIOLATION => "segmentation violation",
+        EXCEPTION_DATATYPE_MISALIGNMENT => "datatype misalignment",
+        EXCEPTION_BREAKPOINT => "breakpoint",
+        EXCEPTION_SINGLE_STEP => "single step",
+        EXCEPTION_ARRAY_BOUNDS_EXCEEDED => "array bounds exceeded",
+        EXCEPTION_INT_DIVIDE_BY_ZERO => "integer division by zero",
+        EXCEPTION_INT_OVERFLOW => "integer overflow",
+        EXCEPTION_PRIV_INSTRUCTION => "privileged instruction",
+        EXCEPTION_IN_PAGE_ERROR => "in page error",
+        EXCEPTION_NONCONTINUABLE_EXCEPTION => "non continuable exception",
+        EXCEPTION_STACK_OVERFLOW => "stack overflow",
+        EXCEPTION_GUARD_PAGE => "guard page",
+        EXCEPTION_INVALID_HANDLE => "invalid handle",
+        EXCEPTION_POSSIBLE_DEADLOCK => "possible deadlock",
+        _ => "unknown exception code",
+    }
+}
+
 pub fn call_protected(
     handler_data: &HandlerData,
     trampoline: Trampoline,
     ctx: *mut Ctx,
     func: NonNull<Func>,
     param_vec: *const u64,
     return_vec: *mut u64,
-) -> Result<(), CallProtError> {
+) -> Result<(), InvokeError> {
     // TODO: trap early
     // user code error
     //    if let Some(msg) = super::TRAP_EARLY_DATA.with(|cell| cell.replace(None)) {
@@ -58,64 +87,45 @@ pub fn call_protected(
         instruction_pointer,
     } = result.unwrap_err();
 
-    if let Some(TrapData {
-        trapcode,
-        srcloc: _,
-    }) = handler_data.lookup(instruction_pointer as _)
-    {
-        Err(CallProtError(Box::new(match code as DWORD {
+    if let Some(TrapData { trapcode, srcloc }) = handler_data.lookup(instruction_pointer as _) {
+        let exception_code = match code as DWORD {
             EXCEPTION_ACCESS_VIOLATION => ExceptionCode::MemoryOutOfBounds,
             EXCEPTION_ILLEGAL_INSTRUCTION => match trapcode {
                 TrapCode::BadSignature => ExceptionCode::IncorrectCallIndirectSignature,
                 TrapCode::IndirectCallToNull => ExceptionCode::CallIndirectOOB,
                 TrapCode::HeapOutOfBounds => ExceptionCode::MemoryOutOfBounds,
                 TrapCode::TableOutOfBounds => ExceptionCode::CallIndirectOOB,
                 TrapCode::UnreachableCodeReached => ExceptionCode::Unreachable,
-                _ => return Err(CallProtError(Box::new("unknown trap code".to_string()))),
+                _ => {
+                    return Err(InvokeError::UnknownTrapCode {
+                        trap_code: format!("{}", code as DWORD),
+                        srcloc,
+                    })
+                }
             },
             EXCEPTION_STACK_OVERFLOW => ExceptionCode::MemoryOutOfBounds,
             EXCEPTION_INT_DIVIDE_BY_ZERO | EXCEPTION_INT_OVERFLOW => {
                 ExceptionCode::IllegalArithmetic
             }
             _ => {
-                return Err(CallProtError(Box::new(
-                    "unknown exception code".to_string(),
-                )))
+                let signal = get_signal_name(code as DWORD);
+                return Err(InvokeError::UnknownTrap {
+                    address: exception_address as usize,
+                    signal,
+                });
             }
-        })))
-    } else {
-        let signal = match code as DWORD {
-            EXCEPTION_FLT_DENORMAL_OPERAND
-            | EXCEPTION_FLT_DIVIDE_BY_ZERO
-            | EXCEPTION_FLT_INEXACT_RESULT
-            | EXCEPTION_FLT_INVALID_OPERATION
-            | EXCEPTION_FLT_OVERFLOW
-            | EXCEPTION_FLT_STACK_CHECK
-            | EXCEPTION_FLT_UNDERFLOW => "floating-point exception",
-            EXCEPTION_ILLEGAL_INSTRUCTION => "illegal instruction",
-            EXCEPTION_ACCESS_VIOLATION => "segmentation violation",
-            EXCEPTION_DATATYPE_MISALIGNMENT => "datatype misalignment",
-            EXCEPTION_BREAKPOINT => "breakpoint",
-            EXCEPTION_SINGLE_STEP => "single step",
-            EXCEPTION_ARRAY_BOUNDS_EXCEEDED => "array bounds exceeded",
-            EXCEPTION_INT_DIVIDE_BY_ZERO => "int div by zero",
-            EXCEPTION_INT_OVERFLOW => "int overflow",
-            EXCEPTION_PRIV_INSTRUCTION => "priv instruction",
-            EXCEPTION_IN_PAGE_ERROR => "in page error",
-            EXCEPTION_NONCONTINUABLE_EXCEPTION => "non continuable exception",
-            EXCEPTION_STACK_OVERFLOW => "stack overflow",
-            EXCEPTION_GUARD_PAGE => "guard page",
-            EXCEPTION_INVALID_HANDLE => "invalid handle",
-            EXCEPTION_POSSIBLE_DEADLOCK => "possible deadlock",
-            _ => "unknown exception code",
         };
+        return Err(InvokeError::TrapCode {
+            srcloc,
+            code: exception_code,
+        });
+    } else {
+        let signal = get_signal_name(code as DWORD);
 
-        let s = format!(
-            "unhandled trap at {:x} - code #{:x}: {}",
-            exception_address, code, signal,
-        );
-
-        Err(CallProtError(Box::new(s)))
+        Err(InvokeError::UnknownTrap {
+            address: exception_address as usize,
+            signal,
+        })
     }
 }
 

diff --git a/lib/llvm-backend/cpp/object_loader.cpp b/lib/llvm-backend/cpp/object_loader.cpp
@@ -13,7 +13,7 @@ MemoryManager::~MemoryManager() {
   callbacks.dealloc_memory(read_section.base, read_section.size);
   callbacks.dealloc_memory(readwrite_section.base, readwrite_section.size);
 }
-void unwinding_setjmp(jmp_buf stack_out, void (*func)(void *), void *userdata) {
+void unwinding_setjmp(jmp_buf &stack_out, void (*func)(void *), void *userdata) {
   if (setjmp(stack_out)) {
 
   } else {

diff --git a/lib/llvm-backend/cpp/object_loader.hh b/lib/llvm-backend/cpp/object_loader.hh
@@ -49,9 +49,7 @@ typedef struct {
   visit_fde_t visit_fde;
 } callbacks_t;
 
-typedef struct {
-  size_t data, vtable;
-} box_any_t;
+typedef struct runtime_error_t_privdecl runtime_error_t;
 
 enum WasmTrapType {
   Unreachable = 0,
@@ -65,6 +63,9 @@ enum WasmTrapType {
 
 extern "C" void callback_trampoline(void *, void *);
 
+extern "C" void copy_runtime_error(runtime_error_t *src, runtime_error_t *dst);
+extern "C" void free_runtime_error_without_drop(runtime_error_t*);
+
 struct MemoryManager : llvm::RuntimeDyld::MemoryManager {
 public:
   MemoryManager(callbacks_t callbacks) : callbacks(callbacks) {}
@@ -121,7 +122,7 @@ private:
 
 struct WasmErrorSink {
   WasmTrapType *trap_out;
-  box_any_t *user_error;
+  runtime_error_t *user_error;
 };
 
 struct WasmException : std::exception {
@@ -149,17 +150,25 @@ public:
 
 struct UserException : UncatchableException {
 public:
-  UserException(size_t data, size_t vtable) : error_data({data, vtable}) {}
+  UserException(size_t data) {
+      error_data = reinterpret_cast<runtime_error_t*>(data);
+  }
+
+  UserException(const UserException &) = delete;
+
+  ~UserException() {
+    free_runtime_error_without_drop(error_data);
+  }
 
   virtual std::string description() const noexcept override {
     return "user exception";
   }
 
-  // The parts of a `Box<dyn Any>`.
-  box_any_t error_data;
+  // The pointer to `Option<RuntimeError>`.
+  runtime_error_t *error_data;
 
   virtual void write_error(WasmErrorSink &out) const noexcept override {
-    *out.user_error = error_data;
+    copy_runtime_error(error_data, out.user_error);
   }
 };
 
@@ -274,10 +283,10 @@ result_t module_load(const uint8_t *mem_ptr, size_t mem_size,
 
 void module_delete(WasmModule *module) { delete module; }
 
-// Throw a fat pointer that's assumed to be `*mut dyn Any` on the rust
+// Throw a pointer that's assumed to be `*mut Option<RuntimeError>` on the rust
 // side.
-[[noreturn]] void throw_any(size_t data, size_t vtable) {
-  unsafe_unwind(new UserException(data, vtable));
+[[noreturn]] void throw_runtime_error(size_t data) {
+  unsafe_unwind(new UserException(data));
 }
 
 // Throw a pointer that's assumed to be codegen::BreakpointHandler on the
@@ -288,7 +297,7 @@ void module_delete(WasmModule *module) { delete module; }
 
 bool cxx_invoke_trampoline(trampoline_t trampoline, void *ctx, void *func,
                        void *params, void *results, WasmTrapType *trap_out,
-                       box_any_t *user_error, void *invoke_env) noexcept {
+                       runtime_error_t *user_error, void *invoke_env) noexcept {
   try {
     catch_unwind([trampoline, ctx, func, params, results]() {
       trampoline(ctx, func, params, results);