Skip to content

Commit e71f6d8

Browse files
eddybeddyb
committed
trans: report as many errors as possible for constants.
1 parent d735f6b commit e71f6d8

File tree

4 files changed

+107
-41
lines changed

4 files changed

+107
-41
lines changed

‎src/librustc_trans/mir/block.rs‎

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
// except according to those terms.
1010

1111
use llvm::{self, ValueRef};
12+
use rustc_const_eval::ErrKind;
1213
use rustc::middle::lang_items;
1314
use rustc::ty;
1415
use rustc::mir::repr as mir;
@@ -222,41 +223,47 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
222223
let const_cond = common::const_to_opt_uint(cond).map(|c| c == 1);
223224

224225
// Don't translate the panic block if success if known.
225-
let lltarget = self.llblock(target);
226226
if const_cond == Some(expected) {
227-
funclet_br(bcx, lltarget);
227+
funclet_br(self, bcx, target);
228228
return;
229229
}
230230

231-
if const_cond == Some(!expected) {
232-
// Do nothing to end up with an unconditional panic.
231+
// Pass the condition through llvm.expect for branch hinting.
232+
let expect = bcx.ccx().get_intrinsic(&"llvm.expect.i1");
233+
let cond = bcx.call(expect, &[cond, C_bool(bcx.ccx(), expected)], None);
234+
235+
// Create the failure block and the conditional branch to it.
236+
let lltarget = llblock(self, target);
237+
let panic_block = self.fcx.new_block("panic", None);
238+
if expected {
239+
bcx.cond_br(cond, lltarget, panic_block.llbb);
233240
} else {
234-
// Pass the condition through llvm.expect for branch hinting.
235-
let expect = bcx.ccx().get_intrinsic(&"llvm.expect.i1");
236-
let cond = bcx.call(expect, &[cond, C_bool(bcx.ccx(), expected)], None);
237-
238-
// Create the failure block and the conditional branch to it.
239-
// After this point, bcx is the block for the call to panic.
240-
let panic_block = self.fcx.new_block("panic", None);
241-
if expected {
242-
bcx.cond_br(cond, lltarget, panic_block.llbb);
243-
} else {
244-
bcx.cond_br(cond, panic_block.llbb, lltarget);
245-
}
246-
bcx = panic_block.build();
241+
bcx.cond_br(cond, panic_block.llbb, lltarget);
247242
}
248243

244+
// After this point, bcx is the block for the call to panic.
245+
bcx = panic_block.build();
246+
249247
// Get the location information.
250248
let loc = bcx.sess().codemap().lookup_char_pos(terminator.span.lo);
251249
let filename = token::intern_and_get_ident(&loc.file.name);
252250
let filename = C_str_slice(bcx.ccx(), filename);
253251
let line = C_u32(bcx.ccx(), loc.line as u32);
254252

255253
// Put together the arguments to the panic entry point.
256-
let (lang_item, args) = match *msg {
254+
let (lang_item, args, const_err) = match *msg {
257255
mir::AssertMessage::BoundsCheck { ref len, ref index } => {
258-
let len = self.trans_operand(&mut bcx, len);
259-
let index = self.trans_operand(&mut bcx, index);
256+
let len = self.trans_operand(&mut bcx, len).immediate();
257+
let index = self.trans_operand(&mut bcx, index).immediate();
258+
259+
let const_err = common::const_to_opt_uint(len).and_then(|len| {
260+
common::const_to_opt_uint(index).map(|index| {
261+
ErrKind::IndexOutOfBounds {
262+
len: len,
263+
index: index
264+
}
265+
})
266+
});
260267

261268
let file_line = C_struct(bcx.ccx(), &[filename, line], false);
262269
let align = llalign_of_min(bcx.ccx(), common::val_ty(file_line));
@@ -265,7 +272,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
265272
align,
266273
"panic_bounds_check_loc");
267274
(lang_items::PanicBoundsCheckFnLangItem,
268-
vec![file_line, index.immediate(), len.immediate()])
275+
vec![file_line, index, len],
276+
const_err)
269277
}
270278
mir::AssertMessage::Math(ref err) => {
271279
let msg_str = token::intern_and_get_ident(err.description());
@@ -278,10 +286,23 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
278286
msg_file_line,
279287
align,
280288
"panic_loc");
281-
(lang_items::PanicFnLangItem, vec![msg_file_line])
289+
(lang_items::PanicFnLangItem,
290+
vec![msg_file_line],
291+
Some(ErrKind::Math(err.clone())))
282292
}
283293
};
284294

