From e71f6d8ac98a9db4fe58448fc70582339bc97ccd Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Sun, 5 Jun 2016 14:38:29 +0300 Subject: [PATCH] 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