[wasm] Make FuncEnvironment functions fallible (fixes #752);

cranelift/wasm/src/code_translator.rs

@@ -71,7 +71,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
          *  `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<FE: FuncEnvironment + ?Sized>(
             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<FE: FuncEnvironment + ?Sized>(
             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<FE: FuncEnvironment + ?Sized>(
          * 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<FE: FuncEnvironment + ?Sized>(
         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 (sigref, num_args) = state.get_indirect_sig(builder.func, index, environ)?;
-            let table = state.get_table(builder.func, table_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<FE: FuncEnvironment + ?Sized>(
             // 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<FE: FuncEnvironment + ?Sized>(
         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<FE: FuncEnvironment + ?Sized>(
         | 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<FE: FuncEnvironment + ?Sized>(
         | 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<FE: FuncEnvironment + ?Sized>(
         | 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<FE: FuncEnvironment + ?Sized>(
     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<FE: FuncEnvironment + ?Sized>(
         .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<FE: FuncEnvironment + ?Sized>(
     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) {

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<GlobalVariable> {
         // 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<ir::Heap> {
         // 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<ir::Table> {
         // 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<ir::SigRef> {
         // 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<ir::FuncRef> {
         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<ir::Value> {
         Ok(pos.ins().iconst(I32, -1))
     }
-
-    fn return_mode(&self) -> ReturnMode {
-        self.return_mode
-    }
 }
 
 impl<'data> ModuleEnvironment<'data> for DummyEnvironment {

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<GlobalVariable>;
 
     /// 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<ir::Heap>;
 
     /// 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<ir::Table>;
 
     /// 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<ir::SigRef>;
 
     /// 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<ir::FuncRef>;
 
     /// 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.
-    }
+        Ok(())
-
-    /// 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
     }
 }
 

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;

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 super::{HashMap, Occupied, Vacant};
-use crate::environ::{FuncEnvironment, GlobalVariable};
+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<GlobalVariable> {
         let index = GlobalIndex::from_u32(index);
-        *self
+        match self.globals.entry(index) {
-            .globals
+            Occupied(entry) => Ok(*entry.get()),
-            .entry(index)
+            Vacant(entry) => Ok(*entry.insert(environ.make_global(func, index)?)),
-            .or_insert_with(|| 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<ir::Heap> {
         let index = MemoryIndex::from_u32(index);
-        *self
+        match self.heaps.entry(index) {
-            .heaps
+            Occupied(entry) => Ok(*entry.get()),
-            .entry(index)
+            Vacant(entry) => Ok(*entry.insert(environ.make_heap(func, index)?)),
-            .or_insert_with(|| 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<ir::Table> {
         let index = TableIndex::from_u32(index);
-        *self
+        match self.tables.entry(index) {
-            .tables
+            Occupied(entry) => Ok(*entry.get()),
-            .entry(index)
+            Vacant(entry) => Ok(*entry.insert(environ.make_table(func, index)?)),
-            .or_insert_with(|| 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(|| {
+        match self.signatures.entry(index) {
-            let sig = environ.make_indirect_sig(func, index);
+            Occupied(entry) => Ok(*entry.get()),
-            (sig, normal_args(&func.dfg.signatures[sig]))
+            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(|| {
+        match self.functions.entry(index) {
-            let fref = environ.make_direct_func(func, index);
+            Occupied(entry) => Ok(*entry.get()),
+            Vacant(entry) => {
+                let fref = environ.make_direct_func(func, index)?;
                 let sig = func.dfg.ext_funcs[fref].signature;
-            (fref, normal_args(&func.dfg.signatures[sig]))
+                Ok(*entry.insert((fref, normal_args(&func.dfg.signatures[sig]))))
-        })
+            }
+        }
     }
 }