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

Add hooks for Miri panic unwinding #60026

Merged
merged 29 commits into from
Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
187d05f
Add hooks for Miri panic unwinding
Aaron1011 Apr 17, 2019
1991018
Formatting improvements
Aaron1011 Sep 19, 2019
fa4f1b7
Fix incorrect unwrap of dest
Aaron1011 Sep 26, 2019
d87e2da
Remove old intrinsic check
Aaron1011 Sep 26, 2019
499eb5d
More fixes for rustc changes
Aaron1011 Oct 9, 2019
01c11f9
avoid the loop in unwinding stack popping
RalfJung Oct 20, 2019
64a43f4
A few minor tweaks
Aaron1011 Oct 21, 2019
fe3e1c1
Add doc comment
Aaron1011 Oct 21, 2019
8df4248
Some cleanup
Aaron1011 Oct 28, 2019
4f25c91
Fix unwinding logic
Aaron1011 Oct 29, 2019
8ff4d41
Don't attempt to get cwd when printing backtrace under Miri
Aaron1011 Oct 29, 2019
caf3cc1
Add explicit Miri support to libpanic_unwind
Aaron1011 Oct 29, 2019
fe88fc0
Fix up intrinsic implementation
Aaron1011 Oct 29, 2019
848e1d8
More work on miri_start_panic
Aaron1011 Oct 29, 2019
5553476
Use proper intrinsic type
Aaron1011 Oct 29, 2019
b06c83c
Add miri trampoline, fix handling of intrinsic return
Aaron1011 Oct 30, 2019
d5c0aca
Add comment
Aaron1011 Oct 30, 2019
dac3011
Change TODO to FIXME
Aaron1011 Oct 30, 2019
c062afe
Make doc comment more accurate
Aaron1011 Nov 4, 2019
72b555c
Some code cleanup
Aaron1011 Nov 4, 2019
2390077
Move to miri.rs and re-export it
Aaron1011 Nov 4, 2019
6eea0ff
Add more detailed codegen comment
Aaron1011 Nov 4, 2019
607339f
Fix tidy
Aaron1011 Nov 4, 2019
4ecb80d
Remove trampoline, pass `ret` and `unwind` when handling intrinsics
Aaron1011 Nov 5, 2019
ee2dc4b
Fix debug assertion
Aaron1011 Nov 5, 2019
2ed1e89
Rename to
Aaron1011 Nov 6, 2019
68d9853
Fix rebase fallout
Aaron1011 Nov 6, 2019
c0b972a
Return Ok(false) instead of throwing when handling a diverging intrinsic
Aaron1011 Nov 6, 2019
b4545a4
Update
Aaron1011 Nov 9, 2019
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
5 changes: 5 additions & 0 deletions src/libcore/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1348,6 +1348,11 @@ extern "rust-intrinsic" {
/// See documentation of `<*const T>::offset_from` for details.
#[cfg(not(bootstrap))]
pub fn ptr_offset_from<T>(ptr: *const T, base: *const T) -> isize;

/// Internal hook used by Miri to implement unwinding.
/// Perma-unstable: do not use
#[cfg(not(bootstrap))]
pub fn miri_start_panic(data: *mut (dyn crate::any::Any + crate::marker::Send)) -> !;
}

// Some functions are defined here because they accidentally got made
Expand Down
5 changes: 4 additions & 1 deletion src/libpanic_unwind/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ use core::raw;
use core::panic::BoxMeUp;

cfg_if::cfg_if! {
if #[cfg(target_os = "emscripten")] {
if #[cfg(miri)] {
#[path = "miri.rs"]
mod imp;
} else if #[cfg(target_os = "emscripten")] {
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
#[path = "emcc.rs"]
mod imp;
} else if #[cfg(target_arch = "wasm32")] {
Expand Down
23 changes: 23 additions & 0 deletions src/libpanic_unwind/miri.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use core::any::Any;
use alloc::boxed::Box;

pub fn payload() -> *mut u8 {
core::ptr::null_mut()
}

pub unsafe fn panic(data: Box<dyn Any + Send>) -> ! {
core::intrinsics::miri_start_panic(Box::into_raw(data))
}

