From 1387363a7b30e4a161ebe44e332226668892e33e Mon Sep 17 00:00:00 2001 From: ptitSeb Date: Mon, 6 Feb 2023 10:28:39 +0100 Subject: [PATCH] Fix/compile not in memory (#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 --- lib/api/src/sys/instance.rs | 5 ++ lib/api/src/sys/module.rs | 5 ++ lib/c-api/src/wasm_c_api/instance.rs | 6 ++ lib/compiler/src/engine/artifact.rs | 113 ++++++++++++++++++++------- lib/types/src/compilation/target.rs | 7 ++ tests/compilers/traps.rs | 1 + 6 files changed, 109 insertions(+), 28 deletions(-) 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 e6405272dbe..b284ded3655 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. + 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 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 81966689461..a9f95b8873d 100644 --- 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, 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")] @@ -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 { + 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,7 +271,13 @@ 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() { @@ -256,10 +285,20 @@ impl Artifact { } 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::>() .into_boxed_slice(); @@ -276,13 +315,21 @@ 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 + .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 { - &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 @@ -290,12 +337,20 @@ impl Artifact { pub fn finished_dynamic_function_trampolines( &self, ) -> &BoxedSlice { - &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 { - &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 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 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") }