Skip to content

Commit

Permalink
Check arithmetic in the MIR
Browse files Browse the repository at this point in the history
Add, Sub, Mul, Shl, and Shr are checked using a new Rvalue:
CheckedBinaryOp, while Div, Rem and Neg are handled with explicit checks
in the MIR.
  • Loading branch information
Aatch authored and eddyb committed Jun 5, 2016
1 parent f97c411 commit 73f3054
Show file tree
Hide file tree
Showing 10 changed files with 428 additions and 9 deletions.
14 changes: 14 additions & 0 deletions src/librustc/mir/repr.rs
Expand Up @@ -787,6 +787,7 @@ pub enum Rvalue<'tcx> {
Cast(CastKind, Operand<'tcx>, Ty<'tcx>),

BinaryOp(BinOp, Operand<'tcx>, Operand<'tcx>),
CheckedBinaryOp(BinOp, Operand<'tcx>, Operand<'tcx>),

UnaryOp(UnOp, Operand<'tcx>),

Expand Down Expand Up @@ -880,6 +881,16 @@ pub enum BinOp {
Gt,
}

impl BinOp {
pub fn is_checkable(self) -> bool {
use self::BinOp::*;
match self {
Add | Sub | Mul | Shl | Shr => true,
_ => false
}
}
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable)]
pub enum UnOp {
/// The `!` operator for logical inversion
Expand All @@ -898,6 +909,9 @@ impl<'tcx> Debug for Rvalue<'tcx> {
Len(ref a) => write!(fmt, "Len({:?})", a),
Cast(ref kind, ref lv, ref ty) => write!(fmt, "{:?} as {:?} ({:?})", lv, ty, kind),
BinaryOp(ref op, ref a, ref b) => write!(fmt, "{:?}({:?}, {:?})", op, a, b),
CheckedBinaryOp(ref op, ref a, ref b) => {
write!(fmt, "Checked{:?}({:?}, {:?})", op, a, b)
}
UnaryOp(ref op, ref a) => write!(fmt, "{:?}({:?})", op, a),
Box(ref t) => write!(fmt, "Box({:?})", t),
InlineAsm { ref asm, ref outputs, ref inputs } => {
Expand Down
7 changes: 7 additions & 0 deletions src/librustc/mir/tcx.rs
Expand Up @@ -183,6 +183,13 @@ impl<'a, 'gcx, 'tcx> Mir<'tcx> {
let rhs_ty = self.operand_ty(tcx, rhs);
Some(self.binop_ty(tcx, op, lhs_ty, rhs_ty))
}
Rvalue::CheckedBinaryOp(op, ref lhs, ref rhs) => {
let lhs_ty = self.operand_ty(tcx, lhs);
let rhs_ty = self.operand_ty(tcx, rhs);
let ty = self.binop_ty(tcx, op, lhs_ty, rhs_ty);
let ty = tcx.mk_tup(vec![ty, tcx.types.bool]);
Some(ty)
}
Rvalue::UnaryOp(_, ref operand) => {
Some(self.operand_ty(tcx, operand))
}
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/mir/visit.rs
Expand Up @@ -461,6 +461,9 @@ macro_rules! make_mir_visitor {
}

Rvalue::BinaryOp(_bin_op,
ref $($mutability)* lhs,
ref $($mutability)* rhs) |
Rvalue::CheckedBinaryOp(_bin_op,
ref $($mutability)* lhs,
ref $($mutability)* rhs) => {
self.visit_operand(lhs);
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_borrowck/borrowck/mir/gather_moves.rs
Expand Up @@ -595,7 +595,8 @@ fn gather_moves<'a, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> MoveD
bb_ctxt.on_operand(SK::Repeat, operand, source),
Rvalue::Cast(ref _kind, ref operand, ref _ty) =>
bb_ctxt.on_operand(SK::Cast, operand, source),
Rvalue::BinaryOp(ref _binop, ref operand1, ref operand2) => {
Rvalue::BinaryOp(ref _binop, ref operand1, ref operand2) |
Rvalue::CheckedBinaryOp(ref _binop, ref operand1, ref operand2) => {
bb_ctxt.on_operand(SK::BinaryOp, operand1, source);
bb_ctxt.on_operand(SK::BinaryOp, operand2, source);
}
Expand Down
195 changes: 194 additions & 1 deletion src/librustc_mir/build/expr/as_rvalue.rs
Expand Up @@ -10,12 +10,19 @@

//! See docs in build/expr/mod.rs

use std;

use rustc_data_structures::fnv::FnvHashMap;

use build::{BlockAnd, BlockAndExtension, Builder};
use build::expr::category::{Category, RvalueFunc};
use hair::*;
use rustc_const_math::{ConstInt, ConstIsize};
use rustc::middle::const_val::ConstVal;
use rustc::ty;
use rustc::mir::repr::*;
use syntax::ast;
use syntax::codemap::Span;

impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
/// Compile `expr`, yielding an rvalue.
Expand Down Expand Up @@ -66,10 +73,34 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
ExprKind::Binary { op, lhs, rhs } => {
let lhs = unpack!(block = this.as_operand(block, lhs));
let rhs = unpack!(block = this.as_operand(block, rhs));
block.and(Rvalue::BinaryOp(op, lhs, rhs))
this.build_binary_op(block, op, expr_span, expr.ty,
lhs, rhs)
}
ExprKind::Unary { op, arg } => {
let arg = unpack!(block = this.as_operand(block, arg));
// Check for -MIN on signed integers
if op == UnOp::Neg && this.check_overflow() && expr.ty.is_signed() {
let bool_ty = this.hir.bool_ty();

let minval = this.minval_literal(expr_span, expr.ty);
let is_min = this.temp(bool_ty);

this.cfg.push_assign(block, scope_id, expr_span, &is_min,
Rvalue::BinaryOp(BinOp::Eq, arg.clone(), minval));

let of_block = this.cfg.start_new_block();
let ok_block = this.cfg.start_new_block();

this.cfg.terminate(block, scope_id, expr_span,
TerminatorKind::If {
cond: Operand::Consume(is_min),
targets: (of_block, ok_block)
});

this.panic(of_block, "attempted to negate with overflow", expr_span);

block = ok_block;
}
block.and(Rvalue::UnaryOp(op, arg))
}
ExprKind::Box { value, value_extents } => {
Expand Down Expand Up @@ -218,4 +249,166 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
}
}

pub fn build_binary_op(&mut self, mut block: BasicBlock, op: BinOp, span: Span, ty: ty::Ty<'tcx>,
lhs: Operand<'tcx>, rhs: Operand<'tcx>) -> BlockAnd<Rvalue<'tcx>> {
let scope_id = self.innermost_scope_id();
let bool_ty = self.hir.bool_ty();
if self.check_overflow() && op.is_checkable() && ty.is_integral() {
let result_tup = self.hir.tcx().mk_tup(vec![ty, bool_ty]);
let result_value = self.temp(result_tup);

self.cfg.push_assign(block, scope_id, span,
&result_value, Rvalue::CheckedBinaryOp(op,
lhs,
rhs));
let val_fld = Field::new(0);
let of_fld = Field::new(1);

let val = result_value.clone().field(val_fld, ty);
let of = result_value.field(of_fld, bool_ty);

let success = self.cfg.start_new_block();
let failure = self.cfg.start_new_block();

self.cfg.terminate(block, scope_id, span,
TerminatorKind::If {
cond: Operand::Consume(of),
targets: (failure, success)
});
let msg = if op == BinOp::Shl || op == BinOp::Shr {
"shift operation overflowed"
} else {
"arithmetic operation overflowed"
};
self.panic(failure, msg, span);
success.and(Rvalue::Use(Operand::Consume(val)))
} else {
if ty.is_integral() && (op == BinOp::Div || op == BinOp::Rem) {
// Checking division and remainder is more complex, since we 1. always check
// and 2. there are two possible failure cases, divide-by-zero and overflow.

let (zero_msg, overflow_msg) = if op == BinOp::Div {
("attempted to divide by zero",
"attempted to divide with overflow")
} else {
("attempted remainder with a divisor of zero",
"attempted remainder with overflow")
};

// Check for / 0
let is_zero = self.temp(bool_ty);
let zero = self.zero_literal(span, ty);
self.cfg.push_assign(block, scope_id, span, &is_zero,
Rvalue::BinaryOp(BinOp::Eq, rhs.clone(), zero));

let zero_block = self.cfg.start_new_block();
let ok_block = self.cfg.start_new_block();

self.cfg.terminate(block, scope_id, span,
TerminatorKind::If {
cond: Operand::Consume(is_zero),
targets: (zero_block, ok_block)
});

self.panic(zero_block, zero_msg, span);
block = ok_block;

// We only need to check for the overflow in one case:
// MIN / -1, and only for signed values.
if ty.is_signed() {
let neg_1 = self.neg_1_literal(span, ty);
let min = self.minval_literal(span, ty);

let is_neg_1 = self.temp(bool_ty);
let is_min = self.temp(bool_ty);
let of = self.temp(bool_ty);

// this does (rhs == -1) & (lhs == MIN). It could short-circuit instead

self.cfg.push_assign(block, scope_id, span, &is_neg_1,
Rvalue::BinaryOp(BinOp::Eq, rhs.clone(), neg_1));
self.cfg.push_assign(block, scope_id, span, &is_min,
Rvalue::BinaryOp(BinOp::Eq, lhs.clone(), min));

let is_neg_1 = Operand::Consume(is_neg_1);
let is_min = Operand::Consume(is_min);
self.cfg.push_assign(block, scope_id, span, &of,
Rvalue::BinaryOp(BinOp::BitAnd, is_neg_1, is_min));

let of_block = self.cfg.start_new_block();
let ok_block = self.cfg.start_new_block();

self.cfg.terminate(block, scope_id, span,
TerminatorKind::If {
cond: Operand::Consume(of),
targets: (of_block, ok_block)
});

self.panic(of_block, overflow_msg, span);

block = ok_block;
}
}

block.and(Rvalue::BinaryOp(op, lhs, rhs))
}
}

// Helper to get a `-1` value of the appropriate type
fn neg_1_literal(&mut self, span: Span, ty: ty::Ty<'tcx>) -> Operand<'tcx> {
let literal = match ty.sty {
ty::TyInt(ity) => {
let val = match ity {
ast::IntTy::I8 => ConstInt::I8(-1),
ast::IntTy::I16 => ConstInt::I16(-1),
ast::IntTy::I32 => ConstInt::I32(-1),
ast::IntTy::I64 => ConstInt::I64(-1),
ast::IntTy::Is => {
let int_ty = self.hir.tcx().sess.target.int_type;
let val = ConstIsize::new(-1, int_ty).unwrap();
ConstInt::Isize(val)
}
};

Literal::Value { value: ConstVal::Integral(val) }
}
_ => {
span_bug!(span, "Invalid type for neg_1_literal: `{:?}`", ty)
}
};

self.literal_operand(span, ty, literal)
}

// Helper to get the minimum value of the appropriate type
fn minval_literal(&mut self, span: Span, ty: ty::Ty<'tcx>) -> Operand<'tcx> {
let literal = match ty.sty {
ty::TyInt(ity) => {
let val = match ity {
ast::IntTy::I8 => ConstInt::I8(std::i8::MIN),
ast::IntTy::I16 => ConstInt::I16(std::i16::MIN),
ast::IntTy::I32 => ConstInt::I32(std::i32::MIN),
ast::IntTy::I64 => ConstInt::I64(std::i64::MIN),
ast::IntTy::Is => {
let int_ty = self.hir.tcx().sess.target.int_type;
let min = match int_ty {
ast::IntTy::I32 => std::i32::MIN as i64,
ast::IntTy::I64 => std::i64::MIN,
_ => unreachable!()
};
let val = ConstIsize::new(min, int_ty).unwrap();
ConstInt::Isize(val)
}
};

Literal::Value { value: ConstVal::Integral(val) }
}
_ => {
span_bug!(span, "Invalid type for minval_literal: `{:?}`", ty)
}
};

self.literal_operand(span, ty, literal)
}
}
10 changes: 6 additions & 4 deletions src/librustc_mir/build/expr/stmt.rs
Expand Up @@ -63,17 +63,19 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// only affects weird things like `x += {x += 1; x}`
// -- is that equal to `x + (x + 1)` or `2*(x+1)`?

let lhs = this.hir.mirror(lhs);
let lhs_ty = lhs.ty;

// As above, RTL.
let rhs = unpack!(block = this.as_operand(block, rhs));
let lhs = unpack!(block = this.as_lvalue(block, lhs));

// we don't have to drop prior contents or anything
// because AssignOp is only legal for Copy types
// (overloaded ops should be desugared into a call).
this.cfg.push_assign(block, scope_id, expr_span, &lhs,
Rvalue::BinaryOp(op,
Operand::Consume(lhs.clone()),
rhs));
let result = unpack!(block = this.build_binary_op(block, op, expr_span, lhs_ty,
Operand::Consume(lhs.clone()), rhs));
this.cfg.push_assign(block, scope_id, expr_span, &lhs, result);

block.unit()
}
Expand Down
54 changes: 53 additions & 1 deletion src/librustc_mir/build/misc.rs
Expand Up @@ -12,9 +12,14 @@
//! kind of thing.

use build::Builder;
use rustc::ty::Ty;

use rustc_const_math::{ConstInt, ConstUsize, ConstIsize};
use rustc::middle::const_val::ConstVal;
use rustc::ty::{self, Ty};

use rustc::mir::repr::*;
use std::u32;
use syntax::ast;
use syntax::codemap::Span;

impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -50,6 +55,53 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
Rvalue::Aggregate(AggregateKind::Tuple, vec![])
}

// Returns a zero literal operand for the appropriate type, works for
// bool, char, integers and floats.
pub fn zero_literal(&mut self, span: Span, ty: Ty<'tcx>) -> Operand<'tcx> {
let literal = match ty.sty {
ty::TyBool => {
self.hir.false_literal()
}
ty::TyChar => Literal::Value { value: ConstVal::Char('\0') },
ty::TyUint(ity) => {
let val = match ity {
ast::UintTy::U8 => ConstInt::U8(0),
ast::UintTy::U16 => ConstInt::U16(0),
ast::UintTy::U32 => ConstInt::U32(0),
ast::UintTy::U64 => ConstInt::U64(0),
ast::UintTy::Us => {
let uint_ty = self.hir.tcx().sess.target.uint_type;
let val = ConstUsize::new(0, uint_ty).unwrap();
ConstInt::Usize(val)
}
};

Literal::Value { value: ConstVal::Integral(val) }
}
ty::TyInt(ity) => {
let val = match ity {
ast::IntTy::I8 => ConstInt::I8(0),
ast::IntTy::I16 => ConstInt::I16(0),
ast::IntTy::I32 => ConstInt::I32(0),
ast::IntTy::I64 => ConstInt::I64(0),
ast::IntTy::Is => {
let int_ty = self.hir.tcx().sess.target.int_type;
let val = ConstIsize::new(0, int_ty).unwrap();
ConstInt::Isize(val)
}
};

Literal::Value { value: ConstVal::Integral(val) }
}
ty::TyFloat(_) => Literal::Value { value: ConstVal::Float(0.0) },
_ => {
span_bug!(span, "Invalid type for zero_literal: `{:?}`", ty)
}
};

self.literal_operand(span, ty, literal)
}

pub fn push_usize(&mut self,
block: BasicBlock,
scope_id: ScopeId,
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_mir/build/mod.rs
Expand Up @@ -378,6 +378,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
}
}

pub fn check_overflow(&self) -> bool {
self.hir.tcx().sess.opts.debugging_opts.force_overflow_checks.unwrap_or(
self.hir.tcx().sess.opts.debug_assertions)
}
}

///////////////////////////////////////////////////////////////////////////
Expand Down

0 comments on commit 73f3054

Please sign in to comment.