diff --git a/cranelift/interpreter/src/interpreter.rs b/cranelift/interpreter/src/interpreter.rs index 11732be230..3c9964c431 100644 --- a/cranelift/interpreter/src/interpreter.rs +++ b/cranelift/interpreter/src/interpreter.rs @@ -287,7 +287,9 @@ impl<'a> State<'a, DataValue> for InterpreterState<'a> { #[cfg(test)] mod tests { use super::*; + use crate::step::CraneliftTrap; use cranelift_codegen::ir::immediates::Ieee32; + use cranelift_codegen::ir::TrapCode; use cranelift_reader::parse_functions; // Most interpreter tests should use the more ergonomic `test interpret` filetest but this @@ -316,6 +318,28 @@ mod tests { assert_eq!(result, vec![DataValue::B(true)]) } + // We don't have a way to check for traps with the current filetest infrastructure + #[test] + fn udiv_by_zero_traps() { + let code = "function %test() -> i32 { + block0: + v0 = iconst.i32 1 + v1 = udiv_imm.i32 v0, 0 + return v1 + }"; + + let func = parse_functions(code).unwrap().into_iter().next().unwrap(); + let mut env = FunctionStore::default(); + env.add(func.name.to_string(), &func); + let state = InterpreterState::default().with_function_store(env); + let result = Interpreter::new(state).call_by_name("%test", &[]).unwrap(); + + match result { + ControlFlow::Trap(CraneliftTrap::User(TrapCode::IntegerDivisionByZero)) => {} + _ => panic!("Unexpected ControlFlow: {:?}", result), + } + } + // This test verifies that functions can refer to each other using the function store. A double indirection is // required, which is tricky to get right: a referenced function is a FuncRef when called but a FuncIndex inside the // function store. This test would preferably be a CLIF filetest but the filetest infrastructure only looks at a diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index d3b9f56b66..3e2e346fc4 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -72,6 +72,15 @@ where // Indicate that the result of a step is to assign a single value to an instruction's results. let assign = |value: V| ControlFlow::Assign(smallvec![value]); + // Similar to `assign` but converts some errors into traps + let assign_or_trap = |value: ValueResult| match value { + Ok(v) => Ok(assign(v)), + Err(ValueError::IntegerDivisionByZero) => Ok(ControlFlow::Trap(CraneliftTrap::User( + TrapCode::IntegerDivisionByZero, + ))), + Err(e) => Err(e), + }; + // Interpret a binary instruction with the given `op`, assigning the resulting value to the // instruction's results. let binary = |op: fn(V, V) -> ValueResult, @@ -79,18 +88,24 @@ where right: V| -> ValueResult> { Ok(assign(op(left, right)?)) }; - // Same as `binary`, but converts the values to their unsigned form before the operation and - // back to signed form afterwards. Since Cranelift types have no notion of signedness, this - // enables operations that depend on sign. - let binary_unsigned = + // Similar to `binary` but converts select `ValueError`'s into trap `ControlFlow`'s + let binary_can_trap = |op: fn(V, V) -> ValueResult, + left: V, + right: V| + -> ValueResult> { assign_or_trap(op(left, right)) }; + + // Same as `binary_can_trap`, but converts the values to their unsigned form before the + // operation and back to signed form afterwards. Since Cranelift types have no notion of + // signedness, this enables operations that depend on sign. + let binary_unsigned_can_trap = |op: fn(V, V) -> ValueResult, left: V, right: V| -> ValueResult> { - Ok(assign( + assign_or_trap( op( left.convert(ValueConversionKind::ToUnsigned)?, right.convert(ValueConversionKind::ToUnsigned)?, - )? - .convert(ValueConversionKind::ToSigned)?, - )) + ) + .and_then(|v| v.convert(ValueConversionKind::ToSigned)), + ) }; // Choose whether to assign `left` or `right` to the instruction's result based on a `condition`. @@ -425,16 +440,16 @@ where Opcode::Imul => binary(Value::mul, arg(0)?, arg(1)?)?, Opcode::Umulhi => unimplemented!("Umulhi"), Opcode::Smulhi => unimplemented!("Smulhi"), - Opcode::Udiv => binary_unsigned(Value::div, arg(0)?, arg(1)?)?, - Opcode::Sdiv => binary(Value::div, arg(0)?, arg(1)?)?, - Opcode::Urem => binary_unsigned(Value::rem, arg(0)?, arg(1)?)?, - Opcode::Srem => binary(Value::rem, arg(0)?, arg(1)?)?, + Opcode::Udiv => binary_unsigned_can_trap(Value::div, arg(0)?, arg(1)?)?, + Opcode::Sdiv => binary_can_trap(Value::div, arg(0)?, arg(1)?)?, + Opcode::Urem => binary_unsigned_can_trap(Value::rem, arg(0)?, arg(1)?)?, + Opcode::Srem => binary_can_trap(Value::rem, arg(0)?, arg(1)?)?, Opcode::IaddImm => binary(Value::add, arg(0)?, imm_as_ctrl_ty()?)?, Opcode::ImulImm => binary(Value::mul, arg(0)?, imm_as_ctrl_ty()?)?, - Opcode::UdivImm => binary_unsigned(Value::div, arg(0)?, imm())?, - Opcode::SdivImm => binary(Value::div, arg(0)?, imm_as_ctrl_ty()?)?, - Opcode::UremImm => binary_unsigned(Value::rem, arg(0)?, imm())?, - Opcode::SremImm => binary(Value::rem, arg(0)?, imm_as_ctrl_ty()?)?, + Opcode::UdivImm => binary_unsigned_can_trap(Value::div, arg(0)?, imm())?, + Opcode::SdivImm => binary_can_trap(Value::div, arg(0)?, imm_as_ctrl_ty()?)?, + Opcode::UremImm => binary_unsigned_can_trap(Value::rem, arg(0)?, imm())?, + Opcode::SremImm => binary_can_trap(Value::rem, arg(0)?, imm_as_ctrl_ty()?)?, Opcode::IrsubImm => binary(Value::sub, imm_as_ctrl_ty()?, arg(0)?)?, Opcode::IaddCin => unimplemented!("IaddCin"), Opcode::IaddIfcin => unimplemented!("IaddIfcin"), diff --git a/cranelift/interpreter/src/value.rs b/cranelift/interpreter/src/value.rs index 54bff022a2..5c7528ac23 100644 --- a/cranelift/interpreter/src/value.rs +++ b/cranelift/interpreter/src/value.rs @@ -58,7 +58,7 @@ pub trait Value: Clone + From { fn not(self) -> ValueResult; } -#[derive(Error, Debug)] +#[derive(Error, Debug, PartialEq)] pub enum ValueError { #[error("unable to convert type {1} into class {0}")] InvalidType(ValueTypeClass, Type), @@ -66,9 +66,11 @@ pub enum ValueError { InvalidValue(Type), #[error("unable to convert to primitive integer")] InvalidInteger(#[from] std::num::TryFromIntError), + #[error("performed a division by zero")] + IntegerDivisionByZero, } -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum ValueTypeClass { Integer, Boolean, @@ -173,6 +175,10 @@ impl Value for DataValue { DataValue::I16(n) => Ok(n as i64), DataValue::I32(n) => Ok(n as i64), DataValue::I64(n) => Ok(n), + DataValue::U8(n) => Ok(n as i64), + DataValue::U16(n) => Ok(n as i64), + DataValue::U32(n) => Ok(n as i64), + DataValue::U64(n) => Ok(n as i64), _ => Err(ValueError::InvalidType(ValueTypeClass::Integer, self.ty())), } } @@ -309,10 +315,18 @@ impl Value for DataValue { } fn div(self, other: Self) -> ValueResult { + if other.clone().into_int()? == 0 { + return Err(ValueError::IntegerDivisionByZero); + } + binary_match!(/(&self, &other); [I8, I16, I32, I64, U8, U16, U32, U64]) } fn rem(self, other: Self) -> ValueResult { + if other.clone().into_int()? == 0 { + return Err(ValueError::IntegerDivisionByZero); + } + binary_match!(%(&self, &other); [I8, I16, I32, I64]) }