cranelift: Emit a trap when dividing by zero in interpreter

Fixes #3058
This commit is contained in:
Afonso Bordado
2021-07-21 14:43:01 +01:00
committed by Andrew Brown
parent 6be4441bbf
commit df48798396
3 changed files with 71 additions and 18 deletions

View File

@@ -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

View File

@@ -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<V>| 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<V>,
@@ -79,18 +88,24 @@ where
right: V|
-> ValueResult<ControlFlow<V>> { 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<V>,
left: V,
right: V|
-> ValueResult<ControlFlow<V>> { 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<V>, left: V, right: V| -> ValueResult<ControlFlow<V>> {
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"),

View File

@@ -58,7 +58,7 @@ pub trait Value: Clone + From<DataValue> {
fn not(self) -> ValueResult<Self>;
}
#[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<Self> {
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<Self> {
if other.clone().into_int()? == 0 {
return Err(ValueError::IntegerDivisionByZero);
}
binary_match!(%(&self, &other); [I8, I16, I32, I64])
}