From e884f859a938ff3a82d04c3bdf2d64c03cd064a0 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 3 Feb 2023 17:23:46 +0100 Subject: [PATCH 1/8] Example of allocated artifact --- lib/api/src/sys/module.rs | 5 +++++ lib/compiler/src/engine/artifact.rs | 19 ++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/api/src/sys/module.rs b/lib/api/src/sys/module.rs index e6405272dbe..40ace16e661 100644 --- 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 { + if !self.artifact.allocated() { + // Return an error mentioning that the artifact is compiled for a different + // platform. Probably need a new error + return InstantiationError::DifferentStores; + } // Ensure all imports come from the same context. for import in imports { if !import.is_from_store(store) { diff --git a/lib/compiler/src/engine/artifact.rs b/lib/compiler/src/engine/artifact.rs index 81966689461..a4d26e2d41d 100644 --- a/lib/compiler/src/engine/artifact.rs +++ b/lib/compiler/src/engine/artifact.rs @@ -36,9 +36,7 @@ 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, finished_function_call_trampolines: BoxedSlice, finished_dynamic_function_trampolines: BoxedSlice, @@ -48,6 +46,14 @@ pub struct Artifact { finished_function_lengths: BoxedSlice, } +/// 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, +} + impl Artifact { /// Compile a data buffer into a `ArtifactBuild`, which may then be instantiated. #[cfg(feature = "compiler")] @@ -82,6 +88,13 @@ impl Artifact { Self::from_parts(&mut inner_engine, artifact) } + /// 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. #[cfg(not(feature = "compiler"))] pub fn new(_engine: &Engine, _data: &[u8]) -> Result { From 1e68d0e01cf017f7bd95a73bd1c32bb67fecb816 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 3 Feb 2023 17:28:07 +0100 Subject: [PATCH 2/8] Improved to compile? --- lib/compiler/src/engine/artifact.rs | 48 +++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/lib/compiler/src/engine/artifact.rs b/lib/compiler/src/engine/artifact.rs index a4d26e2d41d..ddfee706b5e 100644 --- a/lib/compiler/src/engine/artifact.rs +++ b/lib/compiler/src/engine/artifact.rs @@ -211,12 +211,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, + }), }) } @@ -261,7 +263,12 @@ 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 + .expect("It must be allocated") + .frame_info_registration + .as_ref() + { let mut info = frame_info_registration.lock().unwrap(); if info.is_some() { @@ -269,10 +276,18 @@ impl Artifact { } let finished_function_extents = self + .allocated + .expect("It must be allocated") .finished_functions .values() .copied() - .zip(self.finished_function_lengths.values().copied()) + .zip( + self.allocated + .expect("It must be allocated") + .finished_function_lengths + .values() + .copied(), + ) .map(|(ptr, length)| FunctionExtent { ptr, length }) .collect::>() .into_boxed_slice(); @@ -289,13 +304,19 @@ impl Artifact { /// Returns the functions allocated in memory or this `Artifact` /// ready to be run. pub fn finished_functions(&self) -> &BoxedSlice { - &self.finished_functions + &self + .allocated + .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 { - &self.finished_function_call_trampolines + &self + .allocated + .expect("It must be allocated") + .finished_function_call_trampolines } /// Returns the dynamic function trampolines allocated in memory @@ -303,12 +324,15 @@ impl Artifact { pub fn finished_dynamic_function_trampolines( &self, ) -> &BoxedSlice { - &self.finished_dynamic_function_trampolines + &self + .allocated + .expect("It must be allocated") + .finished_dynamic_function_trampolines } /// Returns the associated VM signatures for this `Artifact`. pub fn signatures(&self) -> &BoxedSlice { - &self.signatures + &self.allocated.expect("It must be allocated").signatures } /// Do preinstantiation logic that is executed before instantiating From 3081c2e3d40365122c552042175445776f709627 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Fri, 3 Feb 2023 17:30:39 +0100 Subject: [PATCH 3/8] Improve a bit more? --- lib/compiler/src/engine/artifact.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/compiler/src/engine/artifact.rs b/lib/compiler/src/engine/artifact.rs index ddfee706b5e..517ffe88632 100644 --- a/lib/compiler/src/engine/artifact.rs +++ b/lib/compiler/src/engine/artifact.rs @@ -758,14 +758,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(ArtifactBuild { + 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, + }), }) } } From ac67702fa0ab0ca3d3c583c80c566d8cf4a6c3ca Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Fri, 3 Feb 2023 17:55:32 +0100 Subject: [PATCH 4/8] Fixed build --- lib/api/src/sys/module.rs | 2 +- lib/compiler/src/engine/artifact.rs | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/api/src/sys/module.rs b/lib/api/src/sys/module.rs index 40ace16e661..47e0cee2a01 100644 --- a/lib/api/src/sys/module.rs +++ b/lib/api/src/sys/module.rs @@ -320,7 +320,7 @@ impl Module { if !self.artifact.allocated() { // Return an error mentioning that the artifact is compiled for a different // platform. Probably need a new error - return InstantiationError::DifferentStores; + return Err(InstantiationError::DifferentStores); } // Ensure all imports come from the same context. for import in imports { diff --git a/lib/compiler/src/engine/artifact.rs b/lib/compiler/src/engine/artifact.rs index 517ffe88632..eb4d4afe09d 100644 --- a/lib/compiler/src/engine/artifact.rs +++ b/lib/compiler/src/engine/artifact.rs @@ -18,6 +18,7 @@ use enumset::EnumSet; use std::mem; use std::sync::Arc; use std::sync::Mutex; +use wasmer_object::object::Architecture; #[cfg(feature = "static-artifact-create")] use wasmer_object::{emit_compilation, emit_data, get_object_for_target, Object}; #[cfg(any(feature = "static-artifact-create", feature = "static-artifact-load"))] @@ -265,6 +266,7 @@ impl Artifact { pub fn register_frame_info(&self) { if let Some(frame_info_registration) = self .allocated + .as_ref() .expect("It must be allocated") .frame_info_registration .as_ref() @@ -277,12 +279,14 @@ impl Artifact { let finished_function_extents = self .allocated + .as_ref() .expect("It must be allocated") .finished_functions .values() .copied() .zip( self.allocated + .as_ref() .expect("It must be allocated") .finished_function_lengths .values() @@ -306,6 +310,7 @@ impl Artifact { pub fn finished_functions(&self) -> &BoxedSlice { &self .allocated + .as_ref() .expect("It must be allocated") .finished_functions } @@ -315,6 +320,7 @@ impl Artifact { pub fn finished_function_call_trampolines(&self) -> &BoxedSlice { &self .allocated + .as_ref() .expect("It must be allocated") .finished_function_call_trampolines } @@ -326,13 +332,18 @@ impl Artifact { ) -> &BoxedSlice { &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 { - &self.allocated.expect("It must be allocated").signatures + &self + .allocated + .as_ref() + .expect("It must be allocated") + .signatures } /// Do preinstantiation logic that is executed before instantiating @@ -758,7 +769,7 @@ impl Artifact { Ok(Self { artifact, - allocated: Some(ArtifactBuild { + allocated: Some(AllocatedArtifact { finished_functions: finished_functions.into_boxed_slice(), finished_function_call_trampolines: finished_function_call_trampolines .into_boxed_slice(), From 38b4b7d59b19c3d16aec9a2a9fff87f179be1c95 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Fri, 3 Feb 2023 18:00:57 +0100 Subject: [PATCH 5/8] Better error when Instancing fail because of OS/Arch issue --- lib/api/src/sys/instance.rs | 5 +++++ lib/api/src/sys/module.rs | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/api/src/sys/instance.rs b/lib/api/src/sys/instance.rs index a1572c681af..c139a24ce46 100644 --- 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 for InstantiationError { diff --git a/lib/api/src/sys/module.rs b/lib/api/src/sys/module.rs index 47e0cee2a01..714518a9c63 100644 --- a/lib/api/src/sys/module.rs +++ b/lib/api/src/sys/module.rs @@ -320,7 +320,7 @@ impl Module { if !self.artifact.allocated() { // Return an error mentioning that the artifact is compiled for a different // platform. Probably need a new error - return Err(InstantiationError::DifferentStores); + return Err(InstantiationError::DifferentArchOS); } // Ensure all imports come from the same context. for import in imports { From 3ce439511359c8dcb462220a546718a40eda55e3 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Fri, 3 Feb 2023 18:24:36 +0100 Subject: [PATCH 6/8] Add missing brnach for new error --- tests/compilers/traps.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/compilers/traps.rs b/tests/compilers/traps.rs index 4c7bb0c90d6..9b6ea9d070e 100644 --- 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") } From f4bc65f798b084dd666da72aa3e6330a1bdbabe2 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Fri, 3 Feb 2023 18:36:00 +0100 Subject: [PATCH 7/8] Fixed comment --- lib/api/src/sys/module.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/src/sys/module.rs b/lib/api/src/sys/module.rs index 714518a9c63..b284ded3655 100644 --- a/lib/api/src/sys/module.rs +++ b/lib/api/src/sys/module.rs @@ -319,7 +319,7 @@ impl Module { ) -> Result { if !self.artifact.allocated() { // Return an error mentioning that the artifact is compiled for a different - // platform. Probably need a new error + // platform. return Err(InstantiationError::DifferentArchOS); } // Ensure all imports come from the same context. From dee08f7708da5fc32e9f22d23f6c11cf40998235 Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Fri, 3 Feb 2023 18:46:59 +0100 Subject: [PATCH 8/8] Fix linter --- lib/c-api/src/wasm_c_api/instance.rs | 6 ++++++ lib/compiler/src/engine/artifact.rs | 19 +++++++++++++------ lib/types/src/compilation/target.rs | 7 +++++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/lib/c-api/src/wasm_c_api/instance.rs b/lib/c-api/src/wasm_c_api/instance.rs index a0261c26a62..ab07dff6570 100644 --- 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 index eb4d4afe09d..a9f95b8873d 100644 --- a/lib/compiler/src/engine/artifact.rs +++ b/lib/compiler/src/engine/artifact.rs @@ -18,21 +18,20 @@ use enumset::EnumSet; use std::mem; use std::sync::Arc; use std::sync::Mutex; -use wasmer_object::object::Architecture; #[cfg(feature = "static-artifact-create")] 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}; @@ -86,7 +85,7 @@ 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 @@ -134,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 { + if !target.is_native() { + return Ok(Self { + artifact, + allocated: None, + }); + } let module_info = artifact.create_module_info(); let ( finished_functions, diff --git a/lib/types/src/compilation/target.rs b/lib/types/src/compilation/target.rs index 9ba6d7dd85f..fd1dbc97a86 100644 --- 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 { &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