Merge #348

348: Internal runtime ↔️ backend abstraction refactor. r=lachlansneff a=lachlansneff This replaces the `ProtectedCaller` and `FuncResolver` traits with a combined `RunnableModule` trait. I'd like to get this merged, since it's standalone, but there are follow-up PRs I'd like to make that will further change the abstraction Co-authored-by: Lachlan Sneff <lachlan.sneff@gmail.com> Co-authored-by: Syrus Akbary <me@syrusakbary.com>
wasmerio · Apr 17, 2019 · e8d335c · e8d335c
2 parents c55952b + 088263a
commit e8d335c
Show file tree

Hide file tree

Showing 20 changed files with 318 additions and 595 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]**
+- [#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
 - [#351](https://github.com/wasmerio/wasmer/pull/351) Add hidden option to specify wasm program name (can be used to improve error messages)

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bors.toml b/bors.toml
@@ -2,8 +2,9 @@ status = [
     "ci/circleci: lint",
     "ci/circleci: test",
     "ci/circleci: test-macos",
+    "ci/circleci: test-rust-nightly",
     "continuous-integration/appveyor/branch"
 ]
 required_approvals = 1
 timeout_sec = 900
-delete_merged_branches = true 
+delete_merged_branches = true 
diff --git a/lib/clif-backend/src/module.rs b/lib/clif-backend/src/module.rs
@@ -73,16 +73,15 @@ impl Module {
             handler_data.clone(),
         )?;
 
-        let protected_caller = Caller::new(&self.info, handler_data, trampolines);
-
         let cache_gen = Box::new(CacheGenerator::new(
             backend_cache,
             Arc::clone(&func_resolver.memory),
         ));
 
+        let runnable_module = Caller::new(handler_data, trampolines, func_resolver);
+
         Ok(ModuleInner {
-            func_resolver: Box::new(func_resolver),
-            protected_caller: Box::new(protected_caller),
+            runnable_module: Box::new(runnable_module),
             cache_gen,
 
             info: self.info,
@@ -103,16 +102,15 @@ impl Module {
             )
             .map_err(|e| CacheError::Unknown(format!("{:?}", e)))?;
 
-        let protected_caller = Caller::new(&info, handler_data, trampolines);
-
         let cache_gen = Box::new(CacheGenerator::new(
             backend_cache,
             Arc::clone(&func_resolver.memory),
         ));
 
+        let runnable_module = Caller::new(handler_data, trampolines, func_resolver);
+
         Ok(ModuleInner {
-            func_resolver: Box::new(func_resolver),
-            protected_caller: Box::new(protected_caller),
+            runnable_module: Box::new(runnable_module),
             cache_gen,
 
             info,

diff --git a/lib/clif-backend/src/resolver.rs b/lib/clif-backend/src/resolver.rs
@@ -21,7 +21,6 @@ use wasmer_runtime_core::cache::Error as CacheError;
 use wasmer_runtime_core::{
     self,
     backend::{
-        self,
         sys::{Memory, Protect},
         SigRegistry,
     },
@@ -357,13 +356,8 @@ pub struct FuncResolver {
     pub(crate) memory: Arc<Memory>,
 }
 
-// Implements FuncResolver trait.
-impl backend::FuncResolver for FuncResolver {
-    fn get(
-        &self,
-        _module: &wasmer_runtime_core::module::ModuleInner,
-        index: LocalFuncIndex,
-    ) -> Option<NonNull<vm::Func>> {
+impl FuncResolver {
+    pub fn lookup(&self, index: LocalFuncIndex) -> Option<NonNull<vm::Func>> {
         lookup_func(&self.map, &self.memory, index)
     }
 }

diff --git a/lib/clif-backend/src/signal/mod.rs b/lib/clif-backend/src/signal/mod.rs
@@ -1,16 +1,14 @@
 use crate::relocation::{TrapData, TrapSink};
+use crate::resolver::FuncResolver;
 use crate::trampoline::Trampolines;
-use hashbrown::HashSet;
 use libc::c_void;
 use std::{any::Any, cell::Cell, ptr::NonNull, sync::Arc};
 use wasmer_runtime_core::{
-    backend::{ProtectedCaller, Token, UserTrapper},
-    error::RuntimeResult,
-    export::Context,
-    module::{ExportIndex, ModuleInfo, ModuleInner},
+    backend::{RunnableModule, UserTrapper},
+    module::ModuleInfo,
     typed_func::{Wasm, WasmTrapInfo},
-    types::{FuncIndex, FuncSig, LocalOrImport, SigIndex, Type, Value},
-    vm::{self, ImportBacking},
+    types::{LocalFuncIndex, SigIndex},
+    vm,
 };
 
 #[cfg(unix)]
@@ -39,130 +37,44 @@ impl UserTrapper for Trapper {
 }
 
 pub struct Caller {
-    func_export_set: HashSet<FuncIndex>,
     handler_data: HandlerData,
     trampolines: Arc<Trampolines>,
+    resolver: FuncResolver,
 }
 
 impl Caller {
     pub fn new(
-        module: &ModuleInfo,
         handler_data: HandlerData,
         trampolines: Arc<Trampolines>,
+        resolver: FuncResolver,
     ) -> Self {
-        let mut func_export_set = HashSet::new();
-        for export_index in module.exports.values() {
-            if let ExportIndex::Func(func_index) = export_index {
-                func_export_set.insert(*func_index);
-            }
-        }
-        if let Some(start_func_index) = module.start_func {
-            func_export_set.insert(start_func_index);
-        }
-
         Self {
-            func_export_set,
             handler_data,
             trampolines,
+            resolver,
         }
     }
 }
 
-impl ProtectedCaller for Caller {
-    fn call(
-        &self,
-        module: &ModuleInner,
-        func_index: FuncIndex,
-        params: &[Value],
-        import_backing: &ImportBacking,
-        vmctx: *mut vm::Ctx,
-        _: Token,
-    ) -> RuntimeResult<Vec<Value>> {
-        let (func_ptr, ctx, signature, sig_index) =
-            get_func_from_index(&module, import_backing, func_index);
-
-        let vmctx_ptr = match ctx {
-            Context::External(external_vmctx) => external_vmctx,
-            Context::Internal => vmctx,
-        };
-
-        assert!(self.func_export_set.contains(&func_index));
-
-        assert!(
-            signature.returns().len() <= 1,
-            "multi-value returns not yet supported"
-        );
-
-        assert!(
-            signature.check_param_value_types(params),
-            "incorrect signature"
-        );
-
-        let param_vec: Vec<u64> = params
-            .iter()
-            .map(|val| match val {
-                Value::I32(x) => *x as u64,
-                Value::I64(x) => *x as u64,
-                Value::F32(x) => x.to_bits() as u64,
-                Value::F64(x) => x.to_bits(),
-            })
-            .collect();
-
-        let mut return_vec = vec![0; signature.returns().len()];
-
-        let trampoline = self
-            .trampolines
-            .lookup(sig_index)
-            .expect("that trampoline doesn't exist");
-
-        #[cfg(not(target_os = "windows"))]
-        call_protected(&self.handler_data, || unsafe {
-            // Leap of faith.
-            trampoline(
-                vmctx_ptr,
-                func_ptr,
-                param_vec.as_ptr(),
-                return_vec.as_mut_ptr(),
-            );
-        })?;
-
-        // the trampoline is called from C on windows
-        #[cfg(target_os = "windows")]
-        call_protected(
-            &self.handler_data,
-            trampoline,
-            vmctx_ptr,
-            func_ptr,
-            param_vec.as_ptr(),
-            return_vec.as_mut_ptr(),
-        )?;
-
-        Ok(return_vec
-            .iter()
-            .zip(signature.returns().iter())
-            .map(|(&x, ty)| match ty {
-                Type::I32 => Value::I32(x as i32),
-                Type::I64 => Value::I64(x as i64),
-                Type::F32 => Value::F32(f32::from_bits(x as u32)),
-                Type::F64 => Value::F64(f64::from_bits(x as u64)),
-            })
-            .collect())
+impl RunnableModule for Caller {
+    fn get_func(&self, _: &ModuleInfo, func_index: LocalFuncIndex) -> Option<NonNull<vm::Func>> {
+        self.resolver.lookup(func_index)
     }
 
-    fn get_wasm_trampoline(&self, module: &ModuleInner, sig_index: SigIndex) -> Option<Wasm> {
+    fn get_trampoline(&self, _: &ModuleInfo, sig_index: SigIndex) -> Option<Wasm> {
         unsafe extern "C" fn invoke(
             trampoline: unsafe extern "C" fn(*mut vm::Ctx, NonNull<vm::Func>, *const u64, *mut u64),
             ctx: *mut vm::Ctx,
             func: NonNull<vm::Func>,
             args: *const u64,
             rets: *mut u64,
-            trap_info: *mut WasmTrapInfo,
+            _trap_info: *mut WasmTrapInfo,
             invoke_env: Option<NonNull<c_void>>,
         ) -> bool {
             let handler_data = &*invoke_env.unwrap().cast().as_ptr();
 
             #[cfg(not(target_os = "windows"))]
-            let res = call_protected(handler_data, || unsafe {
+            let res = call_protected(handler_data, || {
                 // Leap of faith.
                 trampoline(ctx, func, args, rets);
             })
@@ -194,40 +106,6 @@ impl ProtectedCaller for Caller {
     }
 }
 
-fn get_func_from_index<'a>(
-    module: &'a ModuleInner,
-    import_backing: &ImportBacking,
-    func_index: FuncIndex,
-) -> (NonNull<vm::Func>, Context, &'a FuncSig, SigIndex) {
-    let sig_index = *module
-        .info
-        .func_assoc
-        .get(func_index)
-        .expect("broken invariant, incorrect func index");
-
-    let (func_ptr, ctx) = match func_index.local_or_import(&module.info) {
-        LocalOrImport::Local(local_func_index) => (
-            module
-                .func_resolver
-                .get(&module, local_func_index)
-                .expect("broken invariant, func resolver not synced with module.exports")
-                .cast(),
-            Context::Internal,
-        ),
-        LocalOrImport::Import(imported_func_index) => {
-            let imported_func = import_backing.imported_func(imported_func_index);
-            (
-                NonNull::new(imported_func.func as *mut _).unwrap(),
-                Context::External(imported_func.vmctx),
-            )
-        }
-    };
-
-    let signature = &module.info.signatures[sig_index];
-
-    (func_ptr, ctx, signature, sig_index)
-}
-
 unsafe impl Send for HandlerData {}
 unsafe impl Sync for HandlerData {}
 

diff --git a/lib/clif-backend/src/trampoline.rs b/lib/clif-backend/src/trampoline.rs
@@ -6,7 +6,6 @@ use cranelift_codegen::{
     isa, Context,
 };
 use hashbrown::HashMap;
-use std::ffi::c_void;
 use std::{iter, mem, ptr::NonNull};
 use wasmer_runtime_core::{
     backend::sys::{Memory, Protect},