From 4b136691ee2deb2569e816bc6f027bc39e1ef406 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Wed, 29 Jul 2020 12:55:41 +0000 Subject: [PATCH] refactor globals snapshot --- Cargo.lock | 69 +++++----- client/executor/wasmtime/Cargo.toml | 8 +- client/executor/wasmtime/src/imports.rs | 4 +- .../executor/wasmtime/src/instance_wrapper.rs | 20 ++- .../src/instance_wrapper/globals_snapshot.rs | 129 ++++++------------ 5 files changed, 88 insertions(+), 142 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0fe8e4919efb1..7adcbf9971563 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4273,7 +4273,7 @@ dependencies = [ "parity-scale-codec", "parity-wasm 0.41.0", "pretty_assertions", - "pwasm-utils", + "pwasm-utils 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde", "sp-core", "sp-io", @@ -5500,6 +5500,16 @@ dependencies = [ "prost", ] +[[package]] +name = "pwasm-utils" +version = "0.12.0" +source = "git+https://github.com/paritytech/wasm-utils?branch=expose-globals#3ea2c9a70d2dffb6e40bdf2523647c5c12802e50" +dependencies = [ + "byteorder", + "log", + "parity-wasm 0.41.0", +] + [[package]] name = "pwasm-utils" version = "0.12.0" @@ -6608,20 +6618,17 @@ name = "sc-executor-wasmtime" version = "0.8.0-rc5" dependencies = [ "assert_matches", - "cranelift-codegen", - "cranelift-wasm", "log", "parity-scale-codec", "parity-wasm 0.41.0", + "pwasm-utils 0.12.0 (git+https://github.com/paritytech/wasm-utils?branch=expose-globals)", "sc-executor-common", "scoped-tls", "sp-allocator", "sp-core", "sp-runtime-interface", "sp-wasm-interface", - "substrate-wasmtime", - "wasmtime-environ", - "wasmtime-runtime", + "wasmtime", ] [[package]] @@ -8655,31 +8662,6 @@ dependencies = [ name = "substrate-wasm-builder-runner" version = "1.0.6" -[[package]] -name = "substrate-wasmtime" -version = "0.19.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d75a69f5b3afef86e3e372529bf3fb1f7219b20287c4490e4cb4b4e91970f4f5" -dependencies = [ - "anyhow", - "backtrace", - "cfg-if", - "lazy_static", - "libc", - "log", - "region", - "rustc-demangle", - "smallvec 1.4.1", - "target-lexicon", - "wasmparser 0.59.0", - "wasmtime-environ", - "wasmtime-jit", - "wasmtime-profiling", - "wasmtime-runtime", - "wat", - "winapi 0.3.8", -] - [[package]] name = "subtle" version = "1.0.0" @@ -9728,6 +9710,31 @@ version = "0.59.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a950e6a618f62147fd514ff445b2a0b53120d382751960797f85f058c7eda9b9" +[[package]] +name = "wasmtime" +version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1cd3c4f449382779ef6e0a7c3ec6752ae614e20a42e4100000c3efdc973100e2" +dependencies = [ + "anyhow", + "backtrace", + "cfg-if", + "lazy_static", + "libc", + "log", + "region", + "rustc-demangle", + "smallvec 1.4.1", + "target-lexicon", + "wasmparser 0.59.0", + "wasmtime-environ", + "wasmtime-jit", + "wasmtime-profiling", + "wasmtime-runtime", + "wat", + "winapi 0.3.8", +] + [[package]] name = "wasmtime-debug" version = "0.19.0" diff --git a/client/executor/wasmtime/Cargo.toml b/client/executor/wasmtime/Cargo.toml index 0267cf6efadc0..821c6b33d1696 100644 --- a/client/executor/wasmtime/Cargo.toml +++ b/client/executor/wasmtime/Cargo.toml @@ -21,12 +21,8 @@ sp-wasm-interface = { version = "2.0.0-rc5", path = "../../../primitives/wasm-in sp-runtime-interface = { version = "2.0.0-rc5", path = "../../../primitives/runtime-interface" } sp-core = { version = "2.0.0-rc5", path = "../../../primitives/core" } sp-allocator = { version = "2.0.0-rc5", path = "../../../primitives/allocator" } -wasmtime = { package = "substrate-wasmtime", version = "0.19.0" } -wasmtime-runtime = { version = "0.19.0" } -wasmtime-environ = { version = "0.19.0" } -cranelift-wasm = { version = "0.66.0" } -cranelift-codegen = { version = "0.66.0" } - +wasmtime = "0.19" +pwasm-utils = { git = "https://github.com/paritytech/wasm-utils", branch = "expose-globals" } [dev-dependencies] assert_matches = "1.3.0" diff --git a/client/executor/wasmtime/src/imports.rs b/client/executor/wasmtime/src/imports.rs index 41498e2b0fa34..add62df5cef45 100644 --- a/client/executor/wasmtime/src/imports.rs +++ b/client/executor/wasmtime/src/imports.rs @@ -294,7 +294,7 @@ fn into_wasmtime_val_type(val_ty: ValueType) -> wasmtime::ValType { /// Converts a `Val` into a substrate runtime interface `Value`. /// /// Panics if the given value doesn't have a corresponding variant in `Value`. -fn into_value(val: Val) -> Value { +pub fn into_value(val: Val) -> Value { match val { Val::I32(v) => Value::I32(v), Val::I64(v) => Value::I64(v), @@ -304,7 +304,7 @@ fn into_value(val: Val) -> Value { } } -fn into_wasmtime_val(value: Value) -> wasmtime::Val { +pub fn into_wasmtime_val(value: Value) -> wasmtime::Val { match value { Value::I32(v) => Val::I32(v), Value::I64(v) => Val::I64(v), diff --git a/client/executor/wasmtime/src/instance_wrapper.rs b/client/executor/wasmtime/src/instance_wrapper.rs index d31193688b9ad..9a4e44d3b106c 100644 --- a/client/executor/wasmtime/src/instance_wrapper.rs +++ b/client/executor/wasmtime/src/instance_wrapper.rs @@ -29,36 +29,36 @@ use sc_executor_common::{ }; use sp_wasm_interface::{Pointer, WordSize, Value}; use wasmtime::{Engine, Instance, Module, Memory, Table, Val, Func, Extern, Global, Store}; +use parity_wasm::elements; mod globals_snapshot; pub use globals_snapshot::GlobalsSnapshot; pub struct ModuleWrapper { - imported_globals_count: u32, - globals_count: u32, module: Module, data_segments_snapshot: DataSegmentsSnapshot, } impl ModuleWrapper { pub fn new(engine: &Engine, code: &[u8]) -> Result { - let module = Module::new(engine, code) + let mut raw_module: elements::Module = elements::deserialize_buffer(code) + .map_err(|e| Error::from(format!("cannot decode module: {}", e)))?; + pwasm_utils::export_mutable_globals(&mut raw_module, "exported_internal_global"); + let instrumented_code = elements::serialize(raw_module) + .map_err(|e| Error::from(format!("cannot encode module: {}", e)))?; + + let module = Module::new(engine, &instrumented_code) .map_err(|e| Error::from(format!("cannot create module: {}", e)))?; let module_info = WasmModuleInfo::new(code) .ok_or_else(|| Error::from("cannot deserialize module".to_string()))?; - let declared_globals_count = module_info.declared_globals_count(); - let imported_globals_count = module_info.imported_globals_count(); - let globals_count = imported_globals_count + declared_globals_count; let data_segments_snapshot = DataSegmentsSnapshot::take(&module_info) .map_err(|e| Error::from(format!("cannot take data segments snapshot: {}", e)))?; Ok(Self { module, - imported_globals_count, - globals_count, data_segments_snapshot, }) } @@ -78,8 +78,6 @@ impl ModuleWrapper { /// routines. pub struct InstanceWrapper { instance: Instance, - globals_count: u32, - imported_globals_count: u32, // The memory instance of the `instance`. // // It is important to make sure that we don't make any copies of this to make it easier to proof @@ -143,8 +141,6 @@ impl InstanceWrapper { Ok(Self { table: get_table(&instance), instance, - globals_count: module_wrapper.globals_count, - imported_globals_count: module_wrapper.imported_globals_count, memory, _not_send_nor_sync: marker::PhantomData, }) diff --git a/client/executor/wasmtime/src/instance_wrapper/globals_snapshot.rs b/client/executor/wasmtime/src/instance_wrapper/globals_snapshot.rs index dd99d63ae2518..b97515f846fe5 100644 --- a/client/executor/wasmtime/src/instance_wrapper/globals_snapshot.rs +++ b/client/executor/wasmtime/src/instance_wrapper/globals_snapshot.rs @@ -17,115 +17,62 @@ // along with this program. If not, see . use super::InstanceWrapper; -use sc_executor_common::{ - error::{Error, Result}, -}; +use sc_executor_common::error::{Result, Error}; use sp_wasm_interface::Value; -use cranelift_codegen::ir; -use cranelift_wasm::GlobalIndex; -use wasmtime_runtime::{ExportGlobal, Export}; +use crate::imports::{into_value, into_wasmtime_val}; + +struct SavedValue { + index: usize, + value: Value, +} /// A snapshot of a global variables values. This snapshot can be used later for restoring the /// values to the preserved state. /// /// Technically, a snapshot stores only values of mutable global variables. This is because /// immutable global variables always have the same values. -pub struct GlobalsSnapshot { - handle: wasmtime_runtime::InstanceHandle, - preserved_mut_globals: Vec<(*mut wasmtime_runtime::VMGlobalDefinition, Value)>, -} +pub struct GlobalsSnapshot(Vec); impl GlobalsSnapshot { /// Take a snapshot of global variables for a given instance. pub fn take(instance_wrapper: &InstanceWrapper) -> Result { - // EVIL: - // Usage of an undocumented function. - let handle = instance_wrapper.instance.handle().clone().handle; - - let mut preserved_mut_globals = vec![]; - - for global_idx in instance_wrapper.imported_globals_count..instance_wrapper.globals_count { - let (def, global) = match handle.lookup_by_declaration( - &wasmtime_environ::EntityIndex::Global(GlobalIndex::from_u32(global_idx)), - ) { - Export::Global(ExportGlobal { definition, global, .. }) => (definition, global), - _ => unreachable!("only globals can be returned for a global request"), - }; - - // skip immutable globals. - if !global.mutability { - continue; - } - - let value = unsafe { - // Safety of this function solely depends on the correctness of the reference and - // the type information of the global. - read_global(def, global.ty)? - }; - preserved_mut_globals.push((def, value)); - } - - Ok(Self { - preserved_mut_globals, - handle, - }) + let data = instance_wrapper.instance + .exports() + .enumerate() + .filter_map(|(index, export)| { + if export.name().starts_with("exported_internal_global") { + export.into_global().map( + |g| SavedValue { index, value: into_value(g.get()) } + ) + } else { None } + }) + .collect::>(); + + Ok(Self(data)) } /// Apply the snapshot to the given instance. /// /// This instance must be the same that was used for creation of this snapshot. pub fn apply(&self, instance_wrapper: &InstanceWrapper) -> Result<()> { - if instance_wrapper.instance.handle().handle != self.handle { - return Err(Error::from("unexpected instance handle".to_string())); - } - - for (def, value) in &self.preserved_mut_globals { - unsafe { - // The following writes are safe if the precondition that this is the same instance - // this snapshot was created with: - // - // 1. These pointers must be still not-NULL and allocated. - // 2. The set of global variables is fixed for the lifetime of the same instance. - // 3. We obviously assume that the wasmtime references are correct in the first place. - // 4. We write the data with the same type it was read in the first place. - write_global(*def, *value)?; + let mut current = 0; + for (index, export) in instance_wrapper.instance.exports().enumerate() { + if current >= self.0.len() { break; } + let current_saved = &self.0[current]; + if index < current_saved.index { continue; } + else if index > current_saved.index { current += 1; continue; } + else { + export.into_global() + .ok_or_else( + || Error::Other("Wrong instance in GlobalsSnapshot::apply: what should be global is not global.".to_string()) + )? + .set(into_wasmtime_val(current_saved.value)) + .map_err( + |_e| Error::Other("Wrong instance in GlobalsSnapshot::apply: global saved type does not matched applied.".to_string()) + )?; } } - Ok(()) - } -} - -unsafe fn read_global( - def: *const wasmtime_runtime::VMGlobalDefinition, - ty: ir::Type, -) -> Result { - let def = def - .as_ref() - .ok_or_else(|| Error::from("wasmtime global reference is null during read".to_string()))?; - let val = match ty { - ir::types::I32 => Value::I32(*def.as_i32()), - ir::types::I64 => Value::I64(*def.as_i64()), - ir::types::F32 => Value::F32(*def.as_u32()), - ir::types::F64 => Value::F64(*def.as_u64()), - _ => { - return Err(Error::from(format!( - "unsupported global variable type: {}", - ty - ))) - } - }; - Ok(val) -} -unsafe fn write_global(def: *mut wasmtime_runtime::VMGlobalDefinition, value: Value) -> Result<()> { - let def = def - .as_mut() - .ok_or_else(|| Error::from("wasmtime global reference is null during write".to_string()))?; - match value { - Value::I32(v) => *def.as_i32_mut() = v, - Value::I64(v) => *def.as_i64_mut() = v, - Value::F32(v) => *def.as_u32_mut() = v, - Value::F64(v) => *def.as_u64_mut() = v, + Ok(()) } - Ok(()) -} +} \ No newline at end of file