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

const-eval: disallow unwinding across functions that !fn_can_unwind() #85546

Merged
merged 15 commits into from May 28, 2021
Merged
75 changes: 41 additions & 34 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2579,7 +2579,7 @@ where
fn adjust_for_abi(&mut self, cx: &C, abi: SpecAbi);
}

fn fn_can_unwind(
pub fn fn_can_unwind(
panic_strategy: PanicStrategy,
codegen_fn_attr_flags: CodegenFnAttrFlags,
call_conv: Conv,
Expand Down Expand Up @@ -2641,6 +2641,43 @@ fn fn_can_unwind(
}
}

pub fn conv_from_spec_abi(tcx: TyCtxt<'_>, abi: SpecAbi) -> Conv {
use rustc_target::spec::abi::Abi::*;
match tcx.sess.target.adjust_abi(abi) {
RustIntrinsic | PlatformIntrinsic | Rust | RustCall => Conv::Rust,

// It's the ABI's job to select this, not ours.
System { .. } => bug!("system abi should be selected elsewhere"),
EfiApi => bug!("eficall abi should be selected elsewhere"),

Stdcall { .. } => Conv::X86Stdcall,
Fastcall => Conv::X86Fastcall,
Vectorcall => Conv::X86VectorCall,
Thiscall { .. } => Conv::X86ThisCall,
C { .. } => Conv::C,
Unadjusted => Conv::C,
Win64 => Conv::X86_64Win64,
SysV64 => Conv::X86_64SysV,
Aapcs => Conv::ArmAapcs,
CCmseNonSecureCall => Conv::CCmseNonSecureCall,
PtxKernel => Conv::PtxKernel,
Msp430Interrupt => Conv::Msp430Intr,
X86Interrupt => Conv::X86Intr,
AmdGpuKernel => Conv::AmdGpuKernel,
AvrInterrupt => Conv::AvrInterrupt,
AvrNonBlockingInterrupt => Conv::AvrNonBlockingInterrupt,
Wasm => Conv::C,

// These API constants ought to be more specific...
Cdecl => Conv::C,
}
}

pub fn fn_ptr_codegen_fn_attr_flags() -> CodegenFnAttrFlags {
// Assume that fn pointers may always unwind
CodegenFnAttrFlags::UNWIND
}
RalfJung marked this conversation as resolved.
Show resolved Hide resolved

impl<'tcx, C> FnAbiExt<'tcx, C> for call::FnAbi<'tcx, Ty<'tcx>>
where
C: LayoutOf<Ty = Ty<'tcx>, TyAndLayout = TyAndLayout<'tcx>>
Expand All @@ -2650,10 +2687,7 @@ where
+ HasParamEnv<'tcx>,
{
fn of_fn_ptr(cx: &C, sig: ty::PolyFnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self {
// Assume that fn pointers may always unwind
let codegen_fn_attr_flags = CodegenFnAttrFlags::UNWIND;

call::FnAbi::new_internal(cx, sig, extra_args, None, codegen_fn_attr_flags, false)
call::FnAbi::new_internal(cx, sig, extra_args, None, fn_ptr_codegen_fn_attr_flags(), false)
}

fn of_instance(cx: &C, instance: ty::Instance<'tcx>, extra_args: &[Ty<'tcx>]) -> Self {
Expand Down Expand Up @@ -2689,35 +2723,7 @@ where

let sig = cx.tcx().normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), sig);

use rustc_target::spec::abi::Abi::*;
let conv = match cx.tcx().sess.target.adjust_abi(sig.abi) {
RustIntrinsic | PlatformIntrinsic | Rust | RustCall => Conv::Rust,

// It's the ABI's job to select this, not ours.
System { .. } => bug!("system abi should be selected elsewhere"),
EfiApi => bug!("eficall abi should be selected elsewhere"),

Stdcall { .. } => Conv::X86Stdcall,
Fastcall => Conv::X86Fastcall,
Vectorcall => Conv::X86VectorCall,
Thiscall { .. } => Conv::X86ThisCall,
C { .. } => Conv::C,
Unadjusted => Conv::C,
Win64 => Conv::X86_64Win64,
SysV64 => Conv::X86_64SysV,
Aapcs => Conv::ArmAapcs,
CCmseNonSecureCall => Conv::CCmseNonSecureCall,
PtxKernel => Conv::PtxKernel,
Msp430Interrupt => Conv::Msp430Intr,
X86Interrupt => Conv::X86Intr,
AmdGpuKernel => Conv::AmdGpuKernel,
AvrInterrupt => Conv::AvrInterrupt,
AvrNonBlockingInterrupt => Conv::AvrNonBlockingInterrupt,
Wasm => Conv::C,

// These API constants ought to be more specific...
Cdecl => Conv::C,
};
let conv = conv_from_spec_abi(cx.tcx(), sig.abi);

let mut inputs = sig.inputs();
let extra_args = if sig.abi == RustCall {
Expand Down Expand Up @@ -2753,6 +2759,7 @@ where
target.os == "linux" && target.arch == "sparc64" && target_env_gnu_like;
let linux_powerpc_gnu_like =
target.os == "linux" && target.arch == "powerpc" && target_env_gnu_like;
use SpecAbi::*;
let rust_abi = matches!(sig.abi, RustIntrinsic | PlatformIntrinsic | Rust | RustCall);

// Handle safe Rust thin and fat pointers.
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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)? {
Expand Down
59 changes: 40 additions & 19 deletions compiler/rustc_mir/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(());
Expand All @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_mir/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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 {}
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_mir/src/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
Loading