Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miri: make backtrace function names and spans match up #70590

Merged
merged 7 commits into from
Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 8 additions & 13 deletions src/librustc_middle/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ pub struct ConstEvalErr<'tcx> {

#[derive(Debug)]
pub struct FrameInfo<'tcx> {
/// This span is in the caller.
pub call_site: Span,
pub instance: ty::Instance<'tcx>,
pub span: Span,
pub lint_root: Option<hir::HirId>,
}

Expand All @@ -65,12 +64,12 @@ impl<'tcx> fmt::Display for FrameInfo<'tcx> {
if tcx.def_key(self.instance.def_id()).disambiguated_data.data
== DefPathData::ClosureExpr
{
write!(f, "inside call to closure")?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My original intention was to mean "inside this call to", but I guess that's obvious from the call site being displayed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that text was always attached to the call site, not the inside of the call.

write!(f, "inside closure")?;
} else {
write!(f, "inside call to `{}`", self.instance)?;
write!(f, "inside `{}`", self.instance)?;
}
if !self.call_site.is_dummy() {
let lo = tcx.sess.source_map().lookup_char_pos(self.call_site.lo());
if !self.span.is_dummy() {
let lo = tcx.sess.source_map().lookup_char_pos(self.span.lo());
write!(f, " at {}:{}:{}", lo.file.name, lo.line, lo.col.to_usize() + 1)?;
}
Ok(())
Expand Down Expand Up @@ -169,13 +168,9 @@ impl<'tcx> ConstEvalErr<'tcx> {
err.span_label(self.span, span_msg);
}
// Add spans for the stacktrace.
// 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
// on constant values.
if !self.stacktrace.is_empty() {
for frame_info in &self.stacktrace[..self.stacktrace.len() - 1] {
err.span_label(frame_info.call_site, frame_info.to_string());
}
// Skip the first, which is the place of the error.
for frame_info in self.stacktrace.iter().skip(1) {
err.span_label(frame_info.span, frame_info.to_string());
}
// Let the caller finish the job.
emit(err)
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,6 @@ pub fn error_to_const_error<'mir, 'tcx, M: Machine<'mir, 'tcx>>(
mut error: InterpErrorInfo<'tcx>,
) -> ConstEvalErr<'tcx> {
error.print_backtrace();
let stacktrace = ecx.generate_stacktrace(None);
let stacktrace = ecx.generate_stacktrace();
ConstEvalErr { error: error.kind, stacktrace, span: ecx.tcx.span }
}
1 change: 0 additions & 1 deletion src/librustc_mir/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ fn eval_body_using_ecx<'mir, 'tcx>(

ecx.push_stack_frame(
cid.instance,
body.span,
body,
Some(ret.into()),
StackPopCleanup::None { cleanup: false },
Expand Down
11 changes: 4 additions & 7 deletions src/librustc_mir/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use std::hash::Hash;
use rustc_data_structures::fx::FxHashMap;

use rustc_ast::ast::Mutability;
use rustc_hir::def_id::DefId;
use rustc_middle::mir::AssertMessage;
use rustc_span::symbol::Symbol;
use rustc_span::{def_id::DefId, Span};

use crate::interpret::{
self, AllocId, Allocation, GlobalId, ImmTy, InterpCx, InterpResult, Memory, MemoryKind, OpTy,
Expand Down Expand Up @@ -64,7 +64,6 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter> {
/// If this returns successfully (`Ok`), the function should just be evaluated normally.
fn hook_panic_fn(
&mut self,
span: Span,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx>],
) -> InterpResult<'tcx> {
Expand All @@ -77,7 +76,7 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter> {

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 span = self.find_closest_untracked_caller_location();
let (file, line, col) = self.location_triple_for_span(span);
Err(ConstEvalErrKind::Panic { msg, file, line, col }.into())
} else {
Expand Down Expand Up @@ -191,7 +190,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {

fn find_mir_or_eval_fn(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
span: Span,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx>],
ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>,
Expand All @@ -213,7 +211,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {
} else {
// Some functions we support even if they are non-const -- but avoid testing
// that for const fn!
ecx.hook_panic_fn(span, instance, args)?;
ecx.hook_panic_fn(instance, args)?;
// We certainly do *not* want to actually call the fn
// though, so be sure we return here.
throw_unsup_format!("calling non-const function `{}`", instance)
Expand Down Expand Up @@ -248,13 +246,12 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {

fn call_intrinsic(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
span: Span,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx>],
ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>,
_unwind: Option<mir::BasicBlock>,
) -> InterpResult<'tcx> {
if ecx.emulate_intrinsic(span, instance, args, ret)? {
if ecx.emulate_intrinsic(instance, args, ret)? {
return Ok(());
}
// An intrinsic that we do not support
Expand Down
33 changes: 7 additions & 26 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, Align, HasDataLayout, LayoutOf, Size, TyAnd
use rustc_middle::ty::query::TyCtxtAt;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
use rustc_span::source_map::{self, Span, DUMMY_SP};
use rustc_span::source_map::DUMMY_SP;

use super::{
Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy,
Expand Down Expand Up @@ -57,9 +57,6 @@ pub struct Frame<'mir, 'tcx, Tag = (), Extra = ()> {
/// The def_id and substs of the current function.
pub instance: ty::Instance<'tcx>,

/// The span of the call site.
pub span: source_map::Span,

/// Extra data for the machine.
pub extra: Extra,

Expand Down Expand Up @@ -502,7 +499,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn push_stack_frame(
&mut self,
instance: ty::Instance<'tcx>,
span: Span,
body: &'mir mir::Body<'tcx>,
return_place: Option<PlaceTy<'tcx, M::PointerTag>>,
return_to_block: StackPopCleanup,
Expand All @@ -522,7 +518,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// empty local array, we fill it in below, after we are inside the stack frame and
// all methods actually know about the frame
locals: IndexVec::new(),
span,
instance,
stmt: 0,
extra,
Expand All @@ -541,7 +536,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// statics and constants don't have `Storage*` statements, no need to look for them
Some(DefKind::Static) | Some(DefKind::Const) | Some(DefKind::AssocConst) => {}
_ => {
trace!("push_stack_frame: {:?}: num_bbs: {}", span, body.basic_blocks().len());
for block in body.basic_blocks() {
for stmt in block.statements.iter() {
use rustc_middle::mir::StatementKind::{StorageDead, StorageLive};
Expand Down Expand Up @@ -859,33 +853,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

pub fn generate_stacktrace(&self, explicit_span: Option<Span>) -> Vec<FrameInfo<'tcx>> {
let mut last_span = None;
pub fn generate_stacktrace(&self) -> Vec<FrameInfo<'tcx>> {
let mut frames = Vec::new();
for frame in self.stack().iter().rev() {
// make sure we don't emit frames that are duplicates of the previous
if explicit_span == Some(frame.span) {
last_span = Some(frame.span);
continue;
}
if let Some(last) = last_span {
if last == frame.span {
continue;
}
} else {
last_span = Some(frame.span);
}
oli-obk marked this conversation as resolved.
Show resolved Hide resolved

let lint_root = frame.current_source_info().and_then(|source_info| {
let source_info = frame.current_source_info();
let lint_root = 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);

frames.push(FrameInfo { call_site: frame.span, instance: frame.instance, lint_root });
frames.push(FrameInfo { span, instance: frame.instance, lint_root });
}
trace!("generate stacktrace: {:#?}, {:?}", frames, explicit_span);
trace!("generate stacktrace: {:#?}", frames);
frames
}
}
Expand All @@ -899,7 +881,6 @@ where
fn hash_stable(&self, hcx: &mut StableHashingContext<'ctx>, hasher: &mut StableHasher) {
self.body.hash_stable(hcx, hasher);
self.instance.hash_stable(hcx, hasher);
self.span.hash_stable(hcx, hasher);
self.return_to_block.hash_stable(hcx, hasher);
self.return_place.as_ref().map(|r| &**r).hash_stable(hcx, hasher);
self.locals.hash_stable(hcx, hasher);
Expand Down
6 changes: 2 additions & 4 deletions src/librustc_mir/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use rustc_middle::ty::layout::{LayoutOf, Primitive, Size};
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::Span;

use super::{ImmTy, InterpCx, Machine, OpTy, PlaceTy};

Expand Down Expand Up @@ -78,7 +77,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Returns `true` if emulation happened.
pub fn emulate_intrinsic(
&mut self,
span: Span,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, M::PointerTag>],
ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>,
Expand All @@ -101,7 +99,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// `src/librustc_middle/ty/constness.rs`
match intrinsic_name {
sym::caller_location => {
let span = self.find_closest_untracked_caller_location().unwrap_or(span);
let span = self.find_closest_untracked_caller_location();
let location = self.alloc_caller_location_for_span(span);
self.write_scalar(location.ptr, dest)?;
}
Expand All @@ -118,7 +116,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
sym::needs_drop => self.tcx.types.bool,
sym::type_id => self.tcx.types.u64,
sym::type_name => self.tcx.mk_static_str(),
_ => span_bug!(span, "Already checked for nullary intrinsics"),
_ => bug!("already checked for nullary intrinsics"),
};
let val = self.const_eval(gid, ty)?;
self.copy_op(val, dest)?;
Expand Down
23 changes: 13 additions & 10 deletions src/librustc_mir/interpret/intrinsics/caller_location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,19 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Walks up the callstack from the intrinsic's callsite, searching for the first callsite in a
/// frame which is not `#[track_caller]`. If the first frame found lacks `#[track_caller]`, then
/// `None` is returned and the callsite of the function invocation itself should be used.
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
crate fn find_closest_untracked_caller_location(&self) -> Option<Span> {
let mut caller_span = None;
for next_caller in self.stack.iter().rev() {
if !next_caller.instance.def.requires_caller_location(*self.tcx) {
return caller_span;
}
caller_span = Some(next_caller.span);
}

caller_span
crate fn find_closest_untracked_caller_location(&self) -> Span {
self.stack
.iter()
.rev()
// Skip `#[track_caller]` frames.
.skip_while(|frame| frame.instance.def.requires_caller_location(*self.tcx))
// Find next frame with source info.
.find_map(|frame| frame.current_source_info())
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
.map(|si| si.span)
// Fallback to current frame. That one has to have source_info as only
// currently unwinding frames without cleanup do *not* have it -- and those
// frames do not call this intrinsic.
.unwrap_or_else(|| self.frame().current_source_info().unwrap().span)
}

/// Allocate a `const core::panic::Location` with the provided filename and line/column numbers.
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::hash::Hash;

use rustc_middle::mir;
use rustc_middle::ty::{self, Ty};
use rustc_span::{def_id::DefId, Span};
use rustc_span::def_id::DefId;

use super::{
AllocId, Allocation, AllocationExtra, Frame, ImmTy, InterpCx, InterpResult, Memory, MemoryKind,
Expand Down Expand Up @@ -135,7 +135,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
/// was used.
fn find_mir_or_eval_fn(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
span: Span,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, Self::PointerTag>],
ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>,
Expand All @@ -156,7 +155,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
/// responsibility to advance the instruction pointer as appropriate.
fn call_intrinsic(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
span: Span,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, Self::PointerTag>],
ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>,
Expand Down
22 changes: 5 additions & 17 deletions src/librustc_mir/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::convert::TryFrom;
use rustc_middle::ty::layout::{self, LayoutOf, TyAndLayout};
use rustc_middle::ty::Instance;
use rustc_middle::{mir, ty};
use rustc_span::source_map::Span;
use rustc_target::spec::abi::Abi;

use super::{
Expand Down Expand Up @@ -71,14 +70,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Some((dest, ret)) => Some((self.eval_place(dest)?, *ret)),
None => None,
};
self.eval_fn_call(
fn_val,
terminator.source_info.span,
abi,
&args[..],
ret,
*cleanup,
)?;
self.eval_fn_call(fn_val, abi, &args[..], ret, *cleanup)?;
}

Drop { ref location, target, unwind } => {
Expand All @@ -88,7 +80,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
trace!("TerminatorKind::drop: {:?}, type {}", location, ty);

let instance = Instance::resolve_drop_in_place(*self.tcx, ty);
self.drop_in_place(place, instance, terminator.source_info.span, target, unwind)?;
self.drop_in_place(place, instance, target, unwind)?;
}

Assert { ref cond, expected, ref msg, target, cleanup } => {
Expand Down Expand Up @@ -196,7 +188,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
fn eval_fn_call(
&mut self,
fn_val: FnVal<'tcx, M::ExtraFnVal>,
span: Span,
caller_abi: Abi,
args: &[OpTy<'tcx, M::PointerTag>],
ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>,
Expand Down Expand Up @@ -242,7 +233,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
match instance.def {
ty::InstanceDef::Intrinsic(..) => {
assert!(caller_abi == Abi::RustIntrinsic || caller_abi == Abi::PlatformIntrinsic);
M::call_intrinsic(self, span, instance, args, ret, unwind)
M::call_intrinsic(self, instance, args, ret, unwind)
}
ty::InstanceDef::VtableShim(..)
| ty::InstanceDef::ReifyShim(..)
Expand All @@ -252,14 +243,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
| ty::InstanceDef::CloneShim(..)
| ty::InstanceDef::Item(_) => {
// We need MIR for this fn
let body = match M::find_mir_or_eval_fn(self, span, instance, args, ret, unwind)? {
let body = match M::find_mir_or_eval_fn(self, instance, args, ret, unwind)? {
Some(body) => body,
None => return Ok(()),
};

self.push_stack_frame(
instance,
span,
body,
ret.map(|p| p.0),
StackPopCleanup::Goto { ret: ret.map(|p| p.1), unwind },
Expand Down Expand Up @@ -407,7 +397,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
OpTy::from(ImmTy { layout: this_receiver_ptr, imm: receiver_place.ptr.into() });
trace!("Patched self operand to {:#?}", args[0]);
// recurse with concrete function
self.eval_fn_call(drop_fn, span, caller_abi, &args, ret, unwind)
self.eval_fn_call(drop_fn, caller_abi, &args, ret, unwind)
}
}
}
Expand All @@ -416,7 +406,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
&mut self,
place: PlaceTy<'tcx, M::PointerTag>,
instance: ty::Instance<'tcx>,
span: Span,
target: mir::BasicBlock,
unwind: Option<mir::BasicBlock>,
) -> InterpResult<'tcx> {
Expand Down Expand Up @@ -444,7 +433,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

self.eval_fn_call(
FnVal::Instance(instance),
span,
Abi::Rust,
&[arg.into()],
Some((dest.into(), target)),
Expand Down
Loading