Skip to content

Commit

Permalink
Implement data.drop and memory.init and get the rest of the bulk …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
fitzgen authored Mar 10, 2020
1 parent 11510ec commit 674a620
Show file tree
Hide file tree
Showing 10 changed files with 237 additions and 42 deletions.
6 changes: 0 additions & 6 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,6 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool {
("reference_types", "table_copy_on_imported_tables") => return false,
("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"),
Expand Down
2 changes: 1 addition & 1 deletion crates/environ/src/data_structures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub mod entity {

pub mod 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,
MemoryIndex, SignatureIndex, Table, TableElementType, TableIndex,
};
Expand Down
120 changes: 106 additions & 14 deletions crates/environ/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,17 @@ impl BuiltinFunctionIndex {
pub const fn get_imported_memory_fill_index() -> Self {
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.
pub const fn builtin_functions_total_number() -> u32 {
11
13
}

/// Return the index as an u32 number.
Expand Down Expand Up @@ -120,6 +128,12 @@ pub struct FuncEnvironment<'module_environment> {
/// (it's the same for both local and imported memories).
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: VMOffsets,
}
Expand All @@ -140,6 +154,8 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
elem_drop_sig: None,
memory_copy_sig: None,
memory_fill_sig: None,
memory_init_sig: None,
data_drop_sig: None,
offsets: VMOffsets::new(target_config.pointer_bytes(), module),
}
}
Expand Down Expand Up @@ -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`
/// and address of the loaded function.
fn translate_load_builtin_function_address(
Expand Down Expand Up @@ -1076,23 +1144,47 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m

fn translate_memory_init(
&mut self,
_pos: FuncCursor,
_index: MemoryIndex,
mut pos: FuncCursor,
memory_index: MemoryIndex,
_heap: ir::Heap,
_seg_index: u32,
_dst: ir::Value,
_src: ir::Value,
_len: ir::Value,
seg_index: u32,
dst: ir::Value,
src: ir::Value,
len: ir::Value,
) -> WasmResult<()> {
Err(WasmError::Unsupported(
"bulk memory: `memory.init`".to_string(),
))
let (func_sig, func_idx) = self.get_memory_init_func(&mut pos.func);

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<()> {
Err(WasmError::Unsupported(
"bulk memory: `data.drop`".to_string(),
))
fn translate_data_drop(&mut self, mut pos: FuncCursor, seg_index: u32) -> WasmResult<()> {
let (func_sig, func_idx) = self.get_data_drop_func(&mut pos.func);
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(
Expand Down
14 changes: 11 additions & 3 deletions crates/environ/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ use crate::WASM_MAX_PAGES;
use cranelift_codegen::ir;
use cranelift_entity::{EntityRef, PrimaryMap};
use cranelift_wasm::{
DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, ElemIndex,
FuncIndex, Global, GlobalIndex, Memory, MemoryIndex, SignatureIndex, Table, TableIndex,
DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex,
ElemIndex, FuncIndex, Global, GlobalIndex, Memory, MemoryIndex, SignatureIndex, Table,
TableIndex,
};
use indexmap::IndexMap;
use more_asserts::assert_ge;
use std::collections::HashMap;
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
use std::sync::{
atomic::{AtomicUsize, Ordering::SeqCst},
Arc,
};

/// A WebAssembly table initializer.
#[derive(Clone, Debug, Hash)]
Expand Down Expand Up @@ -168,6 +172,9 @@ pub struct Module {
/// WebAssembly passive elements.
pub passive_elements: HashMap<ElemIndex, Box<[FuncIndex]>>,

/// WebAssembly passive data segments.
pub passive_data: HashMap<DataIndex, Arc<[u8]>>,

/// WebAssembly table initializers.
pub func_names: HashMap<FuncIndex, String>,
}
Expand Down Expand Up @@ -223,6 +230,7 @@ impl Module {
start_func: None,
table_elements: Vec::new(),
passive_elements: HashMap::new(),
passive_data: HashMap::new(),
func_names: HashMap::new(),
local: ModuleLocal {
num_imported_funcs: 0,
Expand Down
24 changes: 14 additions & 10 deletions crates/environ/src/module_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ use cranelift_entity::PrimaryMap;
use cranelift_wasm::{
self, translate_module, DataIndex, DefinedFuncIndex, ElemIndex, FuncIndex, Global, GlobalIndex,
Memory, MemoryIndex, ModuleTranslationState, SignatureIndex, Table, TableIndex,
TargetEnvironment, WasmError, WasmResult,
TargetEnvironment, WasmResult,
};
use std::convert::TryFrom;
use std::sync::Arc;

/// Contains function data: byte code and its offset in the module.
#[derive(Hash)]
Expand Down Expand Up @@ -391,18 +392,21 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
}

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(())
}

fn declare_passive_data(
&mut self,
_data_index: DataIndex,
_data: &'data [u8],
) -> WasmResult<()> {
Err(WasmError::Unsupported(
"bulk memory: passive data".to_string(),
))
fn declare_passive_data(&mut self, data_index: DataIndex, data: &'data [u8]) -> WasmResult<()> {
let old = self
.result
.module
.passive_data
.insert(data_index, Arc::from(data));
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<()> {
Expand Down
62 changes: 60 additions & 2 deletions crates/runtime/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ use std::{mem, ptr, slice};
use thiserror::Error;
use wasmtime_environ::entity::{packed_option::ReservedValue, BoxedSlice, EntityRef, PrimaryMap};
use wasmtime_environ::wasm::{
DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, ElemIndex,
FuncIndex, GlobalIndex, GlobalInit, MemoryIndex, SignatureIndex, TableIndex,
DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex,
ElemIndex, FuncIndex, GlobalIndex, GlobalInit, MemoryIndex, SignatureIndex, TableIndex,
};
use wasmtime_environ::{ir, DataInitializer, Module, TableElements, VMOffsets};

Expand Down Expand Up @@ -92,6 +92,10 @@ pub(crate) struct Instance {
/// empty slice.
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.
finished_functions: BoxedSlice<DefinedFuncIndex, *mut [VMFunctionBody]>,

Expand Down Expand Up @@ -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
/// imported, foreign table.
pub(crate) fn get_table(&self, table_index: TableIndex) -> &Table {
Expand Down Expand Up @@ -824,6 +879,8 @@ impl InstanceHandle {

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 instance = Instance {
refcount: Cell::new(1),
Expand All @@ -833,6 +890,7 @@ impl InstanceHandle {
memories,
tables,
passive_elements: Default::default(),
passive_data,
finished_functions,
dbg_jit_registration,
host_state,
Expand Down
Loading

0 comments on commit 674a620

Please sign in to comment.