Skip to content

Commit

Permalink
Auto merge of #70771 - RalfJung:ctfe-loop, r=oli-obk
Browse files Browse the repository at this point in the history
 Miri terminator handling: only do progress sanity check for 'Call' terminator

This will still catch mistakes in bad intrinsic/foreign-item shims, which is the main source of errors here.

Fixes #70723
r? @oli-obk
  • Loading branch information
bors committed Apr 6, 2020
2 parents 733f104 + e92bde3 commit bd18bc9
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 31 deletions.
26 changes: 17 additions & 9 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use rustc_middle::ty::layout::{self, TyAndLayout};
use rustc_middle::ty::{
self, fold::BottomUpFolder, query::TyCtxtAt, subst::SubstsRef, Ty, TyCtxt, TypeFoldable,
};
use rustc_span::source_map::DUMMY_SP;
use rustc_span::{source_map::DUMMY_SP, Span};
use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Size, TargetDataLayout};

use super::{
Expand Down Expand Up @@ -256,7 +256,7 @@ pub(super) fn mir_assign_valid_types<'tcx>(
/// or compute the layout.
#[cfg_attr(not(debug_assertions), inline(always))]
pub(super) fn from_known_layout<'tcx>(
tcx: TyCtxt<'tcx>,
tcx: TyCtxtAt<'tcx>,
known_layout: Option<TyAndLayout<'tcx>>,
compute: impl FnOnce() -> InterpResult<'tcx, TyAndLayout<'tcx>>,
) -> InterpResult<'tcx, TyAndLayout<'tcx>> {
Expand All @@ -265,12 +265,14 @@ pub(super) fn from_known_layout<'tcx>(
Some(known_layout) => {
if cfg!(debug_assertions) {
let check_layout = compute()?;
assert!(
mir_assign_valid_types(tcx, check_layout, known_layout),
"expected type differs from actual type.\nexpected: {:?}\nactual: {:?}",
known_layout.ty,
check_layout.ty,
);
if !mir_assign_valid_types(tcx.tcx, check_layout, known_layout) {
span_bug!(
tcx.span,
"expected type differs from actual type.\nexpected: {:?}\nactual: {:?}",
known_layout.ty,
check_layout.ty,
);
}
}
Ok(known_layout)
}
Expand All @@ -294,6 +296,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

#[inline(always)]
pub fn set_span(&mut self, span: Span) {
self.tcx.span = span;
self.memory.tcx.span = span;
}

#[inline(always)]
pub fn force_ptr(
&self,
Expand Down Expand Up @@ -444,7 +452,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// have to support that case (mostly by skipping all caching).
match frame.locals.get(local).and_then(|state| state.layout.get()) {
None => {
let layout = from_known_layout(self.tcx.tcx, layout, || {
let layout = from_known_layout(self.tcx, layout, || {
let local_ty = frame.body.local_decls[local].ty;
let local_ty =
self.subst_from_frame_and_normalize_erasing_regions(frame, local_ty);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ty::ConstKind::Value(val_val) => val_val,
};
// Other cases need layout.
let layout = from_known_layout(self.tcx.tcx, layout, || self.layout_of(val.ty))?;
let layout = from_known_layout(self.tcx, layout, || self.layout_of(val.ty))?;
let op = match val_val {
ConstValue::ByRef { alloc, offset } => {
let id = self.tcx.alloc_map.lock().create_memory_alloc(alloc);
Expand Down
14 changes: 8 additions & 6 deletions src/librustc_mir/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,12 +867,14 @@ where
) -> InterpResult<'tcx> {
// We do NOT compare the types for equality, because well-typed code can
// actually "transmute" `&mut T` to `&T` in an assignment without a cast.
assert!(
mir_assign_valid_types(self.tcx.tcx, src.layout, dest.layout),
"type mismatch when copying!\nsrc: {:?},\ndest: {:?}",
src.layout.ty,
dest.layout.ty,
);
if !mir_assign_valid_types(self.tcx.tcx, src.layout, dest.layout) {
span_bug!(
self.tcx.span,
"type mismatch when copying!\nsrc: {:?},\ndest: {:?}",
src.layout.ty,
dest.layout.ty,
);
}

// Let us see if the layout is simple so we take a shortcut, avoid force_allocation.
let src = match self.try_read_immediate(src)? {
Expand Down
11 changes: 2 additions & 9 deletions src/librustc_mir/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

fn statement(&mut self, stmt: &mir::Statement<'tcx>) -> InterpResult<'tcx> {
info!("{:?}", stmt);
self.set_span(stmt.source_info.span);

use rustc_middle::mir::StatementKind::*;

// Some statements (e.g., box) push new stack frames.
// We have to record the stack frame number *before* executing the statement.
let frame_idx = self.cur_frame();
self.tcx.span = stmt.source_info.span;
self.memory.tcx.span = stmt.source_info.span;

match &stmt.kind {
Assign(box (place, rvalue)) => self.eval_rvalue_into_place(rvalue, *place)?,
Expand Down Expand Up @@ -276,16 +275,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

fn terminator(&mut self, terminator: &mir::Terminator<'tcx>) -> InterpResult<'tcx> {
info!("{:?}", terminator.kind);
self.tcx.span = terminator.source_info.span;
self.memory.tcx.span = terminator.source_info.span;

let old_stack = self.cur_frame();
let old_bb = self.frame().block;
self.set_span(terminator.source_info.span);

self.eval_terminator(terminator)?;
if !self.stack.is_empty() {
// This should change *something*
assert!(self.cur_frame() != old_stack || self.frame().block != old_bb);
if let Some(block) = self.frame().block {
info!("// executing {:?}", block);
}
Expand Down
21 changes: 17 additions & 4 deletions src/librustc_mir/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

Call { ref func, ref args, destination, ref cleanup, .. } => {
let old_stack = self.cur_frame();
let old_bb = self.frame().block;
let func = self.eval_operand(func, None)?;
let (fn_val, abi) = match func.layout.ty.kind {
ty::FnPtr(sig) => {
Expand All @@ -64,14 +66,23 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let sig = func.layout.ty.fn_sig(*self.tcx);
(FnVal::Instance(self.resolve(def_id, substs)?), sig.abi())
}
_ => bug!("invalid callee of type {:?}", func.layout.ty),
_ => span_bug!(
terminator.source_info.span,
"invalid callee of type {:?}",
func.layout.ty
),
};
let args = self.eval_operands(args)?;
let ret = match destination {
Some((dest, ret)) => Some((self.eval_place(dest)?, ret)),
None => None,
};
self.eval_fn_call(fn_val, abi, &args[..], ret, *cleanup)?;
// Sanity-check that `eval_fn_call` either pushed a new frame or
// did a jump to another block.
if self.cur_frame() == old_stack && self.frame().block == old_bb {
span_bug!(terminator.source_info.span, "evaluating this call made no progress");
}
}

Drop { location, target, unwind } => {
Expand Down Expand Up @@ -116,9 +127,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
| FalseEdges { .. }
| FalseUnwind { .. }
| Yield { .. }
| GeneratorDrop => {
bug!("{:#?} should have been eliminated by MIR pass", terminator.kind)
}
| GeneratorDrop => span_bug!(
terminator.source_info.span,
"{:#?} should have been eliminated by MIR pass",
terminator.kind
),
}

Ok(())
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}

fn eval_constant(&mut self, c: &Constant<'tcx>, source_info: SourceInfo) -> Option<OpTy<'tcx>> {
self.ecx.tcx.span = c.span;

// FIXME we need to revisit this for #67176
if c.needs_subst() {
return None;
Expand All @@ -435,6 +433,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
match self.ecx.eval_const_to_op(c.literal, None) {
Ok(op) => Some(op),
Err(error) => {
// Make sure errors point at the constant.
self.ecx.set_span(c.span);
let err = error_to_const_error(&self.ecx, error);
if let Some(lint_root) = self.lint_root(source_info) {
let lint_only = match c.literal.val {
Expand Down Expand Up @@ -820,6 +820,7 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
fn visit_statement(&mut self, statement: &mut Statement<'tcx>, location: Location) {
trace!("visit_statement: {:?}", statement);
let source_info = statement.source_info;
self.ecx.set_span(source_info.span);
self.source_info = Some(source_info);
if let StatementKind::Assign(box (place, ref mut rval)) = statement.kind {
let place_ty: Ty<'tcx> = place.ty(&self.local_decls, self.tcx).ty;
Expand Down Expand Up @@ -870,6 +871,7 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {

fn visit_terminator(&mut self, terminator: &mut Terminator<'tcx>, location: Location) {
let source_info = terminator.source_info;
self.ecx.set_span(source_info.span);
self.source_info = Some(source_info);
self.super_terminator(terminator, location);
match &mut terminator.kind {
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/consts/const-eval/issue-70723.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#![feature(const_loop)]

static _X: () = loop {}; //~ ERROR could not evaluate static initializer

fn main() {}
9 changes: 9 additions & 0 deletions src/test/ui/consts/const-eval/issue-70723.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0080]: could not evaluate static initializer
--> $DIR/issue-70723.rs:3:17
|
LL | static _X: () = loop {};
| ^^^^^^^ exceeded interpreter step limit (see `#[const_eval_limit]`)

error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.

0 comments on commit bd18bc9

Please sign in to comment.