Implement data.drop and memory.init and get the rest of the bulk memory spec tests passing (#1264)

* Enable the already-passing `bulk-memoryoperations/imports.wast` test

* Implement support for the `memory.init` instruction and passive data

This adds support for passive data segments and the `memory.init` instruction
from the bulk memory operations proposal. Passive data segments are stored on
the Wasm module and then `memory.init` instructions copy their contents into
memory.

* Implement the `data.drop` instruction

This allows wasm modules to deallocate passive data segments that it doesn't
need anymore. We keep track of which segments have not been dropped on an
`Instance` and when dropping them, remove the entry from the instance's hash
map. The module always needs all of the segments for new instantiations.

* Enable final bulk memory operations spec test

This requires special casing an expected error message for an `assert_trap`,
since the expected error message contains the index of an uninitialized table
element, but our trap implementation doesn't save that diagnostic information
and shepherd it out.
This commit is contained in:
Nick Fitzgerald
2020-03-10 07:30:11 -07:00
committed by GitHub
parent 11510ec426
commit 674a6208d8
10 changed files with 237 additions and 42 deletions

View File

@@ -196,12 +196,6 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool {
("reference_types", "table_copy_on_imported_tables") => return false, ("reference_types", "table_copy_on_imported_tables") => return false,
("reference_types", _) => return true, ("reference_types", _) => 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,
_ => {} _ => {}
}, },
_ => panic!("unrecognized strategy"), _ => panic!("unrecognized strategy"),

View File

