From 2421576f91dea0c6067f41ae963e2ac2b4255970 Mon Sep 17 00:00:00 2001
From: Sergei Pepyakin <sergei@parity.io>
Date: Sat, 25 Jan 2020 23:53:08 +0100
Subject: [PATCH] Cursed implementation of allowing missing imports on wasmtime
(#4730)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Quick and dirty impl.
* Clean up
* Check the signatures.
* Fix tests.
* Apply suggestions from code review
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
* Ignore non function members.
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
---
.../executor/src/integration_tests/mod.rs | 90 +++++------
substrate/client/executor/src/lib.rs | 8 +-
substrate/client/executor/src/wasm_runtime.rs | 6 +-
substrate/client/executor/wasmi/src/lib.rs | 34 ++---
.../client/executor/wasmtime/src/runtime.rs | 140 +++++++++++++++++-
.../client/executor/wasmtime/src/util.rs | 18 +++
6 files changed, 215 insertions(+), 81 deletions(-)
diff --git a/substrate/client/executor/src/integration_tests/mod.rs b/substrate/client/executor/src/integration_tests/mod.rs
index 00f4eb33b02..5b2fc5ca0c9 100644
--- a/substrate/client/executor/src/integration_tests/mod.rs
+++ b/substrate/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<Blake2Hasher, u64>;
-
-#[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<Item = Value>,
- ) -> Result<Option<Value>> {
- 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 =
- (wasmtime_missing_externals::WasmtimeHostFunctions, sp_io::SubstrateHostFunctions);
-
-#[cfg(not(feature = "wasmtime"))]
type HostFunctions = sp_io::SubstrateHostFunctions;
fn call_in_wasm<E: Externalities>(
@@ -114,40 +72,66 @@ 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"))]
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))]
-#[should_panic(expected = "Function `yet_another_missing_external` is only a stub. Calling a stub is not allowed.")]
-#[cfg(not(feature = "wasmtime"))]
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)]
diff --git a/substrate/client/executor/src/lib.rs b/substrate/client/executor/src/lib.rs
index a054f4dc76e..152e3a49848 100644
--- a/substrate/client/executor/src/lib.rs
+++ b/substrate/client/executor/src/lib.rs
@@ -68,7 +68,7 @@ pub fn call_in_wasm<HF: sp_wasm_interface::HostFunctions>(
ext: &mut dyn Externalities,
code: &[u8],
heap_pages: u64,
- allow_missing_imports: bool,
+ allow_missing_func_imports: bool,
) -> error::Result<Vec<u8>> {
call_in_wasm_with_host_functions(
function,
@@ -78,7 +78,7 @@ pub fn call_in_wasm<HF: sp_wasm_interface::HostFunctions>(
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<Vec<u8>> {
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/substrate/client/executor/src/wasm_runtime.rs b/substrate/client/executor/src/wasm_runtime.rs
index 036b28f7640..b8966c3af27 100644
--- a/substrate/client/executor/src/wasm_runtime.rs
+++ b/substrate/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<Box<dyn WasmRuntime>, 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<dyn WasmRuntime> { 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<dyn WasmRuntime> { Box::new(runtime) }),
}
}
diff --git a/substrate/client/executor/wasmi/src/lib.rs b/substrate/client/executor/wasmi/src/lib.rs
index d3d0ee6e1e3..cf8125e7745 100644
--- a/substrate/client/executor/wasmi/src/lib.rs
+++ b/substrate/client/executor/wasmi/src/lib.rs
@@ -38,7 +38,7 @@ struct FunctionExecutor<'a> {
memory: MemoryRef,
table: Option<TableRef>,
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<TableRef>,
host_functions: &'a [&'static dyn Function],
- allow_missing_imports: bool,
+ allow_missing_func_imports: bool,
missing_functions: &'a [String],
) -> Result<Self, Error> {
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<Vec<String>>,
}
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<String>,
) -> Result<Vec<u8>, 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<String>), 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<String>,
}
@@ -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<WasmiRuntime, WasmError> {
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/substrate/client/executor/wasmtime/src/runtime.rs b/substrate/client/executor/wasmtime/src/runtime.rs
index f9b1b6209c3..77a11ddc7d6 100644
--- a/substrate/client/executor/wasmtime/src/runtime.rs
+++ b/substrate/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},
@@ -86,8 +91,9 @@ pub fn create_instance(
code: &[u8],
heap_pages: u64,
host_functions: Vec<&'static dyn Function>,
+ allow_missing_func_imports: bool,
) -> std::result::Result<WasmtimeRuntime, WasmError> {
- 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 +121,95 @@ pub fn create_instance(
})
}
+#[derive(Debug)]
+struct MissingFunction {
+ name: String,
+ sig: cranelift_codegen::ir::Signature,
+}
+
+#[derive(Debug)]
+struct MissingFunctionStubs {
+ stubs: HashMap<String, Vec<MissingFunction>>,
+}
+
+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 scan_missing_functions(
+ code: &[u8],
+ host_functions: &[&'static dyn Function],
+) -> std::result::Result<MissingFunctionStubs, WasmError> {
+ let isa = target_isa()?;
+ let call_conv = isa.default_call_conv();
+
+ 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
+ }
+ _ => {
+ // 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);
+
+ if import_entry.module() == "env" {
+ if let Some(hf) = host_functions
+ .iter()
+ .find(|hf| hf.name() == import_entry.field())
+ {
+ if signature == hf.signature() {
+ continue;
+ }
+ }
+ }
+
+ // 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(
+ 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 +222,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 {
+ 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, &[], missing_functions_stubs)?;
+ context.name_instance(module, instance);
+ }
// Compile the wasm module.
let module = context.compile_module(&code)
@@ -199,6 +310,7 @@ fn instantiate_env_module(
global_exports: Rc<RefCell<HashMap<String, Option<Export>>>>,
compiler: Compiler,
host_functions: &[&'static dyn Function],
+ missing_functions_stubs: Vec<MissingFunction>,
) -> std::result::Result<InstanceHandle, WasmError>
{
let isa = target_isa()?;
@@ -231,6 +343,26 @@ fn instantiate_env_module(
finished_functions.push(trampoline);
}
+ for MissingFunction { name, sig } in missing_functions_stubs {
+ let sig = translate_signature(
+ sig,
+ pointer_type,
+ );
+ let sig_id = module.signatures.push(sig.clone());
+ let func_id = module.functions.push(sig_id);
+ module
+ .exports
+ .insert(name, 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();
diff --git a/substrate/client/executor/wasmtime/src/util.rs b/substrate/client/executor/wasmtime/src/util.rs
index 551295911a9..77fb8af3867 100644
--- a/substrate/client/executor/wasmtime/src/util.rs
+++ b/substrate/client/executor/wasmtime/src/util.rs
@@ -51,6 +51,24 @@ pub fn checked_range(offset: usize, len: usize, max: usize) -> Option<Range<usiz
}
}
+/// Convert from a parity wasm FunctionType to wasm interface's Signature.
+pub fn convert_parity_wasm_signature(func_ty: &parity_wasm::elements::FunctionType) -> 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::<Vec<_>>(),
+ 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 {
--
GitLab