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

adjust Miri to needs of changed unwinding strategy #1227

Merged
merged 3 commits into from
Mar 15, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1572c433eed495d0ade41511ae106b180e02851d
7cdbc87a49b0b705a41a004a6d486b0952521ae7
16 changes: 8 additions & 8 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@ pub struct FrameData<'tcx> {
/// Extra data for Stacked Borrows.
pub call_id: stacked_borrows::CallId,

/// If this is Some(), then this is a special "catch unwind" frame (the frame of the closure
/// called by `__rustc_maybe_catch_panic`). When this frame is popped during unwinding a panic,
/// we stop unwinding, use the `CatchUnwindData` to
/// store the panic payload, and continue execution in the parent frame.
pub catch_panic: Option<CatchUnwindData<'tcx>>,
/// If this is Some(), then this is a special "catch unwind" frame (the frame of `try_fn`
/// called by `try`). When this frame is popped during unwinding a panic,
/// we stop unwinding, use the `CatchUnwindData` to handle catching.
pub catch_unwind: Option<CatchUnwindData<'tcx>>,
}

/// Extra memory kinds
Expand Down Expand Up @@ -163,7 +162,8 @@ pub struct Evaluator<'tcx> {

/// The temporary used for storing the argument of
/// the call to `miri_start_panic` (the panic payload) when unwinding.
pub(crate) panic_payload: Option<ImmTy<'tcx, Tag>>,
/// This is pointer-sized, and matches the `Payload` type in `src/libpanic_unwind/miri.rs`.
pub(crate) panic_payload: Option<Scalar<Tag>>,
}

impl<'tcx> Evaluator<'tcx> {
Expand Down Expand Up @@ -405,15 +405,15 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
let call_id = stacked_borrows.map_or(NonZeroU64::new(1).unwrap(), |stacked_borrows| {
stacked_borrows.borrow_mut().new_call()
});
Ok(FrameData { call_id, catch_panic: None })
Ok(FrameData { call_id, catch_unwind: None })
}

#[inline(always)]
fn stack_pop(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
extra: FrameData<'tcx>,
unwinding: bool,
) -> InterpResult<'tcx, StackPopInfo> {
) -> InterpResult<'tcx, StackPopJump> {
ecx.handle_stack_pop(extra, unwinding)
}

Expand Down
14 changes: 5 additions & 9 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx

