From 7b3e10c751ba79344591763bc205986b425dc445 Mon Sep 17 00:00:00 2001 From: hyd-dev Date: Fri, 21 May 2021 20:40:36 +0800 Subject: [PATCH] const-eval: disallow unwinding across functions that `!fn_can_unwind()` --- compiler/rustc_middle/src/ty/layout.rs | 75 ++++++++++--------- .../rustc_mir/src/interpret/eval_context.rs | 37 ++++++++- compiler/rustc_mir/src/interpret/mod.rs | 4 +- .../rustc_mir/src/interpret/terminator.rs | 37 +++++++-- 4 files changed, 108 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index c2e9dba6c8e..03a026500d7 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -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, @@ -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 +} + impl<'tcx, C> FnAbiExt<'tcx, C> for call::FnAbi<'tcx, Ty<'tcx>> where C: LayoutOf, TyAndLayout = TyAndLayout<'tcx>> @@ -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 { @@ -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 { @@ -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. diff --git a/compiler/rustc_mir/src/interpret/eval_context.rs b/compiler/rustc_mir/src/interpret/eval_context.rs index e9dd7a3fe68..78d9c7f44f9 100644 --- a/compiler/rustc_mir/src/interpret/eval_context.rs +++ b/compiler/rustc_mir/src/interpret/eval_context.rs @@ -134,6 +134,15 @@ pub struct FrameInfo<'tcx> { pub lint_root: Option, } +/// Unwind information. +#[derive(Clone, Copy, Eq, PartialEq, Debug, HashStable)] +pub enum StackPopUnwind { + /// The cleanup block. + Cleanup(Option), + /// Unwinding is not allowed (UB). + NotAllowed, +} + #[derive(Clone, 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 @@ -141,7 +150,7 @@ pub enum StackPopCleanup { /// 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, unwind: Option }, + Goto { ret: Option, 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 @@ -807,9 +816,29 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // 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::Goto { ret, unwind } => ( + true, + Some(if unwinding { + let def_id = frame.body.source.def_id(); + match unwind { + StackPopUnwind::Cleanup(unwind) + // `fn_sig()` can't be used on closures, but closures always have + // "rust-call" ABI, which always allows unwinding anyway. + if self.tcx.is_closure(def_id) || self.fn_can_unwind( + self.tcx.codegen_fn_attrs(def_id).flags, + self.tcx.fn_sig(def_id).abi(), + ) => + { + unwind + } + _ => { + throw_ub_format!("unwind past a frame that does not allow unwinding") + } + } + } else { + ret + }), + ), StackPopCleanup::None { cleanup, .. } => (cleanup, None), }; diff --git a/compiler/rustc_mir/src/interpret/mod.rs b/compiler/rustc_mir/src/interpret/mod.rs index 9b95f691167..2b9fe565997 100644 --- a/compiler/rustc_mir/src/interpret/mod.rs +++ b/compiler/rustc_mir/src/interpret/mod.rs @@ -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}; diff --git a/compiler/rustc_mir/src/interpret/terminator.rs b/compiler/rustc_mir/src/interpret/terminator.rs index a3dc8aaef32..cd172c87eb1 100644 --- a/compiler/rustc_mir/src/interpret/terminator.rs +++ b/compiler/rustc_mir/src/interpret/terminator.rs @@ -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> { + pub(super) 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, 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,7 @@ 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, *cleanup, can_unwind)?; // 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 { @@ -220,6 +236,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { args: &[OpTy<'tcx, M::PointerTag>], ret: Option<(&PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>, unwind: Option, + can_unwind: bool, ) -> InterpResult<'tcx> { trace!("eval_fn_call: {:#?}", fn_val); @@ -287,7 +304,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { instance, body, ret.map(|p| p.0), - StackPopCleanup::Goto { ret: ret.map(|p| p.1), unwind }, + StackPopCleanup::Goto { + ret: ret.map(|p| p.1), + unwind: if can_unwind { + StackPopUnwind::Cleanup(unwind) + } else { + StackPopUnwind::NotAllowed + }, + }, )?; // If an error is raised here, pop the frame again to get an accurate backtrace. @@ -432,7 +456,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { OpTy::from(ImmTy::from_immediate(receiver_place.ptr.into(), this_receiver_ptr)); trace!("Patched self operand to {:#?}", args[0]); // recurse with concrete function - self.eval_fn_call(drop_fn, caller_abi, &args, ret, unwind) + self.eval_fn_call(drop_fn, caller_abi, &args, ret, unwind, can_unwind) } } } @@ -472,6 +496,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &[arg.into()], Some((&dest.into(), target)), unwind, + true, ) } }