From ed826cb389b17273002e729160bf076213b7e2f2 Mon Sep 17 00:00:00 2001 From: losfair Date: Tue, 18 Feb 2020 02:35:19 +0800 Subject: [PATCH 1/2] Cleanup various FIXMEs and remove protect_unix. --- lib/singlepass-backend/src/codegen_x64.rs | 71 ++++++++++------------ lib/singlepass-backend/src/lib.rs | 1 - lib/singlepass-backend/src/protect_unix.rs | 49 --------------- 3 files changed, 31 insertions(+), 90 deletions(-) delete mode 100644 lib/singlepass-backend/src/protect_unix.rs diff --git a/lib/singlepass-backend/src/codegen_x64.rs b/lib/singlepass-backend/src/codegen_x64.rs index 4c15a688c26..dbf20eed636 100644 --- a/lib/singlepass-backend/src/codegen_x64.rs +++ b/lib/singlepass-backend/src/codegen_x64.rs @@ -3,7 +3,6 @@ use crate::emitter_x64::*; use crate::machine::*; -use crate::protect_unix; #[cfg(target_arch = "aarch64")] use dynasmrt::aarch64::Assembler; #[cfg(target_arch = "x86_64")] @@ -28,7 +27,7 @@ use wasmer_runtime_core::{ }, cache::{Artifact, Error as CacheError}, codegen::*, - fault::raw::register_preservation_trampoline, + fault::{self, raw::register_preservation_trampoline}, loader::CodeMemory, memory::MemoryType, module::{ModuleInfo, ModuleInner}, @@ -369,6 +368,11 @@ impl RunnableModule for X64ExecutionContext { } fn get_trampoline(&self, _: &ModuleInfo, sig_index: SigIndex) -> Option { + // Correctly unwinding from `catch_unsafe_unwind` on hardware exceptions depends + // on the signal handlers being installed. Here we call `ensure_sighandler` "statically" + // outside `invoke()`. + fault::ensure_sighandler(); + unsafe extern "C" fn invoke( _trampoline: Trampoline, ctx: *mut vm::Ctx, @@ -383,8 +387,9 @@ impl RunnableModule for X64ExecutionContext { let args = slice::from_raw_parts(args, num_params_plus_one.unwrap().as_ptr() as usize - 1); - let ret = match protect_unix::call_protected( + let ret = match fault::catch_unsafe_unwind( || { + // Puts the arguments onto the stack and calls Wasm entry. #[cfg(target_arch = "x86_64")] { let args_reverse: SmallVec<[u64; 8]> = args.iter().cloned().rev().collect(); @@ -395,6 +400,9 @@ impl RunnableModule for X64ExecutionContext { func.as_ptr(), ) } + + // FIXME: Currently we are doing a hack here to convert between native aarch64 and + // "emulated" x86 ABIs. Ideally, this should be done using handwritten assembly. #[cfg(target_arch = "aarch64")] { struct CallCtx<'a> { @@ -519,7 +527,7 @@ impl RunnableModule for X64ExecutionContext { true } Err(err) => { - *error_out = Some(err.0); + *error_out = Some(err); false } }; @@ -545,8 +553,7 @@ impl RunnableModule for X64ExecutionContext { } unsafe fn do_early_trap(&self, data: Box) -> ! { - protect_unix::TRAP_EARLY_DATA.with(|x| x.set(Some(data))); - protect_unix::trigger_trap(); + fault::begin_unsafe_unwind(data); } fn get_code(&self) -> Option<&[u8]> { @@ -1686,14 +1693,11 @@ impl X64FunctionCode { Location::GPR(GPR::RSP), ); - // FIXME: Possible dynasm bug. This is a workaround. - // Using RSP as the source/destination operand of a `mov` instruction produces invalid code. - a.emit_mov(Size::S64, Location::GPR(GPR::RSP), Location::GPR(GPR::RCX)); for (i, r) in used_xmms.iter().enumerate() { a.emit_mov( Size::S64, Location::XMM(*r), - Location::Memory(GPR::RCX, (i * 8) as i32), + Location::Memory(GPR::RSP, (i * 8) as i32), ); } for r in used_xmms.iter().rev() { @@ -1771,37 +1775,26 @@ impl X64FunctionCode { } } match *param { - // Dynasm bug: RSP in memory operand does not work - Location::Imm64(_) | Location::XMM(_) => { - a.emit_mov( - Size::S64, - Location::GPR(GPR::RAX), - Location::XMM(XMM::XMM0), - ); - a.emit_mov( - Size::S64, - Location::GPR(GPR::RCX), - Location::XMM(XMM::XMM1), - ); - a.emit_sub(Size::S64, Location::Imm32(8), Location::GPR(GPR::RSP)); - a.emit_mov(Size::S64, Location::GPR(GPR::RSP), Location::GPR(GPR::RCX)); - a.emit_mov(Size::S64, *param, Location::GPR(GPR::RAX)); - a.emit_mov( - Size::S64, - Location::GPR(GPR::RAX), - Location::Memory(GPR::RCX, 0), - ); - a.emit_mov( - Size::S64, - Location::XMM(XMM::XMM0), - Location::GPR(GPR::RAX), - ); + Location::Imm64(_) => { + // Dummy value slot to be filled with `mov`. + a.emit_push(Size::S64, Location::GPR(GPR::RAX)); + + // Use R10 as the temporary register here, since it is callee-saved and not + // used by the callback `cb`. + a.emit_mov(Size::S64, *param, Location::GPR(GPR::R10)); a.emit_mov( Size::S64, - Location::XMM(XMM::XMM1), - Location::GPR(GPR::RCX), + Location::GPR(GPR::R10), + Location::Memory(GPR::RSP, 0), ); } + Location::XMM(_) => { + // Dummy value slot to be filled with `mov`. + a.emit_push(Size::S64, Location::GPR(GPR::RAX)); + + // XMM registers can be directly stored to memory. + a.emit_mov(Size::S64, *param, Location::Memory(GPR::RSP, 0)); + } _ => a.emit_push(Size::S64, *param), } } @@ -1873,12 +1866,10 @@ impl X64FunctionCode { // Restore XMMs. if used_xmms.len() > 0 { - // FIXME: Possible dynasm bug. This is a workaround. - a.emit_mov(Size::S64, Location::GPR(GPR::RSP), Location::GPR(GPR::RDX)); for (i, r) in used_xmms.iter().enumerate() { a.emit_mov( Size::S64, - Location::Memory(GPR::RDX, (i * 8) as i32), + Location::Memory(GPR::RSP, (i * 8) as i32), Location::XMM(*r), ); } diff --git a/lib/singlepass-backend/src/lib.rs b/lib/singlepass-backend/src/lib.rs index 2afdaa7f57d..98339cca304 100644 --- a/lib/singlepass-backend/src/lib.rs +++ b/lib/singlepass-backend/src/lib.rs @@ -40,7 +40,6 @@ extern crate smallvec; mod codegen_x64; mod emitter_x64; mod machine; -pub mod protect_unix; #[cfg(target_arch = "aarch64")] mod translator_aarch64; diff --git a/lib/singlepass-backend/src/protect_unix.rs b/lib/singlepass-backend/src/protect_unix.rs deleted file mode 100644 index 96f10748909..00000000000 --- a/lib/singlepass-backend/src/protect_unix.rs +++ /dev/null @@ -1,49 +0,0 @@ -//! Installing signal handlers allows us to handle traps and out-of-bounds memory -//! accesses that occur when runniing WebAssembly. -//! -//! This code is inspired by: https://github.com/pepyakin/wasmtime/commit/625a2b6c0815b21996e111da51b9664feb174622 -//! -//! When a WebAssembly module triggers any traps, we perform recovery here. -//! -//! This module uses TLS (thread-local storage) to track recovery information. Since the four signals we're handling -//! are very special, the async signal unsafety of Rust's TLS implementation generally does not affect the correctness here -//! unless you have memory unsafety elsewhere in your code. -//! -use std::any::Any; -use std::cell::Cell; -use wasmer_runtime_core::codegen::BreakpointMap; -use wasmer_runtime_core::fault::{begin_unsafe_unwind, catch_unsafe_unwind, ensure_sighandler}; - -thread_local! { - pub static TRAP_EARLY_DATA: Cell>> = Cell::new(None); -} - -pub unsafe fn trigger_trap() -> ! { - begin_unsafe_unwind(Box::new(())); -} - -pub struct CallProtError(pub Box); - -pub fn call_protected( - f: impl FnOnce() -> T, - breakpoints: Option, -) -> Result { - ensure_sighandler(); - unsafe { - let ret = catch_unsafe_unwind(|| f(), breakpoints); - match ret { - Ok(x) => Ok(x), - Err(e) => Err(CallProtError( - if let Some(data) = TRAP_EARLY_DATA.with(|cell| cell.replace(None)) { - data - } else { - e - }, - )), - } - } -} - -pub unsafe fn throw(payload: Box) -> ! { - begin_unsafe_unwind(payload); -} From b1e2a7fb4dcb27234995f22205ab5776ce51dca1 Mon Sep 17 00:00:00 2001 From: losfair Date: Tue, 18 Feb 2020 02:38:33 +0800 Subject: [PATCH 2/2] Update changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ef0dd4d811..ae21d18b040 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## **[Unreleased]** +- [#1228](https://github.com/wasmerio/wasmer/pull/1228) Singlepass cleanup: Resolve several FIXMEs and remove protect_unix. - [#1218](https://github.com/wasmerio/wasmer/pull/1218) Enable Cranelift verifier in debug mode. Fix bug with table indices being the wrong type. - [#787](https://github.com/wasmerio/wasmer/pull/787) New crate `wasmer-interface-types` to implement WebAssembly Interface Types. - [#1213](https://github.com/wasmerio/wasmer/pull/1213) Fixed WASI `fdstat` to detect `isatty` properly.