Skip to content

Commit

Permalink
cranelift: Emit a trap when dividing by zero in interpreter
Browse files Browse the repository at this point in the history
  • Loading branch information
afonso360 committed Jul 21, 2021
1 parent 065190f commit 3ba61e6
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 8 deletions.
24 changes: 24 additions & 0 deletions cranelift/interpreter/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
17 changes: 12 additions & 5 deletions cranelift/interpreter/src/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,32 @@ 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]);

// Check a ValueResult for a trap, and either assign or return that trap.
let assign_or_trap = |res: ValueResult<V>| match res {
Ok(value) => Ok(assign(value)),
Err(ValueError::Trap(code)) => Ok(ControlFlow::Trap(CraneliftTrap::User(code))),
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>,
left: V,
right: V|
-> ValueResult<ControlFlow<V>> { Ok(assign(op(left, right)?)) };
-> ValueResult<ControlFlow<V>> { assign_or_trap(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 =
|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`.
Expand Down
16 changes: 13 additions & 3 deletions cranelift/interpreter/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use core::convert::TryFrom;
use core::fmt::{self, Display, Formatter};
use cranelift_codegen::data_value::DataValue;
use cranelift_codegen::ir::immediates::{Ieee32, Ieee64};
use cranelift_codegen::ir::{types, Type};
use cranelift_codegen::ir::{types, TrapCode, Type};
use thiserror::Error;

pub type ValueResult<T> = Result<T, ValueError>;
Expand Down Expand Up @@ -58,17 +58,19 @@ 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),
#[error("unable to convert value into type {0}")]
InvalidValue(Type),
#[error("unable to convert to primitive integer")]
InvalidInteger(#[from] std::num::TryFromIntError),
#[error("the operation on this value raised a trap")]
Trap(TrapCode),
}

#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum ValueTypeClass {
Integer,
Boolean,
Expand Down Expand Up @@ -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())),
}
}
Expand Down Expand Up @@ -309,6 +315,10 @@ impl Value for DataValue {
}

fn div(self, other: Self) -> ValueResult<Self> {
if other.clone().into_int()? == 0 {
return Err(ValueError::Trap(TrapCode::IntegerDivisionByZero));
}

binary_match!(/(&self, &other); [I8, I16, I32, I64, U8, U16, U32, U64])
}

Expand Down

0 comments on commit 3ba61e6

Please sign in to comment.