From 02e114cf3dcaf158152dca236528277d749d1fdf Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 29 Apr 2019 11:27:13 +0200 Subject: [PATCH] [wasm] Make FuncEnvironment functions fallible (fixes #752); --- cranelift/wasm/src/code_translator.rs | 62 ++++++++++++++------------- cranelift/wasm/src/environ/dummy.rs | 48 +++++++++++++-------- cranelift/wasm/src/environ/spec.rs | 39 +++++++++++------ cranelift/wasm/src/lib.rs | 11 ++++- cranelift/wasm/src/state.rs | 62 +++++++++++++++------------ 5 files changed, 131 insertions(+), 91 deletions(-) diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs index f83b462cef..6c339f3e1f 100644 --- a/cranelift/wasm/src/code_translator.rs +++ b/cranelift/wasm/src/code_translator.rs @@ -71,7 +71,7 @@ pub fn translate_operator( * `get_global` and `set_global` are handled by the environment. ***********************************************************************************/ Operator::GetGlobal { global_index } => { - let val = match state.get_global(builder.func, global_index, environ) { + let val = match state.get_global(builder.func, global_index, environ)? { GlobalVariable::Const(val) => val, GlobalVariable::Memory { gv, offset, ty } => { let addr = builder.ins().global_value(environ.pointer_type(), gv); @@ -82,7 +82,7 @@ pub fn translate_operator( state.push1(val); } Operator::SetGlobal { global_index } => { - match state.get_global(builder.func, global_index, environ) { + match state.get_global(builder.func, global_index, environ)? { GlobalVariable::Const(_) => panic!("global #{} is a constant", global_index), GlobalVariable::Memory { gv, offset, ty } => { let addr = builder.ins().global_value(environ.pointer_type(), gv); @@ -137,7 +137,7 @@ pub fn translate_operator( builder.ins().jump(loop_body, &[]); state.push_loop(loop_body, next, num_return_values(ty)); builder.switch_to_block(loop_body); - environ.translate_loop_header(builder.cursor()); + environ.translate_loop_header(builder.cursor())?; } Operator::If { ty } => { let val = state.pop1(); @@ -348,7 +348,7 @@ pub fn translate_operator( * argument referring to an index in the external functions table of the module. ************************************************************************************/ Operator::Call { function_index } => { - let (fref, num_args) = state.get_direct_func(builder.func, function_index, environ); + let (fref, num_args) = state.get_direct_func(builder.func, function_index, environ)?; let call = environ.translate_call( builder.cursor(), FuncIndex::from_u32(function_index), @@ -369,8 +369,8 @@ pub fn translate_operator( Operator::CallIndirect { index, table_index } => { // `index` is the index of the function's signature and `table_index` is the index of // the table to search the function in. - let (sigref, num_args) = state.get_indirect_sig(builder.func, index, environ); - let table = state.get_table(builder.func, table_index, environ); + let (sigref, num_args) = state.get_indirect_sig(builder.func, index, environ)?; + let table = state.get_table(builder.func, table_index, environ)?; let callee = state.pop1(); let call = environ.translate_call_indirect( builder.cursor(), @@ -398,13 +398,13 @@ pub fn translate_operator( // The WebAssembly MVP only supports one linear memory, but we expect the reserved // argument to be a memory index. let heap_index = MemoryIndex::from_u32(reserved); - let heap = state.get_heap(builder.func, reserved, environ); + let heap = state.get_heap(builder.func, reserved, environ)?; let val = state.pop1(); state.push1(environ.translate_memory_grow(builder.cursor(), heap_index, heap, val)?) } Operator::MemorySize { reserved } => { let heap_index = MemoryIndex::from_u32(reserved); - let heap = state.get_heap(builder.func, reserved, environ); + let heap = state.get_heap(builder.func, reserved, environ)?; state.push1(environ.translate_memory_size(builder.cursor(), heap_index, heap)?); } /******************************* Load instructions *********************************** @@ -414,72 +414,72 @@ pub fn translate_operator( Operator::I32Load8U { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Uload8, I32, builder, state, environ); + translate_load(offset, ir::Opcode::Uload8, I32, builder, state, environ)?; } Operator::I32Load16U { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Uload16, I32, builder, state, environ); + translate_load(offset, ir::Opcode::Uload16, I32, builder, state, environ)?; } Operator::I32Load8S { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Sload8, I32, builder, state, environ); + translate_load(offset, ir::Opcode::Sload8, I32, builder, state, environ)?; } Operator::I32Load16S { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Sload16, I32, builder, state, environ); + translate_load(offset, ir::Opcode::Sload16, I32, builder, state, environ)?; } Operator::I64Load8U { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Uload8, I64, builder, state, environ); + translate_load(offset, ir::Opcode::Uload8, I64, builder, state, environ)?; } Operator::I64Load16U { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Uload16, I64, builder, state, environ); + translate_load(offset, ir::Opcode::Uload16, I64, builder, state, environ)?; } Operator::I64Load8S { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Sload8, I64, builder, state, environ); + translate_load(offset, ir::Opcode::Sload8, I64, builder, state, environ)?; } Operator::I64Load16S { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Sload16, I64, builder, state, environ); + translate_load(offset, ir::Opcode::Sload16, I64, builder, state, environ)?; } Operator::I64Load32S { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Sload32, I64, builder, state, environ); + translate_load(offset, ir::Opcode::Sload32, I64, builder, state, environ)?; } Operator::I64Load32U { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Uload32, I64, builder, state, environ); + translate_load(offset, ir::Opcode::Uload32, I64, builder, state, environ)?; } Operator::I32Load { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Load, I32, builder, state, environ); + translate_load(offset, ir::Opcode::Load, I32, builder, state, environ)?; } Operator::F32Load { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Load, F32, builder, state, environ); + translate_load(offset, ir::Opcode::Load, F32, builder, state, environ)?; } Operator::I64Load { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Load, I64, builder, state, environ); + translate_load(offset, ir::Opcode::Load, I64, builder, state, environ)?; } Operator::F64Load { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_load(offset, ir::Opcode::Load, F64, builder, state, environ); + translate_load(offset, ir::Opcode::Load, F64, builder, state, environ)?; } /****************************** Store instructions *********************************** * Wasm specifies an integer alignment flag but we drop it in Cranelift. @@ -497,7 +497,7 @@ pub fn translate_operator( | Operator::F64Store { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_store(offset, ir::Opcode::Store, builder, state, environ); + translate_store(offset, ir::Opcode::Store, builder, state, environ)?; } Operator::I32Store8 { memarg: MemoryImmediate { flags: _, offset }, @@ -505,7 +505,7 @@ pub fn translate_operator( | Operator::I64Store8 { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_store(offset, ir::Opcode::Istore8, builder, state, environ); + translate_store(offset, ir::Opcode::Istore8, builder, state, environ)?; } Operator::I32Store16 { memarg: MemoryImmediate { flags: _, offset }, @@ -513,12 +513,12 @@ pub fn translate_operator( | Operator::I64Store16 { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_store(offset, ir::Opcode::Istore16, builder, state, environ); + translate_store(offset, ir::Opcode::Istore16, builder, state, environ)?; } Operator::I64Store32 { memarg: MemoryImmediate { flags: _, offset }, } => { - translate_store(offset, ir::Opcode::Istore32, builder, state, environ); + translate_store(offset, ir::Opcode::Istore32, builder, state, environ)?; } /****************************** Nullary Operators ************************************/ Operator::I32Const { value } => state.push1(builder.ins().iconst(I32, i64::from(value))), @@ -1175,10 +1175,10 @@ fn translate_load( builder: &mut FunctionBuilder, state: &mut TranslationState, environ: &mut FE, -) { +) -> WasmResult<()> { let addr32 = state.pop1(); // We don't yet support multiple linear memories. - let heap = state.get_heap(builder.func, 0, environ); + let heap = state.get_heap(builder.func, 0, environ)?; let (base, offset) = get_heap_addr(heap, addr32, offset, environ.pointer_type(), builder); // Note that we don't set `is_aligned` here, even if the load instruction's // alignment immediate says it's aligned, because WebAssembly's immediate @@ -1188,6 +1188,7 @@ fn translate_load( .ins() .Load(opcode, result_ty, flags, offset.into(), base); state.push1(dfg.first_result(load)); + Ok(()) } /// Translate a store instruction. @@ -1197,18 +1198,19 @@ fn translate_store( builder: &mut FunctionBuilder, state: &mut TranslationState, environ: &mut FE, -) { +) -> WasmResult<()> { let (addr32, val) = state.pop2(); let val_ty = builder.func.dfg.value_type(val); // We don't yet support multiple linear memories. - let heap = state.get_heap(builder.func, 0, environ); + let heap = state.get_heap(builder.func, 0, environ)?; let (base, offset) = get_heap_addr(heap, addr32, offset, environ.pointer_type(), builder); // See the comments in `translate_load` about the flags. let flags = MemFlags::new(); builder .ins() .Store(opcode, val_ty, flags, offset.into(), val, base); + Ok(()) } fn translate_icmp(cc: IntCC, builder: &mut FunctionBuilder, state: &mut TranslationState) { diff --git a/cranelift/wasm/src/environ/dummy.rs b/cranelift/wasm/src/environ/dummy.rs index 70b592bbda..e32a3260ab 100644 --- a/cranelift/wasm/src/environ/dummy.rs +++ b/cranelift/wasm/src/environ/dummy.rs @@ -182,18 +182,26 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ self.mod_info.config } - fn make_global(&mut self, func: &mut ir::Function, index: GlobalIndex) -> GlobalVariable { + fn return_mode(&self) -> ReturnMode { + self.return_mode + } + + fn make_global( + &mut self, + func: &mut ir::Function, + index: GlobalIndex, + ) -> WasmResult { // Just create a dummy `vmctx` global. let offset = cast::i32((index.index() * 8) + 8).unwrap().into(); let vmctx = func.create_global_value(ir::GlobalValueData::VMContext {}); - GlobalVariable::Memory { + Ok(GlobalVariable::Memory { gv: vmctx, offset, ty: self.mod_info.globals[index].entity.ty, - } + }) } - fn make_heap(&mut self, func: &mut ir::Function, _index: MemoryIndex) -> ir::Heap { + fn make_heap(&mut self, func: &mut ir::Function, _index: MemoryIndex) -> WasmResult { // Create a static heap whose base address is stored at `vmctx+0`. let addr = func.create_global_value(ir::GlobalValueData::VMContext); let gv = func.create_global_value(ir::GlobalValueData::Load { @@ -203,7 +211,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ readonly: true, }); - func.create_heap(ir::HeapData { + Ok(func.create_heap(ir::HeapData { base: gv, min_size: 0.into(), offset_guard_size: 0x8000_0000.into(), @@ -211,10 +219,10 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ bound: 0x1_0000_0000.into(), }, index_type: I32, - }) + })) } - fn make_table(&mut self, func: &mut ir::Function, _index: TableIndex) -> ir::Table { + fn make_table(&mut self, func: &mut ir::Function, _index: TableIndex) -> WasmResult { // Create a table whose base address is stored at `vmctx+0`. let vmctx = func.create_global_value(ir::GlobalValueData::VMContext); let base_gv = func.create_global_value(ir::GlobalValueData::Load { @@ -230,32 +238,40 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ readonly: true, }); - func.create_table(ir::TableData { + Ok(func.create_table(ir::TableData { base_gv, min_size: Uimm64::new(0), bound_gv, element_size: Uimm64::from(u64::from(self.pointer_bytes()) * 2), index_type: I32, - }) + })) } - fn make_indirect_sig(&mut self, func: &mut ir::Function, index: SignatureIndex) -> ir::SigRef { + fn make_indirect_sig( + &mut self, + func: &mut ir::Function, + index: SignatureIndex, + ) -> WasmResult { // A real implementation would probably change the calling convention and add `vmctx` and // signature index arguments. - func.import_signature(self.vmctx_sig(index)) + Ok(func.import_signature(self.vmctx_sig(index))) } - fn make_direct_func(&mut self, func: &mut ir::Function, index: FuncIndex) -> ir::FuncRef { + fn make_direct_func( + &mut self, + func: &mut ir::Function, + index: FuncIndex, + ) -> WasmResult { let sigidx = self.mod_info.functions[index].entity; // A real implementation would probably add a `vmctx` argument. // And maybe attempt some signature de-duplication. let signature = func.import_signature(self.vmctx_sig(sigidx)); let name = get_func_name(index); - func.import_function(ir::ExtFuncData { + Ok(func.import_function(ir::ExtFuncData { name, signature, colocated: false, - }) + })) } fn translate_call_indirect( @@ -340,10 +356,6 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ ) -> WasmResult { Ok(pos.ins().iconst(I32, -1)) } - - fn return_mode(&self) -> ReturnMode { - self.return_mode - } } impl<'data> ModuleEnvironment<'data> for DummyEnvironment { diff --git a/cranelift/wasm/src/environ/spec.rs b/cranelift/wasm/src/environ/spec.rs index 9b96d42edf..3173a07886 100644 --- a/cranelift/wasm/src/environ/spec.rs +++ b/cranelift/wasm/src/environ/spec.rs @@ -110,6 +110,13 @@ pub trait FuncEnvironment { self.target_config().pointer_bytes() } + /// Should the code be structured to use a single `fallthrough_return` instruction at the end + /// of the function body, rather than `return` instructions as needed? This is used by VMs + /// to append custom epilogues. + fn return_mode(&self) -> ReturnMode { + ReturnMode::NormalReturns + } + /// Set up the necessary preamble definitions in `func` to access the global variable /// identified by `index`. /// @@ -117,19 +124,23 @@ pub trait FuncEnvironment { /// /// Return the global variable reference that should be used to access the global and the /// WebAssembly type of the global. - fn make_global(&mut self, func: &mut ir::Function, index: GlobalIndex) -> GlobalVariable; + fn make_global( + &mut self, + func: &mut ir::Function, + index: GlobalIndex, + ) -> WasmResult; /// Set up the necessary preamble definitions in `func` to access the linear memory identified /// by `index`. /// /// The index space covers both imported and locally declared memories. - fn make_heap(&mut self, func: &mut ir::Function, index: MemoryIndex) -> ir::Heap; + fn make_heap(&mut self, func: &mut ir::Function, index: MemoryIndex) -> WasmResult; /// Set up the necessary preamble definitions in `func` to access the table identified /// by `index`. /// /// The index space covers both imported and locally declared tables. - fn make_table(&mut self, func: &mut ir::Function, index: TableIndex) -> ir::Table; + fn make_table(&mut self, func: &mut ir::Function, index: TableIndex) -> WasmResult; /// Set up a signature definition in the preamble of `func` that can be used for an indirect /// call with signature `index`. @@ -140,7 +151,11 @@ pub trait FuncEnvironment { /// /// The signature will only be used for indirect calls, even if the module has direct function /// calls with the same WebAssembly type. - fn make_indirect_sig(&mut self, func: &mut ir::Function, index: SignatureIndex) -> ir::SigRef; + fn make_indirect_sig( + &mut self, + func: &mut ir::Function, + index: SignatureIndex, + ) -> WasmResult; /// Set up an external function definition in the preamble of `func` that can be used to /// directly call the function `index`. @@ -153,7 +168,11 @@ pub trait FuncEnvironment { /// /// The function's signature will only be used for direct calls, even if the module has /// indirect calls with the same WebAssembly type. - fn make_direct_func(&mut self, func: &mut ir::Function, index: FuncIndex) -> ir::FuncRef; + fn make_direct_func( + &mut self, + func: &mut ir::Function, + index: FuncIndex, + ) -> WasmResult; /// Translate a `call_indirect` WebAssembly instruction at `pos`. /// @@ -226,15 +245,9 @@ pub trait FuncEnvironment { /// /// This can be used to insert explicit interrupt or safepoint checking at /// the beginnings of loops. - fn translate_loop_header(&mut self, _pos: FuncCursor) { + fn translate_loop_header(&mut self, _pos: FuncCursor) -> WasmResult<()> { // By default, don't emit anything. - } - - /// Should the code be structured to use a single `fallthrough_return` instruction at the end - /// of the function body, rather than `return` instructions as needed? This is used by VMs - /// to append custom epilogues. - fn return_mode(&self) -> ReturnMode { - ReturnMode::NormalReturns + Ok(()) } } diff --git a/cranelift/wasm/src/lib.rs b/cranelift/wasm/src/lib.rs index edd5c3215f..c475b342bc 100644 --- a/cranelift/wasm/src/lib.rs +++ b/cranelift/wasm/src/lib.rs @@ -38,9 +38,16 @@ extern crate alloc as std; extern crate std; #[cfg(not(feature = "std"))] -use hashmap_core::{map as hash_map, HashMap}; +use hashmap_core::{ + hash_map::Entry::{Occupied, Vacant}, + map as hash_map, HashMap, +}; #[cfg(feature = "std")] -use std::collections::{hash_map, HashMap}; +use std::collections::{ + hash_map, + hash_map::Entry::{Occupied, Vacant}, + HashMap, +}; mod code_translator; mod environ; diff --git a/cranelift/wasm/src/state.rs b/cranelift/wasm/src/state.rs index f392631237..adfa903748 100644 --- a/cranelift/wasm/src/state.rs +++ b/cranelift/wasm/src/state.rs @@ -3,8 +3,8 @@ //! The `TranslationState` struct defined in this module is used to keep track of the WebAssembly //! value and control stacks during the translation of a single function. -use super::HashMap; -use crate::environ::{FuncEnvironment, GlobalVariable}; +use super::{HashMap, Occupied, Vacant}; +use crate::environ::{FuncEnvironment, GlobalVariable, WasmResult}; use crate::translation_utils::{FuncIndex, GlobalIndex, MemoryIndex, SignatureIndex, TableIndex}; use cranelift_codegen::ir::{self, Ebb, Inst, Value}; use std::vec::Vec; @@ -285,12 +285,12 @@ impl TranslationState { func: &mut ir::Function, index: u32, environ: &mut FE, - ) -> GlobalVariable { + ) -> WasmResult { let index = GlobalIndex::from_u32(index); - *self - .globals - .entry(index) - .or_insert_with(|| environ.make_global(func, index)) + match self.globals.entry(index) { + Occupied(entry) => Ok(*entry.get()), + Vacant(entry) => Ok(*entry.insert(environ.make_global(func, index)?)), + } } /// Get the `Heap` reference that should be used to access linear memory `index`. @@ -300,12 +300,12 @@ impl TranslationState { func: &mut ir::Function, index: u32, environ: &mut FE, - ) -> ir::Heap { + ) -> WasmResult { let index = MemoryIndex::from_u32(index); - *self - .heaps - .entry(index) - .or_insert_with(|| environ.make_heap(func, index)) + match self.heaps.entry(index) { + Occupied(entry) => Ok(*entry.get()), + Vacant(entry) => Ok(*entry.insert(environ.make_heap(func, index)?)), + } } /// Get the `Table` reference that should be used to access table `index`. @@ -315,12 +315,12 @@ impl TranslationState { func: &mut ir::Function, index: u32, environ: &mut FE, - ) -> ir::Table { + ) -> WasmResult { let index = TableIndex::from_u32(index); - *self - .tables - .entry(index) - .or_insert_with(|| environ.make_table(func, index)) + match self.tables.entry(index) { + Occupied(entry) => Ok(*entry.get()), + Vacant(entry) => Ok(*entry.insert(environ.make_table(func, index)?)), + } } /// Get the `SigRef` reference that should be used to make an indirect call with signature @@ -332,12 +332,15 @@ impl TranslationState { func: &mut ir::Function, index: u32, environ: &mut FE, - ) -> (ir::SigRef, usize) { + ) -> WasmResult<(ir::SigRef, usize)> { let index = SignatureIndex::from_u32(index); - *self.signatures.entry(index).or_insert_with(|| { - let sig = environ.make_indirect_sig(func, index); - (sig, normal_args(&func.dfg.signatures[sig])) - }) + match self.signatures.entry(index) { + Occupied(entry) => Ok(*entry.get()), + Vacant(entry) => { + let sig = environ.make_indirect_sig(func, index)?; + Ok(*entry.insert((sig, normal_args(&func.dfg.signatures[sig])))) + } + } } /// Get the `FuncRef` reference that should be used to make a direct call to function @@ -349,13 +352,16 @@ impl TranslationState { func: &mut ir::Function, index: u32, environ: &mut FE, - ) -> (ir::FuncRef, usize) { + ) -> WasmResult<(ir::FuncRef, usize)> { let index = FuncIndex::from_u32(index); - *self.functions.entry(index).or_insert_with(|| { - let fref = environ.make_direct_func(func, index); - let sig = func.dfg.ext_funcs[fref].signature; - (fref, normal_args(&func.dfg.signatures[sig])) - }) + match self.functions.entry(index) { + Occupied(entry) => Ok(*entry.get()), + Vacant(entry) => { + let fref = environ.make_direct_func(func, index)?; + let sig = func.dfg.ext_funcs[fref].signature; + Ok(*entry.insert((fref, normal_args(&func.dfg.signatures[sig])))) + } + } } }