@@ -22,7 +22,7 @@ pub mod entity {
pub mod wasm { pub mod wasm {
pub use cranelift_wasm::{ pub use cranelift_wasm::{
get_vmctx_value_label, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, get_vmctx_value_label, DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex,
DefinedTableIndex, ElemIndex, FuncIndex, Global, GlobalIndex, GlobalInit, Memory, DefinedTableIndex, ElemIndex, FuncIndex, Global, GlobalIndex, GlobalInit, Memory,
MemoryIndex, SignatureIndex, Table, TableElementType, TableIndex, MemoryIndex, SignatureIndex, Table, TableElementType, TableIndex,
}; };

View File

@@ -72,9 +72,17 @@ impl BuiltinFunctionIndex {
pub const fn get_imported_memory_fill_index() -> Self { pub const fn get_imported_memory_fill_index() -> Self {
Self(10) Self(10)
} }
/// Returns an index for wasm's `memory.init` instruction.
pub const fn get_memory_init_index() -> Self {
Self(11)
}
/// Returns an index for wasm's `data.drop` instruction.
pub const fn get_data_drop_index() -> Self {
Self(12)
}
/// Returns the total number of builtin functions. /// Returns the total number of builtin functions.
pub const fn builtin_functions_total_number() -> u32 { pub const fn builtin_functions_total_number() -> u32 {
11 13
} }
/// Return the index as an u32 number. /// Return the index as an u32 number.
@@ -120,6 +128,12 @@ pub struct FuncEnvironment<'module_environment> {
/// (it's the same for both local and imported memories). /// (it's the same for both local and imported memories).
memory_fill_sig: Option<ir::SigRef>, memory_fill_sig: Option<ir::SigRef>,
/// The external function signature for implementing wasm's `memory.init`.
memory_init_sig: Option<ir::SigRef>,
/// The external function signature for implementing wasm's `data.drop`.
data_drop_sig: Option<ir::SigRef>,
/// Offsets to struct fields accessed by JIT code. /// Offsets to struct fields accessed by JIT code.
offsets: VMOffsets, offsets: VMOffsets,
} }
@@ -140,6 +154,8 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
elem_drop_sig: None, elem_drop_sig: None,
memory_copy_sig: None, memory_copy_sig: None,
memory_fill_sig: None, memory_fill_sig: None,
memory_init_sig: None,
data_drop_sig: None,
offsets: VMOffsets::new(target_config.pointer_bytes(), module), offsets: VMOffsets::new(target_config.pointer_bytes(), module),
} }
} }
@@ -423,6 +439,58 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
} }
} }
fn get_memory_init_sig(&mut self, func: &mut Function) -> ir::SigRef {
let sig = self.memory_init_sig.unwrap_or_else(|| {
func.import_signature(Signature {
params: vec![
AbiParam::special(self.pointer_type(), ArgumentPurpose::VMContext),
// Memory index.
AbiParam::new(I32),
// Data index.
AbiParam::new(I32),
// Destination address.
AbiParam::new(I32),
// Source index within the data segment.
AbiParam::new(I32),
// Length.
AbiParam::new(I32),
// Source location.
AbiParam::new(I32),
],
returns: vec![],
call_conv: self.target_config.default_call_conv,
})
});
self.memory_init_sig = Some(sig);
sig
}
fn get_memory_init_func(&mut self, func: &mut Function) -> (ir::SigRef, BuiltinFunctionIndex) {
let sig = self.get_memory_init_sig(func);
(sig, BuiltinFunctionIndex::get_memory_init_index())
}
fn get_data_drop_sig(&mut self, func: &mut Function) -> ir::SigRef {
let sig = self.data_drop_sig.unwrap_or_else(|| {
func.import_signature(Signature {
params: vec![
AbiParam::special(self.pointer_type(), ArgumentPurpose::VMContext),
// Data index.
AbiParam::new(I32),
],
returns: vec![],
call_conv: self.target_config.default_call_conv,
})
});
self.data_drop_sig = Some(sig);
sig
}
fn get_data_drop_func(&mut self, func: &mut Function) -> (ir::SigRef, BuiltinFunctionIndex) {
let sig = self.get_data_drop_sig(func);
(sig, BuiltinFunctionIndex::get_data_drop_index())
}
/// Translates load of builtin function and returns a pair of values `vmctx` /// Translates load of builtin function and returns a pair of values `vmctx`
/// and address of the loaded function. /// and address of the loaded function.
fn translate_load_builtin_function_address( fn translate_load_builtin_function_address(
@@ -1076,23 +1144,47 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
fn translate_memory_init( fn translate_memory_init(
&mut self, &mut self,
_pos: FuncCursor, mut pos: FuncCursor,
_index: MemoryIndex, memory_index: MemoryIndex,
_heap: ir::Heap, _heap: ir::Heap,
_seg_index: u32, seg_index: u32,
_dst: ir::Value, dst: ir::Value,
_src: ir::Value, src: ir::Value,
_len: ir::Value, len: ir::Value,
) -> WasmResult<()> { ) -> WasmResult<()> {
Err(WasmError::Unsupported( let (func_sig, func_idx) = self.get_memory_init_func(&mut pos.func);
"bulk memory: `memory.init`".to_string(),
)) let memory_index_arg = pos.ins().iconst(I32, memory_index.index() 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,
memory_index_arg,
seg_index_arg,
dst,
src,
len,
src_loc_arg,
],
);
Ok(())
} }
fn translate_data_drop(&mut self, _pos: FuncCursor, _seg_index: u32) -> WasmResult<()> { fn translate_data_drop(&mut self, mut pos: FuncCursor, seg_index: u32) -> WasmResult<()> {
Err(WasmError::Unsupported( let (func_sig, func_idx) = self.get_data_drop_func(&mut pos.func);
"bulk memory: `data.drop`".to_string(), let seg_index_arg = pos.ins().iconst(I32, seg_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, seg_index_arg]);
Ok(())
} }
fn translate_table_size( fn translate_table_size(

View File

@@ -5,13 +5,17 @@ use crate::WASM_MAX_PAGES;
use cranelift_codegen::ir; use cranelift_codegen::ir;
use cranelift_entity::{EntityRef, PrimaryMap}; use cranelift_entity::{EntityRef, PrimaryMap};
use cranelift_wasm::{ use cranelift_wasm::{
DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, ElemIndex, DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex,
FuncIndex, Global, GlobalIndex, Memory, MemoryIndex, SignatureIndex, Table, TableIndex, ElemIndex, FuncIndex, Global, GlobalIndex, Memory, MemoryIndex, SignatureIndex, Table,
TableIndex,
}; };
use indexmap::IndexMap; use indexmap::IndexMap;
use more_asserts::assert_ge; use more_asserts::assert_ge;
use std::collections::HashMap; use std::collections::HashMap;
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use std::sync::{
atomic::{AtomicUsize, Ordering::SeqCst},
Arc,
};
/// A WebAssembly table initializer. /// A WebAssembly table initializer.
#[derive(Clone, Debug, Hash)] #[derive(Clone, Debug, Hash)]
@@ -168,6 +172,9 @@ pub struct Module {
/// WebAssembly passive elements. /// WebAssembly passive elements.
pub passive_elements: HashMap<ElemIndex, Box<[FuncIndex]>>, pub passive_elements: HashMap<ElemIndex, Box<[FuncIndex]>>,
/// WebAssembly passive data segments.
pub passive_data: HashMap<DataIndex, Arc<[u8]>>,
/// WebAssembly table initializers. /// WebAssembly table initializers.
pub func_names: HashMap<FuncIndex, String>, pub func_names: HashMap<FuncIndex, String>,
} }
@@ -223,6 +230,7 @@ impl Module {
start_func: None, start_func: None,
table_elements: Vec::new(), table_elements: Vec::new(),
passive_elements: HashMap::new(), passive_elements: HashMap::new(),
passive_data: HashMap::new(),
func_names: HashMap::new(), func_names: HashMap::new(),
local: ModuleLocal { local: ModuleLocal {
num_imported_funcs: 0, num_imported_funcs: 0,

View File

@@ -8,9 +8,10 @@ use cranelift_entity::PrimaryMap;
use cranelift_wasm::{ use cranelift_wasm::{
self, translate_module, DataIndex, DefinedFuncIndex, ElemIndex, FuncIndex, Global, GlobalIndex, self, translate_module, DataIndex, DefinedFuncIndex, ElemIndex, FuncIndex, Global, GlobalIndex,
Memory, MemoryIndex, ModuleTranslationState, SignatureIndex, Table, TableIndex, Memory, MemoryIndex, ModuleTranslationState, SignatureIndex, Table, TableIndex,
TargetEnvironment, WasmError, WasmResult, TargetEnvironment, WasmResult,
}; };
use std::convert::TryFrom; use std::convert::TryFrom;
use std::sync::Arc;
/// Contains function data: byte code and its offset in the module. /// Contains function data: byte code and its offset in the module.
#[derive(Hash)] #[derive(Hash)]
@@ -391,18 +392,21 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
} }
fn reserve_passive_data(&mut self, count: u32) -> WasmResult<()> { fn reserve_passive_data(&mut self, count: u32) -> WasmResult<()> {
self.result.module.passive_elements.reserve(count as usize); self.result.module.passive_data.reserve(count as usize);
Ok(()) Ok(())
} }
fn declare_passive_data( fn declare_passive_data(&mut self, data_index: DataIndex, data: &'data [u8]) -> WasmResult<()> {
&mut self, let old = self
_data_index: DataIndex, .result
_data: &'data [u8], .module
) -> WasmResult<()> { .passive_data
Err(WasmError::Unsupported( .insert(data_index, Arc::from(data));
"bulk memory: passive data".to_string(), debug_assert!(
)) old.is_none(),
"a module can't have duplicate indices, this would be a cranelift-wasm bug"
);
Ok(())
} }
fn declare_func_name(&mut self, func_index: FuncIndex, name: &'data str) -> WasmResult<()> { fn declare_func_name(&mut self, func_index: FuncIndex, name: &'data str) -> WasmResult<()> {

View File

@@ -29,8 +29,8 @@ use std::{mem, ptr, slice};
use thiserror::Error; use thiserror::Error;
use wasmtime_environ::entity::{packed_option::ReservedValue, BoxedSlice, EntityRef, PrimaryMap}; use wasmtime_environ::entity::{packed_option::ReservedValue, BoxedSlice, EntityRef, PrimaryMap};
use wasmtime_environ::wasm::{ use wasmtime_environ::wasm::{
DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, ElemIndex, DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex,
FuncIndex, GlobalIndex, GlobalInit, MemoryIndex, SignatureIndex, TableIndex, ElemIndex, FuncIndex, GlobalIndex, GlobalInit, MemoryIndex, SignatureIndex, TableIndex,
}; };
use wasmtime_environ::{ir, DataInitializer, Module, TableElements, VMOffsets}; use wasmtime_environ::{ir, DataInitializer, Module, TableElements, VMOffsets};
@@ -92,6 +92,10 @@ pub(crate) struct Instance {
/// empty slice. /// empty slice.
passive_elements: RefCell<HashMap<ElemIndex, Box<[VMCallerCheckedAnyfunc]>>>, passive_elements: RefCell<HashMap<ElemIndex, Box<[VMCallerCheckedAnyfunc]>>>,
/// Passive data segments from our module. As `data.drop`s happen, entries
/// get removed. A missing entry is considered equivalent to an empty slice.
passive_data: RefCell<HashMap<DataIndex, Arc<[u8]>>>,
/// Pointers to functions in executable memory. /// Pointers to functions in executable memory.
finished_functions: BoxedSlice<DefinedFuncIndex, *mut [VMFunctionBody]>, finished_functions: BoxedSlice<DefinedFuncIndex, *mut [VMFunctionBody]>,
@@ -747,6 +751,57 @@ impl Instance {
} }
} }
/// Performs the `memory.init` operation.
///
/// # Errors
///
/// Returns a `Trap` error if the destination range is out of this module's
/// memory's bounds or if the source range is outside the data segment's
/// bounds.
pub(crate) fn memory_init(
&self,
memory_index: MemoryIndex,
data_index: DataIndex,
dst: u32,
src: u32,
len: u32,
source_loc: ir::SourceLoc,
) -> Result<(), Trap> {
// https://webassembly.github.io/bulk-memory-operations/core/exec/instructions.html#exec-memory-init
let memory = self.get_memory(memory_index);
let passive_data = self.passive_data.borrow();
let data = passive_data
.get(&data_index)
.map_or(&[][..], |data| &**data);
if src
.checked_add(len)
.map_or(true, |n| n as usize > data.len())
|| dst
.checked_add(len)
.map_or(true, |m| m as usize > memory.current_length)
{
return Err(Trap::wasm(source_loc, ir::TrapCode::HeapOutOfBounds));
}
let src_slice = &data[src as usize..(src + len) as usize];
unsafe {
let dst_start = memory.base.add(dst as usize);
let dst_slice = slice::from_raw_parts_mut(dst_start, len as usize);
dst_slice.copy_from_slice(src_slice);
}
Ok(())
}
/// Drop the given data segment, truncating its length to zero.
pub(crate) fn data_drop(&self, data_index: DataIndex) {
let mut passive_data = self.passive_data.borrow_mut();
passive_data.remove(&data_index);
}
/// Get a table by index regardless of whether it is locally-defined or an /// Get a table by index regardless of whether it is locally-defined or an
/// imported, foreign table. /// imported, foreign table.
pub(crate) fn get_table(&self, table_index: TableIndex) -> &Table { pub(crate) fn get_table(&self, table_index: TableIndex) -> &Table {
@@ -824,6 +879,8 @@ impl InstanceHandle {
let offsets = VMOffsets::new(mem::size_of::<*const u8>() as u8, &module.local); let offsets = VMOffsets::new(mem::size_of::<*const u8>() as u8, &module.local);
let passive_data = RefCell::new(module.passive_data.clone());
let handle = { let handle = {
let instance = Instance { let instance = Instance {
refcount: Cell::new(1), refcount: Cell::new(1),
@@ -833,6 +890,7 @@ impl InstanceHandle {
memories, memories,
tables, tables,
passive_elements: Default::default(), passive_elements: Default::default(),
passive_data,
finished_functions, finished_functions,
dbg_jit_registration, dbg_jit_registration,
host_state, host_state,

View File

@@ -36,7 +36,7 @@ use crate::table::Table;
use crate::traphandlers::raise_lib_trap; use crate::traphandlers::raise_lib_trap;
use crate::vmcontext::VMContext; use crate::vmcontext::VMContext;
use wasmtime_environ::ir; use wasmtime_environ::ir;
use wasmtime_environ::wasm::{DefinedMemoryIndex, ElemIndex, MemoryIndex, TableIndex}; use wasmtime_environ::wasm::{DataIndex, DefinedMemoryIndex, ElemIndex, MemoryIndex, TableIndex};
/// Implementation of f32.ceil /// Implementation of f32.ceil
pub extern "C" fn wasmtime_f32_ceil(x: f32) -> f32 { pub extern "C" fn wasmtime_f32_ceil(x: f32) -> f32 {
@@ -299,3 +299,32 @@ pub unsafe extern "C" fn wasmtime_imported_memory_fill(
raise_lib_trap(trap); raise_lib_trap(trap);
} }
} }
/// Implementation of `memory.init`.
pub unsafe extern "C" fn wasmtime_memory_init(
vmctx: *mut VMContext,
memory_index: u32,
data_index: u32,
dst: u32,
src: u32,
len: u32,
source_loc: u32,
) {
let result = {
let memory_index = MemoryIndex::from_u32(memory_index);
let data_index = DataIndex::from_u32(data_index);
let source_loc = ir::SourceLoc::new(source_loc);
let instance = (&mut *vmctx).instance();
instance.memory_init(memory_index, data_index, dst, src, len, source_loc)
};
if let Err(trap) = result {
raise_lib_trap(trap);
}
}
/// Implementation of `data.drop`.
pub unsafe extern "C" fn wasmtime_data_drop(vmctx: *mut VMContext, data_index: u32) {
let data_index = DataIndex::from_u32(data_index);
let instance = (&mut *vmctx).instance();
instance.data_drop(data_index)
}

View File

@@ -84,7 +84,7 @@ fn trap_code_to_expected_string(trap_code: ir::TrapCode) -> String {
match trap_code { match trap_code {
StackOverflow => "call stack exhausted".to_string(), StackOverflow => "call stack exhausted".to_string(),
HeapOutOfBounds => "out of bounds memory access".to_string(), HeapOutOfBounds => "out of bounds memory access".to_string(),
TableOutOfBounds => "undefined element: out of bounds".to_string(), TableOutOfBounds => "undefined element: out of bounds table access".to_string(),
OutOfBounds => "out of bounds".to_string(), // Note: not covered by the test suite OutOfBounds => "out of bounds".to_string(), // Note: not covered by the test suite
IndirectCallToNull => "uninitialized element".to_string(), IndirectCallToNull => "uninitialized element".to_string(),
BadSignature => "indirect call type mismatch".to_string(), BadSignature => "indirect call type mismatch".to_string(),

View File

@@ -565,6 +565,10 @@ impl VMBuiltinFunctionsArray {
wasmtime_memory_fill as usize; wasmtime_memory_fill as usize;
ptrs[BuiltinFunctionIndex::get_imported_memory_fill_index().index() as usize] = ptrs[BuiltinFunctionIndex::get_imported_memory_fill_index().index() as usize] =
wasmtime_imported_memory_fill as usize; wasmtime_imported_memory_fill as usize;
ptrs[BuiltinFunctionIndex::get_memory_init_index().index() as usize] =
wasmtime_memory_init as usize;
ptrs[BuiltinFunctionIndex::get_data_drop_index().index() as usize] =
wasmtime_data_drop as usize;
debug_assert!(ptrs.iter().cloned().all(|p| p != 0)); debug_assert!(ptrs.iter().cloned().all(|p| p != 0));

View File

@@ -196,19 +196,25 @@ impl WastContext {
Ok(()) Ok(())
} }
fn assert_trap(&self, result: Outcome, message: &str) -> Result<()> { fn assert_trap(&self, result: Outcome, expected: &str) -> Result<()> {
let trap = match result { let trap = match result {
Outcome::Ok(values) => bail!("expected trap, got {:?}", values), Outcome::Ok(values) => bail!("expected trap, got {:?}", values),
Outcome::Trap(t) => t, Outcome::Trap(t) => t,
}; };
if trap.message().contains(message) { let actual = trap.message();
if actual.contains(expected)
// `bulk-memory-operations/bulk.wast` checks for a message that
// specifies which element is uninitialized, but our traps don't
// shepherd that information out.
|| (expected.contains("uninitialized element 2") && actual.contains("uninitialized element"))
{
return Ok(()); return Ok(());
} }
if cfg!(feature = "lightbeam") { if cfg!(feature = "lightbeam") {
println!("TODO: Check the assert_trap message: {}", message); println!("TODO: Check the assert_trap message: {}", expected);
return Ok(()); return Ok(());
} }
bail!("expected {}, got {}", message, trap.message()) bail!("expected '{}', got '{}'", expected, actual)
} }
/// Run a wast script from a byte buffer. /// Run a wast script from a byte buffer.