Auto merge of #75396 - RalfJung:miri-spans, r=oli-obk

Miri: improve spans of required_const failures

In https://github.com/rust-lang/rust/pull/75339 I added a loop evaluating all consts required by a function body. Unfortunately, if one of their evaluations fails, then the span used for that was that of the first statement in the function body, which happened to work form some existing test but is not sensible in general.

This PR changes it to point to the whole function instead, which is at least not wrong.

r? @oli-obk
This commit is contained in:
bors 2020-08-12 20:44:19 +00:00
commit 576d27c5a6
11 changed files with 69 additions and 56 deletions

View File

@ -301,12 +301,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
Ok(())
}
fn after_stack_push(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
// Enforce stack size limit.
if !ecx.tcx.sess.recursion_limit().value_within_limit(ecx.stack().len()) {
#[inline(always)]
fn init_frame_extra(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
frame: Frame<'mir, 'tcx>,
) -> InterpResult<'tcx, Frame<'mir, 'tcx>> {
// Enforce stack size limit. Add 1 because this is run before the new frame is pushed.
if !ecx.tcx.sess.recursion_limit().value_within_limit(ecx.stack().len() + 1) {
throw_exhaust!(StackFrameLimitReached)
} else {
Ok(())
Ok(frame)
}
}

View File

@ -16,7 +16,7 @@ use rustc_middle::ty::layout::{self, TyAndLayout};
use rustc_middle::ty::{
self, query::TyCtxtAt, subst::SubstsRef, ParamEnv, Ty, TyCtxt, TypeFoldable,
};
use rustc_span::{source_map::DUMMY_SP, Pos, Span};
use rustc_span::{Pos, Span};
use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Size, TargetDataLayout};
use super::{
@ -83,9 +83,11 @@ pub struct Frame<'mir, 'tcx, Tag = (), Extra = ()> {
////////////////////////////////////////////////////////////////////////////////
// Current position within the function
////////////////////////////////////////////////////////////////////////////////
/// If this is `None`, we are unwinding and this function doesn't need any clean-up.
/// Just continue the same as with `Resume`.
pub loc: Option<mir::Location>,
/// If this is `Err`, we are not currently executing any particular statement in
/// this frame (can happen e.g. during frame initialization, and during unwinding on
/// frames without cleanup code).
/// We basically abuse `Result` as `Either`.
pub(super) loc: Result<mir::Location, Span>,
}
/// What we store about a frame in an interpreter backtrace.
@ -189,7 +191,14 @@ impl<'mir, 'tcx, Tag> Frame<'mir, 'tcx, Tag> {
impl<'mir, 'tcx, Tag, Extra> Frame<'mir, 'tcx, Tag, Extra> {
/// Return the `SourceInfo` of the current instruction.
pub fn current_source_info(&self) -> Option<&mir::SourceInfo> {
self.loc.map(|loc| self.body.source_info(loc))
self.loc.ok().map(|loc| self.body.source_info(loc))
}
pub fn current_span(&self) -> Span {
match self.loc {
Ok(loc) => self.body.source_info(loc).span,
Err(span) => span,
}
}
}
@ -324,11 +333,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
#[inline(always)]
pub fn cur_span(&self) -> Span {
self.stack()
.last()
.and_then(|f| f.current_source_info())
.map(|si| si.span)
.unwrap_or(self.tcx.span)
self.stack().last().map(|f| f.current_span()).unwrap_or(self.tcx.span)
}
#[inline(always)]
@ -640,7 +645,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// first push a stack frame so we have access to the local substs
let pre_frame = Frame {
body,
loc: Some(mir::Location::START),
loc: Err(body.span), // Span used for errors caused during preamble.
return_to_block,
return_place,
// empty local array, we fill it in below, after we are inside the stack frame and
@ -654,9 +659,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Make sure all the constants required by this frame evaluate successfully (post-monomorphization check).
for const_ in &body.required_consts {
let span = const_.span;
let const_ =
self.subst_from_current_frame_and_normalize_erasing_regions(const_.literal);
self.const_to_op(const_, None)?;
self.const_to_op(const_, None).map_err(|err| {
// If there was an error, set the span of the current frame to this constant.
// Avoiding doing this when evaluation succeeds.
self.frame_mut().loc = Err(span);
err
})?;
}
// Locals are initially uninitialized.
@ -683,8 +694,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
// done
self.frame_mut().locals = locals;
M::after_stack_push(self)?;
self.frame_mut().loc = Ok(mir::Location::START);
info!("ENTERING({}) {}", self.frame_idx(), self.frame().instance);
Ok(())
@ -693,7 +704,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Jump to the given block.
#[inline]
pub fn go_to_block(&mut self, target: mir::BasicBlock) {
self.frame_mut().loc = Some(mir::Location { block: target, statement_index: 0 });
self.frame_mut().loc = Ok(mir::Location { block: target, statement_index: 0 });
}
/// *Return* to the given `target` basic block.
@ -715,7 +726,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// If `target` is `None`, that indicates the function does not need cleanup during
/// unwinding, and we will just keep propagating that upwards.
pub fn unwind_to_block(&mut self, target: Option<mir::BasicBlock>) {
self.frame_mut().loc = target.map(|block| mir::Location { block, statement_index: 0 });
self.frame_mut().loc = match target {
Some(block) => Ok(mir::Location { block, statement_index: 0 }),
None => Err(self.frame_mut().body.span),
};
}
/// Pops the current frame from the stack, deallocating the
@ -743,8 +757,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
assert_eq!(
unwinding,
match self.frame().loc {
None => true,
Some(loc) => self.body().basic_blocks()[loc.block].is_cleanup,
Ok(loc) => self.body().basic_blocks()[loc.block].is_cleanup,
Err(_) => true,
}
);
@ -920,14 +934,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn generate_stacktrace(&self) -> Vec<FrameInfo<'tcx>> {
let mut frames = Vec::new();
for frame in self.stack().iter().rev() {
let source_info = frame.current_source_info();
let lint_root = source_info.and_then(|source_info| {
let lint_root = frame.current_source_info().and_then(|source_info| {
match &frame.body.source_scopes[source_info.scope].local_data {
mir::ClearCrossCrate::Set(data) => Some(data.lint_root),
mir::ClearCrossCrate::Clear => None,
}
});
let span = source_info.map_or(DUMMY_SP, |source_info| source_info.span);
let span = frame.current_span();
frames.push(FrameInfo { span, instance: frame.instance, lint_root });
}

View File

@ -30,8 +30,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Assert that there is always such a frame.
.unwrap();
// Assert that the frame we look at is actually executing code currently
// (`current_source_info` is None when we are unwinding and the frame does
// not require cleanup).
// (`loc` is `Err` when we are unwinding and the frame does not require cleanup).
let loc = frame.loc.unwrap();
// If this is a `Call` terminator, use the `fn_span` instead.
let block = &frame.body.basic_blocks()[loc.block];

View File

@ -409,12 +409,4 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
) -> Self::PointerTag {
()
}
#[inline(always)]
fn init_frame_extra(
_ecx: &mut InterpCx<$mir, $tcx, Self>,
frame: Frame<$mir, $tcx>,
) -> InterpResult<$tcx, Frame<$mir, $tcx>> {
Ok(frame)
}
}

View File

@ -47,8 +47,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
let loc = match self.frame().loc {
Some(loc) => loc,
None => {
Ok(loc) => loc,
Err(_) => {
// We are unwinding and this fn has no cleanup code.
// Just go on unwinding.
trace!("unwinding: skipping frame");
@ -283,7 +283,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.eval_terminator(terminator)?;
if !self.stack().is_empty() {
if let Some(loc) = self.frame().loc {
if let Ok(loc) = self.frame().loc {
info!("// executing {:?}", loc.block);
}
}

View File

@ -281,6 +281,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
Ok(())
}
#[inline(always)]
fn init_frame_extra(
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
frame: Frame<'mir, 'tcx>,
) -> InterpResult<'tcx, Frame<'mir, 'tcx>> {
Ok(frame)
}
#[inline(always)]
fn stack(
ecx: &'a InterpCx<'mir, 'tcx, Self>,

View File

@ -24,17 +24,17 @@ error: any use of this value will cause an error
--> $DIR/const-err-multi.rs:7:19
|
LL | pub const C: u8 = A as u8;
| ------------------^^^^^^^-
| ------------------^-------
| |
| referenced constant has errors
error: any use of this value will cause an error
--> $DIR/const-err-multi.rs:9:19
--> $DIR/const-err-multi.rs:9:24
|
LL | pub const D: i8 = 50 - A;
| ------------------^^^^^^-
| |
| referenced constant has errors
| -----------------------^-
| |
| referenced constant has errors
error: aborting due to 4 previous errors

View File

@ -8,8 +8,8 @@ impl<T> PrintName<T> {
}
const fn no_codegen<T>() {
if false { //~ERROR evaluation of constant value failed
let _ = PrintName::<T>::VOID;
if false {
let _ = PrintName::<T>::VOID; //~ERROR evaluation of constant value failed
}
}

View File

@ -25,12 +25,10 @@ LL | #![warn(const_err, unconditional_panic)]
| ^^^^^^^^^
error[E0080]: evaluation of constant value failed
--> $DIR/erroneous-const.rs:11:5
--> $DIR/erroneous-const.rs:12:17
|
LL | / if false {
LL | | let _ = PrintName::<T>::VOID;
LL | | }
| |_____^ referenced constant has errors
LL | let _ = PrintName::<T>::VOID;
| ^^^^^^^^^^^^^^^^^^^^ referenced constant has errors
error[E0080]: could not evaluate static initializer
--> $DIR/erroneous-const.rs:16:22

View File

@ -1,11 +1,11 @@
// build-fail
pub const unsafe fn fake_type<T>() -> T {
hint_unreachable() //~ ERROR evaluation of constant value failed
hint_unreachable()
}
pub const unsafe fn hint_unreachable() -> ! {
fake_type()
fake_type() //~ ERROR evaluation of constant value failed
}
trait Const {

View File

@ -1,11 +1,9 @@
error[E0080]: evaluation of constant value failed
--> $DIR/uninhabited-const-issue-61744.rs:4:5
--> $DIR/uninhabited-const-issue-61744.rs:8:5
|
LL | hint_unreachable()
| ^^^^^^^^^^^^^^^^^^
| ------------------
| |
| reached the configured maximum number of stack frames
| inside `fake_type::<!>` at $DIR/uninhabited-const-issue-61744.rs:4:5
| inside `fake_type::<!>` at $DIR/uninhabited-const-issue-61744.rs:4:5
| inside `fake_type::<!>` at $DIR/uninhabited-const-issue-61744.rs:4:5
| inside `fake_type::<!>` at $DIR/uninhabited-const-issue-61744.rs:4:5
@ -72,8 +70,9 @@ LL | hint_unreachable()
| inside `fake_type::<i32>` at $DIR/uninhabited-const-issue-61744.rs:4:5
...
LL | fake_type()
| -----------
| ^^^^^^^^^^^
| |
| reached the configured maximum number of stack frames
| inside `hint_unreachable` at $DIR/uninhabited-const-issue-61744.rs:8:5
| inside `hint_unreachable` at $DIR/uninhabited-const-issue-61744.rs:8:5
| inside `hint_unreachable` at $DIR/uninhabited-const-issue-61744.rs:8:5