From 25ee6eb4dfe68756c7ae8a0c72ec2a7d8177c146 Mon Sep 17 00:00:00 2001 From: Sergei Pepyakin Date: Thu, 14 Nov 2019 09:40:00 +0100 Subject: [PATCH] Additional wasm diagnostics (#4097) * Wasm diagnostics * Pass the error * Make errno optional * Cargo.lock * Log the error --- Cargo.lock | 32 ++++++++++++++++++++------- core/executor/Cargo.toml | 5 ++++- core/executor/src/error.rs | 4 ++++ core/executor/src/wasmi_execution.rs | 33 ++++++++++------------------ core/primitives/Cargo.toml | 2 +- core/sr-sandbox/Cargo.toml | 2 +- core/wasm-interface/Cargo.toml | 2 +- node/cli/Cargo.toml | 3 ++- node/executor/Cargo.toml | 3 +++ 9 files changed, 51 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7a0eecbf3fb92..f8535aface5e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3198,6 +3198,11 @@ name = "parity-wasm" version = "0.40.3" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "parity-wasm" +version = "0.41.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "parking_lot" version = "0.6.4" @@ -4376,7 +4381,7 @@ dependencies = [ "sr-std 2.0.0", "substrate-primitives 2.0.0", "wabt 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)", - "wasmi 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", + "wasmi 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -5716,7 +5721,7 @@ dependencies = [ "substrate-wasm-interface 2.0.0", "test-case 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "wabt 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)", - "wasmi 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", + "wasmi 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "wasmtime-environ 0.2.0 (git+https://github.com/CraneStation/wasmtime.git?rev=71dd73d6)", "wasmtime-jit 0.2.0 (git+https://github.com/CraneStation/wasmtime.git?rev=71dd73d6)", "wasmtime-runtime 0.2.0 (git+https://github.com/CraneStation/wasmtime.git?rev=71dd73d6)", @@ -5986,7 +5991,7 @@ dependencies = [ "tiny-bip39 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "tiny-keccak 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "twox-hash 1.5.0 (registry+https://github.com/rust-lang/crates.io-index)", - "wasmi 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", + "wasmi 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "zeroize 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -6429,7 +6434,7 @@ name = "substrate-wasm-interface" version = "2.0.0" dependencies = [ "impl-trait-for-tuples 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", - "wasmi 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", + "wasmi 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -7231,15 +7236,16 @@ dependencies = [ [[package]] name = "wasmi" -version = "0.5.1" +version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ + "errno 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)", "memory_units 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "num-rational 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "num-traits 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", - "parity-wasm 0.40.3 (registry+https://github.com/rust-lang/crates.io-index)", - "wasmi-validation 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "parity-wasm 0.41.0 (registry+https://github.com/rust-lang/crates.io-index)", + "wasmi-validation 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -7250,6 +7256,14 @@ dependencies = [ "parity-wasm 0.40.3 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "wasmi-validation" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "parity-wasm 0.41.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "wasmparser" version = "0.39.2" @@ -7867,6 +7881,7 @@ dependencies = [ "checksum parity-util-mem 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "570093f39f786beea92dcc09e45d8aae7841516ac19a50431953ac82a0e8f85c" "checksum parity-wasm 0.32.0 (registry+https://github.com/rust-lang/crates.io-index)" = "16ad52817c4d343339b3bc2e26861bd21478eda0b7509acf83505727000512ac" "checksum parity-wasm 0.40.3 (registry+https://github.com/rust-lang/crates.io-index)" = "1e39faaa292a687ea15120b1ac31899b13586446521df6c149e46f1584671e0f" +"checksum parity-wasm 0.41.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ddfc878dac00da22f8f61e7af3157988424567ab01d9920b962ef7dcbd7cd865" "checksum parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)" = "f0802bff09003b291ba756dc7e79313e51cc31667e94afbe847def490424cde5" "checksum parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ab41b4aed082705d1056416ae4468b6ea99d52599ecf3169b00088d43113e337" "checksum parking_lot 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fa7767817701cce701d5585b9c4db3cdd02086398322c1d7e8bf5094a96a2ce7" @@ -8081,8 +8096,9 @@ dependencies = [ "checksum wasm-bindgen-webidl 0.2.54 (registry+https://github.com/rust-lang/crates.io-index)" = "3021567c515a746a64ad0b269d120d46e687c0c95702a4750623db935ae6b5e7" "checksum wasm-gc-api 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)" = "d0c32691b6c7e6c14e7f8fd55361a9088b507aa49620fcd06c09b3a1082186b9" "checksum wasm-timer 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "aa3e01d234bb71760e685cfafa5e2c96f8ad877c161a721646356651069e26ac" -"checksum wasmi 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f31d26deb2d9a37e6cfed420edce3ed604eab49735ba89035e13c98f9a528313" +"checksum wasmi 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "bf617d864d25af3587aa745529f7aaa541066c876d57e050c0d0c85c61c92aff" "checksum wasmi-validation 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "6bc0356e3df56e639fc7f7d8a99741915531e27ed735d911ed83d7e1339c8188" +"checksum wasmi-validation 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ea78c597064ba73596099281e2f4cfc019075122a65cdda3205af94f0b264d93" "checksum wasmparser 0.39.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e5083b449454f7de0b15f131eee17de54b5a71dcb9adcf11df2b2f78fad0cd82" "checksum wasmtime-debug 0.2.0 (git+https://github.com/CraneStation/wasmtime.git?rev=71dd73d6)" = "" "checksum wasmtime-environ 0.2.0 (git+https://github.com/CraneStation/wasmtime.git?rev=71dd73d6)" = "" diff --git a/core/executor/Cargo.toml b/core/executor/Cargo.toml index 36512a1e9d4ee..e0f2b8258043e 100644 --- a/core/executor/Cargo.toml +++ b/core/executor/Cargo.toml @@ -13,7 +13,7 @@ trie = { package = "substrate-trie", path = "../trie" } serializer = { package = "substrate-serializer", path = "../serializer" } runtime_version = { package = "sr-version", path = "../sr-version" } panic-handler = { package = "substrate-panic-handler", path = "../panic-handler" } -wasmi = "0.5.1" +wasmi = "0.6.2" parity-wasm = "0.40.3" lazy_static = "1.4.0" wasm-interface = { package = "substrate-wasm-interface", path = "../wasm-interface" } @@ -55,3 +55,6 @@ wasmtime = [ "wasmtime-jit", "wasmtime-runtime", ] +wasmi-errno = [ + "wasmi/errno" +] diff --git a/core/executor/src/error.rs b/core/executor/src/error.rs index 83168d598e03c..58a84e09902e9 100644 --- a/core/executor/src/error.rs +++ b/core/executor/src/error.rs @@ -117,6 +117,10 @@ pub enum WasmError { CodeNotFound, /// Failure to reinitialize runtime instance from snapshot. ApplySnapshotFailed, + /// Failure to erase the wasm memory. + /// + /// Depending on the implementation might mean failure of allocating memory. + ErasingFailed(String), /// Wasm code failed validation. InvalidModule, /// Wasm code could not be deserialized. diff --git a/core/executor/src/wasmi_execution.rs b/core/executor/src/wasmi_execution.rs index e8e4aa3e53196..7f480695798fa 100644 --- a/core/executor/src/wasmi_execution.rs +++ b/core/executor/src/wasmi_execution.rs @@ -28,7 +28,7 @@ use crate::sandbox; use crate::allocator; use crate::wasm_utils::interpret_runtime_api_result; use crate::wasm_runtime::WasmRuntime; -use log::trace; +use log::{error, trace}; use parity_wasm::elements::{deserialize_buffer, DataSegment, Instruction, Module as RawModule}; use wasm_interface::{ FunctionContext, Pointer, WordSize, Sandbox, MemoryId, Result as WResult, Function, @@ -500,7 +500,7 @@ impl StateSnapshot { // First, erase the memory and copy the data segments into it. memory .erase() - .map_err(|_| WasmError::ApplySnapshotFailed)?; + .map_err(|e| WasmError::ErasingFailed(e.to_string()))?; for (offset, contents) in &self.data_segments { memory .set(*offset, contents) @@ -536,23 +536,6 @@ pub struct WasmiRuntime { host_functions: Vec<&'static dyn Function>, } -impl WasmiRuntime { - /// Perform an operation with the clean version of the runtime wasm instance. - fn with(&self, f: F) -> R - where - F: FnOnce(&ModuleRef) -> R, - { - self.state_snapshot.apply(&self.instance).expect( - "applying the snapshot can only fail if the passed instance is different - from the one that was used for creation of the snapshot; - we use the snapshot that is directly associated with the instance; - thus the snapshot was created using the instance; - qed", - ); - f(&self.instance) - } -} - impl WasmRuntime for WasmiRuntime { fn update_heap_pages(&mut self, heap_pages: u64) -> bool { self.state_snapshot.heap_pages == heap_pages @@ -568,9 +551,15 @@ impl WasmRuntime for WasmiRuntime { method: &str, data: &[u8], ) -> Result, Error> { - self.with(|module| { - call_in_wasm_module(ext, module, method, data, &self.host_functions) - }) + self.state_snapshot.apply(&self.instance) + .map_err(|e| { + // Snapshot restoration failed. This is pretty unexpected since this can happen + // if some invariant is broken or if the system is under extreme memory pressure + // (so erasing fails). + error!(target: "wasm-executor", "snapshot restoration failed: {}", e); + e + })?; + call_in_wasm_module(ext, &self.instance, method, data, &self.host_functions) } } diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml index 80aff356ad22e..f9888928dc13d 100644 --- a/core/primitives/Cargo.toml +++ b/core/primitives/Cargo.toml @@ -14,7 +14,7 @@ twox-hash = { version = "1.5.0", default-features = false, optional = true } byteorder = { version = "1.3.2", default-features = false } primitive-types = { version = "0.6", default-features = false, features = ["codec"] } impl-serde = { version = "0.2.3", optional = true } -wasmi = { version = "0.5.1", optional = true } +wasmi = { version = "0.6.2", optional = true } hash-db = { version = "0.15.2", default-features = false } hash256-std-hasher = { version = "0.15.2", default-features = false } ed25519-dalek = { version = "0.9.1", default-features = false, features = ["u64_backend"], optional = true } diff --git a/core/sr-sandbox/Cargo.toml b/core/sr-sandbox/Cargo.toml index 20d569f043c60..32e79781b2459 100755 --- a/core/sr-sandbox/Cargo.toml +++ b/core/sr-sandbox/Cargo.toml @@ -5,7 +5,7 @@ authors = ["Parity Technologies "] edition = "2018" [dependencies] -wasmi = { version = "0.5.1", optional = true } +wasmi = { version = "0.6.2", optional = true } primitives = { package = "substrate-primitives", path = "../primitives", default-features = false } rstd = { package = "sr-std", path = "../sr-std", default-features = false } runtime-io = { package = "sr-io", path = "../sr-io", default-features = false } diff --git a/core/wasm-interface/Cargo.toml b/core/wasm-interface/Cargo.toml index dcda5061c91a1..90dc7b812d9f9 100644 --- a/core/wasm-interface/Cargo.toml +++ b/core/wasm-interface/Cargo.toml @@ -5,5 +5,5 @@ authors = ["Parity Technologies "] edition = "2018" [dependencies] -wasmi = "0.5.1" +wasmi = "0.6.2" impl-trait-for-tuples = "0.1.2" diff --git a/node/cli/Cargo.toml b/node/cli/Cargo.toml index 20379d52e768d..0d6fc2aa50a8b 100644 --- a/node/cli/Cargo.toml +++ b/node/cli/Cargo.toml @@ -124,7 +124,8 @@ cli = [ "tokio", "exit-future", "ctrlc", - "substrate-service/rocksdb" + "substrate-service/rocksdb", + "node-executor/wasmi-errno", ] wasmtime = [ "cli", diff --git a/node/executor/Cargo.toml b/node/executor/Cargo.toml index 93f29910edbe0..8e23187962dac 100644 --- a/node/executor/Cargo.toml +++ b/node/executor/Cargo.toml @@ -37,6 +37,9 @@ criterion = "0.3.0" wasmtime = [ "substrate-executor/wasmtime", ] +wasmi-errno = [ + "substrate-executor/wasmi-errno", +] stress-test = [] [[bench]]