Merge #366

366: Remove UserTrapper trait r=lachlansneff a=lachlansneff This removes the `UserTrapper` trait and the thread_local EARLY_TRAPPER variable. Fixes bug reported in #365. Co-authored-by: Lachlan Sneff <[email protected]>
wasmerio · Apr 18, 2019 · b146a9f · b146a9f
2 parents ccad887 + 36d73d1
commit b146a9f
Show file tree

Hide file tree

Showing 9 changed files with 18 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@ All PRs to the Wasmer repository must add to this file.
 Blocks of changes will separated by version increments.
 
 ## **[Unreleased]**
+- [#366](https://github.com/wasmerio/wasmer/pull/366) Remove `UserTrapper` trait to fix [#365](https://github.com/wasmerio/wasmer/issues/365).
 - [#348](https://github.com/wasmerio/wasmer/pull/348) Refactor internal runtime ↔️ backend abstraction.
 - [#355](https://github.com/wasmerio/wasmer/pull/355) Misc changes to `Cargo.toml`s for publishing
 - [#352](https://github.com/wasmerio/wasmer/pull/352) Bump version numbers to 0.3.0

diff --git a/lib/clif-backend/src/signal/mod.rs b/lib/clif-backend/src/signal/mod.rs
@@ -4,7 +4,7 @@ use crate::trampoline::Trampolines;
 use libc::c_void;
 use std::{any::Any, cell::Cell, ptr::NonNull, sync::Arc};
 use wasmer_runtime_core::{
-    backend::{RunnableModule, UserTrapper},
+    backend::RunnableModule,
     module::ModuleInfo,
     typed_func::{Wasm, WasmTrapInfo},
     types::{LocalFuncIndex, SigIndex},
@@ -27,15 +27,6 @@ thread_local! {
     pub static TRAP_EARLY_DATA: Cell<Option<Box<dyn Any>>> = Cell::new(None);
 }
 
-pub struct Trapper;
-
-impl UserTrapper for Trapper {
-    unsafe fn do_early_trap(&self, data: Box<dyn Any>) -> ! {
-        TRAP_EARLY_DATA.with(|cell| cell.set(Some(data)));
-        trigger_trap()
-    }
-}
-
 pub struct Caller {
     handler_data: HandlerData,
     trampolines: Arc<Trampolines>,
@@ -101,8 +92,9 @@ impl RunnableModule for Caller {
         })
     }
 
-    fn get_early_trapper(&self) -> Box<dyn UserTrapper> {
-        Box::new(Trapper)
+    unsafe fn do_early_trap(&self, data: Box<dyn Any>) -> ! {
+        TRAP_EARLY_DATA.with(|cell| cell.set(Some(data)));
+        trigger_trap()
     }
 }
 

diff --git a/lib/llvm-backend/src/backend.rs b/lib/llvm-backend/src/backend.rs
@@ -18,7 +18,7 @@ use std::{
     sync::Once,
 };
 use wasmer_runtime_core::{
-    backend::{RunnableModule, UserTrapper},
+    backend::RunnableModule,
     module::ModuleInfo,
     structures::TypedIndex,
     typed_func::{Wasm, WasmTrapInfo},
@@ -317,14 +317,6 @@ impl RunnableModule for LLVMBackend {
         Some(unsafe { Wasm::from_raw_parts(trampoline, invoke_trampoline, None) })
     }
 
-    fn get_early_trapper(&self) -> Box<dyn UserTrapper> {
-        Box::new(Placeholder)
-    }
-}
-
-struct Placeholder;
-
-impl UserTrapper for Placeholder {
     unsafe fn do_early_trap(&self, data: Box<dyn Any>) -> ! {
         throw_any(Box::leak(data))
     }

diff --git a/lib/runtime-core/src/backend.rs b/lib/runtime-core/src/backend.rs
@@ -65,10 +65,6 @@ pub trait Compiler {
     unsafe fn from_cache(&self, cache: Artifact, _: Token) -> Result<ModuleInner, CacheError>;
 }
 
-pub trait UserTrapper {
-    unsafe fn do_early_trap(&self, data: Box<dyn Any>) -> !;
-}
-
 pub trait RunnableModule: Send + Sync {
     /// This returns a pointer to the function designated by the `local_func_index`
     /// parameter.
@@ -83,7 +79,7 @@ pub trait RunnableModule: Send + Sync {
     /// signature and an invoke function that can call the trampoline.
     fn get_trampoline(&self, info: &ModuleInfo, sig_index: SigIndex) -> Option<Wasm>;
 
-    fn get_early_trapper(&self) -> Box<dyn UserTrapper>;
+    unsafe fn do_early_trap(&self, data: Box<dyn Any>) -> !;
 }
 
 pub trait CacheGen: Send + Sync {

diff --git a/lib/runtime-core/src/module.rs b/lib/runtime-core/src/module.rs
@@ -4,7 +4,6 @@ use crate::{
     error,
     import::ImportObject,
     structures::{Map, TypedIndex},
-    typed_func::EARLY_TRAPPER,
     types::{
         FuncIndex, FuncSig, GlobalDescriptor, GlobalIndex, GlobalInit, ImportedFuncIndex,
         ImportedGlobalIndex, ImportedMemoryIndex, ImportedTableIndex, Initializer,
@@ -92,10 +91,6 @@ pub struct Module {
 
 impl Module {
     pub(crate) fn new(inner: Arc<ModuleInner>) -> Self {
-        unsafe {
-            EARLY_TRAPPER
-                .with(|ucell| *ucell.get() = Some(inner.runnable_module.get_early_trapper()));
-        }
         Module { inner }
     }
 

diff --git a/lib/runtime-core/src/typed_func.rs b/lib/runtime-core/src/typed_func.rs
@@ -1,5 +1,4 @@
 use crate::{
-    backend::UserTrapper,
     error::RuntimeError,
     export::{Context, Export, FuncPointer},
     import::IsExport,
@@ -8,7 +7,6 @@ use crate::{
 };
 use std::{
     any::Any,
-    cell::UnsafeCell,
     ffi::c_void,
     fmt,
     marker::PhantomData,
@@ -17,10 +15,6 @@ use std::{
     sync::Arc,
 };
 
-thread_local! {
-    pub static EARLY_TRAPPER: UnsafeCell<Option<Box<dyn UserTrapper>>> = UnsafeCell::new(None);
-}
-
 #[repr(C)]
 pub enum WasmTrapInfo {
     Unreachable = 0,
@@ -354,12 +348,7 @@ macro_rules! impl_traits {
                     };
 
                     unsafe {
-                        if let Some(early_trapper) = &*EARLY_TRAPPER.with(|ucell| ucell.get()) {
-                            early_trapper.do_early_trap(err)
-                        } else {
-                            eprintln!("panic handling not setup");
-                            std::process::exit(1)
-                        }
+                        (&*ctx.module).runnable_module.do_early_trap(err)
                     }
                 }
 

diff --git a/lib/runtime-core/src/vm.rs b/lib/runtime-core/src/vm.rs
@@ -22,7 +22,7 @@ pub struct Ctx {
 
     local_backing: *mut LocalBacking,
     import_backing: *mut ImportBacking,
-    module: *const ModuleInner,
+    pub(crate) module: *const ModuleInner,
 
     pub data: *mut c_void,
     pub data_finalizer: Option<fn(data: *mut c_void)>,
@@ -544,11 +544,12 @@ mod vm_ctx_tests {
 
     fn generate_module() -> ModuleInner {
         use super::Func;
-        use crate::backend::{sys::Memory, Backend, CacheGen, RunnableModule, UserTrapper};
+        use crate::backend::{sys::Memory, Backend, CacheGen, RunnableModule};
         use crate::cache::Error as CacheError;
         use crate::typed_func::Wasm;
         use crate::types::{LocalFuncIndex, SigIndex};
         use hashbrown::HashMap;
+        use std::any::Any;
         use std::ptr::NonNull;
         struct Placeholder;
         impl RunnableModule for Placeholder {
@@ -563,7 +564,7 @@ mod vm_ctx_tests {
             fn get_trampoline(&self, _module: &ModuleInfo, _sig_index: SigIndex) -> Option<Wasm> {
                 unimplemented!()
             }
-            fn get_early_trapper(&self) -> Box<dyn UserTrapper> {
+            unsafe fn do_early_trap(&self, _: Box<dyn Any>) -> ! {
                 unimplemented!()
             }
         }

diff --git a/lib/runtime/examples/call.rs b/lib/runtime/examples/call.rs
@@ -59,9 +59,9 @@ fn main() -> Result<(), error::Error> {
       },
     })?;
 
-    let foo = instance.dyn_func("dbz")?;
+    let foo: Func<(), i32> = instance.func("dbz")?;
 
-    let result = foo.call(&[]);
+    let result = foo.call();
 
     println!("result: {:?}", result);
 

diff --git a/lib/singlepass-backend/src/codegen_x64.rs b/lib/singlepass-backend/src/codegen_x64.rs
@@ -11,7 +11,7 @@ use smallvec::SmallVec;
 use std::ptr::NonNull;
 use std::{any::Any, collections::HashMap, sync::Arc};
 use wasmer_runtime_core::{
-    backend::{RunnableModule, UserTrapper},
+    backend::RunnableModule,
     memory::MemoryType,
     module::ModuleInfo,
     structures::{Map, TypedIndex},
@@ -249,17 +249,9 @@ impl RunnableModule for X64ExecutionContext {
         })
     }
 
-    fn get_early_trapper(&self) -> Box<dyn UserTrapper> {
-        pub struct Trapper;
-
-        impl UserTrapper for Trapper {
-            unsafe fn do_early_trap(&self, data: Box<Any>) -> ! {
-                protect_unix::TRAP_EARLY_DATA.with(|x| x.set(Some(data)));
-                protect_unix::trigger_trap();
-            }
-        }
-
-        Box::new(Trapper)
+    unsafe fn do_early_trap(&self, data: Box<Any>) -> ! {
+        protect_unix::TRAP_EARLY_DATA.with(|x| x.set(Some(data)));
+        protect_unix::trigger_trap();
     }
 }