From e9d3bf8851330d228bb0355541a8f7490e5567d0 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 24 Jan 2020 14:28:04 +0100 Subject: [PATCH 1/6] Quick and dirty impl. --- client/executor/src/integration_tests/mod.rs | 22 ++- client/executor/src/lib.rs | 8 +- client/executor/src/wasm_runtime.rs | 6 +- client/executor/wasmi/src/lib.rs | 34 ++-- client/executor/wasmtime/src/runtime.rs | 160 ++++++++++++++++++- 5 files changed, 198 insertions(+), 32 deletions(-) diff --git a/client/executor/src/integration_tests/mod.rs b/client/executor/src/integration_tests/mod.rs index 00f4eb33b02bf..78031061574ac 100644 --- a/client/executor/src/integration_tests/mod.rs +++ b/client/executor/src/integration_tests/mod.rs @@ -70,7 +70,7 @@ mod wasmtime_missing_externals { #[cfg(feature = "wasmtime")] type HostFunctions = - (wasmtime_missing_externals::WasmtimeHostFunctions, sp_io::SubstrateHostFunctions); + (sp_io::SubstrateHostFunctions); #[cfg(not(feature = "wasmtime"))] type HostFunctions = sp_io::SubstrateHostFunctions; @@ -114,8 +114,14 @@ fn returning_should_work(wasm_method: WasmExecutionMethod) { #[test_case(WasmExecutionMethod::Interpreted)] #[cfg_attr(feature = "wasmtime", test_case(WasmExecutionMethod::Compiled))] -#[should_panic(expected = "Function `missing_external` is only a stub. Calling a stub is not allowed.")] -#[cfg(not(feature = "wasmtime"))] +#[cfg_attr( + feature = "wasmtime", + should_panic(expected = "call to undefined external function with index 67") +)] +#[cfg_attr( + not(feature = "wasmtime"), + should_panic(expected = "Function `missing_external` is only a stub. Calling a stub is not allowed.") +)] fn call_not_existing_function(wasm_method: WasmExecutionMethod) { let mut ext = TestExternalities::default(); let mut ext = ext.ext(); @@ -133,8 +139,14 @@ fn call_not_existing_function(wasm_method: WasmExecutionMethod) { #[test_case(WasmExecutionMethod::Interpreted)] #[cfg_attr(feature = "wasmtime", test_case(WasmExecutionMethod::Compiled))] -#[should_panic(expected = "Function `yet_another_missing_external` is only a stub. Calling a stub is not allowed.")] -#[cfg(not(feature = "wasmtime"))] +#[cfg_attr( + feature = "wasmtime", + should_panic(expected = "call to undefined external function with index 68") +)] +#[cfg_attr( + not(feature = "wasmtime"), + should_panic(expected = "Function `yet_another_missing_external` is only a stub. Calling a stub is not allowed.") +)] fn call_yet_another_not_existing_function(wasm_method: WasmExecutionMethod) { let mut ext = TestExternalities::default(); let mut ext = ext.ext(); diff --git a/client/executor/src/lib.rs b/client/executor/src/lib.rs index a054f4dc76e54..152e3a498485d 100644 --- a/client/executor/src/lib.rs +++ b/client/executor/src/lib.rs @@ -68,7 +68,7 @@ pub fn call_in_wasm( ext: &mut dyn Externalities, code: &[u8], heap_pages: u64, - allow_missing_imports: bool, + allow_missing_func_imports: bool, ) -> error::Result> { call_in_wasm_with_host_functions( function, @@ -78,7 +78,7 @@ pub fn call_in_wasm( code, heap_pages, HF::host_functions(), - allow_missing_imports, + allow_missing_func_imports, ) } @@ -92,14 +92,14 @@ pub fn call_in_wasm_with_host_functions( code: &[u8], heap_pages: u64, host_functions: Vec<&'static dyn sp_wasm_interface::Function>, - allow_missing_imports: bool, + allow_missing_func_imports: bool, ) -> error::Result> { let instance = wasm_runtime::create_wasm_runtime_with_code( execution_method, heap_pages, code, host_functions, - allow_missing_imports, + allow_missing_func_imports, )?; // It is safe, as we delete the instance afterwards. diff --git a/client/executor/src/wasm_runtime.rs b/client/executor/src/wasm_runtime.rs index 036b28f764003..b8966c3af2739 100644 --- a/client/executor/src/wasm_runtime.rs +++ b/client/executor/src/wasm_runtime.rs @@ -191,15 +191,15 @@ pub fn create_wasm_runtime_with_code( heap_pages: u64, code: &[u8], host_functions: Vec<&'static dyn Function>, - allow_missing_imports: bool, + allow_missing_func_imports: bool, ) -> Result, WasmError> { match wasm_method { WasmExecutionMethod::Interpreted => - sc_executor_wasmi::create_instance(code, heap_pages, host_functions, allow_missing_imports) + sc_executor_wasmi::create_instance(code, heap_pages, host_functions, allow_missing_func_imports) .map(|runtime| -> Box { Box::new(runtime) }), #[cfg(feature = "wasmtime")] WasmExecutionMethod::Compiled => - sc_executor_wasmtime::create_instance(code, heap_pages, host_functions) + sc_executor_wasmtime::create_instance(code, heap_pages, host_functions, allow_missing_func_imports) .map(|runtime| -> Box { Box::new(runtime) }), } } diff --git a/client/executor/wasmi/src/lib.rs b/client/executor/wasmi/src/lib.rs index d3d0ee6e1e3ef..cf8125e774529 100644 --- a/client/executor/wasmi/src/lib.rs +++ b/client/executor/wasmi/src/lib.rs @@ -38,7 +38,7 @@ struct FunctionExecutor<'a> { memory: MemoryRef, table: Option, host_functions: &'a [&'static dyn Function], - allow_missing_imports: bool, + allow_missing_func_imports: bool, missing_functions: &'a [String], } @@ -48,7 +48,7 @@ impl<'a> FunctionExecutor<'a> { heap_base: u32, t: Option, host_functions: &'a [&'static dyn Function], - allow_missing_imports: bool, + allow_missing_func_imports: bool, missing_functions: &'a [String], ) -> Result { Ok(FunctionExecutor { @@ -57,7 +57,7 @@ impl<'a> FunctionExecutor<'a> { memory: m, table: t, host_functions, - allow_missing_imports, + allow_missing_func_imports, missing_functions, }) } @@ -273,15 +273,15 @@ impl<'a> Sandbox for FunctionExecutor<'a> { struct Resolver<'a> { host_functions: &'a[&'static dyn Function], - allow_missing_imports: bool, + allow_missing_func_imports: bool, missing_functions: RefCell>, } impl<'a> Resolver<'a> { - fn new(host_functions: &'a[&'static dyn Function], allow_missing_imports: bool) -> Resolver<'a> { + fn new(host_functions: &'a[&'static dyn Function], allow_missing_func_imports: bool) -> Resolver<'a> { Resolver { host_functions, - allow_missing_imports, + allow_missing_func_imports, missing_functions: RefCell::new(Vec::new()), } } @@ -311,7 +311,7 @@ impl<'a> wasmi::ModuleImportResolver for Resolver<'a> { } } - if self.allow_missing_imports { + if self.allow_missing_func_imports { trace!(target: "wasm-executor", "Could not find function `{}`, a stub will be provided instead.", name); let id = self.missing_functions.borrow().len() + self.host_functions.len(); self.missing_functions.borrow_mut().push(name.to_string()); @@ -336,7 +336,7 @@ impl<'a> wasmi::Externals for FunctionExecutor<'a> { .map_err(|msg| Error::FunctionExecution(function.name().to_string(), msg)) .map_err(wasmi::Trap::from) .map(|v| v.map(Into::into)) - } else if self.allow_missing_imports + } else if self.allow_missing_func_imports && index >= self.host_functions.len() && index < self.host_functions.len() + self.missing_functions.len() { @@ -381,7 +381,7 @@ fn call_in_wasm_module( method: &str, data: &[u8], host_functions: &[&'static dyn Function], - allow_missing_imports: bool, + allow_missing_func_imports: bool, missing_functions: &Vec, ) -> Result, Error> { // extract a reference to a linear memory, optional reference to a table @@ -397,7 +397,7 @@ fn call_in_wasm_module( heap_base, table, host_functions, - allow_missing_imports, + allow_missing_func_imports, missing_functions, )?; @@ -433,9 +433,9 @@ fn instantiate_module( heap_pages: usize, module: &Module, host_functions: &[&'static dyn Function], - allow_missing_imports: bool, + allow_missing_func_imports: bool, ) -> Result<(ModuleRef, Vec), Error> { - let resolver = Resolver::new(host_functions, allow_missing_imports); + let resolver = Resolver::new(host_functions, allow_missing_func_imports); // start module instantiation. Don't run 'start' function yet. let intermediate_instance = ModuleInstance::new( module, @@ -575,7 +575,7 @@ pub struct WasmiRuntime { host_functions: Vec<&'static dyn Function>, /// Enable stub generation for functions that are not available in `host_functions`. /// These stubs will error when the wasm blob tries to call them. - allow_missing_imports: bool, + allow_missing_func_imports: bool, /// List of missing functions detected during function resolution missing_functions: Vec, } @@ -607,7 +607,7 @@ impl WasmRuntime for WasmiRuntime { method, data, &self.host_functions, - self.allow_missing_imports, + self.allow_missing_func_imports, &self.missing_functions, ) } @@ -617,7 +617,7 @@ pub fn create_instance( code: &[u8], heap_pages: u64, host_functions: Vec<&'static dyn Function>, - allow_missing_imports: bool, + allow_missing_func_imports: bool, ) -> Result { let module = Module::from_buffer(&code).map_err(|_| WasmError::InvalidModule)?; @@ -632,7 +632,7 @@ pub fn create_instance( heap_pages as usize, &module, &host_functions, - allow_missing_imports, + allow_missing_func_imports, ).map_err(|e| WasmError::Instantiation(e.to_string()))?; // Take state snapshot before executing anything. @@ -648,7 +648,7 @@ pub fn create_instance( instance, state_snapshot, host_functions, - allow_missing_imports, + allow_missing_func_imports, missing_functions, }) } diff --git a/client/executor/wasmtime/src/runtime.rs b/client/executor/wasmtime/src/runtime.rs index f9b1b6209c3b7..5d93462a3a17a 100644 --- a/client/executor/wasmtime/src/runtime.rs +++ b/client/executor/wasmtime/src/runtime.rs @@ -86,8 +86,9 @@ pub fn create_instance( code: &[u8], heap_pages: u64, host_functions: Vec<&'static dyn Function>, + allow_missing_func_imports: bool, ) -> std::result::Result { - let (compiled_module, context) = create_compiled_unit(code, &host_functions)?; + let (compiled_module, context) = create_compiled_unit(code, &host_functions, allow_missing_func_imports)?; // Inspect the module for the min and max memory sizes. let (min_memory_size, max_memory_size) = { @@ -115,9 +116,122 @@ pub fn create_instance( }) } +#[derive(Debug)] +struct MissingFunction { + name: String, + sig: cranelift_codegen::ir::Signature, +} + +#[derive(Debug)] +struct MissingFunctionStubs { + stubs: HashMap>, +} + +impl MissingFunctionStubs { + fn new() -> Self { + Self { + stubs: HashMap::new(), + } + } + + fn insert(&mut self, module: String, name: String, sig: cranelift_codegen::ir::Signature) { + self.stubs.entry(module).or_insert_with(Vec::new).push(MissingFunction { + name, + sig, + }); + } + + fn stubs(&self, module: &str) -> &[MissingFunction] { + self.stubs.get(module).map(|stubs| &*stubs as &[_]).unwrap_or(&[]) + } +} + +fn scan_missing_functions( + code: &[u8], + host_functions: &[&'static dyn Function], +) -> std::result::Result { + let isa = target_isa()?; + let call_conv = isa.default_call_conv(); + + // TODO: + let module = parity_wasm::elements::Module::from_bytes(code) + .map_err(|e| WasmError::Other(format!("cannot deserialize error: {}", e)))?; + + let types = module.type_section().map(|ts| ts.types()).unwrap_or(&[]); + let import_entries = module + .import_section() + .map(|is| is.entries()) + .unwrap_or(&[]); + + let mut missing_functions = MissingFunctionStubs::new(); + for import_entry in import_entries { + let func_ty = match import_entry.external() { + parity_wasm::elements::External::Function(func_ty_idx) => { + let parity_wasm::elements::Type::Function(ref func_ty) = + types.get(*func_ty_idx as usize).ok_or_else(|| { + WasmError::Other(format!("corrupted module, type out of bounds")) + })?; + func_ty + } + _ => { + // The executor doesn't provide any non function imports! + return Err(WasmError::Other(format!( + "{}:{} is not a function", + import_entry.module(), + import_entry.field() + ))); + } + }; + + if import_entry.module() == "env" { + if host_functions + .iter() + .find(|hf| hf.name() == import_entry.field()) + .is_some() + { + // TODO: Check the signature. + continue; + } + } + + fn cranelift_ir_type(val_ty: &parity_wasm::elements::ValueType) -> ir::types::Type { + use parity_wasm::elements::ValueType::*; + match *val_ty { + I32 => ir::types::I32, + I64 => ir::types::I64, + F32 => ir::types::F32, + F64 => ir::types::F64, + } + } + + // This import is not a function, not from env module, or doesn't have a corresponding host + // function, or the signature is not matching. + let sig = ir::Signature { + params: func_ty.params().iter() + .map(cranelift_ir_type) + .map(ir::AbiParam::new) + .collect(), + returns: func_ty.return_type().iter() + .map(cranelift_ir_type) + .map(ir::AbiParam::new) + .collect(), + call_conv: call_conv.clone(), + }; + + missing_functions.insert( + import_entry.module().to_string(), + import_entry.field().to_string(), + sig + ); + } + + Ok(missing_functions) +} + fn create_compiled_unit( code: &[u8], host_functions: &[&'static dyn Function], + allow_missing_func_imports: bool, ) -> std::result::Result<(CompiledModule, Context), WasmError> { let compilation_strategy = CompilationStrategy::Cranelift; @@ -130,8 +244,27 @@ fn create_compiled_unit( // Instantiate and link the env module. let global_exports = context.get_global_exports(); let compiler = new_compiler(compilation_strategy)?; - let env_module = instantiate_env_module(global_exports, compiler, host_functions)?; - context.name_instance("env".to_owned(), env_module); + + let mut missing_functions_stubs = if allow_missing_func_imports { + dbg!(scan_missing_functions(code, host_functions)?) + } else { + // If there are in fact missing functions they will be detected at the instantiation time + // and the module will be rejected. + MissingFunctionStubs::new() + }; + + let env_missing_functions = missing_functions_stubs.stubs.remove("env").unwrap_or_else(|| Vec::new()); + context.name_instance( + "env".to_owned(), + instantiate_env_module(global_exports, compiler, host_functions, &env_missing_functions)? + ); + + for (module, missing_functions_stubs) in missing_functions_stubs.stubs { + let compiler = new_compiler(compilation_strategy)?; + let global_exports = context.get_global_exports(); + let instance = instantiate_env_module(global_exports, compiler, &[], &env_missing_functions)?; + context.name_instance(module, instance); + } // Compile the wasm module. let module = context.compile_module(&code) @@ -199,6 +332,7 @@ fn instantiate_env_module( global_exports: Rc>>>, compiler: Compiler, host_functions: &[&'static dyn Function], + missing_functions_stubs: &[MissingFunction], ) -> std::result::Result { let isa = target_isa()?; @@ -231,6 +365,26 @@ fn instantiate_env_module( finished_functions.push(trampoline); } + for MissingFunction { name, sig } in missing_functions_stubs { + let sig = translate_signature( + sig.clone(), // TODO: Unnecessary clone + pointer_type + ); + let sig_id = module.signatures.push(sig.clone()); + let func_id = module.functions.push(sig_id); + module + .exports + .insert(name.to_string(), wasmtime_environ::Export::Function(func_id)); + let trampoline = make_trampoline( + isa.as_ref(), + &mut code_memory, + &mut fn_builder_ctx, + func_id.index() as u32, + &sig, + )?; + finished_functions.push(trampoline); + } + code_memory.publish(); let imports = Imports::none(); From 1bf80e7579e6807d9de1f4a4e9dce41a2bd5bcda Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 24 Jan 2020 14:57:08 +0100 Subject: [PATCH 2/6] Clean up --- client/executor/src/integration_tests/mod.rs | 42 -------------------- client/executor/wasmtime/src/runtime.rs | 21 ++++------ 2 files changed, 7 insertions(+), 56 deletions(-) diff --git a/client/executor/src/integration_tests/mod.rs b/client/executor/src/integration_tests/mod.rs index 78031061574ac..b0fb54e789bd9 100644 --- a/client/executor/src/integration_tests/mod.rs +++ b/client/executor/src/integration_tests/mod.rs @@ -31,48 +31,6 @@ use sp_trie::{TrieConfiguration, trie_types::Layout}; use crate::WasmExecutionMethod; pub type TestExternalities = CoreTestExternalities; - -#[cfg(feature = "wasmtime")] -mod wasmtime_missing_externals { - use sp_wasm_interface::{Function, FunctionContext, HostFunctions, Result, Signature, Value}; - - pub struct WasmtimeHostFunctions; - - impl HostFunctions for WasmtimeHostFunctions { - fn host_functions() -> Vec<&'static dyn Function> { - vec![MISSING_EXTERNAL_FUNCTION, YET_ANOTHER_MISSING_EXTERNAL_FUNCTION] - } - } - - struct MissingExternalFunction(&'static str); - - impl Function for MissingExternalFunction { - fn name(&self) -> &str { self.0 } - - fn signature(&self) -> Signature { - Signature::new(vec![], None) - } - - fn execute( - &self, - _context: &mut dyn FunctionContext, - _args: &mut dyn Iterator, - ) -> Result> { - panic!("should not be called"); - } - } - - static MISSING_EXTERNAL_FUNCTION: &'static MissingExternalFunction = - &MissingExternalFunction("missing_external"); - static YET_ANOTHER_MISSING_EXTERNAL_FUNCTION: &'static MissingExternalFunction = - &MissingExternalFunction("yet_another_missing_external"); -} - -#[cfg(feature = "wasmtime")] -type HostFunctions = - (sp_io::SubstrateHostFunctions); - -#[cfg(not(feature = "wasmtime"))] type HostFunctions = sp_io::SubstrateHostFunctions; fn call_in_wasm( diff --git a/client/executor/wasmtime/src/runtime.rs b/client/executor/wasmtime/src/runtime.rs index 5d93462a3a17a..89b60f33311ab 100644 --- a/client/executor/wasmtime/src/runtime.rs +++ b/client/executor/wasmtime/src/runtime.rs @@ -140,10 +140,6 @@ impl MissingFunctionStubs { sig, }); } - - fn stubs(&self, module: &str) -> &[MissingFunction] { - self.stubs.get(module).map(|stubs| &*stubs as &[_]).unwrap_or(&[]) - } } fn scan_missing_functions( @@ -153,7 +149,6 @@ fn scan_missing_functions( let isa = target_isa()?; let call_conv = isa.default_call_conv(); - // TODO: let module = parity_wasm::elements::Module::from_bytes(code) .map_err(|e| WasmError::Other(format!("cannot deserialize error: {}", e)))?; @@ -184,12 +179,10 @@ fn scan_missing_functions( }; if import_entry.module() == "env" { - if host_functions + if let Some(hf) = host_functions .iter() .find(|hf| hf.name() == import_entry.field()) - .is_some() { - // TODO: Check the signature. continue; } } @@ -246,7 +239,7 @@ fn create_compiled_unit( let compiler = new_compiler(compilation_strategy)?; let mut missing_functions_stubs = if allow_missing_func_imports { - dbg!(scan_missing_functions(code, host_functions)?) + scan_missing_functions(code, host_functions)? } else { // If there are in fact missing functions they will be detected at the instantiation time // and the module will be rejected. @@ -256,13 +249,13 @@ fn create_compiled_unit( let env_missing_functions = missing_functions_stubs.stubs.remove("env").unwrap_or_else(|| Vec::new()); context.name_instance( "env".to_owned(), - instantiate_env_module(global_exports, compiler, host_functions, &env_missing_functions)? + instantiate_env_module(global_exports, compiler, host_functions, env_missing_functions)? ); for (module, missing_functions_stubs) in missing_functions_stubs.stubs { let compiler = new_compiler(compilation_strategy)?; let global_exports = context.get_global_exports(); - let instance = instantiate_env_module(global_exports, compiler, &[], &env_missing_functions)?; + let instance = instantiate_env_module(global_exports, compiler, &[], missing_functions_stubs)?; context.name_instance(module, instance); } @@ -332,7 +325,7 @@ fn instantiate_env_module( global_exports: Rc>>>, compiler: Compiler, host_functions: &[&'static dyn Function], - missing_functions_stubs: &[MissingFunction], + missing_functions_stubs: Vec, ) -> std::result::Result { let isa = target_isa()?; @@ -367,14 +360,14 @@ fn instantiate_env_module( for MissingFunction { name, sig } in missing_functions_stubs { let sig = translate_signature( - sig.clone(), // TODO: Unnecessary clone + sig, pointer_type ); let sig_id = module.signatures.push(sig.clone()); let func_id = module.functions.push(sig_id); module .exports - .insert(name.to_string(), wasmtime_environ::Export::Function(func_id)); + .insert(name, wasmtime_environ::Export::Function(func_id)); let trampoline = make_trampoline( isa.as_ref(), &mut code_memory, From 086f94b054e68ab8bb64e6b00bf58d772f0c6e2d Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 24 Jan 2020 16:01:51 +0100 Subject: [PATCH 3/6] Check the signatures. --- client/executor/wasmtime/src/runtime.rs | 34 ++++++++----------------- client/executor/wasmtime/src/util.rs | 18 +++++++++++++ 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/client/executor/wasmtime/src/runtime.rs b/client/executor/wasmtime/src/runtime.rs index 89b60f33311ab..5da6db5228e38 100644 --- a/client/executor/wasmtime/src/runtime.rs +++ b/client/executor/wasmtime/src/runtime.rs @@ -18,7 +18,12 @@ use crate::function_executor::FunctionExecutorState; use crate::trampoline::{EnvState, make_trampoline}; -use crate::util::{cranelift_ir_signature, read_memory_into, write_memory_from}; +use crate::util::{ + cranelift_ir_signature, + convert_parity_wasm_signature, + read_memory_into, + write_memory_from +}; use sc_executor_common::{ error::{Error, Result, WasmError}, @@ -177,39 +182,22 @@ fn scan_missing_functions( ))); } }; + let signature = convert_parity_wasm_signature(func_ty); if import_entry.module() == "env" { if let Some(hf) = host_functions .iter() .find(|hf| hf.name() == import_entry.field()) { - continue; - } - } - - fn cranelift_ir_type(val_ty: &parity_wasm::elements::ValueType) -> ir::types::Type { - use parity_wasm::elements::ValueType::*; - match *val_ty { - I32 => ir::types::I32, - I64 => ir::types::I64, - F32 => ir::types::F32, - F64 => ir::types::F64, + if signature == hf.signature() { + continue; + } } } // This import is not a function, not from env module, or doesn't have a corresponding host // function, or the signature is not matching. - let sig = ir::Signature { - params: func_ty.params().iter() - .map(cranelift_ir_type) - .map(ir::AbiParam::new) - .collect(), - returns: func_ty.return_type().iter() - .map(cranelift_ir_type) - .map(ir::AbiParam::new) - .collect(), - call_conv: call_conv.clone(), - }; + let sig = cranelift_ir_signature(signature, &call_conv); missing_functions.insert( import_entry.module().to_string(), diff --git a/client/executor/wasmtime/src/util.rs b/client/executor/wasmtime/src/util.rs index 551295911a9cf..77fb8af3867a4 100644 --- a/client/executor/wasmtime/src/util.rs +++ b/client/executor/wasmtime/src/util.rs @@ -51,6 +51,24 @@ pub fn checked_range(offset: usize, len: usize, max: usize) -> Option Signature { + fn convert_value_type(val_ty: parity_wasm::elements::ValueType) -> ValueType { + use parity_wasm::elements::ValueType::*; + match val_ty { + I32 => ValueType::I32, + I64 => ValueType::I64, + F32 => ValueType::F32, + F64 => ValueType::F64, + } + } + + Signature::new( + func_ty.params().iter().cloned().map(convert_value_type).collect::>(), + func_ty.return_type().map(convert_value_type), + ) +} + /// Convert a wasm_interface Signature into a cranelift_codegen Signature. pub fn cranelift_ir_signature(signature: Signature, call_conv: &isa::CallConv) -> ir::Signature { ir::Signature { From 1c05ed80f81b9b2089be34e2f4186a031b35b13b Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 24 Jan 2020 16:43:33 +0100 Subject: [PATCH 4/6] Fix tests. --- client/executor/src/integration_tests/mod.rs | 60 ++++++++++++-------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/client/executor/src/integration_tests/mod.rs b/client/executor/src/integration_tests/mod.rs index b0fb54e789bd9..5b2fc5ca0c972 100644 --- a/client/executor/src/integration_tests/mod.rs +++ b/client/executor/src/integration_tests/mod.rs @@ -72,52 +72,66 @@ fn returning_should_work(wasm_method: WasmExecutionMethod) { #[test_case(WasmExecutionMethod::Interpreted)] #[cfg_attr(feature = "wasmtime", test_case(WasmExecutionMethod::Compiled))] -#[cfg_attr( - feature = "wasmtime", - should_panic(expected = "call to undefined external function with index 67") -)] -#[cfg_attr( - not(feature = "wasmtime"), - should_panic(expected = "Function `missing_external` is only a stub. Calling a stub is not allowed.") -)] fn call_not_existing_function(wasm_method: WasmExecutionMethod) { - let mut ext = TestExternalities::default(); - let mut ext = ext.ext(); - let test_code = WASM_BINARY; + let mut ext = TestExternalities::default(); + let mut ext = ext.ext(); + let test_code = WASM_BINARY; - call_in_wasm( + match call_in_wasm( "test_calling_missing_external", &[], wasm_method, &mut ext, &test_code[..], 8, - ).unwrap(); + ) { + Ok(_) => panic!("was expected an `Err`"), + Err(e) => { + match wasm_method { + WasmExecutionMethod::Interpreted => assert_eq!( + &format!("{:?}", e), + "Wasmi(Trap(Trap { kind: Host(Other(\"Function `missing_external` is only a stub. Calling a stub is not allowed.\")) }))" + ), + #[cfg(feature = "wasmtime")] + WasmExecutionMethod::Compiled => assert_eq!( + &format!("{:?}", e), + "Other(\"call to undefined external function with index 67\")" + ), + } + } + } } #[test_case(WasmExecutionMethod::Interpreted)] #[cfg_attr(feature = "wasmtime", test_case(WasmExecutionMethod::Compiled))] -#[cfg_attr( - feature = "wasmtime", - should_panic(expected = "call to undefined external function with index 68") -)] -#[cfg_attr( - not(feature = "wasmtime"), - should_panic(expected = "Function `yet_another_missing_external` is only a stub. Calling a stub is not allowed.") -)] fn call_yet_another_not_existing_function(wasm_method: WasmExecutionMethod) { let mut ext = TestExternalities::default(); let mut ext = ext.ext(); let test_code = WASM_BINARY; - call_in_wasm( + match call_in_wasm( "test_calling_yet_another_missing_external", &[], wasm_method, &mut ext, &test_code[..], 8, - ).unwrap(); + ) { + Ok(_) => panic!("was expected an `Err`"), + Err(e) => { + match wasm_method { + WasmExecutionMethod::Interpreted => assert_eq!( + &format!("{:?}", e), + "Wasmi(Trap(Trap { kind: Host(Other(\"Function `yet_another_missing_external` is only a stub. Calling a stub is not allowed.\")) }))" + ), + #[cfg(feature = "wasmtime")] + WasmExecutionMethod::Compiled => assert_eq!( + &format!("{:?}", e), + "Other(\"call to undefined external function with index 68\")" + ), + } + } + } } #[test_case(WasmExecutionMethod::Interpreted)] From 0995a73e1e3aa6e787367349db3cc35ca5453b01 Mon Sep 17 00:00:00 2001 From: Sergei Pepyakin Date: Sat, 25 Jan 2020 16:40:59 +0100 Subject: [PATCH 5/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Bastian Köcher --- client/executor/wasmtime/src/runtime.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/executor/wasmtime/src/runtime.rs b/client/executor/wasmtime/src/runtime.rs index 5da6db5228e38..004f95f9e0a9d 100644 --- a/client/executor/wasmtime/src/runtime.rs +++ b/client/executor/wasmtime/src/runtime.rs @@ -195,14 +195,14 @@ fn scan_missing_functions( } } - // This import is not a function, not from env module, or doesn't have a corresponding host + // This import is a function, not from env module, or doesn't have a corresponding host // function, or the signature is not matching. let sig = cranelift_ir_signature(signature, &call_conv); missing_functions.insert( import_entry.module().to_string(), import_entry.field().to_string(), - sig + sig, ); } @@ -237,7 +237,7 @@ fn create_compiled_unit( let env_missing_functions = missing_functions_stubs.stubs.remove("env").unwrap_or_else(|| Vec::new()); context.name_instance( "env".to_owned(), - instantiate_env_module(global_exports, compiler, host_functions, env_missing_functions)? + instantiate_env_module(global_exports, compiler, host_functions, env_missing_functions)?, ); for (module, missing_functions_stubs) in missing_functions_stubs.stubs { @@ -349,7 +349,7 @@ fn instantiate_env_module( for MissingFunction { name, sig } in missing_functions_stubs { let sig = translate_signature( sig, - pointer_type + pointer_type, ); let sig_id = module.signatures.push(sig.clone()); let func_id = module.functions.push(sig_id); From 465889a6150576d7d3d8f0da1ed24c30d78b008d Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Sat, 25 Jan 2020 21:10:26 +0100 Subject: [PATCH 6/6] Ignore non function members. --- client/executor/wasmtime/src/runtime.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/client/executor/wasmtime/src/runtime.rs b/client/executor/wasmtime/src/runtime.rs index 004f95f9e0a9d..77a11ddc7d680 100644 --- a/client/executor/wasmtime/src/runtime.rs +++ b/client/executor/wasmtime/src/runtime.rs @@ -174,12 +174,9 @@ fn scan_missing_functions( func_ty } _ => { - // The executor doesn't provide any non function imports! - return Err(WasmError::Other(format!( - "{}:{} is not a function", - import_entry.module(), - import_entry.field() - ))); + // We are looking only for missing **functions** here. Any other items, be they + // missing or not, will be handled at the resolution stage later. + continue; } }; let signature = convert_parity_wasm_signature(func_ty); @@ -195,8 +192,8 @@ fn scan_missing_functions( } } - // This import is a function, not from env module, or doesn't have a corresponding host - // function, or the signature is not matching. + // This function is either not from the env module, or doesn't have a corresponding host + // function, or the signature is not matching. Add it to the list. let sig = cranelift_ir_signature(signature, &call_conv); missing_functions.insert(