From d3fa8c91af0592ef2e5949294daa05a47138534d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 14 Feb 2020 01:42:27 +0100 Subject: [PATCH] Adds a test to ensure that we clear the heap between calls into runtime (#4903) * Adds a test to ensure that we clear the heap between calls into runtime The tests shows that we currently not clearing the heap in wasmtime. For now we don't run the test for wasmtime. * Fix compilation --- .../executor/common/src/wasm_runtime.rs | 5 +++- .../client/executor/runtime-test/src/lib.rs | 14 +++++++++++ .../executor/src/integration_tests/mod.rs | 23 +++++++++++++++++++ substrate/client/executor/wasmi/src/lib.rs | 13 +++++++++++ .../executor/wasmtime/src/instance_wrapper.rs | 22 ++++++++++++++++-- .../client/executor/wasmtime/src/runtime.rs | 8 ++++++- 6 files changed, 81 insertions(+), 4 deletions(-) diff --git a/substrate/client/executor/common/src/wasm_runtime.rs b/substrate/client/executor/common/src/wasm_runtime.rs index 9b0ad6b780..7af6c2bd53 100644 --- a/substrate/client/executor/common/src/wasm_runtime.rs +++ b/substrate/client/executor/common/src/wasm_runtime.rs @@ -17,7 +17,7 @@ //! Definitions for a wasm runtime. use crate::error::Error; -use sp_wasm_interface::Function; +use sp_wasm_interface::{Function, Value}; /// A trait that defines an abstract wasm runtime. /// @@ -28,4 +28,7 @@ pub trait WasmRuntime { /// Call a method in the Substrate runtime by name. Returns the encoded result on success. fn call(&mut self, method: &str, data: &[u8]) -> Result, Error>; + + /// Get the value from a global with the given `name`. + fn get_global_val(&self, name: &str) -> Result, Error>; } diff --git a/substrate/client/executor/runtime-test/src/lib.rs b/substrate/client/executor/runtime-test/src/lib.rs index b183398c02..38a16ae39e 100644 --- a/substrate/client/executor/runtime-test/src/lib.rs +++ b/substrate/client/executor/runtime-test/src/lib.rs @@ -275,6 +275,20 @@ sp_core::wasm_export_functions! { data.to_vec() } + + // Check that the heap at `heap_base + offset` don't contains the test message. + // After the check succeeds the test message is written into the heap. + // + // It is expected that the given pointer is not allocated. + fn check_and_set_in_heap(heap_base: u32, offset: u32) { + let test_message = b"Hello invalid heap memory"; + let ptr = unsafe { (heap_base + offset) as *mut u8 }; + + let message_slice = unsafe { sp_std::slice::from_raw_parts_mut(ptr, test_message.len()) }; + + assert_ne!(test_message, message_slice); + message_slice.copy_from_slice(test_message); + } } #[cfg(not(feature = "std"))] diff --git a/substrate/client/executor/src/integration_tests/mod.rs b/substrate/client/executor/src/integration_tests/mod.rs index 8c48ec7fcb..c0516d3ac7 100644 --- a/substrate/client/executor/src/integration_tests/mod.rs +++ b/substrate/client/executor/src/integration_tests/mod.rs @@ -565,3 +565,26 @@ fn restoration_of_globals(wasm_method: WasmExecutionMethod) { let res = instance.call("allocates_huge_stack_array", &false.encode()); assert!(res.is_ok()); } + +#[test_case(WasmExecutionMethod::Interpreted)] +fn heap_is_reset_between_calls(wasm_method: WasmExecutionMethod) { + let mut instance = crate::wasm_runtime::create_wasm_runtime_with_code( + wasm_method, + 1024, + &WASM_BINARY[..], + HostFunctions::host_functions(), + true, + ).expect("Creates instance"); + + let heap_base = instance.get_global_val("__heap_base") + .expect("`__heap_base` is valid") + .expect("`__heap_base` exists") + .as_i32() + .expect("`__heap_base` is an `i32`"); + + let params = (heap_base as u32, 512u32 * 64 * 1024).encode(); + instance.call("check_and_set_in_heap", ¶ms).unwrap(); + + // Cal it a second time to check that the heap was freed. + instance.call("check_and_set_in_heap", ¶ms).unwrap(); +} diff --git a/substrate/client/executor/wasmi/src/lib.rs b/substrate/client/executor/wasmi/src/lib.rs index 7965c7f95e..b90c0f05f5 100644 --- a/substrate/client/executor/wasmi/src/lib.rs +++ b/substrate/client/executor/wasmi/src/lib.rs @@ -669,6 +669,19 @@ impl WasmRuntime for WasmiRuntime { &self.missing_functions, ) } + + fn get_global_val(&self, name: &str) -> Result, Error> { + match self.instance.export_by_name(name) { + Some(global) => Ok(Some( + global + .as_global() + .ok_or_else(|| format!("`{}` is not a global", name))? + .get() + .into() + )), + None => Ok(None), + } + } } pub fn create_instance( diff --git a/substrate/client/executor/wasmtime/src/instance_wrapper.rs b/substrate/client/executor/wasmtime/src/instance_wrapper.rs index 8f722f6490..013651cd7a 100644 --- a/substrate/client/executor/wasmtime/src/instance_wrapper.rs +++ b/substrate/client/executor/wasmtime/src/instance_wrapper.rs @@ -21,10 +21,10 @@ use crate::util; use crate::imports::Imports; use sc_executor_common::error::{Error, Result}; -use sp_wasm_interface::{Pointer, WordSize}; +use sp_wasm_interface::{Pointer, WordSize, Value}; use std::slice; use std::marker; -use wasmtime::{Instance, Module, Memory, Table}; +use wasmtime::{Instance, Module, Memory, Table, Val}; /// Wrap the given WebAssembly Instance of a wasm module with Substrate-runtime. /// @@ -127,6 +127,24 @@ impl InstanceWrapper { Ok(heap_base as u32) } + + /// Get the value from a global with the given `name`. + pub fn get_global_val(&self, name: &str) -> Result> { + let global = match self.instance.get_export(name) { + Some(global) => global, + None => return Ok(None), + }; + + let global = global.global().ok_or_else(|| format!("`{}` is not a global", name))?; + + match global.get() { + Val::I32(val) => Ok(Some(Value::I32(val))), + Val::I64(val) => Ok(Some(Value::I64(val))), + Val::F32(val) => Ok(Some(Value::F32(val))), + Val::F64(val) => Ok(Some(Value::F64(val))), + _ => Err("Unknow value type".into()), + } + } } /// Extract linear memory instance from the given instance. diff --git a/substrate/client/executor/wasmtime/src/runtime.rs b/substrate/client/executor/wasmtime/src/runtime.rs index 106a398dfc..b99d334787 100644 --- a/substrate/client/executor/wasmtime/src/runtime.rs +++ b/substrate/client/executor/wasmtime/src/runtime.rs @@ -27,7 +27,7 @@ use sc_executor_common::{ }; use sp_allocator::FreeingBumpHeapAllocator; use sp_runtime_interface::unpack_ptr_and_len; -use sp_wasm_interface::{Function, Pointer, WordSize}; +use sp_wasm_interface::{Function, Pointer, WordSize, Value}; use wasmtime::{Config, Engine, Module, Store}; /// A `WasmRuntime` implementation using wasmtime to compile the runtime module to machine code @@ -55,6 +55,12 @@ impl WasmRuntime for WasmtimeRuntime { self.heap_pages, ) } + + fn get_global_val(&self, name: &str) -> Result> { + // Yeah, there is no better way currently :( + InstanceWrapper::new(&self.module, &self.imports, self.heap_pages)? + .get_global_val(name) + } } /// Create a new `WasmtimeRuntime` given the code. This function performs translation from Wasm to