From a50a1c21235c0ce450f6a6c36948540c07ed57ac Mon Sep 17 00:00:00 2001 From: James Miller Date: Thu, 31 Mar 2016 18:50:07 +1300 Subject: [PATCH 1/5] Check arithmetic in the MIR 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. --- src/librustc/mir/repr.rs | 14 ++ src/librustc/mir/tcx.rs | 7 + src/librustc/mir/visit.rs | 3 + .../borrowck/mir/gather_moves.rs | 3 +- src/librustc_mir/build/expr/as_rvalue.rs | 195 +++++++++++++++++- src/librustc_mir/build/expr/stmt.rs | 10 +- src/librustc_mir/build/misc.rs | 54 ++++- src/librustc_mir/build/mod.rs | 5 + src/librustc_trans/mir/rvalue.rs | 145 ++++++++++++- 9 files changed, 427 insertions(+), 9 deletions(-) diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs index 9ec05a9b2927c..ad646abaa5759 100644 --- a/src/librustc/mir/repr.rs +++ b/src/librustc/mir/repr.rs @@ -757,6 +757,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>), @@ -850,6 +851,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 @@ -868,6 +879,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 } => { diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs index d710417bf20d8..f77c843f76470 100644 --- a/src/librustc/mir/tcx.rs +++ b/src/librustc/mir/tcx.rs @@ -189,6 +189,13 @@ impl<'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)) } diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 10afd3dd953d0..859667313fb27 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -450,6 +450,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); diff --git a/src/librustc_borrowck/borrowck/mir/gather_moves.rs b/src/librustc_borrowck/borrowck/mir/gather_moves.rs index 2b1b743afe9b5..5f202a4f029dc 100644 --- a/src/librustc_borrowck/borrowck/mir/gather_moves.rs +++ b/src/librustc_borrowck/borrowck/mir/gather_moves.rs @@ -543,7 +543,8 @@ fn gather_moves<'tcx>(mir: &Mir<'tcx>, tcx: &TyCtxt<'tcx>) -> MoveData<'tcx> { 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); } diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index 8992381135ea8..d34ab36fa3927 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -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,'tcx> Builder<'a,'tcx> { /// Compile `expr`, yielding an rvalue. @@ -66,10 +73,34 @@ impl<'a,'tcx> Builder<'a,'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 } => { @@ -221,4 +252,166 @@ impl<'a,'tcx> Builder<'a,'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> { + 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) + } } diff --git a/src/librustc_mir/build/expr/stmt.rs b/src/librustc_mir/build/expr/stmt.rs index 3c1672b919751..a854fb92ac26d 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -67,6 +67,9 @@ impl<'a,'tcx> Builder<'a,'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)); @@ -74,10 +77,9 @@ impl<'a,'tcx> Builder<'a,'tcx> { // 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() } diff --git a/src/librustc_mir/build/misc.rs b/src/librustc_mir/build/misc.rs index 5daaf37d87814..f216782f73746 100644 --- a/src/librustc_mir/build/misc.rs +++ b/src/librustc_mir/build/misc.rs @@ -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,'tcx> Builder<'a,'tcx> { @@ -50,6 +55,53 @@ impl<'a,'tcx> Builder<'a,'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, diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index b088425d58a2a..e4d9f2deba18f 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -341,6 +341,11 @@ impl<'a,'tcx> Builder<'a,'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) + } } /////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index 67d7f44cbbf41..d7e03ed7d1977 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use llvm::ValueRef; +use llvm::{self, ValueRef}; use rustc::ty::{self, Ty}; use rustc::ty::cast::{CastTy, IntTy}; use middle::const_val::ConstVal; @@ -18,7 +18,9 @@ use rustc::mir::repr as mir; use asm; use base; use callee::Callee; -use common::{self, C_uint, BlockAndBuilder, Result}; +use common::{self, val_ty, + C_null, + C_uint, C_undef, C_u8, BlockAndBuilder, Result}; use datum::{Datum, Lvalue}; use debuginfo::DebugLoc; use declare; @@ -440,6 +442,21 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { }; (bcx, operand) } + mir::Rvalue::CheckedBinaryOp(op, ref lhs, ref rhs) => { + let lhs = self.trans_operand(&bcx, lhs); + let rhs = self.trans_operand(&bcx, rhs); + let result = self.trans_scalar_checked_binop(&bcx, op, + lhs.immediate(), rhs.immediate(), + lhs.ty); + let val_ty = self.mir.binop_ty(bcx.tcx(), op, lhs.ty, rhs.ty); + let operand_ty = bcx.tcx().mk_tup(vec![val_ty, bcx.tcx().types.bool]); + let operand = OperandRef { + val: OperandValue::Immediate(result), + ty: operand_ty + }; + + (bcx, operand) + } mir::Rvalue::UnaryOp(op, ref operand) => { let operand = self.trans_operand(&bcx, operand); @@ -602,6 +619,57 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } } } + + pub fn trans_scalar_checked_binop(&mut self, + bcx: &BlockAndBuilder<'bcx, 'tcx>, + op: mir::BinOp, + lhs: ValueRef, + rhs: ValueRef, + input_ty: Ty<'tcx>) -> ValueRef { + let (val, of) = match op { + // These are checked using intrinsics + mir::BinOp::Add | mir::BinOp::Sub | mir::BinOp::Mul => { + let oop = match op { + mir::BinOp::Add => OverflowOp::Add, + mir::BinOp::Sub => OverflowOp::Sub, + mir::BinOp::Mul => OverflowOp::Mul, + _ => unreachable!() + }; + let intrinsic = get_overflow_intrinsic(oop, bcx, input_ty); + let res = bcx.call(intrinsic, &[lhs, rhs], None); + + let val = bcx.extract_value(res, 0); + let of = bcx.extract_value(res, 1); + + (val, bcx.zext(of, Type::bool(bcx.ccx()))) + } + mir::BinOp::Shl | mir::BinOp::Shr => { + let lhs_llty = val_ty(lhs); + let rhs_llty = val_ty(rhs); + let invert_mask = bcx.with_block(|bcx| { + common::shift_mask_val(bcx, lhs_llty, rhs_llty, true) + }); + let outer_bits = bcx.and(rhs, invert_mask); + + let of = bcx.icmp(llvm::IntNE, outer_bits, C_null(rhs_llty)); + let val = self.trans_scalar_binop(bcx, op, lhs, rhs, input_ty); + + (val, bcx.zext(of, Type::bool(bcx.ccx()))) + } + _ => { + // Fall back to regular translation with a constant-false overflow flag + (self.trans_scalar_binop(bcx, op, lhs, rhs, input_ty), + C_u8(bcx.ccx(), 0)) + } + }; + + let val_ty = val_ty(val); + let res_ty = Type::struct_(bcx.ccx(), &[val_ty, Type::bool(bcx.ccx())], false); + + let mut res_val = C_undef(res_ty); + res_val = bcx.insert_value(res_val, val, 0); + bcx.insert_value(res_val, of, 1) + } } pub fn rvalue_creates_operand<'bcx, 'tcx>(mir: &mir::Mir<'tcx>, @@ -612,6 +680,7 @@ pub fn rvalue_creates_operand<'bcx, 'tcx>(mir: &mir::Mir<'tcx>, mir::Rvalue::Len(..) | mir::Rvalue::Cast(..) | // (*) mir::Rvalue::BinaryOp(..) | + mir::Rvalue::CheckedBinaryOp(..) | mir::Rvalue::UnaryOp(..) | mir::Rvalue::Box(..) => true, @@ -632,3 +701,75 @@ pub fn rvalue_creates_operand<'bcx, 'tcx>(mir: &mir::Mir<'tcx>, // (*) this is only true if the type is suitable } + +#[derive(Copy, Clone)] +enum OverflowOp { + Add, Sub, Mul +} + +fn get_overflow_intrinsic(oop: OverflowOp, bcx: &BlockAndBuilder, ty: Ty) -> ValueRef { + use syntax::ast::IntTy::*; + use syntax::ast::UintTy::*; + use rustc::ty::{TyInt, TyUint}; + + let tcx = bcx.tcx(); + + let new_sty = match ty.sty { + TyInt(Is) => match &tcx.sess.target.target.target_pointer_width[..] { + "32" => TyInt(I32), + "64" => TyInt(I64), + _ => panic!("unsupported target word size") + }, + TyUint(Us) => match &tcx.sess.target.target.target_pointer_width[..] { + "32" => TyUint(U32), + "64" => TyUint(U64), + _ => panic!("unsupported target word size") + }, + ref t @ TyUint(_) | ref t @ TyInt(_) => t.clone(), + _ => panic!("tried to get overflow intrinsic for op applied to non-int type") + }; + + let name = match oop { + OverflowOp::Add => match new_sty { + TyInt(I8) => "llvm.sadd.with.overflow.i8", + TyInt(I16) => "llvm.sadd.with.overflow.i16", + TyInt(I32) => "llvm.sadd.with.overflow.i32", + TyInt(I64) => "llvm.sadd.with.overflow.i64", + + TyUint(U8) => "llvm.uadd.with.overflow.i8", + TyUint(U16) => "llvm.uadd.with.overflow.i16", + TyUint(U32) => "llvm.uadd.with.overflow.i32", + TyUint(U64) => "llvm.uadd.with.overflow.i64", + + _ => unreachable!(), + }, + OverflowOp::Sub => match new_sty { + TyInt(I8) => "llvm.ssub.with.overflow.i8", + TyInt(I16) => "llvm.ssub.with.overflow.i16", + TyInt(I32) => "llvm.ssub.with.overflow.i32", + TyInt(I64) => "llvm.ssub.with.overflow.i64", + + TyUint(U8) => "llvm.usub.with.overflow.i8", + TyUint(U16) => "llvm.usub.with.overflow.i16", + TyUint(U32) => "llvm.usub.with.overflow.i32", + TyUint(U64) => "llvm.usub.with.overflow.i64", + + _ => unreachable!(), + }, + OverflowOp::Mul => match new_sty { + TyInt(I8) => "llvm.smul.with.overflow.i8", + TyInt(I16) => "llvm.smul.with.overflow.i16", + TyInt(I32) => "llvm.smul.with.overflow.i32", + TyInt(I64) => "llvm.smul.with.overflow.i64", + + TyUint(U8) => "llvm.umul.with.overflow.i8", + TyUint(U16) => "llvm.umul.with.overflow.i16", + TyUint(U32) => "llvm.umul.with.overflow.i32", + TyUint(U64) => "llvm.umul.with.overflow.i64", + + _ => unreachable!(), + }, + }; + + bcx.ccx().get_intrinsic(&name) +} From 196de1a90d03dbd366542dc286f49ed41e611ed7 Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 29 Apr 2016 00:04:45 +1200 Subject: [PATCH 2/5] Add a `with_cond` method Factors out the common pattern across the several places that do arithmetic checks --- src/librustc_mir/build/block.rs | 28 ++++++++++ src/librustc_mir/build/cfg.rs | 7 ++- src/librustc_mir/build/expr/as_rvalue.rs | 65 ++++++++---------------- 3 files changed, 54 insertions(+), 46 deletions(-) diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 49029f9642e08..202715001b84a 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -12,6 +12,7 @@ use build::{BlockAnd, BlockAndExtension, Builder}; use hair::*; use rustc::mir::repr::*; use rustc::hir; +use syntax::codemap::Span; impl<'a,'tcx> Builder<'a,'tcx> { pub fn ast_block(&mut self, @@ -79,4 +80,31 @@ impl<'a,'tcx> Builder<'a,'tcx> { block.unit() }) } + + // Helper method for generating MIR inside a conditional block. + pub fn with_cond(&mut self, block: BasicBlock, span: Span, + cond: Operand<'tcx>, f: F) -> BasicBlock + where F: FnOnce(&mut Builder<'a, 'tcx>, BasicBlock) -> BasicBlock { + let scope_id = self.innermost_scope_id(); + + let then_block = self.cfg.start_new_block(); + let else_block = self.cfg.start_new_block(); + + self.cfg.terminate(block, scope_id, span, + TerminatorKind::If { + cond: cond, + targets: (then_block, else_block) + }); + + let after = f(self, then_block); + + // If the returned block isn't terminated, add a branch to the "else" + // block + if !self.cfg.terminated(after) { + self.cfg.terminate(after, scope_id, span, + TerminatorKind::Goto { target: else_block }); + } + + else_block + } } diff --git a/src/librustc_mir/build/cfg.rs b/src/librustc_mir/build/cfg.rs index 4859257f291c9..7ffef989e2f00 100644 --- a/src/librustc_mir/build/cfg.rs +++ b/src/librustc_mir/build/cfg.rs @@ -86,7 +86,7 @@ impl<'tcx> CFG<'tcx> { scope: ScopeId, span: Span, kind: TerminatorKind<'tcx>) { - debug_assert!(self.block_data(block).terminator.is_none(), + debug_assert!(!self.terminated(block), "terminate: block {:?} already has a terminator set", block); self.block_data_mut(block).terminator = Some(Terminator { span: span, @@ -94,4 +94,9 @@ impl<'tcx> CFG<'tcx> { kind: kind, }); } + + /// Returns whether or not the given block has been terminated or not + pub fn terminated(&self, block: BasicBlock) -> bool { + self.block_data(block).terminator.is_some() + } } diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index d34ab36fa3927..2816b78c15f36 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -88,18 +88,11 @@ impl<'a,'tcx> Builder<'a,'tcx> { 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 = this.with_cond( + block, expr_span, Operand::Consume(is_min), |this, block| { + this.panic(block, "attempted to negate with overflow", expr_span); + block + }); } block.and(Rvalue::UnaryOp(op, arg)) } @@ -271,21 +264,18 @@ impl<'a,'tcx> Builder<'a,'tcx> { 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))) + + block = self.with_cond(block, span, Operand::Consume(of), |this, block| { + this.panic(block, msg, span); + block + }); + + block.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 @@ -305,17 +295,10 @@ impl<'a,'tcx> Builder<'a,'tcx> { 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; + block = self.with_cond(block, span, Operand::Consume(is_zero), |this, block| { + this.panic(block, zero_msg, span); + block + }); // We only need to check for the overflow in one case: // MIN / -1, and only for signed values. @@ -339,18 +322,10 @@ impl<'a,'tcx> Builder<'a,'tcx> { 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 = self.with_cond(block, span, Operand::Consume(of), |this, block| { + this.panic(block, overflow_msg, span); + block + }); } } From 6d3f7a9837df8d528e2fb8d3f1a279c3e24376a4 Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 29 Apr 2016 00:08:01 +1200 Subject: [PATCH 3/5] Enable the overflow-related tests for MIR --- src/test/compile-fail/const-err.rs | 2 -- src/test/compile-fail/const-eval-overflow.rs | 2 -- src/test/run-fail/divide-by-zero.rs | 2 -- src/test/run-fail/mod-zero.rs | 2 -- src/test/run-fail/overflowing-add.rs | 2 -- src/test/run-fail/overflowing-lsh-1.rs | 2 -- src/test/run-fail/overflowing-lsh-2.rs | 2 -- src/test/run-fail/overflowing-lsh-3.rs | 2 -- src/test/run-fail/overflowing-lsh-4.rs | 2 -- src/test/run-fail/overflowing-mul.rs | 2 -- src/test/run-fail/overflowing-neg.rs | 2 -- src/test/run-fail/overflowing-rsh-1.rs | 2 -- src/test/run-fail/overflowing-rsh-2.rs | 2 -- src/test/run-fail/overflowing-rsh-3.rs | 2 -- src/test/run-fail/overflowing-rsh-4.rs | 2 -- src/test/run-fail/overflowing-rsh-5.rs | 2 -- src/test/run-fail/overflowing-rsh-6.rs | 2 -- src/test/run-fail/overflowing-sub.rs | 2 -- src/test/run-make/debug-assertions/debug.rs | 1 - src/test/run-pass/issue-8460.rs | 2 -- 20 files changed, 39 deletions(-) diff --git a/src/test/compile-fail/const-err.rs b/src/test/compile-fail/const-err.rs index 3fb9a3f236ced..999564444e27c 100644 --- a/src/test/compile-fail/const-err.rs +++ b/src/test/compile-fail/const-err.rs @@ -11,7 +11,6 @@ // these errors are not actually "const_err", they occur in trans/consts // and are unconditional warnings that can't be denied or allowed -#![feature(rustc_attrs)] #![allow(exceeding_bitshifts)] #![allow(const_err)] @@ -19,7 +18,6 @@ fn black_box(_: T) { unimplemented!() } -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let a = -std::i8::MIN; //~^ WARN attempted to negate with overflow diff --git a/src/test/compile-fail/const-eval-overflow.rs b/src/test/compile-fail/const-eval-overflow.rs index 96013551ef492..3dfcb5bb29a24 100644 --- a/src/test/compile-fail/const-eval-overflow.rs +++ b/src/test/compile-fail/const-eval-overflow.rs @@ -8,7 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(rustc_attrs)] #![allow(unused_imports)] // Note: the relevant lint pass here runs before some of the constant @@ -104,7 +103,6 @@ const VALS_U64: (u64, u64, u64, u64) = //~^ ERROR attempted to multiply with overflow ); -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { foo(VALS_I8); foo(VALS_I16); diff --git a/src/test/run-fail/divide-by-zero.rs b/src/test/run-fail/divide-by-zero.rs index d3817b25d6100..3d9bee3c86a56 100644 --- a/src/test/run-fail/divide-by-zero.rs +++ b/src/test/run-fail/divide-by-zero.rs @@ -12,8 +12,6 @@ // error-pattern:attempted to divide by zero -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let y = 0; let _z = 1 / y; diff --git a/src/test/run-fail/mod-zero.rs b/src/test/run-fail/mod-zero.rs index 7a151c8c572f6..093dad5838b60 100644 --- a/src/test/run-fail/mod-zero.rs +++ b/src/test/run-fail/mod-zero.rs @@ -12,8 +12,6 @@ // error-pattern:attempted remainder with a divisor of zero -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let y = 0; let _z = 1 % y; diff --git a/src/test/run-fail/overflowing-add.rs b/src/test/run-fail/overflowing-add.rs index c989cc594536b..cb75b6c2493f1 100644 --- a/src/test/run-fail/overflowing-add.rs +++ b/src/test/run-fail/overflowing-add.rs @@ -14,8 +14,6 @@ // compile-flags: -C debug-assertions -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _x = 200u8 + 200u8 + 200u8; } diff --git a/src/test/run-fail/overflowing-lsh-1.rs b/src/test/run-fail/overflowing-lsh-1.rs index a27210112982a..75e885a280e70 100644 --- a/src/test/run-fail/overflowing-lsh-1.rs +++ b/src/test/run-fail/overflowing-lsh-1.rs @@ -15,8 +15,6 @@ #![warn(exceeding_bitshifts)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _x = 1_i32 << 32; } diff --git a/src/test/run-fail/overflowing-lsh-2.rs b/src/test/run-fail/overflowing-lsh-2.rs index fe0bcc5b98545..b87e8226ed7e4 100644 --- a/src/test/run-fail/overflowing-lsh-2.rs +++ b/src/test/run-fail/overflowing-lsh-2.rs @@ -15,8 +15,6 @@ #![warn(exceeding_bitshifts)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _x = 1 << -1; } diff --git a/src/test/run-fail/overflowing-lsh-3.rs b/src/test/run-fail/overflowing-lsh-3.rs index aac220d32d9ce..0c8751a24d3cc 100644 --- a/src/test/run-fail/overflowing-lsh-3.rs +++ b/src/test/run-fail/overflowing-lsh-3.rs @@ -15,8 +15,6 @@ #![warn(exceeding_bitshifts)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _x = 1_u64 << 64; } diff --git a/src/test/run-fail/overflowing-lsh-4.rs b/src/test/run-fail/overflowing-lsh-4.rs index 7e8b266da49be..f8fb584805012 100644 --- a/src/test/run-fail/overflowing-lsh-4.rs +++ b/src/test/run-fail/overflowing-lsh-4.rs @@ -18,8 +18,6 @@ #![warn(exceeding_bitshifts)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { // this signals overflow when checking is on let x = 1_i8 << 17; diff --git a/src/test/run-fail/overflowing-mul.rs b/src/test/run-fail/overflowing-mul.rs index 8cba700bbf9a3..ab09ee194b023 100644 --- a/src/test/run-fail/overflowing-mul.rs +++ b/src/test/run-fail/overflowing-mul.rs @@ -13,8 +13,6 @@ // error-pattern:thread '
' panicked at 'arithmetic operation overflowed' // compile-flags: -C debug-assertions -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let x = 200u8 * 4; } diff --git a/src/test/run-fail/overflowing-neg.rs b/src/test/run-fail/overflowing-neg.rs index 2d9d746bef324..6c3b99ba30e67 100644 --- a/src/test/run-fail/overflowing-neg.rs +++ b/src/test/run-fail/overflowing-neg.rs @@ -13,8 +13,6 @@ // error-pattern:thread '
' panicked at 'attempted to negate with overflow' // compile-flags: -C debug-assertions -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _x = -std::i8::MIN; } diff --git a/src/test/run-fail/overflowing-rsh-1.rs b/src/test/run-fail/overflowing-rsh-1.rs index 63c808dc80a4e..8d78b1f2a872f 100644 --- a/src/test/run-fail/overflowing-rsh-1.rs +++ b/src/test/run-fail/overflowing-rsh-1.rs @@ -15,8 +15,6 @@ #![warn(exceeding_bitshifts)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _x = -1_i32 >> 32; } diff --git a/src/test/run-fail/overflowing-rsh-2.rs b/src/test/run-fail/overflowing-rsh-2.rs index 8b89e57c85bb5..84d05706c1a40 100644 --- a/src/test/run-fail/overflowing-rsh-2.rs +++ b/src/test/run-fail/overflowing-rsh-2.rs @@ -15,8 +15,6 @@ #![warn(exceeding_bitshifts)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _x = -1_i32 >> -1; } diff --git a/src/test/run-fail/overflowing-rsh-3.rs b/src/test/run-fail/overflowing-rsh-3.rs index 8874587064c35..ad67e775ebd7f 100644 --- a/src/test/run-fail/overflowing-rsh-3.rs +++ b/src/test/run-fail/overflowing-rsh-3.rs @@ -15,8 +15,6 @@ #![warn(exceeding_bitshifts)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _x = -1_i64 >> 64; } diff --git a/src/test/run-fail/overflowing-rsh-4.rs b/src/test/run-fail/overflowing-rsh-4.rs index d74fd8a6b8e41..9da565b441435 100644 --- a/src/test/run-fail/overflowing-rsh-4.rs +++ b/src/test/run-fail/overflowing-rsh-4.rs @@ -18,8 +18,6 @@ #![warn(exceeding_bitshifts)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { // this signals overflow when checking is on let x = 2_i8 >> 17; diff --git a/src/test/run-fail/overflowing-rsh-5.rs b/src/test/run-fail/overflowing-rsh-5.rs index 249b952a5dca2..57c57cfbc6803 100644 --- a/src/test/run-fail/overflowing-rsh-5.rs +++ b/src/test/run-fail/overflowing-rsh-5.rs @@ -15,8 +15,6 @@ #![warn(exceeding_bitshifts)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _n = 1i64 >> [64][0]; } diff --git a/src/test/run-fail/overflowing-rsh-6.rs b/src/test/run-fail/overflowing-rsh-6.rs index 1227f35444a60..24674cb50325b 100644 --- a/src/test/run-fail/overflowing-rsh-6.rs +++ b/src/test/run-fail/overflowing-rsh-6.rs @@ -16,8 +16,6 @@ #![warn(exceeding_bitshifts)] #![feature(const_indexing)] -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _n = 1i64 >> [64][0]; } diff --git a/src/test/run-fail/overflowing-sub.rs b/src/test/run-fail/overflowing-sub.rs index ce243a50e0b66..18bc27bffe5c1 100644 --- a/src/test/run-fail/overflowing-sub.rs +++ b/src/test/run-fail/overflowing-sub.rs @@ -13,8 +13,6 @@ // error-pattern:thread '
' panicked at 'arithmetic operation overflowed' // compile-flags: -C debug-assertions -#![feature(rustc_attrs)] -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { let _x = 42u8 - (42u8 + 1); } diff --git a/src/test/run-make/debug-assertions/debug.rs b/src/test/run-make/debug-assertions/debug.rs index fb54161c2c127..65682cb86c368 100644 --- a/src/test/run-make/debug-assertions/debug.rs +++ b/src/test/run-make/debug-assertions/debug.rs @@ -37,7 +37,6 @@ fn debug_assert() { } fn overflow() { - #[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn add(a: u8, b: u8) -> u8 { a + b } add(200u8, 200u8); diff --git a/src/test/run-pass/issue-8460.rs b/src/test/run-pass/issue-8460.rs index 7589bce31f480..8d15fe30a1b07 100644 --- a/src/test/run-pass/issue-8460.rs +++ b/src/test/run-pass/issue-8460.rs @@ -19,13 +19,11 @@ use std::thread; macro_rules! check { ($($e:expr),*) => { $(assert!(thread::spawn({ - #[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. move|| { $e; } }).join().is_err());)* } } -#[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. fn main() { check![ isize::min_value() / -1, From 60a0813e439a5e94439cb7f4ab438062c098a5f2 Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 29 Apr 2016 14:10:56 +1200 Subject: [PATCH 4/5] Change `with_cond` to `build_cond_br` This is simpler to work with than `with_cond`. --- src/librustc_mir/build/block.rs | 19 ++++-------- src/librustc_mir/build/cfg.rs | 7 +---- src/librustc_mir/build/expr/as_rvalue.rs | 38 ++++++++++++------------ 3 files changed, 25 insertions(+), 39 deletions(-) diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 202715001b84a..3b92e9c31daf5 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -81,10 +81,10 @@ impl<'a,'tcx> Builder<'a,'tcx> { }) } - // Helper method for generating MIR inside a conditional block. - pub fn with_cond(&mut self, block: BasicBlock, span: Span, - cond: Operand<'tcx>, f: F) -> BasicBlock - where F: FnOnce(&mut Builder<'a, 'tcx>, BasicBlock) -> BasicBlock { + // Helper method for generating a conditional branch + // Returns (TrueBlock, FalseBlock) + pub fn build_cond_br(&mut self, block: BasicBlock, span: Span, + cond: Operand<'tcx>) -> (BasicBlock, BasicBlock) { let scope_id = self.innermost_scope_id(); let then_block = self.cfg.start_new_block(); @@ -96,15 +96,6 @@ impl<'a,'tcx> Builder<'a,'tcx> { targets: (then_block, else_block) }); - let after = f(self, then_block); - - // If the returned block isn't terminated, add a branch to the "else" - // block - if !self.cfg.terminated(after) { - self.cfg.terminate(after, scope_id, span, - TerminatorKind::Goto { target: else_block }); - } - - else_block + (then_block, else_block) } } diff --git a/src/librustc_mir/build/cfg.rs b/src/librustc_mir/build/cfg.rs index 7ffef989e2f00..4859257f291c9 100644 --- a/src/librustc_mir/build/cfg.rs +++ b/src/librustc_mir/build/cfg.rs @@ -86,7 +86,7 @@ impl<'tcx> CFG<'tcx> { scope: ScopeId, span: Span, kind: TerminatorKind<'tcx>) { - debug_assert!(!self.terminated(block), + debug_assert!(self.block_data(block).terminator.is_none(), "terminate: block {:?} already has a terminator set", block); self.block_data_mut(block).terminator = Some(Terminator { span: span, @@ -94,9 +94,4 @@ impl<'tcx> CFG<'tcx> { kind: kind, }); } - - /// Returns whether or not the given block has been terminated or not - pub fn terminated(&self, block: BasicBlock) -> bool { - self.block_data(block).terminator.is_some() - } } diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index 2816b78c15f36..1b46aee868bc1 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -88,11 +88,10 @@ impl<'a,'tcx> Builder<'a,'tcx> { this.cfg.push_assign(block, scope_id, expr_span, &is_min, Rvalue::BinaryOp(BinOp::Eq, arg.clone(), minval)); - block = this.with_cond( - block, expr_span, Operand::Consume(is_min), |this, block| { - this.panic(block, "attempted to negate with overflow", expr_span); - block - }); + let (of_block, ok_block) = this.build_cond_br(block, expr_span, + Operand::Consume(is_min)); + this.panic(of_block, "attempted to negate with overflow", expr_span); + block = ok_block; } block.and(Rvalue::UnaryOp(op, arg)) } @@ -246,7 +245,8 @@ impl<'a,'tcx> Builder<'a,'tcx> { } } - pub fn build_binary_op(&mut self, mut block: BasicBlock, op: BinOp, span: Span, ty: ty::Ty<'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> { let scope_id = self.innermost_scope_id(); let bool_ty = self.hir.bool_ty(); @@ -270,12 +270,10 @@ impl<'a,'tcx> Builder<'a,'tcx> { "arithmetic operation overflowed" }; - block = self.with_cond(block, span, Operand::Consume(of), |this, block| { - this.panic(block, msg, span); - block - }); + let (of_block, ok_block) = self.build_cond_br(block, span, Operand::Consume(of)); + self.panic(of_block, msg, span); - block.and(Rvalue::Use(Operand::Consume(val))) + ok_block.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 @@ -295,10 +293,11 @@ impl<'a,'tcx> Builder<'a,'tcx> { self.cfg.push_assign(block, scope_id, span, &is_zero, Rvalue::BinaryOp(BinOp::Eq, rhs.clone(), zero)); - block = self.with_cond(block, span, Operand::Consume(is_zero), |this, block| { - this.panic(block, zero_msg, span); - block - }); + let (zero_block, ok_block) = self.build_cond_br(block, span, + Operand::Consume(is_zero)); + 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. @@ -322,10 +321,11 @@ impl<'a,'tcx> Builder<'a,'tcx> { self.cfg.push_assign(block, scope_id, span, &of, Rvalue::BinaryOp(BinOp::BitAnd, is_neg_1, is_min)); - block = self.with_cond(block, span, Operand::Consume(of), |this, block| { - this.panic(block, overflow_msg, span); - block - }); + let (of_block, ok_block) = self.build_cond_br(block, span, + Operand::Consume(of)); + self.panic(of_block, overflow_msg, span); + + block = ok_block; } } From 3d33d46888df27d16e7a1db78a1214d214e6d463 Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 29 Apr 2016 14:11:43 +1200 Subject: [PATCH 5/5] Replace the fallback case with a `bug!` This branch shouldn't be hit so if it is, it's probably a mistake. --- src/librustc_trans/mir/rvalue.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index d7e03ed7d1977..e3e95fd4e7c0e 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -19,8 +19,7 @@ use asm; use base; use callee::Callee; use common::{self, val_ty, - C_null, - C_uint, C_undef, C_u8, BlockAndBuilder, Result}; + C_null, C_uint, C_undef, BlockAndBuilder, Result}; use datum::{Datum, Lvalue}; use debuginfo::DebugLoc; use declare; @@ -657,9 +656,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { (val, bcx.zext(of, Type::bool(bcx.ccx()))) } _ => { - // Fall back to regular translation with a constant-false overflow flag - (self.trans_scalar_binop(bcx, op, lhs, rhs, input_ty), - C_u8(bcx.ccx(), 0)) + bug!("Operator `{:?}` is not a checkable operator", op) } };