295+
// If we know we always panic, and the error message
296+
// is also constant, then we can produce a warning.
297+
if const_cond == Some(!expected) {
298+
if let Some(err) = const_err {
299+
let _ = consts::const_err(bcx.ccx(),
300+
terminator.span,
301+
Err::<(), _>(err),
302+
consts::TrueConst::No);
303+
}
304+
}
305+
285306
// Obtain the panic entry point.
286307
let def_id = common::langcall(bcx.tcx(), Some(terminator.span), "", lang_item);
287308
let callee = Callee::def(bcx.ccx(), def_id,
@@ -290,15 +311,13 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
290311

291312
// Translate the actual panic invoke/call.
292313
if let Some(unwind) = cleanup {
293-
let uwbcx = self.bcx(unwind);
294-
let unwind = self.make_landing_pad(uwbcx);
295314
bcx.invoke(llfn,
296315
&args,
297316
self.unreachable_block().llbb,
298-
unwind.llbb(),
299-
cleanup_bundle.as_ref());
317+
llblock(self, unwind),
318+
cleanup_bundle);
300319
} else {
301-
bcx.call(llfn, &args, cleanup_bundle.as_ref());
320+
bcx.call(llfn, &args, cleanup_bundle);
302321
bcx.unreachable();
303322
}
304323
}

‎src/librustc_trans/mir/constant.rs‎

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -270,15 +270,22 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> {
270270
fn trans(&mut self) -> Result<Const<'tcx>, ConstEvalFailure> {
271271
let tcx = self.ccx.tcx();
272272
let mut bb = mir::START_BLOCK;
273+
274+
// Make sure to evaluate all statemenets to
275+
// report as many errors as we possibly can.
276+
let mut failure = Ok(());
277+
273278
loop {
274279
let data = self.mir.basic_block_data(bb);
275280
for statement in &data.statements {
276281
match statement.kind {
277282
mir::StatementKind::Assign(ref dest, ref rvalue) => {
278283
let ty = self.mir.lvalue_ty(tcx, dest);
279284
let ty = self.monomorphize(&ty).to_ty(tcx);
280-
let value = self.const_rvalue(rvalue, ty, statement.span)?;
281-
self.store(dest, value, statement.span);
285+
match self.const_rvalue(rvalue, ty, statement.span) {
286+
Ok(value) => self.store(dest, value, statement.span),
287+
Err(err) => if failure.is_ok() { failure = Err(err); }
288+
}
282289
}
283290
}
284291
}
@@ -289,6 +296,7 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> {
289296
mir::TerminatorKind::Drop { target, .. } | // No dropping.
290297
mir::TerminatorKind::Goto { target } => target,
291298
mir::TerminatorKind::Return => {
299+
failure?;
292300
return Ok(self.return_value.unwrap_or_else(|| {
293301
span_bug!(span, "no returned value in constant");
294302
}))
@@ -311,7 +319,10 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> {
311319
ErrKind::Math(err.clone())
312320
}
313321
};
314-
consts::const_err(self.ccx, span, Err(err), TrueConst::Yes)?;
322+
match consts::const_err(self.ccx, span, Err(err), TrueConst::Yes) {
323+
Ok(()) => {}
324+
Err(err) => if failure.is_ok() { failure = Err(err); }
325+
}
315326
}
316327
target
317328
}
@@ -327,15 +338,21 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> {
327338
func, fn_ty)
328339
};
329340