pub unsafe fn cleanup(ptr: *mut u8) -> Box<dyn Any + Send> {
Box::from_raw(ptr)
}


// This is required by the compiler to exist (e.g., it's a lang item),
// but is never used by Miri. Therefore, we just use a stub here
#[lang = "eh_personality"]
#[cfg(not(test))]
fn rust_eh_personality() {
unsafe { core::intrinsics::abort() }
}
15 changes: 15 additions & 0 deletions src/librustc_codegen_ssa/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
_ => FnAbi::new(&bx, sig, &extra_args)
};

// This should never be reachable at runtime:
// We should only emit a call to this intrinsic in #[cfg(miri)] mode,
// which means that we will never actually use the generate object files
// (we will just be interpreting the MIR)
Aaron1011 marked this conversation as resolved.
Show resolved Hide resolved
//
// Note that we still need to be able to codegen *something* for this intrisnic:
// Miri currently uses Xargo to build a special libstd. As a side effect,
// we generate normal object files for libstd - while these are never used,
// we still need to be able to build them.
if intrinsic == Some("miri_start_panic") {
bx.abort();
bx.unreachable();
return;
}

// Emit a panic or a no-op for `panic_if_uninhabited`.
if intrinsic == Some("panic_if_uninhabited") {
let ty = instance.unwrap().substs.type_at(0);
Expand Down
13 changes: 5 additions & 8 deletions src/librustc_mir/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
args: &[OpTy<'tcx>],
dest: Option<PlaceTy<'tcx>>,
ret: Option<mir::BasicBlock>,
_unwind: Option<mir::BasicBlock> // unwinding is not supported in consts
) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
debug!("eval_fn_call: {:?}", instance);
// Only check non-glue functions
Expand All @@ -336,7 +337,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
// Some functions we support even if they are non-const -- but avoid testing
// that for const fn! We certainly do *not* want to actually call the fn
// though, so be sure we return here.
return if ecx.hook_fn(instance, args, dest)? {
return if ecx.hook_panic_fn(instance, args, dest)? {
ecx.goto_block(ret)?; // fully evaluated and done
Ok(None)
} else {
Expand Down Expand Up @@ -374,7 +375,9 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
span: Span,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx>],
dest: PlaceTy<'tcx>,
dest: Option<PlaceTy<'tcx>>,
_ret: Option<mir::BasicBlock>,
_unwind: Option<mir::BasicBlock>
) -> InterpResult<'tcx> {
if ecx.emulate_intrinsic(span, instance, args, dest)? {
return Ok(());
Expand Down Expand Up @@ -469,12 +472,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
fn stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
Ok(())
}

/// Called immediately before a stack frame gets popped.
#[inline(always)]
fn stack_pop(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _extra: ()) -> InterpResult<'tcx> {
Ok(())
}
}

/// Extracts a field of a (variant of a) const.
Expand Down
180 changes: 123 additions & 57 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rustc_data_structures::fx::FxHashMap;

use super::{
Immediate, Operand, MemPlace, MPlaceTy, Place, PlaceTy, ScalarMaybeUndef,
Memory, Machine
Memory, Machine, StackPopInfo
};

pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
Expand Down Expand Up @@ -60,6 +60,9 @@ pub struct Frame<'mir, 'tcx, Tag=(), Extra=()> {
/// The span of the call site.
pub span: source_map::Span,

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

////////////////////////////////////////////////////////////////////////////////
// Return place and locals
////////////////////////////////////////////////////////////////////////////////
Expand All @@ -82,21 +85,22 @@ pub struct Frame<'mir, 'tcx, Tag=(), Extra=()> {
////////////////////////////////////////////////////////////////////////////////
/// The block that is currently executed (or will be executed after the above call stacks
/// return).
pub block: mir::BasicBlock,
/// If this is `None`, we are unwinding and this function doesn't need any clean-up.
/// Just continue the same as with `Resume`.
pub block: Option<mir::BasicBlock>,

/// The index of the currently evaluated statement.
pub stmt: usize,

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

#[derive(Clone, Eq, PartialEq, Debug)] // Miri debug-prints these
pub enum StackPopCleanup {
/// Jump to the next block in the caller, or cause UB if None (that's a function
/// that may never return). Also store layout of return place so
/// we can validate it at that layout.
Goto(Option<mir::BasicBlock>),
/// `ret` stores the block we jump to on a normal return, while 'unwind'
/// stores the block used for cleanup during unwinding
Goto { ret: Option<mir::BasicBlock>, unwind: Option<mir::BasicBlock> },
/// Just do nohing: Used by Main and for the box_alloc hook in miri.
/// `cleanup` says whether locals are deallocated. Static computation
/// wants them leaked to intern what they need (and just throw away
Expand Down Expand Up @@ -489,7 +493,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let extra = M::stack_push(self)?;
self.stack.push(Frame {
body,
block: mir::START_BLOCK,
block: Some(mir::START_BLOCK),
return_to_block,
return_place,
// empty local array, we fill it in below, after we are inside the stack frame and
Expand Down Expand Up @@ -547,60 +551,118 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

pub(super) fn pop_stack_frame(&mut self) -> InterpResult<'tcx> {
info!("LEAVING({}) {}", self.cur_frame(), self.frame().instance);
/// Pops the current frame from the stack, deallocating the
/// memory for allocated locals.
///
/// If `unwinding` is `false`, then we are performing a normal return
/// from a function. In this case, we jump back into the frame of the caller,
/// and continue execution as normal.
///
/// If `unwinding` is `true`, then we are in the middle of a panic,
/// and need to unwind this frame. In this case, we jump to the
/// `cleanup` block for the function, which is responsible for running
/// `Drop` impls for any locals that have been initialized at this point.
/// The cleanup block ends with a special `Resume` terminator, which will
/// cause us to continue unwinding.
pub(super) fn pop_stack_frame(
&mut self,
unwinding: bool
) -> InterpResult<'tcx> {
info!("LEAVING({}) {} (unwinding = {})",
self.cur_frame(), self.frame().instance, unwinding);

// Sanity check `unwinding`.
assert_eq!(
unwinding,
match self.frame().block {
None => true,
Some(block) => self.body().basic_blocks()[block].is_cleanup
}
);

::log_settings::settings().indentation -= 1;
let frame = self.stack.pop().expect(
"tried to pop a stack frame, but there were none",
);
M::stack_pop(self, frame.extra)?;
// Abort early if we do not want to clean up: We also avoid validation in that case,
let stack_pop_info = M::stack_pop(self, frame.extra, unwinding)?;
if let (false, StackPopInfo::StopUnwinding) = (unwinding, stack_pop_info) {
bug!("Attempted to stop unwinding while there is no unwinding!");
}

// Now where do we jump next?

// Determine if we leave this function normally or via unwinding.
let cur_unwinding = if let StackPopInfo::StopUnwinding = stack_pop_info {
false
} else {
unwinding
};

// Usually we want to clean up (deallocate locals), but in a few rare cases we don't.
// In that case, we return early. We also avoid validation in that case,
// because this is CTFE and the final value will be thoroughly validated anyway.
match frame.return_to_block {
StackPopCleanup::Goto(_) => {},
StackPopCleanup::None { cleanup } => {
if !cleanup {
assert!(self.stack.is_empty(), "only the topmost frame should ever be leaked");
// Leak the locals, skip validation.
return Ok(());
}
}
let (cleanup, next_block) = match frame.return_to_block {
StackPopCleanup::Goto { ret, unwind } => {
(true, Some(if cur_unwinding { unwind } else { ret }))
},
StackPopCleanup::None { cleanup, .. } => (cleanup, None)
};

if !cleanup {
assert!(self.stack.is_empty(), "only the topmost frame should ever be leaked");
assert!(next_block.is_none(), "tried to skip cleanup when we have a next block!");
// Leak the locals, skip validation.
return Ok(());
}
// Deallocate all locals that are backed by an allocation.

// Cleanup: deallocate all locals that are backed by an allocation.
for local in frame.locals {
self.deallocate_local(local.value)?;
}
// Validate the return value. Do this after deallocating so that we catch dangling
// references.
if let Some(return_place) = frame.return_place {
if M::enforce_validity(self) {
// Data got changed, better make sure it matches the type!
// It is still possible that the return place held invalid data while
// the function is running, but that's okay because nobody could have
// accessed that same data from the "outside" to observe any broken
// invariant -- that is, unless a function somehow has a ptr to
// its return place... but the way MIR is currently generated, the
// return place is always a local and then this cannot happen.
self.validate_operand(
self.place_to_op(return_place)?,
vec![],
None,
)?;
}


trace!("StackPopCleanup: {:?} StackPopInfo: {:?} cur_unwinding = {:?}",
frame.return_to_block, stack_pop_info, cur_unwinding);
if cur_unwinding {
// Follow the unwind edge.
let unwind = next_block.expect("Encounted StackPopCleanup::None when unwinding!");
let next_frame = self.frame_mut();
// If `unwind` is `None`, we'll leave that function immediately again.
next_frame.block = unwind;
next_frame.stmt = 0;
} else {
// Uh, that shouldn't happen... the function did not intend to return
throw_ub!(Unreachable)
}
// Jump to new block -- *after* validation so that the spans make more sense.
match frame.return_to_block {
StackPopCleanup::Goto(block) => {
self.goto_block(block)?;
// Follow the normal return edge.
// Validate the return value. Do this after deallocating so that we catch dangling
// references.
if let Some(return_place) = frame.return_place {
if M::enforce_validity(self) {
// Data got changed, better make sure it matches the type!
// It is still possible that the return place held invalid data while
// the function is running, but that's okay because nobody could have
// accessed that same data from the "outside" to observe any broken
// invariant -- that is, unless a function somehow has a ptr to
// its return place... but the way MIR is currently generated, the
// return place is always a local and then this cannot happen.
self.validate_operand(
self.place_to_op(return_place)?,
vec![],
None,
)?;
}
} else {
// Uh, that shouldn't happen... the function did not intend to return
throw_ub!(Unreachable);
}

// Jump to new block -- *after* validation so that the spans make more sense.
if let Some(ret) = next_block {
self.goto_block(ret)?;
}
StackPopCleanup::None { .. } => {}
}
RalfJung marked this conversation as resolved.
Show resolved Hide resolved

if self.stack.len() > 0 {
info!("CONTINUING({}) {}", self.cur_frame(), self.frame().instance);
info!("CONTINUING({}) {} (unwinding = {})",
self.cur_frame(), self.frame().instance, cur_unwinding);
}

Ok(())
Expand Down Expand Up @@ -745,16 +807,20 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} else {
last_span = Some(span);
}
let block = &body.basic_blocks()[block];
let source_info = if stmt < block.statements.len() {
block.statements[stmt].source_info
} else {
block.terminator().source_info
};
let lint_root = match body.source_scope_local_data {
mir::ClearCrossCrate::Set(ref ivs) => Some(ivs[source_info.scope].lint_root),
mir::ClearCrossCrate::Clear => None,
};

let lint_root = block.and_then(|block| {
let block = &body.basic_blocks()[block];
let source_info = if stmt < block.statements.len() {
block.statements[stmt].source_info
} else {
block.terminator().source_info
};
match body.source_scope_local_data {
mir::ClearCrossCrate::Set(ref ivs) => Some(ivs[source_info.scope].lint_root),
mir::ClearCrossCrate::Clear => None,
}
});

frames.push(FrameInfo { call_site: span, instance, lint_root });
}
trace!("generate stacktrace: {:#?}, {:?}", frames, explicit_span);
Expand Down
13 changes: 10 additions & 3 deletions src/librustc_mir/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,17 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
span: Span,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, M::PointerTag>],
dest: PlaceTy<'tcx, M::PointerTag>,
dest: Option<PlaceTy<'tcx, M::PointerTag>>,
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
) -> InterpResult<'tcx, bool> {
let substs = instance.substs;

// We currently do not handle any diverging intrinsics.
let dest = match dest {
Some(dest) => dest,
None => return Ok(false)
};
let intrinsic_name = &*self.tcx.item_name(instance.def_id()).as_str();

match intrinsic_name {
"caller_location" => {
let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span);
Expand Down Expand Up @@ -347,9 +353,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Ok(true)
}

/// "Intercept" a function call because we have something special to do for it.
/// "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_fn(
pub fn hook_panic_fn(
&mut self,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, M::PointerTag>],
Expand Down
Loading