From 89af5dc1072ca1e5cd23f5a2f1a95326a2bbd45b Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Sun, 26 Apr 2020 12:05:12 -0700 Subject: [PATCH] Fix new RuntimeError implementation for the Singlepass backend --- lib/clif-backend/src/signal/mod.rs | 6 +-- lib/llvm-backend/src/backend.rs | 8 ++-- lib/runtime-core/src/error.rs | 54 +++++++++++++---------- lib/runtime-core/src/fault.rs | 35 ++++++++------- lib/runtime-core/src/typed_func.rs | 2 +- lib/singlepass-backend/src/codegen_x64.rs | 6 +-- tests/high_level_api.rs | 11 +++++ 7 files changed, 73 insertions(+), 49 deletions(-) diff --git a/lib/clif-backend/src/signal/mod.rs b/lib/clif-backend/src/signal/mod.rs index cf3d1eb02..19e71c700 100644 --- a/lib/clif-backend/src/signal/mod.rs +++ b/lib/clif-backend/src/signal/mod.rs @@ -7,7 +7,7 @@ use libc::c_void; use std::{cell::Cell, ptr::NonNull, sync::Arc}; use wasmer_runtime_core::{ backend::RunnableModule, - error::{InvokeError, RuntimeError}, + error::RuntimeError, module::ModuleInfo, typed_func::{Trampoline, Wasm}, types::{LocalFuncIndex, SigIndex}, @@ -62,7 +62,7 @@ impl RunnableModule for Caller { func: NonNull, args: *const u64, rets: *mut u64, - error_out: *mut Option, + error_out: *mut Option, invoke_env: Option>, ) -> bool { let handler_data = &*invoke_env.unwrap().cast().as_ptr(); @@ -82,7 +82,7 @@ impl RunnableModule for Caller { // probably makes the most sense to actually do a translation here to a // a generic type defined in runtime-core // TODO: figure out _this_ error return story - *error_out = Some(err); + *error_out = Some(err.into()); false } Ok(()) => true, diff --git a/lib/llvm-backend/src/backend.rs b/lib/llvm-backend/src/backend.rs index 18d5a8ce3..ef3881112 100644 --- a/lib/llvm-backend/src/backend.rs +++ b/lib/llvm-backend/src/backend.rs @@ -66,7 +66,7 @@ extern "C" { params: *const u64, results: *mut u64, trap_out: *mut i32, - error_out: *mut Option, + error_out: *mut Option, invoke_env: Option>, ) -> bool; } @@ -79,7 +79,7 @@ unsafe extern "C" fn invoke_trampoline( func_ptr: NonNull, params: *const u64, results: *mut u64, - error_out: *mut Option, + error_out: *mut Option, invoke_env: Option>, ) -> bool { let mut trap_out: i32 = -1; @@ -105,11 +105,11 @@ unsafe extern "C" fn invoke_trampoline( 5 => ExceptionCode::MisalignedAtomicAccess, _ => return ret, }; - Some(InvokeError::TrapCode { + Some(RuntimeError::InvokeError(InvokeError::TrapCode { code: exception_code, // TODO: srcloc: 0, - }) + })) }; } ret diff --git a/lib/runtime-core/src/error.rs b/lib/runtime-core/src/error.rs index 44c12ad6e..c1ffc5315 100644 --- a/lib/runtime-core/src/error.rs +++ b/lib/runtime-core/src/error.rs @@ -213,27 +213,6 @@ impl std::error::Error for RuntimeError {} */ -/// An `InternalError` is an error that happened inside of Wasmer and is a -/// catch-all for errors that would otherwise be returned as -/// `RuntimeError(Box::new())`. -/// -/// This type provides greater visibility into the kinds of things that may fail -/// and improves the ability of users to handle them, though these errors may be -/// extremely rare and impossible to handle. -#[derive(Debug)] -pub enum RuntimeError { - /// When an invoke returns an error - InvokeError(InvokeError), - /// A metering triggered error value. - /// - /// An error of this type indicates that it was returned by the metering system. - Metering(Box), - /// A user triggered error value. - /// - /// An error returned from a host function. - User(Box), -} - /// TODO: #[derive(Debug)] pub enum InvokeError { @@ -278,14 +257,39 @@ impl From for RuntimeError { } } +//impl std::error::Error for InvokeError {} + +/// An `InternalError` is an error that happened inside of Wasmer and is a +/// catch-all for errors that would otherwise be returned as +/// `RuntimeError(Box::new())`. +/// +/// This type provides greater visibility into the kinds of things that may fail +/// and improves the ability of users to handle them, though these errors may be +/// extremely rare and impossible to handle. +#[derive(Debug)] +pub enum RuntimeError { + /// When an invoke returns an error + InvokeError(InvokeError), + /// A metering triggered error value. + /// + /// An error of this type indicates that it was returned by the metering system. + Metering(Box), + /// A frozen state of Wasm used to pause and resume execution. Not strictly an + /// "error", but this happens while executing and therefore is a `RuntimeError` + /// from the persective of the caller that expected the code to fully execute. + InstanceImage(Box), + /// A user triggered error value. + /// + /// An error returned from a host function. + User(Box), +} + impl PartialEq for RuntimeError { fn eq(&self, _other: &RuntimeError) -> bool { false } } -//impl std::error::Error for InvokeError {} - impl std::error::Error for RuntimeError {} impl std::fmt::Display for RuntimeError { @@ -294,6 +298,10 @@ impl std::fmt::Display for RuntimeError { // TODO: update invoke error formatting RuntimeError::InvokeError(_) => write!(f, "Error when calling invoke"), RuntimeError::Metering(_) => write!(f, "unknown metering error type"), + RuntimeError::InstanceImage(_) => write!( + f, + "Execution interrupted by a suspend signal: instance image returned" + ), RuntimeError::User(user_error) => { write!(f, "User supplied error: ")?; if let Some(s) = user_error.downcast_ref::() { diff --git a/lib/runtime-core/src/fault.rs b/lib/runtime-core/src/fault.rs index 44cae2683..4d7d919ac 100644 --- a/lib/runtime-core/src/fault.rs +++ b/lib/runtime-core/src/fault.rs @@ -30,7 +30,7 @@ pub mod raw { use crate::codegen::{BreakpointInfo, BreakpointMap}; use crate::error::{InvokeError, RuntimeError}; use crate::state::x64::{build_instance_image, read_stack, X64Register, GPR}; -use crate::state::{CodeVersion, ExecutionStateImage, InstanceImage}; +use crate::state::{CodeVersion, ExecutionStateImage}; use crate::vm; use libc::{mmap, mprotect, siginfo_t, MAP_ANON, MAP_PRIVATE, PROT_NONE, PROT_READ, PROT_WRITE}; use nix::sys::signal::{ @@ -61,7 +61,7 @@ type SetJmpBuffer = [i32; SETJMP_BUFFER_LEN]; struct UnwindInfo { jmpbuf: SetJmpBuffer, // in breakpoints: Option, - payload: Option, // out + payload: Option>, // out } /// A store for boundary register preservation. @@ -195,7 +195,7 @@ pub unsafe fn catch_unsafe_unwind R>( // error let ret = (*unwind).as_mut().unwrap().payload.take().unwrap(); *unwind = old; - Err(ret) + Err(*ret) } else { let ret = f(); // implicit control flow to the error case... @@ -205,7 +205,7 @@ pub unsafe fn catch_unsafe_unwind R>( } /// Begins an unsafe unwind. -pub unsafe fn begin_unsafe_unwind(e: RuntimeError) -> ! { +pub unsafe fn begin_unsafe_unwind(e: Box) -> ! { let unwind = UNWIND.with(|x| x.get()); let inner = (*unwind) .as_mut() @@ -279,7 +279,11 @@ extern "C" fn signal_trap_handler( static ARCH: Architecture = Architecture::Aarch64; let mut should_unwind = false; - let mut unwind_result: Option> = None; + let mut unwind_result: Option> = None; + let get_unwind_result = |uw_result: Option>| -> Box { + uw_result + .unwrap_or_else(|| Box::new(RuntimeError::InvokeError(InvokeError::FailedWithNoError))) + }; unsafe { let fault = get_fault_info(siginfo as _, ucontext); @@ -313,7 +317,7 @@ extern "C" fn signal_trap_handler( if let Some(Ok(())) = out { } else if let Some(Err(e)) = out { should_unwind = true; - unwind_result = Some(Err(e)); + unwind_result = Some(Box::new(e)); } } } @@ -328,7 +332,7 @@ extern "C" fn signal_trap_handler( }) }); if should_unwind { - begin_unsafe_unwind(unwind_result.unwrap().unwrap_err()); + begin_unsafe_unwind(get_unwind_result(unwind_result)); } if early_return { return; @@ -357,7 +361,7 @@ extern "C" fn signal_trap_handler( return false; } Some(Err(e)) => { - unwind_result = Some(Err(e)); + unwind_result = Some(Box::new(e)); return true; } None => {} @@ -389,7 +393,7 @@ extern "C" fn signal_trap_handler( if is_suspend_signal { // If this is a suspend signal, we parse the runtime state and return the resulting image. let image = build_instance_image(ctx, es_image); - unwind_result = Some(Ok(image)); + unwind_result = Some(Box::new(RuntimeError::InstanceImage(Box::new(image)))); } else { // Otherwise, this is a real exception and we just throw it to the caller. if !es_image.frames.is_empty() { @@ -417,11 +421,12 @@ extern "C" fn signal_trap_handler( None }); if let Some(code) = exc_code { - unwind_result = Some(Err(RuntimeError::InvokeError(InvokeError::TrapCode { - code, - // TODO: - srcloc: 0, - }))); + unwind_result = + Some(Box::new(RuntimeError::InvokeError(InvokeError::TrapCode { + code, + // TODO: + srcloc: 0, + }))); } } @@ -429,7 +434,7 @@ extern "C" fn signal_trap_handler( }); if should_unwind { - begin_unsafe_unwind(unwind_result.unwrap().unwrap_err()); + begin_unsafe_unwind(get_unwind_result(unwind_result)); } } } diff --git a/lib/runtime-core/src/typed_func.rs b/lib/runtime-core/src/typed_func.rs index b4db0fbac..5cef19ca4 100644 --- a/lib/runtime-core/src/typed_func.rs +++ b/lib/runtime-core/src/typed_func.rs @@ -37,7 +37,7 @@ pub type Invoke = unsafe extern "C" fn( func: NonNull, args: *const u64, rets: *mut u64, - error_out: *mut Option, + error_out: *mut Option, extra: Option>, ) -> bool; diff --git a/lib/singlepass-backend/src/codegen_x64.rs b/lib/singlepass-backend/src/codegen_x64.rs index 63fd95dec..bc666d1ff 100644 --- a/lib/singlepass-backend/src/codegen_x64.rs +++ b/lib/singlepass-backend/src/codegen_x64.rs @@ -507,7 +507,7 @@ impl RunnableModule for X64ExecutionContext { func: NonNull, args: *const u64, rets: *mut u64, - error_out: *mut Option, + error_out: *mut Option, num_params_plus_one: Option>, ) -> bool { let rm: &Box = &(&*(*ctx).module).runnable_module; @@ -655,7 +655,7 @@ impl RunnableModule for X64ExecutionContext { true } Err(err) => { - *error_out = Some(InvokeError::Breakpoint(Box::new(err))); + *error_out = Some(InvokeError::Breakpoint(Box::new(err)).into()); false } }; @@ -681,7 +681,7 @@ impl RunnableModule for X64ExecutionContext { } unsafe fn do_early_trap(&self, data: RuntimeError) -> ! { - fault::begin_unsafe_unwind(data); + fault::begin_unsafe_unwind(Box::new(data)); } fn get_code(&self) -> Option<&[u8]> { diff --git a/tests/high_level_api.rs b/tests/high_level_api.rs index 5d9ae70fb..a7b71d9b3 100644 --- a/tests/high_level_api.rs +++ b/tests/high_level_api.rs @@ -268,7 +268,18 @@ wasmer_backends! { let result = foo.call(); + dbg!(&result); + if let Err(e) = &result { + dbg!(&e); + eprintln!("{}", &e); + } + match &result { + Err(RuntimeError::User(e)) => { dbg!("USER", &e); } , + Err(e) => {dbg!("GENERIC",&e); } , + _ => { dbg!("OKaY!"); }, + } if let Err(RuntimeError::User(e)) = result { + dbg!("WAT"); let exit_code = e.downcast::().unwrap(); assert_eq!(exit_code.code, 42); } else {