Skip to content

Commit

Permalink
Adds a test to ensure that we clear the heap between calls into runti…
Browse files Browse the repository at this point in the history
…me (paritytech#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
  • Loading branch information
bkchr authored and General-Beck committed Feb 18, 2020
1 parent 7d502be commit e09b497
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 4 deletions.
5 changes: 4 additions & 1 deletion client/executor/common/src/wasm_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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<Vec<u8>, Error>;

/// Get the value from a global with the given `name`.
fn get_global_val(&self, name: &str) -> Result<Option<Value>, Error>;
}
14 changes: 14 additions & 0 deletions client/executor/runtime-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
23 changes: 23 additions & 0 deletions client/executor/src/integration_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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", &params).unwrap();

// Cal it a second time to check that the heap was freed.
instance.call("check_and_set_in_heap", &params).unwrap();
}
13 changes: 13 additions & 0 deletions client/executor/wasmi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,19 @@ impl WasmRuntime for WasmiRuntime {
&self.missing_functions,
)
}

fn get_global_val(&self, name: &str) -> Result<Option<sp_wasm_interface::Value>, 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(
Expand Down
22 changes: 20 additions & 2 deletions client/executor/wasmtime/src/instance_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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<Option<Value>> {
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.
Expand Down
8 changes: 7 additions & 1 deletion client/executor/wasmtime/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -55,6 +55,12 @@ impl WasmRuntime for WasmtimeRuntime {
self.heap_pages,
)
}

fn get_global_val(&self, name: &str) -> Result<Option<Value>> {
// 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
Expand Down

0 comments on commit e09b497

Please sign in to comment.