From 0633a0e3801e4efc9ab07bf811e442bd379ce93a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 8 Feb 2020 22:21:20 +0100 Subject: [PATCH] remove Panic variant from InterpError --- src/librustc/mir/interpret/error.rs | 22 +++-- src/librustc/mir/interpret/mod.rs | 14 ---- src/librustc_mir/const_eval/error.rs | 21 +++-- src/librustc_mir/const_eval/machine.rs | 62 ++++++++++---- src/librustc_mir/interpret/intrinsics.rs | 26 ------ .../interpret/intrinsics/caller_location.rs | 12 +-- src/librustc_mir/transform/const_prop.rs | 83 +++++++++---------- 7 files changed, 117 insertions(+), 123 deletions(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 86e7bb28e00..e819dfbacd8 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -139,7 +139,6 @@ impl<'tcx> ConstEvalErr<'tcx> { lint_root: Option, ) -> Result<(), ErrorHandled> { let must_error = match self.error { - InterpError::MachineStop(_) => bug!("CTFE does not stop"), err_inval!(Layout(LayoutError::Unknown(_))) | err_inval!(TooGeneric) => { return Err(ErrorHandled::TooGeneric); } @@ -149,9 +148,18 @@ impl<'tcx> ConstEvalErr<'tcx> { }; trace!("reporting const eval failure at {:?}", self.span); + let err_msg = match &self.error { + InterpError::MachineStop(msg) => { + // A custom error (`ConstEvalErrKind` in `librustc_mir/interp/const_eval/error.rs`). + // Should be turned into a string by now. + msg.downcast_ref::().expect("invalid MachineStop payload").clone() + } + err => err.to_string(), + }; + let add_span_labels = |err: &mut DiagnosticBuilder<'_>| { if !must_error { - err.span_label(self.span, self.error.to_string()); + err.span_label(self.span, err_msg.clone()); } // Skip the last, which is just the environment of the constant. The stacktrace // is sometimes empty because we create "fake" eval contexts in CTFE to do work @@ -183,7 +191,7 @@ impl<'tcx> ConstEvalErr<'tcx> { ); } else { let mut err = if must_error { - struct_error(tcx, &self.error.to_string()) + struct_error(tcx, &err_msg) } else { struct_error(tcx, message) }; @@ -259,6 +267,9 @@ impl<'tcx> From> for InterpErrorInfo<'tcx> { } } +/// Information about a panic. +/// +/// FIXME: this is not actually an InterpError, and should probably be moved to another module. #[derive(Clone, RustcEncodable, RustcDecodable, HashStable, PartialEq)] pub enum PanicInfo { Panic { msg: Symbol, line: u32, col: u32, file: Symbol }, @@ -616,8 +627,6 @@ impl fmt::Debug for ResourceExhaustionInfo { } pub enum InterpError<'tcx> { - /// The program panicked. - Panic(PanicInfo), /// The program caused undefined behavior. UndefinedBehavior(UndefinedBehaviorInfo), /// The program did something the interpreter does not support (some of these *might* be UB @@ -650,8 +659,7 @@ impl fmt::Debug for InterpError<'_> { InvalidProgram(ref msg) => write!(f, "{:?}", msg), UndefinedBehavior(ref msg) => write!(f, "{:?}", msg), ResourceExhaustion(ref msg) => write!(f, "{:?}", msg), - Panic(ref msg) => write!(f, "{:?}", msg), - MachineStop(_) => write!(f, "machine caused execution to stop"), + MachineStop(_) => bug!("unhandled MachineStop"), } } } diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 54e196f4b33..b4cfd860152 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -37,15 +37,6 @@ macro_rules! err_ub_format { ($($tt:tt)*) => { err_ub!(Ub(format!($($tt)*))) }; } -#[macro_export] -macro_rules! err_panic { - ($($tt:tt)*) => { - $crate::mir::interpret::InterpError::Panic( - $crate::mir::interpret::PanicInfo::$($tt)* - ) - }; -} - #[macro_export] macro_rules! err_exhaust { ($($tt:tt)*) => { @@ -80,11 +71,6 @@ macro_rules! throw_ub_format { ($($tt:tt)*) => { throw_ub!(Ub(format!($($tt)*))) }; } -#[macro_export] -macro_rules! throw_panic { - ($($tt:tt)*) => { return Err(err_panic!($($tt)*).into()) }; -} - #[macro_export] macro_rules! throw_exhaust { ($($tt:tt)*) => { return Err(err_exhaust!($($tt)*).into()) }; diff --git a/src/librustc_mir/const_eval/error.rs b/src/librustc_mir/const_eval/error.rs index c2db3c31f85..e0e78546099 100644 --- a/src/librustc_mir/const_eval/error.rs +++ b/src/librustc_mir/const_eval/error.rs @@ -2,32 +2,39 @@ use std::error::Error; use std::fmt; use super::InterpCx; -use crate::interpret::{ConstEvalErr, InterpErrorInfo, Machine}; +use crate::interpret::{ConstEvalErr, InterpError, InterpErrorInfo, Machine, PanicInfo}; + +/// The CTFE machine has some custom error kinds. #[derive(Clone, Debug)] -pub enum ConstEvalError { +pub enum ConstEvalErrKind { NeedsRfc(String), ConstAccessesStatic, + Panic(PanicInfo), } -impl<'tcx> Into> for ConstEvalError { +// The errors become `MachineStop` with plain strings when being raised. +// `ConstEvalErr` (in `librustc/mir/interpret/error.rs`) knows to +// handle these. +impl<'tcx> Into> for ConstEvalErrKind { fn into(self) -> InterpErrorInfo<'tcx> { - err_unsup!(Unsupported(self.to_string())).into() + InterpError::MachineStop(Box::new(self.to_string())).into() } } -impl fmt::Display for ConstEvalError { +impl fmt::Display for ConstEvalErrKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use self::ConstEvalError::*; + use self::ConstEvalErrKind::*; match *self { NeedsRfc(ref msg) => { write!(f, "\"{}\" needs an rfc before being allowed inside constants", msg) } ConstAccessesStatic => write!(f, "constant accesses static"), + Panic(ref msg) => write!(f, "{:?}", msg), } } } -impl Error for ConstEvalError {} +impl Error for ConstEvalErrKind {} /// Turn an interpreter error into something to report to the user. /// As a side-effect, if RUSTC_CTFE_BACKTRACE is set, this prints the backtrace. diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index 1aed91baba6..688eee05dda 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -9,10 +9,11 @@ use std::hash::Hash; use rustc_data_structures::fx::FxHashMap; use rustc_span::source_map::Span; +use rustc_span::symbol::Symbol; use crate::interpret::{ self, snapshot, AllocId, Allocation, AssertMessage, GlobalId, ImmTy, InterpCx, InterpResult, - Memory, MemoryKind, OpTy, PlaceTy, Pointer, Scalar, + Memory, MemoryKind, OpTy, PanicInfo, PlaceTy, Pointer, Scalar, }; use super::error::*; @@ -56,6 +57,32 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> { self.dump_place(*dest); return Ok(true); } + + /// "Intercept" a function call to a panic-related function + /// because we have something special to do for it. + /// Returns `true` if an intercept happened. + pub fn hook_panic_fn( + &mut self, + span: Span, + instance: ty::Instance<'tcx>, + args: &[OpTy<'tcx>], + ) -> InterpResult<'tcx, bool> { + let def_id = instance.def_id(); + if Some(def_id) == self.tcx.lang_items().panic_fn() + || Some(def_id) == self.tcx.lang_items().begin_panic_fn() + { + // &'static str + assert!(args.len() == 1); + + let msg_place = self.deref_operand(args[0])?; + let msg = Symbol::intern(self.read_str(msg_place)?); + let span = self.find_closest_untracked_caller_location().unwrap_or(span); + let (file, line, col) = self.location_triple_for_span(span); + Err(ConstEvalErrKind::Panic(PanicInfo::Panic { msg, file, line, col }).into()) + } else { + Ok(false) + } + } } /// Number of steps until the detector even starts doing anything. @@ -212,7 +239,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, Ok(body) => *body, Err(err) => { if let err_unsup!(NoMirFor(ref path)) = err.kind { - return Err(ConstEvalError::NeedsRfc(format!( + return Err(ConstEvalErrKind::NeedsRfc(format!( "calling extern function `{}`", path )) @@ -246,7 +273,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, } // An intrinsic that we do not support let intrinsic_name = ecx.tcx.item_name(instance.def_id()); - Err(ConstEvalError::NeedsRfc(format!("calling intrinsic `{}`", intrinsic_name)).into()) + Err(ConstEvalErrKind::NeedsRfc(format!("calling intrinsic `{}`", intrinsic_name)).into()) } fn assert_panic( @@ -256,7 +283,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, _unwind: Option, ) -> InterpResult<'tcx> { use rustc::mir::interpret::PanicInfo::*; - Err(match msg { + // Convert `PanicInfo` to `PanicInfo`. + let err = match msg { BoundsCheck { ref len, ref index } => { let len = ecx .read_immediate(ecx.eval_operand(len, None)?) @@ -268,21 +296,21 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, .expect("can't eval index") .to_scalar()? .to_machine_usize(&*ecx)?; - err_panic!(BoundsCheck { len, index }) + BoundsCheck { len, index } } - Overflow(op) => err_panic!(Overflow(*op)), - OverflowNeg => err_panic!(OverflowNeg), - DivisionByZero => err_panic!(DivisionByZero), - RemainderByZero => err_panic!(RemainderByZero), - ResumedAfterReturn(generator_kind) => err_panic!(ResumedAfterReturn(*generator_kind)), - ResumedAfterPanic(generator_kind) => err_panic!(ResumedAfterPanic(*generator_kind)), + Overflow(op) => Overflow(*op), + OverflowNeg => OverflowNeg, + DivisionByZero => DivisionByZero, + RemainderByZero => RemainderByZero, + ResumedAfterReturn(generator_kind) => ResumedAfterReturn(*generator_kind), + ResumedAfterPanic(generator_kind) => ResumedAfterPanic(*generator_kind), Panic { .. } => bug!("`Panic` variant cannot occur in MIR"), - } - .into()) + }; + Err(ConstEvalErrKind::Panic(err).into()) } fn ptr_to_int(_mem: &Memory<'mir, 'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx, u64> { - Err(ConstEvalError::NeedsRfc("pointer-to-integer cast".to_string()).into()) + Err(ConstEvalErrKind::NeedsRfc("pointer-to-integer cast".to_string()).into()) } fn binary_ptr_op( @@ -291,7 +319,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, _left: ImmTy<'tcx>, _right: ImmTy<'tcx>, ) -> InterpResult<'tcx, (Scalar, bool, Ty<'tcx>)> { - Err(ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into()) + Err(ConstEvalErrKind::NeedsRfc("pointer arithmetic or comparison".to_string()).into()) } fn find_foreign_static( @@ -321,7 +349,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, _ecx: &mut InterpCx<'mir, 'tcx, Self>, _dest: PlaceTy<'tcx>, ) -> InterpResult<'tcx> { - Err(ConstEvalError::NeedsRfc("heap allocations via `box` keyword".to_string()).into()) + Err(ConstEvalErrKind::NeedsRfc("heap allocations via `box` keyword".to_string()).into()) } fn before_terminator(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { @@ -355,7 +383,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, if memory_extra.can_access_statics { Ok(()) } else { - Err(ConstEvalError::ConstAccessesStatic.into()) + Err(ConstEvalErrKind::ConstAccessesStatic.into()) } } } diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index f85da760ada..a83b5412790 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -376,32 +376,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(true) } - /// "Intercept" a function call to a panic-related function - /// because we have something special to do for it. - /// Returns `true` if an intercept happened. - pub fn hook_panic_fn( - &mut self, - span: Span, - instance: ty::Instance<'tcx>, - args: &[OpTy<'tcx, M::PointerTag>], - ) -> InterpResult<'tcx, bool> { - let def_id = instance.def_id(); - if Some(def_id) == self.tcx.lang_items().panic_fn() - || Some(def_id) == self.tcx.lang_items().begin_panic_fn() - { - // &'static str - assert!(args.len() == 1); - - let msg_place = self.deref_operand(args[0])?; - let msg = Symbol::intern(self.read_str(msg_place)?); - let span = self.find_closest_untracked_caller_location().unwrap_or(span); - let (file, line, col) = self.location_triple_for_span(span); - throw_panic!(Panic { msg, file, line, col }) - } else { - return Ok(false); - } - } - pub fn exact_div( &mut self, a: ImmTy<'tcx, M::PointerTag>, diff --git a/src/librustc_mir/interpret/intrinsics/caller_location.rs b/src/librustc_mir/interpret/intrinsics/caller_location.rs index 0525108d2d1..566601f0cae 100644 --- a/src/librustc_mir/interpret/intrinsics/caller_location.rs +++ b/src/librustc_mir/interpret/intrinsics/caller_location.rs @@ -54,12 +54,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { location } - pub fn alloc_caller_location_for_span(&mut self, span: Span) -> MPlaceTy<'tcx, M::PointerTag> { - let (file, line, column) = self.location_triple_for_span(span); - self.alloc_caller_location(file, line, column) - } - - pub(super) fn location_triple_for_span(&self, span: Span) -> (Symbol, u32, u32) { + crate fn location_triple_for_span(&self, span: Span) -> (Symbol, u32, u32) { let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span); let caller = self.tcx.sess.source_map().lookup_char_pos(topmost.lo()); ( @@ -68,4 +63,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { caller.col_display as u32 + 1, ) } + + pub fn alloc_caller_location_for_span(&mut self, span: Span) -> MPlaceTy<'tcx, M::PointerTag> { + let (file, line, column) = self.location_triple_for_span(span); + self.alloc_caller_location(file, line, column) + } } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index a7da4f7c164..43a6382646f 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -4,7 +4,7 @@ use std::borrow::Cow; use std::cell::Cell; -use rustc::mir::interpret::{InterpResult, PanicInfo, Scalar}; +use rustc::mir::interpret::{InterpError, InterpResult, PanicInfo, Scalar}; use rustc::mir::visit::{ MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor, }; @@ -25,7 +25,7 @@ use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_hir::HirId; use rustc_index::vec::IndexVec; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::Span; use syntax::ast::Mutability; use crate::const_eval::error_to_const_error; @@ -410,15 +410,10 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } } - fn use_ecx(&mut self, source_info: SourceInfo, f: F) -> Option + fn use_ecx(&mut self, f: F) -> Option where F: FnOnce(&mut Self) -> InterpResult<'tcx, T>, { - self.ecx.tcx.span = source_info.span; - // FIXME(eddyb) move this to the `Panic(_)` error case, so that - // `f(self)` is always called, and that the only difference when the - // scope's `local_data` is missing, is that the lint isn't emitted. - let lint_root = self.lint_root(source_info)?; let r = match f(self) { Ok(val) => Some(val), Err(error) => { @@ -447,20 +442,10 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { | ResourceExhaustion(_) => { // Ignore these errors. } - Panic(_) => { - let diagnostic = error_to_const_error(&self.ecx, error); - diagnostic.report_as_lint( - self.ecx.tcx, - "this expression will panic at runtime", - lint_root, - None, - ); - } } None } }; - self.ecx.tcx.span = DUMMY_SP; r } @@ -504,37 +489,47 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } } - fn eval_place(&mut self, place: &Place<'tcx>, source_info: SourceInfo) -> Option> { + fn eval_place(&mut self, place: &Place<'tcx>) -> Option> { trace!("eval_place(place={:?})", place); - self.use_ecx(source_info, |this| this.ecx.eval_place_to_op(place, None)) + self.use_ecx(|this| this.ecx.eval_place_to_op(place, None)) } fn eval_operand(&mut self, op: &Operand<'tcx>, source_info: SourceInfo) -> Option> { match *op { Operand::Constant(ref c) => self.eval_constant(c, source_info), - Operand::Move(ref place) | Operand::Copy(ref place) => { - self.eval_place(place, source_info) - } + Operand::Move(ref place) | Operand::Copy(ref place) => self.eval_place(place), } } + fn report_panic_as_lint(&self, source_info: SourceInfo, panic: PanicInfo) -> Option<()> { + // Somewhat convoluted way to re-use the CTFE error reporting code. + let lint_root = self.lint_root(source_info)?; + let error = InterpError::MachineStop(Box::new(format!("{:?}", panic))); + let mut diagnostic = error_to_const_error(&self.ecx, error.into()); + diagnostic.span = source_info.span; // fix the span + diagnostic.report_as_lint( + self.tcx.at(source_info.span), + "this expression will panic at runtime", + lint_root, + None, + ); + None + } + fn check_unary_op( &mut self, op: UnOp, arg: &Operand<'tcx>, source_info: SourceInfo, ) -> Option<()> { - self.use_ecx(source_info, |this| { + if self.use_ecx(|this| { let val = this.ecx.read_immediate(this.ecx.eval_operand(arg, None)?)?; let (_res, overflow, _ty) = this.ecx.overflowing_unary_op(op, val)?; - - if overflow { - assert_eq!(op, UnOp::Neg, "Neg is the only UnOp that can overflow"); - throw_panic!(OverflowNeg); - } - - Ok(()) - })?; + Ok(overflow) + })? { + assert_eq!(op, UnOp::Neg, "Neg is the only UnOp that can overflow"); + self.report_panic_as_lint(source_info, PanicInfo::OverflowNeg)?; + } Some(()) } @@ -548,9 +543,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { place_layout: TyLayout<'tcx>, overflow_check: bool, ) -> Option<()> { - let r = self.use_ecx(source_info, |this| { - this.ecx.read_immediate(this.ecx.eval_operand(right, None)?) - })?; + let r = + self.use_ecx(|this| this.ecx.read_immediate(this.ecx.eval_operand(right, None)?))?; if op == BinOp::Shr || op == BinOp::Shl { let left_bits = place_layout.size.bits(); let right_size = r.layout.size; @@ -575,16 +569,13 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { // in MIR. However, if overflow checking is disabled, then there won't be any // `Assert` statement and so we have to do additional checking here. if !overflow_check { - self.use_ecx(source_info, |this| { + if self.use_ecx(|this| { let l = this.ecx.read_immediate(this.ecx.eval_operand(left, None)?)?; let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?; - - if overflow { - throw_panic!(Overflow(op)); - } - - Ok(()) - })?; + Ok(overflow) + })? { + self.report_panic_as_lint(source_info, PanicInfo::Overflow(op))?; + } } Some(()) @@ -642,7 +633,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { _ => {} } - self.use_ecx(source_info, |this| { + self.use_ecx(|this| { trace!("calling eval_rvalue_into_place(rvalue = {:?}, place = {:?})", rvalue, place); this.ecx.eval_rvalue_into_place(rvalue, place)?; Ok(()) @@ -675,7 +666,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } // FIXME> figure out what tho do when try_read_immediate fails - let imm = self.use_ecx(source_info, |this| this.ecx.try_read_immediate(value)); + let imm = self.use_ecx(|this| this.ecx.try_read_immediate(value)); if let Some(Ok(imm)) = imm { match *imm { @@ -698,7 +689,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { if let ty::Tuple(substs) = ty { // Only do it if tuple is also a pair with two scalars if substs.len() == 2 { - let opt_ty1_ty2 = self.use_ecx(source_info, |this| { + let opt_ty1_ty2 = self.use_ecx(|this| { let ty1 = substs[0].expect_ty(); let ty2 = substs[1].expect_ty(); let ty_is_scalar = |ty| {