Merge #1710

1710: Move function call trampolines to Artifact instead of EngineInner. r=nlewycky a=nlewycky # Description Move function call trampolines to Artifact instead of EngineInner. # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Nick Lewycky <[email protected]> Co-authored-by: nlewycky <[email protected]> Co-authored-by: Syrus Akbary <[email protected]>
wasmerio · Oct 20, 2020 · a3ffc9e · a3ffc9e
2 parents b04b4b0 + f55121c
commit a3ffc9e
Show file tree

Hide file tree

Showing 18 changed files with 131 additions and 135 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 
 ## **[Unreleased]**
 
+- [#1710](https://github.com/wasmerio/wasmer/pull/1710) Memory for function call trampolines is now owned by the Artifact.
 ### Added
 
 - [#1741](https://github.com/wasmerio/wasmer/pull/1741) Implement `wasm_memory_type` in the Wasm C API.

diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs
@@ -18,7 +18,7 @@ use wasmer_vm::{
 /// A function defined in the Wasm module
 #[derive(Clone, PartialEq)]
 pub struct WasmFunctionDefinition {
-    // The trampoline to do the call
+    // Address of the trampoline to do the call.
     pub(crate) trampoline: VMTrampoline,
 }
 
@@ -95,6 +95,7 @@ impl Function {
                 kind: VMFunctionKind::Dynamic,
                 vmctx,
                 signature: ty.clone(),
+                call_trampoline: None,
             },
         }
     }
@@ -144,6 +145,7 @@ impl Function {
                 kind: VMFunctionKind::Dynamic,
                 vmctx,
                 signature: ty.clone(),
+                call_trampoline: None,
             },
         }
     }
@@ -185,6 +187,7 @@ impl Function {
                 vmctx,
                 signature,
                 kind: VMFunctionKind::Static,
+                call_trampoline: None,
             },
         }
     }
@@ -238,6 +241,7 @@ impl Function {
                 kind: VMFunctionKind::Static,
                 vmctx,
                 signature,
+                call_trampoline: None,
             },
         }
     }
@@ -351,15 +355,20 @@ impl Function {
     }
 
     pub(crate) fn from_export(store: &Store, wasmer_export: ExportFunction) -> Self {
-        let vmsignature = store.engine().register_signature(&wasmer_export.signature);
-        let trampoline = store
-            .engine()
-            .function_call_trampoline(vmsignature)
-            .expect("Can't get call trampoline for the function");
-        Self {
-            store: store.clone(),
-            definition: FunctionDefinition::Wasm(WasmFunctionDefinition { trampoline }),
-            exported: wasmer_export,
+        if let Some(trampoline) = wasmer_export.call_trampoline {
+            Self {
+                store: store.clone(),
+                definition: FunctionDefinition::Wasm(WasmFunctionDefinition { trampoline }),
+                exported: wasmer_export,
+            }
+        } else {
+            Self {
+                store: store.clone(),
+                definition: FunctionDefinition::Host(HostFunctionDefinition {
+                    has_env: !wasmer_export.vmctx.is_null(),
+                }),
+                exported: wasmer_export,
+            }
         }
     }
 

diff --git a/lib/api/src/native.rs b/lib/api/src/native.rs
@@ -69,6 +69,7 @@ where
             vmctx: other.vmctx,
             signature,
             kind: other.arg_kind,
+            call_trampoline: None,
         }
     }
 }
@@ -88,6 +89,7 @@ where
                 vmctx: other.vmctx,
                 signature,
                 kind: other.arg_kind,
+                call_trampoline: None,
             },
         }
     }

diff --git a/lib/api/src/types.rs b/lib/api/src/types.rs
@@ -78,6 +78,7 @@ impl ValFuncRef for Val {
             // are converted to use the trampolines with static signatures).
             kind: wasmer_vm::VMFunctionKind::Static,
             vmctx: item.vmctx,
+            call_trampoline: None,
         };
         let f = Function::from_export(store, export);
         Self::FuncRef(f)

diff --git a/lib/engine-jit/src/artifact.rs b/lib/engine-jit/src/artifact.rs
@@ -20,12 +20,15 @@ use wasmer_types::{
     FunctionIndex, LocalFunctionIndex, MemoryIndex, OwnedDataInitializer, SignatureIndex,
     TableIndex,
 };
