From 4c351a77f7f6a9b673884797619116dcde9e4231 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Tue, 22 Dec 2020 22:27:28 +0100 Subject: [PATCH] Create dedicated MiddlewareError --- CHANGELOG.md | 2 +- lib/api/src/lib.rs | 3 +- lib/compiler/src/error.rs | 56 +++++++++++++++++++++++ lib/compiler/src/lib.rs | 4 +- lib/compiler/src/translator/middleware.rs | 4 +- lib/middlewares/src/metering.rs | 4 +- tests/compilers/middlewares.rs | 4 +- 7 files changed, 68 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3dc8a2aa45..5cb38691658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ * [#1955](https://github.com/wasmerio/wasmer/pull/1955) Set `jit` as a default feature of the `wasmer-wasm-c-api` crate * [#1944](https://github.com/wasmerio/wasmer/pull/1944) Require `WasmerEnv` to be `Send + Sync` even in dynamic functions. * [#1963](https://github.com/wasmerio/wasmer/pull/1963) Removed `to_wasm_error` in favour of `impl From for WasmError` -* [#1962](https://github.com/wasmerio/wasmer/pull/1962) Replace `wasmparser::Result` with `wasmer::WasmResult` in middleware +* [#1962](https://github.com/wasmerio/wasmer/pull/1962) Replace `wasmparser::Result<()>` with `Result<(), MiddlewareError>` in middleware, allowing implementors to return errors in `FunctionMiddleware::feed` ### Fixed diff --git a/lib/api/src/lib.rs b/lib/api/src/lib.rs index 738925df8b2..b35fe6a0460 100644 --- a/lib/api/src/lib.rs +++ b/lib/api/src/lib.rs @@ -304,7 +304,8 @@ pub use crate::utils::is_wasm; pub use target_lexicon::{Architecture, CallingConvention, OperatingSystem, Triple, HOST}; #[cfg(feature = "compiler")] pub use wasmer_compiler::{ - wasmparser, CompilerConfig, FunctionMiddleware, MiddlewareReaderState, ModuleMiddleware, + wasmparser, CompilerConfig, FunctionMiddleware, MiddlewareError, MiddlewareReaderState, + ModuleMiddleware, }; pub use wasmer_compiler::{ CompileError, CpuFeature, Features, ParseCpuFeatureError, Target, WasmError, WasmResult, diff --git a/lib/compiler/src/error.rs b/lib/compiler/src/error.rs index 0d554c15e8c..cf107ad6363 100644 --- a/lib/compiler/src/error.rs +++ b/lib/compiler/src/error.rs @@ -48,6 +48,27 @@ impl From for CompileError { } } +/// A error in the middleware. +#[derive(Debug)] +#[cfg_attr(feature = "std", derive(Error))] +#[cfg_attr(feature = "std", error("Error in middleware {name}: {message}"))] +pub struct MiddlewareError { + /// The name of the middleware where the error was created + pub name: String, + /// The error message + pub message: String, +} + +impl MiddlewareError { + /// Create a new `MiddlewareError` + pub fn new, B: Into>(name: A, message: B) -> Self { + Self { + name: name.into(), + message: message.into(), + } + } +} + /// A WebAssembly translation error. /// /// When a WebAssembly function can't be translated, one of these error codes will be returned @@ -80,11 +101,21 @@ pub enum WasmError { #[cfg_attr(feature = "std", error("Implementation limit exceeded"))] ImplLimitExceeded, + /// An error from the middleware error. + #[cfg_attr(feature = "std", error("{0}"))] + Middleware(MiddlewareError), + /// A generic error. #[cfg_attr(feature = "std", error("{0}"))] Generic(String), } +impl From for WasmError { + fn from(original: MiddlewareError) -> Self { + Self::Middleware(original) + } +} + /// The error that can happen while parsing a `str` /// to retrieve a [`CpuFeature`](crate::target::CpuFeature). #[derive(Debug)] @@ -97,3 +128,28 @@ pub enum ParseCpuFeatureError { /// A convenient alias for a `Result` that uses `WasmError` as the error type. pub type WasmResult = Result; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn middleware_error_can_be_created() { + let msg = String::from("Something went wrong"); + let error = MiddlewareError::new("manipulator3000", msg); + assert_eq!(error.name, "manipulator3000"); + assert_eq!(error.message, "Something went wrong"); + } + + #[test] + fn middleware_error_be_converted_to_wasm_error() { + let error = WasmError::from(MiddlewareError::new("manipulator3000", "foo")); + match error { + WasmError::Middleware(MiddlewareError { name, message }) => { + assert_eq!(name, "manipulator3000"); + assert_eq!(message, "foo"); + } + err => panic!("Unexpected error: {:?}", err), + } + } +} diff --git a/lib/compiler/src/lib.rs b/lib/compiler/src/lib.rs index 090a484f8d2..e814c6bdf4b 100644 --- a/lib/compiler/src/lib.rs +++ b/lib/compiler/src/lib.rs @@ -69,7 +69,9 @@ mod sourceloc; pub use crate::address_map::{FunctionAddressMap, InstructionAddressMap}; #[cfg(feature = "translator")] pub use crate::compiler::{Compiler, CompilerConfig, Symbol, SymbolRegistry}; -pub use crate::error::{CompileError, ParseCpuFeatureError, WasmError, WasmResult}; +pub use crate::error::{ + CompileError, MiddlewareError, ParseCpuFeatureError, WasmError, WasmResult, +}; pub use crate::function::{ Compilation, CompiledFunction, CompiledFunctionFrameInfo, CustomSections, Dwarf, FunctionBody, Functions, diff --git a/lib/compiler/src/translator/middleware.rs b/lib/compiler/src/translator/middleware.rs index e7f7f300aa5..5efcc562f9b 100644 --- a/lib/compiler/src/translator/middleware.rs +++ b/lib/compiler/src/translator/middleware.rs @@ -9,7 +9,7 @@ use wasmer_types::LocalFunctionIndex; use wasmer_vm::ModuleInfo; use wasmparser::{BinaryReader, Operator, Type}; -use crate::error::WasmResult; +use crate::error::{MiddlewareError, WasmResult}; /// A shared builder for function middlewares. pub trait ModuleMiddleware: Debug + Send + Sync { @@ -34,7 +34,7 @@ pub trait FunctionMiddleware: Debug { &mut self, operator: Operator<'a>, state: &mut MiddlewareReaderState<'a>, - ) -> WasmResult<()> { + ) -> Result<(), MiddlewareError> { state.push_operator(operator); Ok(()) } diff --git a/lib/middlewares/src/metering.rs b/lib/middlewares/src/metering.rs index 130c39961e5..f02ac1348ba 100644 --- a/lib/middlewares/src/metering.rs +++ b/lib/middlewares/src/metering.rs @@ -7,7 +7,7 @@ use std::sync::Mutex; use wasmer::wasmparser::{Operator, Type as WpType, TypeOrFuncType as WpTypeOrFuncType}; use wasmer::{ ExportIndex, FunctionMiddleware, GlobalInit, GlobalType, Instance, LocalFunctionIndex, - MiddlewareReaderState, ModuleMiddleware, Mutability, Type, WasmResult, + MiddlewareError, MiddlewareReaderState, ModuleMiddleware, Mutability, Type, }; use wasmer_types::GlobalIndex; use wasmer_vm::ModuleInfo; @@ -116,7 +116,7 @@ impl u64 + Copy + Clone + Send + Sync> FunctionMiddleware &mut self, operator: Operator<'a>, state: &mut MiddlewareReaderState<'a>, - ) -> WasmResult<()> { + ) -> Result<(), MiddlewareError> { // Get the cost of the current operator, and add it to the accumulator. // This needs to be done before the metering logic, to prevent operators like `Call` from escaping metering in some // corner cases. diff --git a/tests/compilers/middlewares.rs b/tests/compilers/middlewares.rs index 4564cf50ff5..3a9e5516656 100644 --- a/tests/compilers/middlewares.rs +++ b/tests/compilers/middlewares.rs @@ -28,7 +28,7 @@ impl FunctionMiddleware for Add2Mul { &mut self, operator: Operator<'a>, state: &mut MiddlewareReaderState<'a>, - ) -> WasmResult<()> { + ) -> Result<(), MiddlewareError> { match operator { Operator::I32Add => { state.push_operator(Operator::I32Mul); @@ -66,7 +66,7 @@ impl FunctionMiddleware for Fusion { &mut self, operator: Operator<'a>, state: &mut MiddlewareReaderState<'a>, - ) -> WasmResult<()> { + ) -> Result<(), MiddlewareError> { match (operator, self.state) { (Operator::I32Add, 0) => { self.state = 1;