// Second: some functions that we forward to MIR implementations.
match link_name {
// This matches calls to the *foreign* item `__rust_start_panic*, that is,
// calls to `extern "Rust" { fn __rust_start_panic(...) }`.
// This matches calls to the foreign item `__rust_start_panic`, that is,
// calls to `extern "Rust" { fn __rust_start_panic(...) }`
// (and `__rust_panic_cleanup`, respectively).
// We forward this to the underlying *implementation* in the panic runtime crate.
// Normally, this will be either `libpanic_unwind` or `libpanic_abort`, but it could
// also be a custom user-provided implementation via `#![feature(panic_runtime)]`
"__rust_start_panic" => {
"__rust_start_panic" | "__rust_panic_cleanup"=> {
// FIXME we might want to cache this... but it's not really performance-critical.
let panic_runtime = tcx
.crates()
Expand All @@ -164,7 +165,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
.expect("No panic runtime found!");
let panic_runtime = tcx.crate_name(*panic_runtime);
let start_panic_instance =
this.resolve_path(&[&*panic_runtime.as_str(), "__rust_start_panic"])?;
this.resolve_path(&[&*panic_runtime.as_str(), link_name])?;
return Ok(Some(&*this.load_mir(start_panic_instance.def, None)?));
}
_ => {}
Expand Down Expand Up @@ -291,11 +292,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.write_scalar(new_ptr, dest)?;
}

"__rust_maybe_catch_panic" => {
this.handle_catch_panic(args, dest, ret)?;
return Ok(false);
}

"memcmp" => {
let left = this.read_scalar(args[0])?.not_undef()?;
let right = this.read_scalar(args[1])?.not_undef()?;
Expand Down
7 changes: 2 additions & 5 deletions src/shims/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// First handle intrinsics without return place.
let (dest, ret) = match ret {
None => match intrinsic_name {
"abort" => {
// FIXME: remove, once the intrinsic on the rustc side is fixed.
throw_machine_stop!(TerminationInfo::Abort);
}
"miri_start_panic" => return this.handle_miri_start_panic(args, unwind),
_ => throw_unsup_format!("unimplemented (diverging) intrinsic: {}", intrinsic_name),
},
Some(p) => p,
};

match intrinsic_name {
"miri_start_panic" => return this.handle_miri_start_panic(args, unwind),
"try" => return this.handle_try(args, dest, ret),

"arith_offset" => {
let offset = this.read_scalar(args[1])?.to_machine_isize(this)?;
Expand Down
124 changes: 62 additions & 62 deletions src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,22 @@ use rustc_target::spec::PanicStrategy;

use crate::*;

/// Holds all of the relevant data for a call to
/// `__rust_maybe_catch_panic`.
///
/// If a panic occurs, we update this data with
/// the information from the panic site.
/// Holds all of the relevant data for when unwinding hits a `try` frame.
#[derive(Debug)]
pub struct CatchUnwindData<'tcx> {
/// The dereferenced `data_ptr` argument passed to `__rust_maybe_catch_panic`.
pub data_place: MPlaceTy<'tcx, Tag>,
/// The dereferenced `vtable_ptr` argument passed to `__rust_maybe_catch_panic`.
pub vtable_place: MPlaceTy<'tcx, Tag>,
/// The `dest` from the original call to `__rust_maybe_catch_panic`.
pub dest: PlaceTy<'tcx, Tag>,
/// The `catch_fn` callback to call in case of a panic.
catch_fn: Scalar<Tag>,
/// The `data` argument for that callback.
data: Scalar<Tag>,
/// The return place from the original call to `try`.
dest: PlaceTy<'tcx, Tag>,
/// The return block from the original call to `try`.
ret: mir::BasicBlock,
}

impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
/// Handles the special "miri_start_panic" intrinsic, which is called
/// Handles the special `miri_start_panic` intrinsic, which is called
/// by libpanic_unwind to delegate the actual unwinding process to Miri.
fn handle_miri_start_panic(
&mut self,
Expand All @@ -46,47 +44,50 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
trace!("miri_start_panic: {:?}", this.frame().span);

// Get the raw pointer stored in arg[0] (the panic payload).
let scalar = this.read_immediate(args[0])?;
let payload = this.read_scalar(args[0])?.not_undef()?;
assert!(
this.machine.panic_payload.is_none(),
"the panic runtime should avoid double-panics"
);
this.machine.panic_payload = Some(scalar);
this.machine.panic_payload = Some(payload);

// Jump to the unwind block to begin unwinding.
this.unwind_to_block(unwind);
return Ok(());
}

fn handle_catch_panic(
/// Handles the `try` intrinsic, the underlying implementation of `std::panicking::try`.
fn handle_try(
&mut self,
args: &[OpTy<'tcx, Tag>],
dest: PlaceTy<'tcx, Tag>,
ret: mir::BasicBlock,
) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let tcx = &{ this.tcx.tcx };

// fn __rust_maybe_catch_panic(
// f: fn(*mut u8),
// data: *mut u8,
// data_ptr: *mut usize,
// vtable_ptr: *mut usize,
// ) -> u32
// Signature:
// fn r#try(try_fn: fn(*mut u8), data: *mut u8, catch_fn: fn(*mut u8, *mut u8)) -> i32
// Calls `try_fn` with `data` as argument. If that executes normally, returns 0.
// If that unwinds, calls `catch_fn` with the first argument being `data` and
// then second argument being a target-dependent `payload` (i.e. it is up to us to define
// what that is), and returns 1.
// The `payload` is passed (by libstd) to `__rust_panic_cleanup`, which is then expected to
// return a `Box<dyn Any + Send + 'static>`.
// In Miri, `miri_start_panic` is passed exactly that type, so we make the `payload` simply
// a pointer to `Box<dyn Any + Send + 'static>`.

// Get all the arguments.
let f = this.read_scalar(args[0])?.not_undef()?;
let f_arg = this.read_scalar(args[1])?.not_undef()?;
let data_place = this.deref_operand(args[2])?;
let vtable_place = this.deref_operand(args[3])?;

// Now we make a function call, and pass `f_arg` as first and only argument.
let f_instance = this.memory.get_fn(f)?.as_instance()?;
trace!("__rust_maybe_catch_panic: {:?}", f_instance);
let ret_place = MPlaceTy::dangling(this.layout_of(tcx.mk_unit())?, this).into();
let try_fn = this.read_scalar(args[0])?.not_undef()?;
let data = this.read_scalar(args[1])?.not_undef()?;
let catch_fn = this.read_scalar(args[2])?.not_undef()?;

// Now we make a function call, and pass `data` as first and only argument.
let f_instance = this.memory.get_fn(try_fn)?.as_instance()?;
trace!("try_fn: {:?}", f_instance);
let ret_place = MPlaceTy::dangling(this.layout_of(this.tcx.mk_unit())?, this).into();
this.call_function(
f_instance,
&[f_arg.into()],
&[data.into()],
Some(ret_place),
// Directly return to caller.
StackPopCleanup::Goto { ret: Some(ret), unwind: None },
Expand All @@ -95,12 +96,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// We ourselves will return `0`, eventually (will be overwritten if we catch a panic).
this.write_null(dest)?;

// In unwind mode, we tag this frame with some extra data.
// In unwind mode, we tag this frame with the extra data needed to catch unwinding.
// This lets `handle_stack_pop` (below) know that we should stop unwinding
// when we pop this frame.
if this.tcx.tcx.sess.panic_strategy() == PanicStrategy::Unwind {
this.frame_mut().extra.catch_panic =
Some(CatchUnwindData { data_place, vtable_place, dest })
if this.tcx.sess.panic_strategy() == PanicStrategy::Unwind {
this.frame_mut().extra.catch_unwind = Some(CatchUnwindData { catch_fn, data, dest, ret });
}

return Ok(());
Expand All @@ -110,45 +110,45 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
&mut self,
mut extra: FrameData<'tcx>,
unwinding: bool,
) -> InterpResult<'tcx, StackPopInfo> {
) -> InterpResult<'tcx, StackPopJump> {
let this = self.eval_context_mut();

trace!("handle_stack_pop(extra = {:?}, unwinding = {})", extra, unwinding);
if let Some(stacked_borrows) = this.memory.extra.stacked_borrows.as_ref() {
stacked_borrows.borrow_mut().end_call(extra.call_id);
}

// We only care about `catch_panic` if we're unwinding - if we're doing a normal
// return, then we don't need to do anything special.
let res = if let (true, Some(unwind_data)) = (unwinding, extra.catch_panic.take()) {
// We've just popped a frame that was pushed by `__rust_maybe_catch_panic`,
if let (true, Some(catch_unwind)) = (unwinding, extra.catch_unwind.take()) {
// We've just popped a frame that was pushed by `try`,
// and we are unwinding, so we should catch that.
trace!("unwinding: found catch_panic frame during unwinding: {:?}", this.frame().span);

// `panic_payload` now holds a `*mut (dyn Any + Send)`,
// provided by the `miri_start_panic` intrinsic.
// We want to split this into its consituient parts -
// the data and vtable pointers - and store them according to
// `unwind_data`, i.e., we store them where `__rust_maybe_catch_panic`
// was told to put them.
let payload = this.machine.panic_payload.take().unwrap();
let payload = this.ref_to_mplace(payload)?;
let payload_data_place = payload.ptr;
let payload_vtable_place = payload.meta.unwrap_meta();

this.write_scalar(payload_data_place, unwind_data.data_place.into())?;
this.write_scalar(payload_vtable_place, unwind_data.vtable_place.into())?;
// We set the return value of `try` to 1, since there was a panic.
this.write_scalar(Scalar::from_i32(1), catch_unwind.dest)?;

// We set the return value of `__rust_maybe_catch_panic` to 1,
// since there was a panic.
let dest = unwind_data.dest;
this.write_scalar(Scalar::from_int(1, dest.layout.size), dest)?;
// `panic_payload` holds what was passed to `miri_start_panic`.
// This is exactly the second argument we need to pass to `catch_fn`.
let payload = this.machine.panic_payload.take().unwrap();

StackPopInfo::StopUnwinding
// Push the `catch_fn` stackframe.
let f_instance = this.memory.get_fn(catch_unwind.catch_fn)?.as_instance()?;
trace!("catch_fn: {:?}", f_instance);
let ret_place = MPlaceTy::dangling(this.layout_of(this.tcx.mk_unit())?, this).into();
this.call_function(
f_instance,
&[catch_unwind.data.into(), payload.into()],
Some(ret_place),
// Directly return to caller of `try`.
StackPopCleanup::Goto { ret: Some(catch_unwind.ret), unwind: None },
)?;

// We pushed a new stack frame, the engine should not do any jumping now!
Ok(StackPopJump::NoJump)
} else {
StackPopInfo::Normal
};
if let Some(stacked_borrows) = this.memory.extra.stacked_borrows.as_ref() {
stacked_borrows.borrow_mut().end_call(extra.call_id);
Ok(StackPopJump::Normal)
}
Ok(res)
}

/// Starta a panic in the interpreter with the given message as payload.
Expand Down