-use wasmer_vm::{FunctionBodyPtr, MemoryStyle, ModuleInfo, TableStyle, VMSharedSignatureIndex};
+use wasmer_vm::{
+    FunctionBodyPtr, MemoryStyle, ModuleInfo, TableStyle, VMSharedSignatureIndex, VMTrampoline,
+};
 
 /// A compiled wasm module, ready to be instantiated.
 pub struct JITArtifact {
     serializable: SerializableModule,
     finished_functions: BoxedSlice<LocalFunctionIndex, FunctionBodyPtr>,
+    finished_function_call_trampolines: BoxedSlice<SignatureIndex, VMTrampoline>,
     finished_dynamic_function_trampolines: BoxedSlice<FunctionIndex, FunctionBodyPtr>,
     signatures: BoxedSlice<SignatureIndex, VMSharedSignatureIndex>,
     frame_info_registration: Mutex<Option<GlobalFrameInfoRegistration>>,
@@ -150,7 +153,7 @@ impl JITArtifact {
     ) -> Result<Self, CompileError> {
         let (
             finished_functions,
-            _finished_function_call_trampolines,
+            finished_function_call_trampolines,
             finished_dynamic_function_trampolines,
             custom_sections,
         ) = inner_jit.allocate(
@@ -201,13 +204,16 @@ impl JITArtifact {
         inner_jit.publish_eh_frame(eh_frame)?;
 
         let finished_functions = finished_functions.into_boxed_slice();
+        let finished_function_call_trampolines =
+            finished_function_call_trampolines.into_boxed_slice();
         let finished_dynamic_function_trampolines =
             finished_dynamic_function_trampolines.into_boxed_slice();
         let signatures = signatures.into_boxed_slice();
 
         Ok(Self {
             serializable,
             finished_functions,
+            finished_function_call_trampolines,
             finished_dynamic_function_trampolines,
             signatures,
             frame_info_registration: Mutex::new(None),
@@ -270,6 +276,10 @@ impl Artifact for JITArtifact {
         &self.finished_functions
     }
 
+    fn finished_function_call_trampolines(&self) -> &BoxedSlice<SignatureIndex, VMTrampoline> {
+        &self.finished_function_call_trampolines
+    }
+
     // TODO: return *const instead of *mut
     fn finished_dynamic_function_trampolines(&self) -> &BoxedSlice<FunctionIndex, FunctionBodyPtr> {
         &self.finished_dynamic_function_trampolines

diff --git a/lib/engine-jit/src/engine.rs b/lib/engine-jit/src/engine.rs
@@ -1,7 +1,6 @@
 //! JIT compilation.
 
 use crate::{CodeMemory, JITArtifact};
-use std::collections::HashMap;
 use std::sync::{Arc, Mutex};
 #[cfg(feature = "compiler")]
 use wasmer_compiler::Compiler;
@@ -33,7 +32,6 @@ impl JITEngine {
         Self {
             inner: Arc::new(Mutex::new(JITEngineInner {
                 compiler: Some(compiler),
-                function_call_trampolines: HashMap::new(),
                 code_memory: vec![],
                 signatures: SignatureRegistry::new(),
                 features,
@@ -61,7 +59,6 @@ impl JITEngine {
             inner: Arc::new(Mutex::new(JITEngineInner {
                 #[cfg(feature = "compiler")]
                 compiler: None,
-                function_call_trampolines: HashMap::new(),
                 code_memory: vec![],
                 signatures: SignatureRegistry::new(),
                 features: Features::default(),
@@ -98,11 +95,6 @@ impl Engine for JITEngine {
         compiler.signatures().lookup(sig)
     }
 
-    /// Retrieves a trampoline given a signature
-    fn function_call_trampoline(&self, sig: VMSharedSignatureIndex) -> Option<VMTrampoline> {
-        self.inner().function_call_trampoline(sig)
-    }
-
     /// Validates a WebAssembly module
     fn validate(&self, binary: &[u8]) -> Result<(), CompileError> {
         self.inner().validate(binary)
@@ -150,8 +142,6 @@ pub struct JITEngineInner {
     /// The compiler
     #[cfg(feature = "compiler")]
     compiler: Option<Box<dyn Compiler + Send>>,
-    /// Pointers to trampoline functions used to enter particular signatures
-    function_call_trampolines: HashMap<VMSharedSignatureIndex, VMTrampoline>,
     /// The features to compile the Wasm module with
     features: Features,
     /// The code memory is responsible of publishing the compiled
@@ -196,15 +186,15 @@ impl JITEngineInner {
     #[allow(clippy::type_complexity)]
     pub(crate) fn allocate(
         &mut self,
-        module: &ModuleInfo,
+        _module: &ModuleInfo,
         functions: &PrimaryMap<LocalFunctionIndex, FunctionBody>,
         function_call_trampolines: &PrimaryMap<SignatureIndex, FunctionBody>,
         dynamic_function_trampolines: &PrimaryMap<FunctionIndex, FunctionBody>,
         custom_sections: &PrimaryMap<SectionIndex, CustomSection>,
     ) -> Result<
         (
             PrimaryMap<LocalFunctionIndex, FunctionBodyPtr>,
-            PrimaryMap<SignatureIndex, FunctionBodyPtr>,
+            PrimaryMap<SignatureIndex, VMTrampoline>,
             PrimaryMap<FunctionIndex, FunctionBodyPtr>,
             PrimaryMap<SectionIndex, SectionBodyPtr>,
         ),
@@ -241,20 +231,15 @@ impl JITEngineInner {
             .map(|slice| FunctionBodyPtr(slice as *mut [_]))
             .collect::<PrimaryMap<LocalFunctionIndex, _>>();
 
-        let mut allocated_function_call_trampolines: PrimaryMap<SignatureIndex, FunctionBodyPtr> =
+        let mut allocated_function_call_trampolines: PrimaryMap<SignatureIndex, VMTrampoline> =
             PrimaryMap::new();
-        for ((sig_index, _), ptr) in function_call_trampolines.iter().zip(
-            allocated_functions
-                .drain(0..function_call_trampolines.len())
-                .map(|slice| FunctionBodyPtr(slice as *mut [_])),
-        ) {
-            let func_type = &module.signatures[sig_index];
-            let index = self.signatures.register(&func_type);
-            allocated_function_call_trampolines.push(ptr);
-            let trampoline = unsafe {
-                std::mem::transmute::<*const VMFunctionBody, VMTrampoline>((**ptr).as_ptr())
-            };
-            self.function_call_trampolines.insert(index, trampoline);
+        for ptr in allocated_functions
+            .drain(0..function_call_trampolines.len())
+            .map(|slice| slice.as_ptr())
+        {
+            let trampoline =
+                unsafe { std::mem::transmute::<*const VMFunctionBody, VMTrampoline>(ptr) };
+            allocated_function_call_trampolines.push(trampoline);
         }
 
         let allocated_dynamic_function_trampolines = allocated_functions
@@ -309,9 +294,4 @@ impl JITEngineInner {
     pub fn signatures(&self) -> &SignatureRegistry {
         &self.signatures
     }
-
-    /// Gets the trampoline pre-registered for a particular signature
-    pub fn function_call_trampoline(&self, sig: VMSharedSignatureIndex) -> Option<VMTrampoline> {
-        self.function_call_trampolines.get(&sig).cloned()
-    }
 }
diff --git a/lib/engine-native/src/artifact.rs b/lib/engine-native/src/artifact.rs
@@ -19,9 +19,7 @@ use wasmer_compiler::{CompileError, Features, OperatingSystem, Symbol, SymbolReg
 use wasmer_compiler::{
     CompileModuleInfo, FunctionBodyData, ModuleEnvironment, ModuleTranslationState,
 };
-use wasmer_engine::{
-    Artifact, DeserializeError, InstantiationError, LinkError, RuntimeError, SerializeError,
-};
+use wasmer_engine::{Artifact, DeserializeError, InstantiationError, SerializeError};
 #[cfg(feature = "compiler")]
 use wasmer_engine::{Engine, Tunables};
 #[cfg(feature = "compiler")]
@@ -42,9 +40,8 @@ use wasmer_vm::{
 pub struct NativeArtifact {
     sharedobject_path: PathBuf,
     metadata: ModuleMetadata,
-    #[allow(dead_code)]
-    library: Option<Library>,
     finished_functions: BoxedSlice<LocalFunctionIndex, FunctionBodyPtr>,
+    finished_function_call_trampolines: BoxedSlice<SignatureIndex, VMTrampoline>,
     finished_dynamic_function_trampolines: BoxedSlice<FunctionIndex, FunctionBodyPtr>,
     signatures: BoxedSlice<SignatureIndex, VMSharedSignatureIndex>,
 }
@@ -324,14 +321,17 @@ impl NativeArtifact {
         sharedobject_path: PathBuf,
     ) -> Result<Self, CompileError> {
         let finished_functions: PrimaryMap<LocalFunctionIndex, FunctionBodyPtr> = PrimaryMap::new();
+        let finished_function_call_trampolines: PrimaryMap<SignatureIndex, VMTrampoline> =
+            PrimaryMap::new();
         let finished_dynamic_function_trampolines: PrimaryMap<FunctionIndex, FunctionBodyPtr> =
             PrimaryMap::new();
         let signatures: PrimaryMap<SignatureIndex, VMSharedSignatureIndex> = PrimaryMap::new();
         Ok(Self {
             sharedobject_path,
             metadata,
-            library: None,
             finished_functions: finished_functions.into_boxed_slice(),
+            finished_function_call_trampolines: finished_function_call_trampolines
+                .into_boxed_slice(),
             finished_dynamic_function_trampolines: finished_dynamic_function_trampolines
                 .into_boxed_slice(),
             signatures: signatures.into_boxed_slice(),
@@ -367,14 +367,17 @@ impl NativeArtifact {
             }
         }
 
-        // Retrieve function call trampolines (for all signatures in the module)
-        for (sig_index, func_type) in metadata.compile_info.module.signatures.iter() {
+        // Retrieve function call trampolines
+        let mut finished_function_call_trampolines: PrimaryMap<SignatureIndex, VMTrampoline> =
+            PrimaryMap::with_capacity(metadata.compile_info.module.signatures.len());
+        for sig_index in metadata.compile_info.module.signatures.keys() {
             let function_name = metadata.symbol_to_name(Symbol::FunctionCallTrampoline(sig_index));
             unsafe {
                 let trampoline: LibrarySymbol<VMTrampoline> = lib
                     .get(function_name.as_bytes())
                     .map_err(to_compile_error)?;
-                engine_inner.add_trampoline(&func_type, *trampoline);
+                let raw = *trampoline.into_raw();
+                finished_function_call_trampolines.push(raw);
             }
         }
 
@@ -418,21 +421,23 @@ impl NativeArtifact {
 
         // Compute indices into the shared signature table.
         let signatures = {
-            let signature_registry = engine_inner.signatures();
             metadata
                 .compile_info
                 .module
                 .signatures
                 .values()
-                .map(|sig| signature_registry.register(sig))
+                .map(|sig| engine_inner.signatures().register(sig))
                 .collect::<PrimaryMap<_, _>>()
         };
 
+        engine_inner.add_library(lib);
+
         Ok(Self {
             sharedobject_path,
             metadata,
-            library: Some(lib),
             finished_functions: finished_functions.into_boxed_slice(),
+            finished_function_call_trampolines: finished_function_call_trampolines
+                .into_boxed_slice(),
             finished_dynamic_function_trampolines: finished_dynamic_function_trampolines
                 .into_boxed_slice(),
             signatures: signatures.into_boxed_slice(),
@@ -571,6 +576,10 @@ impl Artifact for NativeArtifact {
         &self.finished_functions
     }
 
+    fn finished_function_call_trampolines(&self) -> &BoxedSlice<SignatureIndex, VMTrampoline> {
+        &self.finished_function_call_trampolines
+    }
+
     fn finished_dynamic_function_trampolines(&self) -> &BoxedSlice<FunctionIndex, FunctionBodyPtr> {
         &self.finished_dynamic_function_trampolines
     }
@@ -580,11 +589,6 @@ impl Artifact for NativeArtifact {
     }
 
     fn preinstantiate(&self) -> Result<(), InstantiationError> {
-        if self.library.is_none() {
-            return Err(InstantiationError::Link(LinkError::Trap(
-                RuntimeError::new("Cross compiled artifacts can't be instantiated."),
-            )));
-        }
         Ok(())
     }