From 6d01fd41034045e44d301c1f76fc93ea8427c6be Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 13 Feb 2020 18:03:45 -0800 Subject: [PATCH 01/11] deps: Update `wat` to 1.0.9 --- Cargo.toml | 2 +- crates/api/Cargo.toml | 2 +- crates/fuzzing/Cargo.toml | 2 +- crates/lightbeam/Cargo.toml | 2 +- crates/test-programs/Cargo.toml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 372369823adb..6aa979ad6477 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ anyhow = "1.0.19" target-lexicon = { version = "0.10.0", default-features = false } pretty_env_logger = "0.3.0" file-per-thread-logger = "0.1.1" -wat = "1.0.2" +wat = "1.0.10" libc = "0.2.60" rayon = "1.2.1" wasm-webidl-bindings = "0.8" diff --git a/crates/api/Cargo.toml b/crates/api/Cargo.toml index a443d5db0b70..652930c0e320 100644 --- a/crates/api/Cargo.toml +++ b/crates/api/Cargo.toml @@ -33,7 +33,7 @@ wasi-common = { path = "../wasi-common", version = "0.12.0" } pretty_env_logger = "0.3.0" rayon = "1.2.1" file-per-thread-logger = "0.1.1" -wat = "1.0" +wat = "1.0.10" tempfile = "3.1" [badges] diff --git a/crates/fuzzing/Cargo.toml b/crates/fuzzing/Cargo.toml index 658659ffd66e..74eec7c21372 100644 --- a/crates/fuzzing/Cargo.toml +++ b/crates/fuzzing/Cargo.toml @@ -17,4 +17,4 @@ wasmprinter = "0.2.1" wasmtime = { path = "../api", version = "0.12.0" } [dev-dependencies] -wat = "1.0" +wat = "1.0.10" diff --git a/crates/lightbeam/Cargo.toml b/crates/lightbeam/Cargo.toml index dbe63c7011b1..f51889b9b2a7 100644 --- a/crates/lightbeam/Cargo.toml +++ b/crates/lightbeam/Cargo.toml @@ -27,7 +27,7 @@ more-asserts = "0.2.1" [dev-dependencies] lazy_static = "1.2" -wat = "1.0.2" +wat = "1.0.9" quickcheck = "0.9.0" anyhow = "1.0" diff --git a/crates/test-programs/Cargo.toml b/crates/test-programs/Cargo.toml index 7a57e79e3881..60b8b8508199 100644 --- a/crates/test-programs/Cargo.toml +++ b/crates/test-programs/Cargo.toml @@ -18,7 +18,7 @@ pretty_env_logger = "0.3.0" tempfile = "3.1.0" os_pipe = "0.9" anyhow = "1.0.19" -wat = "1.0.2" +wat = "1.0.10" [features] test_programs = [] From 33b4a37bcb04144727aca90322b1d6993bad88f2 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 7 Feb 2020 14:05:33 -0800 Subject: [PATCH 02/11] Add support for `table.copy` This adds support for the `table.copy` instruction from the bulk memory proposal. It also supports multiple tables, which were introduced by the reference types proposal. Part of #928 --- build.rs | 2 + crates/api/src/externals.rs | 39 +++- crates/environ/src/func_environ.rs | 187 ++++++++++++++++-- crates/environ/src/module.rs | 6 +- crates/environ/src/module_environ.rs | 31 ++- crates/runtime/src/instance.rs | 46 +++-- crates/runtime/src/lib.rs | 5 +- crates/runtime/src/libcalls.rs | 95 ++++++++- crates/runtime/src/table.rs | 54 ++++- crates/runtime/src/trap_registry.rs | 2 +- crates/runtime/src/traphandlers.rs | 16 ++ crates/runtime/src/vmcontext.rs | 16 ++ tests/misc_testsuite/table_copy.wast | 63 ++++++ .../table_copy_on_imported_tables.wast | 165 ++++++++++++++++ tests/wast_testsuites.rs | 13 ++ 15 files changed, 696 insertions(+), 44 deletions(-) create mode 100644 tests/misc_testsuite/table_copy.wast create mode 100644 tests/misc_testsuite/table_copy_on_imported_tables.wast diff --git a/build.rs b/build.rs index 0678bdc9aa33..718ee96cc55a 100644 --- a/build.rs +++ b/build.rs @@ -181,6 +181,8 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { ("simd", "simd_splat") => return true, // FIXME Unsupported feature: proposed SIMD operator I8x16ShrS ("reference_types", _) => return true, + + ("bulk_memory_operations", "table_copy") => return false, ("bulk_memory_operations", _) => return true, _ => {} diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index 12a0c5ed34ea..1735c10be65a 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -5,8 +5,8 @@ use crate::{ExternType, GlobalType, MemoryType, TableType, ValType}; use crate::{Func, Store}; use anyhow::{anyhow, bail, Result}; use std::slice; -use wasmtime_environ::wasm; -use wasmtime_runtime::InstanceHandle; +use wasmtime_environ::{ir, wasm}; +use wasmtime_runtime::{self as runtime, InstanceHandle}; // Externals @@ -407,6 +407,41 @@ impl Table { } } + /// Copy `len` elements from `src_table[src_index..]` into + /// `dst_table[dst_index..]`. + /// + /// # Errors + /// + /// Returns an error if the range is out of bounds of either the source or + /// destination tables. + pub fn copy( + dst_table: &Table, + dst_index: u32, + src_table: &Table, + src_index: u32, + len: u32, + ) -> Result<()> { + // NB: We must use the `dst_table`'s `wasmtime_handle` for the + // `dst_table_index` and vice versa for `src_table` since each table can + // come from different modules. + + let dst_table_index = dst_table.wasmtime_table_index(); + let dst_table = dst_table.wasmtime_handle.get_defined_table(dst_table_index); + + let src_table_index = src_table.wasmtime_table_index(); + let src_table = src_table.wasmtime_handle.get_defined_table(src_table_index); + + runtime::Table::copy( + dst_table, + src_table, + dst_index, + src_index, + len, + ir::SourceLoc::default(), + )?; + Ok(()) + } + pub(crate) fn wasmtime_export(&self) -> &wasmtime_runtime::Export { &self.wasmtime_export } diff --git a/crates/environ/src/func_environ.rs b/crates/environ/src/func_environ.rs index 14857bbb6d9a..737e762a024e 100644 --- a/crates/environ/src/func_environ.rs +++ b/crates/environ/src/func_environ.rs @@ -23,6 +23,7 @@ pub fn get_func_name(func_index: FuncIndex) -> ir::ExternalName { } /// An index type for builtin functions. +#[derive(Copy, Clone, Debug)] pub struct BuiltinFunctionIndex(u32); impl BuiltinFunctionIndex { @@ -42,9 +43,28 @@ impl BuiltinFunctionIndex { pub const fn get_imported_memory32_size_index() -> Self { Self(3) } + /// Returns an index for wasm's `table.copy` when both tables are locally + /// defined. + pub const fn get_table_copy_defined_defined_index() -> Self { + Self(4) + } + /// Returns an index for wasm's `table.copy` when the destination table is + /// locally defined and the source table is imported. + pub const fn get_table_copy_defined_imported_index() -> Self { + Self(5) + } + /// Returns an index for wasm's `table.copy` when the destination table is + /// imported and the source table is locally defined. + pub const fn get_table_copy_imported_defined_index() -> Self { + Self(6) + } + /// Returns an index for wasm's `table.copy` when both tables are imported. + pub const fn get_table_copy_imported_imported_index() -> Self { + Self(7) + } /// Returns the total number of builtin functions. pub const fn builtin_functions_total_number() -> u32 { - 4 + 8 } /// Return the index as an u32 number. @@ -72,6 +92,10 @@ pub struct FuncEnvironment<'module_environment> { /// for locally-defined memories. memory_grow_sig: Option, + /// The external function signature for implementing wasm's `table.copy` + /// (it's the same for both local and imported tables). + table_copy_sig: Option, + /// Offsets to struct fields accessed by JIT code. offsets: VMOffsets, } @@ -87,6 +111,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> { vmctx: None, memory32_size_sig: None, memory_grow_sig: None, + table_copy_sig: None, offsets: VMOffsets::new(target_config.pointer_bytes(), module), } } @@ -178,6 +203,97 @@ impl<'module_environment> FuncEnvironment<'module_environment> { } } + // NB: All `table_copy` libcall variants have the same signature. + fn get_table_copy_sig(&mut self, func: &mut Function) -> ir::SigRef { + let sig = self.table_copy_sig.unwrap_or_else(|| { + func.import_signature(Signature { + params: vec![ + AbiParam::special(self.pointer_type(), ArgumentPurpose::VMContext), + // Destination table index. + AbiParam::new(I32), + // Source table index. + AbiParam::new(I32), + // Index within destination table. + AbiParam::new(I32), + // Index within source table. + AbiParam::new(I32), + // Number of elements to copy. + AbiParam::new(I32), + // Source location. + AbiParam::new(I32), + ], + returns: vec![], + call_conv: self.target_config.default_call_conv, + }) + }); + self.table_copy_sig = Some(sig); + sig + } + + fn get_table_copy_func( + &mut self, + func: &mut Function, + dst_table_index: TableIndex, + src_table_index: TableIndex, + ) -> (ir::SigRef, usize, usize, BuiltinFunctionIndex) { + let sig = self.get_table_copy_sig(func); + match ( + self.module.is_imported_table(dst_table_index), + self.module.is_imported_table(src_table_index), + ) { + (false, false) => { + let dst_table_index = self + .module + .defined_table_index(dst_table_index) + .unwrap() + .index(); + let src_table_index = self + .module + .defined_table_index(src_table_index) + .unwrap() + .index(); + ( + sig, + dst_table_index, + src_table_index, + BuiltinFunctionIndex::get_table_copy_defined_defined_index(), + ) + } + (false, true) => { + let dst_table_index = self + .module + .defined_table_index(dst_table_index) + .unwrap() + .index(); + ( + sig, + dst_table_index, + src_table_index.as_u32() as usize, + BuiltinFunctionIndex::get_table_copy_defined_imported_index(), + ) + } + (true, false) => { + let src_table_index = self + .module + .defined_table_index(src_table_index) + .unwrap() + .index(); + ( + sig, + dst_table_index.as_u32() as usize, + src_table_index, + BuiltinFunctionIndex::get_table_copy_imported_defined_index(), + ) + } + (true, true) => ( + sig, + dst_table_index.as_u32() as usize, + src_table_index.as_u32() as usize, + BuiltinFunctionIndex::get_table_copy_imported_imported_index(), + ), + } + } + /// Translates load of builtin function and returns a pair of values `vmctx` /// and address of the loaded function. fn translate_load_builtin_function_address( @@ -782,7 +898,9 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m _src: ir::Value, _len: ir::Value, ) -> WasmResult<()> { - Err(WasmError::Unsupported("bulk memory".to_string())) + Err(WasmError::Unsupported( + "bulk memory: `memory.copy`".to_string(), + )) } fn translate_memory_fill( @@ -794,7 +912,9 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m _val: ir::Value, _len: ir::Value, ) -> WasmResult<()> { - Err(WasmError::Unsupported("bulk memory".to_string())) + Err(WasmError::Unsupported( + "bulk memory: `memory.fill`".to_string(), + )) } fn translate_memory_init( @@ -807,11 +927,15 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m _src: ir::Value, _len: ir::Value, ) -> WasmResult<()> { - Err(WasmError::Unsupported("bulk memory".to_string())) + Err(WasmError::Unsupported( + "bulk memory: `memory.copy`".to_string(), + )) } fn translate_data_drop(&mut self, _pos: FuncCursor, _seg_index: u32) -> WasmResult<()> { - Err(WasmError::Unsupported("bulk memory".to_string())) + Err(WasmError::Unsupported( + "bulk memory: `data.drop`".to_string(), + )) } fn translate_table_size( @@ -820,21 +944,50 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m _index: TableIndex, _table: ir::Table, ) -> WasmResult { - Err(WasmError::Unsupported("bulk memory".to_string())) + Err(WasmError::Unsupported( + "bulk memory: `table.size`".to_string(), + )) } fn translate_table_copy( &mut self, - _pos: FuncCursor, - _dst_table_index: TableIndex, + mut pos: FuncCursor, + dst_table_index: TableIndex, _dst_table: ir::Table, - _src_table_index: TableIndex, + src_table_index: TableIndex, _src_table: ir::Table, - _dst: ir::Value, - _src: ir::Value, - _len: ir::Value, + dst: ir::Value, + src: ir::Value, + len: ir::Value, ) -> WasmResult<()> { - Err(WasmError::Unsupported("bulk memory".to_string())) + use cranelift_codegen::cursor::Cursor; + + let (func_sig, dst_table_index_arg, src_table_index_arg, func_idx) = + self.get_table_copy_func(&mut pos.func, dst_table_index, src_table_index); + + let dst_table_index_arg = pos.ins().iconst(I32, dst_table_index_arg as i64); + let src_table_index_arg = pos.ins().iconst(I32, src_table_index_arg as i64); + + let src_loc = pos.srcloc(); + let src_loc_arg = pos.ins().iconst(I32, src_loc.bits() as i64); + + let (vmctx, func_addr) = self.translate_load_builtin_function_address(&mut pos, func_idx); + + pos.ins().call_indirect( + func_sig, + func_addr, + &[ + vmctx, + dst_table_index_arg, + src_table_index_arg, + dst, + src, + len, + src_loc_arg, + ], + ); + + Ok(()) } fn translate_table_init( @@ -847,10 +1000,14 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m _src: ir::Value, _len: ir::Value, ) -> WasmResult<()> { - Err(WasmError::Unsupported("bulk memory".to_string())) + Err(WasmError::Unsupported( + "bulk memory: `table.init`".to_string(), + )) } fn translate_elem_drop(&mut self, _pos: FuncCursor, _seg_index: u32) -> WasmResult<()> { - Err(WasmError::Unsupported("bulk memory".to_string())) + Err(WasmError::Unsupported( + "bulk memory: `elem.drop`".to_string(), + )) } } diff --git a/crates/environ/src/module.rs b/crates/environ/src/module.rs index 91a2e5272553..499f470a3069 100644 --- a/crates/environ/src/module.rs +++ b/crates/environ/src/module.rs @@ -6,7 +6,7 @@ use cranelift_codegen::ir; use cranelift_entity::{EntityRef, PrimaryMap}; use cranelift_wasm::{ DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, FuncIndex, Global, - GlobalIndex, Memory, MemoryIndex, SignatureIndex, Table, TableIndex, + GlobalIndex, Memory, MemoryIndex, PassiveElemIndex, SignatureIndex, Table, TableIndex, }; use indexmap::IndexMap; use more_asserts::assert_ge; @@ -165,6 +165,9 @@ pub struct Module { /// WebAssembly table initializers. pub table_elements: Vec, + /// WebAssembly passive elements. + pub passive_elements: HashMap>, + /// WebAssembly table initializers. pub func_names: HashMap, } @@ -219,6 +222,7 @@ impl Module { exports: IndexMap::new(), start_func: None, table_elements: Vec::new(), + passive_elements: HashMap::new(), func_names: HashMap::new(), local: ModuleLocal { num_imported_funcs: 0, diff --git a/crates/environ/src/module_environ.rs b/crates/environ/src/module_environ.rs index 0528a3d56cf7..7839bdd643cc 100644 --- a/crates/environ/src/module_environ.rs +++ b/crates/environ/src/module_environ.rs @@ -336,12 +336,20 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data fn declare_passive_element( &mut self, - _: PassiveElemIndex, - _: Box<[FuncIndex]>, + elem_index: PassiveElemIndex, + segments: Box<[FuncIndex]>, ) -> WasmResult<()> { - Err(WasmError::Unsupported( - "bulk memory: passive element segment".into(), - )) + let old = self + .result + .module + .passive_elements + .insert(elem_index, segments); + debug_assert!( + old.is_none(), + "should never get duplicate element indices, that would be a bug in `cranelift_wasm`'s \ + translation" + ); + Ok(()) } fn define_function_body( @@ -382,9 +390,18 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data Ok(()) } - fn declare_passive_data(&mut self, _: PassiveDataIndex, _: &'data [u8]) -> WasmResult<()> { + fn reserve_passive_data(&mut self, count: u32) -> WasmResult<()> { + self.result.module.passive_elements.reserve(count as usize); + Ok(()) + } + + fn declare_passive_data( + &mut self, + _data_index: PassiveDataIndex, + _data: &'data [u8], + ) -> WasmResult<()> { Err(WasmError::Unsupported( - "bulk memory: passive data segment".into(), + "bulk memory: passive data".to_string(), )) } diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 0ed0c4b7511f..c8e4dac5b7f0 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -526,6 +526,30 @@ impl Instance { let align = mem::align_of_val(self); Layout::from_size_align(size, align).unwrap() } + + /// Get a table by index regardless of whether it is locally-defined or an + /// imported, foreign table. + pub(crate) fn get_table(&self, table_index: TableIndex) -> &Table { + if let Some(defined_table_index) = self.module.local.defined_table_index(table_index) { + &self.tables[defined_table_index] + } else { + self.get_foreign_table(table_index) + } + } + + /// Get a locally-defined table. + pub(crate) fn get_defined_table(&self, index: DefinedTableIndex) -> &Table { + &self.tables[index] + } + + /// Get an imported, foreign table. + pub(crate) fn get_foreign_table(&self, index: TableIndex) -> &Table { + let import = self.imported_table(index); + let foreign_instance = unsafe { (&mut *(import).vmctx).instance() }; + let foreign_table = unsafe { &mut *(import).from }; + let foreign_index = foreign_instance.table_index(foreign_table); + &foreign_instance.tables[foreign_index] + } } /// A handle holding an `Instance` of a WebAssembly module. @@ -778,6 +802,11 @@ impl InstanceHandle { self.instance().table_set(table_index, index, val) } + /// Get a table defined locally within this module. + pub fn get_defined_table(&self, index: DefinedTableIndex) -> &Table { + self.instance().get_defined_table(index) + } + /// Return a reference to the contained `Instance`. pub(crate) fn instance(&self) -> &Instance { unsafe { &*(self.instance as *const Instance) } @@ -813,7 +842,7 @@ fn check_table_init_bounds(instance: &Instance) -> Result<(), InstantiationError let module = Arc::clone(&instance.module); for init in &module.table_elements { let start = get_table_init_start(init, instance); - let table = get_table(init, instance); + let table = instance.get_table(init.table_index); let size = usize::try_from(table.size()).unwrap(); if size < start + init.elements.len() { @@ -913,26 +942,13 @@ fn get_table_init_start(init: &TableElements, instance: &Instance) -> usize { start } -/// Return a byte-slice view of a table's data. -fn get_table<'instance>(init: &TableElements, instance: &'instance Instance) -> &'instance Table { - if let Some(defined_table_index) = instance.module.local.defined_table_index(init.table_index) { - &instance.tables[defined_table_index] - } else { - let import = instance.imported_table(init.table_index); - let foreign_instance = unsafe { (&mut *(import).vmctx).instance() }; - let foreign_table = unsafe { &mut *(import).from }; - let foreign_index = foreign_instance.table_index(foreign_table); - &foreign_instance.tables[foreign_index] - } -} - /// Initialize the table memory from the provided initializers. fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { let vmctx = instance.vmctx_ptr(); let module = Arc::clone(&instance.module); for init in &module.table_elements { let start = get_table_init_start(init, instance); - let table = get_table(init, instance); + let table = instance.get_table(init.table_index); for (i, func_idx) in init.elements.iter().enumerate() { let callee_sig = instance.module.local.functions[*func_idx]; diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index f1038019c7f2..736c40e2df41 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -42,9 +42,12 @@ pub use crate::instance::{InstanceHandle, InstantiationError, LinkError}; pub use crate::jit_int::GdbJitImageRegistration; pub use crate::mmap::Mmap; pub use crate::sig_registry::SignatureRegistry; +pub use crate::table::Table; pub use crate::trap_registry::{TrapDescription, TrapRegistration, TrapRegistry}; pub use crate::traphandlers::resume_panic; -pub use crate::traphandlers::{catch_traps, raise_user_trap, wasmtime_call_trampoline, Trap}; +pub use crate::traphandlers::{ + catch_traps, raise_lib_trap, raise_user_trap, wasmtime_call_trampoline, Trap, +}; pub use crate::vmcontext::{ VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionImport, VMGlobalDefinition, VMGlobalImport, VMInvokeArgument, VMMemoryDefinition, VMMemoryImport, VMSharedSignatureIndex, diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 9349bca791fd..aa4b04f448fc 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -2,8 +2,11 @@ //! inline rather than calling them, particularly when CPUs have special //! instructions which compute them directly. +use crate::table::Table; +use crate::traphandlers::raise_lib_trap; use crate::vmcontext::VMContext; -use wasmtime_environ::wasm::{DefinedMemoryIndex, MemoryIndex}; +use wasmtime_environ::ir; +use wasmtime_environ::wasm::{DefinedMemoryIndex, DefinedTableIndex, MemoryIndex, TableIndex}; /// Implementation of f32.ceil pub extern "C" fn wasmtime_f32_ceil(x: f32) -> f32 { @@ -137,3 +140,93 @@ pub unsafe extern "C" fn wasmtime_imported_memory32_size( instance.imported_memory_size(memory_index) } + +/// Implementation of `table.copy` when both tables are locally defined. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_table_copy_defined_defined( + vmctx: *mut VMContext, + dst_table_index: u32, + src_table_index: u32, + dst: u32, + src: u32, + len: u32, + source_loc: u32, +) { + let dst_table_index = DefinedTableIndex::from_u32(dst_table_index); + let src_table_index = DefinedTableIndex::from_u32(src_table_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + let dst_table = instance.get_defined_table(dst_table_index); + let src_table = instance.get_defined_table(src_table_index); + if let Err(trap) = Table::copy(dst_table, src_table, dst, src, len, source_loc) { + raise_lib_trap(trap); + } +} + +/// Implementation of `table.copy` when the destination table is locally defined +/// and the source table is imported. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_table_copy_defined_imported( + vmctx: *mut VMContext, + dst_table_index: u32, + src_table_index: u32, + dst: u32, + src: u32, + len: u32, + source_loc: u32, +) { + let dst_table_index = DefinedTableIndex::from_u32(dst_table_index); + let src_table_index = TableIndex::from_u32(src_table_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + let dst_table = instance.get_defined_table(dst_table_index); + let src_table = instance.get_foreign_table(src_table_index); + if let Err(trap) = Table::copy(dst_table, src_table, dst, src, len, source_loc) { + raise_lib_trap(trap); + } +} + +/// Implementation of `table.copy` when the destination table is imported +/// and the source table is locally defined. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_table_copy_imported_defined( + vmctx: *mut VMContext, + dst_table_index: u32, + src_table_index: u32, + dst: u32, + src: u32, + len: u32, + source_loc: u32, +) { + let dst_table_index = TableIndex::from_u32(dst_table_index); + let src_table_index = DefinedTableIndex::from_u32(src_table_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + let dst_table = instance.get_foreign_table(dst_table_index); + let src_table = instance.get_defined_table(src_table_index); + if let Err(trap) = Table::copy(dst_table, src_table, dst, src, len, source_loc) { + raise_lib_trap(trap); + } +} + +/// Implementation of `table.copy` when both tables are imported. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_table_copy_imported_imported( + vmctx: *mut VMContext, + dst_table_index: u32, + src_table_index: u32, + dst: u32, + src: u32, + len: u32, + source_loc: u32, +) { + let dst_table_index = TableIndex::from_u32(dst_table_index); + let src_table_index = TableIndex::from_u32(src_table_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + let dst_table = instance.get_foreign_table(dst_table_index); + let src_table = instance.get_foreign_table(src_table_index); + if let Err(trap) = Table::copy(dst_table, src_table, dst, src, len, source_loc) { + raise_lib_trap(trap); + } +} diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index 843fd630e48e..45672a75fa7f 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -3,10 +3,12 @@ //! `Table` is to WebAssembly tables what `LinearMemory` is to WebAssembly linear memories. use crate::vmcontext::{VMCallerCheckedAnyfunc, VMTableDefinition}; +use crate::{Trap, TrapDescription}; +use backtrace::Backtrace; use std::cell::RefCell; use std::convert::{TryFrom, TryInto}; use wasmtime_environ::wasm::TableElementType; -use wasmtime_environ::{TablePlan, TableStyle}; +use wasmtime_environ::{ir, TablePlan, TableStyle}; /// A table instance. #[derive(Debug)] @@ -87,6 +89,56 @@ impl Table { } } + /// Copy `len` elements from `src_table[src_index..]` into `dst_table[dst_index..]`. + /// + /// # Errors + /// + /// Returns an error if the range is out of bounds of either the source or + /// destination tables. + pub fn copy( + dst_table: &Self, + src_table: &Self, + dst_index: u32, + src_index: u32, + len: u32, + source_loc: ir::SourceLoc, + ) -> Result<(), Trap> { + // https://webassembly.github.io/bulk-memory-operations/core/exec/instructions.html#exec-table-copy + + if src_index + .checked_add(len) + .map_or(true, |n| n > src_table.size()) + || dst_index + .checked_add(len) + .map_or(true, |m| m > dst_table.size()) + { + return Err(Trap::Wasm { + desc: TrapDescription { + source_loc, + trap_code: ir::TrapCode::TableOutOfBounds, + }, + backtrace: Backtrace::new(), + }); + } + + let srcs = src_index..src_index + len; + let dsts = dst_index..dst_index + len; + + // Note on the unwraps: the bounds check above means that these will + // never panic. + if dst_index <= src_index { + for (s, d) in (srcs).zip(dsts) { + dst_table.set(d, src_table.get(s).unwrap()).unwrap(); + } + } else { + for (s, d) in srcs.rev().zip(dsts.rev()) { + dst_table.set(d, src_table.get(s).unwrap()).unwrap(); + } + } + + Ok(()) + } + /// Return a `VMTableDefinition` for exposing the table to compiled wasm code. pub fn vmtable(&self) -> VMTableDefinition { let mut vec = self.vec.borrow_mut(); diff --git a/crates/runtime/src/trap_registry.rs b/crates/runtime/src/trap_registry.rs index 317fa920aa4b..f8f4c8b56dc3 100644 --- a/crates/runtime/src/trap_registry.rs +++ b/crates/runtime/src/trap_registry.rs @@ -84,7 +84,7 @@ fn trap_code_to_expected_string(trap_code: ir::TrapCode) -> String { match trap_code { StackOverflow => "call stack exhausted".to_string(), HeapOutOfBounds => "out of bounds memory access".to_string(), - TableOutOfBounds => "undefined element".to_string(), + TableOutOfBounds => "undefined element: out of bounds".to_string(), OutOfBounds => "out of bounds".to_string(), // Note: not covered by the test suite IndirectCallToNull => "uninitialized element".to_string(), BadSignature => "indirect call type mismatch".to_string(), diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs index ba05cb37eb89..1978c5cc8801 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -73,6 +73,20 @@ pub unsafe fn raise_user_trap(data: Box) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::UserTrap(data))) } +/// Raises a trap from inside library code immediately. +/// +/// This function performs as-if a wasm trap was just executed. This trap +/// payload is then returned from `wasmtime_call` and `wasmtime_call_trampoline` +/// below. +/// +/// # Safety +/// +/// Only safe to call when wasm code is on the stack, aka `wasmtime_call` or +/// `wasmtime_call_trampoline` must have been previously called. +pub unsafe fn raise_lib_trap(trap: Trap) -> ! { + tls::with(|info| info.unwrap().unwind_with(UnwindReason::LibTrap(trap))) +} + /// Carries a Rust panic across wasm code and resumes the panic on the other /// side. /// @@ -192,6 +206,7 @@ enum UnwindReason { None, Panic(Box), UserTrap(Box), + LibTrap(Trap), Trap { backtrace: Backtrace, pc: usize }, } @@ -219,6 +234,7 @@ impl CallThreadState { debug_assert_eq!(ret, 0); Err(Trap::User(data)) } + UnwindReason::LibTrap(trap) => Err(trap), UnwindReason::Trap { backtrace, pc } => { debug_assert_eq!(ret, 0); let instance = unsafe { InstanceHandle::from_vmctx(self.vmctx) }; diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index a11ddef09479..d656e68602dc 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -537,6 +537,7 @@ impl VMBuiltinFunctionsArray { pub fn initialized() -> Self { use crate::libcalls::*; let mut ptrs = [0; Self::len()]; + ptrs[BuiltinFunctionIndex::get_memory32_grow_index().index() as usize] = wasmtime_memory32_grow as usize; ptrs[BuiltinFunctionIndex::get_imported_memory32_grow_index().index() as usize] = @@ -545,6 +546,21 @@ impl VMBuiltinFunctionsArray { wasmtime_memory32_size as usize; ptrs[BuiltinFunctionIndex::get_imported_memory32_size_index().index() as usize] = wasmtime_imported_memory32_size as usize; + + ptrs[BuiltinFunctionIndex::get_table_copy_defined_defined_index().index() as usize] = + wasmtime_table_copy_defined_defined as usize; + + ptrs[BuiltinFunctionIndex::get_table_copy_defined_imported_index().index() as usize] = + wasmtime_table_copy_defined_imported as usize; + + ptrs[BuiltinFunctionIndex::get_table_copy_imported_defined_index().index() as usize] = + wasmtime_table_copy_imported_defined as usize; + + ptrs[BuiltinFunctionIndex::get_table_copy_imported_imported_index().index() as usize] = + wasmtime_table_copy_imported_imported as usize; + + debug_assert!(ptrs.iter().cloned().all(|p| p != 0)); + Self { ptrs } } } diff --git a/tests/misc_testsuite/table_copy.wast b/tests/misc_testsuite/table_copy.wast new file mode 100644 index 000000000000..f06258ea249d --- /dev/null +++ b/tests/misc_testsuite/table_copy.wast @@ -0,0 +1,63 @@ +(module + (func $f (param i32 i32 i32) (result i32) (local.get 0)) + (func $g (param i32 i32 i32) (result i32) (local.get 1)) + (func $h (param i32 i32 i32) (result i32) (local.get 2)) + + ;; Indices: 0 1 2 3 4 5 6 7 8 + (table funcref (elem $f $g $h $f $g $h $f $g $h)) + ;; After table.copy: $g $h $f + + (func (export "copy") (param i32 i32 i32) + local.get 0 + local.get 1 + local.get 2 + table.copy) + + (func (export "call") (param i32 i32 i32 i32) (result i32) + local.get 0 + local.get 1 + local.get 2 + local.get 3 + call_indirect (param i32 i32 i32) (result i32)) +) + +;; Call $f at 0 +(assert_return + (invoke "call" (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 0)) + (i32.const 1)) + +;; Call $g at 1 +(assert_return + (invoke "call" (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 1)) + (i32.const 1)) + +;; Call $h at 2 +(assert_return + (invoke "call" (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 2)) + (i32.const 1)) + +;; Do a `table.copy` to rearrange the elements. Copy from 4..7 to 0..3. +(invoke "copy" (i32.const 0) (i32.const 4) (i32.const 3)) + +;; Call $g at 0 +(assert_return + (invoke "call" (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0)) + (i32.const 1)) + +;; Call $h at 1 +(assert_return + (invoke "call" (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 1)) + (i32.const 1)) + +;; Call $f at 2 +(assert_return + (invoke "call" (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 2)) + (i32.const 1)) + +;; Copying up to the end does not trap. +(invoke "copy" (i32.const 7) (i32.const 0) (i32.const 2)) + +;; Copying past the end traps. +(assert_trap + (invoke "copy" (i32.const 7) (i32.const 0) (i32.const 3)) + "undefined element") diff --git a/tests/misc_testsuite/table_copy_on_imported_tables.wast b/tests/misc_testsuite/table_copy_on_imported_tables.wast new file mode 100644 index 000000000000..dae846c2d9e5 --- /dev/null +++ b/tests/misc_testsuite/table_copy_on_imported_tables.wast @@ -0,0 +1,165 @@ +(module $m + (func $f (param i32 i32 i32 i32 i32 i32) (result i32) (local.get 0)) + (func $g (param i32 i32 i32 i32 i32 i32) (result i32) (local.get 1)) + (func $h (param i32 i32 i32 i32 i32 i32) (result i32) (local.get 2)) + + (table $t (export "t") funcref (elem $f $g $h $f $g $h))) + +(register "m" $m) + +(module $n + (table $t (import "m" "t") 6 funcref) + + (func $i (param i32 i32 i32 i32 i32 i32) (result i32) (local.get 3)) + (func $j (param i32 i32 i32 i32 i32 i32) (result i32) (local.get 4)) + (func $k (param i32 i32 i32 i32 i32 i32) (result i32) (local.get 5)) + + (table $u (export "u") funcref (elem $i $j $k $i $j $k)) + + (func (export "copy_into_t_from_u") (param i32 i32 i32) + local.get 0 + local.get 1 + local.get 2 + table.copy $t $u) + + (func (export "copy_into_u_from_t") (param i32 i32 i32) + local.get 0 + local.get 1 + local.get 2 + table.copy $u $t) + + (func (export "call_t") (param i32 i32 i32 i32 i32 i32 i32) (result i32) + local.get 0 + local.get 1 + local.get 2 + local.get 3 + local.get 4 + local.get 5 + local.get 6 + call_indirect $t (param i32 i32 i32 i32 i32 i32) (result i32)) + + (func (export "call_u") (param i32 i32 i32 i32 i32 i32 i32) (result i32) + local.get 0 + local.get 1 + local.get 2 + local.get 3 + local.get 4 + local.get 5 + local.get 6 + call_indirect $u (param i32 i32 i32 i32 i32 i32) (result i32))) + +;; Everything has what we initially expect. +(assert_return + (invoke "call_t" (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) + (i32.const 0)) + (i32.const 1)) +(assert_return + (invoke "call_t" (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) + (i32.const 1)) + (i32.const 1)) +(assert_return + (invoke "call_t" (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 0) + (i32.const 2)) + (i32.const 1)) +(assert_return + (invoke "call_u" (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0) + (i32.const 0)) + (i32.const 1)) +(assert_return + (invoke "call_u" (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 0) + (i32.const 1)) + (i32.const 1)) +(assert_return + (invoke "call_u" (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 1) + (i32.const 2)) + (i32.const 1)) + +;; Now test copying between a local and an imported table. + +;; Copy $i $j $k into $t at 3..6 from $u at 0..3. +(invoke "copy_into_t_from_u" (i32.const 3) (i32.const 0) (i32.const 3)) + +(assert_return + (invoke "call_t" (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0) + (i32.const 3)) + (i32.const 1)) +(assert_return + (invoke "call_t" (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 0) + (i32.const 4)) + (i32.const 1)) +(assert_return + (invoke "call_t" (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 1) + (i32.const 5)) + (i32.const 1)) + +;; Copy $f $g $h into $u at 0..3 from $t at 0..3. +(invoke "copy_into_u_from_t" (i32.const 0) (i32.const 0) (i32.const 3)) + +(assert_return + (invoke "call_u" (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) + (i32.const 0)) + (i32.const 1)) +(assert_return + (invoke "call_u" (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) + (i32.const 1)) + (i32.const 1)) +(assert_return + (invoke "call_u" (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 0) + (i32.const 2)) + (i32.const 1)) + +(register "n" $n) + +(module $o + (table $t (import "m" "t") 6 funcref) + (table $u (import "n" "u") 6 funcref) + + (func (export "copy_into_t_from_u_2") (param i32 i32 i32) + local.get 0 + local.get 1 + local.get 2 + table.copy $t $u) + + (func (export "copy_into_u_from_t_2") (param i32 i32 i32) + local.get 0 + local.get 1 + local.get 2 + table.copy $u $t) + + (func (export "call_t_2") (param i32 i32 i32 i32 i32 i32 i32) (result i32) + local.get 0 + local.get 1 + local.get 2 + local.get 3 + local.get 4 + local.get 5 + local.get 6 + call_indirect $t (param i32 i32 i32 i32 i32 i32) (result i32)) + + (func (export "call_u_2") (param i32 i32 i32 i32 i32 i32 i32) (result i32) + local.get 0 + local.get 1 + local.get 2 + local.get 3 + local.get 4 + local.get 5 + local.get 6 + call_indirect $u (param i32 i32 i32 i32 i32 i32) (result i32))) + +;; Now test copying between two imported tables. + +;; Copy $i into $t at 0 from $u at 3. +(invoke "copy_into_t_from_u_2" (i32.const 0) (i32.const 3) (i32.const 1)) + +(assert_return + (invoke "call_t_2" (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0) + (i32.const 0)) + (i32.const 1)) + +;; Copy $g into $u at 4 from $t at 1. +(invoke "copy_into_u_from_t_2" (i32.const 4) (i32.const 1) (i32.const 1)) + +(assert_return + (invoke "call_u_2" (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) + (i32.const 4)) + (i32.const 1)) diff --git a/tests/wast_testsuites.rs b/tests/wast_testsuites.rs index a4a1d0e64ffb..a1ce8d845486 100644 --- a/tests/wast_testsuites.rs +++ b/tests/wast_testsuites.rs @@ -16,12 +16,25 @@ fn run_wast(wast: &str, strategy: Strategy) -> anyhow::Result<()> { // by reference types. let reftypes = simd || wast.iter().any(|s| s == "reference-types"); + // Reference types assumes support for bulk memory. + let bulk_mem = reftypes + || wast.iter().any(|s| s == "bulk-memory-operations") + || wast.iter().any(|s| s == "table_copy.wast") + || wast + .iter() + .any(|s| s == "table_copy_on_imported_tables.wast"); + + // And bulk memory also assumes support for reference types (e.g. multiple + // tables). + let reftypes = reftypes || bulk_mem; + let multi_val = wast.iter().any(|s| s == "multi-value"); let mut cfg = Config::new(); cfg.wasm_simd(simd) .wasm_reference_types(reftypes) .wasm_multi_value(multi_val) + .wasm_bulk_memory(bulk_mem) .strategy(strategy)? .cranelift_debug_verifier(true); From cb97e4ec8eb4e2981747fe612bb1d21d64766b32 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 18 Feb 2020 16:41:43 -0800 Subject: [PATCH 03/11] Implement `table.init` and `elem.drop` from the bulk memory proposal --- build.rs | 4 +- crates/api/src/instance.rs | 16 ++- crates/environ/src/data_structures.rs | 4 +- crates/environ/src/func_environ.rs | 139 ++++++++++++++++--- crates/environ/src/module.rs | 5 + crates/runtime/src/instance.rs | 172 ++++++++++++++++++++---- crates/runtime/src/libcalls.rs | 34 ++++- crates/runtime/src/vmcontext.rs | 10 +- crates/wast/src/wast.rs | 12 +- tests/misc_testsuite/elem-ref-null.wast | 2 + tests/misc_testsuite/elem_drop.wast | 7 + tests/wast_testsuites.rs | 2 + 12 files changed, 352 insertions(+), 55 deletions(-) mode change 100644 => 100755 crates/environ/src/data_structures.rs create mode 100644 tests/misc_testsuite/elem-ref-null.wast create mode 100644 tests/misc_testsuite/elem_drop.wast diff --git a/build.rs b/build.rs index 718ee96cc55a..1d71b908fe7c 100644 --- a/build.rs +++ b/build.rs @@ -182,7 +182,9 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { ("reference_types", _) => return true, - ("bulk_memory_operations", "table_copy") => return false, + ("bulk_memory_operations", "table_copy") + | ("bulk_memory_operations", "table_init") + | ("bulk_memory_operations", "elem") => return false, ("bulk_memory_operations", _) => return true, _ => {} diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index 1a44b656b27b..065627dc515e 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -1,10 +1,10 @@ use crate::externals::Extern; use crate::module::Module; -use crate::runtime::Store; +use crate::runtime::{Config, Store}; use crate::trap::Trap; use anyhow::{Error, Result}; use wasmtime_jit::{CompiledModule, Resolver}; -use wasmtime_runtime::{Export, InstanceHandle, InstantiationError}; +use wasmtime_runtime::{Export, InstanceHandle, InstantiationError, LinkError}; struct SimpleResolver<'a> { imports: &'a [Extern], @@ -19,6 +19,7 @@ impl Resolver for SimpleResolver<'_> { } fn instantiate( + config: &Config, compiled_module: &CompiledModule, imports: &[Extern], ) -> Result { @@ -29,6 +30,14 @@ fn instantiate( .map_err(|e| -> Error { match e { InstantiationError::StartTrap(trap) => Trap::from_jit(trap).into(), + e @ InstantiationError::TableOutOfBounds(_) => { + let msg = e.to_string(); + if config.validating_config.operator_config.enable_bulk_memory { + Trap::new(msg).into() + } else { + InstantiationError::Link(LinkError(msg)).into() + } + } other => other.into(), } })?; @@ -105,7 +114,8 @@ impl Instance { /// [issue]: https://github.com/bytecodealliance/wasmtime/issues/727 pub fn new(module: &Module, imports: &[Extern]) -> Result { let store = module.store(); - let instance_handle = instantiate(module.compiled_module(), imports)?; + let config = store.engine().config(); + let instance_handle = instantiate(config, module.compiled_module(), imports)?; let exports = { let mut exports = Vec::with_capacity(module.exports().len()); diff --git a/crates/environ/src/data_structures.rs b/crates/environ/src/data_structures.rs old mode 100644 new mode 100755 index 6be6c33dab47..e12dee3ca7b7 --- a/crates/environ/src/data_structures.rs +++ b/crates/environ/src/data_structures.rs @@ -17,13 +17,13 @@ pub mod isa { } pub mod entity { - pub use cranelift_entity::{BoxedSlice, EntityRef, PrimaryMap}; + pub use cranelift_entity::{packed_option, BoxedSlice, EntityRef, PrimaryMap}; } pub mod wasm { pub use cranelift_wasm::{ get_vmctx_value_label, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, FuncIndex, Global, GlobalIndex, GlobalInit, Memory, MemoryIndex, - SignatureIndex, Table, TableElementType, TableIndex, + PassiveElemIndex, SignatureIndex, Table, TableElementType, TableIndex, }; } diff --git a/crates/environ/src/func_environ.rs b/crates/environ/src/func_environ.rs index 737e762a024e..f3a11ff735e4 100644 --- a/crates/environ/src/func_environ.rs +++ b/crates/environ/src/func_environ.rs @@ -1,7 +1,7 @@ use crate::module::{MemoryPlan, MemoryStyle, ModuleLocal, TableStyle}; use crate::vmoffsets::VMOffsets; use crate::WASM_PAGE_SIZE; -use cranelift_codegen::cursor::FuncCursor; +use cranelift_codegen::cursor::{Cursor, FuncCursor}; use cranelift_codegen::ir; use cranelift_codegen::ir::condcodes::*; use cranelift_codegen::ir::immediates::{Offset32, Uimm64}; @@ -62,9 +62,17 @@ impl BuiltinFunctionIndex { pub const fn get_table_copy_imported_imported_index() -> Self { Self(7) } + /// Returns an index for wasm's `table.init`. + pub const fn get_table_init_index() -> Self { + Self(8) + } + /// Returns an index for wasm's `elem.drop`. + pub const fn get_elem_drop_index() -> Self { + Self(9) + } /// Returns the total number of builtin functions. pub const fn builtin_functions_total_number() -> u32 { - 8 + 10 } /// Return the index as an u32 number. @@ -96,6 +104,12 @@ pub struct FuncEnvironment<'module_environment> { /// (it's the same for both local and imported tables). table_copy_sig: Option, + /// The external function signature for implementing wasm's `table.init`. + table_init_sig: Option, + + /// The external function signature for implementing wasm's `elem.drop`. + elem_drop_sig: Option, + /// Offsets to struct fields accessed by JIT code. offsets: VMOffsets, } @@ -112,6 +126,8 @@ impl<'module_environment> FuncEnvironment<'module_environment> { memory32_size_sig: None, memory_grow_sig: None, table_copy_sig: None, + table_init_sig: None, + elem_drop_sig: None, offsets: VMOffsets::new(target_config.pointer_bytes(), module), } } @@ -294,6 +310,67 @@ impl<'module_environment> FuncEnvironment<'module_environment> { } } + fn get_table_init_sig(&mut self, func: &mut Function) -> ir::SigRef { + let sig = self.table_init_sig.unwrap_or_else(|| { + func.import_signature(Signature { + params: vec![ + AbiParam::special(self.pointer_type(), ArgumentPurpose::VMContext), + // Table index. + AbiParam::new(I32), + // Segment index. + AbiParam::new(I32), + // Destination index within table. + AbiParam::new(I32), + // Source index within segment. + AbiParam::new(I32), + // Number of elements to initialize. + AbiParam::new(I32), + // Source location. + AbiParam::new(I32), + ], + returns: vec![], + call_conv: self.target_config.default_call_conv, + }) + }); + self.table_init_sig = Some(sig); + sig + } + + fn get_table_init_func( + &mut self, + func: &mut Function, + table_index: TableIndex, + ) -> (ir::SigRef, usize, BuiltinFunctionIndex) { + let sig = self.get_table_init_sig(func); + let table_index = table_index.as_u32() as usize; + ( + sig, + table_index, + BuiltinFunctionIndex::get_table_init_index(), + ) + } + + fn get_elem_drop_sig(&mut self, func: &mut Function) -> ir::SigRef { + let sig = self.elem_drop_sig.unwrap_or_else(|| { + func.import_signature(Signature { + params: vec![ + AbiParam::special(self.pointer_type(), ArgumentPurpose::VMContext), + // Element index. + AbiParam::new(I32), + ], + returns: vec![], + call_conv: self.target_config.default_call_conv, + }) + }); + self.elem_drop_sig = Some(sig); + sig + } + + fn get_elem_drop_func(&mut self, func: &mut Function) -> (ir::SigRef, BuiltinFunctionIndex) { + let sig = self.get_elem_drop_sig(func); + (sig, BuiltinFunctionIndex::get_elem_drop_index()) + } + /// Translates load of builtin function and returns a pair of values `vmctx` /// and address of the loaded function. fn translate_load_builtin_function_address( @@ -960,8 +1037,6 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m src: ir::Value, len: ir::Value, ) -> WasmResult<()> { - use cranelift_codegen::cursor::Cursor; - let (func_sig, dst_table_index_arg, src_table_index_arg, func_idx) = self.get_table_copy_func(&mut pos.func, dst_table_index, src_table_index); @@ -992,22 +1067,52 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m fn translate_table_init( &mut self, - _pos: FuncCursor, - _seg_index: u32, - _table_index: TableIndex, + mut pos: FuncCursor, + seg_index: u32, + table_index: TableIndex, _table: ir::Table, - _dst: ir::Value, - _src: ir::Value, - _len: ir::Value, + dst: ir::Value, + src: ir::Value, + len: ir::Value, ) -> WasmResult<()> { - Err(WasmError::Unsupported( - "bulk memory: `table.init`".to_string(), - )) + let (func_sig, table_index_arg, func_idx) = + self.get_table_init_func(&mut pos.func, table_index); + + let table_index_arg = pos.ins().iconst(I32, table_index_arg as i64); + let seg_index_arg = pos.ins().iconst(I32, seg_index as i64); + + let src_loc = pos.srcloc(); + let src_loc_arg = pos.ins().iconst(I32, src_loc.bits() as i64); + + let (vmctx, func_addr) = self.translate_load_builtin_function_address(&mut pos, func_idx); + + pos.ins().call_indirect( + func_sig, + func_addr, + &[ + vmctx, + table_index_arg, + seg_index_arg, + dst, + src, + len, + src_loc_arg, + ], + ); + + Ok(()) } - fn translate_elem_drop(&mut self, _pos: FuncCursor, _seg_index: u32) -> WasmResult<()> { - Err(WasmError::Unsupported( - "bulk memory: `elem.drop`".to_string(), - )) + fn translate_elem_drop(&mut self, mut pos: FuncCursor, elem_index: u32) -> WasmResult<()> { + let (func_sig, func_idx) = self.get_elem_drop_func(&mut pos.func); + + let elem_index_arg = pos.ins().iconst(I32, elem_index as i64); + + let (vmctx, func_addr) = self.translate_load_builtin_function_address(&mut pos, func_idx); + + pos.ins() + .call_indirect(func_sig, func_addr, &[vmctx, elem_index_arg]); + + Ok(()) } } diff --git a/crates/environ/src/module.rs b/crates/environ/src/module.rs index 499f470a3069..aacc6fe747f7 100644 --- a/crates/environ/src/module.rs +++ b/crates/environ/src/module.rs @@ -237,6 +237,11 @@ impl Module { }, } } + + /// Get the given passive element, if it exists. + pub fn get_passive_element(&self, index: PassiveElemIndex) -> Option<&[FuncIndex]> { + self.passive_elements.get(&index).map(|es| &**es) + } } impl ModuleLocal { diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index c8e4dac5b7f0..4ae120d8f1b8 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -14,24 +14,25 @@ use crate::vmcontext::{ VMGlobalDefinition, VMGlobalImport, VMMemoryDefinition, VMMemoryImport, VMSharedSignatureIndex, VMTableDefinition, VMTableImport, }; -use crate::TrapRegistration; +use crate::{TrapDescription, TrapRegistration}; use memoffset::offset_of; use more_asserts::assert_lt; use std::alloc::{self, Layout}; use std::any::Any; -use std::cell::Cell; +use std::cell::{Cell, RefCell}; +use std::collections::HashMap; use std::collections::HashSet; use std::convert::TryFrom; use std::rc::Rc; use std::sync::Arc; use std::{mem, ptr, slice}; use thiserror::Error; -use wasmtime_environ::entity::{BoxedSlice, EntityRef, PrimaryMap}; +use wasmtime_environ::entity::{packed_option::ReservedValue, BoxedSlice, EntityRef, PrimaryMap}; use wasmtime_environ::wasm::{ DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, FuncIndex, - GlobalIndex, GlobalInit, MemoryIndex, SignatureIndex, TableIndex, + GlobalIndex, GlobalInit, MemoryIndex, PassiveElemIndex, SignatureIndex, TableIndex, }; -use wasmtime_environ::{DataInitializer, Module, TableElements, VMOffsets}; +use wasmtime_environ::{ir, DataInitializer, Module, TableElements, VMOffsets}; cfg_if::cfg_if! { if #[cfg(unix)] { @@ -86,6 +87,10 @@ pub(crate) struct Instance { /// WebAssembly table data. tables: BoxedSlice, + /// Passive elements in this instantiation. As `elem.drop`s happen, these + /// entries get replaced into empty slices. + passive_elements: RefCell>>, + /// Pointers to functions in executable memory. finished_functions: BoxedSlice, @@ -124,6 +129,14 @@ impl Instance { unsafe { *self.signature_ids_ptr().add(index) } } + pub(crate) fn module(&self) -> &Arc { + &self.module + } + + pub(crate) fn module_ref(&self) -> &Module { + &*self.module + } + /// Return a pointer to the `VMSharedSignatureIndex`s. fn signature_ids_ptr(&self) -> *mut VMSharedSignatureIndex { unsafe { self.vmctx_plus_offset(self.offsets.vmctx_signature_ids_begin()) } @@ -527,6 +540,91 @@ impl Instance { Layout::from_size_align(size, align).unwrap() } + /// Get a `VMCallerCheckedAnyfunc` for the given `FuncIndex`. + fn get_caller_checked_anyfunc(&self, index: FuncIndex) -> VMCallerCheckedAnyfunc { + if index == FuncIndex::reserved_value() { + return VMCallerCheckedAnyfunc::default(); + } + + let sig = self.module.local.functions[index]; + let type_index = self.signature_id(sig); + + let (func_ptr, vmctx) = if let Some(def_index) = self.module.local.defined_func_index(index) + { + ( + self.finished_functions[def_index] as *const _, + self.vmctx_ptr(), + ) + } else { + let import = self.imported_function(index); + (import.body, import.vmctx) + }; + VMCallerCheckedAnyfunc { + func_ptr, + type_index, + vmctx, + } + } + + /// The `table.init` operation: initializes a portion of a table with a + /// passive element. + /// + /// # Errors + /// + /// Returns a `Trap` error when the range within the table is out of bounds + /// or the range within the passive element is out of bounds. + pub(crate) fn table_init( + &self, + table_index: TableIndex, + elem_index: PassiveElemIndex, + dst: u32, + src: u32, + len: u32, + source_loc: ir::SourceLoc, + ) -> Result<(), Trap> { + // https://webassembly.github.io/bulk-memory-operations/core/exec/instructions.html#exec-table-init + + let table = self.get_table(table_index); + let passive_elements = self.passive_elements.borrow(); + let elem = passive_elements + .get(&elem_index) + .map(|e| &**e) + .unwrap_or_else(|| &[]); + + if src + .checked_add(len) + .map_or(true, |n| n as usize > elem.len()) + || dst.checked_add(len).map_or(true, |m| m > table.size()) + { + return Err(Trap::Wasm { + desc: TrapDescription { + source_loc, + trap_code: ir::TrapCode::TableOutOfBounds, + }, + backtrace: backtrace::Backtrace::new(), + }); + } + + for (dst, src) in (dst..dst + len).zip(src..src + len) { + table + .set(dst, elem[src as usize].clone()) + .expect("should never panic because we already did the bounds check above"); + } + + Ok(()) + } + + /// Drop an element. + pub(crate) fn elem_drop(&self, elem_index: PassiveElemIndex) { + // https://webassembly.github.io/reference-types/core/exec/instructions.html#exec-elem-drop + + let mut passive_elements = self.passive_elements.borrow_mut(); + // Note that dropping a non-passive element is a no-op (not a trap). + if let Some(elem) = passive_elements.get_mut(&elem_index) { + mem::replace(elem, vec![].into_boxed_slice()); + } + } + /// Get a table by index regardless of whether it is locally-defined or an /// imported, foreign table. pub(crate) fn get_table(&self, table_index: TableIndex) -> &Table { @@ -611,6 +709,7 @@ impl InstanceHandle { offsets, memories, tables, + passive_elements: Default::default(), finished_functions, dbg_jit_registration, host_state, @@ -681,6 +780,7 @@ impl InstanceHandle { // Apply the initializers. initialize_tables(instance)?; + initialize_passive_elements(instance); initialize_memories(instance, data_initializers)?; initialize_globals(instance); @@ -721,12 +821,12 @@ impl InstanceHandle { /// Return a reference-counting pointer to a module. pub fn module(&self) -> &Arc { - &self.instance().module + self.instance().module() } /// Return a reference to a module. pub fn module_ref(&self) -> &Module { - &self.instance().module + self.instance().module_ref() } /// Lookup an export with the given name. @@ -846,9 +946,9 @@ fn check_table_init_bounds(instance: &Instance) -> Result<(), InstantiationError let size = usize::try_from(table.size()).unwrap(); if size < start + init.elements.len() { - return Err(InstantiationError::Link(LinkError( + return Err(InstantiationError::TableOutOfBounds( "elements segment does not fit".to_owned(), - ))); + )); } } @@ -944,31 +1044,15 @@ fn get_table_init_start(init: &TableElements, instance: &Instance) -> usize { /// Initialize the table memory from the provided initializers. fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { - let vmctx = instance.vmctx_ptr(); let module = Arc::clone(&instance.module); for init in &module.table_elements { let start = get_table_init_start(init, instance); let table = instance.get_table(init.table_index); for (i, func_idx) in init.elements.iter().enumerate() { - let callee_sig = instance.module.local.functions[*func_idx]; - let (callee_ptr, callee_vmctx) = - if let Some(index) = instance.module.local.defined_func_index(*func_idx) { - (instance.finished_functions[index] as *const _, vmctx) - } else { - let imported_func = instance.imported_function(*func_idx); - (imported_func.body, imported_func.vmctx) - }; - let type_index = instance.signature_id(callee_sig); + let anyfunc = instance.get_caller_checked_anyfunc(*func_idx); table - .set( - u32::try_from(start + i).unwrap(), - VMCallerCheckedAnyfunc { - func_ptr: callee_ptr, - type_index, - vmctx: callee_vmctx, - }, - ) + .set(u32::try_from(start + i).unwrap(), anyfunc) .unwrap(); } } @@ -976,6 +1060,33 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { Ok(()) } +/// Initialize the `Instance::passive_elements` map by resolving the +/// `Module::passive_elements`'s `FuncIndex`s into `VMCallerCheckedAnyfunc`s for +/// this instance. +fn initialize_passive_elements(instance: &Instance) { + let mut passive_elements = instance.passive_elements.borrow_mut(); + debug_assert!( + passive_elements.is_empty(), + "should only be called once, at initialization time" + ); + + passive_elements.extend( + instance + .module + .passive_elements + .iter() + .map(|(idx, segments)| { + ( + *idx, + segments + .iter() + .map(|s| instance.get_caller_checked_anyfunc(*s)) + .collect(), + ) + }), + ); +} + /// Allocate memory for just the memories of the current module. fn create_memories( module: &Module, @@ -1059,6 +1170,13 @@ pub enum InstantiationError { #[error("Insufficient resources: {0}")] Resource(String), + /// A table out of bounds error. + /// + /// Depending on whether bulk memory is enabled or not, this is either a + /// link error or a trap raised during instantiation. + #[error("Table out of bounds error: {0}")] + TableOutOfBounds(String), + /// A wasm link error occured. #[error("Failed to link module")] Link(#[from] LinkError), diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index aa4b04f448fc..3e4c0f93e9ca 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -6,7 +6,9 @@ use crate::table::Table; use crate::traphandlers::raise_lib_trap; use crate::vmcontext::VMContext; use wasmtime_environ::ir; -use wasmtime_environ::wasm::{DefinedMemoryIndex, DefinedTableIndex, MemoryIndex, TableIndex}; +use wasmtime_environ::wasm::{ + DefinedMemoryIndex, DefinedTableIndex, MemoryIndex, PassiveElemIndex, TableIndex, +}; /// Implementation of f32.ceil pub extern "C" fn wasmtime_f32_ceil(x: f32) -> f32 { @@ -230,3 +232,33 @@ pub unsafe extern "C" fn wasmtime_table_copy_imported_imported( raise_lib_trap(trap); } } + +/// Implementation of `table.init`. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_table_init( + vmctx: *mut VMContext, + table_index: u32, + elem_index: u32, + dst: u32, + src: u32, + len: u32, + source_loc: u32, +) { + let table_index = TableIndex::from_u32(table_index); + let source_loc = ir::SourceLoc::new(source_loc); + let elem_index = PassiveElemIndex::from_u32(elem_index); + + let instance = (&mut *vmctx).instance(); + + if let Err(trap) = instance.table_init(table_index, elem_index, dst, src, len, source_loc) { + raise_lib_trap(trap); + } +} + +/// Implementation of `elem.drop`. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_elem_drop(vmctx: *mut VMContext, elem_index: u32) { + let elem_index = PassiveElemIndex::from_u32(elem_index); + let instance = (&mut *vmctx).instance(); + instance.elem_drop(elem_index); +} diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index d656e68602dc..613cf9af9952 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -536,12 +536,14 @@ impl VMBuiltinFunctionsArray { pub fn initialized() -> Self { use crate::libcalls::*; + let mut ptrs = [0; Self::len()]; ptrs[BuiltinFunctionIndex::get_memory32_grow_index().index() as usize] = wasmtime_memory32_grow as usize; ptrs[BuiltinFunctionIndex::get_imported_memory32_grow_index().index() as usize] = wasmtime_imported_memory32_grow as usize; + ptrs[BuiltinFunctionIndex::get_memory32_size_index().index() as usize] = wasmtime_memory32_size as usize; ptrs[BuiltinFunctionIndex::get_imported_memory32_size_index().index() as usize] = @@ -549,16 +551,18 @@ impl VMBuiltinFunctionsArray { ptrs[BuiltinFunctionIndex::get_table_copy_defined_defined_index().index() as usize] = wasmtime_table_copy_defined_defined as usize; - ptrs[BuiltinFunctionIndex::get_table_copy_defined_imported_index().index() as usize] = wasmtime_table_copy_defined_imported as usize; - ptrs[BuiltinFunctionIndex::get_table_copy_imported_defined_index().index() as usize] = wasmtime_table_copy_imported_defined as usize; - ptrs[BuiltinFunctionIndex::get_table_copy_imported_imported_index().index() as usize] = wasmtime_table_copy_imported_imported as usize; + ptrs[BuiltinFunctionIndex::get_table_init_index().index() as usize] = + wasmtime_table_init as usize; + ptrs[BuiltinFunctionIndex::get_elem_drop_index().index() as usize] = + wasmtime_elem_drop as usize; + debug_assert!(ptrs.iter().cloned().all(|p| p != 0)); Self { ptrs } diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 621177b0daae..4d6ac4f4e51a 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -287,7 +287,7 @@ impl WastContext { Err(e) => e, }; let error_message = format!("{:?}", err); - if !error_message.contains(&message) { + if !is_matching_assert_invalid_error_message(&message, &error_message) { bail!( "assert_invalid: expected \"{}\", got \"{}\"", message, @@ -343,6 +343,16 @@ impl WastContext { } } +fn is_matching_assert_invalid_error_message(expected: &str, actual: &str) -> bool { + actual.contains(expected) + // Waiting on https://github.com/WebAssembly/bulk-memory-operations/pull/137 + // to propagate to WebAssembly/testsuite. + || (expected.contains("unknown table") && actual.contains("unknown elem")) + // `elem.wast` and `proposals/bulk-memory-operations/elem.wast` disagree + // on the expected error message for the same error. + || (expected.contains("out of bounds") && actual.contains("does not fit")) +} + fn extract_lane_as_i8(bytes: u128, lane: usize) -> i8 { (bytes >> (lane * 8)) as i8 } diff --git a/tests/misc_testsuite/elem-ref-null.wast b/tests/misc_testsuite/elem-ref-null.wast new file mode 100644 index 000000000000..c904c332ddd3 --- /dev/null +++ b/tests/misc_testsuite/elem-ref-null.wast @@ -0,0 +1,2 @@ +(module + (elem funcref (ref.null))) diff --git a/tests/misc_testsuite/elem_drop.wast b/tests/misc_testsuite/elem_drop.wast new file mode 100644 index 000000000000..a95804b5943b --- /dev/null +++ b/tests/misc_testsuite/elem_drop.wast @@ -0,0 +1,7 @@ +(module + (table 1 1 funcref) + (elem (i32.const 0) funcref (ref.func 0)) + (func (export "elem.drop non-passive element") + (elem.drop 0))) + +(invoke "elem.drop non-passive element") diff --git a/tests/wast_testsuites.rs b/tests/wast_testsuites.rs index a1ce8d845486..cb7599bc94fb 100644 --- a/tests/wast_testsuites.rs +++ b/tests/wast_testsuites.rs @@ -20,6 +20,8 @@ fn run_wast(wast: &str, strategy: Strategy) -> anyhow::Result<()> { let bulk_mem = reftypes || wast.iter().any(|s| s == "bulk-memory-operations") || wast.iter().any(|s| s == "table_copy.wast") + || wast.iter().any(|s| s == "elem_drop.wast") + || wast.iter().any(|s| s == "elem-ref-null.wast") || wast .iter() .any(|s| s == "table_copy_on_imported_tables.wast"); From 98ecef1700c974fbb0fc20fe08b4bf628ee7ef52 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 19 Feb 2020 15:37:49 -0800 Subject: [PATCH 04/11] Implement the `memory.copy` instruction from the bulk memory proposal --- crates/environ/src/func_environ.rs | 92 +++++++++++-- crates/runtime/src/instance.rs | 68 ++++++++- crates/runtime/src/libcalls.rs | 36 +++++ crates/runtime/src/vmcontext.rs | 5 + .../misc_testsuite/imported-memory-copy.wast | 129 ++++++++++++++++++ tests/misc_testsuite/memory-copy.wast | 124 +++++++++++++++++ tests/wast_testsuites.rs | 2 + 7 files changed, 445 insertions(+), 11 deletions(-) create mode 100644 tests/misc_testsuite/imported-memory-copy.wast create mode 100644 tests/misc_testsuite/memory-copy.wast diff --git a/crates/environ/src/func_environ.rs b/crates/environ/src/func_environ.rs index f3a11ff735e4..39e8d5f0b47b 100644 --- a/crates/environ/src/func_environ.rs +++ b/crates/environ/src/func_environ.rs @@ -70,9 +70,17 @@ impl BuiltinFunctionIndex { pub const fn get_elem_drop_index() -> Self { Self(9) } + /// Returns an index for wasm's `memory.copy` for locally defined memories. + pub const fn get_memory_copy_index() -> Self { + Self(10) + } + /// Returns an index for wasm's `memory.copy` for imported memories. + pub const fn get_imported_memory_copy_index() -> Self { + Self(11) + } /// Returns the total number of builtin functions. pub const fn builtin_functions_total_number() -> u32 { - 10 + 12 } /// Return the index as an u32 number. @@ -110,6 +118,10 @@ pub struct FuncEnvironment<'module_environment> { /// The external function signature for implementing wasm's `elem.drop`. elem_drop_sig: Option, + /// The external function signature for implementing wasm's `memory.copy` + /// (it's the same for both local and imported memories). + memory_copy_sig: Option, + /// Offsets to struct fields accessed by JIT code. offsets: VMOffsets, } @@ -128,6 +140,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> { table_copy_sig: None, table_init_sig: None, elem_drop_sig: None, + memory_copy_sig: None, offsets: VMOffsets::new(target_config.pointer_bytes(), module), } } @@ -371,6 +384,51 @@ impl<'module_environment> FuncEnvironment<'module_environment> { (sig, BuiltinFunctionIndex::get_elem_drop_index()) } + fn get_memory_copy_sig(&mut self, func: &mut Function) -> ir::SigRef { + let sig = self.memory_copy_sig.unwrap_or_else(|| { + func.import_signature(Signature { + params: vec![ + AbiParam::special(self.pointer_type(), ArgumentPurpose::VMContext), + // Memory index. + AbiParam::new(I32), + // Destination address. + AbiParam::new(I32), + // Source address. + AbiParam::new(I32), + // Length. + AbiParam::new(I32), + // Source location. + AbiParam::new(I32), + ], + returns: vec![], + call_conv: self.target_config.default_call_conv, + }) + }); + self.memory_copy_sig = Some(sig); + sig + } + + fn get_memory_copy_func( + &mut self, + func: &mut Function, + memory_index: MemoryIndex, + ) -> (ir::SigRef, usize, BuiltinFunctionIndex) { + let sig = self.get_memory_copy_sig(func); + if let Some(defined_memory_index) = self.module.defined_memory_index(memory_index) { + ( + sig, + defined_memory_index.index(), + BuiltinFunctionIndex::get_memory_copy_index(), + ) + } else { + ( + sig, + memory_index.index(), + BuiltinFunctionIndex::get_imported_memory_copy_index(), + ) + } + } + /// Translates load of builtin function and returns a pair of values `vmctx` /// and address of the loaded function. fn translate_load_builtin_function_address( @@ -968,16 +1026,30 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m fn translate_memory_copy( &mut self, - _pos: FuncCursor, - _index: MemoryIndex, + mut pos: FuncCursor, + memory_index: MemoryIndex, _heap: ir::Heap, - _dst: ir::Value, - _src: ir::Value, - _len: ir::Value, + dst: ir::Value, + src: ir::Value, + len: ir::Value, ) -> WasmResult<()> { - Err(WasmError::Unsupported( - "bulk memory: `memory.copy`".to_string(), - )) + let (func_sig, memory_index, func_idx) = + self.get_memory_copy_func(&mut pos.func, memory_index); + + let memory_index_arg = pos.ins().iconst(I32, memory_index as i64); + + let (vmctx, func_addr) = self.translate_load_builtin_function_address(&mut pos, func_idx); + + let src_loc = pos.srcloc(); + let src_loc_arg = pos.ins().iconst(I32, src_loc.bits() as i64); + + pos.ins().call_indirect( + func_sig, + func_addr, + &[vmctx, memory_index_arg, dst, src, len, src_loc_arg], + ); + + Ok(()) } fn translate_memory_fill( @@ -1005,7 +1077,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m _len: ir::Value, ) -> WasmResult<()> { Err(WasmError::Unsupported( - "bulk memory: `memory.copy`".to_string(), + "bulk memory: `memory.init`".to_string(), )) } diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 4ae120d8f1b8..395969388af4 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -15,6 +15,7 @@ use crate::vmcontext::{ VMTableDefinition, VMTableImport, }; use crate::{TrapDescription, TrapRegistration}; +use backtrace::Backtrace; use memoffset::offset_of; use more_asserts::assert_lt; use std::alloc::{self, Layout}; @@ -601,7 +602,7 @@ impl Instance { source_loc, trap_code: ir::TrapCode::TableOutOfBounds, }, - backtrace: backtrace::Backtrace::new(), + backtrace: Backtrace::new(), }); } @@ -625,6 +626,71 @@ impl Instance { } } + /// Do a `memory.copy` for a locally defined memory. + /// + /// # Errors + /// + /// Returns a `Trap` error when the source or destination ranges are out of + /// bounds. + pub(crate) fn defined_memory_copy( + &self, + memory_index: DefinedMemoryIndex, + dst: u32, + src: u32, + len: u32, + source_loc: ir::SourceLoc, + ) -> Result<(), Trap> { + // https://webassembly.github.io/reference-types/core/exec/instructions.html#exec-memory-copy + + let memory = self.memory(memory_index); + if src + .checked_add(len) + .map_or(true, |n| n as usize > memory.current_length) + || dst + .checked_add(len) + .map_or(true, |m| m as usize > memory.current_length) + { + return Err(Trap::Wasm { + desc: TrapDescription { + source_loc, + trap_code: ir::TrapCode::HeapOutOfBounds, + }, + backtrace: Backtrace::new(), + }); + } + + let dst = isize::try_from(dst).unwrap(); + let src = isize::try_from(src).unwrap(); + + // Bounds and casts are checked above, by this point we know that + // everything is safe. + unsafe { + let dst = memory.base.offset(dst); + let src = memory.base.offset(src); + ptr::copy(src, dst, len as usize); + } + + Ok(()) + } + + /// Perform a `memory.copy` on an imported memory. + pub(crate) fn imported_memory_copy( + &self, + memory_index: MemoryIndex, + dst: u32, + src: u32, + len: u32, + source_loc: ir::SourceLoc, + ) -> Result<(), Trap> { + let import = self.imported_memory(memory_index); + unsafe { + let foreign_instance = (&*import.vmctx).instance(); + let foreign_memory = &*import.from; + let foreign_index = foreign_instance.memory_index(foreign_memory); + foreign_instance.defined_memory_copy(foreign_index, dst, src, len, source_loc) + } + } + /// Get a table by index regardless of whether it is locally-defined or an /// imported, foreign table. pub(crate) fn get_table(&self, table_index: TableIndex) -> &Table { diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 3e4c0f93e9ca..b83627f72ba9 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -262,3 +262,39 @@ pub unsafe extern "C" fn wasmtime_elem_drop(vmctx: *mut VMContext, elem_index: u let instance = (&mut *vmctx).instance(); instance.elem_drop(elem_index); } + +/// Implementation of `memory.copy` for locally defined memories. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_memory_copy( + vmctx: *mut VMContext, + memory_index: u32, + dst: u32, + src: u32, + len: u32, + source_loc: u32, +) { + let memory_index = DefinedMemoryIndex::from_u32(memory_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + if let Err(trap) = instance.defined_memory_copy(memory_index, dst, src, len, source_loc) { + raise_lib_trap(trap); + } +} + +/// Implementation of `memory.copy` for imported memories. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_imported_memory_copy( + vmctx: *mut VMContext, + memory_index: u32, + dst: u32, + src: u32, + len: u32, + source_loc: u32, +) { + let memory_index = MemoryIndex::from_u32(memory_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + if let Err(trap) = instance.imported_memory_copy(memory_index, dst, src, len, source_loc) { + raise_lib_trap(trap); + } +} diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index 613cf9af9952..50b02f7b4874 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -563,6 +563,11 @@ impl VMBuiltinFunctionsArray { ptrs[BuiltinFunctionIndex::get_elem_drop_index().index() as usize] = wasmtime_elem_drop as usize; + ptrs[BuiltinFunctionIndex::get_memory_copy_index().index() as usize] = + wasmtime_memory_copy as usize; + ptrs[BuiltinFunctionIndex::get_imported_memory_copy_index().index() as usize] = + wasmtime_imported_memory_copy as usize; + debug_assert!(ptrs.iter().cloned().all(|p| p != 0)); Self { ptrs } diff --git a/tests/misc_testsuite/imported-memory-copy.wast b/tests/misc_testsuite/imported-memory-copy.wast new file mode 100644 index 000000000000..4787173cc787 --- /dev/null +++ b/tests/misc_testsuite/imported-memory-copy.wast @@ -0,0 +1,129 @@ +(module $foreign + (memory (export "mem") 1 1) + (data 0 (i32.const 1000) "hello") + (data 0 (i32.const 2000) "olleh")) + +(register "foreign" $foreign) + +(module + (memory (import "foreign" "mem") 1 1) + + (func $is_char (param i32 i32) (result i32) + local.get 0 + i32.load8_u + local.get 1 + i32.eq) + + (func (export "is hello?") (param i32) (result i32) + local.get 0 + i32.const 104 ;; 'h' + call $is_char + + local.get 0 + i32.const 1 + i32.add + i32.const 101 ;; 'e' + call $is_char + + local.get 0 + i32.const 2 + i32.add + i32.const 108 ;; 'l' + call $is_char + + local.get 0 + i32.const 3 + i32.add + i32.const 108 ;; 'l' + call $is_char + + local.get 0 + i32.const 4 + i32.add + i32.const 111 ;; 'o' + call $is_char + + i32.and + i32.and + i32.and + i32.and + ) + + (func (export "is olleh?") (param i32) (result i32) + local.get 0 + i32.const 111 ;; 'o' + call $is_char + + local.get 0 + i32.const 1 + i32.add + i32.const 108 ;; 'l' + call $is_char + + local.get 0 + i32.const 2 + i32.add + i32.const 108 ;; 'l' + call $is_char + + local.get 0 + i32.const 3 + i32.add + i32.const 101 ;; 'e' + call $is_char + + local.get 0 + i32.const 4 + i32.add + i32.const 104 ;; 'h' + call $is_char + + i32.and + i32.and + i32.and + i32.and + ) + + (func (export "memory.copy") (param i32 i32 i32) + local.get 0 + local.get 1 + local.get 2 + memory.copy)) + +;; Our memory has our initial data in the right places. +(assert_return + (invoke "is hello?" (i32.const 1000)) + (i32.const 1)) +(assert_return + (invoke "is olleh?" (i32.const 2000)) + (i32.const 1)) + +;; Non-overlapping memory copy with dst < src. +(invoke "memory.copy" (i32.const 500) (i32.const 1000) (i32.const 5)) +(assert_return + (invoke "is hello?" (i32.const 500)) + (i32.const 1)) + +;; Non-overlapping memory copy with dst > src. +(invoke "memory.copy" (i32.const 1500) (i32.const 1000) (i32.const 5)) +(assert_return + (invoke "is hello?" (i32.const 1500)) + (i32.const 1)) + +;; Overlapping memory copy with dst < src. +(invoke "memory.copy" (i32.const 1998) (i32.const 2000) (i32.const 5)) +(assert_return + (invoke "is olleh?" (i32.const 1998)) + (i32.const 1)) + +;; Overlapping memory copy with dst > src. +(invoke "memory.copy" (i32.const 2000) (i32.const 1998) (i32.const 5)) +(assert_return + (invoke "is olleh?" (i32.const 2000)) + (i32.const 1)) + +;; Overlapping memory copy with dst = src. +(invoke "memory.copy" (i32.const 2000) (i32.const 2000) (i32.const 5)) +(assert_return + (invoke "is olleh?" (i32.const 2000)) + (i32.const 1)) diff --git a/tests/misc_testsuite/memory-copy.wast b/tests/misc_testsuite/memory-copy.wast new file mode 100644 index 000000000000..d3392644237b --- /dev/null +++ b/tests/misc_testsuite/memory-copy.wast @@ -0,0 +1,124 @@ +(module + (memory 1 1) + (data 0 (i32.const 1000) "hello") + (data 0 (i32.const 2000) "olleh") + + (func $is_char (param i32 i32) (result i32) + local.get 0 + i32.load8_u + local.get 1 + i32.eq) + + (func (export "is hello?") (param i32) (result i32) + local.get 0 + i32.const 104 ;; 'h' + call $is_char + + local.get 0 + i32.const 1 + i32.add + i32.const 101 ;; 'e' + call $is_char + + local.get 0 + i32.const 2 + i32.add + i32.const 108 ;; 'l' + call $is_char + + local.get 0 + i32.const 3 + i32.add + i32.const 108 ;; 'l' + call $is_char + + local.get 0 + i32.const 4 + i32.add + i32.const 111 ;; 'o' + call $is_char + + i32.and + i32.and + i32.and + i32.and + ) + + (func (export "is olleh?") (param i32) (result i32) + local.get 0 + i32.const 111 ;; 'o' + call $is_char + + local.get 0 + i32.const 1 + i32.add + i32.const 108 ;; 'l' + call $is_char + + local.get 0 + i32.const 2 + i32.add + i32.const 108 ;; 'l' + call $is_char + + local.get 0 + i32.const 3 + i32.add + i32.const 101 ;; 'e' + call $is_char + + local.get 0 + i32.const 4 + i32.add + i32.const 104 ;; 'h' + call $is_char + + i32.and + i32.and + i32.and + i32.and + ) + + (func (export "memory.copy") (param i32 i32 i32) + local.get 0 + local.get 1 + local.get 2 + memory.copy)) + +;; Our memory has our initial data in the right places. +(assert_return + (invoke "is hello?" (i32.const 1000)) + (i32.const 1)) +(assert_return + (invoke "is olleh?" (i32.const 2000)) + (i32.const 1)) + +;; Non-overlapping memory copy with dst < src. +(invoke "memory.copy" (i32.const 500) (i32.const 1000) (i32.const 5)) +(assert_return + (invoke "is hello?" (i32.const 500)) + (i32.const 1)) + +;; Non-overlapping memory copy with dst > src. +(invoke "memory.copy" (i32.const 1500) (i32.const 1000) (i32.const 5)) +(assert_return + (invoke "is hello?" (i32.const 1500)) + (i32.const 1)) + +;; Overlapping memory copy with dst < src. +(invoke "memory.copy" (i32.const 1998) (i32.const 2000) (i32.const 5)) +(assert_return + (invoke "is olleh?" (i32.const 1998)) + (i32.const 1)) + +;; Overlapping memory copy with dst > src. +(invoke "memory.copy" (i32.const 2000) (i32.const 1998) (i32.const 5)) +(assert_return + (invoke "is olleh?" (i32.const 2000)) + (i32.const 1)) + +;; Overlapping memory copy with dst = src. +(invoke "memory.copy" (i32.const 2000) (i32.const 2000) (i32.const 5)) +(assert_return + (invoke "is olleh?" (i32.const 2000)) + (i32.const 1)) diff --git a/tests/wast_testsuites.rs b/tests/wast_testsuites.rs index cb7599bc94fb..5a4ebedfdf7b 100644 --- a/tests/wast_testsuites.rs +++ b/tests/wast_testsuites.rs @@ -22,6 +22,8 @@ fn run_wast(wast: &str, strategy: Strategy) -> anyhow::Result<()> { || wast.iter().any(|s| s == "table_copy.wast") || wast.iter().any(|s| s == "elem_drop.wast") || wast.iter().any(|s| s == "elem-ref-null.wast") + || wast.iter().any(|s| s == "memory-copy.wast") + || wast.iter().any(|s| s == "imported-memory-copy.wast") || wast .iter() .any(|s| s == "table_copy_on_imported_tables.wast"); From 44c28612fbeeffce0fca5b5dc15d7e21e74e271e Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 19 Feb 2020 16:41:05 -0800 Subject: [PATCH 05/11] Implement the `memory.fill` instruction from the bulk memory proposal --- build.rs | 4 +- crates/environ/src/func_environ.rs | 90 +++++++++++++++++++++++++++--- crates/runtime/src/instance.rs | 64 +++++++++++++++++++++ crates/runtime/src/libcalls.rs | 36 ++++++++++++ crates/runtime/src/vmcontext.rs | 4 ++ 5 files changed, 188 insertions(+), 10 deletions(-) diff --git a/build.rs b/build.rs index 1d71b908fe7c..4d2961e9a5db 100644 --- a/build.rs +++ b/build.rs @@ -184,7 +184,9 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { ("bulk_memory_operations", "table_copy") | ("bulk_memory_operations", "table_init") - | ("bulk_memory_operations", "elem") => return false, + | ("bulk_memory_operations", "elem") + | ("bulk_memory_operations", "memory_copy") + | ("bulk_memory_operations", "memory_fill") => return false, ("bulk_memory_operations", _) => return true, _ => {} diff --git a/crates/environ/src/func_environ.rs b/crates/environ/src/func_environ.rs index 39e8d5f0b47b..bbddb708e664 100644 --- a/crates/environ/src/func_environ.rs +++ b/crates/environ/src/func_environ.rs @@ -78,9 +78,17 @@ impl BuiltinFunctionIndex { pub const fn get_imported_memory_copy_index() -> Self { Self(11) } + /// Returns an index for wasm's `memory.fill` for locally defined memories. + pub const fn get_memory_fill_index() -> Self { + Self(12) + } + /// Returns an index for wasm's `memory.fill` for imported memories. + pub const fn get_imported_memory_fill_index() -> Self { + Self(13) + } /// Returns the total number of builtin functions. pub const fn builtin_functions_total_number() -> u32 { - 12 + 14 } /// Return the index as an u32 number. @@ -122,6 +130,10 @@ pub struct FuncEnvironment<'module_environment> { /// (it's the same for both local and imported memories). memory_copy_sig: Option, + /// The external function signature for implementing wasm's `memory.fill` + /// (it's the same for both local and imported memories). + memory_fill_sig: Option, + /// Offsets to struct fields accessed by JIT code. offsets: VMOffsets, } @@ -141,6 +153,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> { table_init_sig: None, elem_drop_sig: None, memory_copy_sig: None, + memory_fill_sig: None, offsets: VMOffsets::new(target_config.pointer_bytes(), module), } } @@ -429,6 +442,51 @@ impl<'module_environment> FuncEnvironment<'module_environment> { } } + fn get_memory_fill_sig(&mut self, func: &mut Function) -> ir::SigRef { + let sig = self.memory_fill_sig.unwrap_or_else(|| { + func.import_signature(Signature { + params: vec![ + AbiParam::special(self.pointer_type(), ArgumentPurpose::VMContext), + // Memory index. + AbiParam::new(I32), + // Destination address. + AbiParam::new(I32), + // Value. + AbiParam::new(I32), + // Length. + AbiParam::new(I32), + // Source location. + AbiParam::new(I32), + ], + returns: vec![], + call_conv: self.target_config.default_call_conv, + }) + }); + self.memory_fill_sig = Some(sig); + sig + } + + fn get_memory_fill_func( + &mut self, + func: &mut Function, + memory_index: MemoryIndex, + ) -> (ir::SigRef, usize, BuiltinFunctionIndex) { + let sig = self.get_memory_fill_sig(func); + if let Some(defined_memory_index) = self.module.defined_memory_index(memory_index) { + ( + sig, + defined_memory_index.index(), + BuiltinFunctionIndex::get_memory_fill_index(), + ) + } else { + ( + sig, + memory_index.index(), + BuiltinFunctionIndex::get_imported_memory_fill_index(), + ) + } + } + /// Translates load of builtin function and returns a pair of values `vmctx` /// and address of the loaded function. fn translate_load_builtin_function_address( @@ -1054,16 +1112,30 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m fn translate_memory_fill( &mut self, - _pos: FuncCursor, - _index: MemoryIndex, + mut pos: FuncCursor, + memory_index: MemoryIndex, _heap: ir::Heap, - _dst: ir::Value, - _val: ir::Value, - _len: ir::Value, + dst: ir::Value, + val: ir::Value, + len: ir::Value, ) -> WasmResult<()> { - Err(WasmError::Unsupported( - "bulk memory: `memory.fill`".to_string(), - )) + let (func_sig, memory_index, func_idx) = + self.get_memory_fill_func(&mut pos.func, memory_index); + + let memory_index_arg = pos.ins().iconst(I32, memory_index as i64); + + let (vmctx, func_addr) = self.translate_load_builtin_function_address(&mut pos, func_idx); + + let src_loc = pos.srcloc(); + let src_loc_arg = pos.ins().iconst(I32, src_loc.bits() as i64); + + pos.ins().call_indirect( + func_sig, + func_addr, + &[vmctx, memory_index_arg, dst, val, len, src_loc_arg], + ); + + Ok(()) } fn translate_memory_init( diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 395969388af4..2d13b65d7aa8 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -643,6 +643,7 @@ impl Instance { // https://webassembly.github.io/reference-types/core/exec/instructions.html#exec-memory-copy let memory = self.memory(memory_index); + if src .checked_add(len) .map_or(true, |n| n as usize > memory.current_length) @@ -691,6 +692,69 @@ impl Instance { } } + /// Perform the `memory.fill` operation on a locally defined memory. + /// + /// # Errors + /// + /// Returns a `Trap` error if the memory range is out of bounds. + pub(crate) fn defined_memory_fill( + &self, + memory_index: DefinedMemoryIndex, + dst: u32, + val: u32, + len: u32, + source_loc: ir::SourceLoc, + ) -> Result<(), Trap> { + let memory = self.memory(memory_index); + + if dst + .checked_add(len) + .map_or(true, |m| m as usize > memory.current_length) + { + return Err(Trap::Wasm { + desc: TrapDescription { + source_loc, + trap_code: ir::TrapCode::HeapOutOfBounds, + }, + backtrace: Backtrace::new(), + }); + } + + let dst = isize::try_from(dst).unwrap(); + let val = val as u8; + + // Bounds and casts are checked above, by this point we know that + // everything is safe. + unsafe { + let dst = memory.base.offset(dst); + ptr::write_bytes(dst, val, len as usize); + } + + Ok(()) + } + + /// Perform the `memory.fill` operation on an imported memory. + /// + /// # Errors + /// + /// Returns a `Trap` error if the memory range is out of bounds. + pub(crate) fn imported_memory_fill( + &self, + memory_index: MemoryIndex, + dst: u32, + val: u32, + len: u32, + source_loc: ir::SourceLoc, + ) -> Result<(), Trap> { + let import = self.imported_memory(memory_index); + unsafe { + let foreign_instance = (&*import.vmctx).instance(); + let foreign_memory = &*import.from; + let foreign_index = foreign_instance.memory_index(foreign_memory); + foreign_instance.defined_memory_fill(foreign_index, dst, val, len, source_loc) + } + } + /// Get a table by index regardless of whether it is locally-defined or an /// imported, foreign table. pub(crate) fn get_table(&self, table_index: TableIndex) -> &Table { diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index b83627f72ba9..65d6331aa49c 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -298,3 +298,39 @@ pub unsafe extern "C" fn wasmtime_imported_memory_copy( raise_lib_trap(trap); } } + +/// Implementation of `memory.fill` for locally defined memories. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_memory_fill( + vmctx: *mut VMContext, + memory_index: u32, + dst: u32, + val: u32, + len: u32, + source_loc: u32, +) { + let memory_index = DefinedMemoryIndex::from_u32(memory_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + if let Err(trap) = instance.defined_memory_fill(memory_index, dst, val, len, source_loc) { + raise_lib_trap(trap); + } +} + +/// Implementation of `memory.fill` for imported memories. +#[no_mangle] +pub unsafe extern "C" fn wasmtime_imported_memory_fill( + vmctx: *mut VMContext, + memory_index: u32, + dst: u32, + val: u32, + len: u32, + source_loc: u32, +) { + let memory_index = MemoryIndex::from_u32(memory_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + if let Err(trap) = instance.imported_memory_fill(memory_index, dst, val, len, source_loc) { + raise_lib_trap(trap); + } +} diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index 50b02f7b4874..6cb640f0e203 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -567,6 +567,10 @@ impl VMBuiltinFunctionsArray { wasmtime_memory_copy as usize; ptrs[BuiltinFunctionIndex::get_imported_memory_copy_index().index() as usize] = wasmtime_imported_memory_copy as usize; + ptrs[BuiltinFunctionIndex::get_memory_fill_index().index() as usize] = + wasmtime_memory_fill as usize; + ptrs[BuiltinFunctionIndex::get_imported_memory_fill_index().index() as usize] = + wasmtime_imported_memory_fill as usize; debug_assert!(ptrs.iter().cloned().all(|p| p != 0)); From 81227892daf6695e9de2c23711ac58ec1cf30384 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 20 Feb 2020 16:19:36 -0800 Subject: [PATCH 06/11] Implement bulk memory's partial failure instantiation semantics Essentially, table and memory out of bounds errors are no longer link errors, but traps after linking. This means that the partail writes / inits are visible. --- build.rs | 3 +- crates/api/src/instance.rs | 8 ++- crates/api/src/trampoline/create_handle.rs | 6 ++ crates/jit/src/instantiate.rs | 7 ++- crates/runtime/src/instance.rs | 67 +++++++++++++++++++--- tests/instantiate.rs | 12 +++- 6 files changed, 88 insertions(+), 15 deletions(-) diff --git a/build.rs b/build.rs index 4d2961e9a5db..1942deec8935 100644 --- a/build.rs +++ b/build.rs @@ -186,7 +186,8 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { | ("bulk_memory_operations", "table_init") | ("bulk_memory_operations", "elem") | ("bulk_memory_operations", "memory_copy") - | ("bulk_memory_operations", "memory_fill") => return false, + | ("bulk_memory_operations", "memory_fill") + | ("bulk_memory_operations", "linking") => return false, ("bulk_memory_operations", _) => return true, _ => {} diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index 065627dc515e..546a1bb45da1 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -26,11 +26,15 @@ fn instantiate( let mut resolver = SimpleResolver { imports }; unsafe { let instance = compiled_module - .instantiate(&mut resolver) + .instantiate( + config.validating_config.operator_config.enable_bulk_memory, + &mut resolver, + ) .map_err(|e| -> Error { match e { InstantiationError::StartTrap(trap) => Trap::from_jit(trap).into(), - e @ InstantiationError::TableOutOfBounds(_) => { + e @ InstantiationError::TableOutOfBounds(_) + | e @ InstantiationError::MemoryOutOfBounds(_) => { let msg = e.to_string(); if config.validating_config.operator_config.enable_bulk_memory { Trap::new(msg).into() diff --git a/crates/api/src/trampoline/create_handle.rs b/crates/api/src/trampoline/create_handle.rs index ea87ee394d19..d3d295de9dd6 100644 --- a/crates/api/src/trampoline/create_handle.rs +++ b/crates/api/src/trampoline/create_handle.rs @@ -42,6 +42,12 @@ pub(crate) fn create_handle( &data_initializers, signatures.into_boxed_slice(), None, + store + .engine() + .config() + .validating_config + .operator_config + .enable_bulk_memory, state, )?) } diff --git a/crates/jit/src/instantiate.rs b/crates/jit/src/instantiate.rs index f860fb2f8d39..957f1c409924 100644 --- a/crates/jit/src/instantiate.rs +++ b/crates/jit/src/instantiate.rs @@ -205,6 +205,7 @@ impl CompiledModule { /// See `InstanceHandle::new` pub unsafe fn instantiate( &self, + is_bulk_memory: bool, resolver: &mut dyn Resolver, ) -> Result { let data_initializers = self @@ -224,6 +225,7 @@ impl CompiledModule { &data_initializers, self.signatures.clone(), self.dbg_jit_registration.as_ref().map(|r| Rc::clone(&r)), + is_bulk_memory, Box::new(()), ) } @@ -277,9 +279,10 @@ pub unsafe fn instantiate( data: &[u8], resolver: &mut dyn Resolver, debug_info: bool, + is_bulk_memory: bool, profiler: Option<&Arc>>>, ) -> Result { - let instance = - CompiledModule::new(compiler, data, debug_info, profiler)?.instantiate(resolver)?; + let instance = CompiledModule::new(compiler, data, debug_info, profiler)? + .instantiate(is_bulk_memory, resolver)?; Ok(instance) } diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 2d13b65d7aa8..5bf035f985fc 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -21,6 +21,7 @@ use more_asserts::assert_lt; use std::alloc::{self, Layout}; use std::any::Any; use std::cell::{Cell, RefCell}; +use std::cmp; use std::collections::HashMap; use std::collections::HashSet; use std::convert::TryFrom; @@ -810,6 +811,7 @@ impl InstanceHandle { data_initializers: &[DataInitializer<'_>], vmshared_signatures: BoxedSlice, dbg_jit_registration: Option>, + is_bulk_memory: bool, host_state: Box, ) -> Result { let tables = create_tables(&module); @@ -904,9 +906,14 @@ impl InstanceHandle { VMBuiltinFunctionsArray::initialized(), ); - // Check initializer bounds before initializing anything. - check_table_init_bounds(instance)?; - check_memory_init_bounds(instance, data_initializers)?; + // Check initializer bounds before initializing anything. Only do this + // when bulk memory is disabled, since the bulk memory proposal changes + // instantiation such that the intermediate results of failed + // initializations are visible. + if !is_bulk_memory { + check_table_init_bounds(instance)?; + check_memory_init_bounds(instance, data_initializers)?; + } // Apply the initializers. initialize_tables(instance)?; @@ -1133,9 +1140,9 @@ fn check_memory_init_bounds( unsafe { let mem_slice = get_memory_slice(init, instance); if mem_slice.get_mut(start..start + init.data.len()).is_none() { - return Err(InstantiationError::Link(LinkError( - "data segment does not fit".to_owned(), - ))); + return Err(InstantiationError::MemoryOutOfBounds( + "data segment does not fit".into(), + )); } } } @@ -1179,11 +1186,29 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { let start = get_table_init_start(init, instance); let table = instance.get_table(init.table_index); + // Even if there aren't any elements being initialized, if its out of + // bounds, then we report an error. This is required by the bulk memory + // proposal and the way that it uses `table.init` during instantiation. + if start > table.size() as usize { + return Err(InstantiationError::TableOutOfBounds( + "active element segment does not fit in table".into(), + )); + } + for (i, func_idx) in init.elements.iter().enumerate() { let anyfunc = instance.get_caller_checked_anyfunc(*func_idx); table .set(u32::try_from(start + i).unwrap(), anyfunc) - .unwrap(); + // Note that when multi-value is disabled, this will never fail + // since we bounds check table element initialization before + // doing any table slot writes. However when multi-value is + // enabled, these become runtime traps and the intermediate + // table slot writes are visible. + .map_err(|()| { + InstantiationError::TableOutOfBounds( + "active element segment does not fit in table".into(), + ) + })?; } } @@ -1239,8 +1264,25 @@ fn initialize_memories( let start = get_memory_init_start(init, instance); unsafe { let mem_slice = get_memory_slice(init, instance); - let to_init = &mut mem_slice[start..start + init.data.len()]; - to_init.copy_from_slice(init.data); + + let end = start.saturating_add(init.data.len()); + let actual_end = cmp::min(mem_slice.len(), end); + let num_uncopied = end - actual_end; + + let to_init = &mut mem_slice[start..actual_end]; + to_init.copy_from_slice(&init.data[..init.data.len() - num_uncopied]); + + // Note that if bulk memory is disabled, we'll never hit this case + // because we did an eager bounds check in + // `check_memory_init_bounds`. However when bulk memory is enabled, + // we need the incremental writes up to the OOB trap to be visible, + // since this is dictated by the updated spec for the bulk memory + // proposal. + if num_uncopied > 0 { + return Err(InstantiationError::MemoryOutOfBounds( + "active data segment does not fit in memory".into(), + )); + } } } @@ -1307,6 +1349,13 @@ pub enum InstantiationError { #[error("Table out of bounds error: {0}")] TableOutOfBounds(String), + /// A memory out of bounds error. + /// + /// Depending on whether bulk memory is enabled or not, this is either a + /// link error or a trap raised during instantiation. + #[error("Memory out of bounds error: {0}")] + MemoryOutOfBounds(String), + /// A wasm link error occured. #[error("Failed to link module")] Link(#[from] LinkError), diff --git a/tests/instantiate.rs b/tests/instantiate.rs index b9bd6e1064eb..a72a2df87851 100644 --- a/tests/instantiate.rs +++ b/tests/instantiate.rs @@ -24,7 +24,17 @@ fn test_environ_translate() { let cache_config = CacheConfig::new_cache_disabled(); let mut compiler = Compiler::new(isa, CompilationStrategy::Auto, cache_config); unsafe { - let instance = instantiate(&mut compiler, &data, &mut resolver, false, None); + let instance = instantiate( + &mut compiler, + &data, + &mut resolver, + // Bulk memory. + false, + // Debug info. + false, + // Profiler. + None, + ); assert!(instance.is_ok()); } } From aec8cc190478823cb9389daae1daa1969dea1bb1 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 24 Feb 2020 12:52:57 -0800 Subject: [PATCH 07/11] List the bulk-memory tests that don't pass, rather than the ones that do We've crossed the threshold where this is easier :) --- build.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/build.rs b/build.rs index 1942deec8935..cafdb32f5b6c 100644 --- a/build.rs +++ b/build.rs @@ -182,13 +182,11 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { ("reference_types", _) => return true, - ("bulk_memory_operations", "table_copy") - | ("bulk_memory_operations", "table_init") - | ("bulk_memory_operations", "elem") - | ("bulk_memory_operations", "memory_copy") - | ("bulk_memory_operations", "memory_fill") - | ("bulk_memory_operations", "linking") => return false, - ("bulk_memory_operations", _) => return true, + // Still working on implementing these. See #928 + ("bulk_memory_operations", "bulk") + | ("bulk_memory_operations", "data") + | ("bulk_memory_operations", "memory_init") + | ("bulk_memory_operations", "imports") => return true, _ => {} }, From 39307b2b36d4f3ba5ce02c689abbcb8f53a6fc8c Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 25 Feb 2020 10:14:59 -0800 Subject: [PATCH 08/11] Fix comments about bulk memory that incorrectly referenced "multi-value" --- crates/runtime/src/instance.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 5bf035f985fc..db0f07c13b32 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -1199,9 +1199,9 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { let anyfunc = instance.get_caller_checked_anyfunc(*func_idx); table .set(u32::try_from(start + i).unwrap(), anyfunc) - // Note that when multi-value is disabled, this will never fail + // Note that when bulk memory is disabled, this will never fail // since we bounds check table element initialization before - // doing any table slot writes. However when multi-value is + // doing any table slot writes. However when bulk memory is // enabled, these become runtime traps and the intermediate // table slot writes are visible. .map_err(|()| { From ef0cabf8b419df78f8733a334e7e5e5c1df7968f Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 25 Feb 2020 12:59:03 -0800 Subject: [PATCH 09/11] Address review feedback --- build.rs | 96 +++++---- crates/api/src/instance.rs | 13 +- crates/environ/src/func_environ.rs | 96 ++------- crates/runtime/src/instance.rs | 100 ++++----- crates/runtime/src/libcalls.rs | 201 ++++++++---------- crates/runtime/src/table.rs | 13 +- crates/runtime/src/traphandlers.rs | 14 ++ crates/runtime/src/vmcontext.rs | 14 +- .../elem-ref-null.wast | 0 .../elem_drop.wast | 0 .../imported-memory-copy.wast | 0 .../memory-copy.wast | 0 .../table_copy.wast | 0 .../table_copy_on_imported_tables.wast | 0 tests/wast_testsuites.rs | 20 +- 15 files changed, 220 insertions(+), 347 deletions(-) rename tests/misc_testsuite/{ => bulk-memory-operations}/elem-ref-null.wast (100%) rename tests/misc_testsuite/{ => bulk-memory-operations}/elem_drop.wast (100%) rename tests/misc_testsuite/{ => bulk-memory-operations}/imported-memory-copy.wast (100%) rename tests/misc_testsuite/{ => bulk-memory-operations}/memory-copy.wast (100%) rename tests/misc_testsuite/{ => bulk-memory-operations}/table_copy.wast (100%) rename tests/misc_testsuite/{ => reference-types}/table_copy_on_imported_tables.wast (100%) diff --git a/build.rs b/build.rs index cafdb32f5b6c..f8a847abe299 100644 --- a/build.rs +++ b/build.rs @@ -26,40 +26,38 @@ fn main() -> anyhow::Result<()> { writeln!(out, "#[allow(non_snake_case)]")?; writeln!(out, "mod {} {{", strategy)?; - test_directory(&mut out, "tests/misc_testsuite", strategy)?; - let spec_tests = test_directory(&mut out, "tests/spec_testsuite", strategy)?; - // Skip running spec_testsuite tests if the submodule isn't checked - // out. - if spec_tests > 0 { - test_directory(&mut out, "tests/spec_testsuite/proposals/simd", strategy) - .expect("generating tests"); - - test_directory( - &mut out, - "tests/spec_testsuite/proposals/multi-value", - strategy, - ) - .expect("generating tests"); - - test_directory( - &mut out, - "tests/spec_testsuite/proposals/reference-types", - strategy, - ) - .expect("generating tests"); - - test_directory( - &mut out, - "tests/spec_testsuite/proposals/bulk-memory-operations", - strategy, - ) - .expect("generating tests"); - } else { - println!( - "cargo:warning=The spec testsuite is disabled. To enable, run `git submodule \ + with_test_module(&mut out, "misc", |out| { + test_directory(out, "tests/misc_testsuite", strategy)?; + test_directory_module(out, "tests/misc_testsuite/bulk-memory-operations", strategy)?; + test_directory_module(out, "tests/misc_testsuite/reference-types", strategy)?; + Ok(()) + })?; + + with_test_module(&mut out, "spec", |out| { + let spec_tests = test_directory(out, "tests/spec_testsuite", strategy)?; + // Skip running spec_testsuite tests if the submodule isn't checked + // out. + if spec_tests > 0 { + test_directory_module(out, "tests/spec_testsuite/proposals/simd", strategy)?; + test_directory_module(out, "tests/spec_testsuite/proposals/multi-value", strategy)?; + test_directory_module( + out, + "tests/spec_testsuite/proposals/reference-types", + strategy, + )?; + test_directory_module( + out, + "tests/spec_testsuite/proposals/bulk-memory-operations", + strategy, + )?; + } else { + println!( + "cargo:warning=The spec testsuite is disabled. To enable, run `git submodule \ update --remote`." - ); - } + ); + } + Ok(()) + })?; writeln!(out, "}}")?; } @@ -72,6 +70,16 @@ fn main() -> anyhow::Result<()> { Ok(()) } +fn test_directory_module( + out: &mut String, + path: impl AsRef, + strategy: &str, +) -> anyhow::Result { + let path = path.as_ref(); + let testsuite = &extract_name(path); + with_test_module(out, testsuite, |out| test_directory(out, path, strategy)) +} + fn test_directory( out: &mut String, path: impl AsRef, @@ -100,11 +108,10 @@ fn test_directory( dir_entries.sort(); let testsuite = &extract_name(path); - start_test_module(out, testsuite)?; for entry in dir_entries.iter() { write_testsuite_tests(out, entry, testsuite, strategy)?; } - finish_test_module(out)?; + Ok(dir_entries.len()) } @@ -119,14 +126,19 @@ fn extract_name(path: impl AsRef) -> String { .replace("/", "_") } -fn start_test_module(out: &mut String, testsuite: &str) -> anyhow::Result<()> { - writeln!(out, "mod {} {{", testsuite)?; - Ok(()) -} +fn with_test_module( + out: &mut String, + testsuite: &str, + f: impl FnOnce(&mut String) -> anyhow::Result, +) -> anyhow::Result { + out.push_str("mod "); + out.push_str(testsuite); + out.push_str(" {\n"); + + let result = f(out)?; -fn finish_test_module(out: &mut String) -> anyhow::Result<()> { out.push_str("}\n"); - Ok(()) + Ok(result) } fn write_testsuite_tests( @@ -180,6 +192,8 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { ("simd", "simd_load_splat") => return true, // FIXME Unsupported feature: proposed SIMD operator V8x16LoadSplat { memarg: MemoryImmediate { flags: 0, offset: 0 } } ("simd", "simd_splat") => return true, // FIXME Unsupported feature: proposed SIMD operator I8x16ShrS + // Still working on implementing these. See #929. + ("reference_types", "table_copy_on_imported_tables") => return false, ("reference_types", _) => return true, // Still working on implementing these. See #928 diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index 546a1bb45da1..7d0b26260a4f 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -4,7 +4,7 @@ use crate::runtime::{Config, Store}; use crate::trap::Trap; use anyhow::{Error, Result}; use wasmtime_jit::{CompiledModule, Resolver}; -use wasmtime_runtime::{Export, InstanceHandle, InstantiationError, LinkError}; +use wasmtime_runtime::{Export, InstanceHandle, InstantiationError}; struct SimpleResolver<'a> { imports: &'a [Extern], @@ -32,15 +32,8 @@ fn instantiate( ) .map_err(|e| -> Error { match e { - InstantiationError::StartTrap(trap) => Trap::from_jit(trap).into(), - e @ InstantiationError::TableOutOfBounds(_) - | e @ InstantiationError::MemoryOutOfBounds(_) => { - let msg = e.to_string(); - if config.validating_config.operator_config.enable_bulk_memory { - Trap::new(msg).into() - } else { - InstantiationError::Link(LinkError(msg)).into() - } + InstantiationError::StartTrap(trap) | InstantiationError::Trap(trap) => { + Trap::from_jit(trap).into() } other => other.into(), } diff --git a/crates/environ/src/func_environ.rs b/crates/environ/src/func_environ.rs index bbddb708e664..0626703b6e1e 100644 --- a/crates/environ/src/func_environ.rs +++ b/crates/environ/src/func_environ.rs @@ -45,50 +45,36 @@ impl BuiltinFunctionIndex { } /// Returns an index for wasm's `table.copy` when both tables are locally /// defined. - pub const fn get_table_copy_defined_defined_index() -> Self { + pub const fn get_table_copy_index() -> Self { Self(4) } - /// Returns an index for wasm's `table.copy` when the destination table is - /// locally defined and the source table is imported. - pub const fn get_table_copy_defined_imported_index() -> Self { - Self(5) - } - /// Returns an index for wasm's `table.copy` when the destination table is - /// imported and the source table is locally defined. - pub const fn get_table_copy_imported_defined_index() -> Self { - Self(6) - } - /// Returns an index for wasm's `table.copy` when both tables are imported. - pub const fn get_table_copy_imported_imported_index() -> Self { - Self(7) - } /// Returns an index for wasm's `table.init`. pub const fn get_table_init_index() -> Self { - Self(8) + Self(5) } /// Returns an index for wasm's `elem.drop`. pub const fn get_elem_drop_index() -> Self { - Self(9) + Self(6) } /// Returns an index for wasm's `memory.copy` for locally defined memories. - pub const fn get_memory_copy_index() -> Self { - Self(10) + pub const fn get_defined_memory_copy_index() -> Self { + Self(7) } /// Returns an index for wasm's `memory.copy` for imported memories. pub const fn get_imported_memory_copy_index() -> Self { - Self(11) + Self(8) } /// Returns an index for wasm's `memory.fill` for locally defined memories. pub const fn get_memory_fill_index() -> Self { - Self(12) + Self(9) } /// Returns an index for wasm's `memory.fill` for imported memories. pub const fn get_imported_memory_fill_index() -> Self { - Self(13) + Self(10) } /// Returns the total number of builtin functions. pub const fn builtin_functions_total_number() -> u32 { - 14 + 11 } /// Return the index as an u32 number. @@ -245,7 +231,6 @@ impl<'module_environment> FuncEnvironment<'module_environment> { } } - // NB: All `table_copy` libcall variants have the same signature. fn get_table_copy_sig(&mut self, func: &mut Function) -> ir::SigRef { let sig = self.table_copy_sig.unwrap_or_else(|| { func.import_signature(Signature { @@ -279,61 +264,12 @@ impl<'module_environment> FuncEnvironment<'module_environment> { src_table_index: TableIndex, ) -> (ir::SigRef, usize, usize, BuiltinFunctionIndex) { let sig = self.get_table_copy_sig(func); - match ( - self.module.is_imported_table(dst_table_index), - self.module.is_imported_table(src_table_index), - ) { - (false, false) => { - let dst_table_index = self - .module - .defined_table_index(dst_table_index) - .unwrap() - .index(); - let src_table_index = self - .module - .defined_table_index(src_table_index) - .unwrap() - .index(); - ( - sig, - dst_table_index, - src_table_index, - BuiltinFunctionIndex::get_table_copy_defined_defined_index(), - ) - } - (false, true) => { - let dst_table_index = self - .module - .defined_table_index(dst_table_index) - .unwrap() - .index(); - ( - sig, - dst_table_index, - src_table_index.as_u32() as usize, - BuiltinFunctionIndex::get_table_copy_defined_imported_index(), - ) - } - (true, false) => { - let src_table_index = self - .module - .defined_table_index(src_table_index) - .unwrap() - .index(); - ( - sig, - dst_table_index.as_u32() as usize, - src_table_index, - BuiltinFunctionIndex::get_table_copy_imported_defined_index(), - ) - } - (true, true) => ( - sig, - dst_table_index.as_u32() as usize, - src_table_index.as_u32() as usize, - BuiltinFunctionIndex::get_table_copy_imported_imported_index(), - ), - } + ( + sig, + dst_table_index.as_u32() as usize, + src_table_index.as_u32() as usize, + BuiltinFunctionIndex::get_table_copy_index(), + ) } fn get_table_init_sig(&mut self, func: &mut Function) -> ir::SigRef { @@ -431,7 +367,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> { ( sig, defined_memory_index.index(), - BuiltinFunctionIndex::get_memory_copy_index(), + BuiltinFunctionIndex::get_defined_memory_copy_index(), ) } else { ( diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index db0f07c13b32..fe549f20e878 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -14,8 +14,7 @@ use crate::vmcontext::{ VMGlobalDefinition, VMGlobalImport, VMMemoryDefinition, VMMemoryImport, VMSharedSignatureIndex, VMTableDefinition, VMTableImport, }; -use crate::{TrapDescription, TrapRegistration}; -use backtrace::Backtrace; +use crate::TrapRegistration; use memoffset::offset_of; use more_asserts::assert_lt; use std::alloc::{self, Layout}; @@ -90,7 +89,8 @@ pub(crate) struct Instance { tables: BoxedSlice, /// Passive elements in this instantiation. As `elem.drop`s happen, these - /// entries get replaced into empty slices. + /// entries get removed. A missing entry is considered equivalent to an + /// empty slice. passive_elements: RefCell>>, /// Pointers to functions in executable memory. @@ -598,15 +598,10 @@ impl Instance { .map_or(true, |n| n as usize > elem.len()) || dst.checked_add(len).map_or(true, |m| m > table.size()) { - return Err(Trap::Wasm { - desc: TrapDescription { - source_loc, - trap_code: ir::TrapCode::TableOutOfBounds, - }, - backtrace: Backtrace::new(), - }); + return Err(Trap::wasm(source_loc, ir::TrapCode::TableOutOfBounds)); } + // TODO(#983): investigate replacing this get/set loop with a `memcpy`. for (dst, src) in (dst..dst + len).zip(src..src + len) { table .set(dst, elem[src as usize].clone()) @@ -621,10 +616,9 @@ impl Instance { // https://webassembly.github.io/reference-types/core/exec/instructions.html#exec-elem-drop let mut passive_elements = self.passive_elements.borrow_mut(); - // Note that dropping a non-passive element is a no-op (not a trap). - if let Some(elem) = passive_elements.get_mut(&elem_index) { - mem::replace(elem, vec![].into_boxed_slice()); - } + passive_elements.remove(&elem_index); + // Note that we don't check that we actually removed an element because + // dropping a non-passive element is a no-op (not a trap). } /// Do a `memory.copy` for a locally defined memory. @@ -652,23 +646,17 @@ impl Instance { .checked_add(len) .map_or(true, |m| m as usize > memory.current_length) { - return Err(Trap::Wasm { - desc: TrapDescription { - source_loc, - trap_code: ir::TrapCode::HeapOutOfBounds, - }, - backtrace: Backtrace::new(), - }); + return Err(Trap::wasm(source_loc, ir::TrapCode::HeapOutOfBounds)); } - let dst = isize::try_from(dst).unwrap(); - let src = isize::try_from(src).unwrap(); + let dst = usize::try_from(dst).unwrap(); + let src = usize::try_from(src).unwrap(); // Bounds and casts are checked above, by this point we know that // everything is safe. unsafe { - let dst = memory.base.offset(dst); - let src = memory.base.offset(src); + let dst = memory.base.add(dst); + let src = memory.base.add(src); ptr::copy(src, dst, len as usize); } @@ -712,13 +700,7 @@ impl Instance { .checked_add(len) .map_or(true, |m| m as usize > memory.current_length) { - return Err(Trap::Wasm { - desc: TrapDescription { - source_loc, - trap_code: ir::TrapCode::HeapOutOfBounds, - }, - backtrace: Backtrace::new(), - }); + return Err(Trap::wasm(source_loc, ir::TrapCode::HeapOutOfBounds)); } let dst = isize::try_from(dst).unwrap(); @@ -760,7 +742,7 @@ impl Instance { /// imported, foreign table. pub(crate) fn get_table(&self, table_index: TableIndex) -> &Table { if let Some(defined_table_index) = self.module.local.defined_table_index(table_index) { - &self.tables[defined_table_index] + self.get_defined_table(defined_table_index) } else { self.get_foreign_table(table_index) } @@ -1083,9 +1065,9 @@ fn check_table_init_bounds(instance: &Instance) -> Result<(), InstantiationError let size = usize::try_from(table.size()).unwrap(); if size < start + init.elements.len() { - return Err(InstantiationError::TableOutOfBounds( - "elements segment does not fit".to_owned(), - )); + return Err(InstantiationError::Link(LinkError( + "table out of bounds: elements segment does not fit".to_owned(), + ))); } } @@ -1140,9 +1122,9 @@ fn check_memory_init_bounds( unsafe { let mem_slice = get_memory_slice(init, instance); if mem_slice.get_mut(start..start + init.data.len()).is_none() { - return Err(InstantiationError::MemoryOutOfBounds( - "data segment does not fit".into(), - )); + return Err(InstantiationError::Link(LinkError( + "memory out of bounds: data segment does not fit".into(), + ))); } } } @@ -1190,9 +1172,10 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { // bounds, then we report an error. This is required by the bulk memory // proposal and the way that it uses `table.init` during instantiation. if start > table.size() as usize { - return Err(InstantiationError::TableOutOfBounds( - "active element segment does not fit in table".into(), - )); + return Err(InstantiationError::Trap(Trap::wasm( + ir::SourceLoc::default(), + ir::TrapCode::HeapOutOfBounds, + ))); } for (i, func_idx) in init.elements.iter().enumerate() { @@ -1205,9 +1188,10 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { // enabled, these become runtime traps and the intermediate // table slot writes are visible. .map_err(|()| { - InstantiationError::TableOutOfBounds( - "active element segment does not fit in table".into(), - ) + InstantiationError::Trap(Trap::wasm( + ir::SourceLoc::default(), + ir::TrapCode::HeapOutOfBounds, + )) })?; } } @@ -1230,6 +1214,7 @@ fn initialize_passive_elements(instance: &Instance) { .module .passive_elements .iter() + .filter(|(_, segments)| !segments.is_empty()) .map(|(idx, segments)| { ( *idx, @@ -1279,9 +1264,10 @@ fn initialize_memories( // since this is dictated by the updated spec for the bulk memory // proposal. if num_uncopied > 0 { - return Err(InstantiationError::MemoryOutOfBounds( - "active data segment does not fit in memory".into(), - )); + return Err(InstantiationError::Trap(Trap::wasm( + ir::SourceLoc::default(), + ir::TrapCode::HeapOutOfBounds, + ))); } } } @@ -1342,24 +1328,14 @@ pub enum InstantiationError { #[error("Insufficient resources: {0}")] Resource(String), - /// A table out of bounds error. - /// - /// Depending on whether bulk memory is enabled or not, this is either a - /// link error or a trap raised during instantiation. - #[error("Table out of bounds error: {0}")] - TableOutOfBounds(String), - - /// A memory out of bounds error. - /// - /// Depending on whether bulk memory is enabled or not, this is either a - /// link error or a trap raised during instantiation. - #[error("Memory out of bounds error: {0}")] - MemoryOutOfBounds(String), - /// A wasm link error occured. #[error("Failed to link module")] Link(#[from] LinkError), + /// A trap ocurred during instantiation, after linking. + #[error("Trap occurred during instantiation")] + Trap(#[source] Trap), + /// A compilation error occured. #[error("Trap occurred while invoking start function")] StartTrap(#[source] Trap), diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 65d6331aa49c..e157736da0c9 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -1,14 +1,42 @@ -//! Runtime library calls. Note that wasm compilers may sometimes perform these -//! inline rather than calling them, particularly when CPUs have special -//! instructions which compute them directly. +//! Runtime library calls. +//! +//! Note that Wasm compilers may sometimes perform these inline rather than +//! calling them, particularly when CPUs have special instructions which compute +//! them directly. +//! +//! These functions are called by compiled Wasm code, and therefore must take +//! certain care about some things: +//! +//! * They must always be `pub extern "C"` and should only contain basic, raw +//! i32/i64/f32/f64/pointer parameters that are safe to pass across the system +//! ABI! +//! +//! * If any nested function propagates an `Err(trap)` out to the library +//! function frame, we need to raise it. This involves some nasty and quite +//! unsafe code under the covers! Notable, after raising the trap, drops +//! **will not** be run for local variables! This can lead to things like +//! leaking `InstanceHandle`s which leads to never deallocating JIT code, +//! instances, and modules! Therefore, always use nested blocks to ensure +//! drops run before raising a trap: +//! +//! ``` +//! pub extern "C" fn my_lib_function(...) { +//! let result = { +//! // Do everything in here so drops run at the end of the block. +//! ... +//! }; +//! if let Err(trap) = result { +//! // Now we can safely raise the trap without leaking! +//! raise_lib_trap(trap); +//! } +//! } +//! ``` use crate::table::Table; use crate::traphandlers::raise_lib_trap; use crate::vmcontext::VMContext; use wasmtime_environ::ir; -use wasmtime_environ::wasm::{ - DefinedMemoryIndex, DefinedTableIndex, MemoryIndex, PassiveElemIndex, TableIndex, -}; +use wasmtime_environ::wasm::{DefinedMemoryIndex, MemoryIndex, PassiveElemIndex, TableIndex}; /// Implementation of f32.ceil pub extern "C" fn wasmtime_f32_ceil(x: f32) -> f32 { @@ -93,7 +121,6 @@ pub extern "C" fn wasmtime_f64_nearest(x: f64) -> f64 { } /// Implementation of memory.grow for locally-defined 32-bit memories. -#[no_mangle] pub unsafe extern "C" fn wasmtime_memory32_grow( vmctx: *mut VMContext, delta: u32, @@ -108,7 +135,6 @@ pub unsafe extern "C" fn wasmtime_memory32_grow( } /// Implementation of memory.grow for imported 32-bit memories. -#[no_mangle] pub unsafe extern "C" fn wasmtime_imported_memory32_grow( vmctx: *mut VMContext, delta: u32, @@ -123,7 +149,6 @@ pub unsafe extern "C" fn wasmtime_imported_memory32_grow( } /// Implementation of memory.size for locally-defined 32-bit memories. -#[no_mangle] pub unsafe extern "C" fn wasmtime_memory32_size(vmctx: *mut VMContext, memory_index: u32) -> u32 { let instance = (&mut *vmctx).instance(); let memory_index = DefinedMemoryIndex::from_u32(memory_index); @@ -132,7 +157,6 @@ pub unsafe extern "C" fn wasmtime_memory32_size(vmctx: *mut VMContext, memory_in } /// Implementation of memory.size for imported 32-bit memories. -#[no_mangle] pub unsafe extern "C" fn wasmtime_imported_memory32_size( vmctx: *mut VMContext, memory_index: u32, @@ -143,9 +167,8 @@ pub unsafe extern "C" fn wasmtime_imported_memory32_size( instance.imported_memory_size(memory_index) } -/// Implementation of `table.copy` when both tables are locally defined. -#[no_mangle] -pub unsafe extern "C" fn wasmtime_table_copy_defined_defined( +/// Implementation of `table.copy`. +pub unsafe extern "C" fn wasmtime_table_copy( vmctx: *mut VMContext, dst_table_index: u32, src_table_index: u32, @@ -154,87 +177,21 @@ pub unsafe extern "C" fn wasmtime_table_copy_defined_defined( len: u32, source_loc: u32, ) { - let dst_table_index = DefinedTableIndex::from_u32(dst_table_index); - let src_table_index = DefinedTableIndex::from_u32(src_table_index); - let source_loc = ir::SourceLoc::new(source_loc); - let instance = (&mut *vmctx).instance(); - let dst_table = instance.get_defined_table(dst_table_index); - let src_table = instance.get_defined_table(src_table_index); - if let Err(trap) = Table::copy(dst_table, src_table, dst, src, len, source_loc) { - raise_lib_trap(trap); - } -} - -/// Implementation of `table.copy` when the destination table is locally defined -/// and the source table is imported. -#[no_mangle] -pub unsafe extern "C" fn wasmtime_table_copy_defined_imported( - vmctx: *mut VMContext, - dst_table_index: u32, - src_table_index: u32, - dst: u32, - src: u32, - len: u32, - source_loc: u32, -) { - let dst_table_index = DefinedTableIndex::from_u32(dst_table_index); - let src_table_index = TableIndex::from_u32(src_table_index); - let source_loc = ir::SourceLoc::new(source_loc); - let instance = (&mut *vmctx).instance(); - let dst_table = instance.get_defined_table(dst_table_index); - let src_table = instance.get_foreign_table(src_table_index); - if let Err(trap) = Table::copy(dst_table, src_table, dst, src, len, source_loc) { - raise_lib_trap(trap); - } -} - -/// Implementation of `table.copy` when the destination table is imported -/// and the source table is locally defined. -#[no_mangle] -pub unsafe extern "C" fn wasmtime_table_copy_imported_defined( - vmctx: *mut VMContext, - dst_table_index: u32, - src_table_index: u32, - dst: u32, - src: u32, - len: u32, - source_loc: u32, -) { - let dst_table_index = TableIndex::from_u32(dst_table_index); - let src_table_index = DefinedTableIndex::from_u32(src_table_index); - let source_loc = ir::SourceLoc::new(source_loc); - let instance = (&mut *vmctx).instance(); - let dst_table = instance.get_foreign_table(dst_table_index); - let src_table = instance.get_defined_table(src_table_index); - if let Err(trap) = Table::copy(dst_table, src_table, dst, src, len, source_loc) { - raise_lib_trap(trap); - } -} - -/// Implementation of `table.copy` when both tables are imported. -#[no_mangle] -pub unsafe extern "C" fn wasmtime_table_copy_imported_imported( - vmctx: *mut VMContext, - dst_table_index: u32, - src_table_index: u32, - dst: u32, - src: u32, - len: u32, - source_loc: u32, -) { - let dst_table_index = TableIndex::from_u32(dst_table_index); - let src_table_index = TableIndex::from_u32(src_table_index); - let source_loc = ir::SourceLoc::new(source_loc); - let instance = (&mut *vmctx).instance(); - let dst_table = instance.get_foreign_table(dst_table_index); - let src_table = instance.get_foreign_table(src_table_index); - if let Err(trap) = Table::copy(dst_table, src_table, dst, src, len, source_loc) { + let result = { + let dst_table_index = TableIndex::from_u32(dst_table_index); + let src_table_index = TableIndex::from_u32(src_table_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + let dst_table = instance.get_table(dst_table_index); + let src_table = instance.get_table(src_table_index); + Table::copy(dst_table, src_table, dst, src, len, source_loc) + }; + if let Err(trap) = result { raise_lib_trap(trap); } } /// Implementation of `table.init`. -#[no_mangle] pub unsafe extern "C" fn wasmtime_table_init( vmctx: *mut VMContext, table_index: u32, @@ -244,19 +201,19 @@ pub unsafe extern "C" fn wasmtime_table_init( len: u32, source_loc: u32, ) { - let table_index = TableIndex::from_u32(table_index); - let source_loc = ir::SourceLoc::new(source_loc); - let elem_index = PassiveElemIndex::from_u32(elem_index); - - let instance = (&mut *vmctx).instance(); - - if let Err(trap) = instance.table_init(table_index, elem_index, dst, src, len, source_loc) { + let result = { + let table_index = TableIndex::from_u32(table_index); + let source_loc = ir::SourceLoc::new(source_loc); + let elem_index = PassiveElemIndex::from_u32(elem_index); + let instance = (&mut *vmctx).instance(); + instance.table_init(table_index, elem_index, dst, src, len, source_loc) + }; + if let Err(trap) = result { raise_lib_trap(trap); } } /// Implementation of `elem.drop`. -#[no_mangle] pub unsafe extern "C" fn wasmtime_elem_drop(vmctx: *mut VMContext, elem_index: u32) { let elem_index = PassiveElemIndex::from_u32(elem_index); let instance = (&mut *vmctx).instance(); @@ -264,8 +221,7 @@ pub unsafe extern "C" fn wasmtime_elem_drop(vmctx: *mut VMContext, elem_index: u } /// Implementation of `memory.copy` for locally defined memories. -#[no_mangle] -pub unsafe extern "C" fn wasmtime_memory_copy( +pub unsafe extern "C" fn wasmtime_defined_memory_copy( vmctx: *mut VMContext, memory_index: u32, dst: u32, @@ -273,16 +229,18 @@ pub unsafe extern "C" fn wasmtime_memory_copy( len: u32, source_loc: u32, ) { - let memory_index = DefinedMemoryIndex::from_u32(memory_index); - let source_loc = ir::SourceLoc::new(source_loc); - let instance = (&mut *vmctx).instance(); - if let Err(trap) = instance.defined_memory_copy(memory_index, dst, src, len, source_loc) { + let result = { + let memory_index = DefinedMemoryIndex::from_u32(memory_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + instance.defined_memory_copy(memory_index, dst, src, len, source_loc) + }; + if let Err(trap) = result { raise_lib_trap(trap); } } /// Implementation of `memory.copy` for imported memories. -#[no_mangle] pub unsafe extern "C" fn wasmtime_imported_memory_copy( vmctx: *mut VMContext, memory_index: u32, @@ -291,16 +249,18 @@ pub unsafe extern "C" fn wasmtime_imported_memory_copy( len: u32, source_loc: u32, ) { - let memory_index = MemoryIndex::from_u32(memory_index); - let source_loc = ir::SourceLoc::new(source_loc); - let instance = (&mut *vmctx).instance(); - if let Err(trap) = instance.imported_memory_copy(memory_index, dst, src, len, source_loc) { + let result = { + let memory_index = MemoryIndex::from_u32(memory_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + instance.imported_memory_copy(memory_index, dst, src, len, source_loc) + }; + if let Err(trap) = result { raise_lib_trap(trap); } } /// Implementation of `memory.fill` for locally defined memories. -#[no_mangle] pub unsafe extern "C" fn wasmtime_memory_fill( vmctx: *mut VMContext, memory_index: u32, @@ -309,16 +269,18 @@ pub unsafe extern "C" fn wasmtime_memory_fill( len: u32, source_loc: u32, ) { - let memory_index = DefinedMemoryIndex::from_u32(memory_index); - let source_loc = ir::SourceLoc::new(source_loc); - let instance = (&mut *vmctx).instance(); - if let Err(trap) = instance.defined_memory_fill(memory_index, dst, val, len, source_loc) { + let result = { + let memory_index = DefinedMemoryIndex::from_u32(memory_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + instance.defined_memory_fill(memory_index, dst, val, len, source_loc) + }; + if let Err(trap) = result { raise_lib_trap(trap); } } /// Implementation of `memory.fill` for imported memories. -#[no_mangle] pub unsafe extern "C" fn wasmtime_imported_memory_fill( vmctx: *mut VMContext, memory_index: u32, @@ -327,10 +289,13 @@ pub unsafe extern "C" fn wasmtime_imported_memory_fill( len: u32, source_loc: u32, ) { - let memory_index = MemoryIndex::from_u32(memory_index); - let source_loc = ir::SourceLoc::new(source_loc); - let instance = (&mut *vmctx).instance(); - if let Err(trap) = instance.imported_memory_fill(memory_index, dst, val, len, source_loc) { + let result = { + let memory_index = MemoryIndex::from_u32(memory_index); + let source_loc = ir::SourceLoc::new(source_loc); + let instance = (&mut *vmctx).instance(); + instance.imported_memory_fill(memory_index, dst, val, len, source_loc) + }; + if let Err(trap) = result { raise_lib_trap(trap); } } diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index 45672a75fa7f..e938b5dd2983 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -3,8 +3,7 @@ //! `Table` is to WebAssembly tables what `LinearMemory` is to WebAssembly linear memories. use crate::vmcontext::{VMCallerCheckedAnyfunc, VMTableDefinition}; -use crate::{Trap, TrapDescription}; -use backtrace::Backtrace; +use crate::Trap; use std::cell::RefCell; use std::convert::{TryFrom, TryInto}; use wasmtime_environ::wasm::TableElementType; @@ -112,13 +111,7 @@ impl Table { .checked_add(len) .map_or(true, |m| m > dst_table.size()) { - return Err(Trap::Wasm { - desc: TrapDescription { - source_loc, - trap_code: ir::TrapCode::TableOutOfBounds, - }, - backtrace: Backtrace::new(), - }); + return Err(Trap::wasm(source_loc, ir::TrapCode::TableOutOfBounds)); } let srcs = src_index..src_index + len; @@ -126,6 +119,8 @@ impl Table { // Note on the unwraps: the bounds check above means that these will // never panic. + // + // TODO(#983): investigate replacing this get/set loop with a `memcpy`. if dst_index <= src_index { for (s, d) in (srcs).zip(dsts) { dst_table.set(d, src_table.get(s).unwrap()).unwrap(); diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs index 1978c5cc8801..7182a6506d66 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -140,6 +140,20 @@ impl fmt::Display for Trap { impl std::error::Error for Trap {} +impl Trap { + /// Construct a new Wasm trap with the given source location and trap code. + /// + /// Internally saves a backtrace when constructed. + pub fn wasm(source_loc: ir::SourceLoc, trap_code: ir::TrapCode) -> Self { + let desc = TrapDescription { + source_loc, + trap_code, + }; + let backtrace = Backtrace::new(); + Trap::Wasm { desc, backtrace } + } +} + /// Call the wasm function pointed to by `callee`. /// /// * `vmctx` - the callee vmctx argument diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index 6cb640f0e203..dfa05b3566ce 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -549,22 +549,16 @@ impl VMBuiltinFunctionsArray { ptrs[BuiltinFunctionIndex::get_imported_memory32_size_index().index() as usize] = wasmtime_imported_memory32_size as usize; - ptrs[BuiltinFunctionIndex::get_table_copy_defined_defined_index().index() as usize] = - wasmtime_table_copy_defined_defined as usize; - ptrs[BuiltinFunctionIndex::get_table_copy_defined_imported_index().index() as usize] = - wasmtime_table_copy_defined_imported as usize; - ptrs[BuiltinFunctionIndex::get_table_copy_imported_defined_index().index() as usize] = - wasmtime_table_copy_imported_defined as usize; - ptrs[BuiltinFunctionIndex::get_table_copy_imported_imported_index().index() as usize] = - wasmtime_table_copy_imported_imported as usize; + ptrs[BuiltinFunctionIndex::get_table_copy_index().index() as usize] = + wasmtime_table_copy as usize; ptrs[BuiltinFunctionIndex::get_table_init_index().index() as usize] = wasmtime_table_init as usize; ptrs[BuiltinFunctionIndex::get_elem_drop_index().index() as usize] = wasmtime_elem_drop as usize; - ptrs[BuiltinFunctionIndex::get_memory_copy_index().index() as usize] = - wasmtime_memory_copy as usize; + ptrs[BuiltinFunctionIndex::get_defined_memory_copy_index().index() as usize] = + wasmtime_defined_memory_copy as usize; ptrs[BuiltinFunctionIndex::get_imported_memory_copy_index().index() as usize] = wasmtime_imported_memory_copy as usize; ptrs[BuiltinFunctionIndex::get_memory_fill_index().index() as usize] = diff --git a/tests/misc_testsuite/elem-ref-null.wast b/tests/misc_testsuite/bulk-memory-operations/elem-ref-null.wast similarity index 100% rename from tests/misc_testsuite/elem-ref-null.wast rename to tests/misc_testsuite/bulk-memory-operations/elem-ref-null.wast diff --git a/tests/misc_testsuite/elem_drop.wast b/tests/misc_testsuite/bulk-memory-operations/elem_drop.wast similarity index 100% rename from tests/misc_testsuite/elem_drop.wast rename to tests/misc_testsuite/bulk-memory-operations/elem_drop.wast diff --git a/tests/misc_testsuite/imported-memory-copy.wast b/tests/misc_testsuite/bulk-memory-operations/imported-memory-copy.wast similarity index 100% rename from tests/misc_testsuite/imported-memory-copy.wast rename to tests/misc_testsuite/bulk-memory-operations/imported-memory-copy.wast diff --git a/tests/misc_testsuite/memory-copy.wast b/tests/misc_testsuite/bulk-memory-operations/memory-copy.wast similarity index 100% rename from tests/misc_testsuite/memory-copy.wast rename to tests/misc_testsuite/bulk-memory-operations/memory-copy.wast diff --git a/tests/misc_testsuite/table_copy.wast b/tests/misc_testsuite/bulk-memory-operations/table_copy.wast similarity index 100% rename from tests/misc_testsuite/table_copy.wast rename to tests/misc_testsuite/bulk-memory-operations/table_copy.wast diff --git a/tests/misc_testsuite/table_copy_on_imported_tables.wast b/tests/misc_testsuite/reference-types/table_copy_on_imported_tables.wast similarity index 100% rename from tests/misc_testsuite/table_copy_on_imported_tables.wast rename to tests/misc_testsuite/reference-types/table_copy_on_imported_tables.wast diff --git a/tests/wast_testsuites.rs b/tests/wast_testsuites.rs index 5a4ebedfdf7b..9ff7107205c2 100644 --- a/tests/wast_testsuites.rs +++ b/tests/wast_testsuites.rs @@ -12,33 +12,19 @@ fn run_wast(wast: &str, strategy: Strategy) -> anyhow::Result<()> { let simd = wast.iter().any(|s| s == "simd"); + let bulk_mem = wast.iter().any(|s| s == "bulk-memory-operations"); + // Some simd tests assume support for multiple tables, which are introduced // by reference types. let reftypes = simd || wast.iter().any(|s| s == "reference-types"); - // Reference types assumes support for bulk memory. - let bulk_mem = reftypes - || wast.iter().any(|s| s == "bulk-memory-operations") - || wast.iter().any(|s| s == "table_copy.wast") - || wast.iter().any(|s| s == "elem_drop.wast") - || wast.iter().any(|s| s == "elem-ref-null.wast") - || wast.iter().any(|s| s == "memory-copy.wast") - || wast.iter().any(|s| s == "imported-memory-copy.wast") - || wast - .iter() - .any(|s| s == "table_copy_on_imported_tables.wast"); - - // And bulk memory also assumes support for reference types (e.g. multiple - // tables). - let reftypes = reftypes || bulk_mem; - let multi_val = wast.iter().any(|s| s == "multi-value"); let mut cfg = Config::new(); cfg.wasm_simd(simd) + .wasm_bulk_memory(bulk_mem) .wasm_reference_types(reftypes) .wasm_multi_value(multi_val) - .wasm_bulk_memory(bulk_mem) .strategy(strategy)? .cranelift_debug_verifier(true); From 235833ab97f2d5dce1b0289f780a5d2fa1e1faf3 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 25 Feb 2020 14:09:23 -0800 Subject: [PATCH 10/11] Ignore a doc test --- crates/runtime/src/libcalls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index e157736da0c9..62840072e7bb 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -19,7 +19,7 @@ //! instances, and modules! Therefore, always use nested blocks to ensure //! drops run before raising a trap: //! -//! ``` +//! ```ignore //! pub extern "C" fn my_lib_function(...) { //! let result = { //! // Do everything in here so drops run at the end of the block. From 66634cc796eafd4d9d21e7e247cfcdee6a400756 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 26 Feb 2020 13:10:52 -0800 Subject: [PATCH 11/11] Do not allow partial segment initialization for tables and memories --- crates/runtime/src/instance.rs | 66 +++++++++---------- .../partial-init-memory-segment.wast | 35 ++++++++++ .../partial-init-table-segment.wast | 35 ++++++++++ 3 files changed, 100 insertions(+), 36 deletions(-) create mode 100644 tests/misc_testsuite/bulk-memory-operations/partial-init-memory-segment.wast create mode 100644 tests/misc_testsuite/bulk-memory-operations/partial-init-table-segment.wast diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index fe549f20e878..94994929af25 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -20,7 +20,6 @@ use more_asserts::assert_lt; use std::alloc::{self, Layout}; use std::any::Any; use std::cell::{Cell, RefCell}; -use std::cmp; use std::collections::HashMap; use std::collections::HashSet; use std::convert::TryFrom; @@ -212,6 +211,16 @@ impl Instance { unsafe { self.vmctx_plus_offset(self.offsets.vmctx_tables_begin()) } } + /// Get a locally defined or imported memory. + pub(crate) fn get_memory(&self, index: MemoryIndex) -> VMMemoryDefinition { + if let Some(defined_index) = self.module.local.defined_memory_index(index) { + self.memory(defined_index) + } else { + let import = self.imported_memory(index); + *unsafe { import.from.as_ref().unwrap() } + } + } + /// Return the indexed `VMMemoryDefinition`. fn memory(&self, index: DefinedMemoryIndex) -> VMMemoryDefinition { unsafe { *self.memory_ptr(index) } @@ -1168,10 +1177,10 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { let start = get_table_init_start(init, instance); let table = instance.get_table(init.table_index); - // Even if there aren't any elements being initialized, if its out of - // bounds, then we report an error. This is required by the bulk memory - // proposal and the way that it uses `table.init` during instantiation. - if start > table.size() as usize { + if start + .checked_add(init.elements.len()) + .map_or(true, |end| end > table.size() as usize) + { return Err(InstantiationError::Trap(Trap::wasm( ir::SourceLoc::default(), ir::TrapCode::HeapOutOfBounds, @@ -1182,17 +1191,7 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> { let anyfunc = instance.get_caller_checked_anyfunc(*func_idx); table .set(u32::try_from(start + i).unwrap(), anyfunc) - // Note that when bulk memory is disabled, this will never fail - // since we bounds check table element initialization before - // doing any table slot writes. However when bulk memory is - // enabled, these become runtime traps and the intermediate - // table slot writes are visible. - .map_err(|()| { - InstantiationError::Trap(Trap::wasm( - ir::SourceLoc::default(), - ir::TrapCode::HeapOutOfBounds, - )) - })?; + .unwrap(); } } @@ -1246,29 +1245,24 @@ fn initialize_memories( data_initializers: &[DataInitializer<'_>], ) -> Result<(), InstantiationError> { for init in data_initializers { + let memory = instance.get_memory(init.location.memory_index); + let start = get_memory_init_start(init, instance); + if start + .checked_add(init.data.len()) + .map_or(true, |end| end > memory.current_length) + { + return Err(InstantiationError::Trap(Trap::wasm( + ir::SourceLoc::default(), + ir::TrapCode::HeapOutOfBounds, + ))); + } + unsafe { let mem_slice = get_memory_slice(init, instance); - - let end = start.saturating_add(init.data.len()); - let actual_end = cmp::min(mem_slice.len(), end); - let num_uncopied = end - actual_end; - - let to_init = &mut mem_slice[start..actual_end]; - to_init.copy_from_slice(&init.data[..init.data.len() - num_uncopied]); - - // Note that if bulk memory is disabled, we'll never hit this case - // because we did an eager bounds check in - // `check_memory_init_bounds`. However when bulk memory is enabled, - // we need the incremental writes up to the OOB trap to be visible, - // since this is dictated by the updated spec for the bulk memory - // proposal. - if num_uncopied > 0 { - return Err(InstantiationError::Trap(Trap::wasm( - ir::SourceLoc::default(), - ir::TrapCode::HeapOutOfBounds, - ))); - } + let end = start + init.data.len(); + let to_init = &mut mem_slice[start..end]; + to_init.copy_from_slice(init.data); } } diff --git a/tests/misc_testsuite/bulk-memory-operations/partial-init-memory-segment.wast b/tests/misc_testsuite/bulk-memory-operations/partial-init-memory-segment.wast new file mode 100644 index 000000000000..471e74a643d1 --- /dev/null +++ b/tests/misc_testsuite/bulk-memory-operations/partial-init-memory-segment.wast @@ -0,0 +1,35 @@ +(module $m + (memory (export "mem") 1) + + (func (export "load") (param i32) (result i32) + local.get 0 + i32.load8_u)) + +(register "m" $m) + +(assert_trap + (module + (memory (import "m" "mem") 1) + + ;; This is in bounds, and should get written to the memory. + (data (i32.const 0) "abc") + + ;; Partially out of bounds. None of these bytes should get written, and + ;; instantiation should trap. + (data (i32.const 65530) "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz") + ) + "out of bounds" +) + +;; The first data segment got written. +(assert_return (invoke $m "load" (i32.const 0)) (i32.const 97)) +(assert_return (invoke $m "load" (i32.const 1)) (i32.const 98)) +(assert_return (invoke $m "load" (i32.const 2)) (i32.const 99)) + +;; The second did not get partially written. +(assert_return (invoke $m "load" (i32.const 65530)) (i32.const 0)) +(assert_return (invoke $m "load" (i32.const 65531)) (i32.const 0)) +(assert_return (invoke $m "load" (i32.const 65532)) (i32.const 0)) +(assert_return (invoke $m "load" (i32.const 65533)) (i32.const 0)) +(assert_return (invoke $m "load" (i32.const 65534)) (i32.const 0)) +(assert_return (invoke $m "load" (i32.const 65535)) (i32.const 0)) diff --git a/tests/misc_testsuite/bulk-memory-operations/partial-init-table-segment.wast b/tests/misc_testsuite/bulk-memory-operations/partial-init-table-segment.wast new file mode 100644 index 000000000000..0086d4b54227 --- /dev/null +++ b/tests/misc_testsuite/bulk-memory-operations/partial-init-table-segment.wast @@ -0,0 +1,35 @@ +(module $m + (table (export "table") funcref (elem $zero $zero $zero $zero $zero $zero $zero $zero $zero $zero)) + + (func $zero (result i32) + (i32.const 0)) + + (func (export "indirect-call") (param i32) (result i32) + local.get 0 + call_indirect (result i32))) + +(register "m" $m) + +(assert_trap + (module + (table (import "m" "table") 10 funcref) + + (func $one (result i32) + (i32.const 1)) + + ;; An in-bounds segment that should get initialized in the table. + (elem (i32.const 7) $one) + + ;; Part of this segment is out of bounds, so none of its elements should be + ;; initialized into the table, and it should trap. + (elem (i32.const 9) $one $one $one) + ) + "out of bounds" +) + +;; The first `$one` segment *was* initialized OK. +(assert_return (invoke "indirect-call" (i32.const 7)) (i32.const 1)) + +;; The second `$one` segment is partially out of bounds, and therefore none of +;; its elements were written into the table. +(assert_return (invoke "indirect-call" (i32.const 9)) (i32.const 0))