bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #85546 - hyd-dev:unwind, r=RalfJung

Browse source 
const-eval: disallow unwinding across functions that `!fn_can_unwind()`

Following https://github.com/rust-lang/miri/pull/1776#discussion_r633074343, so r? `@RalfJung`

This PR turns `unwind` in `StackPopCleanup::Goto` into a new enum `StackPopUnwind`, with a `NotAllowed` variant to indicate that unwinding is not allowed. This variant is chosen based on `rustc_middle::ty::layout::fn_can_unwind()` in `eval_fn_call()` when pushing the frame. A check is added in `unwind_to_block()` to report UB if unwinding happens across a `StackPopUnwind::NotAllowed` frame.

Tested with Miri `HEAD` with [minor changes](https://github.com/rust-lang/miri/compare/HEAD..9cf3c7f0d86325a586fbcbf2acdc9232b861f1d8) and the rust-lang/miri#1776 branch with [these changes](d866c1c52f..626638fbfe).
This commit is contained in:
bors 2021-05-28 08:49:48 +00:00
commit ce0d64e03e
7 changed files with 168 additions and 94 deletions
Download patch file Download diff file Expand all files Collapse all files

6
compiler/rustc_mir/src/const_eval/machine.rs
View file

@ -17,7 +17,7 @@ use rustc_target::spec::abi::Abi;
use crate::interpret::{
self, compile_time_machine, AllocId, Allocation, Frame, ImmTy, InterpCx, InterpResult, Memory,
OpTy, PlaceTy, Pointer, Scalar,
OpTy, PlaceTy, Pointer, Scalar, StackPopUnwind,
};
use super::error::*;
@ -223,7 +223,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
_abi: Abi,
args: &[OpTy<'tcx>],
_ret: Option<(&PlaceTy<'tcx>, mir::BasicBlock)>,
_unwind: Option<mir::BasicBlock>, // unwinding is not supported in consts
_unwind: StackPopUnwind, // unwinding is not supported in consts
) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> {
debug!("find_mir_or_eval_fn: {:?}", instance);
@ -263,7 +263,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx>],
ret: Option<(&PlaceTy<'tcx>, mir::BasicBlock)>,
_unwind: Option<mir::BasicBlock>,
_unwind: StackPopUnwind,
) -> InterpResult<'tcx> {
// Shared intrinsics.
if ecx.emulate_intrinsic(instance, args, ret)? {

59
compiler/rustc_mir/src/interpret/eval_context.rs
View file

@ -134,14 +134,25 @@ pub struct FrameInfo<'tcx> {
pub lint_root: Option<hir::HirId>,
}
#[derive(Clone, Eq, PartialEq, Debug, HashStable)] // Miri debug-prints these
/// Unwind information.
#[derive(Clone, Copy, Eq, PartialEq, Debug, HashStable)]
pub enum StackPopUnwind {
/// The cleanup block.
Cleanup(mir::BasicBlock),
/// No cleanup needs to be done.
Skip,
/// Unwinding is not allowed (UB).
NotAllowed,
}
#[derive(Clone, Copy, Eq, PartialEq, Debug, HashStable)] // 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.
/// `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> },
Goto { ret: Option<mir::BasicBlock>, unwind: StackPopUnwind },
/// Just do nothing: 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
@ -746,13 +757,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// *Unwind* to the given `target` basic block.
/// Do *not* use for returning! Use `return_to_block` instead.
///
/// 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>) {
/// If `target` is `StackPopUnwind::Skip`, that indicates the function does not need cleanup
/// during unwinding, and we will just keep propagating that upwards.
///
/// If `target` is `StackPopUnwind::NotAllowed`, that indicates the function does not allow
/// unwinding, and doing so is UB.
pub fn unwind_to_block(&mut self, target: StackPopUnwind) -> InterpResult<'tcx> {
self.frame_mut().loc = match target {
Some(block) => Ok(mir::Location { block, statement_index: 0 }),
None => Err(self.frame_mut().body.span),
StackPopUnwind::Cleanup(block) => Ok(mir::Location { block, statement_index: 0 }),
StackPopUnwind::Skip => Err(self.frame_mut().body.span),
StackPopUnwind::NotAllowed => {
throw_ub_format!("unwinding past a stack frame that does not allow unwinding")
}
};
Ok(())
}
/// Pops the current frame from the stack, deallocating the
@ -801,21 +819,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}
let return_to_block = frame.return_to_block;
// Now where do we jump next?
// 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.
let (cleanup, next_block) = match frame.return_to_block {
StackPopCleanup::Goto { ret, unwind } => {
(true, Some(if unwinding { unwind } else { ret }))
}
StackPopCleanup::None { cleanup, .. } => (cleanup, None),
let cleanup = match return_to_block {
StackPopCleanup::Goto { .. } => true,
StackPopCleanup::None { cleanup, .. } => cleanup,
};
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!");
assert!(!unwinding, "tried to skip cleanup during unwinding");
// Leak the locals, skip validation, skip machine hook.
return Ok(());
@ -834,16 +851,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Normal return, figure out where to jump.
if unwinding {
// Follow the unwind edge.
let unwind = next_block.expect("Encountered StackPopCleanup::None when unwinding!");
self.unwind_to_block(unwind);
let unwind = match return_to_block {
StackPopCleanup::Goto { unwind, .. } => unwind,
StackPopCleanup::None { .. } => {
panic!("Encountered StackPopCleanup::None when unwinding!")
}
};
self.unwind_to_block(unwind)
} else {
// Follow the normal return edge.
if let Some(ret) = next_block {
self.return_to_block(ret)?;
match return_to_block {
StackPopCleanup::Goto { ret, .. } => self.return_to_block(ret),
StackPopCleanup::None { .. } => Ok(()),
}
}
Ok(())
}
/// Mark a storage as live, killing the previous content.

10
compiler/rustc_mir/src/interpret/machine.rs
View file

@ -14,7 +14,7 @@ use rustc_target::spec::abi::Abi;
use super::{
AllocId, Allocation, CheckInAllocMsg, Frame, ImmTy, InterpCx, InterpResult, LocalValue,
MemPlace, Memory, MemoryKind, OpTy, Operand, PlaceTy, Pointer, Scalar,
MemPlace, Memory, MemoryKind, OpTy, Operand, PlaceTy, Pointer, Scalar, StackPopUnwind,
};
/// Data returned by Machine::stack_pop,
@ -163,7 +163,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
abi: Abi,
args: &[OpTy<'tcx, Self::PointerTag>],
ret: Option<(&PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>,
unwind: Option<mir::BasicBlock>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>>;
/// Execute `fn_val`. It is the hook's responsibility to advance the instruction
@ -174,7 +174,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
abi: Abi,
args: &[OpTy<'tcx, Self::PointerTag>],
ret: Option<(&PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>,
unwind: Option<mir::BasicBlock>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx>;
/// Directly process an intrinsic without pushing a stack frame. It is the hook's
@ -184,7 +184,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
instance: ty::Instance<'tcx>,
args: &[OpTy<'tcx, Self::PointerTag>],
ret: Option<(&PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>,
unwind: Option<mir::BasicBlock>,
unwind: StackPopUnwind,
) -> InterpResult<'tcx>;
/// Called to evaluate `Assert` MIR terminators that trigger a panic.
@ -456,7 +456,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
_abi: Abi,
_args: &[OpTy<$tcx>],
_ret: Option<(&PlaceTy<$tcx>, mir::BasicBlock)>,
_unwind: Option<mir::BasicBlock>,
_unwind: StackPopUnwind,
) -> InterpResult<$tcx> {
match fn_val {}
}

4
compiler/rustc_mir/src/interpret/mod.rs
View file

@ -18,7 +18,9 @@ mod visitor;
pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in one place: here
pub use self::eval_context::{Frame, FrameInfo, InterpCx, LocalState, LocalValue, StackPopCleanup};
pub use self::eval_context::{
Frame, FrameInfo, InterpCx, LocalState, LocalValue, StackPopCleanup, StackPopUnwind,
};
pub use self::intern::{intern_const_alloc_recursive, InternKind};
pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackPopJump};
pub use self::memory::{AllocCheck, AllocRef, AllocRefMut, FnVal, Memory, MemoryKind};

103
compiler/rustc_mir/src/interpret/terminator.rs
View file

@ -1,7 +1,8 @@
use std::borrow::Cow;
use std::convert::TryFrom;
use rustc_middle::ty::layout::TyAndLayout;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::ty::layout::{self, TyAndLayout};
use rustc_middle::ty::Instance;
use rustc_middle::{
mir,
@ -12,9 +13,19 @@ use rustc_target::spec::abi::Abi;
use super::{
FnVal, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, PlaceTy, StackPopCleanup,
StackPopUnwind,
};
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
fn fn_can_unwind(&self, attrs: CodegenFnAttrFlags, abi: Abi) -> bool {
layout::fn_can_unwind(
self.tcx.sess.panic_strategy(),
attrs,
layout::conv_from_spec_abi(*self.tcx, abi),
abi,
)
}
pub(super) fn eval_terminator(
&mut self,
terminator: &mir::Terminator<'tcx>,
@ -58,12 +69,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let old_stack = self.frame_idx();
let old_loc = self.frame().loc;
let func = self.eval_operand(func, None)?;
let (fn_val, abi) = match *func.layout.ty.kind() {
let (fn_val, abi, caller_can_unwind) = match *func.layout.ty.kind() {
ty::FnPtr(sig) => {
let caller_abi = sig.abi();
let fn_ptr = self.read_scalar(&func)?.check_init()?;
let fn_val = self.memory.get_fn(fn_ptr)?;
(fn_val, caller_abi)
(
fn_val,
caller_abi,
self.fn_can_unwind(layout::fn_ptr_codegen_fn_attr_flags(), caller_abi),
)
}
ty::FnDef(def_id, substs) => {
let sig = func.layout.ty.fn_sig(*self.tcx);
@ -72,6 +87,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.resolve(ty::WithOptConstParam::unknown(def_id), substs)?,
),
sig.abi(),
self.fn_can_unwind(self.tcx.codegen_fn_attrs(def_id).flags, sig.abi()),
)
}
_ => span_bug!(
@ -89,7 +105,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
None => None,
};
self.eval_fn_call(fn_val, abi, &args[..], ret, *cleanup)?;
self.eval_fn_call(
fn_val,
abi,
&args[..],
ret,
match (cleanup, caller_can_unwind) {
(Some(cleanup), true) => StackPopUnwind::Cleanup(*cleanup),
(None, true) => StackPopUnwind::Skip,
(_, false) => StackPopUnwind::NotAllowed,
},
)?;
// Sanity-check that `eval_fn_call` either pushed a new frame or
// did a jump to another block.
if self.frame_idx() == old_stack && self.frame().loc == old_loc {
@ -219,7 +245,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
caller_abi: Abi,
args: &[OpTy<'tcx, M::PointerTag>],
ret: Option<(&PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>,
unwind: Option<mir::BasicBlock>,
mut unwind: StackPopUnwind,
) -> InterpResult<'tcx> {
trace!("eval_fn_call: {:#?}", fn_val);
@ -230,37 +256,38 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
};
let get_abi = |this: &Self, instance_ty: Ty<'tcx>| match instance_ty.kind() {
ty::FnDef(..) => instance_ty.fn_sig(*this.tcx).abi(),
ty::Closure(..) => Abi::RustCall,
ty::Generator(..) => Abi::Rust,
_ => span_bug!(this.cur_span(), "unexpected callee ty: {:?}", instance_ty),
};
// ABI check
let check_abi = |this: &Self, instance_ty: Ty<'tcx>| -> InterpResult<'tcx> {
if M::enforce_abi(this) {
let callee_abi = match instance_ty.kind() {
ty::FnDef(..) => instance_ty.fn_sig(*this.tcx).abi(),
ty::Closure(..) => Abi::RustCall,
ty::Generator(..) => Abi::Rust,
_ => span_bug!(this.cur_span(), "unexpected callee ty: {:?}", instance_ty),
};
let normalize_abi = |abi| match abi {
Abi::Rust | Abi::RustCall | Abi::RustIntrinsic | Abi::PlatformIntrinsic =>
// These are all the same ABI, really.
{
Abi::Rust
}
abi => abi,
};
if normalize_abi(caller_abi) != normalize_abi(callee_abi) {
throw_ub_format!(
"calling a function with ABI {} using caller ABI {}",
callee_abi.name(),
caller_abi.name()
)
let check_abi = |callee_abi: Abi| -> InterpResult<'tcx> {
let normalize_abi = |abi| match abi {
Abi::Rust | Abi::RustCall | Abi::RustIntrinsic | Abi::PlatformIntrinsic =>
// These are all the same ABI, really.
{
Abi::Rust
}
abi => abi,
};
if normalize_abi(caller_abi) != normalize_abi(callee_abi) {
throw_ub_format!(
"calling a function with ABI {} using caller ABI {}",
callee_abi.name(),
caller_abi.name()
)
}
Ok(())
};
match instance.def {
ty::InstanceDef::Intrinsic(..) => {
check_abi(self, instance.ty(*self.tcx, self.param_env))?;
if M::enforce_abi(self) {
check_abi(get_abi(self, instance.ty(*self.tcx, self.param_env)))?;
}
assert!(caller_abi == Abi::RustIntrinsic || caller_abi == Abi::PlatformIntrinsic);
M::call_intrinsic(self, instance, args, ret, unwind)
}
@ -281,7 +308,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Check against the ABI of the MIR body we are calling (not the ABI of `instance`;
// these can differ when `find_mir_or_eval_fn` does something clever like resolve
// exported symbol names).
check_abi(self, self.tcx.type_of(body.source.def_id()))?;
let callee_def_id = body.source.def_id();
let callee_abi = get_abi(self, self.tcx.type_of(callee_def_id));
if M::enforce_abi(self) {
check_abi(callee_abi)?;
}
if !matches!(unwind, StackPopUnwind::NotAllowed)
&& !self
.fn_can_unwind(self.tcx.codegen_fn_attrs(callee_def_id).flags, callee_abi)
{
// The callee cannot unwind.
unwind = StackPopUnwind::NotAllowed;
}
self.push_stack_frame(
instance,
@ -471,7 +511,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Abi::Rust,
&[arg.into()],
Some((&dest.into(), target)),
unwind,
match unwind {
Some(cleanup) => StackPopUnwind::Cleanup(cleanup),
None => StackPopUnwind::Skip,
},
)
}
}

5
compiler/rustc_mir/src/transform/const_prop.rs
View file

@ -33,6 +33,7 @@ use crate::interpret::{
self, compile_time_machine, AllocId, Allocation, ConstValue, CtfeValidationMode, Frame, ImmTy,
Immediate, InterpCx, InterpResult, LocalState, LocalValue, MemPlace, Memory, MemoryKind, OpTy,
Operand as InterpOperand, PlaceTy, Pointer, Scalar, ScalarMaybeUninit, StackPopCleanup,
StackPopUnwind,
};
use crate::transform::MirPass;
@ -198,7 +199,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
_abi: Abi,
_args: &[OpTy<'tcx>],
_ret: Option<(&PlaceTy<'tcx>, BasicBlock)>,
_unwind: Option<BasicBlock>,
_unwind: StackPopUnwind,
) -> InterpResult<'tcx, Option<&'mir Body<'tcx>>> {
Ok(None)
}
@ -208,7 +209,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
_instance: ty::Instance<'tcx>,
_args: &[OpTy<'tcx>],
_ret: Option<(&PlaceTy<'tcx>, BasicBlock)>,
_unwind: Option<BasicBlock>,
_unwind: StackPopUnwind,
) -> InterpResult<'tcx> {
throw_machine_stop_str!("calling intrinsics isn't supported in ConstProp")
}