330-
let args = args.iter().map(|arg| {
331-
self.const_operand(arg, span)
332-
}).collect::<Result<Vec<_>, _>>()?;
333-
let value = MirConstContext::trans_def(self.ccx, instance, args)?;
341+
let mut const_args = Vec::with_capacity(args.len());
342+
for arg in args {
343+
match self.const_operand(arg, span) {
344+
Ok(arg) => const_args.push(arg),
345+
Err(err) => if failure.is_ok() { failure = Err(err); }
346+
}
347+
}
334348
if let Some((ref dest, target)) = *destination {
335-
self.store(dest, value, span);
349+
match MirConstContext::trans_def(self.ccx, instance, const_args) {
350+
Ok(value) => self.store(dest, value, span),
351+
Err(err) => if failure.is_ok() { failure = Err(err); }
352+
}
336353
target
337354
} else {
338-
span_bug!(span, "diverging {:?} in constant", terminator.kind)
355+
span_bug!(span, "diverging {:?} in constant", terminator.kind);
339356
}
340357
}
341358
_ => span_bug!(span, "{:?} in constant", terminator.kind)
@@ -425,8 +442,16 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> {
425442
} else {
426443
span_bug!(span, "index is not an integer-constant expression")
427444
};
428-
(Base::Value(const_get_elt(base.llval, &[iv as u32])),
429-
ptr::null_mut())
445+
446+
// Produce an undef instead of a LLVM assertion on OOB.
447+
let len = common::const_to_uint(tr_base.len(self.ccx));
448+
let llelem = if iv < len {
449+
const_get_elt(base.llval, &[iv as u32])
450+
} else {
451+
C_undef(type_of::type_of(self.ccx, projected_ty))
452+
};
453+
454+
(Base::Value(llelem), ptr::null_mut())
430455
}
431456
_ => span_bug!(span, "{:?} in constant", projection.elem)
432457
};
@@ -497,9 +522,17 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> {
497522
}
498523

499524
mir::Rvalue::Aggregate(ref kind, ref operands) => {
500-
let fields = operands.iter().map(|operand| {
501-
Ok(self.const_operand(operand, span)?.llval)
502-
}).collect::<Result<Vec<_>, _>>()?;
525+
// Make sure to evaluate all operands to
526+
// report as many errors as we possibly can.
527+
let mut fields = Vec::with_capacity(operands.len());
528+
let mut failure = Ok(());
529+
for operand in operands {
530+
match self.const_operand(operand, span) {
531+
Ok(val) => fields.push(val.llval),
532+
Err(err) => if failure.is_ok() { failure = Err(err); }
533+
}
534+
}
535+
failure?;
503536

504537
// FIXME Shouldn't need to manually trigger closure instantiations.
505538
if let mir::AggregateKind::Closure(def_id, substs) = *kind {

‎src/librustc_trans/mir/rvalue.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use value::Value;
2727
use Disr;
2828

2929
use super::MirContext;
30+
use super::constant::const_scalar_checked_binop;
3031
use super::operand::{OperandRef, OperandValue};
3132
use super::lvalue::{LvalueRef, get_dataptr, get_meta};
3233

@@ -588,6 +589,17 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
588589
return OperandValue::Pair(val, C_bool(bcx.ccx(), false));
589590
}
590591

592+
// First try performing the operation on constants, which
593+
// will only succeed if both operands are constant.
594+
// This is necessary to determine when an overflow Assert
595+
// will always panic at runtime, and produce a warning.
596+
match const_scalar_checked_binop(bcx.tcx(), op, lhs, rhs, input_ty) {
597+
Some((val, of)) => {
598+
return OperandValue::Pair(val, C_bool(bcx.ccx(), of));
599+
}
600+
None => {}
601+
}
602+
591603
let (val, of) = match op {
592604
// These are checked using intrinsics
593605
mir::BinOp::Add | mir::BinOp::Sub | mir::BinOp::Mul => {

‎src/test/compile-fail/const-err.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11+
// compile-flags: -Zforce-overflow-checks=on
12+
1113
// these errors are not actually "const_err", they occur in trans/consts
1214
// and are unconditional warnings that can't be denied or allowed
1315

0 commit comments

Comments
 (0)