Auto merge of #70771 - RalfJung:ctfe-loop, r=oli-obk

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 https://github.com/rust-lang/rust/issues/70723
r? @oli-obk

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::{
@@ -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>> {
@@ -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)
         }
@@ -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,
@@ -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);

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);

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)? {

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,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);
             }

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) => {
@@ -64,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 {
@@ -72,6 +78,11 @@ 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.
+                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 } => {
@@ -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(())

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<OpTy<'tcx>> {
-        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 {

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() {}

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`.