remove panic, expect and unimplemented from microwasm + use Result as return value for some functions (#652)

crates/environ/src/lightbeam.rs

@@ -54,13 +54,13 @@ impl crate::compilation::Compiler for Lightbeam {
                 i.as_u32(),
                 &wasmparser::FunctionBody::new(0, function_body.data),
             )
-            .expect("Failed to translate function. TODO: Stop this from panicking");
+            .map_err(|e| CompileError::Codegen(format!("Failed to translate function: {}", e)))?;
             relocations.push(reloc_sink.func_relocs);
         }
 
         let code_section = codegen_session
             .into_translated_code_section()
-            .expect("Failed to generate output code. TODO: Stop this from panicking");
+            .map_err(|e| CompileError::Codegen(format!("Failed to generate output code: {}", e)))?;
 
         // TODO pass jump table offsets to Compilation::from_buffer() when they
         // are implemented in lightbeam -- using empty set of offsets for now.

crates/lightbeam/src/error.rs

@@ -12,6 +12,9 @@ pub enum Error {
 
     #[error("Input error: {0}")]
     Input(String),
+
+    #[error("Microwasm error: {0}")]
+    Microwasm(String),
 }
 
 impl From<BinaryReaderError> for Error {

crates/lightbeam/src/function_body.rs

@@ -53,7 +53,7 @@ where
             ty.params().iter().map(SigType::to_microwasm_type),
             ty.returns().iter().map(SigType::to_microwasm_type),
             body,
-        );
+        )?;
 
         let _ = crate::microwasm::dis(
             std::io::stdout(),
@@ -67,14 +67,18 @@ where
         ty.params().iter().map(SigType::to_microwasm_type),
         ty.returns().iter().map(SigType::to_microwasm_type),
         body,
-    );
+    )?;
 
-    translate(
+    let mut body = Vec::new();
-        session,
+    for i in microwasm_conv {
-        reloc_sink,
+        match i {
-        func_idx,
+            Ok(v) => body.extend(v),
-        microwasm_conv.flat_map(|i| i.expect("TODO: Make this not panic")),
+            Err(e) => return Err(Error::Microwasm(e.message.to_string())),
-    )
+        };
+    }
+
+    translate(session, reloc_sink, func_idx, body)?;
+    Ok(())
 }
 
 pub fn translate<M, I, L: Send + Sync + 'static>(
