diff --git a/CHANGELOG.md b/CHANGELOG.md index 282456c85..3e6792196 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## **[Unreleased]** -- [#1401](https://github.com/wasmerio/wasmer/pull/1401) Make breaking change to `RuntimeError`: `RuntimeError` is now more explicit about its possible error values allowing for better insight into why a call may have failed. +- [#1401](https://github.com/wasmerio/wasmer/pull/1401) Make breaking change to `RuntimeError`: `RuntimeError` is now more explicit about its possible error values allowing for better insight into why a call into Wasm failed. - [#1382](https://github.com/wasmerio/wasmer/pull/1382) Refactored test infranstructure (part 2) - [#1380](https://github.com/wasmerio/wasmer/pull/1380) Refactored test infranstructure (part 1) - [#1357](https://github.com/wasmerio/wasmer/pull/1357) Refactored bin commands into separate files diff --git a/examples/callback-guest/callback-guest.rs b/examples/callback-guest/callback-guest.rs index aec238ca1..129ad05c8 100644 --- a/examples/callback-guest/callback-guest.rs +++ b/examples/callback-guest/callback-guest.rs @@ -4,6 +4,7 @@ extern "C" { fn host_callback() -> u32; } +#[no_mangle] fn test_callback() -> u32 { 42 } diff --git a/examples/callback-guest/callback-guest.wasm b/examples/callback-guest/callback-guest.wasm index 3595bcd59..61b247c8e 100755 Binary files a/examples/callback-guest/callback-guest.wasm and b/examples/callback-guest/callback-guest.wasm differ diff --git a/lib/clif-backend/src/signal/mod.rs b/lib/clif-backend/src/signal/mod.rs index 19e71c700..c8f78a467 100644 --- a/lib/clif-backend/src/signal/mod.rs +++ b/lib/clif-backend/src/signal/mod.rs @@ -79,9 +79,6 @@ impl RunnableModule for Caller { match res { Err(err) => { - // 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.into()); false } diff --git a/lib/clif-backend/src/signal/windows.rs b/lib/clif-backend/src/signal/windows.rs index 1228db9a7..46380bbdc 100644 --- a/lib/clif-backend/src/signal/windows.rs +++ b/lib/clif-backend/src/signal/windows.rs @@ -48,9 +48,9 @@ fn get_signal_name(code: DWORD) -> &'static str { EXCEPTION_BREAKPOINT => "breakpoint", EXCEPTION_SINGLE_STEP => "single step", EXCEPTION_ARRAY_BOUNDS_EXCEEDED => "array bounds exceeded", - EXCEPTION_INT_DIVIDE_BY_ZERO => "int div by zero", - EXCEPTION_INT_OVERFLOW => "int overflow", - EXCEPTION_PRIV_INSTRUCTION => "priv instruction", + EXCEPTION_INT_DIVIDE_BY_ZERO => "integer division by zero", + EXCEPTION_INT_OVERFLOW => "integer overflow", + EXCEPTION_PRIV_INSTRUCTION => "privileged instruction", EXCEPTION_IN_PAGE_ERROR => "in page error", EXCEPTION_NONCONTINUABLE_EXCEPTION => "non continuable exception", EXCEPTION_STACK_OVERFLOW => "stack overflow", diff --git a/lib/llvm-backend/cpp/object_loader.hh b/lib/llvm-backend/cpp/object_loader.hh index a20a93a48..0d3e573be 100644 --- a/lib/llvm-backend/cpp/object_loader.hh +++ b/lib/llvm-backend/cpp/object_loader.hh @@ -49,7 +49,7 @@ typedef struct { visit_fde_t visit_fde; } callbacks_t; -using runtime_error_t = void*; +typedef struct runtime_error_t_privdecl runtime_error_t; enum WasmTrapType { Unreachable = 0, @@ -63,7 +63,8 @@ enum WasmTrapType { extern "C" void callback_trampoline(void *, void *); -extern "C" void copy_runtime_error(runtime_error_t src, runtime_error_t dst); +extern "C" void copy_runtime_error(runtime_error_t *src, runtime_error_t *dst); +extern "C" void free_runtime_error_without_drop(runtime_error_t*); struct MemoryManager : llvm::RuntimeDyld::MemoryManager { public: @@ -121,7 +122,7 @@ private: struct WasmErrorSink { WasmTrapType *trap_out; - runtime_error_t user_error; + runtime_error_t *user_error; }; struct WasmException : std::exception { @@ -150,7 +151,13 @@ public: struct UserException : UncatchableException { public: UserException(size_t data) { - error_data = reinterpret_cast(data); + error_data = reinterpret_cast(data); + } + + UserException(const UserException &) = delete; + + ~UserException() { + free_runtime_error_without_drop(error_data); } virtual std::string description() const noexcept override { @@ -158,7 +165,7 @@ public: } // The pointer to `Option`. - runtime_error_t error_data; + runtime_error_t *error_data; virtual void write_error(WasmErrorSink &out) const noexcept override { copy_runtime_error(error_data, out.user_error); @@ -290,7 +297,7 @@ void module_delete(WasmModule *module) { delete module; } bool cxx_invoke_trampoline(trampoline_t trampoline, void *ctx, void *func, void *params, void *results, WasmTrapType *trap_out, - runtime_error_t user_error, void *invoke_env) noexcept { + runtime_error_t *user_error, void *invoke_env) noexcept { try { catch_unwind([trampoline, ctx, func, params, results]() { trampoline(ctx, func, params, results); diff --git a/lib/llvm-backend/src/backend.rs b/lib/llvm-backend/src/backend.rs index b1540712c..0b582a449 100644 --- a/lib/llvm-backend/src/backend.rs +++ b/lib/llvm-backend/src/backend.rs @@ -11,6 +11,7 @@ use inkwell::{ }; use libc::c_char; use std::{ + alloc, cell::RefCell, ffi::{c_void, CString}, mem, @@ -69,6 +70,22 @@ extern "C" { ) -> bool; } +/// Unsafe copy of `RuntimeError`. For use from C++. +/// +/// This copy is unsafe because `RuntimeError` contains non-`Clone` types such as +/// `Box`. +/// +/// This function should only be used when the ownership can be manually tracked. +/// +/// For example, this is safe* when used indirectly through the C++ API with a pointer +/// from `do_early_trap` because `do_early_trap` fully owns the `RuntimeError` and +/// creates and leaks the `Box` itself. +/// +/// *: it is only safe provided the following invariants are upheld: +/// 1. The versions of memory that these 2 pointers point to is only dropped once; +/// the memory itself can be freed provided the inner type is not dropped. +/// 2. The duplicated memory is not brought back into Rust to violate standard +/// mutable aliasing/ownership rules. #[no_mangle] pub unsafe extern "C" fn copy_runtime_error( src: *mut Option, @@ -79,6 +96,18 @@ pub unsafe extern "C" fn copy_runtime_error( ptr::copy::>(src, dst, 1); } +/// Frees the memory of a `Option` without calling its destructor. +/// For use from C++ to safely clean up after `copy_runtime_error`. +#[no_mangle] +pub unsafe extern "C" fn free_runtime_error_without_drop(rte: *mut Option) { + let rte_layout = alloc::Layout::from_size_align( + mem::size_of::>(), + mem::align_of::>(), + ) + .expect("layout of `Option`"); + alloc::dealloc(rte as *mut u8, rte_layout) +} + /// `invoke_trampoline` is a wrapper around `cxx_invoke_trampoline`, for fixing up the obsoleted /// `trap_out` in the C++ part. unsafe extern "C" fn invoke_trampoline( diff --git a/lib/runtime-core/src/error.rs b/lib/runtime-core/src/error.rs index 4b05fd351..512552433 100644 --- a/lib/runtime-core/src/error.rs +++ b/lib/runtime-core/src/error.rs @@ -181,7 +181,7 @@ pub enum InvokeError { FailedWithNoError, /// Indicates that a trap occurred that is not known to Wasmer. UnknownTrap { - /// The address that the trap occured at. + /// The address that the trap occurred at. address: usize, /// The name of the signal. signal: &'static str, @@ -251,7 +251,7 @@ impl std::fmt::Display for InvokeError { /// extremely rare and impossible to handle. #[derive(Debug)] pub enum RuntimeError { - /// When an invoke returns an error + /// An error relating to the invocation of a Wasm function. InvokeError(InvokeError), /// A metering triggered error value. ///