From a524a9af918df6a6d3abd68a642e89f0d34b36f5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 4 Apr 2020 15:53:47 +0200 Subject: [PATCH 1/5] Miri terminator handling: only do progress sanity check for 'Call' terminator --- src/librustc_mir/interpret/step.rs | 5 ----- src/librustc_mir/interpret/terminator.rs | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 407849c2ce27c..2dd732e41eec2 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -279,13 +279,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { 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.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); } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 6ebe5b80370ca..161fbdc9ed45a 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -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) => { @@ -72,6 +74,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { 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. + assert!(self.cur_frame() != old_stack || self.frame().block != old_bb); } Drop { location, target, unwind } => { From 0ad3b1c9bf55a93db75136cfed58975b3bbb5ff2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 4 Apr 2020 16:57:54 +0200 Subject: [PATCH 2/5] add test --- src/test/ui/consts/const-eval/issue-70723.rs | 5 +++++ src/test/ui/consts/const-eval/issue-70723.stderr | 9 +++++++++ 2 files changed, 14 insertions(+) create mode 100644 src/test/ui/consts/const-eval/issue-70723.rs create mode 100644 src/test/ui/consts/const-eval/issue-70723.stderr diff --git a/src/test/ui/consts/const-eval/issue-70723.rs b/src/test/ui/consts/const-eval/issue-70723.rs new file mode 100644 index 0000000000000..8b79d5d53c5c2 --- /dev/null +++ b/src/test/ui/consts/const-eval/issue-70723.rs @@ -0,0 +1,5 @@ +#![feature(const_loop)] + +static _X: () = loop {}; //~ ERROR could not evaluate static initializer + +fn main() {} diff --git a/src/test/ui/consts/const-eval/issue-70723.stderr b/src/test/ui/consts/const-eval/issue-70723.stderr new file mode 100644 index 0000000000000..687d6565a7163 --- /dev/null +++ b/src/test/ui/consts/const-eval/issue-70723.stderr @@ -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`. From fb9b693acc4cfbb9ba138e2de037049dd08df267 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 5 Apr 2020 08:35:31 +0200 Subject: [PATCH 3/5] Miri engine: use span_bug in a few places --- src/librustc_mir/interpret/eval_context.rs | 18 ++++++++++-------- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/place.rs | 14 ++++++++------ src/librustc_mir/interpret/terminator.rs | 18 +++++++++++++----- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 0b182d4228715..54403275ba6ad 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -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>, compute: impl FnOnce() -> InterpResult<'tcx, TyAndLayout<'tcx>>, ) -> InterpResult<'tcx, TyAndLayout<'tcx>> { @@ -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) } @@ -444,7 +446,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); diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 03614b2803f65..3741f31927e94 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -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); diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 716c7c7d93367..828df9a0930f5 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -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)? { diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 161fbdc9ed45a..2d8551b2bbf1e 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -66,7 +66,11 @@ 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 { @@ -76,7 +80,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { 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. - assert!(self.cur_frame() != old_stack || self.frame().block != old_bb); + 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 } => { @@ -121,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(()) From 43cdfbb3fe01c58a24a0a92bae9f6d26c480f438 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 5 Apr 2020 09:27:14 +0200 Subject: [PATCH 4/5] set span more accurately during const_prop --- src/librustc_mir/interpret/eval_context.rs | 8 +++++++- src/librustc_mir/interpret/step.rs | 6 ++---- src/librustc_mir/transform/const_prop.rs | 6 ++++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 54403275ba6ad..3c5f091f46df7 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -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::{ @@ -296,6 +296,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } + #[inline(always)] + pub(crate) fn set_span(&mut self, span: Span) { + self.tcx.span = span; + self.memory.tcx.span = span; + } + #[inline(always)] pub fn force_ptr( &self, diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 2dd732e41eec2..37740878f7043 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -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)?, @@ -276,8 +275,7 @@ 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; + self.set_span(terminator.source_info.span); self.eval_terminator(terminator)?; if !self.stack.is_empty() { diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index c7f63d24c28ab..25719d037f9fc 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -425,8 +425,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } fn eval_constant(&mut self, c: &Constant<'tcx>, source_info: SourceInfo) -> Option> { - self.ecx.tcx.span = c.span; - // FIXME we need to revisit this for #67176 if c.needs_subst() { return None; @@ -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 { @@ -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; @@ -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 { From e92bde373a2f4e3cc5d80233d327de15596c3000 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 5 Apr 2020 17:37:35 +0200 Subject: [PATCH 5/5] make set_span public, as all the fields it touches are public already --- src/librustc_mir/interpret/eval_context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 3c5f091f46df7..72d20644fe8b2 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -297,7 +297,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } #[inline(always)] - pub(crate) fn set_span(&mut self, span: Span) { + pub fn set_span(&mut self, span: Span) { self.tcx.span = span; self.memory.tcx.span = span; }