@@ -283,7 +287,9 @@ where
                         entry.remove_entry();
                     }
                 } else {
-                    panic!("Label defined before being declared");
+                    return Err(Error::Microwasm(
+                        "Label defined before being declared".to_string(),
+                    ));
                 }
             }
             Operator::Block {

crates/lightbeam/src/microwasm.rs

@@ -1,3 +1,4 @@
+use crate::error::Error;
 use crate::module::{ModuleContext, SigType, Signature};
 use smallvec::{smallvec, SmallVec};
 use std::{
@@ -7,7 +8,7 @@ use std::{
     ops::RangeInclusive,
 };
 use wasmparser::{
-    FunctionBody, Ieee32 as WasmIeee32, Ieee64 as WasmIeee64,
+    BinaryReaderError, FunctionBody, Ieee32 as WasmIeee32, Ieee64 as WasmIeee64,
     MemoryImmediate as WasmMemoryImmediate, Operator as WasmOperator, OperatorsReader,
 };
 
@@ -274,24 +275,41 @@ pub const SF32: SignfulType = Type::Float(Size::_32);
 pub const SF64: SignfulType = Type::Float(Size::_64);
 
 impl SignlessType {
-    pub fn from_wasm(other: wasmparser::Type) -> Option<Self> {
+    pub fn from_wasm(other: wasmparser::Type) -> Result<Self, BinaryReaderError> {
         use wasmparser::Type;
 
         match other {
-            Type::I32 => Some(I32),
+            Type::I32 => Ok(I32),
-            Type::I64 => Some(I64),
+            Type::I64 => Ok(I64),
-            Type::F32 => Some(F32),
+            Type::F32 => Ok(F32),
-            Type::F64 => Some(F64),
+            Type::F64 => Ok(F64),
-            Type::EmptyBlockType => None,
+            Type::EmptyBlockType => {
-            _ => unimplemented!(),
+                return Err(BinaryReaderError {
+                    message: "SignlessType with EmptyBlockType",
+                    offset: -1isize as usize,
+                })
+            }
+            _ => {
+                return Err(BinaryReaderError {
+                    message: "SignlessType unimplemented",
+                    offset: -1isize as usize,
+                })
+            }
         }
     }
 }
 
-fn create_returns_from_wasm_type(ty: wasmparser::TypeOrFuncType) -> Vec<SignlessType> {
+fn create_returns_from_wasm_type(
+    ty: wasmparser::TypeOrFuncType,
+) -> Result<Vec<SignlessType>, BinaryReaderError> {
     match ty {
-        wasmparser::TypeOrFuncType::Type(ty) => Vec::from_iter(Type::from_wasm(ty)),
+        wasmparser::TypeOrFuncType::Type(ty) => Ok(Vec::from_iter(Type::from_wasm(ty))),
-        wasmparser::TypeOrFuncType::FuncType(_) => panic!("unsupported func type"),
+        wasmparser::TypeOrFuncType::FuncType(_) => {
+            return Err(BinaryReaderError {
+                message: "Unsupported func type",
+                offset: -1isize as usize,
+            })
+        }
     }
 }
 
@@ -1046,17 +1064,19 @@ where
         params: impl IntoIterator<Item = SignlessType>,
         returns: impl IntoIterator<Item = SignlessType>,
         reader: &'a FunctionBody,
-    ) -> Self {
+    ) -> Result<Self, Error> {
-        // TODO: Don't panic!
         let locals_reader = reader
             .get_locals_reader()
-            .expect("Failed to get locals reader");
+            .map_err(|_| Error::Microwasm("Failed to get locals reader".to_string()))?;
         let mut locals = Vec::from_iter(params);
         let mut consts = Vec::new();
 
         for loc in locals_reader {
-            let (count, ty) = loc.expect("Getting local failed");
+            let (count, ty) =
-            let ty = Type::from_wasm(ty).expect("Invalid local type");
+                loc.map_err(|_| Error::Microwasm("Getting local failed".to_string()))?;
+            let ty = Type::from_wasm(ty)
+                .map_err(|_| Error::Microwasm("Invalid local type".to_string()))?;
+
             locals.extend(std::iter::repeat(ty).take(count as _));
             consts.extend(
                 std::iter::repeat(ty)
@@ -1067,14 +1087,15 @@ where
 
         let num_locals = locals.len() as _;
 
+        let operators_reader = reader
+            .get_operators_reader()
+            .map_err(|_| Error::Microwasm("Failed to get operators reader".to_string()))?;
         let mut out = Self {
             is_done: false,
             stack: locals,
             module: context,
             consts_to_emit: Some(consts),
-            internal: reader
+            internal: operators_reader,
-                .get_operators_reader()
-                .expect("Failed to get operators reader"),
             current_id: 0,
             control_frames: vec![],
             unreachable: false,
@@ -1088,10 +1109,10 @@ where
             kind: ControlFrameKind::Function,
         });
 
-        out
+        Ok(out)
     }
 
-    fn op_sig(&self, op: &WasmOperator) -> OpSig {
+    fn op_sig(&self, op: &WasmOperator) -> Result<OpSig, BinaryReaderError> {
         use self::SigT::T;
         use std::iter::{empty as none, once};
 
@@ -1132,7 +1153,7 @@ where
             };
         }
 
-        match op {
+        let o = match op {
             WasmOperator::Unreachable => OpSig::none(),
             WasmOperator::Nop => OpSig::none(),
 
@@ -1221,8 +1242,18 @@ where
             WasmOperator::F32Const { .. } => sig!(() -> (F32)),
             WasmOperator::F64Const { .. } => sig!(() -> (F64)),
 
-            WasmOperator::RefNull => unimplemented!(),
+            WasmOperator::RefNull => {
-            WasmOperator::RefIsNull => unimplemented!(),
+                return Err(BinaryReaderError {
+                    message: "RefNull unimplemented",
+                    offset: -1isize as usize,
+                })
+            }
+            WasmOperator::RefIsNull => {
+                return Err(BinaryReaderError {
+                    message: "RefIsNull unimplemented",
+                    offset: -1isize as usize,
+                })
+            }
 
             // All comparison operators remove 2 elements and push 1
             WasmOperator::I32Eqz => sig!((I32) -> (I32)),
@@ -1357,8 +1388,14 @@ where
             WasmOperator::I64Extend16S => sig!((I32) -> (I64)),
             WasmOperator::I64Extend32S => sig!((I32) -> (I64)),
 
-            _ => unimplemented!(),
+            _ => {
+                return Err(BinaryReaderError {
+                    message: "Opcode Unimplemented",
+                    offset: -1isize as usize,
+                })
             }
+        };
+        Ok(o)
     }
 
     fn next_id(&mut self) -> u32 {
@@ -1383,14 +1420,19 @@ where
         self.stack.len() as i32 - 1 - idx as i32
     }
 
-    fn apply_op(&mut self, sig: OpSig) {
+    fn apply_op(&mut self, sig: OpSig) -> Result<(), BinaryReaderError> {
         let mut ty_param = None;
 
         for p in sig.input.iter().rev() {
-            let stack_ty = self
+            let stack_ty = match self.stack.pop() {
-                .stack
+                Some(e) => e,
-                .pop()
+                None => {
-                .unwrap_or_else(|| panic!("Stack is empty (while processing {:?})", sig));
+                    return Err(BinaryReaderError {
+                        message: "Stack is empty",
+                        offset: -1isize as usize,
+                    })
+                }
+            };
 
             let ty = match p {
                 SigT::T => {
@@ -1409,21 +1451,34 @@ where
 
         for p in sig.output.into_iter().rev() {
             let ty = match p {
-                SigT::T => ty_param.expect("Type parameter was not set"),
+                SigT::T => match ty_param {
+                    Some(e) => e,
+                    None => {
+                        return Err(BinaryReaderError {
+                            message: "Type parameter was not set",
+                            offset: -1isize as usize,
+                        })
+                    }
+                },
                 SigT::Concrete(ty) => ty,
             };
             self.stack.push(ty);
         }
+        Ok(())
     }
 
     fn block_params(&self) -> Vec<SignlessType> {
         self.stack.clone()
     }
 
-    fn block_params_with_wasm_type(&self, ty: wasmparser::TypeOrFuncType) -> Vec<SignlessType> {
+    fn block_params_with_wasm_type(
+        &self,
+        ty: wasmparser::TypeOrFuncType,
+    ) -> Result<Vec<SignlessType>, BinaryReaderError> {
         let mut out = self.block_params();
-        out.extend(create_returns_from_wasm_type(ty));
+        let return_wasm_type = create_returns_from_wasm_type(ty)?;
-        out
+        out.extend(return_wasm_type);
+        Ok(out)
     }
 }
 
@@ -1483,7 +1538,15 @@ where
                     }
                     WasmOperator::Else => {
                         if depth == 0 {
-                            let block = self.control_frames.last_mut().expect("Failed");
+                            let block = match self.control_frames.last_mut() {
+                                Some(e) => e,
+                                None => {
+                                    return Some(Err(BinaryReaderError {
+                                        message: "unreachable Block else Failed",
+                                        offset: -1isize as usize,
+                                    }))
+                                }
+                            };
 
                             self.stack.truncate(block.arguments as _);
 
@@ -1496,7 +1559,15 @@ where
                     }
                     WasmOperator::End => {
                         if depth == 0 {
-                            let block = self.control_frames.pop().expect("Failed");
+                            let block = match self.control_frames.pop() {
+                                Some(e) => e,
+                                None => {
+                                    return Some(Err(BinaryReaderError {
+                                        message: "unreachable Block end Failed",
+                                        offset: -1isize as usize,
+                                    }))
+                                }
+                            };
 
                             if self.control_frames.is_empty() {
                                 self.is_done = true;
@@ -1536,9 +1607,15 @@ where
             Ok(o) => o,
         };
 
-        let op_sig = self.op_sig(&op);
+        let op_sig = match self.op_sig(&op) {
+            Ok(o) => o,
+            Err(e) => return Some(Err(e)),
+        };
 
-        self.apply_op(op_sig);
+        match self.apply_op(op_sig) {
+            Ok(o) => o,
+            Err(e) => return Some(Err(e)),
+        };
 
         Some(Ok(match op {
             WasmOperator::Unreachable => {
@@ -1548,31 +1625,44 @@ where
             WasmOperator::Nop => smallvec![],
             WasmOperator::Block { ty } => {
                 let id = self.next_id();
+                let return_type_wasm = match create_returns_from_wasm_type(ty) {
+                    Ok(o) => o,
+                    Err(e) => return Some(Err(e)),
+                };
+                let block_param_type_wasm = match self.block_params_with_wasm_type(ty) {
+                    Err(e) => return Some(Err(e)),
+                    Ok(o) => o,
+                };
                 self.control_frames.push(ControlFrame {
                     id,
                     arguments: self.stack.len() as u32,
-                    returns: create_returns_from_wasm_type(ty),
+                    returns: return_type_wasm,
                     kind: ControlFrameKind::Block {
                         needs_end_label: false,
                     },
                 });
-                smallvec![Operator::end(
+                smallvec![Operator::end(block_param_type_wasm, (id, NameTag::End))]
-                    self.block_params_with_wasm_type(ty),
-                    (id, NameTag::End),
-                )]
             }
             WasmOperator::Loop { ty } => {
                 let id = self.next_id();
+                let return_type_wasm = match create_returns_from_wasm_type(ty) {
+                    Ok(o) => o,
+                    Err(e) => return Some(Err(e)),
+                };
+                let block_param_type_wasm = match self.block_params_with_wasm_type(ty) {
+                    Ok(o) => o,
+                    Err(e) => return Some(Err(e)),
+                };
                 self.control_frames.push(ControlFrame {
                     id,
                     arguments: self.stack.len() as u32,
-                    returns: create_returns_from_wasm_type(ty),
+                    returns: return_type_wasm,
                     kind: ControlFrameKind::Loop,
                 });
                 let label = (id, NameTag::Header);
                 smallvec![
                     Operator::loop_(self.block_params(), label),
-                    Operator::end(self.block_params_with_wasm_type(ty), (id, NameTag::End)),
+                    Operator::end(block_param_type_wasm, (id, NameTag::End)),
                     Operator::Br {
                         target: BrTarget::Label(label),
                     },
@@ -1581,10 +1671,18 @@ where
             }
             WasmOperator::If { ty } => {
                 let id = self.next_id();
+                let return_type_wasm = match create_returns_from_wasm_type(ty) {
+                    Ok(o) => o,
+                    Err(e) => return Some(Err(e)),
+                };
+                let block_param_type_wasm = match self.block_params_with_wasm_type(ty) {
+                    Ok(o) => o,
+                    Err(e) => return Some(Err(e)),
+                };
                 self.control_frames.push(ControlFrame {
                     id,
                     arguments: self.stack.len() as u32,
-                    returns: create_returns_from_wasm_type(ty),
+                    returns: return_type_wasm,
                     kind: ControlFrameKind::If { has_else: false },
                 });
                 let (then, else_, end) = (
@@ -1595,7 +1693,7 @@ where
                 smallvec![
                     Operator::block(self.block_params(), then),
                     Operator::block(self.block_params(), else_),
-                    Operator::end(self.block_params_with_wasm_type(ty), end),
+                    Operator::end(block_param_type_wasm, end),
                     Operator::BrIf {
                         then: BrTarget::Label(then).into(),
                         else_: BrTarget::Label(else_).into()
@@ -1605,8 +1703,25 @@ where
             }
             WasmOperator::Else => {
                 // We don't pop it since we're still in the second block.
-                let to_drop = to_drop!(self.control_frames.last().expect("Failed"));
+                let block = match self.control_frames.last() {
-                let block = self.control_frames.last_mut().expect("Failed");
+                    Some(e) => e,
+                    None => {
+                        return Some(Err(BinaryReaderError {
+                            message: "Block else Failed",
+                            offset: -1isize as usize,
+                        }))
+                    }
+                };
+                let to_drop = to_drop!(block);
+                let block = match self.control_frames.last_mut() {
+                    Some(e) => e,
+                    None => {
+                        return Some(Err(BinaryReaderError {
+                            message: "Block else Failed",
+                            offset: -1isize as usize,
+                        }))
+                    }
+                };
 
                 if let ControlFrameKind::If { has_else, .. } = &mut block.kind {
                     *has_else = true;
@@ -1627,7 +1742,15 @@ where
                 )
             }
             WasmOperator::End => {
-                let block = self.control_frames.pop().expect("Failed");
+                let block = match self.control_frames.pop() {
+                    Some(e) => e,
+                    None => {
+                        return Some(Err(BinaryReaderError {
+                            message: "Block End Failed",
+                            offset: -1isize as usize,
+                        }))
+                    }
+                };
 
                 let to_drop = to_drop!(block);
 
@@ -1772,25 +1895,47 @@ where
 
             WasmOperator::GetLocal { local_index } => {
                 // `- 1` because we apply the stack difference _before_ this point
-                let depth = self.local_depth(local_index) - 1;
+                let depth = self.local_depth(local_index).checked_sub(1)?;
-                smallvec![Operator::Pick(
+                let depth = match depth.try_into() {
-                    depth.try_into().expect("Local out of range")
+                    Ok(o) => o,
-                )]
+                    Err(_) => {
+                        return Some(Err(BinaryReaderError {
+                            message: "GetLocal - Local out of range",
+                            offset: -1isize as usize,
+                        }))
+                    }
+                };
+                smallvec![Operator::Pick(depth)]
             }
             WasmOperator::SetLocal { local_index } => {
                 // `+ 1` because we apply the stack difference _before_ this point
-                let depth = self.local_depth(local_index) + 1;
+                let depth = self.local_depth(local_index).checked_add(1)?;
-                smallvec![
+                let depth = match depth.try_into() {
-                    Operator::Swap(depth.try_into().expect("Local out of range")),
+                    Ok(o) => o,
-                    Operator::Drop(0..=0)
+                    Err(_) => {
-                ]
+                        return Some(Err(BinaryReaderError {
+                            message: "SetLocal - Local out of range",
+                            offset: -1isize as usize,
+                        }))
+                    }
+                };
+                smallvec![Operator::Swap(depth), Operator::Drop(0..=0)]
             }
             WasmOperator::TeeLocal { local_index } => {
                 // `+ 1` because we `pick` before `swap`
-                let depth = self.local_depth(local_index) + 1;
+                let depth = self.local_depth(local_index).checked_add(1)?;
+                let depth = match depth.try_into() {
+                    Ok(o) => o,
+                    Err(_) => {
+                        return Some(Err(BinaryReaderError {
+                            message: "SetLocal - Local out of range",
+                            offset: -1isize as usize,
+                        }))
+                    }
+                };
                 smallvec![
                     Operator::Pick(0),
-                    Operator::Swap(depth.try_into().expect("Local out of range")),
+                    Operator::Swap(depth),
                     Operator::Drop(0..=0),
                 ]
             }
@@ -1904,8 +2049,18 @@ where
             WasmOperator::F64Const { value } => {
                 smallvec![Operator::Const(Value::F64(value.into()))]
             }
-            WasmOperator::RefNull => unimplemented!("{:?}", op),
+            WasmOperator::RefNull => {
-            WasmOperator::RefIsNull => unimplemented!("{:?}", op),
+                return Some(Err(BinaryReaderError {
+                    message: "RefNull unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::RefIsNull => {
+                return Some(Err(BinaryReaderError {
+                    message: "RefIsNull unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
             WasmOperator::I32Eqz => smallvec![Operator::Eqz(Size::_32)],
             WasmOperator::I32Eq => smallvec![Operator::Eq(I32)],
             WasmOperator::I32Ne => smallvec![Operator::Ne(I32)],
@@ -2086,24 +2241,94 @@ where
             WasmOperator::I64ReinterpretF64 => smallvec![Operator::I64ReinterpretFromF64],
             WasmOperator::F32ReinterpretI32 => smallvec![Operator::F32ReinterpretFromI32],
             WasmOperator::F64ReinterpretI64 => smallvec![Operator::F64ReinterpretFromI64],
-            WasmOperator::I32Extend8S => unimplemented!("{:?}", op),
+            WasmOperator::I32Extend8S => {
-            WasmOperator::I32Extend16S => unimplemented!("{:?}", op),
+                return Some(Err(BinaryReaderError {
-            WasmOperator::I64Extend8S => unimplemented!("{:?}", op),
+                    message: "I32Extend8S unimplemented",
-            WasmOperator::I64Extend16S => unimplemented!("{:?}", op),
+                    offset: -1isize as usize,
-            WasmOperator::I64Extend32S => unimplemented!("{:?}", op),
+                }))
+            }
+            WasmOperator::I32Extend16S => {
+                return Some(Err(BinaryReaderError {
+                    message: "I32Extend16S unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::I64Extend8S => {
+                return Some(Err(BinaryReaderError {
+                    message: "I64Extend8S unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::I64Extend16S => {
+                return Some(Err(BinaryReaderError {
+                    message: "I64Extend16S unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::I64Extend32S => {
+                return Some(Err(BinaryReaderError {
+                    message: "I64Extend32S unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
 
             // 0xFC operators
             // Non-trapping Float-to-int Conversions
-            WasmOperator::I32TruncSSatF32 => unimplemented!("{:?}", op),
+            WasmOperator::I32TruncSSatF32 => {
-            WasmOperator::I32TruncUSatF32 => unimplemented!("{:?}", op),
+                return Some(Err(BinaryReaderError {
-            WasmOperator::I32TruncSSatF64 => unimplemented!("{:?}", op),
+                    message: "I32TruncSSatF32 unimplemented",
-            WasmOperator::I32TruncUSatF64 => unimplemented!("{:?}", op),
+                    offset: -1isize as usize,
-            WasmOperator::I64TruncSSatF32 => unimplemented!("{:?}", op),
+                }))
-            WasmOperator::I64TruncUSatF32 => unimplemented!("{:?}", op),
+            }
-            WasmOperator::I64TruncSSatF64 => unimplemented!("{:?}", op),
+            WasmOperator::I32TruncUSatF32 => {
-            WasmOperator::I64TruncUSatF64 => unimplemented!("{:?}", op),
+                return Some(Err(BinaryReaderError {
+                    message: "I32TruncUSatF32 unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::I32TruncSSatF64 => {
+                return Some(Err(BinaryReaderError {
+                    message: "I32TruncSSatF64 unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::I32TruncUSatF64 => {
+                return Some(Err(BinaryReaderError {
+                    message: "I32TruncUSatF64 unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::I64TruncSSatF32 => {
+                return Some(Err(BinaryReaderError {
+                    message: "I64TruncSSatF32 unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::I64TruncUSatF32 => {
+                return Some(Err(BinaryReaderError {
+                    message: "I64TruncUSatF32 unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::I64TruncSSatF64 => {
+                return Some(Err(BinaryReaderError {
+                    message: "I64TruncSSatF64 unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
+            WasmOperator::I64TruncUSatF64 => {
+                return Some(Err(BinaryReaderError {
+                    message: "I64TruncUSatF64 unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
 
-            other => unimplemented!("{:?}", other),
+            _other => {
+                return Some(Err(BinaryReaderError {
+                    message: "Opcode unimplemented",
+                    offset: -1isize as usize,
+                }))
+            }
         }))
     }
 }