Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Singlepass cleanup: Resolve several FIXMEs and remove protect_unix. #1228

Merged
merged 2 commits into from
Feb 17, 2020
Merged

Singlepass cleanup: Resolve several FIXMEs and remove protect_unix. #1228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed826cb
Cleanup various FIXMEs and remove protect_unix.
losfair Feb 17, 2020
b1e2a7f
Update changelog.
losfair Feb 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
71 changes: 31 additions & 40 deletions lib/singlepass-backend/src/codegen_x64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand All @@ -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},
Expand Down Expand Up @@ -369,6 +368,11 @@ impl RunnableModule for X64ExecutionContext {
}

fn get_trampoline(&self, _: &ModuleInfo, sig_index: SigIndex) -> Option<Wasm> {
// 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,
Expand All @@ -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();
Expand All @@ -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> {
Expand Down Expand Up @@ -519,7 +527,7 @@ impl RunnableModule for X64ExecutionContext {
true
}
Err(err) => {
*error_out = Some(err.0);
*error_out = Some(err);
false
}
};
Expand All @@ -545,8 +553,7 @@ impl RunnableModule for X64ExecutionContext {
}

unsafe fn do_early_trap(&self, data: Box<dyn Any + Send>) -> ! {
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]> {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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),
);
}
Expand Down
1 change: 0 additions & 1 deletion lib/singlepass-backend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
49 changes: 0 additions & 49 deletions lib/singlepass-backend/src/protect_unix.rs
View file
Open in desktop

This file was deleted.