From d23601a8106564128f62b12dac87281f4242d206 Mon Sep 17 00:00:00 2001 From: Lachlan Sneff Date: Fri, 18 Jan 2019 09:17:44 -0800 Subject: [PATCH 1/8] Add error types and convert most results to wasmer-runtime results --- lib/clif-backend/src/lib.rs | 16 ++-- lib/clif-backend/src/module.rs | 3 +- lib/clif-backend/src/module_env.rs | 6 +- lib/clif-backend/src/resolver.rs | 41 ++++++-- lib/runtime/src/backend.rs | 4 +- lib/runtime/src/backing.rs | 144 ++++++++++++++++++++--------- lib/runtime/src/error.rs | 116 +++++++++++++++++++++++ lib/runtime/src/instance.rs | 33 ++++--- lib/runtime/src/lib.rs | 4 +- lib/runtime/src/module.rs | 3 +- lib/runtime/src/recovery.rs | 13 ++- 11 files changed, 303 insertions(+), 80 deletions(-) create mode 100644 lib/runtime/src/error.rs diff --git a/lib/clif-backend/src/lib.rs b/lib/clif-backend/src/lib.rs index c1d9576c245..0d5df6d5863 100644 --- a/lib/clif-backend/src/lib.rs +++ b/lib/clif-backend/src/lib.rs @@ -11,7 +11,11 @@ use cranelift_codegen::{ settings::{self, Configurable}, }; use target_lexicon::Triple; -use wasmer_runtime::{backend::Compiler, module::ModuleInner}; +use wasmer_runtime::{ + backend::Compiler, + error::{CompileError, CompileResult}, + module::ModuleInner, +}; use wasmparser::{self, WasmDecoder}; pub struct CraneliftCompiler {} @@ -24,7 +28,7 @@ impl CraneliftCompiler { impl Compiler for CraneliftCompiler { // Compiles wasm binary to a wasmer module. - fn compile(&self, wasm: &[u8]) -> Result { + fn compile(&self, wasm: &[u8]) -> CompileResult { validate(wasm)?; let isa = get_isa(); @@ -53,15 +57,15 @@ fn get_isa() -> Box { isa::lookup(Triple::host()).unwrap().finish(flags) } -fn validate(bytes: &[u8]) -> Result<(), String> { +fn validate(bytes: &[u8]) -> CompileResult<()> { let mut parser = wasmparser::ValidatingParser::new(bytes, None); loop { let state = parser.read(); match *state { wasmparser::ParserState::EndWasm => break Ok(()), - wasmparser::ParserState::Error(err) => { - return Err(format!("Validation error: {}", err.message)); - } + wasmparser::ParserState::Error(err) => Err(CompileError::ValidationError { + msg: err.message.to_string(), + })?, _ => {} } } diff --git a/lib/clif-backend/src/module.rs b/lib/clif-backend/src/module.rs index 2e5f01dcad7..f5acc796f21 100644 --- a/lib/clif-backend/src/module.rs +++ b/lib/clif-backend/src/module.rs @@ -10,6 +10,7 @@ use std::{ use wasmer_runtime::{ backend::FuncResolver, backend::SigRegistry, + error::CompileResult, module::ModuleInner, structures::{Map, TypedIndex}, types::{ @@ -67,7 +68,7 @@ impl Module { mut self, isa: &isa::TargetIsa, functions: Map, - ) -> Result { + ) -> CompileResult { // we have to deduplicate `module.func_assoc` let func_assoc = &mut self.module.func_assoc; let sig_registry = &self.module.sig_registry; diff --git a/lib/clif-backend/src/module_env.rs b/lib/clif-backend/src/module_env.rs index 7adb9ea0fcf..a8476f284a9 100644 --- a/lib/clif-backend/src/module_env.rs +++ b/lib/clif-backend/src/module_env.rs @@ -5,6 +5,7 @@ use crate::{ use cranelift_codegen::{ir, isa}; use cranelift_wasm::{self, translate_module, FuncTranslator, ModuleEnvironment}; use wasmer_runtime::{ + error::{CompileError, CompileResult}, module::{DataInitializer, ExportIndex, ImportName, TableInitializer}, structures::{Map, TypedIndex}, types::{ @@ -32,8 +33,9 @@ impl<'module, 'isa> ModuleEnv<'module, 'isa> { } } - pub fn translate(mut self, wasm: &[u8]) -> Result, String> { - translate_module(wasm, &mut self).map_err(|e| e.to_string())?; + pub fn translate(mut self, wasm: &[u8]) -> CompileResult> { + translate_module(wasm, &mut self) + .map_err(|e| CompileError::InternalError { msg: e.to_string() })?; Ok(self.func_bodies) } } diff --git a/lib/clif-backend/src/resolver.rs b/lib/clif-backend/src/resolver.rs index c56d902638e..6cc783be677 100644 --- a/lib/clif-backend/src/resolver.rs +++ b/lib/clif-backend/src/resolver.rs @@ -7,6 +7,7 @@ use std::ptr::{write_unaligned, NonNull}; use wasmer_runtime::{ self, backend::{self, Mmap, Protect}, + error::{CompileError, CompileResult}, structures::Map, types::LocalFuncIndex, vm, vmcalls, @@ -23,7 +24,7 @@ impl FuncResolverBuilder { pub fn new( isa: &isa::TargetIsa, function_bodies: Map, - ) -> Result { + ) -> CompileResult { let mut compiled_functions: Vec> = Vec::with_capacity(function_bodies.len()); let mut relocations = Map::with_capacity(function_bodies.len()); let mut trap_sinks = Map::with_capacity(function_bodies.len()); @@ -38,7 +39,7 @@ impl FuncResolverBuilder { let mut trap_sink = TrapSink::new(); ctx.compile_and_emit(isa, &mut code_buf, &mut reloc_sink, &mut trap_sink) - .map_err(|e| format!("compile error: {}", e.to_string()))?; + .map_err(|e| CompileError::InternalError { msg: e.to_string() })?; ctx.clear(); // Round up each function's size to pointer alignment. total_size += round_up(code_buf.len(), mem::size_of::()); @@ -48,11 +49,24 @@ impl FuncResolverBuilder { trap_sinks.push(trap_sink); } - let mut memory = Mmap::with_size(total_size)?; + let mut memory = Mmap::with_size(total_size) + .map_err(|e| CompileError::InternalError { msg: e.to_string() })?; unsafe { - memory.protect(0..memory.size(), Protect::ReadWrite)?; + memory + .protect(0..memory.size(), Protect::ReadWrite) + .map_err(|e| CompileError::InternalError { msg: e.to_string() })?; } + // Normally, excess memory due to alignment and page-rounding would + // be filled with null-bytes. On x86 (and x86_64), + // "\x00\x00" disassembles to "add byte ptr [eax],al". + // + // If the instruction pointer falls out of its designated area, + // it would be better if it would immediately crash instead of + // continuing on and causing non-local issues. + // + // "\xCC" disassembles to "int3", which will immediately cause + // an interrupt that we can catch if we want. for i in unsafe { memory.as_slice_mut() } { *i = 0xCC; } @@ -77,7 +91,7 @@ impl FuncResolverBuilder { }) } - pub fn finalize(mut self) -> Result { + pub fn finalize(mut self) -> CompileResult { for (index, relocs) in self.relocations.iter() { for ref reloc in relocs { let target_func_address: isize = match reloc.target { @@ -98,11 +112,17 @@ impl FuncResolverBuilder { ir::LibCall::NearestF64 => libcalls::nearbyintf64 as isize, ir::LibCall::Probestack => libcalls::__rust_probestack as isize, _ => { - panic!("unexpected libcall {}", libcall); + Err(CompileError::InternalError { + msg: format!("unexpected libcall: {}", libcall), + })? + // panic!("unexpected libcall {}", libcall); } }, RelocationType::Intrinsic(ref name) => { - panic!("unexpected intrinsic {}", name); + Err(CompileError::InternalError { + msg: format!("unexpected intrinsic: {}", name), + })? + // panic!("unexpected intrinsic {}", name); } RelocationType::VmCall(vmcall) => match vmcall { VmCall::LocalStaticMemoryGrow => vmcalls::local_static_memory_grow as _, @@ -141,7 +161,9 @@ impl FuncResolverBuilder { (target_func_address - reloc_address + reloc_addend) as i32; write_unaligned(reloc_address as *mut i32, reloc_delta_i32); }, - _ => panic!("unsupported reloc kind"), + _ => Err(CompileError::InternalError { + msg: format!("unsupported reloc kind: {}", reloc.reloc), + })?, } } } @@ -149,7 +171,8 @@ impl FuncResolverBuilder { unsafe { self.resolver .memory - .protect(0..self.resolver.memory.size(), Protect::ReadExec)?; + .protect(0..self.resolver.memory.size(), Protect::ReadExec) + .map_err(|e| CompileError::InternalError { msg: e.to_string() })?;; } Ok(self.resolver) diff --git a/lib/runtime/src/backend.rs b/lib/runtime/src/backend.rs index 5b7951bc244..4cc67a5fc75 100644 --- a/lib/runtime/src/backend.rs +++ b/lib/runtime/src/backend.rs @@ -1,4 +1,4 @@ -use crate::{module::ModuleInner, types::LocalFuncIndex, vm}; +use crate::{error::CompileResult, module::ModuleInner, types::LocalFuncIndex, vm}; use std::ptr::NonNull; pub use crate::mmap::{Mmap, Protect}; @@ -6,7 +6,7 @@ pub use crate::sig_registry::SigRegistry; pub trait Compiler { /// Compiles a `Module` from WebAssembly binary format - fn compile(&self, wasm: &[u8]) -> Result; + fn compile(&self, wasm: &[u8]) -> CompileResult; } pub trait FuncResolver { diff --git a/lib/runtime/src/backing.rs b/lib/runtime/src/backing.rs index f84670d3d6e..289588176ff 100644 --- a/lib/runtime/src/backing.rs +++ b/lib/runtime/src/backing.rs @@ -1,4 +1,5 @@ use crate::{ + error::{LinkError, LinkResult}, export::{Context, Export}, import::Imports, memory::LinearMemory, @@ -102,7 +103,6 @@ impl LocalBacking { LocalOrImport::Local(local_memory_index) => { let memory_desc = &module.memories[local_memory_index]; let data_top = init_base + init.data.len(); - println!("data_top: {}", data_top); assert!((memory_desc.min * LinearMemory::PAGE_SIZE) as usize >= data_top); let mem: &mut LinearMemory = &mut memories[local_memory_index]; @@ -305,7 +305,7 @@ impl ImportBacking { module: &ModuleInner, imports: &mut Imports, vmctx: *mut vm::Ctx, - ) -> Result { + ) -> LinkResult { Ok(ImportBacking { functions: import_functions(module, imports, vmctx)?, memories: import_memories(module, imports, vmctx)?, @@ -323,7 +323,7 @@ fn import_functions( module: &ModuleInner, imports: &mut Imports, vmctx: *mut vm::Ctx, -) -> Result, String> { +) -> LinkResult> { let mut functions = Map::with_capacity(module.imported_functions.len()); for (index, ImportName { namespace, name }) in &module.imported_functions { let sig_index = module.func_assoc[index.convert_up(module)]; @@ -346,18 +346,33 @@ fn import_functions( }, }); } else { - return Err(format!( - "unexpected signature for {:?}:{:?}", - namespace, name - )); + Err(LinkError::IncorrectImportSignature { + namespace: namespace.clone(), + name: name.clone(), + expected: expected_sig.clone(), + found: signature.clone(), + })? } } - Some(_) => { - return Err(format!("incorrect import type for {}:{}", namespace, name)); - } - None => { - return Err(format!("import not found: {}:{}", namespace, name)); + Some(export_type) => { + let export_type_name = match export_type { + Export::Function { .. } => "function", + Export::Memory { .. } => "memory", + Export::Table { .. } => "table", + Export::Global { .. } => "global", + } + .to_string(); + Err(LinkError::IncorrectImportType { + namespace: namespace.clone(), + name: name.clone(), + expected: "function".to_string(), + found: export_type_name, + })? } + None => Err(LinkError::ImportNotFound { + namespace: namespace.clone(), + name: name.clone(), + })?, } } Ok(functions.into_boxed_map()) @@ -367,7 +382,7 @@ fn import_memories( module: &ModuleInner, imports: &mut Imports, vmctx: *mut vm::Ctx, -) -> Result, String> { +) -> LinkResult> { let mut memories = Map::with_capacity(module.imported_memories.len()); for (_index, (ImportName { namespace, name }, expected_memory_desc)) in &module.imported_memories @@ -390,18 +405,33 @@ fn import_memories( }, }); } else { - return Err(format!( - "incorrect memory description for {}:{}", - namespace, name, - )); + Err(LinkError::IncorrectMemoryDescription { + namespace: namespace.clone(), + name: name.clone(), + expected: expected_memory_desc.clone(), + found: memory_desc.clone(), + })? } } - Some(_) => { - return Err(format!("incorrect import type for {}:{}", namespace, name)); - } - None => { - return Err(format!("import not found: {}:{}", namespace, name)); + Some(export_type) => { + let export_type_name = match export_type { + Export::Function { .. } => "function", + Export::Memory { .. } => "memory", + Export::Table { .. } => "table", + Export::Global { .. } => "global", + } + .to_string(); + Err(LinkError::IncorrectImportType { + namespace: namespace.clone(), + name: name.clone(), + expected: "memory".to_string(), + found: export_type_name, + })? } + None => Err(LinkError::ImportNotFound { + namespace: namespace.clone(), + name: name.clone(), + })?, } } Ok(memories.into_boxed_map()) @@ -411,7 +441,7 @@ fn import_tables( module: &ModuleInner, imports: &mut Imports, vmctx: *mut vm::Ctx, -) -> Result, String> { +) -> LinkResult> { let mut tables = Map::with_capacity(module.imported_tables.len()); for (_index, (ImportName { namespace, name }, expected_table_desc)) in &module.imported_tables { let table_import = imports @@ -432,18 +462,33 @@ fn import_tables( }, }); } else { - return Err(format!( - "incorrect table description for {}:{}", - namespace, name, - )); + Err(LinkError::IncorrectTableDescription { + namespace: namespace.clone(), + name: name.clone(), + expected: expected_table_desc.clone(), + found: table_desc.clone(), + })? } } - Some(_) => { - return Err(format!("incorrect import type for {}:{}", namespace, name)); - } - None => { - return Err(format!("import not found: {}:{}", namespace, name)); + Some(export_type) => { + let export_type_name = match export_type { + Export::Function { .. } => "function", + Export::Memory { .. } => "memory", + Export::Table { .. } => "table", + Export::Global { .. } => "global", + } + .to_string(); + Err(LinkError::IncorrectImportType { + namespace: namespace.clone(), + name: name.clone(), + expected: "table".to_string(), + found: export_type_name, + })? } + None => Err(LinkError::ImportNotFound { + namespace: namespace.clone(), + name: name.clone(), + })?, } } Ok(tables.into_boxed_map()) @@ -452,7 +497,7 @@ fn import_tables( fn import_globals( module: &ModuleInner, imports: &mut Imports, -) -> Result, String> { +) -> LinkResult> { let mut globals = Map::with_capacity(module.imported_globals.len()); for (_, (ImportName { namespace, name }, imported_global_desc)) in &module.imported_globals { let import = imports @@ -465,18 +510,33 @@ fn import_globals( global: local.inner(), }); } else { - return Err(format!( - "unexpected global description for {:?}:{:?}", - namespace, name - )); + Err(LinkError::IncorrectGlobalDescription { + namespace: namespace.clone(), + name: name.clone(), + expected: imported_global_desc.clone(), + found: global.clone(), + })? } } - Some(_) => { - return Err(format!("incorrect import type for {}:{}", namespace, name)); - } - None => { - return Err(format!("import not found: {}:{}", namespace, name)); + Some(export_type) => { + let export_type_name = match export_type { + Export::Function { .. } => "function", + Export::Memory { .. } => "memory", + Export::Table { .. } => "table", + Export::Global { .. } => "global", + } + .to_string(); + Err(LinkError::IncorrectImportType { + namespace: namespace.clone(), + name: name.clone(), + expected: "global".to_string(), + found: export_type_name, + })? } + None => Err(LinkError::ImportNotFound { + namespace: namespace.clone(), + name: name.clone(), + })?, } } Ok(globals.into_boxed_map()) diff --git a/lib/runtime/src/error.rs b/lib/runtime/src/error.rs new file mode 100644 index 00000000000..8977fd270b4 --- /dev/null +++ b/lib/runtime/src/error.rs @@ -0,0 +1,116 @@ +use crate::types::{FuncSig, GlobalDesc, Memory, MemoryIndex, Table, TableIndex, Type}; + +pub type Result = std::result::Result>; +pub type CompileResult = std::result::Result>; +pub type LinkResult = std::result::Result>; +pub type RuntimeResult = std::result::Result>; +pub type CallResult = std::result::Result>; + +/// This is returned when the chosen compiler is unable to +/// successfully compile the provided webassembly module into +/// a `Module`. +#[derive(Debug, Clone)] +pub enum CompileError { + ValidationError { msg: String }, + InternalError { msg: String }, +} + +/// This is returned when the runtime is unable to +/// correctly link the module with the provided imports. +#[derive(Debug, Clone)] +pub enum LinkError { + IncorrectImportType { + namespace: String, + name: String, + expected: String, + found: String, + }, + IncorrectImportSignature { + namespace: String, + name: String, + expected: FuncSig, + found: FuncSig, + }, + ImportNotFound { + namespace: String, + name: String, + }, + IncorrectMemoryDescription { + namespace: String, + name: String, + expected: Memory, + found: Memory, + }, + IncorrectTableDescription { + namespace: String, + name: String, + expected: Table, + found: Table, + }, + IncorrectGlobalDescription { + namespace: String, + name: String, + expected: GlobalDesc, + found: GlobalDesc, + }, +} + +/// This is the error type returned when calling +/// a webassembly function. +/// +/// The main way to do this is `Instance.call`. +#[derive(Debug, Clone)] +pub enum RuntimeError { + OutOfBoundsAccess { memory: MemoryIndex, addr: u32 }, + IndirectCallSignature { table: TableIndex }, + IndirectCallToNull { table: TableIndex }, + Unknown { msg: String }, +} + +/// This error type is produced by calling a wasm function +/// exported from a module. +/// +/// If the module traps in some way while running, this will +/// be the `CallError::Runtime(RuntimeError)` variant. +#[derive(Debug, Clone)] +pub enum CallError { + Signature { expected: FuncSig, found: Vec }, + NoSuchExport { name: String }, + ExportNotFunc { name: String }, + Runtime(Box), +} + +/// The amalgamation of all errors that can occur +/// during the compilation, instantiation, or execution +/// of a webassembly module. +#[derive(Debug, Clone)] +pub enum Error { + CompileError(Box), + LinkError(Box), + RuntimeError(Box), + CallError(Box), +} + +impl From> for Box { + fn from(compile_err: Box) -> Self { + Box::new(Error::CompileError(compile_err)) + } +} + +impl From> for Box { + fn from(link_err: Box) -> Self { + Box::new(Error::LinkError(link_err)) + } +} + +impl From> for Box { + fn from(runtime_err: Box) -> Self { + Box::new(Error::RuntimeError(runtime_err)) + } +} + +impl From> for Box { + fn from(call_err: Box) -> Self { + Box::new(Error::CallError(call_err)) + } +} diff --git a/lib/runtime/src/instance.rs b/lib/runtime/src/instance.rs index e9b23a7bd44..080feeb1473 100644 --- a/lib/runtime/src/instance.rs +++ b/lib/runtime/src/instance.rs @@ -1,6 +1,7 @@ use crate::recovery::call_protected; use crate::{ backing::{ImportBacking, LocalBacking}, + error::{CallError, CallResult, Result}, export::{ Context, Export, ExportIter, FuncPointer, GlobalPointer, MemoryPointer, TablePointer, }, @@ -31,10 +32,7 @@ pub struct Instance { } impl Instance { - pub(crate) fn new( - module: Rc, - mut imports: Box, - ) -> Result { + pub(crate) fn new(module: Rc, mut imports: Box) -> Result { // We need the backing and import_backing to create a vm::Ctx, but we need // a vm::Ctx to create a backing and an import_backing. The solution is to create an // uninitialized vm::Ctx and then initialize it in-place. @@ -73,17 +71,22 @@ impl Instance { /// /// This will eventually return `Result>, String>` in /// order to support multi-value returns. - pub fn call(&mut self, name: &str, args: &[Value]) -> Result, String> { - let export_index = self - .module - .exports - .get(name) - .ok_or_else(|| format!("there is no export with that name: {}", name))?; + pub fn call(&mut self, name: &str, args: &[Value]) -> CallResult> { + let export_index = + self.module + .exports + .get(name) + .ok_or_else(|| CallError::NoSuchExport { + name: name.to_string(), + })?; let func_index = if let ExportIndex::Func(func_index) = export_index { *func_index } else { - return Err("that export is not a function".to_string()); + return Err(CallError::ExportNotFunc { + name: name.to_string(), + } + .into()); }; self.call_with_index(func_index, args) @@ -103,7 +106,7 @@ impl Instance { &mut self, func_index: FuncIndex, args: &[Value], - ) -> Result, String> { + ) -> CallResult> { let (func_ref, ctx, signature) = self.inner.get_func_from_index(&self.module, func_index); let func_ptr = CodePtr::from_ptr(func_ref.inner() as _); @@ -118,7 +121,10 @@ impl Instance { ); if !signature.check_sig(args) { - return Err("incorrect signature".to_string()); + Err(CallError::Signature { + expected: signature.clone(), + found: args.iter().map(|val| val.ty()).collect(), + })? } let libffi_args: Vec<_> = args @@ -150,6 +156,7 @@ impl Instance { None }) }) + .map_err(|e| CallError::Runtime(e).into()) } } diff --git a/lib/runtime/src/lib.rs b/lib/runtime/src/lib.rs index 7a48de2dd4f..b3803a963bf 100644 --- a/lib/runtime/src/lib.rs +++ b/lib/runtime/src/lib.rs @@ -7,6 +7,7 @@ mod macros; #[doc(hidden)] pub mod backend; mod backing; +pub mod error; pub mod export; pub mod import; mod instance; @@ -23,13 +24,14 @@ pub mod vm; #[doc(hidden)] pub mod vmcalls; +use self::error::CompileResult; pub use self::instance::Instance; #[doc(inline)] pub use self::module::Module; use std::rc::Rc; /// Compile a webassembly module using the provided compiler. -pub fn compile(wasm: &[u8], compiler: &dyn backend::Compiler) -> Result { +pub fn compile(wasm: &[u8], compiler: &dyn backend::Compiler) -> CompileResult { compiler .compile(wasm) .map(|inner| module::Module::new(Rc::new(inner))) diff --git a/lib/runtime/src/module.rs b/lib/runtime/src/module.rs index 7c40c7ef0b6..8a4d890be0e 100644 --- a/lib/runtime/src/module.rs +++ b/lib/runtime/src/module.rs @@ -1,5 +1,6 @@ use crate::{ backend::FuncResolver, + error::Result, import::Imports, sig_registry::SigRegistry, structures::Map, @@ -47,7 +48,7 @@ impl Module { } /// Instantiate a webassembly module with the provided imports. - pub fn instantiate(&self, imports: Imports) -> Result { + pub fn instantiate(&self, imports: Imports) -> Result { Instance::new(Rc::clone(&self.0), Box::new(imports)) } } diff --git a/lib/runtime/src/recovery.rs b/lib/runtime/src/recovery.rs index 7a88e4ae339..ec7a7cbf20c 100644 --- a/lib/runtime/src/recovery.rs +++ b/lib/runtime/src/recovery.rs @@ -4,7 +4,10 @@ //! 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 crate::sighandler::install_sighandler; +use crate::{ + error::{RuntimeError, RuntimeResult}, + sighandler::install_sighandler, +}; use nix::libc::siginfo_t; use nix::sys::signal::{Signal, SIGBUS, SIGFPE, SIGILL, SIGSEGV}; use std::cell::{Cell, UnsafeCell}; @@ -23,7 +26,7 @@ thread_local! { pub static CAUGHT_ADDRESS: Cell = Cell::new(0); } -pub fn call_protected(f: impl FnOnce() -> T) -> Result { +pub fn call_protected(f: impl FnOnce() -> T) -> RuntimeResult { unsafe { let jmp_buf = SETJMP_BUFFER.with(|buf| buf.get()); let prev_jmp_buf = *jmp_buf; @@ -45,7 +48,11 @@ pub fn call_protected(f: impl FnOnce() -> T) -> Result { Err(_) => "error while getting the Signal", _ => "unkown trapped signal", }; - Err(format!("trap at {:#x} - {}", addr, signal)) + // When the trap-handler is fully implemented, this will return more information. + Err(RuntimeError::Unknown { + msg: format!("trap at {:#x} - {}", addr, signal), + } + .into()) } else { let ret = f(); // TODO: Switch stack? *jmp_buf = prev_jmp_buf; From ed87168c484a9610bd3912326447edc335452b14 Mon Sep 17 00:00:00 2001 From: Lachlan Sneff Date: Fri, 18 Jan 2019 10:15:14 -0800 Subject: [PATCH 2/8] Fix spectests --- lib/runtime/build/spectests.rs | 6 ++- lib/runtime/examples/simple/main.rs | 3 +- lib/runtime/src/error.rs | 64 +++++++++++++++++++++++++---- lib/runtime/src/instance.rs | 5 +-- lib/runtime/tests/semantics.rs | 24 ++++++----- 5 files changed, 77 insertions(+), 25 deletions(-) diff --git a/lib/runtime/build/spectests.rs b/lib/runtime/build/spectests.rs index 47260946b0a..1ebe128622c 100644 --- a/lib/runtime/build/spectests.rs +++ b/lib/runtime/build/spectests.rs @@ -81,6 +81,7 @@ use wasmer_clif_backend::CraneliftCompiler; use wasmer_runtime::import::Imports; use wasmer_runtime::types::Value; use wasmer_runtime::{{Instance, module::Module}}; +use wasmer_runtime::error::Result; static IMPORT_MODULE: &str = r#" (module @@ -606,11 +607,12 @@ fn {}_assert_malformed() {{ let func_name = format!("{}_action_invoke", self.command_name()); self.buffer.push_str( format!( - "fn {func_name}(instance: &mut Instance) -> Result<(), String> {{ + "fn {func_name}(instance: &mut Instance) -> Result<()> {{ println!(\"Executing function {{}}\", \"{func_name}\"); let result = instance.call(\"{field}\", &[{args_values}]); {assertion} - result.map(|_| ()) + result?; + Ok(()) }}\n", func_name = func_name, field = field, diff --git a/lib/runtime/examples/simple/main.rs b/lib/runtime/examples/simple/main.rs index d0b1a2fe05f..981ca7ac9b9 100644 --- a/lib/runtime/examples/simple/main.rs +++ b/lib/runtime/examples/simple/main.rs @@ -2,6 +2,7 @@ use wabt::wat2wasm; use wasmer_clif_backend::CraneliftCompiler; use wasmer_runtime::{ self as runtime, + error::Result, export::{Context, Export, FuncPointer}, import::{Imports, NamespaceMap}, types::{FuncSig, Type, Value}, @@ -10,7 +11,7 @@ use wasmer_runtime::{ static EXAMPLE_WASM: &'static [u8] = include_bytes!("simple.wasm"); -fn main() -> Result<(), String> { +fn main() -> Result<()> { let wasm_binary = wat2wasm(IMPORT_MODULE.as_bytes()).expect("WAST not valid or malformed"); let inner_module = runtime::compile(&wasm_binary, &CraneliftCompiler::new())?; diff --git a/lib/runtime/src/error.rs b/lib/runtime/src/error.rs index 8977fd270b4..3a350748b01 100644 --- a/lib/runtime/src/error.rs +++ b/lib/runtime/src/error.rs @@ -9,14 +9,24 @@ pub type CallResult = std::result::Result>; /// This is returned when the chosen compiler is unable to /// successfully compile the provided webassembly module into /// a `Module`. +/// +/// Comparing two `CompileError`s always evaluates to false. #[derive(Debug, Clone)] pub enum CompileError { ValidationError { msg: String }, InternalError { msg: String }, } +impl PartialEq for CompileError { + fn eq(&self, _other: &CompileError) -> bool { + false + } +} + /// This is returned when the runtime is unable to /// correctly link the module with the provided imports. +/// +/// Comparing two `LinkError`s always evaluates to false. #[derive(Debug, Clone)] pub enum LinkError { IncorrectImportType { @@ -55,10 +65,18 @@ pub enum LinkError { }, } +impl PartialEq for LinkError { + fn eq(&self, _other: &LinkError) -> bool { + false + } +} + /// This is the error type returned when calling /// a webassembly function. /// /// The main way to do this is `Instance.call`. +/// +/// Comparing two `RuntimeError`s always evaluates to false. #[derive(Debug, Clone)] pub enum RuntimeError { OutOfBoundsAccess { memory: MemoryIndex, addr: u32 }, @@ -67,50 +85,78 @@ pub enum RuntimeError { Unknown { msg: String }, } +impl PartialEq for RuntimeError { + fn eq(&self, _other: &RuntimeError) -> bool { + false + } +} + /// This error type is produced by calling a wasm function /// exported from a module. /// /// If the module traps in some way while running, this will /// be the `CallError::Runtime(RuntimeError)` variant. +/// +/// Comparing two `CallError`s always evaluates to false. #[derive(Debug, Clone)] pub enum CallError { Signature { expected: FuncSig, found: Vec }, NoSuchExport { name: String }, ExportNotFunc { name: String }, - Runtime(Box), + Runtime(RuntimeError), +} + +impl PartialEq for CallError { + fn eq(&self, _other: &CallError) -> bool { + false + } } /// The amalgamation of all errors that can occur /// during the compilation, instantiation, or execution /// of a webassembly module. +/// +/// Comparing two `Error`s always evaluates to false. #[derive(Debug, Clone)] pub enum Error { - CompileError(Box), - LinkError(Box), - RuntimeError(Box), - CallError(Box), + CompileError(CompileError), + LinkError(LinkError), + RuntimeError(RuntimeError), + CallError(CallError), +} + +impl PartialEq for Error { + fn eq(&self, _other: &Error) -> bool { + false + } } impl From> for Box { fn from(compile_err: Box) -> Self { - Box::new(Error::CompileError(compile_err)) + Box::new(Error::CompileError(*compile_err)) } } impl From> for Box { fn from(link_err: Box) -> Self { - Box::new(Error::LinkError(link_err)) + Box::new(Error::LinkError(*link_err)) } } impl From> for Box { fn from(runtime_err: Box) -> Self { - Box::new(Error::RuntimeError(runtime_err)) + Box::new(Error::RuntimeError(*runtime_err)) } } impl From> for Box { fn from(call_err: Box) -> Self { - Box::new(Error::CallError(call_err)) + Box::new(Error::CallError(*call_err)) + } +} + +impl From> for Box { + fn from(runtime_err: Box) -> Self { + Box::new(CallError::Runtime(*runtime_err)) } } diff --git a/lib/runtime/src/instance.rs b/lib/runtime/src/instance.rs index 028e0d4a9e0..075d4709bf0 100644 --- a/lib/runtime/src/instance.rs +++ b/lib/runtime/src/instance.rs @@ -138,7 +138,7 @@ impl Instance { .chain(iter::once(libffi_arg(&vmctx_ptr))) .collect(); - call_protected(|| { + Ok(call_protected(|| { signature .returns .first() @@ -155,8 +155,7 @@ impl Instance { } None }) - }) - .map_err(|e| CallError::Runtime(e).into()) + })?) } } diff --git a/lib/runtime/tests/semantics.rs b/lib/runtime/tests/semantics.rs index aea478854bc..bd5735108a7 100644 --- a/lib/runtime/tests/semantics.rs +++ b/lib/runtime/tests/semantics.rs @@ -2,7 +2,10 @@ mod tests { use wabt::wat2wasm; use wasmer_clif_backend::CraneliftCompiler; - use wasmer_runtime::import::Imports; + use wasmer_runtime::{ + error::{CallError, RuntimeError}, + import::Imports, + }; // The semantics of stack overflow are documented at: // https://webassembly.org/docs/semantics/#stack-overflow @@ -25,15 +28,16 @@ mod tests { .instantiate(Imports::new()) .expect("WASM can't be instantiated"); let result = instance.call("stack-overflow", &[]); - assert!( - result.is_err(), - "should fail with error due to stack overflow" - ); - // TODO The kind of error and message needs to be defined, not spec defined, maybe RuntimeError or RangeError - if let Err(message) = result { - assert!(!message.contains("segmentation violation")); - assert!(!message.contains("bus error")); + + match result { + Err(err) => match *err { + CallError::Runtime(RuntimeError::Unknown { msg }) => { + assert!(!msg.contains("segmentation violation")); + assert!(!msg.contains("bus error")); + } + _ => unimplemented!(), + }, + Ok(_) => panic!("should fail with error due to stack overflow"), } } - } From 705708cafec4a878570d0dd86562d76645da442e Mon Sep 17 00:00:00 2001 From: Lachlan Sneff Date: Fri, 18 Jan 2019 12:13:01 -0800 Subject: [PATCH 3/8] add 'ProtectedCaller' to runtime --- lib/clif-backend/src/lib.rs | 4 +- lib/runtime/src/backend.rs | 62 ++++++++++++++- lib/runtime/src/backing.rs | 39 +++++---- lib/runtime/src/instance.rs | 155 ++++++++++++++++++++++-------------- lib/runtime/src/lib.rs | 3 +- lib/runtime/src/module.rs | 3 +- 6 files changed, 184 insertions(+), 82 deletions(-) diff --git a/lib/clif-backend/src/lib.rs b/lib/clif-backend/src/lib.rs index 0d5df6d5863..969348688fd 100644 --- a/lib/clif-backend/src/lib.rs +++ b/lib/clif-backend/src/lib.rs @@ -12,7 +12,7 @@ use cranelift_codegen::{ }; use target_lexicon::Triple; use wasmer_runtime::{ - backend::Compiler, + backend::{Compiler, Token}, error::{CompileError, CompileResult}, module::ModuleInner, }; @@ -28,7 +28,7 @@ impl CraneliftCompiler { impl Compiler for CraneliftCompiler { // Compiles wasm binary to a wasmer module. - fn compile(&self, wasm: &[u8]) -> CompileResult { + fn compile(&self, wasm: &[u8], _: Token) -> CompileResult { validate(wasm)?; let isa = get_isa(); diff --git a/lib/runtime/src/backend.rs b/lib/runtime/src/backend.rs index 4cc67a5fc75..0ea709c3b6e 100644 --- a/lib/runtime/src/backend.rs +++ b/lib/runtime/src/backend.rs @@ -1,18 +1,74 @@ -use crate::{error::CompileResult, module::ModuleInner, types::LocalFuncIndex, vm}; +use crate::{ + error::CompileResult, + error::RuntimeResult, + module::ModuleInner, + types::{FuncIndex, LocalFuncIndex, Value}, + vm, +}; use std::ptr::NonNull; pub use crate::mmap::{Mmap, Protect}; pub use crate::sig_registry::SigRegistry; +/// This type cannot be constructed from +/// outside the runtime crate. +pub struct Token { + _private: (), +} + +impl Token { + pub(crate) fn generate() -> Self { + Self { _private: () } + } +} + pub trait Compiler { - /// Compiles a `Module` from WebAssembly binary format - fn compile(&self, wasm: &[u8]) -> CompileResult; + /// Compiles a `Module` from WebAssembly binary format. + /// The `CompileToken` parameter ensures that this can only + /// be called from inside the runtime. + fn compile(&self, wasm: &[u8], _: Token) -> CompileResult; +} + +/// The functionality exposed by this trait is expected to be used +/// for calling functions exported by a webassembly module from +/// host code only. +pub trait ProtectedCaller { + /// This calls the exported function designated by `local_func_index`. + /// Important to note, this supports calling imported functions that are + /// then exported. + /// + /// It's invalid to attempt to call a local function that isn't exported and + /// the implementation is expected to check for that. The implementation + /// is also expected to check for correct parameter types and correct + /// parameter number. + /// + /// The `returns` parameter is filled with dummy values when passed in and upon function + /// return, will be filled with the return values of the wasm function, as long as the + /// call completed successfully. + /// + /// The existance of the Token parameter ensures that this can only be called from + /// within the runtime crate. + fn call( + &self, + module: &ModuleInner, + func_index: FuncIndex, + params: &[Value], + returns: &mut [Value], + vmctx: *mut vm::Ctx, + _: Token, + ) -> RuntimeResult<()>; } pub trait FuncResolver { + /// This returns a pointer to the function designated by the `local_func_index` + /// parameter. + /// + /// The existance of the Token parameter ensures that this can only be called from + /// within the runtime crate. fn get( &self, module: &ModuleInner, local_func_index: LocalFuncIndex, + _: Token, ) -> Option>; } diff --git a/lib/runtime/src/backing.rs b/lib/runtime/src/backing.rs index 662b00858c8..b292a96378d 100644 --- a/lib/runtime/src/backing.rs +++ b/lib/runtime/src/backing.rs @@ -1,4 +1,5 @@ use crate::{ + backend::Token, error::{LinkError, LinkResult}, export::{Context, Export}, import::Imports, @@ -178,14 +179,17 @@ impl LocalBacking { let sig_id = vm::SigId(sig_index.index() as u32); let func_data = match func_index.local_or_import(module) { - LocalOrImport::Local(local_func_index) => vm::ImportedFunc { - func: module - .func_resolver - .get(module, local_func_index) - .unwrap() - .as_ptr(), - vmctx, - }, + LocalOrImport::Local(local_func_index) => { + let token = Token::generate(); + vm::ImportedFunc { + func: module + .func_resolver + .get(module, local_func_index, token) + .unwrap() + .as_ptr(), + vmctx, + } + } LocalOrImport::Import(imported_func_index) => { imports.functions[imported_func_index].clone() } @@ -229,14 +233,17 @@ impl LocalBacking { let sig_id = vm::SigId(sig_index.index() as u32); let func_data = match func_index.local_or_import(module) { - LocalOrImport::Local(local_func_index) => vm::ImportedFunc { - func: module - .func_resolver - .get(module, local_func_index) - .unwrap() - .as_ptr(), - vmctx, - }, + LocalOrImport::Local(local_func_index) => { + let token = Token::generate(); + vm::ImportedFunc { + func: module + .func_resolver + .get(module, local_func_index, token) + .unwrap() + .as_ptr(), + vmctx, + } + } LocalOrImport::Import(imported_func_index) => { imports.functions[imported_func_index].clone() } diff --git a/lib/runtime/src/instance.rs b/lib/runtime/src/instance.rs index 075d4709bf0..ef57f29699f 100644 --- a/lib/runtime/src/instance.rs +++ b/lib/runtime/src/instance.rs @@ -1,5 +1,6 @@ use crate::recovery::call_protected; use crate::{ + backend::Token, backing::{ImportBacking, LocalBacking}, error::{CallError, CallResult, Result}, export::{ @@ -67,11 +68,11 @@ impl Instance { /// Call an exported webassembly function given the export name. /// Pass arguments by wrapping each one in the `Value` enum. - /// The returned value is also returned in a `Value`. + /// The returned values are also each wrapped in a `Value`. /// - /// This will eventually return `Result>, String>` in - /// order to support multi-value returns. - pub fn call(&mut self, name: &str, args: &[Value]) -> CallResult> { + /// This returns `CallResult>` in order to support + /// the future multi-value returns webassembly feature. + pub fn call(&mut self, name: &str, args: &[Value]) -> CallResult> { let export_index = self.module .exports @@ -102,23 +103,13 @@ impl Instance { } impl Instance { - fn call_with_index( - &mut self, - func_index: FuncIndex, - args: &[Value], - ) -> CallResult> { - let (func_ref, ctx, signature) = self.inner.get_func_from_index(&self.module, func_index); - - let func_ptr = CodePtr::from_ptr(func_ref.inner() as _); - let vmctx_ptr = match ctx { - Context::External(vmctx) => vmctx, - Context::Internal => &mut *self.inner.vmctx, - }; - - assert!( - signature.returns.len() <= 1, - "multi-value returns not yet supported" - ); + fn call_with_index(&mut self, func_index: FuncIndex, args: &[Value]) -> CallResult> { + let sig_index = *self + .module + .func_assoc + .get(func_index) + .expect("broken invariant, incorrect func index"); + let signature = self.module.sig_registry.lookup_func_sig(sig_index); if !signature.check_sig(args) { Err(CallError::Signature { @@ -127,35 +118,78 @@ impl Instance { })? } - let libffi_args: Vec<_> = args - .iter() - .map(|val| match val { - Value::I32(ref x) => libffi_arg(x), - Value::I64(ref x) => libffi_arg(x), - Value::F32(ref x) => libffi_arg(x), - Value::F64(ref x) => libffi_arg(x), - }) - .chain(iter::once(libffi_arg(&vmctx_ptr))) - .collect(); - - Ok(call_protected(|| { - signature - .returns - .first() - .map(|ty| match ty { - Type::I32 => Value::I32(unsafe { libffi_call(func_ptr, &libffi_args) }), - Type::I64 => Value::I64(unsafe { libffi_call(func_ptr, &libffi_args) }), - Type::F32 => Value::F32(unsafe { libffi_call(func_ptr, &libffi_args) }), - Type::F64 => Value::F64(unsafe { libffi_call(func_ptr, &libffi_args) }), - }) - .or_else(|| { - // call with no returns - unsafe { - libffi_call::<()>(func_ptr, &libffi_args); - } - None - }) - })?) + // Create an output vector that's full of dummy values. + let mut returns = vec![Value::I32(0); signature.returns.len()]; + + let vmctx = match func_index.local_or_import(&self.module) { + LocalOrImport::Local(local_func_index) => &mut *self.inner.vmctx, + LocalOrImport::Import(imported_func_index) => { + self.inner.import_backing.functions[imported_func_index].vmctx + } + }; + + let token = Token::generate(); + + self.module.protected_caller.call( + &self.module, + func_index, + args, + &mut returns, + vmctx, + token, + )?; + + Ok(returns) + + // let (func_ref, ctx, signature) = self.inner.get_func_from_index(&self.module, func_index); + + // let func_ptr = CodePtr::from_ptr(func_ref.inner() as _); + // let vmctx_ptr = match ctx { + // Context::External(vmctx) => vmctx, + // Context::Internal => &mut *self.inner.vmctx, + // }; + + // assert!( + // signature.returns.len() <= 1, + // "multi-value returns not yet supported" + // ); + + // if !signature.check_sig(args) { + // Err(CallError::Signature { + // expected: signature.clone(), + // found: args.iter().map(|val| val.ty()).collect(), + // })? + // } + + // let libffi_args: Vec<_> = args + // .iter() + // .map(|val| match val { + // Value::I32(ref x) => libffi_arg(x), + // Value::I64(ref x) => libffi_arg(x), + // Value::F32(ref x) => libffi_arg(x), + // Value::F64(ref x) => libffi_arg(x), + // }) + // .chain(iter::once(libffi_arg(&vmctx_ptr))) + // .collect(); + + // Ok(call_protected(|| { + // signature + // .returns + // .first() + // .map(|ty| match ty { + // Type::I32 => Value::I32(unsafe { libffi_call(func_ptr, &libffi_args) }), + // Type::I64 => Value::I64(unsafe { libffi_call(func_ptr, &libffi_args) }), + // Type::F32 => Value::F32(unsafe { libffi_call(func_ptr, &libffi_args) }), + // Type::F64 => Value::F64(unsafe { libffi_call(func_ptr, &libffi_args) }), + // }) + // .or_else(|| { + // // call with no returns + // unsafe { + // libffi_call::<()>(func_ptr, &libffi_args); + // } + // None + // }) + // })?) } } @@ -218,15 +252,18 @@ impl InstanceInner { .expect("broken invariant, incorrect func index"); let (func_ptr, ctx) = match func_index.local_or_import(module) { - LocalOrImport::Local(local_func_index) => ( - module - .func_resolver - .get(&module, local_func_index) - .expect("broken invariant, func resolver not synced with module.exports") - .cast() - .as_ptr() as *const _, - Context::Internal, - ), + LocalOrImport::Local(local_func_index) => { + let token = Token::generate(); + ( + module + .func_resolver + .get(&module, local_func_index, token) + .expect("broken invariant, func resolver not synced with module.exports") + .cast() + .as_ptr() as *const _, + Context::Internal, + ) + } LocalOrImport::Import(imported_func_index) => { let imported_func = &self.import_backing.functions[imported_func_index]; ( diff --git a/lib/runtime/src/lib.rs b/lib/runtime/src/lib.rs index dd7f6225390..f7517c593dc 100644 --- a/lib/runtime/src/lib.rs +++ b/lib/runtime/src/lib.rs @@ -32,7 +32,8 @@ use std::rc::Rc; /// Compile a webassembly module using the provided compiler. pub fn compile(wasm: &[u8], compiler: &dyn backend::Compiler) -> CompileResult { + let token = backend::Token::generate(); compiler - .compile(wasm) + .compile(wasm, token) .map(|inner| module::Module::new(Rc::new(inner))) } diff --git a/lib/runtime/src/module.rs b/lib/runtime/src/module.rs index 7ca3e30bf25..2ccf5f7c02a 100644 --- a/lib/runtime/src/module.rs +++ b/lib/runtime/src/module.rs @@ -1,5 +1,5 @@ use crate::{ - backend::FuncResolver, + backend::{FuncResolver, ProtectedCaller}, error::Result, import::Imports, sig_registry::SigRegistry, @@ -18,6 +18,7 @@ use std::rc::Rc; #[doc(hidden)] pub struct ModuleInner { pub func_resolver: Box, + pub protected_caller: Box, // This are strictly local and the typsystem ensures that. pub memories: Map, pub globals: Map, From 539db9f577385b13c472e744dabfe6ba6ef6a302 Mon Sep 17 00:00:00 2001 From: Lachlan Sneff Date: Fri, 18 Jan 2019 13:29:43 -0800 Subject: [PATCH 4/8] Starting to set up protected call in clif-backend --- lib/clif-backend/Cargo.toml | 5 + lib/clif-backend/src/call/mod.rs | 125 ++++++++++++++++++ .../src => clif-backend/src/call}/recovery.rs | 4 +- .../src/call}/sighandler.rs | 2 +- lib/clif-backend/src/lib.rs | 1 + lib/runtime/Cargo.toml | 1 - lib/runtime/src/backend.rs | 6 +- lib/runtime/src/backing.rs | 43 +++--- lib/runtime/src/instance.rs | 64 +-------- lib/runtime/src/lib.rs | 2 - lib/runtime/src/types.rs | 2 +- 11 files changed, 163 insertions(+), 92 deletions(-) create mode 100644 lib/clif-backend/src/call/mod.rs rename lib/{runtime/src => clif-backend/src/call}/recovery.rs (97%) rename lib/{runtime/src => clif-backend/src/call}/sighandler.rs (97%) diff --git a/lib/clif-backend/Cargo.toml b/lib/clif-backend/Cargo.toml index 60100bb73c0..8c29c803a8a 100644 --- a/lib/clif-backend/Cargo.toml +++ b/lib/clif-backend/Cargo.toml @@ -14,3 +14,8 @@ hashbrown = "0.1" target-lexicon = "0.2.0" wasmparser = "0.23.0" byteorder = "1" +nix = "0.12.0" +# We depend on libffi for now. +# This will be removed asap. +libffi = "0.6.4" + diff --git a/lib/clif-backend/src/call/mod.rs b/lib/clif-backend/src/call/mod.rs new file mode 100644 index 00000000000..9c87fa8c90b --- /dev/null +++ b/lib/clif-backend/src/call/mod.rs @@ -0,0 +1,125 @@ +mod recovery; +mod sighandler; + +use crate::call::recovery::call_protected; +use wasmer_runtime::{ + backend::{Token, ProtectedCaller}, + types::{FuncIndex, Value, Type, FuncSig, LocalOrImport}, + module::ModuleInner, + error::{RuntimeResult}, + export::Context, + vm::{self, ImportBacking}, +}; +use libffi::high::{arg as libffi_arg, call as libffi_call, CodePtr}; +use hashbrown::HashSet; +use std::iter; + +pub struct Caller { + func_export_set: HashSet, +} + +impl Caller { + pub fn new(module: &ModuleInner) -> Self { + + } +} + +impl ProtectedCaller for Caller { + fn call( + &self, + module: &ModuleInner, + func_index: FuncIndex, + params: &[Value], + returns: &mut [Value], + import_backing: &ImportBacking, + vmctx: *mut vm::Ctx, + _: Token, + ) -> RuntimeResult<()> { + let (func_ptr, ctx, signature) = get_func_from_index(&module, import_backing, func_index); + + let vmctx_ptr = match ctx { + Context::External(external_vmctx) => external_vmctx, + Context::Internal => vmctx, + }; + + assert!(self.func_export_set.contains(&func_index)); + + assert!( + returns.len() == signature.returns.len() && signature.returns.len() <= 1, + "multi-value returns not yet supported" + ); + + assert!(signature.check_sig(params), "incorrect signature"); + + let libffi_args: Vec<_> = params + .iter() + .map(|val| match val { + Value::I32(ref x) => libffi_arg(x), + Value::I64(ref x) => libffi_arg(x), + Value::F32(ref x) => libffi_arg(x), + Value::F64(ref x) => libffi_arg(x), + }) + .chain(iter::once(libffi_arg(&vmctx_ptr))) + .collect(); + + let code_ptr = CodePtr::from_ptr(func_ptr as _); + + call_protected(|| { + // Only supports zero or one return values for now. + // To support multiple returns, we will have to + // generate trampolines instead of using libffi. + match signature.returns.first() { + Some(ty) => { + let val = match ty { + Type::I32 => Value::I32(unsafe { libffi_call(code_ptr, &libffi_args) }), + Type::I64 => Value::I64(unsafe { libffi_call(code_ptr, &libffi_args) }), + Type::F32 => Value::F32(unsafe { libffi_call(code_ptr, &libffi_args) }), + Type::F64 => Value::F64(unsafe { libffi_call(code_ptr, &libffi_args) }), + }; + returns[0] = val; + }, + // call with no returns + None => unsafe { + libffi_call::<()>(code_ptr, &libffi_args); + }, + } + }) + } +} + + +fn get_func_from_index<'a>( + module: &'a ModuleInner, + import_backing: &ImportBacking, + func_index: FuncIndex, +) -> (*const vm::Func, Context, &'a FuncSig) { + let sig_index = *module + .func_assoc + .get(func_index) + .expect("broken invariant, incorrect func index"); + + let (func_ptr, ctx) = match func_index.local_or_import(module) { + LocalOrImport::Local(local_func_index) => { + ( + module + .func_resolver + .get(&module, local_func_index) + .expect("broken invariant, func resolver not synced with module.exports") + .cast() + .as_ptr() as *const _, + Context::Internal, + ) + } + LocalOrImport::Import(imported_func_index) => { + let imported_func = import_backing.imported_func(imported_func_index); + ( + imported_func.func as *const _, + Context::External(imported_func.vmctx), + ) + } + }; + + let signature = module.sig_registry.lookup_func_sig(sig_index); + + (func_ptr, ctx, signature) +} \ No newline at end of file diff --git a/lib/runtime/src/recovery.rs b/lib/clif-backend/src/call/recovery.rs similarity index 97% rename from lib/runtime/src/recovery.rs rename to lib/clif-backend/src/call/recovery.rs index ec7a7cbf20c..3733dc51a3f 100644 --- a/lib/runtime/src/recovery.rs +++ b/lib/clif-backend/src/call/recovery.rs @@ -4,10 +4,10 @@ //! 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 crate::{ +use wasmer_runtime::{ error::{RuntimeError, RuntimeResult}, - sighandler::install_sighandler, }; +use crate::call::sighandler::install_sighandler; use nix::libc::siginfo_t; use nix::sys::signal::{Signal, SIGBUS, SIGFPE, SIGILL, SIGSEGV}; use std::cell::{Cell, UnsafeCell}; diff --git a/lib/runtime/src/sighandler.rs b/lib/clif-backend/src/call/sighandler.rs similarity index 97% rename from lib/runtime/src/sighandler.rs rename to lib/clif-backend/src/call/sighandler.rs index 7d5abd49d0d..e216bd6d75c 100644 --- a/lib/runtime/src/sighandler.rs +++ b/lib/clif-backend/src/call/sighandler.rs @@ -4,7 +4,7 @@ //! //! Please read more about this here: https://github.com/CraneStation/wasmtime/issues/15 //! This code is inspired by: https://github.com/pepyakin/wasmtime/commit/625a2b6c0815b21996e111da51b9664feb174622 -use crate::recovery; +use crate::call::recovery; use nix::libc::{c_void, siginfo_t}; use nix::sys::signal::{ sigaction, SaFlags, SigAction, SigHandler, SigSet, SIGBUS, SIGFPE, SIGILL, SIGSEGV, diff --git a/lib/clif-backend/src/lib.rs b/lib/clif-backend/src/lib.rs index 969348688fd..5899dd51f77 100644 --- a/lib/clif-backend/src/lib.rs +++ b/lib/clif-backend/src/lib.rs @@ -5,6 +5,7 @@ mod module; mod module_env; mod relocation; mod resolver; +mod call; use cranelift_codegen::{ isa, diff --git a/lib/runtime/Cargo.toml b/lib/runtime/Cargo.toml index 08f57ec561d..3618aca990c 100644 --- a/lib/runtime/Cargo.toml +++ b/lib/runtime/Cargo.toml @@ -7,7 +7,6 @@ build = "build/mod.rs" [dependencies] hashbrown = "0.1" -libffi = "0.6.4" nix = "0.12.0" page_size = "0.4.1" errno = "0.2.4" diff --git a/lib/runtime/src/backend.rs b/lib/runtime/src/backend.rs index 0ea709c3b6e..3cf7b094571 100644 --- a/lib/runtime/src/backend.rs +++ b/lib/runtime/src/backend.rs @@ -2,6 +2,7 @@ use crate::{ error::CompileResult, error::RuntimeResult, module::ModuleInner, + backing::ImportBacking, types::{FuncIndex, LocalFuncIndex, Value}, vm, }; @@ -54,6 +55,7 @@ pub trait ProtectedCaller { func_index: FuncIndex, params: &[Value], returns: &mut [Value], + import_backing: &ImportBacking, vmctx: *mut vm::Ctx, _: Token, ) -> RuntimeResult<()>; @@ -62,13 +64,9 @@ pub trait ProtectedCaller { pub trait FuncResolver { /// This returns a pointer to the function designated by the `local_func_index` /// parameter. - /// - /// The existance of the Token parameter ensures that this can only be called from - /// within the runtime crate. fn get( &self, module: &ModuleInner, local_func_index: LocalFuncIndex, - _: Token, ) -> Option>; } diff --git a/lib/runtime/src/backing.rs b/lib/runtime/src/backing.rs index b292a96378d..48a5e626843 100644 --- a/lib/runtime/src/backing.rs +++ b/lib/runtime/src/backing.rs @@ -1,5 +1,4 @@ use crate::{ - backend::Token, error::{LinkError, LinkResult}, export::{Context, Export}, import::Imports, @@ -179,17 +178,14 @@ impl LocalBacking { let sig_id = vm::SigId(sig_index.index() as u32); let func_data = match func_index.local_or_import(module) { - LocalOrImport::Local(local_func_index) => { - let token = Token::generate(); - vm::ImportedFunc { - func: module - .func_resolver - .get(module, local_func_index, token) - .unwrap() - .as_ptr(), - vmctx, - } - } + LocalOrImport::Local(local_func_index) => vm::ImportedFunc { + func: module + .func_resolver + .get(module, local_func_index) + .unwrap() + .as_ptr(), + vmctx, + }, LocalOrImport::Import(imported_func_index) => { imports.functions[imported_func_index].clone() } @@ -233,17 +229,14 @@ impl LocalBacking { let sig_id = vm::SigId(sig_index.index() as u32); let func_data = match func_index.local_or_import(module) { - LocalOrImport::Local(local_func_index) => { - let token = Token::generate(); - vm::ImportedFunc { - func: module - .func_resolver - .get(module, local_func_index, token) - .unwrap() - .as_ptr(), - vmctx, - } - } + LocalOrImport::Local(local_func_index) => vm::ImportedFunc { + func: module + .func_resolver + .get(module, local_func_index) + .unwrap() + .as_ptr(), + vmctx, + }, LocalOrImport::Import(imported_func_index) => { imports.functions[imported_func_index].clone() } @@ -322,6 +315,10 @@ impl ImportBacking { }) } + pub fn imported_func(&self, func_index: ImportedFuncIndex) -> vm::ImportedFunc { + self.functions[func_index].clone() + } + pub fn imported_memory(&self, memory_index: ImportedMemoryIndex) -> vm::ImportedMemory { self.memories[memory_index].clone() } diff --git a/lib/runtime/src/instance.rs b/lib/runtime/src/instance.rs index ef57f29699f..760ad2243ae 100644 --- a/lib/runtime/src/instance.rs +++ b/lib/runtime/src/instance.rs @@ -1,4 +1,3 @@ -use crate::recovery::call_protected; use crate::{ backend::Token, backing::{ImportBacking, LocalBacking}, @@ -10,13 +9,12 @@ use crate::{ module::{ExportIndex, Module, ModuleInner}, types::{ FuncIndex, FuncSig, GlobalDesc, GlobalIndex, LocalOrImport, Memory, MemoryIndex, Table, - TableIndex, Type, Value, + TableIndex, Value, }, vm, }; -use libffi::high::{arg as libffi_arg, call as libffi_call, CodePtr}; use std::rc::Rc; -use std::{iter, mem}; +use std::mem; pub(crate) struct InstanceInner { #[allow(dead_code)] @@ -122,7 +120,7 @@ impl Instance { let mut returns = vec![Value::I32(0); signature.returns.len()]; let vmctx = match func_index.local_or_import(&self.module) { - LocalOrImport::Local(local_func_index) => &mut *self.inner.vmctx, + LocalOrImport::Local(_) => &mut *self.inner.vmctx, LocalOrImport::Import(imported_func_index) => { self.inner.import_backing.functions[imported_func_index].vmctx } @@ -135,61 +133,12 @@ impl Instance { func_index, args, &mut returns, + &self.inner.import_backing, vmctx, token, )?; Ok(returns) - - // let (func_ref, ctx, signature) = self.inner.get_func_from_index(&self.module, func_index); - - // let func_ptr = CodePtr::from_ptr(func_ref.inner() as _); - // let vmctx_ptr = match ctx { - // Context::External(vmctx) => vmctx, - // Context::Internal => &mut *self.inner.vmctx, - // }; - - // assert!( - // signature.returns.len() <= 1, - // "multi-value returns not yet supported" - // ); - - // if !signature.check_sig(args) { - // Err(CallError::Signature { - // expected: signature.clone(), - // found: args.iter().map(|val| val.ty()).collect(), - // })? - // } - - // let libffi_args: Vec<_> = args - // .iter() - // .map(|val| match val { - // Value::I32(ref x) => libffi_arg(x), - // Value::I64(ref x) => libffi_arg(x), - // Value::F32(ref x) => libffi_arg(x), - // Value::F64(ref x) => libffi_arg(x), - // }) - // .chain(iter::once(libffi_arg(&vmctx_ptr))) - // .collect(); - - // Ok(call_protected(|| { - // signature - // .returns - // .first() - // .map(|ty| match ty { - // Type::I32 => Value::I32(unsafe { libffi_call(func_ptr, &libffi_args) }), - // Type::I64 => Value::I64(unsafe { libffi_call(func_ptr, &libffi_args) }), - // Type::F32 => Value::F32(unsafe { libffi_call(func_ptr, &libffi_args) }), - // Type::F64 => Value::F64(unsafe { libffi_call(func_ptr, &libffi_args) }), - // }) - // .or_else(|| { - // // call with no returns - // unsafe { - // libffi_call::<()>(func_ptr, &libffi_args); - // } - // None - // }) - // })?) } } @@ -253,11 +202,10 @@ impl InstanceInner { let (func_ptr, ctx) = match func_index.local_or_import(module) { LocalOrImport::Local(local_func_index) => { - let token = Token::generate(); ( module .func_resolver - .get(&module, local_func_index, token) + .get(&module, local_func_index) .expect("broken invariant, func resolver not synced with module.exports") .cast() .as_ptr() as *const _, @@ -387,7 +335,7 @@ impl Namespace for Instance { // TODO Remove this later, only needed for compilation till emscripten is updated impl Instance { - pub fn memory_offset_addr(&self, index: usize, offset: usize) -> *const u8 { + pub fn memory_offset_addr(&self, _index: usize, _offset: usize) -> *const u8 { unimplemented!() } } diff --git a/lib/runtime/src/lib.rs b/lib/runtime/src/lib.rs index f7517c593dc..ba150cdd3b5 100644 --- a/lib/runtime/src/lib.rs +++ b/lib/runtime/src/lib.rs @@ -14,9 +14,7 @@ pub mod instance; pub mod memory; mod mmap; pub mod module; -mod recovery; mod sig_registry; -mod sighandler; pub mod structures; pub mod table; pub mod types; diff --git a/lib/runtime/src/types.rs b/lib/runtime/src/types.rs index d3082acd466..5fd0d1193a2 100644 --- a/lib/runtime/src/types.rs +++ b/lib/runtime/src/types.rs @@ -159,7 +159,7 @@ pub trait LocalImport { #[rustfmt::skip] macro_rules! define_map_index { ($ty:ident) => { - #[derive(Debug, Copy, Clone, PartialEq, Eq)] + #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct $ty (u32); impl TypedIndex for $ty { #[doc(hidden)] From 9ed001804559c04b9439a281b37d9eb229d03490 Mon Sep 17 00:00:00 2001 From: Lachlan Sneff Date: Fri, 18 Jan 2019 14:30:15 -0800 Subject: [PATCH 5/8] transition to protected_caller --- lib/clif-backend/src/call/mod.rs | 50 +++++++++++++++------------ lib/clif-backend/src/call/recovery.rs | 4 +-- lib/clif-backend/src/lib.rs | 2 +- lib/clif-backend/src/module.rs | 35 +++++++++++++++---- lib/runtime/src/backend.rs | 2 +- lib/runtime/src/instance.rs | 22 ++++++------ lib/runtime/src/module.rs | 1 + 7 files changed, 69 insertions(+), 47 deletions(-) diff --git a/lib/clif-backend/src/call/mod.rs b/lib/clif-backend/src/call/mod.rs index 9c87fa8c90b..d2368da9d2d 100644 --- a/lib/clif-backend/src/call/mod.rs +++ b/lib/clif-backend/src/call/mod.rs @@ -2,17 +2,17 @@ mod recovery; mod sighandler; use crate::call::recovery::call_protected; +use hashbrown::HashSet; +use libffi::high::{arg as libffi_arg, call as libffi_call, CodePtr}; +use std::iter; use wasmer_runtime::{ - backend::{Token, ProtectedCaller}, - types::{FuncIndex, Value, Type, FuncSig, LocalOrImport}, - module::ModuleInner, - error::{RuntimeResult}, + backend::{ProtectedCaller, Token}, + error::RuntimeResult, export::Context, + module::{ExportIndex, ModuleInner}, + types::{FuncIndex, FuncSig, LocalOrImport, Type, Value}, vm::{self, ImportBacking}, }; -use libffi::high::{arg as libffi_arg, call as libffi_call, CodePtr}; -use hashbrown::HashSet; -use std::iter; pub struct Caller { func_export_set: HashSet, @@ -20,7 +20,14 @@ pub struct Caller { impl Caller { pub fn new(module: &ModuleInner) -> Self { - + let mut func_export_set = HashSet::new(); + for export_index in module.exports.values() { + if let ExportIndex::Func(func_index) = export_index { + func_export_set.insert(*func_index); + } + } + + Self { func_export_set } } } @@ -66,7 +73,7 @@ impl ProtectedCaller for Caller { call_protected(|| { // Only supports zero or one return values for now. - // To support multiple returns, we will have to + // To support multiple returns, we will have to // generate trampolines instead of using libffi. match signature.returns.first() { Some(ty) => { @@ -77,7 +84,7 @@ impl ProtectedCaller for Caller { Type::F64 => Value::F64(unsafe { libffi_call(code_ptr, &libffi_args) }), }; returns[0] = val; - }, + } // call with no returns None => unsafe { libffi_call::<()>(code_ptr, &libffi_args); @@ -87,7 +94,6 @@ impl ProtectedCaller for Caller { } } - fn get_func_from_index<'a>( module: &'a ModuleInner, import_backing: &ImportBacking, @@ -99,17 +105,15 @@ fn get_func_from_index<'a>( .expect("broken invariant, incorrect func index"); let (func_ptr, ctx) = match func_index.local_or_import(module) { - LocalOrImport::Local(local_func_index) => { - ( - module - .func_resolver - .get(&module, local_func_index) - .expect("broken invariant, func resolver not synced with module.exports") - .cast() - .as_ptr() as *const _, - Context::Internal, - ) - } + LocalOrImport::Local(local_func_index) => ( + module + .func_resolver + .get(&module, local_func_index) + .expect("broken invariant, func resolver not synced with module.exports") + .cast() + .as_ptr() as *const _, + Context::Internal, + ), LocalOrImport::Import(imported_func_index) => { let imported_func = import_backing.imported_func(imported_func_index); ( @@ -122,4 +126,4 @@ fn get_func_from_index<'a>( let signature = module.sig_registry.lookup_func_sig(sig_index); (func_ptr, ctx, signature) -} \ No newline at end of file +} diff --git a/lib/clif-backend/src/call/recovery.rs b/lib/clif-backend/src/call/recovery.rs index 3733dc51a3f..e186dfe6f8a 100644 --- a/lib/clif-backend/src/call/recovery.rs +++ b/lib/clif-backend/src/call/recovery.rs @@ -4,14 +4,12 @@ //! 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 wasmer_runtime::{ - error::{RuntimeError, RuntimeResult}, -}; use crate::call::sighandler::install_sighandler; use nix::libc::siginfo_t; use nix::sys::signal::{Signal, SIGBUS, SIGFPE, SIGILL, SIGSEGV}; use std::cell::{Cell, UnsafeCell}; use std::sync::Once; +use wasmer_runtime::error::{RuntimeError, RuntimeResult}; extern "C" { pub fn setjmp(env: *mut ::nix::libc::c_void) -> ::nix::libc::c_int; diff --git a/lib/clif-backend/src/lib.rs b/lib/clif-backend/src/lib.rs index 5899dd51f77..7fefcf72874 100644 --- a/lib/clif-backend/src/lib.rs +++ b/lib/clif-backend/src/lib.rs @@ -1,11 +1,11 @@ // pub mod codegen; +mod call; mod func_env; mod libcalls; mod module; mod module_env; mod relocation; mod resolver; -mod call; use cranelift_codegen::{ isa, diff --git a/lib/clif-backend/src/module.rs b/lib/clif-backend/src/module.rs index 6fbb525da3c..886da3f7661 100644 --- a/lib/clif-backend/src/module.rs +++ b/lib/clif-backend/src/module.rs @@ -1,4 +1,4 @@ -use crate::resolver::FuncResolverBuilder; +use crate::{call::Caller, resolver::FuncResolverBuilder}; use cranelift_codegen::{ir, isa}; use cranelift_entity::EntityRef; use cranelift_wasm; @@ -8,20 +8,21 @@ use std::{ ptr::NonNull, }; use wasmer_runtime::{ - backend::FuncResolver, backend::SigRegistry, - error::CompileResult, + backend::{FuncResolver, ProtectedCaller, Token}, + error::{CompileResult, RuntimeResult}, module::ModuleInner, structures::{Map, TypedIndex}, types::{ FuncIndex, FuncSig, GlobalIndex, LocalFuncIndex, MemoryIndex, SigIndex, TableIndex, Type, + Value, }, - vm, + vm::{self, ImportBacking}, }; -struct PlaceholderFuncResolver; +struct Placeholder; -impl FuncResolver for PlaceholderFuncResolver { +impl FuncResolver for Placeholder { fn get( &self, _module: &ModuleInner, @@ -31,6 +32,21 @@ impl FuncResolver for PlaceholderFuncResolver { } } +impl ProtectedCaller for Placeholder { + fn call( + &self, + _module: &ModuleInner, + _func_index: FuncIndex, + _params: &[Value], + _returns: &mut [Value], + _import_backing: &ImportBacking, + _vmctx: *mut vm::Ctx, + _: Token, + ) -> RuntimeResult<()> { + Ok(()) + } +} + /// This contains all of the items in a `ModuleInner` except the `func_resolver`. pub struct Module { pub module: ModuleInner, @@ -41,7 +57,9 @@ impl Module { Self { module: ModuleInner { // this is a placeholder - func_resolver: Box::new(PlaceholderFuncResolver), + func_resolver: Box::new(Placeholder), + protected_caller: Box::new(Placeholder), + memories: Map::new(), globals: Map::new(), tables: Map::new(), @@ -78,6 +96,9 @@ impl Module { let func_resolver_builder = FuncResolverBuilder::new(isa, functions)?; self.module.func_resolver = Box::new(func_resolver_builder.finalize()?); + + self.module.protected_caller = Box::new(Caller::new(&self.module)); + Ok(self.module) } } diff --git a/lib/runtime/src/backend.rs b/lib/runtime/src/backend.rs index 3cf7b094571..e4372f6f36d 100644 --- a/lib/runtime/src/backend.rs +++ b/lib/runtime/src/backend.rs @@ -1,8 +1,8 @@ use crate::{ + backing::ImportBacking, error::CompileResult, error::RuntimeResult, module::ModuleInner, - backing::ImportBacking, types::{FuncIndex, LocalFuncIndex, Value}, vm, }; diff --git a/lib/runtime/src/instance.rs b/lib/runtime/src/instance.rs index 760ad2243ae..97b7e347072 100644 --- a/lib/runtime/src/instance.rs +++ b/lib/runtime/src/instance.rs @@ -13,8 +13,8 @@ use crate::{ }, vm, }; -use std::rc::Rc; use std::mem; +use std::rc::Rc; pub(crate) struct InstanceInner { #[allow(dead_code)] @@ -201,17 +201,15 @@ impl InstanceInner { .expect("broken invariant, incorrect func index"); let (func_ptr, ctx) = match func_index.local_or_import(module) { - LocalOrImport::Local(local_func_index) => { - ( - module - .func_resolver - .get(&module, local_func_index) - .expect("broken invariant, func resolver not synced with module.exports") - .cast() - .as_ptr() as *const _, - Context::Internal, - ) - } + LocalOrImport::Local(local_func_index) => ( + module + .func_resolver + .get(&module, local_func_index) + .expect("broken invariant, func resolver not synced with module.exports") + .cast() + .as_ptr() as *const _, + Context::Internal, + ), LocalOrImport::Import(imported_func_index) => { let imported_func = &self.import_backing.functions[imported_func_index]; ( diff --git a/lib/runtime/src/module.rs b/lib/runtime/src/module.rs index 2ccf5f7c02a..dceea000b06 100644 --- a/lib/runtime/src/module.rs +++ b/lib/runtime/src/module.rs @@ -19,6 +19,7 @@ use std::rc::Rc; pub struct ModuleInner { pub func_resolver: Box, pub protected_caller: Box, + // This are strictly local and the typsystem ensures that. pub memories: Map, pub globals: Map, From c18328aa4c094a70fd8a9696d49f862c9a4157b2 Mon Sep 17 00:00:00 2001 From: Lachlan Sneff Date: Fri, 18 Jan 2019 14:53:46 -0800 Subject: [PATCH 6/8] support the start function --- Cargo.lock | 3 ++- lib/clif-backend/src/call/mod.rs | 3 +++ lib/runtime/build/spectests.rs | 18 +++++++++--------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6bd124e7db4..a86db503360 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1114,6 +1114,8 @@ dependencies = [ "cranelift-native 0.26.0 (registry+https://github.com/rust-lang/crates.io-index)", "cranelift-wasm 0.26.0 (registry+https://github.com/rust-lang/crates.io-index)", "hashbrown 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", + "libffi 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", + "nix 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "target-lexicon 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "wasmer-runtime 0.1.0", "wasmparser 0.23.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1140,7 +1142,6 @@ dependencies = [ "errno 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", "field-offset 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "hashbrown 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", - "libffi 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "nix 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "page_size 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "wabt 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/lib/clif-backend/src/call/mod.rs b/lib/clif-backend/src/call/mod.rs index d2368da9d2d..ef1776ad7f5 100644 --- a/lib/clif-backend/src/call/mod.rs +++ b/lib/clif-backend/src/call/mod.rs @@ -26,6 +26,9 @@ impl Caller { func_export_set.insert(*func_index); } } + if let Some(start_func_index) = module.start_func { + func_export_set.insert(start_func_index); + } Self { func_export_set } } diff --git a/lib/runtime/build/spectests.rs b/lib/runtime/build/spectests.rs index 1ebe128622c..044b5bb35dd 100644 --- a/lib/runtime/build/spectests.rs +++ b/lib/runtime/build/spectests.rs @@ -435,7 +435,7 @@ fn {}_assert_invalid() {{ format!( "fn {func_name}(instance: &mut Instance) {{ println!(\"Executing function {{}}\", \"{func_name}\"); - let result = instance.call(\"{field}\", &[{args_values}]).unwrap().expect(\"Missing result in {func_name}\"); + let result = instance.call(\"{field}\", &[{args_values}]).unwrap().first().expect(\"Missing result in {func_name}\").clone(); {assertion} }}\n", func_name=func_name, @@ -494,7 +494,7 @@ fn {}_assert_invalid() {{ format!( "fn {func_name}(instance: &mut Instance) {{ println!(\"Executing function {{}}\", \"{func_name}\"); - let result = instance.call(\"{field}\", &[{args_values}]).unwrap().expect(\"Missing result in {func_name}\"); + let result = instance.call(\"{field}\", &[{args_values}]).unwrap().first().expect(\"Missing result in {func_name}\").clone(); {assertion} }}\n", func_name=func_name, @@ -557,10 +557,10 @@ fn {}_assert_malformed() {{ } else { "should not use this expect result".to_string() }; - let expected_some_result = if expected.len() > 0 { - format!("Ok(Some({}))", wabt2rust_value(&expected[0])) + let expected_vec_result = if expected.len() > 0 { + format!("Ok(vec![{}])", wabt2rust_value(&expected[0])) } else { - "Ok(None)".to_string() + "Ok(vec![])".to_string() }; let return_type = if expected.len() > 0 { wabt2rust_type(&expected[0]) @@ -580,9 +580,9 @@ fn {}_assert_malformed() {{ let assertion = if expected.len() > 0 && is_nan(&expected[0]) { format!( "let expected = {expected_result}; - if let {return_type_destructure} = result.clone().unwrap().unwrap() {{ - assert!((result as {return_type}).is_nan()); - assert_eq!((result as {return_type}).is_sign_positive(), (expected as {return_type}).is_sign_positive()); + if let {return_type_destructure} = result.clone().unwrap().first().unwrap() {{ + assert!((*result as {return_type}).is_nan()); + assert_eq!((*result as {return_type}).is_sign_positive(), (expected as {return_type}).is_sign_positive()); }} else {{ panic!(\"Unexpected result type {{:?}}\", result); }}", @@ -591,7 +591,7 @@ fn {}_assert_malformed() {{ return_type_destructure=return_type_destructure ) } else { - format!("assert_eq!(result, {});", expected_some_result) + format!("assert_eq!(result, {});", expected_vec_result) }; (func_return, assertion) } From ebeea0c71ca09c733d45322b093440a962eb63ff Mon Sep 17 00:00:00 2001 From: Lachlan Sneff Date: Fri, 18 Jan 2019 16:45:30 -0800 Subject: [PATCH 7/8] handle traps naively --- lib/clif-backend/src/call/mod.rs | 12 +- lib/clif-backend/src/call/recovery.rs | 139 ++++++++++++++++++++---- lib/clif-backend/src/call/sighandler.rs | 10 +- lib/clif-backend/src/module.rs | 4 +- lib/clif-backend/src/relocation.rs | 44 ++++++-- lib/clif-backend/src/resolver.rs | 35 ++++-- lib/runtime/examples/test.rs | 20 ++-- lib/runtime/src/error.rs | 2 + 8 files changed, 199 insertions(+), 67 deletions(-) diff --git a/lib/clif-backend/src/call/mod.rs b/lib/clif-backend/src/call/mod.rs index ef1776ad7f5..47e1ce55578 100644 --- a/lib/clif-backend/src/call/mod.rs +++ b/lib/clif-backend/src/call/mod.rs @@ -1,6 +1,8 @@ mod recovery; mod sighandler; +pub use self::recovery::HandlerData; + use crate::call::recovery::call_protected; use hashbrown::HashSet; use libffi::high::{arg as libffi_arg, call as libffi_call, CodePtr}; @@ -16,10 +18,11 @@ use wasmer_runtime::{ pub struct Caller { func_export_set: HashSet, + handler_data: HandlerData, } impl Caller { - pub fn new(module: &ModuleInner) -> Self { + pub fn new(module: &ModuleInner, handler_data: HandlerData) -> Self { let mut func_export_set = HashSet::new(); for export_index in module.exports.values() { if let ExportIndex::Func(func_index) = export_index { @@ -30,7 +33,10 @@ impl Caller { func_export_set.insert(start_func_index); } - Self { func_export_set } + Self { + func_export_set, + handler_data, + } } } @@ -74,7 +80,7 @@ impl ProtectedCaller for Caller { let code_ptr = CodePtr::from_ptr(func_ptr as _); - call_protected(|| { + call_protected(&self.handler_data, || { // Only supports zero or one return values for now. // To support multiple returns, we will have to // generate trampolines instead of using libffi. diff --git a/lib/clif-backend/src/call/recovery.rs b/lib/clif-backend/src/call/recovery.rs index e186dfe6f8a..f96badf567f 100644 --- a/lib/clif-backend/src/call/recovery.rs +++ b/lib/clif-backend/src/call/recovery.rs @@ -5,11 +5,18 @@ //! unless you have memory unsafety elsewhere in your code. use crate::call::sighandler::install_sighandler; -use nix::libc::siginfo_t; +use crate::relocation::{TrapData, TrapSink}; +use cranelift_codegen::ir::TrapCode; +use nix::libc::{c_void, siginfo_t}; use nix::sys::signal::{Signal, SIGBUS, SIGFPE, SIGILL, SIGSEGV}; use std::cell::{Cell, UnsafeCell}; +use std::ptr; use std::sync::Once; -use wasmer_runtime::error::{RuntimeError, RuntimeResult}; +use wasmer_runtime::{ + error::{RuntimeError, RuntimeResult}, + structures::TypedIndex, + types::{MemoryIndex, TableIndex}, +}; extern "C" { pub fn setjmp(env: *mut ::nix::libc::c_void) -> ::nix::libc::c_int; @@ -21,10 +28,39 @@ pub static SIGHANDLER_INIT: Once = Once::new(); thread_local! { pub static SETJMP_BUFFER: UnsafeCell<[::nix::libc::c_int; SETJMP_BUFFER_LEN]> = UnsafeCell::new([0; SETJMP_BUFFER_LEN]); - pub static CAUGHT_ADDRESS: Cell = Cell::new(0); + pub static CAUGHT_ADDRESSES: Cell<(*const c_void, *const c_void)> = Cell::new((ptr::null(), ptr::null())); + pub static CURRENT_EXECUTABLE_BUFFER: Cell<*const c_void> = Cell::new(ptr::null()); } -pub fn call_protected(f: impl FnOnce() -> T) -> RuntimeResult { +pub struct HandlerData { + trap_data: TrapSink, + buffer_ptr: *const c_void, + buffer_size: usize, +} + +impl HandlerData { + pub fn new(trap_data: TrapSink, buffer_ptr: *const c_void, buffer_size: usize) -> Self { + Self { + trap_data, + buffer_ptr, + buffer_size, + } + } + + pub fn lookup(&self, ip: *const c_void) -> Option { + let ip = ip as usize; + let buffer_ptr = self.buffer_ptr as usize; + + if buffer_ptr <= ip && ip < buffer_ptr + self.buffer_size { + let offset = ip - buffer_ptr; + self.trap_data.lookup(offset) + } else { + None + } + } +} + +pub fn call_protected(handler_data: &HandlerData, f: impl FnOnce() -> T) -> RuntimeResult { unsafe { let jmp_buf = SETJMP_BUFFER.with(|buf| buf.get()); let prev_jmp_buf = *jmp_buf; @@ -36,21 +72,59 @@ pub fn call_protected(f: impl FnOnce() -> T) -> RuntimeResult { let signum = setjmp(jmp_buf as *mut ::nix::libc::c_void); if signum != 0 { *jmp_buf = prev_jmp_buf; - let addr = CAUGHT_ADDRESS.with(|cell| cell.get()); - - let signal = match Signal::from_c_int(signum) { - Ok(SIGFPE) => "floating-point exception", - Ok(SIGILL) => "illegal instruction", - Ok(SIGSEGV) => "segmentation violation", - Ok(SIGBUS) => "bus error", - Err(_) => "error while getting the Signal", - _ => "unkown trapped signal", - }; - // When the trap-handler is fully implemented, this will return more information. - Err(RuntimeError::Unknown { - msg: format!("trap at {:#x} - {}", addr, signal), + let (faulting_addr, _) = CAUGHT_ADDRESSES.with(|cell| cell.get()); + + if let Some(TrapData { + trapcode, + srcloc: _, + }) = handler_data.lookup(faulting_addr) + { + Err(match Signal::from_c_int(signum) { + Ok(SIGILL) => match trapcode { + TrapCode::BadSignature => RuntimeError::IndirectCallSignature { + table: TableIndex::new(0), + }, + TrapCode::IndirectCallToNull => RuntimeError::IndirectCallToNull { + table: TableIndex::new(0), + }, + TrapCode::HeapOutOfBounds => RuntimeError::OutOfBoundsAccess { + memory: MemoryIndex::new(0), + addr: 0, + }, + TrapCode::TableOutOfBounds => RuntimeError::TableOutOfBounds { + table: TableIndex::new(0), + }, + _ => RuntimeError::Unknown { + msg: "unknown trap".to_string(), + }, + }, + Ok(SIGSEGV) | Ok(SIGBUS) => { + // I'm too lazy right now to actually check if the address is within one of the memories, + // so just say that it's a memory-out-of-bounds access for now + RuntimeError::OutOfBoundsAccess { + memory: MemoryIndex::new(0), + addr: 0, + } + } + Ok(SIGFPE) => RuntimeError::IllegalArithmeticOperation, + _ => unimplemented!(), + } + .into()) + } else { + let signal = match Signal::from_c_int(signum) { + Ok(SIGFPE) => "floating-point exception", + Ok(SIGILL) => "illegal instruction", + Ok(SIGSEGV) => "segmentation violation", + Ok(SIGBUS) => "bus error", + Err(_) => "error while getting the Signal", + _ => "unkown trapped signal", + }; + // When the trap-handler is fully implemented, this will return more information. + Err(RuntimeError::Unknown { + msg: format!("trap at {:p} - {}", faulting_addr, signal), + } + .into()) } - .into()) } else { let ret = f(); // TODO: Switch stack? *jmp_buf = prev_jmp_buf; @@ -60,7 +134,7 @@ pub fn call_protected(f: impl FnOnce() -> T) -> RuntimeResult { } /// Unwinds to last protected_call. -pub unsafe fn do_unwind(signum: i32, siginfo: *mut siginfo_t) -> ! { +pub unsafe fn do_unwind(signum: i32, siginfo: *mut siginfo_t, ucontext: *const c_void) -> ! { // Since do_unwind is only expected to get called from WebAssembly code which doesn't hold any host resources (locks etc.) // itself, accessing TLS here is safe. In case any other code calls this, it often indicates a memory safety bug and you should // temporarily disable the signal handlers to debug it. @@ -69,9 +143,30 @@ pub unsafe fn do_unwind(signum: i32, siginfo: *mut siginfo_t) -> ! { if *jmp_buf == [0; SETJMP_BUFFER_LEN] { ::std::process::abort(); } - // We only target macos at the moment as other ones might not have si_addr field - #[cfg(target_os = "macos")] - CAUGHT_ADDRESS.with(|cell| cell.set((*siginfo).si_addr as _)); + + CAUGHT_ADDRESSES.with(|cell| cell.set(get_faulting_addr_and_ip(siginfo, ucontext))); longjmp(jmp_buf as *mut ::nix::libc::c_void, signum) } + +#[cfg(all(target_os = "linux", target_arch = "x86_64"))] +unsafe fn get_faulting_addr_and_ip( + siginfo: *mut siginfo_t, + _ucontext: *const c_void, +) -> (*const c_void, *const c_void) { + (ptr::null(), ptr::null()) +} + +#[cfg(all(target_os = "macos", target_arch = "x86_64"))] +unsafe fn get_faulting_addr_and_ip( + siginfo: *mut siginfo_t, + _ucontext: *const c_void, +) -> (*const c_void, *const c_void) { + ((*siginfo).si_addr, ptr::null()) +} + +#[cfg(not(any( + all(target_os = "macos", target_arch = "x86_64"), + all(target_os = "linux", target_arch = "x86_64"), +)))] +compile_error!("This crate doesn't yet support compiling on operating systems other than linux and macos and architectures other than x86_64"); diff --git a/lib/clif-backend/src/call/sighandler.rs b/lib/clif-backend/src/call/sighandler.rs index e216bd6d75c..b3455202e53 100644 --- a/lib/clif-backend/src/call/sighandler.rs +++ b/lib/clif-backend/src/call/sighandler.rs @@ -1,8 +1,6 @@ -//! We install signal handlers to handle WebAssembly traps within -//! our Rust code. Otherwise we will have errors that stop the Rust process -//! such as `process didn't exit successfully: ... (signal: 8, SIGFPE: erroneous arithmetic operation)` +//! Installing signal handlers allows us to handle traps and out-of-bounds memory +//! accesses that occur when runniing webassembly. //! -//! Please read more about this here: https://github.com/CraneStation/wasmtime/issues/15 //! This code is inspired by: https://github.com/pepyakin/wasmtime/commit/625a2b6c0815b21996e111da51b9664feb174622 use crate::call::recovery; use nix::libc::{c_void, siginfo_t}; @@ -25,9 +23,9 @@ pub unsafe fn install_sighandler() { extern "C" fn signal_trap_handler( signum: ::nix::libc::c_int, siginfo: *mut siginfo_t, - _ucontext: *mut c_void, + ucontext: *mut c_void, ) { unsafe { - recovery::do_unwind(signum, siginfo); + recovery::do_unwind(signum, siginfo, ucontext); } } diff --git a/lib/clif-backend/src/module.rs b/lib/clif-backend/src/module.rs index 886da3f7661..665a98d90da 100644 --- a/lib/clif-backend/src/module.rs +++ b/lib/clif-backend/src/module.rs @@ -94,10 +94,10 @@ impl Module { *sig_index = sig_registry.lookup_deduplicated_sigindex(*sig_index); }); - let func_resolver_builder = FuncResolverBuilder::new(isa, functions)?; + let (func_resolver_builder, handler_data) = FuncResolverBuilder::new(isa, functions)?; self.module.func_resolver = Box::new(func_resolver_builder.finalize()?); - self.module.protected_caller = Box::new(Caller::new(&self.module)); + self.module.protected_caller = Box::new(Caller::new(&self.module, handler_data)); Ok(self.module) } diff --git a/lib/clif-backend/src/relocation.rs b/lib/clif-backend/src/relocation.rs index 8c5c5930490..5da1440a63a 100644 --- a/lib/clif-backend/src/relocation.rs +++ b/lib/clif-backend/src/relocation.rs @@ -3,11 +3,11 @@ //! any other calls that this function is doing, so we can "patch" the //! function addrs in runtime with the functions we need. use cranelift_codegen::binemit; +pub use cranelift_codegen::binemit::Reloc; use cranelift_codegen::ir::{self, ExternalName, LibCall, SourceLoc, TrapCode}; +use hashbrown::HashMap; use wasmer_runtime::{structures::TypedIndex, types::LocalFuncIndex}; -pub use cranelift_codegen::binemit::Reloc; - #[derive(Debug, Clone)] pub struct Relocation { /// The relocation code. @@ -133,30 +133,50 @@ impl RelocSink { } } +#[derive(Debug, Clone, Copy)] pub struct TrapData { - pub offset: usize, - pub code: TrapCode, + pub trapcode: TrapCode, + pub srcloc: SourceLoc, } /// Simple implementation of a TrapSink /// that saves the info for later. pub struct TrapSink { - trap_datas: Vec, + trap_datas: HashMap, } impl TrapSink { pub fn new() -> TrapSink { TrapSink { - trap_datas: Vec::new(), + trap_datas: HashMap::new(), } } -} -impl binemit::TrapSink for TrapSink { - fn trap(&mut self, offset: u32, _: SourceLoc, code: TrapCode) { - self.trap_datas.push(TrapData { - offset: offset as usize, - code, + pub fn lookup(&self, offset: usize) -> Option { + self.trap_datas.get(&offset).cloned() + } + + pub fn drain_local(&mut self, current_func_offset: usize, local: &mut LocalTrapSink) { + local.trap_datas.drain(..).for_each(|(offset, trap_data)| { + self.trap_datas + .insert(current_func_offset + offset, trap_data); }); } } + +pub struct LocalTrapSink { + trap_datas: Vec<(usize, TrapData)>, +} + +impl LocalTrapSink { + pub fn new() -> Self { + LocalTrapSink { trap_datas: vec![] } + } +} + +impl binemit::TrapSink for LocalTrapSink { + fn trap(&mut self, offset: u32, srcloc: SourceLoc, trapcode: TrapCode) { + self.trap_datas + .push((offset as usize, TrapData { trapcode, srcloc })); + } +} diff --git a/lib/clif-backend/src/resolver.rs b/lib/clif-backend/src/resolver.rs index 6cc783be677..a1ff7f3aa2a 100644 --- a/lib/clif-backend/src/resolver.rs +++ b/lib/clif-backend/src/resolver.rs @@ -1,5 +1,8 @@ +use crate::call::HandlerData; use crate::libcalls; -use crate::relocation::{Reloc, RelocSink, Relocation, RelocationType, TrapSink, VmCall}; +use crate::relocation::{ + LocalTrapSink, Reloc, RelocSink, Relocation, RelocationType, TrapSink, VmCall, +}; use byteorder::{ByteOrder, LittleEndian}; use cranelift_codegen::{ir, isa, Context}; use std::mem; @@ -17,17 +20,18 @@ use wasmer_runtime::{ pub struct FuncResolverBuilder { resolver: FuncResolver, relocations: Map>, - trap_sinks: Map, } impl FuncResolverBuilder { pub fn new( isa: &isa::TargetIsa, function_bodies: Map, - ) -> CompileResult { + ) -> CompileResult<(Self, HandlerData)> { let mut compiled_functions: Vec> = Vec::with_capacity(function_bodies.len()); let mut relocations = Map::with_capacity(function_bodies.len()); - let mut trap_sinks = Map::with_capacity(function_bodies.len()); + + let mut trap_sink = TrapSink::new(); + let mut local_trap_sink = LocalTrapSink::new(); let mut ctx = Context::new(); let mut total_size = 0; @@ -36,17 +40,20 @@ impl FuncResolverBuilder { ctx.func = func; let mut code_buf = Vec::new(); let mut reloc_sink = RelocSink::new(); - let mut trap_sink = TrapSink::new(); - ctx.compile_and_emit(isa, &mut code_buf, &mut reloc_sink, &mut trap_sink) + ctx.compile_and_emit(isa, &mut code_buf, &mut reloc_sink, &mut local_trap_sink) .map_err(|e| CompileError::InternalError { msg: e.to_string() })?; ctx.clear(); + + // Clear the local trap sink and consolidate all trap info + // into a single location. + trap_sink.drain_local(total_size, &mut local_trap_sink); + // Round up each function's size to pointer alignment. total_size += round_up(code_buf.len(), mem::size_of::()); compiled_functions.push(code_buf); relocations.push(reloc_sink.func_relocs); - trap_sinks.push(trap_sink); } let mut memory = Mmap::with_size(total_size) @@ -84,11 +91,15 @@ impl FuncResolverBuilder { previous_end = new_end; } - Ok(Self { - resolver: FuncResolver { map, memory }, - relocations, - trap_sinks, - }) + let handler_data = HandlerData::new(trap_sink, memory.as_ptr() as _, memory.size()); + + Ok(( + Self { + resolver: FuncResolver { map, memory }, + relocations, + }, + handler_data, + )) } pub fn finalize(mut self) -> CompileResult { diff --git a/lib/runtime/examples/test.rs b/lib/runtime/examples/test.rs index a2096561562..cde7aa04d12 100644 --- a/lib/runtime/examples/test.rs +++ b/lib/runtime/examples/test.rs @@ -4,7 +4,7 @@ use wasmer_runtime::{import::Imports, Instance}; fn main() { let mut instance = create_module_1(); - let result = instance.call("get-0", &[]); + let result = instance.call("call-overwritten", &[]); println!("result: {:?}", result); } @@ -19,15 +19,15 @@ fn main() { fn create_module_1() -> Instance { let module_str = r#"(module - (type (;0;) (func (result i32))) - (import "spectest" "global_i32" (global (;0;) i32)) - (func (;0;) (type 0) (result i32) - get_global 0) - (func (;1;) (type 0) (result i32) - get_global 1) - (global (;1;) i32 (get_global 0)) - (export "get-0" (func 0)) - (export "get-0-ref" (func 1))) + (type (;0;) (func (result i32))) + (type (;1;) (func)) + (table 10 anyfunc) + (elem (i32.const 0) 0) + (func (;0;) (type 0) (i32.const 65)) + (func (;1;) (type 1)) + (func (export "call-overwritten") (type 0) + (call_indirect (type 0) (i32.const 0)) + )) "#; let wasm_binary = wat2wasm(module_str.as_bytes()).expect("WAST not valid or malformed"); let module = wasmer_runtime::compile(&wasm_binary[..], &CraneliftCompiler::new()) diff --git a/lib/runtime/src/error.rs b/lib/runtime/src/error.rs index 3a350748b01..df9d429430c 100644 --- a/lib/runtime/src/error.rs +++ b/lib/runtime/src/error.rs @@ -80,8 +80,10 @@ impl PartialEq for LinkError { #[derive(Debug, Clone)] pub enum RuntimeError { OutOfBoundsAccess { memory: MemoryIndex, addr: u32 }, + TableOutOfBounds { table: TableIndex }, IndirectCallSignature { table: TableIndex }, IndirectCallToNull { table: TableIndex }, + IllegalArithmeticOperation, Unknown { msg: String }, } From de046491d2e6708e451862fbfe50ff3c9370d45c Mon Sep 17 00:00:00 2001 From: Lachlan Sneff Date: Mon, 21 Jan 2019 13:43:48 -0800 Subject: [PATCH 8/8] finish support for traps --- lib/clif-backend/src/call/recovery.rs | 35 ++++++++++++++++++++------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/lib/clif-backend/src/call/recovery.rs b/lib/clif-backend/src/call/recovery.rs index f96badf567f..f5ad6015046 100644 --- a/lib/clif-backend/src/call/recovery.rs +++ b/lib/clif-backend/src/call/recovery.rs @@ -87,10 +87,21 @@ pub fn call_protected(handler_data: &HandlerData, f: impl FnOnce() -> T) -> R TrapCode::IndirectCallToNull => RuntimeError::IndirectCallToNull { table: TableIndex::new(0), }, - TrapCode::HeapOutOfBounds => RuntimeError::OutOfBoundsAccess { - memory: MemoryIndex::new(0), - addr: 0, - }, + TrapCode::HeapOutOfBounds => { + let addr = + (faulting_addr as usize) - (handler_data.buffer_ptr as usize); + if addr <= handler_data.buffer_size { + // in the memory + RuntimeError::OutOfBoundsAccess { + memory: MemoryIndex::new(0), + addr: addr as u32, + } + } else { + // if there's an invalid access outside of the memory, including guard pages + // just kill the process. + panic!("invalid memory access, way out of bounds") + } + } TrapCode::TableOutOfBounds => RuntimeError::TableOutOfBounds { table: TableIndex::new(0), }, @@ -99,11 +110,17 @@ pub fn call_protected(handler_data: &HandlerData, f: impl FnOnce() -> T) -> R }, }, Ok(SIGSEGV) | Ok(SIGBUS) => { - // I'm too lazy right now to actually check if the address is within one of the memories, - // so just say that it's a memory-out-of-bounds access for now - RuntimeError::OutOfBoundsAccess { - memory: MemoryIndex::new(0), - addr: 0, + let addr = (faulting_addr as usize) - (handler_data.buffer_ptr as usize); + if addr <= handler_data.buffer_size { + // in the memory + RuntimeError::OutOfBoundsAccess { + memory: MemoryIndex::new(0), + addr: addr as u32, + } + } else { + // if there's an invalid access outside of the memory, including guard pages + // just kill the process. + panic!("invalid memory access, way out of bounds") } } Ok(SIGFPE) => RuntimeError::IllegalArithmeticOperation,