Fix/compile not in memory (wasmerio#3573)

* Example of allocated artifact * Better error when Instancing fail because of OS/Arch issue * Add missing brnach for new error --------- Co-authored-by: Syrus Akbary <[email protected]>
DFPercush · Feb 6, 2023 · 1387363 · 1387363
1 parent 0ebc872
commit 1387363
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 28 deletions.
diff --git a/lib/api/src/sys/instance.rs b/lib/api/src/sys/instance.rs
@@ -67,6 +67,11 @@ pub enum InstantiationError {
     /// This error occurs when an import from a different store is used.
     #[error("cannot mix imports from different stores")]
     DifferentStores,
+
+    /// Import from a different Store.
+    /// This error occurs when an import from a different store is used.
+    #[error("incorrect OS or architecture")]
+    DifferentArchOS,
 }
 
 impl From<wasmer_compiler::InstantiationError> for InstantiationError {

diff --git a/lib/api/src/sys/module.rs b/lib/api/src/sys/module.rs
@@ -317,6 +317,11 @@ impl Module {
         store: &mut impl AsStoreMut,
         imports: &[crate::Extern],
     ) -> Result<VMInstance, InstantiationError> {
+        if !self.artifact.allocated() {
+            // Return an error mentioning that the artifact is compiled for a different
+            // platform.
+            return Err(InstantiationError::DifferentArchOS);
+        }
         // Ensure all imports come from the same context.
         for import in imports {
             if !import.is_from_store(store) {

diff --git a/lib/c-api/src/wasm_c_api/instance.rs b/lib/c-api/src/wasm_c_api/instance.rs
@@ -85,6 +85,12 @@ pub unsafe extern "C" fn wasm_instance_new(
 
             return None;
         }
+
+        Err(e @ InstantiationError::DifferentArchOS) => {
+            crate::error::update_last_error(e);
+
+            return None;
+        }
     };
 
     Some(Box::new(wasm_instance_t {

diff --git a/lib/compiler/src/engine/artifact.rs b/lib/compiler/src/engine/artifact.rs
@@ -23,22 +23,20 @@ use wasmer_object::{emit_compilation, emit_data, get_object_for_target, Object};
 #[cfg(any(feature = "static-artifact-create", feature = "static-artifact-load"))]
 use wasmer_types::compilation::symbols::ModuleMetadata;
 use wasmer_types::entity::{BoxedSlice, PrimaryMap};
+#[cfg(feature = "static-artifact-create")]
+use wasmer_types::CompileModuleInfo;
 use wasmer_types::MetadataHeader;
 #[cfg(feature = "static-artifact-load")]
 use wasmer_types::SerializableCompilation;
 use wasmer_types::{
     CompileError, CpuFeature, DataInitializer, DeserializeError, FunctionIndex, LocalFunctionIndex,
-    MemoryIndex, ModuleInfo, OwnedDataInitializer, SignatureIndex, TableIndex,
+    MemoryIndex, ModuleInfo, OwnedDataInitializer, SignatureIndex, TableIndex, Target,
 };
-#[cfg(feature = "static-artifact-create")]
-use wasmer_types::{CompileModuleInfo, Target};
 use wasmer_types::{SerializableModule, SerializeError};
 use wasmer_vm::{FunctionBodyPtr, MemoryStyle, TableStyle, VMSharedSignatureIndex, VMTrampoline};
 use wasmer_vm::{InstanceAllocator, StoreObjects, TrapHandlerFn, VMExtern, VMInstance};
 
-/// A compiled wasm module, ready to be instantiated.
-pub struct Artifact {
-    artifact: ArtifactBuild,
+pub struct AllocatedArtifact {
     finished_functions: BoxedSlice<LocalFunctionIndex, FunctionBodyPtr>,
     finished_function_call_trampolines: BoxedSlice<SignatureIndex, VMTrampoline>,
     finished_dynamic_function_trampolines: BoxedSlice<FunctionIndex, FunctionBodyPtr>,
@@ -48,6 +46,14 @@ pub struct Artifact {
     finished_function_lengths: BoxedSlice<LocalFunctionIndex, usize>,
 }
 
+/// A compiled wasm module, ready to be instantiated.
+pub struct Artifact {
+    artifact: ArtifactBuild,
+    // The artifact will only be allocated in memory in case we can execute it
+    // (that means, if the target != host then this will be None).
+    allocated: Option<AllocatedArtifact>,
+}
+
 impl Artifact {
     /// Compile a data buffer into a `ArtifactBuild`, which may then be instantiated.
     #[cfg(feature = "compiler")]
@@ -79,7 +85,14 @@ impl Artifact {
             table_styles,
         )?;
 
-        Self::from_parts(&mut inner_engine, artifact)
+        Self::from_parts(&mut inner_engine, artifact, engine.target())
+    }
+
+    /// This indicates if the Artifact is allocated and can be run by the current
+    /// host. In case it can't be run (for example, if the artifact is cross compiled to
+    /// other architecture), it will return false.
+    pub fn allocated(&self) -> bool {
+        self.allocated.is_some()
     }
 
     /// Compile a data buffer into a `ArtifactBuild`, which may then be instantiated.
@@ -120,14 +133,22 @@ impl Artifact {
         let serializable = SerializableModule::deserialize(metadata_slice)?;
         let artifact = ArtifactBuild::from_serializable(serializable);
         let mut inner_engine = engine.inner_mut();
-        Self::from_parts(&mut inner_engine, artifact).map_err(DeserializeError::Compiler)
+        Self::from_parts(&mut inner_engine, artifact, engine.target())
+            .map_err(DeserializeError::Compiler)
     }
 
     /// Construct a `ArtifactBuild` from component parts.
     pub fn from_parts(
         engine_inner: &mut EngineInner,
         artifact: ArtifactBuild,
+        target: &Target,
     ) -> Result<Self, CompileError> {
+        if !target.is_native() {
+            return Ok(Self {
+                artifact,
+                allocated: None,
+            });
+        }
         let module_info = artifact.create_module_info();
         let (
             finished_functions,
@@ -198,12 +219,14 @@ impl Artifact {
 
         Ok(Self {
             artifact,
-            finished_functions,
-            finished_function_call_trampolines,
-            finished_dynamic_function_trampolines,
-            signatures,
-            frame_info_registration: Some(Mutex::new(None)),
-            finished_function_lengths,
+            allocated: Some(AllocatedArtifact {
+                finished_functions,
+                finished_function_call_trampolines,
+                finished_dynamic_function_trampolines,
+                signatures,
+                frame_info_registration: Some(Mutex::new(None)),
+                finished_function_lengths,
+            }),
         })
     }
 
@@ -248,18 +271,34 @@ impl Artifact {
     ///
     /// This is required to ensure that any traps can be properly symbolicated.
     pub fn register_frame_info(&self) {
-        if let Some(frame_info_registration) = self.frame_info_registration.as_ref() {
+        if let Some(frame_info_registration) = self
+            .allocated
+            .as_ref()
+            .expect("It must be allocated")
+            .frame_info_registration
+            .as_ref()
+        {
             let mut info = frame_info_registration.lock().unwrap();
 
             if info.is_some() {
                 return;
             }
 
             let finished_function_extents = self
+                .allocated
+                .as_ref()
+                .expect("It must be allocated")
                 .finished_functions
                 .values()
                 .copied()
-                .zip(self.finished_function_lengths.values().copied())
+                .zip(
+                    self.allocated
+                        .as_ref()
+                        .expect("It must be allocated")
+                        .finished_function_lengths
+                        .values()
+                        .copied(),
+                )
                 .map(|(ptr, length)| FunctionExtent { ptr, length })
                 .collect::<PrimaryMap<LocalFunctionIndex, _>>()
                 .into_boxed_slice();
@@ -276,26 +315,42 @@ impl Artifact {
     /// Returns the functions allocated in memory or this `Artifact`
     /// ready to be run.
     pub fn finished_functions(&self) -> &BoxedSlice<LocalFunctionIndex, FunctionBodyPtr> {
-        &self.finished_functions
+        &self
+            .allocated
+            .as_ref()
+            .expect("It must be allocated")
+            .finished_functions
     }
 
     /// Returns the function call trampolines allocated in memory of this
     /// `Artifact`, ready to be run.
     pub fn finished_function_call_trampolines(&self) -> &BoxedSlice<SignatureIndex, VMTrampoline> {
-        &self.finished_function_call_trampolines
+        &self
+            .allocated
+            .as_ref()
+            .expect("It must be allocated")
+            .finished_function_call_trampolines
     }
 
     /// Returns the dynamic function trampolines allocated in memory
     /// of this `Artifact`, ready to be run.
     pub fn finished_dynamic_function_trampolines(
         &self,
     ) -> &BoxedSlice<FunctionIndex, FunctionBodyPtr> {
-        &self.finished_dynamic_function_trampolines
+        &self
+            .allocated
+            .as_ref()
+            .expect("It must be allocated")
+            .finished_dynamic_function_trampolines
     }
 
     /// Returns the associated VM signatures for this `Artifact`.
     pub fn signatures(&self) -> &BoxedSlice<SignatureIndex, VMSharedSignatureIndex> {
-        &self.signatures
+        &self
+            .allocated
+            .as_ref()
+            .expect("It must be allocated")
+            .signatures
     }
 
     /// Do preinstantiation logic that is executed before instantiating
@@ -721,14 +776,16 @@ impl Artifact {
 
         Ok(Self {
             artifact,
-            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(),
-            finished_function_lengths,
-            frame_info_registration: None,
+            allocated: Some(AllocatedArtifact {
+                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(),
+                finished_function_lengths,
+                frame_info_registration: None,
+            }),
         })
     }
 }
diff --git a/lib/types/src/compilation/target.rs b/lib/types/src/compilation/target.rs
@@ -192,6 +192,13 @@ impl Target {
     pub fn cpu_features(&self) -> &EnumSet<CpuFeature> {
         &self.cpu_features
     }
+
+    /// Check if target is a native (eq to host) or not
+    pub fn is_native(&self) -> bool {
+        let host = Triple::host();
+        host.operating_system == self.triple.operating_system
+            && host.architecture == self.triple.architecture
+    }
 }
 
 /// The default for the Target will use the HOST as the triple

diff --git a/tests/compilers/traps.rs b/tests/compilers/traps.rs
@@ -273,6 +273,7 @@ fn trap_start_function_import(config: crate::Config) -> Result<()> {
     match err {
         InstantiationError::Link(_)
         | InstantiationError::DifferentStores
+        | InstantiationError::DifferentArchOS
         | InstantiationError::CpuFeature(_) => {
             panic!("It should be a start error")
         }