From 73f3054288ee34b50c1f72404b24fdeb12a38f66 Mon Sep 17 00:00:00 2001 From: James Miller Date: Thu, 31 Mar 2016 18:50:07 +1300 Subject: [PATCH 01/18] 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_mir/transform/qualify_consts.rs | 1 + src/librustc_trans/mir/rvalue.rs | 145 ++++++++++++- 10 files changed, 428 insertions(+), 9 deletions(-) diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs index 1cd837e4853b0..2f16878a94cf9 100644 --- a/src/librustc/mir/repr.rs +++ b/src/librustc/mir/repr.rs @@ -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>), @@ -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 @@ -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 } => { diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs index a1c0d92f60cd6..d0ac98a79587a 100644 --- a/src/librustc/mir/tcx.rs +++ b/src/librustc/mir/tcx.rs @@ -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)) } diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 17a8d040ab478..2f290c813fdce 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -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); diff --git a/src/librustc_borrowck/borrowck/mir/gather_moves.rs b/src/librustc_borrowck/borrowck/mir/gather_moves.rs index fcaa655f749d5..de806eef36f8f 100644 --- a/src/librustc_borrowck/borrowck/mir/gather_moves.rs +++ b/src/librustc_borrowck/borrowck/mir/gather_moves.rs @@ -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); } diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index 2a73346240898..b5b4a01dcc641 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, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// Compile `expr`, yielding an rvalue. @@ -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 } => { @@ -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> { + 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 3324467e70d1d..24369aaff3f5c 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -63,6 +63,9 @@ 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)); @@ -70,10 +73,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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 7317c6f9b3133..00e8909527631 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, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { @@ -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, diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 9d7818a9ba4d6..3faf95331ddab 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -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) + } } /////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 18a1f1595f3c3..57fdfb281d2ef 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -630,6 +630,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { Rvalue::Use(_) | Rvalue::Repeat(..) | Rvalue::UnaryOp(..) | + Rvalue::CheckedBinaryOp(..) | Rvalue::Cast(CastKind::ReifyFnPointer, _, _) | Rvalue::Cast(CastKind::UnsafeFnPointer, _, _) | Rvalue::Cast(CastKind::Unsize, _, _) => {} diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index 6d141862ac3ff..cbd39fffce801 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 rustc::mir::repr as mir; @@ -16,7 +16,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 adt; @@ -430,6 +432,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); @@ -556,6 +573,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>, @@ -566,6 +634,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(..) | mir::Rvalue::Use(..) => @@ -579,3 +648,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 f2c983b248dff67da3f772af0df1483a5accc258 Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 29 Apr 2016 00:04:45 +1200 Subject: [PATCH 02/18] 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 c1626b93f0c41..01930d0ddc098 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, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { pub fn ast_block(&mut self, @@ -81,4 +82,31 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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, 'gcx, '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 b5b4a01dcc641..04609e7f8dd31 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, 'gcx, 'tcx> Builder<'a, 'gcx, '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)) } @@ -268,21 +261,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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 @@ -302,17 +292,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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. @@ -336,18 +319,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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 bcdb2602f8c4f6952f47c78cf41db0ad8223b929 Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 29 Apr 2016 00:08:01 +1200 Subject: [PATCH 03/18] 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 a25255c010caf..85d67f52bfe86 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)] @@ -24,7 +23,6 @@ const FOO: u8 = [5u8][1]; //~^ ERROR array index out of bounds //~^^ ERROR array index out of bounds -#[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 26cc9eda04634..1f5297de5aa8d 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 4648f5c9c79d5..a6a898fef1997 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 12741864eda9e..d25982e601652 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 76e029bab5219..0d9fcc850bbb0 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 a9ee4b882532b..f8dbd41d8bb00 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 179622e49a628..ce8a8c27e52c9 100644 --- a/src/test/run-fail/overflowing-mul.rs +++ b/src/test/run-fail/overflowing-mul.rs @@ -13,8 +13,6 @@ // error-pattern:thread 'main' 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 2bc625f692e53..84e41ea848809 100644 --- a/src/test/run-fail/overflowing-neg.rs +++ b/src/test/run-fail/overflowing-neg.rs @@ -13,8 +13,6 @@ // error-pattern:thread 'main' 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 d37ea693a9fcf..9640054136744 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 a4b7028a474dc..c8c7171d7ad57 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 199da59eb53fd..afcf31f5d69b5 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 d0d89a310e26b..c4b3d61f2af48 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 03588c3576ad3..8793a416286e3 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 914f6d2b5c4ce..e9676b6f70299 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 7eec7699d99c4..96775aef07836 100644 --- a/src/test/run-fail/overflowing-sub.rs +++ b/src/test/run-fail/overflowing-sub.rs @@ -13,8 +13,6 @@ // error-pattern:thread 'main' 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 7fbff36d01e21380894a323bbf537219a8359291 Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 29 Apr 2016 14:10:56 +1200 Subject: [PATCH 04/18] 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 01930d0ddc098..0236a6c0c8042 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -83,10 +83,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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, 'gcx, '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(); @@ -98,15 +98,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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 04609e7f8dd31..683efdc141fbf 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, 'gcx, 'tcx> Builder<'a, 'gcx, '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)) } @@ -243,7 +242,8 @@ 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>, + 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(); @@ -267,12 +267,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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 @@ -292,10 +290,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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. @@ -319,10 +318,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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 156b1fb9e15f63a8525e934a6857c37823b49c0e Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Wed, 25 May 2016 08:39:32 +0300 Subject: [PATCH 05/18] Add a new Assert terminator to MIR for bounds & arithmetic checks. --- src/librustc/mir/repr.rs | 47 +++++++++- src/librustc/mir/visit.rs | 30 ++++++ .../borrowck/mir/dataflow/mod.rs | 3 +- .../borrowck/mir/gather_moves.rs | 16 ++++ src/librustc_const_math/err.rs | 4 +- src/librustc_mir/build/block.rs | 19 ---- src/librustc_mir/build/expr/as_lvalue.rs | 15 ++- src/librustc_mir/build/expr/as_rvalue.rs | 53 ++++++----- src/librustc_mir/build/scope.rs | 69 ++++++-------- src/librustc_mir/transform/no_landing_pads.rs | 1 + src/librustc_mir/transform/qualify_consts.rs | 66 +------------ src/librustc_mir/transform/simplify_cfg.rs | 10 ++ src/librustc_mir/transform/type_check.rs | 18 +++- src/librustc_trans/_match.rs | 2 +- src/librustc_trans/common.rs | 8 +- src/librustc_trans/controlflow.rs | 4 +- src/librustc_trans/expr.rs | 6 +- src/librustc_trans/glue.rs | 2 +- src/librustc_trans/mir/analyze.rs | 1 + src/librustc_trans/mir/block.rs | 93 ++++++++++++++++++- src/librustc_trans/mir/constant.rs | 26 +++--- src/librustc_trans/mir/rvalue.rs | 8 +- 22 files changed, 305 insertions(+), 196 deletions(-) diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs index 2f16878a94cf9..9666741d032b8 100644 --- a/src/librustc/mir/repr.rs +++ b/src/librustc/mir/repr.rs @@ -10,7 +10,7 @@ use graphviz::IntoCow; use middle::const_val::ConstVal; -use rustc_const_math::{ConstUsize, ConstInt}; +use rustc_const_math::{ConstUsize, ConstInt, ConstMathErr}; use hir::def_id::DefId; use ty::subst::Substs; use ty::{self, AdtDef, ClosureSubsts, FnOutput, Region, Ty}; @@ -354,6 +354,16 @@ pub enum TerminatorKind<'tcx> { /// Cleanups to be done if the call unwinds. cleanup: Option }, + + /// Jump to the target if the condition has the expected value, + /// otherwise panic with a message and a cleanup target. + Assert { + cond: Operand<'tcx>, + expected: bool, + msg: AssertMessage<'tcx>, + target: BasicBlock, + cleanup: Option + } } impl<'tcx> Terminator<'tcx> { @@ -389,6 +399,8 @@ impl<'tcx> TerminatorKind<'tcx> { Drop { ref target, unwind: None, .. } => { slice::ref_slice(target).into_cow() } + Assert { target, cleanup: Some(unwind), .. } => vec![target, unwind].into_cow(), + Assert { ref target, .. } => slice::ref_slice(target).into_cow(), } } @@ -413,6 +425,8 @@ impl<'tcx> TerminatorKind<'tcx> { Drop { ref mut target, unwind: None, .. } => { vec![target] } + Assert { ref mut target, cleanup: Some(ref mut unwind), .. } => vec![target, unwind], + Assert { ref mut target, .. } => vec![target] } } } @@ -495,6 +509,26 @@ impl<'tcx> TerminatorKind<'tcx> { } write!(fmt, ")") } + Assert { ref cond, expected, ref msg, .. } => { + write!(fmt, "assert(")?; + if !expected { + write!(fmt, "!")?; + } + write!(fmt, "{:?}, ", cond)?; + + match *msg { + AssertMessage::BoundsCheck { ref len, ref index } => { + write!(fmt, "{:?}, {:?}, {:?}", + "index out of bounds: the len is {} but the index is {}", + len, index)?; + } + AssertMessage::Math(ref err) => { + write!(fmt, "{:?}", err.description())?; + } + } + + write!(fmt, ")") + } } } @@ -532,10 +566,21 @@ impl<'tcx> TerminatorKind<'tcx> { Drop { unwind: Some(_), .. } => { vec!["return".into_cow(), "unwind".into_cow()] } + Assert { cleanup: None, .. } => vec!["".into()], + Assert { .. } => + vec!["success".into_cow(), "unwind".into_cow()] } } } +#[derive(Clone, Debug, RustcEncodable, RustcDecodable)] +pub enum AssertMessage<'tcx> { + BoundsCheck { + len: Operand<'tcx>, + index: Operand<'tcx> + }, + Math(ConstMathErr) +} /////////////////////////////////////////////////////////////////////////// // Statements diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 2f290c813fdce..5c9582b945bb8 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -127,6 +127,11 @@ macro_rules! make_mir_visitor { self.super_terminator_kind(block, kind); } + fn visit_assert_message(&mut self, + msg: & $($mutability)* AssertMessage<'tcx>) { + self.super_assert_message(msg); + } + fn visit_rvalue(&mut self, rvalue: & $($mutability)* Rvalue<'tcx>) { self.super_rvalue(rvalue); @@ -426,6 +431,31 @@ macro_rules! make_mir_visitor { } cleanup.map(|t| self.visit_branch(block, t)); } + + TerminatorKind::Assert { ref $($mutability)* cond, + expected: _, + ref $($mutability)* msg, + target, + cleanup } => { + self.visit_operand(cond); + self.visit_assert_message(msg); + self.visit_branch(block, target); + cleanup.map(|t| self.visit_branch(block, t)); + } + } + } + + fn super_assert_message(&mut self, + msg: & $($mutability)* AssertMessage<'tcx>) { + match *msg { + AssertMessage::BoundsCheck { + ref $($mutability)* len, + ref $($mutability)* index + } => { + self.visit_operand(len); + self.visit_operand(index); + } + AssertMessage::Math(_) => {} } } diff --git a/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs b/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs index 293d2863733f7..81655b5e386f6 100644 --- a/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs +++ b/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs @@ -450,13 +450,14 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> repr::TerminatorKind::Return | repr::TerminatorKind::Resume => {} repr::TerminatorKind::Goto { ref target } | + repr::TerminatorKind::Assert { ref target, cleanup: None, .. } | repr::TerminatorKind::Drop { ref target, location: _, unwind: None } | - repr::TerminatorKind::DropAndReplace { ref target, value: _, location: _, unwind: None } => { self.propagate_bits_into_entry_set_for(in_out, changed, target); } + repr::TerminatorKind::Assert { ref target, cleanup: Some(ref unwind), .. } | repr::TerminatorKind::Drop { ref target, location: _, unwind: Some(ref unwind) } | repr::TerminatorKind::DropAndReplace { ref target, value: _, location: _, unwind: Some(ref unwind) diff --git a/src/librustc_borrowck/borrowck/mir/gather_moves.rs b/src/librustc_borrowck/borrowck/mir/gather_moves.rs index de806eef36f8f..27d208240ac1c 100644 --- a/src/librustc_borrowck/borrowck/mir/gather_moves.rs +++ b/src/librustc_borrowck/borrowck/mir/gather_moves.rs @@ -663,6 +663,22 @@ fn gather_moves<'a, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> MoveD bb_ctxt.on_operand(SK::If, cond, source); } + TerminatorKind::Assert { + ref cond, expected: _, + ref msg, target: _, cleanup: _ + } => { + // The `cond` is always of (copyable) type `bool`, + // so there will never be anything to move. + let _ = cond; + match *msg { + AssertMessage:: BoundsCheck { ref len, ref index } => { + // Same for the usize length and index in bounds-checking. + let _ = (len, index); + } + AssertMessage::Math(_) => {} + } + } + TerminatorKind::SwitchInt { switch_ty: _, values: _, targets: _, ref discr } | TerminatorKind::Switch { adt_def: _, targets: _, ref discr } => { // The `discr` is not consumed; that is instead diff --git a/src/librustc_const_math/err.rs b/src/librustc_const_math/err.rs index 126b3824efec6..9970810d4e278 100644 --- a/src/librustc_const_math/err.rs +++ b/src/librustc_const_math/err.rs @@ -10,7 +10,7 @@ use syntax::ast; -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone, RustcEncodable, RustcDecodable)] pub enum ConstMathErr { NotInRange, CmpBetweenUnequalTypes, @@ -25,7 +25,7 @@ pub enum ConstMathErr { } pub use self::ConstMathErr::*; -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone, RustcEncodable, RustcDecodable)] pub enum Op { Add, Sub, diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 0236a6c0c8042..c1626b93f0c41 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -12,7 +12,6 @@ use build::{BlockAnd, BlockAndExtension, Builder}; use hair::*; use rustc::mir::repr::*; use rustc::hir; -use syntax::codemap::Span; impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { pub fn ast_block(&mut self, @@ -82,22 +81,4 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { block.unit() }) } - - // 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(); - let else_block = self.cfg.start_new_block(); - - self.cfg.terminate(block, scope_id, span, - TerminatorKind::If { - cond: cond, - targets: (then_block, else_block) - }); - - (then_block, else_block) - } } diff --git a/src/librustc_mir/build/expr/as_lvalue.rs b/src/librustc_mir/build/expr/as_lvalue.rs index 15ea3f0e6e867..bb5aca2d8d7c3 100644 --- a/src/librustc_mir/build/expr/as_lvalue.rs +++ b/src/librustc_mir/build/expr/as_lvalue.rs @@ -66,15 +66,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { idx.clone(), Operand::Consume(len.clone()))); - let (success, failure) = (this.cfg.start_new_block(), this.cfg.start_new_block()); - this.cfg.terminate(block, - scope_id, - expr_span, - TerminatorKind::If { - cond: Operand::Consume(lt), - targets: (success, failure), - }); - this.panic_bounds_check(failure, idx.clone(), Operand::Consume(len), expr_span); + let msg = AssertMessage::BoundsCheck { + len: Operand::Consume(len), + index: idx.clone() + }; + let success = this.assert(block, Operand::Consume(lt), true, + msg, expr_span); success.and(slice.index(idx)) } ExprKind::SelfRef => { diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index 683efdc141fbf..40b2fa61c133b 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -12,6 +12,7 @@ use std; +use rustc_const_math::{ConstMathErr, Op}; use rustc_data_structures::fnv::FnvHashMap; use build::{BlockAnd, BlockAndExtension, Builder}; @@ -88,10 +89,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { this.cfg.push_assign(block, scope_id, expr_span, &is_min, Rvalue::BinaryOp(BinOp::Eq, arg.clone(), minval)); - 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; + let err = ConstMathErr::Overflow(Op::Neg); + block = this.assert(block, Operand::Consume(is_min), false, + AssertMessage::Math(err), expr_span); } block.and(Rvalue::UnaryOp(op, arg)) } @@ -261,27 +261,32 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let val = result_value.clone().field(val_fld, ty); let of = result_value.field(of_fld, bool_ty); - let msg = if op == BinOp::Shl || op == BinOp::Shr { - "shift operation overflowed" - } else { - "arithmetic operation overflowed" - }; + let err = ConstMathErr::Overflow(match op { + BinOp::Add => Op::Add, + BinOp::Sub => Op::Sub, + BinOp::Mul => Op::Mul, + BinOp::Shl => Op::Shl, + BinOp::Shr => Op::Shr, + _ => { + bug!("MIR build_binary_op: {:?} is not checkable", op) + } + }); - let (of_block, ok_block) = self.build_cond_br(block, span, Operand::Consume(of)); - self.panic(of_block, msg, span); + block = self.assert(block, Operand::Consume(of), false, + AssertMessage::Math(err), span); - ok_block.and(Rvalue::Use(Operand::Consume(val))) + 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 // 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") + let (zero_err, overflow_err) = if op == BinOp::Div { + (ConstMathErr::DivisionByZero, + ConstMathErr::Overflow(Op::Div)) } else { - ("attempted remainder with a divisor of zero", - "attempted remainder with overflow") + (ConstMathErr::RemainderByZero, + ConstMathErr::Overflow(Op::Rem)) }; // Check for / 0 @@ -290,11 +295,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.cfg.push_assign(block, scope_id, span, &is_zero, Rvalue::BinaryOp(BinOp::Eq, rhs.clone(), zero)); - 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; + block = self.assert(block, Operand::Consume(is_zero), false, + AssertMessage::Math(zero_err), span); // We only need to check for the overflow in one case: // MIN / -1, and only for signed values. @@ -318,11 +320,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.cfg.push_assign(block, scope_id, span, &of, Rvalue::BinaryOp(BinOp::BitAnd, is_neg_1, is_min)); - let (of_block, ok_block) = self.build_cond_br(block, span, - Operand::Consume(of)); - self.panic(of_block, overflow_msg, span); - - block = ok_block; + block = self.assert(block, Operand::Consume(of), false, + AssertMessage::Math(overflow_err), span); } } diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index cd81fc764f4af..209649dd2fd18 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -517,7 +517,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } - pub fn build_drop_and_replace(&mut self, block: BasicBlock, span: Span, @@ -538,48 +537,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { next_target.unit() } - // Panicking - // ========= - // FIXME: should be moved into their own module - pub fn panic_bounds_check(&mut self, - block: BasicBlock, - index: Operand<'tcx>, - len: Operand<'tcx>, - span: Span) { - // fn(&(filename: &'static str, line: u32), index: usize, length: usize) -> ! - let region = ty::ReStatic; // FIXME(mir-borrowck): use a better region? - let func = self.lang_function(lang_items::PanicBoundsCheckFnLangItem); - let args = self.hir.tcx().replace_late_bound_regions(&func.ty.fn_args(), |_| region).0; - - let ref_ty = args[0]; - let tup_ty = if let ty::TyRef(_, tyandmut) = ref_ty.sty { - tyandmut.ty - } else { - span_bug!(span, "unexpected panic_bound_check type: {:?}", func.ty); - }; - - let (tuple, tuple_ref) = (self.temp(tup_ty), self.temp(ref_ty)); - let (file, line) = self.span_to_fileline_args(span); - let elems = vec![Operand::Constant(file), Operand::Constant(line)]; - let scope_id = self.innermost_scope_id(); - // FIXME: We should have this as a constant, rather than a stack variable (to not pollute - // icache with cold branch code), however to achieve that we either have to rely on rvalue - // promotion or have some way, in MIR, to create constants. - self.cfg.push_assign(block, scope_id, span, &tuple, // tuple = (file_arg, line_arg); - Rvalue::Aggregate(AggregateKind::Tuple, elems)); - // FIXME: is this region really correct here? - self.cfg.push_assign(block, scope_id, span, &tuple_ref, // tuple_ref = &tuple; - Rvalue::Ref(region, BorrowKind::Shared, tuple)); - let cleanup = self.diverge_cleanup(); - self.cfg.terminate(block, scope_id, span, TerminatorKind::Call { - func: Operand::Constant(func), - args: vec![Operand::Consume(tuple_ref), index, len], - destination: None, - cleanup: cleanup, - }); - } - /// Create diverge cleanup and branch to it from `block`. + // FIXME: Remove this (used only for unreachable cases in match). pub fn panic(&mut self, block: BasicBlock, msg: &'static str, span: Span) { // fn(&(msg: &'static str filename: &'static str, line: u32)) -> ! let region = ty::ReStatic; // FIXME(mir-borrowck): use a better region? @@ -622,6 +581,32 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }); } + /// Create an Assert terminator and return the success block. + /// If the boolean condition operand is not the expected value, + /// a runtime panic will be caused with the given message. + pub fn assert(&mut self, block: BasicBlock, + cond: Operand<'tcx>, + expected: bool, + msg: AssertMessage<'tcx>, + span: Span) + -> BasicBlock { + let scope_id = self.innermost_scope_id(); + + let success_block = self.cfg.start_new_block(); + let cleanup = self.diverge_cleanup(); + + self.cfg.terminate(block, scope_id, span, + TerminatorKind::Assert { + cond: cond, + expected: expected, + msg: msg, + target: success_block, + cleanup: cleanup + }); + + success_block + } + fn lang_function(&mut self, lang_item: lang_items::LangItem) -> Constant<'tcx> { let funcdid = match self.hir.tcx().lang_items.require(lang_item) { Ok(d) => d, diff --git a/src/librustc_mir/transform/no_landing_pads.rs b/src/librustc_mir/transform/no_landing_pads.rs index 67710c4328569..590106e0a225b 100644 --- a/src/librustc_mir/transform/no_landing_pads.rs +++ b/src/librustc_mir/transform/no_landing_pads.rs @@ -30,6 +30,7 @@ impl<'tcx> MutVisitor<'tcx> for NoLandingPads { /* nothing to do */ }, TerminatorKind::Call { cleanup: ref mut unwind, .. } | + TerminatorKind::Assert { cleanup: ref mut unwind, .. } | TerminatorKind::DropAndReplace { ref mut unwind, .. } | TerminatorKind::Drop { ref mut unwind, .. } => { unwind.take(); diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 57fdfb281d2ef..b9eec6ecd9c58 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -332,61 +332,6 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { } } - /// Returns true if the block ends in a bounds check branch, i.e.: - /// len = Len(array); - /// cond = Lt(idx, len); - /// if cond { - /// ... - /// } else { - /// loc = (...); - /// loc_ref = &loc; - /// panic_bounds_check(loc_ref, idx, len); - /// } - fn is_bounds_check(&self, bb: BasicBlock, - cond_op: &Operand<'tcx>, - if_else: BasicBlock) -> bool { - use rustc::mir::repr::Lvalue::*; - use rustc::mir::repr::Operand::Consume; - use rustc::mir::repr::Rvalue::*; - use rustc::mir::repr::StatementKind::*; - use rustc::mir::repr::TerminatorKind::*; - - let stmts = &self.mir[bb].statements; - let stmts_panic = &self.mir[if_else].statements; - if stmts.len() < 2 || stmts_panic.len() != 2 { - return false; - } - - let all = (&stmts[stmts.len() - 2].kind, - &stmts[stmts.len() - 1].kind, - cond_op, - &stmts_panic[0].kind, - &stmts_panic[1].kind, - &self.mir[if_else].terminator().kind); - match all { - (&Assign(Temp(len), Len(_)), - &Assign(Temp(cond), BinaryOp(BinOp::Lt, ref idx, Consume(Temp(len2)))), - /* if */ &Consume(Temp(cond2)), /* {...} else */ - &Assign(Temp(loc), Aggregate(..)), - &Assign(Temp(loc_ref), Ref(_, _, Temp(loc2))), - &Call { - func: Operand::Constant(Constant { - literal: Literal::Item { def_id, .. }, .. - }), - ref args, - destination: None, - .. - }) => { - len == len2 && cond == cond2 && loc == loc2 && - args[0] == Consume(Temp(loc_ref)) && - args[1] == *idx && - args[2] == Consume(Temp(len)) && - Some(def_id) == self.tcx.lang_items.panic_bounds_check_fn() - } - _ => false - } - } - /// Qualify a whole const, static initializer or const fn. fn qualify_const(&mut self) -> Qualif { let mir = self.mir; @@ -402,6 +347,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { TerminatorKind::Goto { target } | // Drops are considered noops. TerminatorKind::Drop { target, .. } | + TerminatorKind::Assert { target, .. } | TerminatorKind::Call { destination: Some((_, target)), .. } => { Some(target) } @@ -411,15 +357,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { return Qualif::empty(); } - // Need to allow bounds checking branches. - TerminatorKind::If { ref cond, targets: (if_true, if_else) } => { - if self.is_bounds_check(bb, cond, if_else) { - Some(if_true) - } else { - None - } - } - + TerminatorKind::If {..} | TerminatorKind::Switch {..} | TerminatorKind::SwitchInt {..} | TerminatorKind::DropAndReplace { .. } | diff --git a/src/librustc_mir/transform/simplify_cfg.rs b/src/librustc_mir/transform/simplify_cfg.rs index 526157a49c734..d008918026ab8 100644 --- a/src/librustc_mir/transform/simplify_cfg.rs +++ b/src/librustc_mir/transform/simplify_cfg.rs @@ -181,7 +181,17 @@ fn simplify_branches(mir: &mut Mir) { } } + TerminatorKind::Assert { target, cond: Operand::Constant(Constant { + literal: Literal::Value { + value: ConstVal::Bool(cond) + }, .. + }), expected, .. } if cond == expected => { + changed = true; + TerminatorKind::Goto { target: target } + } + TerminatorKind::SwitchInt { ref targets, .. } if targets.len() == 1 => { + changed = true; TerminatorKind::Goto { target: targets[0] } } _ => continue diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index efac8ea846111..019ed670d1f83 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -431,6 +431,21 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { self.check_call_inputs(mir, term, &sig, args); } } + TerminatorKind::Assert { ref cond, ref msg, .. } => { + let cond_ty = mir.operand_ty(tcx, cond); + if cond_ty != tcx.types.bool { + span_mirbug!(self, term, "bad Assert ({:?}, not bool", cond_ty); + } + + if let AssertMessage::BoundsCheck { ref len, ref index } = *msg { + if mir.operand_ty(tcx, len) != tcx.types.usize { + span_mirbug!(self, len, "bounds-check length non-usize {:?}", len) + } + if mir.operand_ty(tcx, index) != tcx.types.usize { + span_mirbug!(self, index, "bounds-check index non-usize {:?}", index) + } + } + } } } @@ -561,7 +576,8 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } TerminatorKind::Drop { target, unwind, .. } | - TerminatorKind::DropAndReplace { target, unwind, .. } => { + TerminatorKind::DropAndReplace { target, unwind, .. } | + TerminatorKind::Assert { target, cleanup: unwind, .. } => { self.assert_iscleanup(mir, block, target, is_cleanup); if let Some(unwind) = unwind { if is_cleanup { diff --git a/src/librustc_trans/_match.rs b/src/librustc_trans/_match.rs index 419e19532dd7c..f524bc8596a54 100644 --- a/src/librustc_trans/_match.rs +++ b/src/librustc_trans/_match.rs @@ -880,7 +880,7 @@ fn compare_values<'blk, 'tcx>(cx: Block<'blk, 'tcx>, rhs_t: Ty<'tcx>, debug_loc: DebugLoc) -> Result<'blk, 'tcx> { - let did = langcall(bcx, + let did = langcall(bcx.tcx(), None, &format!("comparison of `{}`", rhs_t), StrEqFnLangItem); diff --git a/src/librustc_trans/common.rs b/src/librustc_trans/common.rs index f35d87d0741ba..e9b7b590b19a2 100644 --- a/src/librustc_trans/common.rs +++ b/src/librustc_trans/common.rs @@ -1165,18 +1165,18 @@ pub fn normalize_and_test_predicates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }) } -pub fn langcall(bcx: Block, +pub fn langcall(tcx: TyCtxt, span: Option, msg: &str, li: LangItem) -> DefId { - match bcx.tcx().lang_items.require(li) { + match tcx.lang_items.require(li) { Ok(id) => id, Err(s) => { let msg = format!("{} {}", msg, s); match span { - Some(span) => bcx.tcx().sess.span_fatal(span, &msg[..]), - None => bcx.tcx().sess.fatal(&msg[..]), + Some(span) => tcx.sess.span_fatal(span, &msg[..]), + None => tcx.sess.fatal(&msg[..]), } } } diff --git a/src/librustc_trans/controlflow.rs b/src/librustc_trans/controlflow.rs index f793f0a6d553b..0f686227c6f98 100644 --- a/src/librustc_trans/controlflow.rs +++ b/src/librustc_trans/controlflow.rs @@ -400,7 +400,7 @@ pub fn trans_fail<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let align = machine::llalign_of_min(ccx, val_ty(expr_file_line_const)); let expr_file_line = consts::addr_of(ccx, expr_file_line_const, align, "panic_loc"); let args = vec!(expr_file_line); - let did = langcall(bcx, Some(call_info.span), "", PanicFnLangItem); + let did = langcall(bcx.tcx(), Some(call_info.span), "", PanicFnLangItem); Callee::def(ccx, did, ccx.tcx().mk_substs(Substs::empty())) .call(bcx, call_info.debug_loc(), ArgVals(&args), None).bcx } @@ -428,7 +428,7 @@ pub fn trans_fail_bounds_check<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let align = machine::llalign_of_min(ccx, val_ty(file_line_const)); let file_line = consts::addr_of(ccx, file_line_const, align, "panic_bounds_check_loc"); let args = vec!(file_line, index, len); - let did = langcall(bcx, Some(call_info.span), "", PanicBoundsCheckFnLangItem); + let did = langcall(bcx.tcx(), Some(call_info.span), "", PanicBoundsCheckFnLangItem); Callee::def(ccx, did, ccx.tcx().mk_substs(Substs::empty())) .call(bcx, call_info.debug_loc(), ArgVals(&args), None).bcx } diff --git a/src/librustc_trans/expr.rs b/src/librustc_trans/expr.rs index d75516ff64837..465ebace1b830 100644 --- a/src/librustc_trans/expr.rs +++ b/src/librustc_trans/expr.rs @@ -2230,11 +2230,11 @@ impl OverflowOpViaIntrinsic { binop_debug_loc); let expect = bcx.ccx().get_intrinsic(&"llvm.expect.i1"); - Call(bcx, expect, &[cond, C_integral(Type::i1(bcx.ccx()), 0, false)], - binop_debug_loc); + let expected = Call(bcx, expect, &[cond, C_bool(bcx.ccx(), false)], + binop_debug_loc); let bcx = - base::with_cond(bcx, cond, |bcx| + base::with_cond(bcx, expected, |bcx| controlflow::trans_fail(bcx, info, InternedString::new("arithmetic operation overflowed"))); diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index 9787915840109..211efeb4e4baa 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -53,7 +53,7 @@ pub fn trans_exchange_free_dyn<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, -> Block<'blk, 'tcx> { let _icx = push_ctxt("trans_exchange_free"); - let def_id = langcall(bcx, None, "", ExchangeFreeFnLangItem); + let def_id = langcall(bcx.tcx(), None, "", ExchangeFreeFnLangItem); let args = [PointerCast(bcx, v, Type::i8p(bcx.ccx())), size, align]; Callee::def(bcx.ccx(), def_id, bcx.tcx().mk_substs(Substs::empty())) .call(bcx, debug_loc, ArgVals(&args), None).bcx diff --git a/src/librustc_trans/mir/analyze.rs b/src/librustc_trans/mir/analyze.rs index 03df1c451f0d1..bce639ac8f76e 100644 --- a/src/librustc_trans/mir/analyze.rs +++ b/src/librustc_trans/mir/analyze.rs @@ -161,6 +161,7 @@ pub fn cleanup_kinds<'bcx,'tcx>(_bcx: Block<'bcx,'tcx>, /* nothing to do */ } TerminatorKind::Call { cleanup: unwind, .. } | + TerminatorKind::Assert { cleanup: unwind, .. } | TerminatorKind::DropAndReplace { unwind, .. } | TerminatorKind::Drop { unwind, .. } => { if let Some(unwind) = unwind { diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index eb962b6615442..e4d137d36ac37 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -9,6 +9,7 @@ // except according to those terms. use llvm::{self, ValueRef}; +use rustc::middle::lang_items; use rustc::ty; use rustc::mir::repr as mir; use abi::{Abi, FnType, ArgType}; @@ -16,7 +17,9 @@ use adt; use base; use build; use callee::{Callee, CalleeData, Fn, Intrinsic, NamedTupleConstructor, Virtual}; -use common::{self, type_is_fat_ptr, Block, BlockAndBuilder, LandingPad, C_undef}; +use common::{self, type_is_fat_ptr, Block, BlockAndBuilder, LandingPad}; +use common::{C_bool, C_str_slice, C_struct, C_u32, C_undef}; +use consts; use debuginfo::DebugLoc; use Disr; use machine::{llalign_of_min, llbitsize_of_real}; @@ -24,7 +27,9 @@ use meth; use type_of; use glue; use type_::Type; + use rustc_data_structures::fnv::FnvHashMap; +use syntax::parse::token; use super::{MirContext, TempRef}; use super::analyze::CleanupKind; @@ -212,6 +217,92 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } } + mir::TerminatorKind::Assert { ref cond, expected, ref msg, target, cleanup } => { + let cond = self.trans_operand(&bcx, cond).immediate(); + let const_cond = common::const_to_opt_uint(cond).map(|c| c == 1); + + // Don't translate the panic block if success if known. + let lltarget = self.llblock(target); + if const_cond == Some(expected) { + funclet_br(bcx, lltarget); + return; + } + + if const_cond == Some(!expected) { + // Do nothing to end up with an unconditional panic. + } else { + // Pass the condition through llvm.expect for branch hinting. + let expect = bcx.ccx().get_intrinsic(&"llvm.expect.i1"); + let cond = bcx.call(expect, &[cond, C_bool(bcx.ccx(), expected)], None); + + // Create the failure block and the conditional branch to it. + // After this point, bcx is the block for the call to panic. + let panic_block = self.fcx.new_block("panic", None); + if expected { + bcx.cond_br(cond, lltarget, panic_block.llbb); + } else { + bcx.cond_br(cond, panic_block.llbb, lltarget); + } + bcx = panic_block.build(); + } + + // Get the location information. + let loc = bcx.sess().codemap().lookup_char_pos(terminator.span.lo); + let filename = token::intern_and_get_ident(&loc.file.name); + let filename = C_str_slice(bcx.ccx(), filename); + let line = C_u32(bcx.ccx(), loc.line as u32); + + // Put together the arguments to the panic entry point. + let (lang_item, args) = match *msg { + mir::AssertMessage::BoundsCheck { ref len, ref index } => { + let len = self.trans_operand(&mut bcx, len); + let index = self.trans_operand(&mut bcx, index); + + let file_line = C_struct(bcx.ccx(), &[filename, line], false); + let align = llalign_of_min(bcx.ccx(), common::val_ty(file_line)); + let file_line = consts::addr_of(bcx.ccx(), + file_line, + align, + "panic_bounds_check_loc"); + (lang_items::PanicBoundsCheckFnLangItem, + vec![file_line, index.immediate(), len.immediate()]) + } + mir::AssertMessage::Math(ref err) => { + let msg_str = token::intern_and_get_ident(err.description()); + let msg_str = C_str_slice(bcx.ccx(), msg_str); + let msg_file_line = C_struct(bcx.ccx(), + &[msg_str, filename, line], + false); + let align = llalign_of_min(bcx.ccx(), common::val_ty(msg_file_line)); + let msg_file_line = consts::addr_of(bcx.ccx(), + msg_file_line, + align, + "panic_loc"); + (lang_items::PanicFnLangItem, vec![msg_file_line]) + } + }; + + // Obtain the panic entry point. + let def_id = common::langcall(bcx.tcx(), Some(terminator.span), "", lang_item); + let callee = Callee::def(bcx.ccx(), def_id, + bcx.ccx().empty_substs_for_def_id(def_id)); + let llfn = callee.reify(bcx.ccx()).val; + + // Translate the actual panic invoke/call. + if let Some(unwind) = cleanup { + let uwbcx = self.bcx(unwind); + let unwind = self.make_landing_pad(uwbcx); + bcx.invoke(llfn, + &args, + self.unreachable_block().llbb, + unwind.llbb(), + cleanup_bundle.as_ref()); + } else { + bcx.call(llfn, &args, cleanup_bundle.as_ref()); + bcx.unreachable(); + } + } + mir::TerminatorKind::DropAndReplace { .. } => { bug!("undesugared DropAndReplace in trans: {:?}", data); } diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index 0403c7b1f757b..9498a244e80e7 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -287,14 +287,21 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { })) } - // This is only supported to make bounds checking work. - mir::TerminatorKind::If { ref cond, targets: (true_bb, false_bb) } => { + mir::TerminatorKind::Assert { ref cond, expected, ref msg, target, .. } => { let cond = self.const_operand(cond, span)?; - if common::const_to_uint(cond.llval) != 0 { - true_bb - } else { - false_bb + let cond_bool = common::const_to_uint(cond.llval) != 0; + if cond_bool != expected { + let err = match *msg { + mir::AssertMessage::BoundsCheck {..} => { + ErrKind::IndexOutOfBounds + } + mir::AssertMessage::Math(ref err) => { + ErrKind::Math(err.clone()) + } + }; + consts::const_err(self.ccx, span, Err(err), TrueConst::Yes)?; } + target } mir::TerminatorKind::Call { ref func, ref args, ref destination, .. } => { @@ -308,13 +315,6 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { func, fn_ty) }; - // Indexing OOB doesn't call a const fn, handle it. - if Some(instance.def) == tcx.lang_items.panic_bounds_check_fn() { - consts::const_err(self.ccx, span, - Err(ErrKind::IndexOutOfBounds), - TrueConst::Yes)?; - } - let args = args.iter().map(|arg| { self.const_operand(arg, span) }).collect::, _>>()?; diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index cbd39fffce801..b0567ab26e778 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -17,13 +17,13 @@ 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 adt; use machine; use type_of; +use type_::Type; use tvec; use value::Value; use Disr; @@ -611,9 +611,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) } }; From f1f453cf3b124711b2e20315937403d89913bcd2 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Wed, 25 May 2016 11:55:44 +0300 Subject: [PATCH 06/18] trans: generalize OperandValue::FatPtr to all pairs of immediates. --- src/librustc_trans/common.rs | 58 ++++++++++++++++ src/librustc_trans/mir/block.rs | 107 +++++++++++++++++++---------- src/librustc_trans/mir/constant.rs | 18 +++-- src/librustc_trans/mir/operand.rs | 100 +++++++++++++++++++++------ src/librustc_trans/mir/rvalue.rs | 46 +++++-------- 5 files changed, 238 insertions(+), 91 deletions(-) diff --git a/src/librustc_trans/common.rs b/src/librustc_trans/common.rs index e9b7b590b19a2..884833ca79a2f 100644 --- a/src/librustc_trans/common.rs +++ b/src/librustc_trans/common.rs @@ -39,6 +39,7 @@ use monomorphize; use type_::Type; use value::Value; use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::layout::Layout; use rustc::traits::{self, SelectionContext, ProjectionMode}; use rustc::ty::fold::TypeFoldable; use rustc::hir; @@ -99,6 +100,63 @@ pub fn type_is_immediate<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<'tcx>) - } } +/// Returns Some([a, b]) if the type has a pair of fields with types a and b. +pub fn type_pair_fields<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<'tcx>) + -> Option<[Ty<'tcx>; 2]> { + match ty.sty { + ty::TyEnum(adt, substs) | ty::TyStruct(adt, substs) => { + assert_eq!(adt.variants.len(), 1); + let fields = &adt.variants[0].fields; + if fields.len() != 2 { + return None; + } + Some([monomorphize::field_ty(ccx.tcx(), substs, &fields[0]), + monomorphize::field_ty(ccx.tcx(), substs, &fields[1])]) + } + ty::TyClosure(_, ty::ClosureSubsts { upvar_tys: tys, .. }) | + ty::TyTuple(tys) => { + if tys.len() != 2 { + return None; + } + Some([tys[0], tys[1]]) + } + _ => None + } +} + +/// Returns true if the type is represented as a pair of immediates. +pub fn type_is_imm_pair<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<'tcx>) + -> bool { + let tcx = ccx.tcx(); + let layout = tcx.normalizing_infer_ctxt(ProjectionMode::Any).enter(|infcx| { + match ty.layout(&infcx) { + Ok(layout) => layout, + Err(err) => { + bug!("type_is_imm_pair: layout for `{:?}` failed: {}", + ty, err); + } + } + }); + + match *layout { + Layout::FatPointer { .. } => true, + Layout::Univariant { ref variant, .. } => { + // There must be only 2 fields. + if variant.offset_after_field.len() != 2 { + return false; + } + + match type_pair_fields(ccx, ty) { + Some([a, b]) => { + type_is_immediate(ccx, a) && type_is_immediate(ccx, b) + } + None => false + } + } + _ => false + } +} + /// Identify types which have size zero at runtime. pub fn type_is_zero_size<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool { use machine::llsize_of_alloc; diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index e4d137d36ac37..cf41f5bf2bcbf 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -17,7 +17,7 @@ use adt; use base; use build; use callee::{Callee, CalleeData, Fn, Intrinsic, NamedTupleConstructor, Virtual}; -use common::{self, type_is_fat_ptr, Block, BlockAndBuilder, LandingPad}; +use common::{self, Block, BlockAndBuilder, LandingPad}; use common::{C_bool, C_str_slice, C_struct, C_u32, C_undef}; use consts; use debuginfo::DebugLoc; @@ -36,7 +36,7 @@ use super::analyze::CleanupKind; use super::constant::Const; use super::lvalue::{LvalueRef, load_fat_ptr}; use super::operand::OperandRef; -use super::operand::OperandValue::{self, FatPtr, Immediate, Ref}; +use super::operand::OperandValue::*; impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { pub fn trans_block(&mut self, bb: mir::BasicBlock) { @@ -410,8 +410,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } } - let val = self.trans_operand(&bcx, arg).val; - self.trans_argument(&bcx, val, &mut llargs, &fn_ty, + let op = self.trans_operand(&bcx, arg); + self.trans_argument(&bcx, op, &mut llargs, &fn_ty, &mut idx, &mut callee.data); } if let Some(tup) = untuple { @@ -449,7 +449,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { if let ReturnDest::IndirectOperand(dst, _) = ret_dest { // Make a fake operand for store_return let op = OperandRef { - val: OperandValue::Ref(dst), + val: Ref(dst), ty: sig.output.unwrap() }; self.store_return(&bcx, ret_dest, fn_ty.ret, op); @@ -487,7 +487,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { ret_bcx.at_start(|ret_bcx| { debug_loc.apply_to_bcx(ret_bcx); let op = OperandRef { - val: OperandValue::Immediate(invokeret), + val: Immediate(invokeret), ty: sig.output.unwrap() }; self.store_return(&ret_bcx, ret_dest, fn_ty.ret, op); @@ -498,7 +498,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { fn_ty.apply_attrs_callsite(llret); if let Some((_, target)) = *destination { let op = OperandRef { - val: OperandValue::Immediate(llret), + val: Immediate(llret), ty: sig.output.unwrap() }; self.store_return(&bcx, ret_dest, fn_ty.ret, op); @@ -513,25 +513,36 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { fn trans_argument(&mut self, bcx: &BlockAndBuilder<'bcx, 'tcx>, - val: OperandValue, + mut op: OperandRef<'tcx>, llargs: &mut Vec, fn_ty: &FnType, next_idx: &mut usize, callee: &mut CalleeData) { - // Treat the values in a fat pointer separately. - if let FatPtr(ptr, meta) = val { - if *next_idx == 0 { - if let Virtual(idx) = *callee { - let llfn = bcx.with_block(|bcx| { - meth::get_virtual_method(bcx, meta, idx) - }); - let llty = fn_ty.llvm_type(bcx.ccx()).ptr_to(); - *callee = Fn(bcx.pointercast(llfn, llty)); + if let Pair(a, b) = op.val { + // Treat the values in a fat pointer separately. + if common::type_is_fat_ptr(bcx.tcx(), op.ty) { + let (ptr, meta) = (a, b); + if *next_idx == 0 { + if let Virtual(idx) = *callee { + let llfn = bcx.with_block(|bcx| { + meth::get_virtual_method(bcx, meta, idx) + }); + let llty = fn_ty.llvm_type(bcx.ccx()).ptr_to(); + *callee = Fn(bcx.pointercast(llfn, llty)); + } } + + let imm_op = |x| OperandRef { + val: Immediate(x), + // We won't be checking the type again. + ty: bcx.tcx().types.err + }; + self.trans_argument(bcx, imm_op(ptr), llargs, fn_ty, next_idx, callee); + self.trans_argument(bcx, imm_op(meta), llargs, fn_ty, next_idx, callee); + return; } - self.trans_argument(bcx, Immediate(ptr), llargs, fn_ty, next_idx, callee); - self.trans_argument(bcx, Immediate(meta), llargs, fn_ty, next_idx, callee); - return; + + op = op.pack_if_pair(bcx); } let arg = &fn_ty.args[*next_idx]; @@ -547,7 +558,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } // Force by-ref if we have to load through a cast pointer. - let (mut llval, by_ref) = match val { + let (mut llval, by_ref) = match op.val { Immediate(llval) if arg.is_indirect() || arg.cast.is_some() => { let llscratch = build::AllocaFcx(bcx.fcx(), arg.original_ty, "arg"); bcx.store(llval, llscratch); @@ -555,7 +566,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } Immediate(llval) => (llval, false), Ref(llval) => (llval, true), - FatPtr(_, _) => bug!("fat pointers handled above") + Pair(..) => bug!("pairs handled above") }; if by_ref && !arg.is_indirect() { @@ -602,12 +613,16 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { let ptr = adt::trans_field_ptr_builder(bcx, &base_repr, base, Disr(0), n); let val = if common::type_is_fat_ptr(bcx.tcx(), ty) { let (lldata, llextra) = load_fat_ptr(bcx, ptr); - FatPtr(lldata, llextra) + Pair(lldata, llextra) } else { // trans_argument will load this if it needs to Ref(ptr) }; - self.trans_argument(bcx, val, llargs, fn_ty, next_idx, callee); + let op = OperandRef { + val: val, + ty: ty + }; + self.trans_argument(bcx, op, llargs, fn_ty, next_idx, callee); } } @@ -619,11 +634,29 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { elem = bcx.trunc(elem, Type::i1(bcx.ccx())); } // If the tuple is immediate, the elements are as well - let val = Immediate(elem); - self.trans_argument(bcx, val, llargs, fn_ty, next_idx, callee); + let op = OperandRef { + val: Immediate(elem), + ty: ty + }; + self.trans_argument(bcx, op, llargs, fn_ty, next_idx, callee); + } + } + Pair(a, b) => { + let elems = [a, b]; + for (n, &ty) in arg_types.iter().enumerate() { + let mut elem = elems[n]; + // Truncate bools to i1, if needed + if ty.is_bool() && common::val_ty(elem) != Type::i1(bcx.ccx()) { + elem = bcx.trunc(elem, Type::i1(bcx.ccx())); + } + // Pair is always made up of immediates + let op = OperandRef { + val: Immediate(elem), + ty: ty + }; + self.trans_argument(bcx, op, llargs, fn_ty, next_idx, callee); } } - FatPtr(_, _) => bug!("tuple is a fat pointer?!") } } @@ -779,7 +812,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { let f = Callee::def(bcx.ccx(), def_id, substs); let datum = f.reify(bcx.ccx()); val = OperandRef { - val: OperandValue::Immediate(datum.val), + val: Immediate(datum.val), ty: datum.ty }; } @@ -806,17 +839,15 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { self.temps[idx as usize] = TempRef::Operand(Some(op)); } DirectOperand(idx) => { - let op = if type_is_fat_ptr(bcx.tcx(), op.ty) { - let llval = op.immediate(); - let ptr = bcx.extract_value(llval, 0); - let meta = bcx.extract_value(llval, 1); - - OperandRef { - val: OperandValue::FatPtr(ptr, meta), - ty: op.ty - } + // If there is a cast, we have to store and reload. + let op = if ret_ty.cast.is_some() { + let tmp = bcx.with_block(|bcx| { + base::alloc_ty(bcx, op.ty, "tmp_ret") + }); + ret_ty.store(bcx, op.immediate(), tmp); + self.trans_load(bcx, tmp, op.ty) } else { - op + op.unpack_if_pair(bcx) }; self.temps[idx as usize] = TempRef::Operand(Some(op)); } diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index 9498a244e80e7..e73d02a1e29f1 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -98,9 +98,15 @@ impl<'tcx> Const<'tcx> { Const::new(val, ty) } + fn get_pair(&self) -> (ValueRef, ValueRef) { + (const_get_elt(self.llval, &[0]), + const_get_elt(self.llval, &[1])) + } + fn get_fat_ptr(&self) -> (ValueRef, ValueRef) { - (const_get_elt(self.llval, &[abi::FAT_PTR_ADDR as u32]), - const_get_elt(self.llval, &[abi::FAT_PTR_EXTRA as u32])) + assert_eq!(abi::FAT_PTR_ADDR, 0); + assert_eq!(abi::FAT_PTR_EXTRA, 1); + self.get_pair() } fn as_lvalue(&self) -> ConstLvalue<'tcx> { @@ -115,9 +121,9 @@ impl<'tcx> Const<'tcx> { let llty = type_of::immediate_type_of(ccx, self.ty); let llvalty = val_ty(self.llval); - let val = if common::type_is_fat_ptr(ccx.tcx(), self.ty) { - let (data, extra) = self.get_fat_ptr(); - OperandValue::FatPtr(data, extra) + let val = if common::type_is_imm_pair(ccx, self.ty) { + let (a, b) = self.get_pair(); + OperandValue::Pair(a, b) } else if common::type_is_immediate(ccx, self.ty) && llty == llvalty { // If the types match, we can use the value directly. OperandValue::Immediate(self.llval) @@ -656,7 +662,7 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { consts::ptrcast(data_ptr, ll_cast_ty) } } else { - bug!("Unexpected non-FatPtr operand") + bug!("Unexpected non-fat-pointer operand") } } }; diff --git a/src/librustc_trans/mir/operand.rs b/src/librustc_trans/mir/operand.rs index 107ec1159f010..9e04f1cb207b3 100644 --- a/src/librustc_trans/mir/operand.rs +++ b/src/librustc_trans/mir/operand.rs @@ -13,12 +13,12 @@ use rustc::ty::Ty; use rustc::mir::repr as mir; use base; use common::{self, Block, BlockAndBuilder}; -use datum; use value::Value; +use type_of; +use type_::Type; use std::fmt; -use super::lvalue::load_fat_ptr; use super::{MirContext, TempRef}; /// The representation of a Rust value. The enum variant is in fact @@ -31,9 +31,8 @@ pub enum OperandValue { Ref(ValueRef), /// A single LLVM value. Immediate(ValueRef), - /// A fat pointer. The first ValueRef is the data and the second - /// is the extra. - FatPtr(ValueRef, ValueRef) + /// A pair of immediate LLVM values. Used by fat pointers too. + Pair(ValueRef, ValueRef) } /// An `OperandRef` is an "SSA" reference to a Rust value, along with @@ -64,15 +63,15 @@ impl<'tcx> fmt::Debug for OperandRef<'tcx> { write!(f, "OperandRef(Immediate({:?}) @ {:?})", Value(i), self.ty) } - OperandValue::FatPtr(a, d) => { - write!(f, "OperandRef(FatPtr({:?}, {:?}) @ {:?})", - Value(a), Value(d), self.ty) + OperandValue::Pair(a, b) => { + write!(f, "OperandRef(Pair({:?}, {:?}) @ {:?})", + Value(a), Value(b), self.ty) } } } } -impl<'tcx> OperandRef<'tcx> { +impl<'bcx, 'tcx> OperandRef<'tcx> { /// Asserts that this operand refers to a scalar and returns /// a reference to its value. pub fn immediate(self) -> ValueRef { @@ -81,6 +80,54 @@ impl<'tcx> OperandRef<'tcx> { _ => bug!() } } + + /// If this operand is a Pair, we return an + /// Immediate aggregate with the two values. + pub fn pack_if_pair(mut self, bcx: &BlockAndBuilder<'bcx, 'tcx>) + -> OperandRef<'tcx> { + if let OperandValue::Pair(a, b) = self.val { + // Reconstruct the immediate aggregate. + let llty = type_of::type_of(bcx.ccx(), self.ty); + let mut llpair = common::C_undef(llty); + let elems = [a, b]; + for i in 0..2 { + let mut elem = elems[i]; + // Extend boolean i1's to i8. + if common::val_ty(elem) == Type::i1(bcx.ccx()) { + elem = bcx.zext(elem, Type::i8(bcx.ccx())); + } + llpair = bcx.insert_value(llpair, elem, i); + } + self.val = OperandValue::Immediate(llpair); + } + self + } + + /// If this operand is a pair in an Immediate, + /// we return a Pair with the two halves. + pub fn unpack_if_pair(mut self, bcx: &BlockAndBuilder<'bcx, 'tcx>) + -> OperandRef<'tcx> { + if let OperandValue::Immediate(llval) = self.val { + // Deconstruct the immediate aggregate. + if common::type_is_imm_pair(bcx.ccx(), self.ty) { + let mut a = bcx.extract_value(llval, 0); + let mut b = bcx.extract_value(llval, 1); + + let pair_fields = common::type_pair_fields(bcx.ccx(), self.ty); + if let Some([a_ty, b_ty]) = pair_fields { + if a_ty.is_bool() { + a = bcx.trunc(a, Type::i1(bcx.ccx())); + } + if b_ty.is_bool() { + b = bcx.trunc(b, Type::i1(bcx.ccx())); + } + } + + self.val = OperandValue::Pair(a, b); + } + } + self + } } impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { @@ -92,15 +139,24 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { { debug!("trans_load: {:?} @ {:?}", Value(llval), ty); - let val = match datum::appropriate_rvalue_mode(bcx.ccx(), ty) { - datum::ByValue => { - OperandValue::Immediate(base::load_ty_builder(bcx, llval, ty)) - } - datum::ByRef if common::type_is_fat_ptr(bcx.tcx(), ty) => { - let (lldata, llextra) = load_fat_ptr(bcx, llval); - OperandValue::FatPtr(lldata, llextra) - } - datum::ByRef => OperandValue::Ref(llval) + let val = if common::type_is_imm_pair(bcx.ccx(), ty) { + let a_ptr = bcx.struct_gep(llval, 0); + let b_ptr = bcx.struct_gep(llval, 1); + + // This is None only for fat pointers, which don't + // need any special load-time behavior anyway. + let pair_fields = common::type_pair_fields(bcx.ccx(), ty); + let (a, b) = if let Some([a_ty, b_ty]) = pair_fields { + (base::load_ty_builder(bcx, a_ptr, a_ty), + base::load_ty_builder(bcx, b_ptr, b_ty)) + } else { + (bcx.load(a_ptr), bcx.load(b_ptr)) + }; + OperandValue::Pair(a, b) + } else if common::type_is_immediate(bcx.ccx(), ty) { + OperandValue::Immediate(base::load_ty_builder(bcx, llval, ty)) + } else { + OperandValue::Ref(llval) }; OperandRef { val: val, ty: ty } @@ -173,8 +229,12 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { match operand.val { OperandValue::Ref(r) => base::memcpy_ty(bcx, lldest, r, operand.ty), OperandValue::Immediate(s) => base::store_ty(bcx, s, lldest, operand.ty), - OperandValue::FatPtr(data, extra) => { - base::store_fat_ptr(bcx, data, extra, lldest, operand.ty); + OperandValue::Pair(a, b) => { + use build::*; + let a = base::from_immediate(bcx, a); + let b = base::from_immediate(bcx, b); + Store(bcx, a, StructGEP(bcx, lldest, 0)); + Store(bcx, b, StructGEP(bcx, lldest, 1)); } } } diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index b0567ab26e778..d019677d0e691 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -16,14 +16,12 @@ use rustc::mir::repr as mir; use asm; use base; use callee::Callee; -use common::{self, val_ty, - C_null, C_uint, C_undef, BlockAndBuilder, Result}; +use common::{self, val_ty, C_null, C_uint, BlockAndBuilder, Result}; use datum::{Datum, Lvalue}; use debuginfo::DebugLoc; use adt; use machine; use type_of; -use type_::Type; use tvec; use value::Value; use Disr; @@ -68,9 +66,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { // `CoerceUnsized` can be passed by a where-clause, // so the (generic) MIR may not be able to expand it. let operand = self.trans_operand(&bcx, source); + let operand = operand.pack_if_pair(&bcx); bcx.with_block(|bcx| { match operand.val { - OperandValue::FatPtr(..) => bug!(), + OperandValue::Pair(..) => bug!(), OperandValue::Immediate(llval) => { // unsize from an immediate structure. We don't // really need a temporary alloca here, but @@ -255,7 +254,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { assert!(common::type_is_fat_ptr(bcx.tcx(), cast_ty)); match operand.val { - OperandValue::FatPtr(lldata, llextra) => { + OperandValue::Pair(lldata, llextra) => { // unsize from a fat pointer - this is a // "trait-object-to-supertrait" coercion, for // example, @@ -264,7 +263,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { // the types match up. let llcast_ty = type_of::fat_ptr_base_ty(bcx.ccx(), cast_ty); let lldata = bcx.pointercast(lldata, llcast_ty); - OperandValue::FatPtr(lldata, llextra) + OperandValue::Pair(lldata, llextra) } OperandValue::Immediate(lldata) => { // "standard" unsize @@ -272,7 +271,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { base::unsize_thin_ptr(bcx, lldata, operand.ty, cast_ty) }); - OperandValue::FatPtr(lldata, llextra) + OperandValue::Pair(lldata, llextra) } OperandValue::Ref(_) => { bug!("by-ref operand {:?} in trans_rvalue_operand", @@ -343,13 +342,13 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { mir::CastKind::Misc => { // Casts from a fat-ptr. let ll_cast_ty = type_of::immediate_type_of(bcx.ccx(), cast_ty); let ll_from_ty = type_of::immediate_type_of(bcx.ccx(), operand.ty); - if let OperandValue::FatPtr(data_ptr, meta_ptr) = operand.val { + if let OperandValue::Pair(data_ptr, meta_ptr) = operand.val { if common::type_is_fat_ptr(bcx.tcx(), cast_ty) { let ll_cft = ll_cast_ty.field_types(); let ll_fft = ll_from_ty.field_types(); let data_cast = bcx.pointercast(data_ptr, ll_cft[0]); assert_eq!(ll_cft[1].kind(), ll_fft[1].kind()); - OperandValue::FatPtr(data_cast, meta_ptr) + OperandValue::Pair(data_cast, meta_ptr) } else { // cast to thin-ptr // Cast of fat-ptr to thin-ptr is an extraction of data-ptr and // pointer-cast of that pointer to desired pointer type. @@ -357,7 +356,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { OperandValue::Immediate(llval) } } else { - bug!("Unexpected non-FatPtr operand") + bug!("Unexpected non-Pair operand") } } }; @@ -386,8 +385,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } } else { OperandRef { - val: OperandValue::FatPtr(tr_lvalue.llval, - tr_lvalue.llextra), + val: OperandValue::Pair(tr_lvalue.llval, + tr_lvalue.llextra), ty: ref_ty, } }; @@ -408,8 +407,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { let rhs = self.trans_operand(&bcx, rhs); let llresult = if common::type_is_fat_ptr(bcx.tcx(), lhs.ty) { match (lhs.val, rhs.val) { - (OperandValue::FatPtr(lhs_addr, lhs_extra), - OperandValue::FatPtr(rhs_addr, rhs_extra)) => { + (OperandValue::Pair(lhs_addr, lhs_extra), + OperandValue::Pair(rhs_addr, rhs_extra)) => { bcx.with_block(|bcx| { base::compare_fat_ptrs(bcx, lhs_addr, lhs_extra, @@ -441,7 +440,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { 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), + val: result, ty: operand_ty }; @@ -579,7 +578,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { op: mir::BinOp, lhs: ValueRef, rhs: ValueRef, - input_ty: Ty<'tcx>) -> ValueRef { + input_ty: Ty<'tcx>) -> OperandValue { let (val, of) = match op { // These are checked using intrinsics mir::BinOp::Add | mir::BinOp::Sub | mir::BinOp::Mul => { @@ -592,10 +591,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { 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()))) + (bcx.extract_value(res, 0), + bcx.extract_value(res, 1)) } mir::BinOp::Shl | mir::BinOp::Shr => { let lhs_llty = val_ty(lhs); @@ -608,19 +605,14 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { 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()))) + (val, of) } _ => { bug!("Operator `{:?}` is not a checkable operator", op) } }; - 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) + OperandValue::Pair(val, of) } } From cab35ff4b80c5fd67280aa1f8ed69c1ba1930fdb Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Wed, 25 May 2016 11:58:08 +0300 Subject: [PATCH 07/18] trans: support uses of projections of immediate pairs. --- src/librustc_trans/mir/analyze.rs | 17 +++++++++++++++++ src/librustc_trans/mir/operand.rs | 20 ++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/librustc_trans/mir/analyze.rs b/src/librustc_trans/mir/analyze.rs index bce639ac8f76e..59143bc01bf7a 100644 --- a/src/librustc_trans/mir/analyze.rs +++ b/src/librustc_trans/mir/analyze.rs @@ -39,6 +39,8 @@ pub fn lvalue_temps<'bcx,'tcx>(bcx: Block<'bcx,'tcx>, // in an ValueRef without an alloca. assert!(common::type_is_immediate(bcx.ccx(), ty) || common::type_is_fat_ptr(bcx.tcx(), ty)); + } else if common::type_is_imm_pair(bcx.ccx(), ty) { + // We allow pairs and uses of any of their 2 fields. } else { // These sorts of types require an alloca. Note that // type_is_immediate() may *still* be true, particularly @@ -111,6 +113,21 @@ impl<'mir, 'bcx, 'tcx> Visitor<'tcx> for TempAnalyzer<'mir, 'bcx, 'tcx> { context: LvalueContext) { debug!("visit_lvalue(lvalue={:?}, context={:?})", lvalue, context); + // Allow uses of projections of immediate pair fields. + if let mir::Lvalue::Projection(ref proj) = *lvalue { + if let mir::Lvalue::Temp(index) = proj.base { + let ty = self.mir.temp_decls[index as usize].ty; + let ty = self.bcx.monomorphize(&ty); + if common::type_is_imm_pair(self.bcx.ccx(), ty) { + if let mir::ProjectionElem::Field(..) = proj.elem { + if let LvalueContext::Consume = context { + return; + } + } + } + } + } + match *lvalue { mir::Lvalue::Temp(index) => { match context { diff --git a/src/librustc_trans/mir/operand.rs b/src/librustc_trans/mir/operand.rs index 9e04f1cb207b3..c21f112b5f603 100644 --- a/src/librustc_trans/mir/operand.rs +++ b/src/librustc_trans/mir/operand.rs @@ -187,6 +187,26 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } } + // Moves out of pair fields are trivial. + if let &mir::Lvalue::Projection(ref proj) = lvalue { + if let mir::Lvalue::Temp(index) = proj.base { + let temp_ref = &self.temps[index as usize]; + if let &TempRef::Operand(Some(o)) = temp_ref { + match (o.val, &proj.elem) { + (OperandValue::Pair(a, b), + &mir::ProjectionElem::Field(ref f, ty)) => { + let llval = [a, b][f.index()]; + return OperandRef { + val: OperandValue::Immediate(llval), + ty: bcx.monomorphize(&ty) + }; + } + _ => {} + } + } + } + } + // for most lvalues, to consume them we just load them // out from their home let tr_lvalue = self.trans_lvalue(bcx, lvalue); From 4adc967ed168d5469e39267d4ac81383434830b4 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Thu, 26 May 2016 15:42:29 +0300 Subject: [PATCH 08/18] mir: report when overflow checks would be missing cross-crate. --- src/librustc_mir/build/expr/as_rvalue.rs | 4 +-- src/librustc_mir/build/mod.rs | 21 ++++++++++--- src/librustc_mir/hair/cx/mod.rs | 39 ++++++++++++++++++++++-- src/librustc_mir/mir_map.rs | 16 +--------- 4 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index 40b2fa61c133b..bb7a628f3ba50 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -80,7 +80,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { 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() { + if op == UnOp::Neg && expr.ty.is_signed() && this.check_overflow() { let bool_ty = this.hir.bool_ty(); let minval = this.minval_literal(expr_span, expr.ty); @@ -247,7 +247,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, '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() { + if op.is_checkable() && ty.is_integral() && self.check_overflow() { let result_tup = self.hir.tcx().mk_tup(vec![ty, bool_ty]); let result_value = self.temp(result_tup); diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 3faf95331ddab..f3b1a87138859 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -55,6 +55,8 @@ pub struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { cached_resume_block: Option, /// cached block with the RETURN terminator cached_return_block: Option, + + has_warned_about_xcrate_overflows: bool } struct CFG<'tcx> { @@ -273,7 +275,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { var_indices: FnvHashMap(), unit_temp: None, cached_resume_block: None, - cached_return_block: None + cached_return_block: None, + has_warned_about_xcrate_overflows: false }; assert_eq!(builder.cfg.start_new_block(), START_BLOCK); @@ -379,9 +382,19 @@ 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) + pub fn check_overflow(&mut self) -> bool { + let check = self.hir.tcx().sess.opts.debugging_opts.force_overflow_checks + .unwrap_or(self.hir.tcx().sess.opts.debug_assertions); + + if !check && self.hir.may_be_inlined_cross_crate() { + if !self.has_warned_about_xcrate_overflows { + self.hir.tcx().sess.span_warn(self.fn_span, + "overflow checks would be missing when used from another crate"); + self.has_warned_about_xcrate_overflows = true; + } + } + + check } } diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs index fad6cfb7ae1aa..659a326e852df 100644 --- a/src/librustc_mir/hair/cx/mod.rs +++ b/src/librustc_mir/hair/cx/mod.rs @@ -17,32 +17,63 @@ use hair::*; use rustc::mir::repr::*; +use rustc::mir::transform::MirSource; use rustc::middle::const_val::ConstVal; use rustc_const_eval as const_eval; use rustc::hir::def_id::DefId; +use rustc::hir::intravisit::FnKind; +use rustc::hir::map::blocks::FnLikeNode; use rustc::infer::InferCtxt; use rustc::ty::subst::{Subst, Substs}; use rustc::ty::{self, Ty, TyCtxt}; use syntax::parse::token; use rustc::hir; use rustc_const_math::{ConstInt, ConstUsize}; +use syntax::attr; #[derive(Copy, Clone)] pub struct Cx<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { tcx: TyCtxt<'a, 'gcx, 'tcx>, infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, - constness: hir::Constness + constness: hir::Constness, + + /// True if this MIR can get inlined in other crates. + inline: bool } impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { pub fn new(infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, - constness: hir::Constness) + src: MirSource) -> Cx<'a, 'gcx, 'tcx> { + let (constness, inline) = match src { + MirSource::Const(_) | + MirSource::Static(..) => (hir::Constness::Const, false), + MirSource::Fn(id) => { + let def_id = infcx.tcx.map.local_def_id(id); + let fn_like = FnLikeNode::from_node(infcx.tcx.map.get(id)); + match fn_like.map(|f| f.kind()) { + Some(FnKind::ItemFn(_, _, _, c, _, _, attrs)) => { + let scheme = infcx.tcx.lookup_item_type(def_id); + let any_types = !scheme.generics.types.is_empty(); + (c, any_types || attr::requests_inline(attrs)) + } + Some(FnKind::Method(_, m, _, attrs)) => { + let scheme = infcx.tcx.lookup_item_type(def_id); + let any_types = !scheme.generics.types.is_empty(); + (m.constness, any_types || attr::requests_inline(attrs)) + } + _ => (hir::Constness::NotConst, true) + } + } + MirSource::Promoted(..) => bug!() + }; + Cx { tcx: infcx.tcx, infcx: infcx, constness: constness, + inline: inline } } } @@ -154,6 +185,10 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { pub fn tcx(&self) -> TyCtxt<'a, 'gcx, 'tcx> { self.tcx } + + pub fn may_be_inlined_cross_crate(&self) -> bool { + self.inline + } } mod block; diff --git a/src/librustc_mir/mir_map.rs b/src/librustc_mir/mir_map.rs index 73cfdeda74a88..a55fbe3641c66 100644 --- a/src/librustc_mir/mir_map.rs +++ b/src/librustc_mir/mir_map.rs @@ -32,7 +32,6 @@ use rustc::ty::subst::Substs; use rustc::util::nodemap::NodeMap; use rustc::hir; use rustc::hir::intravisit::{self, FnKind, Visitor}; -use rustc::hir::map::blocks::FnLikeNode; use syntax::ast; use syntax::codemap::Span; @@ -116,20 +115,7 @@ impl<'a, 'gcx, 'tcx> CxBuilder<'a, 'gcx, 'tcx> { { let src = self.src; let mir = self.infcx.enter(|infcx| { - let constness = match src { - MirSource::Const(_) | - MirSource::Static(..) => hir::Constness::Const, - MirSource::Fn(id) => { - let fn_like = FnLikeNode::from_node(infcx.tcx.map.get(id)); - match fn_like.map(|f| f.kind()) { - Some(FnKind::ItemFn(_, _, _, c, _, _, _)) => c, - Some(FnKind::Method(_, m, _, _)) => m.constness, - _ => hir::Constness::NotConst - } - } - MirSource::Promoted(..) => bug!() - }; - let (mut mir, scope_auxiliary) = f(Cx::new(&infcx, constness)); + let (mut mir, scope_auxiliary) = f(Cx::new(&infcx, src)); // Convert the Mir to global types. let mut globalizer = GlobalizeMir { From 702c47baae8e417d5ca377acb886893e902f2afa Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Thu, 26 May 2016 19:02:26 +0300 Subject: [PATCH 09/18] core: mark relevant functions with #[rustc_inherit_overflow_checks]. --- src/libcore/iter/iterator.rs | 1 + src/libcore/iter/mod.rs | 4 ++++ src/libcore/num/mod.rs | 6 +++--- src/libcore/ops.rs | 11 +++++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/libcore/iter/iterator.rs b/src/libcore/iter/iterator.rs index b80f77c0d25a9..71ca5ccdc8dfb 100644 --- a/src/libcore/iter/iterator.rs +++ b/src/libcore/iter/iterator.rs @@ -172,6 +172,7 @@ pub trait Iterator { /// assert_eq!(a.iter().count(), 5); /// ``` #[inline] + #[rustc_inherit_overflow_checks] #[stable(feature = "rust1", since = "1.0.0")] fn count(self) -> usize where Self: Sized { // Might overflow. diff --git a/src/libcore/iter/mod.rs b/src/libcore/iter/mod.rs index f964527b4b41f..ae1e311682617 100644 --- a/src/libcore/iter/mod.rs +++ b/src/libcore/iter/mod.rs @@ -510,6 +510,7 @@ impl Iterator for Chain where } #[inline] + #[rustc_inherit_overflow_checks] fn count(self) -> usize { match self.state { ChainState::Both => self.a.count() + self.b.count(), @@ -932,6 +933,7 @@ impl Iterator for Enumerate where I: Iterator { /// /// Might panic if the index of the element overflows a `usize`. #[inline] + #[rustc_inherit_overflow_checks] fn next(&mut self) -> Option<(usize, ::Item)> { self.iter.next().map(|a| { let ret = (self.count, a); @@ -947,6 +949,7 @@ impl Iterator for Enumerate where I: Iterator { } #[inline] + #[rustc_inherit_overflow_checks] fn nth(&mut self, n: usize) -> Option<(usize, I::Item)> { self.iter.nth(n).map(|a| { let i = self.count + n; @@ -1008,6 +1011,7 @@ impl Iterator for Peekable { } #[inline] + #[rustc_inherit_overflow_checks] fn count(self) -> usize { (if self.peeked.is_some() { 1 } else { 0 }) + self.iter.count() } diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index 5988a6375d44e..883e9206dde1d 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -1033,7 +1033,7 @@ macro_rules! int_impl { /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[inline] - #[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. + #[rustc_inherit_overflow_checks] pub fn pow(self, mut exp: u32) -> Self { let mut base = self; let mut acc = Self::one(); @@ -1075,7 +1075,7 @@ macro_rules! int_impl { /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[inline] - #[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. + #[rustc_inherit_overflow_checks] pub fn abs(self) -> Self { if self.is_negative() { // Note that the #[inline] above means that the overflow @@ -2061,7 +2061,7 @@ macro_rules! uint_impl { /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[inline] - #[rustc_no_mir] // FIXME #29769 MIR overflow checking is TBD. + #[rustc_inherit_overflow_checks] pub fn pow(self, mut exp: u32) -> Self { let mut base = self; let mut acc = Self::one(); diff --git a/src/libcore/ops.rs b/src/libcore/ops.rs index a2f84230afc44..50c4dc697c206 100644 --- a/src/libcore/ops.rs +++ b/src/libcore/ops.rs @@ -208,6 +208,7 @@ macro_rules! add_impl { type Output = $t; #[inline] + #[rustc_inherit_overflow_checks] fn add(self, other: $t) -> $t { self + other } } @@ -261,6 +262,7 @@ macro_rules! sub_impl { type Output = $t; #[inline] + #[rustc_inherit_overflow_checks] fn sub(self, other: $t) -> $t { self - other } } @@ -314,6 +316,7 @@ macro_rules! mul_impl { type Output = $t; #[inline] + #[rustc_inherit_overflow_checks] fn mul(self, other: $t) -> $t { self * other } } @@ -511,6 +514,7 @@ macro_rules! neg_impl_core { type Output = $t; #[inline] + #[rustc_inherit_overflow_checks] fn neg(self) -> $t { let $id = self; $body } } @@ -788,6 +792,7 @@ macro_rules! shl_impl { type Output = $t; #[inline] + #[rustc_inherit_overflow_checks] fn shl(self, other: $f) -> $t { self << other } @@ -859,6 +864,7 @@ macro_rules! shr_impl { type Output = $t; #[inline] + #[rustc_inherit_overflow_checks] fn shr(self, other: $f) -> $t { self >> other } @@ -923,6 +929,7 @@ macro_rules! add_assign_impl { #[stable(feature = "op_assign_traits", since = "1.8.0")] impl AddAssign for $t { #[inline] + #[rustc_inherit_overflow_checks] fn add_assign(&mut self, other: $t) { *self += other } } )+) @@ -967,6 +974,7 @@ macro_rules! sub_assign_impl { #[stable(feature = "op_assign_traits", since = "1.8.0")] impl SubAssign for $t { #[inline] + #[rustc_inherit_overflow_checks] fn sub_assign(&mut self, other: $t) { *self -= other } } )+) @@ -1011,6 +1019,7 @@ macro_rules! mul_assign_impl { #[stable(feature = "op_assign_traits", since = "1.8.0")] impl MulAssign for $t { #[inline] + #[rustc_inherit_overflow_checks] fn mul_assign(&mut self, other: $t) { *self *= other } } )+) @@ -1275,6 +1284,7 @@ macro_rules! shl_assign_impl { #[stable(feature = "op_assign_traits", since = "1.8.0")] impl ShlAssign<$f> for $t { #[inline] + #[rustc_inherit_overflow_checks] fn shl_assign(&mut self, other: $f) { *self <<= other } @@ -1337,6 +1347,7 @@ macro_rules! shr_assign_impl { #[stable(feature = "op_assign_traits", since = "1.8.0")] impl ShrAssign<$f> for $t { #[inline] + #[rustc_inherit_overflow_checks] fn shr_assign(&mut self, other: $f) { *self >>= other } From d8dddbf2014f92cadfbf14e75a71c345a9e12498 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Thu, 26 May 2016 20:02:56 +0300 Subject: [PATCH 10/18] Respect #[rustc_inherit_overflow_checks] in mir::build and trans. --- src/librustc_mir/build/expr/as_rvalue.rs | 4 +- src/librustc_mir/build/mod.rs | 20 +--------- src/librustc_mir/hair/cx/mod.rs | 47 ++++++++++++++---------- src/librustc_trans/mir/rvalue.rs | 11 +++++- src/libsyntax/feature_gate.rs | 7 ++++ 5 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index bb7a628f3ba50..67cf5473f79c0 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -80,7 +80,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ExprKind::Unary { op, arg } => { let arg = unpack!(block = this.as_operand(block, arg)); // Check for -MIN on signed integers - if op == UnOp::Neg && expr.ty.is_signed() && this.check_overflow() { + if this.hir.check_overflow() && op == UnOp::Neg && expr.ty.is_signed() { let bool_ty = this.hir.bool_ty(); let minval = this.minval_literal(expr_span, expr.ty); @@ -247,7 +247,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { lhs: Operand<'tcx>, rhs: Operand<'tcx>) -> BlockAnd> { let scope_id = self.innermost_scope_id(); let bool_ty = self.hir.bool_ty(); - if op.is_checkable() && ty.is_integral() && self.check_overflow() { + if self.hir.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); diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index f3b1a87138859..9d7818a9ba4d6 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -55,8 +55,6 @@ pub struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { cached_resume_block: Option, /// cached block with the RETURN terminator cached_return_block: Option, - - has_warned_about_xcrate_overflows: bool } struct CFG<'tcx> { @@ -275,8 +273,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { var_indices: FnvHashMap(), unit_temp: None, cached_resume_block: None, - cached_return_block: None, - has_warned_about_xcrate_overflows: false + cached_return_block: None }; assert_eq!(builder.cfg.start_new_block(), START_BLOCK); @@ -381,21 +378,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } } - - pub fn check_overflow(&mut self) -> bool { - let check = self.hir.tcx().sess.opts.debugging_opts.force_overflow_checks - .unwrap_or(self.hir.tcx().sess.opts.debug_assertions); - - if !check && self.hir.may_be_inlined_cross_crate() { - if !self.has_warned_about_xcrate_overflows { - self.hir.tcx().sess.span_warn(self.fn_span, - "overflow checks would be missing when used from another crate"); - self.has_warned_about_xcrate_overflows = true; - } - } - - check - } } /////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs index 659a326e852df..25860ae7ef1ee 100644 --- a/src/librustc_mir/hair/cx/mod.rs +++ b/src/librustc_mir/hair/cx/mod.rs @@ -30,7 +30,7 @@ use rustc::ty::{self, Ty, TyCtxt}; use syntax::parse::token; use rustc::hir; use rustc_const_math::{ConstInt, ConstUsize}; -use syntax::attr; +use syntax::attr::AttrMetaMethods; #[derive(Copy, Clone)] pub struct Cx<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { @@ -38,42 +38,49 @@ pub struct Cx<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, constness: hir::Constness, - /// True if this MIR can get inlined in other crates. - inline: bool + /// True if this constant/function needs overflow checks. + check_overflow: bool } impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { pub fn new(infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, src: MirSource) -> Cx<'a, 'gcx, 'tcx> { - let (constness, inline) = match src { + let constness = match src { MirSource::Const(_) | - MirSource::Static(..) => (hir::Constness::Const, false), + MirSource::Static(..) => hir::Constness::Const, MirSource::Fn(id) => { - let def_id = infcx.tcx.map.local_def_id(id); let fn_like = FnLikeNode::from_node(infcx.tcx.map.get(id)); match fn_like.map(|f| f.kind()) { - Some(FnKind::ItemFn(_, _, _, c, _, _, attrs)) => { - let scheme = infcx.tcx.lookup_item_type(def_id); - let any_types = !scheme.generics.types.is_empty(); - (c, any_types || attr::requests_inline(attrs)) - } - Some(FnKind::Method(_, m, _, attrs)) => { - let scheme = infcx.tcx.lookup_item_type(def_id); - let any_types = !scheme.generics.types.is_empty(); - (m.constness, any_types || attr::requests_inline(attrs)) - } - _ => (hir::Constness::NotConst, true) + Some(FnKind::ItemFn(_, _, _, c, _, _, _)) => c, + Some(FnKind::Method(_, m, _, _)) => m.constness, + _ => hir::Constness::NotConst } } MirSource::Promoted(..) => bug!() }; + let attrs = infcx.tcx.map.attrs(src.item_id()); + + // Some functions always have overflow checks enabled, + // however, they may not get codegen'd, depending on + // the settings for the crate they are translated in. + let mut check_overflow = attrs.iter().any(|item| { + item.check_name("rustc_inherit_overflow_checks") + }); + + // Respect -Z force-overflow-checks=on and -C debug-assertions. + check_overflow |= infcx.tcx.sess.opts.debugging_opts.force_overflow_checks + .unwrap_or(infcx.tcx.sess.opts.debug_assertions); + + // Constants and const fn's always need overflow checks. + check_overflow |= constness == hir::Constness::Const; + Cx { tcx: infcx.tcx, infcx: infcx, constness: constness, - inline: inline + check_overflow: check_overflow } } } @@ -186,8 +193,8 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { self.tcx } - pub fn may_be_inlined_cross_crate(&self) -> bool { - self.inline + pub fn check_overflow(&self) -> bool { + self.check_overflow } } diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index d019677d0e691..3e3908845e36c 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -16,7 +16,7 @@ use rustc::mir::repr as mir; use asm; use base; use callee::Callee; -use common::{self, val_ty, C_null, C_uint, BlockAndBuilder, Result}; +use common::{self, val_ty, C_bool, C_null, C_uint, BlockAndBuilder, Result}; use datum::{Datum, Lvalue}; use debuginfo::DebugLoc; use adt; @@ -579,6 +579,15 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { lhs: ValueRef, rhs: ValueRef, input_ty: Ty<'tcx>) -> OperandValue { + // This case can currently arise only from functions marked + // with #[rustc_inherit_overflow_checks] and inlined from + // another crate (mostly core::num generic/#[inline] fns), + // while the current crate doesn't use overflow checks. + if !bcx.ccx().check_overflow() { + let val = self.trans_scalar_binop(bcx, op, lhs, rhs, input_ty); + return OperandValue::Pair(val, C_bool(bcx.ccx(), false)); + } + let (val, of) = match op { // These are checked using intrinsics mir::BinOp::Add | mir::BinOp::Sub | mir::BinOp::Mul => { diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index c11347f5762ba..86c4a33896d15 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -502,6 +502,13 @@ pub const KNOWN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeGat is just used to make tests pass \ and will never be stable", cfg_fn!(rustc_attrs))), + ("rustc_inherit_overflow_checks", Whitelisted, Gated("rustc_attrs", + "the `#[rustc_inherit_overflow_checks]` \ + attribute is just used to control \ + overflow checking behavior of several \ + libcore functions that are inlined \ + across crates and will never be stable", + cfg_fn!(rustc_attrs))), ("allow_internal_unstable", Normal, Gated("allow_internal_unstable", EXPLAIN_ALLOW_INTERNAL_UNSTABLE, From b6ce2aa4eacdbad3e98b17d3a66daea3cc7f2880 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Thu, 26 May 2016 20:19:12 +0300 Subject: [PATCH 11/18] rustc_const_eval: remove unused arithmetic ErrKind variants. --- src/librustc_const_eval/eval.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/librustc_const_eval/eval.rs b/src/librustc_const_eval/eval.rs index 5613b1211199a..a61318c47a48d 100644 --- a/src/librustc_const_eval/eval.rs +++ b/src/librustc_const_eval/eval.rs @@ -380,12 +380,6 @@ pub enum ErrKind { NotOn(ConstVal), CallOn(ConstVal), - DivideByZero, - DivideWithOverflow, - ModuloByZero, - ModuloWithOverflow, - ShiftLeftWithOverflow, - ShiftRightWithOverflow, MissingStructField, NonConstPath, UnimplementedConstVal(&'static str), @@ -436,12 +430,6 @@ impl ConstEvalErr { NotOn(ref const_val) => format!("not on {}", const_val.description()).into_cow(), CallOn(ref const_val) => format!("call on {}", const_val.description()).into_cow(), - DivideByZero => "attempted to divide by zero".into_cow(), - DivideWithOverflow => "attempted to divide with overflow".into_cow(), - ModuloByZero => "attempted remainder with a divisor of zero".into_cow(), - ModuloWithOverflow => "attempted remainder with overflow".into_cow(), - ShiftLeftWithOverflow => "attempted left shift with overflow".into_cow(), - ShiftRightWithOverflow => "attempted right shift with overflow".into_cow(), MissingStructField => "nonexistent struct field".into_cow(), NonConstPath => "non-constant path in constant expression".into_cow(), UnimplementedConstVal(what) => From afc598e07543cc0c927e976c7e3621e44843135c Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Thu, 26 May 2016 20:19:45 +0300 Subject: [PATCH 12/18] rustc_const_eval: strings are not indexable in Rust 1.x. --- src/librustc_const_eval/eval.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/librustc_const_eval/eval.rs b/src/librustc_const_eval/eval.rs index a61318c47a48d..b7cdb05a5d74f 100644 --- a/src/librustc_const_eval/eval.rs +++ b/src/librustc_const_eval/eval.rs @@ -856,9 +856,6 @@ pub fn eval_const_expr_partial<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, Integral(U8(data[idx as usize])) }, - Str(ref s) if idx as usize >= s.len() => signal!(e, IndexOutOfBounds), - // FIXME: return a const char - Str(_) => signal!(e, UnimplementedConstVal("indexing into str")), _ => signal!(e, IndexedNonVec), } } From 1447fbf183e2c6c7889257bb363de3228324fb6e Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Thu, 26 May 2016 22:09:48 +0300 Subject: [PATCH 13/18] rustc_const_eval: track the length and index in IndexOutOfBounds. --- src/librustc_const_eval/eval.rs | 19 ++++++++++++++----- src/librustc_trans/consts.rs | 5 ++++- src/librustc_trans/mir/constant.rs | 9 +++++++-- src/test/compile-fail/array_const_index-0.rs | 2 +- src/test/compile-fail/array_const_index-1.rs | 3 ++- src/test/compile-fail/const-array-oob.rs | 4 +++- src/test/compile-fail/const-err-early.rs | 6 ++++-- src/test/compile-fail/const-err.rs | 6 +++--- src/test/compile-fail/const-slice-oob.rs | 3 ++- 9 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/librustc_const_eval/eval.rs b/src/librustc_const_eval/eval.rs index b7cdb05a5d74f..785b734b79976 100644 --- a/src/librustc_const_eval/eval.rs +++ b/src/librustc_const_eval/eval.rs @@ -390,7 +390,7 @@ pub enum ErrKind { IndexedNonVec, IndexNegative, IndexNotInt, - IndexOutOfBounds, + IndexOutOfBounds { len: u64, index: u64 }, RepeatCountNotNatural, RepeatCountNotInt, @@ -441,7 +441,10 @@ impl ConstEvalErr { IndexedNonVec => "indexing is only supported for arrays".into_cow(), IndexNegative => "indices must be non-negative integers".into_cow(), IndexNotInt => "indices must be integers".into_cow(), - IndexOutOfBounds => "array index out of bounds".into_cow(), + IndexOutOfBounds { len, index } => { + format!("index out of bounds: the len is {} but the index is {}", + len, index).into_cow() + } RepeatCountNotNatural => "repeat count must be a natural number".into_cow(), RepeatCountNotInt => "repeat count must be integers".into_cow(), @@ -835,7 +838,9 @@ pub fn eval_const_expr_partial<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }; assert_eq!(idx as usize as u64, idx); match arr { - Array(_, n) if idx >= n => signal!(e, IndexOutOfBounds), + Array(_, n) if idx >= n => { + signal!(e, IndexOutOfBounds { len: n, index: idx }) + } Array(v, n) => if let hir::ExprVec(ref v) = tcx.map.expect_expr(v).node { assert_eq!(n as usize as u64, n); eval_const_expr_partial(tcx, &v[idx as usize], ty_hint, fn_args)? @@ -843,7 +848,9 @@ pub fn eval_const_expr_partial<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, bug!() }, - Repeat(_, n) if idx >= n => signal!(e, IndexOutOfBounds), + Repeat(_, n) if idx >= n => { + signal!(e, IndexOutOfBounds { len: n, index: idx }) + } Repeat(elem, _) => eval_const_expr_partial( tcx, &tcx.map.expect_expr(elem), @@ -851,7 +858,9 @@ pub fn eval_const_expr_partial<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, fn_args, )?, - ByteStr(ref data) if idx >= data.len() as u64 => signal!(e, IndexOutOfBounds), + ByteStr(ref data) if idx >= data.len() as u64 => { + signal!(e, IndexOutOfBounds { len: data.len() as u64, index: idx }) + } ByteStr(data) => { Integral(U8(data[idx as usize])) }, diff --git a/src/librustc_trans/consts.rs b/src/librustc_trans/consts.rs index bd36c18a47ee2..6788d5de6d869 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -716,7 +716,10 @@ fn const_expr_unadjusted<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, if iv >= len { // FIXME #3170: report this earlier on in the const-eval // pass. Reporting here is a bit late. - const_err(cx, e.span, Err(ErrKind::IndexOutOfBounds), trueconst)?; + const_err(cx, e.span, Err(ErrKind::IndexOutOfBounds { + len: len, + index: iv + }), trueconst)?; C_undef(val_ty(arr).element_type()) } else { const_get_elt(arr, &[iv as c_uint]) diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index e73d02a1e29f1..e32e542ed8996 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -298,8 +298,13 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { let cond_bool = common::const_to_uint(cond.llval) != 0; if cond_bool != expected { let err = match *msg { - mir::AssertMessage::BoundsCheck {..} => { - ErrKind::IndexOutOfBounds + mir::AssertMessage::BoundsCheck { ref len, ref index } => { + let len = self.const_operand(len, span)?; + let index = self.const_operand(index, span)?; + ErrKind::IndexOutOfBounds { + len: common::const_to_uint(len.llval), + index: common::const_to_uint(index.llval) + } } mir::AssertMessage::Math(ref err) => { ErrKind::Math(err.clone()) diff --git a/src/test/compile-fail/array_const_index-0.rs b/src/test/compile-fail/array_const_index-0.rs index 1134dbfd1c46c..e65230389f9c0 100644 --- a/src/test/compile-fail/array_const_index-0.rs +++ b/src/test/compile-fail/array_const_index-0.rs @@ -10,7 +10,7 @@ const A: &'static [i32] = &[]; const B: i32 = (&A)[1]; -//~^ ERROR: array index out of bounds +//~^ ERROR index out of bounds: the len is 0 but the index is 1 fn main() { let _ = B; diff --git a/src/test/compile-fail/array_const_index-1.rs b/src/test/compile-fail/array_const_index-1.rs index e59895cda442a..69d84e24c4982 100644 --- a/src/test/compile-fail/array_const_index-1.rs +++ b/src/test/compile-fail/array_const_index-1.rs @@ -9,7 +9,8 @@ // except according to those terms. const A: [i32; 0] = []; -const B: i32 = A[1]; //~ ERROR: array index out of bounds +const B: i32 = A[1]; +//~^ ERROR index out of bounds: the len is 0 but the index is 1 fn main() { let _ = B; diff --git a/src/test/compile-fail/const-array-oob.rs b/src/test/compile-fail/const-array-oob.rs index 15426febbcdd7..faabed4fd5e42 100644 --- a/src/test/compile-fail/const-array-oob.rs +++ b/src/test/compile-fail/const-array-oob.rs @@ -8,13 +8,15 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// ignore-tidy-linelength + #![feature(const_indexing)] const FOO: [u32; 3] = [1, 2, 3]; const BAR: u32 = FOO[5]; // no error, because the error below occurs before regular const eval const BLUB: [u32; FOO[4]] = [5, 6]; -//~^ ERROR array length constant evaluation error: array index out of bounds [E0250] +//~^ ERROR array length constant evaluation error: index out of bounds: the len is 3 but the index is 4 [E0250] fn main() { let _ = BAR; diff --git a/src/test/compile-fail/const-err-early.rs b/src/test/compile-fail/const-err-early.rs index 7567791c24066..f666140970b6e 100644 --- a/src/test/compile-fail/const-err-early.rs +++ b/src/test/compile-fail/const-err-early.rs @@ -15,8 +15,10 @@ pub const A: i8 = -std::i8::MIN; //~ ERROR attempted to negate with overflow pub const B: u8 = 200u8 + 200u8; //~ ERROR attempted to add with overflow pub const C: u8 = 200u8 * 4; //~ ERROR attempted to multiply with overflow pub const D: u8 = 42u8 - (42u8 + 1); //~ ERROR attempted to subtract with overflow -pub const E: u8 = [5u8][1]; //~ ERROR index out of bounds +pub const E: u8 = [5u8][1]; +//~^ ERROR index out of bounds: the len is 1 but the index is 1 fn main() { - let _e = [6u8][1]; //~ ERROR: array index out of bounds + let _e = [6u8][1]; + //~^ ERROR index out of bounds: the len is 1 but the index is 1 } diff --git a/src/test/compile-fail/const-err.rs b/src/test/compile-fail/const-err.rs index 85d67f52bfe86..ac2ef6662a46e 100644 --- a/src/test/compile-fail/const-err.rs +++ b/src/test/compile-fail/const-err.rs @@ -20,8 +20,8 @@ fn black_box(_: T) { // Make sure that the two uses get two errors. const FOO: u8 = [5u8][1]; -//~^ ERROR array index out of bounds -//~^^ ERROR array index out of bounds +//~^ ERROR index out of bounds: the len is 1 but the index is 1 +//~^^ ERROR index out of bounds: the len is 1 but the index is 1 fn main() { let a = -std::i8::MIN; @@ -34,7 +34,7 @@ fn main() { let d = 42u8 - (42u8 + 1); //~^ WARN attempted to subtract with overflow let _e = [5u8][1]; - //~^ WARN array index out of bounds + //~^ WARN index out of bounds: the len is 1 but the index is 1 black_box(a); black_box(b); black_box(c); diff --git a/src/test/compile-fail/const-slice-oob.rs b/src/test/compile-fail/const-slice-oob.rs index b50468c33fd8b..d63b0097e5a0c 100644 --- a/src/test/compile-fail/const-slice-oob.rs +++ b/src/test/compile-fail/const-slice-oob.rs @@ -9,7 +9,8 @@ // except according to those terms. const FOO: &'static[u32] = &[1, 2, 3]; -const BAR: u32 = FOO[5]; //~ ERROR array index out of bounds +const BAR: u32 = FOO[5]; +//~^ ERROR index out of bounds: the len is 3 but the index is 5 fn main() { let _ = BAR; From b8c5053a02f5468e58f856d06586d1a06560d224 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Thu, 26 May 2016 22:53:27 +0300 Subject: [PATCH 14/18] trans: use the same messages for both MIR and old arithmetic checks. --- src/librustc_trans/base.rs | 15 ++++++++------- src/librustc_trans/expr.rs | 20 +++++++++++++++----- 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-pow.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 +- 17 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 712805061ea8c..cd20a75db81bf 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -769,12 +769,12 @@ pub fn fail_if_zero_or_overflows<'blk, 'tcx>(cx: Block<'blk, 'tcx>, rhs: ValueRef, rhs_t: Ty<'tcx>) -> Block<'blk, 'tcx> { - let (zero_text, overflow_text) = if divrem.node == hir::BiDiv { - ("attempted to divide by zero", - "attempted to divide with overflow") + use rustc_const_math::{ConstMathErr, Op}; + + let (zero_err, overflow_err) = if divrem.node == hir::BiDiv { + (ConstMathErr::DivisionByZero, ConstMathErr::Overflow(Op::Div)) } else { - ("attempted remainder with a divisor of zero", - "attempted remainder with overflow") + (ConstMathErr::RemainderByZero, ConstMathErr::Overflow(Op::Rem)) }; let debug_loc = call_info.debug_loc(); @@ -802,7 +802,7 @@ pub fn fail_if_zero_or_overflows<'blk, 'tcx>(cx: Block<'blk, 'tcx>, } }; let bcx = with_cond(cx, is_zero, |bcx| { - controlflow::trans_fail(bcx, call_info, InternedString::new(zero_text)) + controlflow::trans_fail(bcx, call_info, InternedString::new(zero_err.description())) }); // To quote LLVM's documentation for the sdiv instruction: @@ -828,7 +828,8 @@ pub fn fail_if_zero_or_overflows<'blk, 'tcx>(cx: Block<'blk, 'tcx>, C_integral(llty, min, true), debug_loc); with_cond(bcx, is_min, |bcx| { - controlflow::trans_fail(bcx, call_info, InternedString::new(overflow_text)) + controlflow::trans_fail(bcx, call_info, + InternedString::new(overflow_err.description())) }) }) } else { diff --git a/src/librustc_trans/expr.rs b/src/librustc_trans/expr.rs index 465ebace1b830..09fd3abd91f11 100644 --- a/src/librustc_trans/expr.rs +++ b/src/librustc_trans/expr.rs @@ -2220,6 +2220,8 @@ impl OverflowOpViaIntrinsic { rhs: ValueRef, binop_debug_loc: DebugLoc) -> (Block<'blk, 'tcx>, ValueRef) { + use rustc_const_math::{ConstMathErr, Op}; + let llfn = self.to_intrinsic(bcx, lhs_t); let val = Call(bcx, llfn, &[lhs, rhs], binop_debug_loc); @@ -2233,10 +2235,16 @@ impl OverflowOpViaIntrinsic { let expected = Call(bcx, expect, &[cond, C_bool(bcx.ccx(), false)], binop_debug_loc); + let op = match *self { + OverflowOpViaIntrinsic::Add => Op::Add, + OverflowOpViaIntrinsic::Sub => Op::Sub, + OverflowOpViaIntrinsic::Mul => Op::Mul + }; + let bcx = base::with_cond(bcx, expected, |bcx| controlflow::trans_fail(bcx, info, - InternedString::new("arithmetic operation overflowed"))); + InternedString::new(ConstMathErr::Overflow(op).description()))); (bcx, result) } @@ -2252,6 +2260,8 @@ impl OverflowOpViaInputCheck { binop_debug_loc: DebugLoc) -> (Block<'blk, 'tcx>, ValueRef) { + use rustc_const_math::{ConstMathErr, Op}; + let lhs_llty = val_ty(lhs); let rhs_llty = val_ty(rhs); @@ -2266,16 +2276,16 @@ impl OverflowOpViaInputCheck { let outer_bits = And(bcx, rhs, invert_mask, binop_debug_loc); let cond = build_nonzero_check(bcx, outer_bits, binop_debug_loc); - let result = match *self { + let (result, op) = match *self { OverflowOpViaInputCheck::Shl => - build_unchecked_lshift(bcx, lhs, rhs, binop_debug_loc), + (build_unchecked_lshift(bcx, lhs, rhs, binop_debug_loc), Op::Shl), OverflowOpViaInputCheck::Shr => - build_unchecked_rshift(bcx, lhs_t, lhs, rhs, binop_debug_loc), + (build_unchecked_rshift(bcx, lhs_t, lhs, rhs, binop_debug_loc), Op::Shr) }; let bcx = base::with_cond(bcx, cond, |bcx| controlflow::trans_fail(bcx, info, - InternedString::new("shift operation overflowed"))); + InternedString::new(ConstMathErr::Overflow(op).description()))); (bcx, result) } diff --git a/src/test/run-fail/mod-zero.rs b/src/test/run-fail/mod-zero.rs index 093dad5838b60..686c3eb2f83b8 100644 --- a/src/test/run-fail/mod-zero.rs +++ b/src/test/run-fail/mod-zero.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:attempted remainder with a divisor of zero +// error-pattern:attempted to calculate the remainder with a divisor of zero fn main() { let y = 0; diff --git a/src/test/run-fail/overflowing-add.rs b/src/test/run-fail/overflowing-add.rs index 1f5297de5aa8d..ecb8c676cf700 100644 --- a/src/test/run-fail/overflowing-add.rs +++ b/src/test/run-fail/overflowing-add.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'arithmetic operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to add with overflow' // compile-flags: -C debug-assertions diff --git a/src/test/run-fail/overflowing-lsh-1.rs b/src/test/run-fail/overflowing-lsh-1.rs index a6a898fef1997..e277886d003dc 100644 --- a/src/test/run-fail/overflowing-lsh-1.rs +++ b/src/test/run-fail/overflowing-lsh-1.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift left with overflow' // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] diff --git a/src/test/run-fail/overflowing-lsh-2.rs b/src/test/run-fail/overflowing-lsh-2.rs index d25982e601652..42cb0f2d55bc1 100644 --- a/src/test/run-fail/overflowing-lsh-2.rs +++ b/src/test/run-fail/overflowing-lsh-2.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift left with overflow' // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] diff --git a/src/test/run-fail/overflowing-lsh-3.rs b/src/test/run-fail/overflowing-lsh-3.rs index 0d9fcc850bbb0..8c6623dcf50ce 100644 --- a/src/test/run-fail/overflowing-lsh-3.rs +++ b/src/test/run-fail/overflowing-lsh-3.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift left with overflow' // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] diff --git a/src/test/run-fail/overflowing-lsh-4.rs b/src/test/run-fail/overflowing-lsh-4.rs index f8dbd41d8bb00..3b7a00a2c73c6 100644 --- a/src/test/run-fail/overflowing-lsh-4.rs +++ b/src/test/run-fail/overflowing-lsh-4.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift left with overflow' // compile-flags: -C debug-assertions // This function is checking that our automatic truncation does not diff --git a/src/test/run-fail/overflowing-mul.rs b/src/test/run-fail/overflowing-mul.rs index ce8a8c27e52c9..0e168bf6ffbc6 100644 --- a/src/test/run-fail/overflowing-mul.rs +++ b/src/test/run-fail/overflowing-mul.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'arithmetic operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to multiply with overflow' // compile-flags: -C debug-assertions fn main() { diff --git a/src/test/run-fail/overflowing-pow.rs b/src/test/run-fail/overflowing-pow.rs index e9fea9e1141ed..9172374ec2818 100644 --- a/src/test/run-fail/overflowing-pow.rs +++ b/src/test/run-fail/overflowing-pow.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// error-pattern:thread 'main' panicked at 'arithmetic operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to multiply with overflow' // compile-flags: -C debug-assertions fn main() { diff --git a/src/test/run-fail/overflowing-rsh-1.rs b/src/test/run-fail/overflowing-rsh-1.rs index 9640054136744..d275792485d51 100644 --- a/src/test/run-fail/overflowing-rsh-1.rs +++ b/src/test/run-fail/overflowing-rsh-1.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift right with overflow' // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] diff --git a/src/test/run-fail/overflowing-rsh-2.rs b/src/test/run-fail/overflowing-rsh-2.rs index c8c7171d7ad57..1b888cddf6443 100644 --- a/src/test/run-fail/overflowing-rsh-2.rs +++ b/src/test/run-fail/overflowing-rsh-2.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift right with overflow' // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] diff --git a/src/test/run-fail/overflowing-rsh-3.rs b/src/test/run-fail/overflowing-rsh-3.rs index afcf31f5d69b5..be5c213493d6d 100644 --- a/src/test/run-fail/overflowing-rsh-3.rs +++ b/src/test/run-fail/overflowing-rsh-3.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift right with overflow' // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] diff --git a/src/test/run-fail/overflowing-rsh-4.rs b/src/test/run-fail/overflowing-rsh-4.rs index c4b3d61f2af48..820d9611d6acb 100644 --- a/src/test/run-fail/overflowing-rsh-4.rs +++ b/src/test/run-fail/overflowing-rsh-4.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift right with overflow' // compile-flags: -C debug-assertions // This function is checking that our (type-based) automatic diff --git a/src/test/run-fail/overflowing-rsh-5.rs b/src/test/run-fail/overflowing-rsh-5.rs index 8793a416286e3..b87be696fcb21 100644 --- a/src/test/run-fail/overflowing-rsh-5.rs +++ b/src/test/run-fail/overflowing-rsh-5.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift right with overflow' // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] diff --git a/src/test/run-fail/overflowing-rsh-6.rs b/src/test/run-fail/overflowing-rsh-6.rs index e9676b6f70299..554675686b5e2 100644 --- a/src/test/run-fail/overflowing-rsh-6.rs +++ b/src/test/run-fail/overflowing-rsh-6.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'shift operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to shift right with overflow' // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] diff --git a/src/test/run-fail/overflowing-sub.rs b/src/test/run-fail/overflowing-sub.rs index 96775aef07836..1cb207240ca49 100644 --- a/src/test/run-fail/overflowing-sub.rs +++ b/src/test/run-fail/overflowing-sub.rs @@ -10,7 +10,7 @@ // ignore-pretty : (#23623) problems when ending with // comments -// error-pattern:thread 'main' panicked at 'arithmetic operation overflowed' +// error-pattern:thread 'main' panicked at 'attempted to subtract with overflow' // compile-flags: -C debug-assertions fn main() { From d735f6bf33a8654e387d248f46e0ad6d98280a05 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Fri, 27 May 2016 14:40:05 +0300 Subject: [PATCH 15/18] trans: implement CheckedBinaryOp in mir::constant. --- src/librustc_trans/mir/constant.rs | 182 ++++++++++++++++++----------- 1 file changed, 112 insertions(+), 70 deletions(-) diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index e32e542ed8996..a516a84f0c60f 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -12,12 +12,13 @@ use llvm::{self, ValueRef}; use rustc::middle::const_val::ConstVal; use rustc_const_eval::ErrKind; use rustc_const_math::ConstInt::*; +use rustc_const_math::ConstMathErr; use rustc::hir::def_id::DefId; use rustc::infer::TransNormalize; use rustc::mir::repr as mir; use rustc::mir::tcx::LvalueTy; use rustc::traits; -use rustc::ty::{self, Ty, TypeFoldable}; +use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc::ty::cast::{CastTy, IntTy}; use rustc::ty::subst::Substs; use {abi, adt, base, Disr}; @@ -713,73 +714,28 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { let ty = lhs.ty; let binop_ty = self.mir.binop_ty(tcx, op, lhs.ty, rhs.ty); let (lhs, rhs) = (lhs.llval, rhs.llval); - assert!(!ty.is_simd()); - let is_float = ty.is_fp(); - let signed = ty.is_signed(); - - if let (Some(lhs), Some(rhs)) = (to_const_int(lhs, ty, tcx), - to_const_int(rhs, ty, tcx)) { - let result = match op { - mir::BinOp::Add => lhs + rhs, - mir::BinOp::Sub => lhs - rhs, - mir::BinOp::Mul => lhs * rhs, - mir::BinOp::Div => lhs / rhs, - mir::BinOp::Rem => lhs % rhs, - mir::BinOp::Shl => lhs << rhs, - mir::BinOp::Shr => lhs >> rhs, - _ => Ok(lhs) - }; - consts::const_err(self.ccx, span, - result.map_err(ErrKind::Math), - TrueConst::Yes)?; - } - - let llval = unsafe { - match op { - mir::BinOp::Add if is_float => llvm::LLVMConstFAdd(lhs, rhs), - mir::BinOp::Add => llvm::LLVMConstAdd(lhs, rhs), - - mir::BinOp::Sub if is_float => llvm::LLVMConstFSub(lhs, rhs), - mir::BinOp::Sub => llvm::LLVMConstSub(lhs, rhs), - - mir::BinOp::Mul if is_float => llvm::LLVMConstFMul(lhs, rhs), - mir::BinOp::Mul => llvm::LLVMConstMul(lhs, rhs), - - mir::BinOp::Div if is_float => llvm::LLVMConstFDiv(lhs, rhs), - mir::BinOp::Div if signed => llvm::LLVMConstSDiv(lhs, rhs), - mir::BinOp::Div => llvm::LLVMConstUDiv(lhs, rhs), + Const::new(const_scalar_binop(op, lhs, rhs, ty), binop_ty) + } - mir::BinOp::Rem if is_float => llvm::LLVMConstFRem(lhs, rhs), - mir::BinOp::Rem if signed => llvm::LLVMConstSRem(lhs, rhs), - mir::BinOp::Rem => llvm::LLVMConstURem(lhs, rhs), + mir::Rvalue::CheckedBinaryOp(op, ref lhs, ref rhs) => { + let lhs = self.const_operand(lhs, span)?; + let rhs = self.const_operand(rhs, span)?; + let ty = lhs.ty; + let val_ty = self.mir.binop_ty(tcx, op, lhs.ty, rhs.ty); + let binop_ty = tcx.mk_tup(vec![val_ty, tcx.types.bool]); + let (lhs, rhs) = (lhs.llval, rhs.llval); + assert!(!ty.is_fp()); - mir::BinOp::BitXor => llvm::LLVMConstXor(lhs, rhs), - mir::BinOp::BitAnd => llvm::LLVMConstAnd(lhs, rhs), - mir::BinOp::BitOr => llvm::LLVMConstOr(lhs, rhs), - mir::BinOp::Shl => { - let rhs = base::cast_shift_const_rhs(op.to_hir_binop(), lhs, rhs); - llvm::LLVMConstShl(lhs, rhs) - } - mir::BinOp::Shr => { - let rhs = base::cast_shift_const_rhs(op.to_hir_binop(), lhs, rhs); - if signed { llvm::LLVMConstAShr(lhs, rhs) } - else { llvm::LLVMConstLShr(lhs, rhs) } - } - mir::BinOp::Eq | mir::BinOp::Ne | - mir::BinOp::Lt | mir::BinOp::Le | - mir::BinOp::Gt | mir::BinOp::Ge => { - if is_float { - let cmp = base::bin_op_to_fcmp_predicate(op.to_hir_binop()); - llvm::ConstFCmp(cmp, lhs, rhs) - } else { - let cmp = base::bin_op_to_icmp_predicate(op.to_hir_binop(), - signed); - llvm::ConstICmp(cmp, lhs, rhs) - } - } + match const_scalar_checked_binop(tcx, op, lhs, rhs, ty) { + Some((llval, of)) => { + let llof = C_bool(self.ccx, of); + Const::new(C_struct(self.ccx, &[llval, llof], false), binop_ty) } - }; - Const::new(llval, binop_ty) + None => { + span_bug!(span, "{:?} got non-integer operands: {:?} and {:?}", + rvalue, Value(lhs), Value(rhs)); + } + } } mir::Rvalue::UnaryOp(op, ref operand) => { @@ -792,11 +748,6 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { } } mir::UnOp::Neg => { - if let Some(cval) = to_const_int(lloperand, operand.ty, tcx) { - consts::const_err(self.ccx, span, - (-cval).map_err(ErrKind::Math), - TrueConst::Yes)?; - } let is_float = operand.ty.is_fp(); unsafe { if is_float { @@ -815,6 +766,97 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { Ok(val) } + +} + +pub fn const_scalar_binop(op: mir::BinOp, + lhs: ValueRef, + rhs: ValueRef, + input_ty: Ty) -> ValueRef { + assert!(!input_ty.is_simd()); + let is_float = input_ty.is_fp(); + let signed = input_ty.is_signed(); + + unsafe { + match op { + mir::BinOp::Add if is_float => llvm::LLVMConstFAdd(lhs, rhs), + mir::BinOp::Add => llvm::LLVMConstAdd(lhs, rhs), + + mir::BinOp::Sub if is_float => llvm::LLVMConstFSub(lhs, rhs), + mir::BinOp::Sub => llvm::LLVMConstSub(lhs, rhs), + + mir::BinOp::Mul if is_float => llvm::LLVMConstFMul(lhs, rhs), + mir::BinOp::Mul => llvm::LLVMConstMul(lhs, rhs), + + mir::BinOp::Div if is_float => llvm::LLVMConstFDiv(lhs, rhs), + mir::BinOp::Div if signed => llvm::LLVMConstSDiv(lhs, rhs), + mir::BinOp::Div => llvm::LLVMConstUDiv(lhs, rhs), + + mir::BinOp::Rem if is_float => llvm::LLVMConstFRem(lhs, rhs), + mir::BinOp::Rem if signed => llvm::LLVMConstSRem(lhs, rhs), + mir::BinOp::Rem => llvm::LLVMConstURem(lhs, rhs), + + mir::BinOp::BitXor => llvm::LLVMConstXor(lhs, rhs), + mir::BinOp::BitAnd => llvm::LLVMConstAnd(lhs, rhs), + mir::BinOp::BitOr => llvm::LLVMConstOr(lhs, rhs), + mir::BinOp::Shl => { + let rhs = base::cast_shift_const_rhs(op.to_hir_binop(), lhs, rhs); + llvm::LLVMConstShl(lhs, rhs) + } + mir::BinOp::Shr => { + let rhs = base::cast_shift_const_rhs(op.to_hir_binop(), lhs, rhs); + if signed { llvm::LLVMConstAShr(lhs, rhs) } + else { llvm::LLVMConstLShr(lhs, rhs) } + } + mir::BinOp::Eq | mir::BinOp::Ne | + mir::BinOp::Lt | mir::BinOp::Le | + mir::BinOp::Gt | mir::BinOp::Ge => { + if is_float { + let cmp = base::bin_op_to_fcmp_predicate(op.to_hir_binop()); + llvm::ConstFCmp(cmp, lhs, rhs) + } else { + let cmp = base::bin_op_to_icmp_predicate(op.to_hir_binop(), + signed); + llvm::ConstICmp(cmp, lhs, rhs) + } + } + } + } +} + +pub fn const_scalar_checked_binop<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + op: mir::BinOp, + lllhs: ValueRef, + llrhs: ValueRef, + input_ty: Ty<'tcx>) + -> Option<(ValueRef, bool)> { + if let (Some(lhs), Some(rhs)) = (to_const_int(lllhs, input_ty, tcx), + to_const_int(llrhs, input_ty, tcx)) { + let result = match op { + mir::BinOp::Add => lhs + rhs, + mir::BinOp::Sub => lhs - rhs, + mir::BinOp::Mul => lhs * rhs, + mir::BinOp::Shl => lhs << rhs, + mir::BinOp::Shr => lhs >> rhs, + _ => { + bug!("Operator `{:?}` is not a checkable operator", op) + } + }; + + let of = match result { + Ok(_) => false, + Err(ConstMathErr::Overflow(_)) | + Err(ConstMathErr::ShiftNegative) => true, + Err(err) => { + bug!("Operator `{:?}` on `{:?}` and `{:?}` errored: {}", + op, lhs, rhs, err.description()); + } + }; + + Some((const_scalar_binop(op, lllhs, llrhs, input_ty), of)) + } else { + None + } } impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { From e71f6d8ac98a9db4fe58448fc70582339bc97ccd Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Sun, 5 Jun 2016 14:38:29 +0300 Subject: [PATCH 16/18] trans: report as many errors as possible for constants. --- src/librustc_trans/mir/block.rs | 73 +++++++++++++++++++----------- src/librustc_trans/mir/constant.rs | 61 +++++++++++++++++++------ src/librustc_trans/mir/rvalue.rs | 12 +++++ src/test/compile-fail/const-err.rs | 2 + 4 files changed, 107 insertions(+), 41 deletions(-) diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index cf41f5bf2bcbf..b404475b5844b 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -9,6 +9,7 @@ // except according to those terms. use llvm::{self, ValueRef}; +use rustc_const_eval::ErrKind; use rustc::middle::lang_items; use rustc::ty; use rustc::mir::repr as mir; @@ -222,30 +223,27 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { let const_cond = common::const_to_opt_uint(cond).map(|c| c == 1); // Don't translate the panic block if success if known. - let lltarget = self.llblock(target); if const_cond == Some(expected) { - funclet_br(bcx, lltarget); + funclet_br(self, bcx, target); return; } - if const_cond == Some(!expected) { - // Do nothing to end up with an unconditional panic. + // Pass the condition through llvm.expect for branch hinting. + let expect = bcx.ccx().get_intrinsic(&"llvm.expect.i1"); + let cond = bcx.call(expect, &[cond, C_bool(bcx.ccx(), expected)], None); + + // Create the failure block and the conditional branch to it. + let lltarget = llblock(self, target); + let panic_block = self.fcx.new_block("panic", None); + if expected { + bcx.cond_br(cond, lltarget, panic_block.llbb); } else { - // Pass the condition through llvm.expect for branch hinting. - let expect = bcx.ccx().get_intrinsic(&"llvm.expect.i1"); - let cond = bcx.call(expect, &[cond, C_bool(bcx.ccx(), expected)], None); - - // Create the failure block and the conditional branch to it. - // After this point, bcx is the block for the call to panic. - let panic_block = self.fcx.new_block("panic", None); - if expected { - bcx.cond_br(cond, lltarget, panic_block.llbb); - } else { - bcx.cond_br(cond, panic_block.llbb, lltarget); - } - bcx = panic_block.build(); + bcx.cond_br(cond, panic_block.llbb, lltarget); } + // After this point, bcx is the block for the call to panic. + bcx = panic_block.build(); + // Get the location information. let loc = bcx.sess().codemap().lookup_char_pos(terminator.span.lo); let filename = token::intern_and_get_ident(&loc.file.name); @@ -253,10 +251,19 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { let line = C_u32(bcx.ccx(), loc.line as u32); // Put together the arguments to the panic entry point. - let (lang_item, args) = match *msg { + let (lang_item, args, const_err) = match *msg { mir::AssertMessage::BoundsCheck { ref len, ref index } => { - let len = self.trans_operand(&mut bcx, len); - let index = self.trans_operand(&mut bcx, index); + let len = self.trans_operand(&mut bcx, len).immediate(); + let index = self.trans_operand(&mut bcx, index).immediate(); + + let const_err = common::const_to_opt_uint(len).and_then(|len| { + common::const_to_opt_uint(index).map(|index| { + ErrKind::IndexOutOfBounds { + len: len, + index: index + } + }) + }); let file_line = C_struct(bcx.ccx(), &[filename, line], false); let align = llalign_of_min(bcx.ccx(), common::val_ty(file_line)); @@ -265,7 +272,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { align, "panic_bounds_check_loc"); (lang_items::PanicBoundsCheckFnLangItem, - vec![file_line, index.immediate(), len.immediate()]) + vec![file_line, index, len], + const_err) } mir::AssertMessage::Math(ref err) => { let msg_str = token::intern_and_get_ident(err.description()); @@ -278,10 +286,23 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { msg_file_line, align, "panic_loc"); - (lang_items::PanicFnLangItem, vec![msg_file_line]) + (lang_items::PanicFnLangItem, + vec![msg_file_line], + Some(ErrKind::Math(err.clone()))) } }; + // If we know we always panic, and the error message + // is also constant, then we can produce a warning. + if const_cond == Some(!expected) { + if let Some(err) = const_err { + let _ = consts::const_err(bcx.ccx(), + terminator.span, + Err::<(), _>(err), + consts::TrueConst::No); + } + } + // Obtain the panic entry point. let def_id = common::langcall(bcx.tcx(), Some(terminator.span), "", lang_item); let callee = Callee::def(bcx.ccx(), def_id, @@ -290,15 +311,13 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { // Translate the actual panic invoke/call. if let Some(unwind) = cleanup { - let uwbcx = self.bcx(unwind); - let unwind = self.make_landing_pad(uwbcx); bcx.invoke(llfn, &args, self.unreachable_block().llbb, - unwind.llbb(), - cleanup_bundle.as_ref()); + llblock(self, unwind), + cleanup_bundle); } else { - bcx.call(llfn, &args, cleanup_bundle.as_ref()); + bcx.call(llfn, &args, cleanup_bundle); bcx.unreachable(); } } diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index a516a84f0c60f..d352a8241acd4 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -270,6 +270,11 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { fn trans(&mut self) -> Result, ConstEvalFailure> { let tcx = self.ccx.tcx(); let mut bb = mir::START_BLOCK; + + // Make sure to evaluate all statemenets to + // report as many errors as we possibly can. + let mut failure = Ok(()); + loop { let data = self.mir.basic_block_data(bb); for statement in &data.statements { @@ -277,8 +282,10 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { mir::StatementKind::Assign(ref dest, ref rvalue) => { let ty = self.mir.lvalue_ty(tcx, dest); let ty = self.monomorphize(&ty).to_ty(tcx); - let value = self.const_rvalue(rvalue, ty, statement.span)?; - self.store(dest, value, statement.span); + match self.const_rvalue(rvalue, ty, statement.span) { + Ok(value) => self.store(dest, value, statement.span), + Err(err) => if failure.is_ok() { failure = Err(err); } + } } } } @@ -289,6 +296,7 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { mir::TerminatorKind::Drop { target, .. } | // No dropping. mir::TerminatorKind::Goto { target } => target, mir::TerminatorKind::Return => { + failure?; return Ok(self.return_value.unwrap_or_else(|| { span_bug!(span, "no returned value in constant"); })) @@ -311,7 +319,10 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { ErrKind::Math(err.clone()) } }; - consts::const_err(self.ccx, span, Err(err), TrueConst::Yes)?; + match consts::const_err(self.ccx, span, Err(err), TrueConst::Yes) { + Ok(()) => {} + Err(err) => if failure.is_ok() { failure = Err(err); } + } } target } @@ -327,15 +338,21 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { func, fn_ty) }; - let args = args.iter().map(|arg| { - self.const_operand(arg, span) - }).collect::, _>>()?; - let value = MirConstContext::trans_def(self.ccx, instance, args)?; + let mut const_args = Vec::with_capacity(args.len()); + for arg in args { + match self.const_operand(arg, span) { + Ok(arg) => const_args.push(arg), + Err(err) => if failure.is_ok() { failure = Err(err); } + } + } if let Some((ref dest, target)) = *destination { - self.store(dest, value, span); + match MirConstContext::trans_def(self.ccx, instance, const_args) { + Ok(value) => self.store(dest, value, span), + Err(err) => if failure.is_ok() { failure = Err(err); } + } target } else { - span_bug!(span, "diverging {:?} in constant", terminator.kind) + span_bug!(span, "diverging {:?} in constant", terminator.kind); } } _ => span_bug!(span, "{:?} in constant", terminator.kind) @@ -425,8 +442,16 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { } else { span_bug!(span, "index is not an integer-constant expression") }; - (Base::Value(const_get_elt(base.llval, &[iv as u32])), - ptr::null_mut()) + + // Produce an undef instead of a LLVM assertion on OOB. + let len = common::const_to_uint(tr_base.len(self.ccx)); + let llelem = if iv < len { + const_get_elt(base.llval, &[iv as u32]) + } else { + C_undef(type_of::type_of(self.ccx, projected_ty)) + }; + + (Base::Value(llelem), ptr::null_mut()) } _ => span_bug!(span, "{:?} in constant", projection.elem) }; @@ -497,9 +522,17 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { } mir::Rvalue::Aggregate(ref kind, ref operands) => { - let fields = operands.iter().map(|operand| { - Ok(self.const_operand(operand, span)?.llval) - }).collect::, _>>()?; + // Make sure to evaluate all operands to + // report as many errors as we possibly can. + let mut fields = Vec::with_capacity(operands.len()); + let mut failure = Ok(()); + for operand in operands { + match self.const_operand(operand, span) { + Ok(val) => fields.push(val.llval), + Err(err) => if failure.is_ok() { failure = Err(err); } + } + } + failure?; // FIXME Shouldn't need to manually trigger closure instantiations. if let mir::AggregateKind::Closure(def_id, substs) = *kind { diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index 3e3908845e36c..ba351d07016c5 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -27,6 +27,7 @@ use value::Value; use Disr; use super::MirContext; +use super::constant::const_scalar_checked_binop; use super::operand::{OperandRef, OperandValue}; use super::lvalue::{LvalueRef, get_dataptr, get_meta}; @@ -588,6 +589,17 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { return OperandValue::Pair(val, C_bool(bcx.ccx(), false)); } + // First try performing the operation on constants, which + // will only succeed if both operands are constant. + // This is necessary to determine when an overflow Assert + // will always panic at runtime, and produce a warning. + match const_scalar_checked_binop(bcx.tcx(), op, lhs, rhs, input_ty) { + Some((val, of)) => { + return OperandValue::Pair(val, C_bool(bcx.ccx(), of)); + } + None => {} + } + let (val, of) = match op { // These are checked using intrinsics mir::BinOp::Add | mir::BinOp::Sub | mir::BinOp::Mul => { diff --git a/src/test/compile-fail/const-err.rs b/src/test/compile-fail/const-err.rs index ac2ef6662a46e..a1d3888e78ea0 100644 --- a/src/test/compile-fail/const-err.rs +++ b/src/test/compile-fail/const-err.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// compile-flags: -Zforce-overflow-checks=on + // these errors are not actually "const_err", they occur in trans/consts // and are unconditional warnings that can't be denied or allowed From da081e1eac64d981209a742f8edea1a55127ce42 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Thu, 2 Jun 2016 21:32:07 -0400 Subject: [PATCH 17/18] [MIR] Handle call return values that need to be casted properly. --- src/librustc_trans/mir/block.rs | 44 ++++++++++++++++++++++++++-- src/test/run-pass/mir_cast_fn_ret.rs | 24 +++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 src/test/run-pass/mir_cast_fn_ret.rs diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index b404475b5844b..c8f9d46f7493f 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -19,11 +19,11 @@ use base; use build; use callee::{Callee, CalleeData, Fn, Intrinsic, NamedTupleConstructor, Virtual}; use common::{self, Block, BlockAndBuilder, LandingPad}; -use common::{C_bool, C_str_slice, C_struct, C_u32, C_undef}; +use common::{C_bool, C_str_slice, C_struct, C_u32, C_uint, C_undef}; use consts; use debuginfo::DebugLoc; use Disr; -use machine::{llalign_of_min, llbitsize_of_real}; +use machine::{llalign_of_min, llbitsize_of_real, llsize_of_store}; use meth; use type_of; use glue; @@ -39,6 +39,8 @@ use super::lvalue::{LvalueRef, load_fat_ptr}; use super::operand::OperandRef; use super::operand::OperandValue::*; +use std::cmp; + impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { pub fn trans_block(&mut self, bb: mir::BasicBlock) { let mut bcx = self.bcx(bb); @@ -852,7 +854,43 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { match dest { Nothing => (), - Store(dst) => ret_ty.store(bcx, op.immediate(), dst), + Store(dst) => { + if let Some(llcast_ty) = ret_ty.cast { + let ccx = bcx.ccx(); + // The actual return type is a struct, but the ABI + // adaptation code has cast it into some scalar type. The + // code that follows is the only reliable way I have + // found to do a transform like i64 -> {i32,i32}. + // Basically we dump the data onto the stack then memcpy it. + // + // Other approaches I tried: + // - Casting rust ret pointer to the foreign type and using Store + // is (a) unsafe if size of foreign type > size of rust type and + // (b) runs afoul of strict aliasing rules, yielding invalid + // assembly under -O (specifically, the store gets removed). + // - Truncating foreign type to correct integral type and then + // bitcasting to the struct type yields invalid cast errors. + + // We instead thus allocate some scratch space... + let llscratch = bcx.alloca(llcast_ty, "fn_ret_cast"); + bcx.with_block(|bcx| base::call_lifetime_start(bcx, llscratch)); + + // ...where we first store the value... + bcx.store(op.immediate(), llscratch); + + // ...and then memcpy it to the intended destination. + base::call_memcpy(bcx, + bcx.pointercast(dst, Type::i8p(ccx)), + bcx.pointercast(llscratch, Type::i8p(ccx)), + C_uint(ccx, llsize_of_store(ccx, ret_ty.original_ty)), + cmp::min(llalign_of_min(ccx, ret_ty.original_ty), + llalign_of_min(ccx, llcast_ty)) as u32); + + bcx.with_block(|bcx| base::call_lifetime_end(bcx, llscratch)); + } else { + ret_ty.store(bcx, op.immediate(), dst); + } + } IndirectOperand(tmp, idx) => { let op = self.trans_load(bcx, tmp, op.ty); self.temps[idx as usize] = TempRef::Operand(Some(op)); diff --git a/src/test/run-pass/mir_cast_fn_ret.rs b/src/test/run-pass/mir_cast_fn_ret.rs new file mode 100644 index 0000000000000..5bdc14f659cd5 --- /dev/null +++ b/src/test/run-pass/mir_cast_fn_ret.rs @@ -0,0 +1,24 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(rustc_attrs)] + +pub extern "C" fn foo() -> (u8, u8, u8) { + (1, 2, 3) +} + +#[rustc_mir] +pub fn bar() -> u8 { + foo().2 +} + +fn main() { + assert_eq!(bar(), 3); +} From cee244d4f02df90732c9e182f3567036ac695928 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Sun, 5 Jun 2016 15:34:13 +0300 Subject: [PATCH 18/18] trans: update Luqmana's patch for generalized pair handling. --- src/librustc_trans/mir/block.rs | 95 ++++++++++++++++------------ src/test/run-pass/mir_cast_fn_ret.rs | 18 ++++-- 2 files changed, 67 insertions(+), 46 deletions(-) diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index c8f9d46f7493f..b7aca4c8d7fed 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -852,63 +852,74 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { op: OperandRef<'tcx>) { use self::ReturnDest::*; - match dest { - Nothing => (), + // Handle the simple cases that don't require casts, first. + let llcast_ty = match dest { + Nothing => return, Store(dst) => { if let Some(llcast_ty) = ret_ty.cast { - let ccx = bcx.ccx(); - // The actual return type is a struct, but the ABI - // adaptation code has cast it into some scalar type. The - // code that follows is the only reliable way I have - // found to do a transform like i64 -> {i32,i32}. - // Basically we dump the data onto the stack then memcpy it. - // - // Other approaches I tried: - // - Casting rust ret pointer to the foreign type and using Store - // is (a) unsafe if size of foreign type > size of rust type and - // (b) runs afoul of strict aliasing rules, yielding invalid - // assembly under -O (specifically, the store gets removed). - // - Truncating foreign type to correct integral type and then - // bitcasting to the struct type yields invalid cast errors. - - // We instead thus allocate some scratch space... - let llscratch = bcx.alloca(llcast_ty, "fn_ret_cast"); - bcx.with_block(|bcx| base::call_lifetime_start(bcx, llscratch)); - - // ...where we first store the value... - bcx.store(op.immediate(), llscratch); - - // ...and then memcpy it to the intended destination. - base::call_memcpy(bcx, - bcx.pointercast(dst, Type::i8p(ccx)), - bcx.pointercast(llscratch, Type::i8p(ccx)), - C_uint(ccx, llsize_of_store(ccx, ret_ty.original_ty)), - cmp::min(llalign_of_min(ccx, ret_ty.original_ty), - llalign_of_min(ccx, llcast_ty)) as u32); - - bcx.with_block(|bcx| base::call_lifetime_end(bcx, llscratch)); + llcast_ty } else { ret_ty.store(bcx, op.immediate(), dst); + return; } } IndirectOperand(tmp, idx) => { let op = self.trans_load(bcx, tmp, op.ty); self.temps[idx as usize] = TempRef::Operand(Some(op)); + return; } DirectOperand(idx) => { - // If there is a cast, we have to store and reload. - let op = if ret_ty.cast.is_some() { - let tmp = bcx.with_block(|bcx| { - base::alloc_ty(bcx, op.ty, "tmp_ret") - }); - ret_ty.store(bcx, op.immediate(), tmp); - self.trans_load(bcx, tmp, op.ty) + if let Some(llcast_ty) = ret_ty.cast { + llcast_ty } else { - op.unpack_if_pair(bcx) - }; + let op = op.unpack_if_pair(bcx); + self.temps[idx as usize] = TempRef::Operand(Some(op)); + return; + } + } + }; + + // The actual return type is a struct, but the ABI + // adaptation code has cast it into some scalar type. The + // code that follows is the only reliable way I have + // found to do a transform like i64 -> {i32,i32}. + // Basically we dump the data onto the stack then memcpy it. + // + // Other approaches I tried: + // - Casting rust ret pointer to the foreign type and using Store + // is (a) unsafe if size of foreign type > size of rust type and + // (b) runs afoul of strict aliasing rules, yielding invalid + // assembly under -O (specifically, the store gets removed). + // - Truncating foreign type to correct integral type and then + // bitcasting to the struct type yields invalid cast errors. + + // We instead thus allocate some scratch space... + let llscratch = bcx.alloca(llcast_ty, "fn_ret_cast"); + bcx.with_block(|bcx| base::call_lifetime_start(bcx, llscratch)); + + // ...where we first store the value... + bcx.store(op.immediate(), llscratch); + + let ccx = bcx.ccx(); + match dest { + Store(dst) => { + // ...and then memcpy it to the intended destination. + base::call_memcpy(bcx, + bcx.pointercast(dst, Type::i8p(ccx)), + bcx.pointercast(llscratch, Type::i8p(ccx)), + C_uint(ccx, llsize_of_store(ccx, ret_ty.original_ty)), + cmp::min(llalign_of_min(ccx, ret_ty.original_ty), + llalign_of_min(ccx, llcast_ty)) as u32); + } + DirectOperand(idx) => { + let llptr = bcx.pointercast(llscratch, ret_ty.original_ty.ptr_to()); + let op = self.trans_load(bcx, llptr, op.ty); self.temps[idx as usize] = TempRef::Operand(Some(op)); } + Nothing | IndirectOperand(_, _) => bug!() } + + bcx.with_block(|bcx| base::call_lifetime_end(bcx, llscratch)); } } diff --git a/src/test/run-pass/mir_cast_fn_ret.rs b/src/test/run-pass/mir_cast_fn_ret.rs index 5bdc14f659cd5..8a723967aff5f 100644 --- a/src/test/run-pass/mir_cast_fn_ret.rs +++ b/src/test/run-pass/mir_cast_fn_ret.rs @@ -10,15 +10,25 @@ #![feature(rustc_attrs)] -pub extern "C" fn foo() -> (u8, u8, u8) { +pub extern "C" fn tuple2() -> (u16, u8) { + (1, 2) +} + +pub extern "C" fn tuple3() -> (u8, u8, u8) { (1, 2, 3) } #[rustc_mir] -pub fn bar() -> u8 { - foo().2 +pub fn test2() -> u8 { + tuple2().1 +} + +#[rustc_mir] +pub fn test3() -> u8 { + tuple3().2 } fn main() { - assert_eq!(bar(), 3); + assert_eq!(test2(), 2); + assert_eq!(test3(), 3); }