Cursed implementation of allowing missing imports on wasmtime (parity…

…tech#4730)

* Quick and dirty impl.

* Clean up

* Check the signatures.

* Fix tests.

* Apply suggestions from code review

Co-Authored-By: Bastian Köcher <[email protected]>

* Ignore non function members.

Co-authored-by: Bastian Köcher <[email protected]>

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)]

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.

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) }),
 	}
 }

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,
 	})
 }