Skip to content

Commit

Permalink
Auto merge of rust-lang#85316 - eddyb:cg-ssa-on-demand-cleanuppad, r=…
Browse files Browse the repository at this point in the history 
…nagisa

rustc_codegen_ssa: generate MSVC cleanup pads on demand, like GNU landing pads.

This unblocks rust-lang#84993 in terms of codegen tests, as it brings the MSVC-style (`cleanup_pad`) EH (LLVM) block order in line with the GNU-style (`landing_pad`) EH (LLVM) block order, by having both of them be on-demand (instead of MSVC-style being eager and GNU-style lazy/on-demand).

It also unifies the two implementations a bit, similar to rust-lang#84699, but in the opposite direction (as that attempt made both kinds of EH pads eagerly built).

~~Opening as draft because I haven't done enough Windows testing just yet, of both this PR, and of rust-lang#84993 rebased on it.~~ (**EDIT**: seems to be working as expected)

r? `@nagisa`
  • Loading branch information
@bors
bors committed May 16, 2021
2 parents 3f46b82 + cb23a79 commit 747a5d2
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 112 deletions.
133 changes: 102 additions & 31 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
use rustc_span::source_map::Span;
use rustc_span::{sym, Symbol};
use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode};
use rustc_target::abi::{self, LayoutOf};
use rustc_target::abi::{self, HasDataLayout, LayoutOf};
use rustc_target::spec::abi::Abi;

/// Used by `FunctionCx::codegen_terminator` for emitting common patterns
Expand All @@ -32,13 +32,34 @@ struct TerminatorCodegenHelper<'tcx> {
}

impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
/// Returns the associated funclet from `FunctionCx::funclets` for the
/// `funclet_bb` member if it is not `None`.
/// Returns the appropriate `Funclet` for the current funclet, if on MSVC,
/// either already previously cached, or newly created, by `landing_pad_for`.
fn funclet<'b, Bx: BuilderMethods<'a, 'tcx>>(
&self,
fx: &'b FunctionCx<'a, 'tcx, Bx>,
fx: &'b mut FunctionCx<'a, 'tcx, Bx>,
) -> Option<&'b Bx::Funclet> {
self.funclet_bb.and_then(|funcl| fx.funclets[funcl].as_ref())
let funclet_bb = self.funclet_bb?;
if base::wants_msvc_seh(fx.cx.tcx().sess) {
// If `landing_pad_for` hasn't been called yet to create the `Funclet`,
// it has to be now. This may not seem necessary, as RPO should lead
// to all the unwind edges being visited (and so to `landing_pad_for`
// getting called for them), before building any of the blocks inside
// the funclet itself - however, if MIR contains edges that end up not
// being needed in the LLVM IR after monomorphization, the funclet may
// be unreachable, and we don't have yet a way to skip building it in
// such an eventuality (which may be a better solution than this).
if fx.funclets[funclet_bb].is_none() {
fx.landing_pad_for(funclet_bb);
}

Some(
fx.funclets[funclet_bb]
.as_ref()
.expect("landing_pad_for didn't also create funclets entry"),
)
} else {
None
}
}

fn lltarget<Bx: BuilderMethods<'a, 'tcx>>(
Expand All @@ -54,10 +75,10 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
(Some(f), Some(t_f)) if f == t_f || !base::wants_msvc_seh(fx.cx.tcx().sess) => {
(lltarget, false)
}
// jump *into* cleanup - need a landing pad if GNU
(None, Some(_)) => (fx.landing_pad_to(target), false),
// jump *into* cleanup - need a landing pad if GNU, cleanup pad if MSVC
(None, Some(_)) => (fx.landing_pad_for(target), false),
(Some(_), None) => span_bug!(span, "{:?} - jump out of cleanup?", self.terminator),
(Some(_), Some(_)) => (fx.landing_pad_to(target), true),
(Some(_), Some(_)) => (fx.landing_pad_for(target), true),
}
}

Expand Down Expand Up @@ -1170,38 +1191,88 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}
}

/// Returns the landing-pad wrapper around the given basic block.
///
/// No-op in MSVC SEH scheme.
fn landing_pad_to(&mut self, target_bb: mir::BasicBlock) -> Bx::BasicBlock {
if let Some(block) = self.landing_pads[target_bb] {
return block;
/// Returns the landing/cleanup pad wrapper around the given basic block.
// FIXME(eddyb) rename this to `eh_pad_for`.
fn landing_pad_for(&mut self, bb: mir::BasicBlock) -> Bx::BasicBlock {
if let Some(landing_pad) = self.landing_pads[bb] {
return landing_pad;
}

let block = self.blocks[target_bb];
let landing_pad = self.landing_pad_uncached(block);
self.landing_pads[target_bb] = Some(landing_pad);
let landing_pad = self.landing_pad_for_uncached(bb);
self.landing_pads[bb] = Some(landing_pad);
landing_pad
}

fn landing_pad_uncached(&mut self, target_bb: Bx::BasicBlock) -> Bx::BasicBlock {
// FIXME(eddyb) rename this to `eh_pad_for_uncached`.
fn landing_pad_for_uncached(&mut self, bb: mir::BasicBlock) -> Bx::BasicBlock {
let llbb = self.blocks[bb];
if base::wants_msvc_seh(self.cx.sess()) {
span_bug!(self.mir.span, "landing pad was not inserted?")
}

let mut bx = self.new_block("cleanup");
let funclet;
let ret_llbb;
match self.mir[bb].terminator.as_ref().map(|t| &t.kind) {
// This is a basic block that we're aborting the program for,
// notably in an `extern` function. These basic blocks are inserted
// so that we assert that `extern` functions do indeed not panic,
// and if they do we abort the process.
//
// On MSVC these are tricky though (where we're doing funclets). If
// we were to do a cleanuppad (like below) the normal functions like
// `longjmp` would trigger the abort logic, terminating the
// program. Instead we insert the equivalent of `catch(...)` for C++
// which magically doesn't trigger when `longjmp` files over this
// frame.
//
// Lots more discussion can be found on #48251 but this codegen is
// modeled after clang's for:
//
// try {
// foo();
// } catch (...) {
// bar();
// }
Some(&mir::TerminatorKind::Abort) => {
let mut cs_bx = self.new_block(&format!("cs_funclet{:?}", bb));
let mut cp_bx = self.new_block(&format!("cp_funclet{:?}", bb));
ret_llbb = cs_bx.llbb();

let cs = cs_bx.catch_switch(None, None, 1);
cs_bx.add_handler(cs, cp_bx.llbb());

// The "null" here is actually a RTTI type descriptor for the
// C++ personality function, but `catch (...)` has no type so
// it's null. The 64 here is actually a bitfield which
// represents that this is a catch-all block.
let null = cp_bx.const_null(
cp_bx.type_i8p_ext(cp_bx.cx().data_layout().instruction_address_space),
);
let sixty_four = cp_bx.const_i32(64);
funclet = cp_bx.catch_pad(cs, &[null, sixty_four, null]);
cp_bx.br(llbb);
}
_ => {
let mut cleanup_bx = self.new_block(&format!("funclet_{:?}", bb));
ret_llbb = cleanup_bx.llbb();
funclet = cleanup_bx.cleanup_pad(None, &[]);
cleanup_bx.br(llbb);
}
}
self.funclets[bb] = Some(funclet);
ret_llbb
} else {
let mut bx = self.new_block("cleanup");

let llpersonality = self.cx.eh_personality();
let llretty = self.landing_pad_type();
let lp = bx.landing_pad(llretty, llpersonality, 1);
bx.set_cleanup(lp);
let llpersonality = self.cx.eh_personality();
let llretty = self.landing_pad_type();
let lp = bx.landing_pad(llretty, llpersonality, 1);
bx.set_cleanup(lp);

let slot = self.get_personality_slot(&mut bx);
slot.storage_live(&mut bx);
Pair(bx.extract_value(lp, 0), bx.extract_value(lp, 1)).store(&mut bx, slot);
let slot = self.get_personality_slot(&mut bx);
slot.storage_live(&mut bx);
Pair(bx.extract_value(lp, 0), bx.extract_value(lp, 1)).store(&mut bx, slot);

bx.br(target_bb);
bx.llbb()
bx.br(llbb);
bx.llbb()
}
}

fn landing_pad_type(&self) -> Bx::Type {
Expand Down
88 changes: 7 additions & 81 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
use crate::base;
use crate::traits::*;
use rustc_errors::ErrorReported;
use rustc_middle::mir;
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt, TyAndLayout};
use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
use rustc_target::abi::call::{FnAbi, PassMode};
use rustc_target::abi::HasDataLayout;

use std::iter;

use rustc_index::bit_set::BitSet;
use rustc_index::vec::IndexVec;

use self::analyze::CleanupKind;
use self::debuginfo::{FunctionDebugContext, PerLocalVarDebugInfo};
use self::place::PlaceRef;
use rustc_middle::mir::traversal;
Expand Down Expand Up @@ -49,12 +46,13 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
/// The funclet status of each basic block
cleanup_kinds: IndexVec<mir::BasicBlock, analyze::CleanupKind>,

/// When targeting MSVC, this stores the cleanup info for each funclet
/// BB. This is initialized as we compute the funclets' head block in RPO.
/// When targeting MSVC, this stores the cleanup info for each funclet BB.
/// This is initialized at the same time as the `landing_pads` entry for the
/// funclets' head block, i.e. when needed by an unwind / `cleanup_ret` edge.
funclets: IndexVec<mir::BasicBlock, Option<Bx::Funclet>>,

/// This stores the landing-pad block for a given BB, computed lazily on GNU
/// and eagerly on MSVC.
/// This stores the cached landing/cleanup pad block for a given BB.
// FIXME(eddyb) rename this to `eh_pads`.
landing_pads: IndexVec<mir::BasicBlock, Option<Bx::BasicBlock>>,

/// Cached unreachable block
Expand Down Expand Up @@ -165,7 +163,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
})
.collect();

let (landing_pads, funclets) = create_funclets(&mir, &mut bx, &cleanup_kinds, &block_bxs);
let mut fx = FunctionCx {
instance,
mir,
Expand All @@ -176,8 +173,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
blocks: block_bxs,
unreachable_block: None,
cleanup_kinds,
landing_pads,
funclets,
landing_pads: IndexVec::from_elem(None, mir.basic_blocks()),
funclets: IndexVec::from_fn_n(|_| None, mir.basic_blocks().len()),
locals: IndexVec::new(),
debug_context,
per_local_var_debug_info: None,
Expand Down Expand Up @@ -273,77 +270,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
}
}

fn create_funclets<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
mir: &'tcx mir::Body<'tcx>,
bx: &mut Bx,
cleanup_kinds: &IndexVec<mir::BasicBlock, CleanupKind>,
block_bxs: &IndexVec<mir::BasicBlock, Bx::BasicBlock>,
) -> (
IndexVec<mir::BasicBlock, Option<Bx::BasicBlock>>,
IndexVec<mir::BasicBlock, Option<Bx::Funclet>>,
) {
iter::zip(block_bxs.iter_enumerated(), cleanup_kinds)
.map(|((bb, &llbb), cleanup_kind)| {
match *cleanup_kind {
CleanupKind::Funclet if base::wants_msvc_seh(bx.sess()) => {}
_ => return (None, None),
}

let funclet;
let ret_llbb;
match mir[bb].terminator.as_ref().map(|t| &t.kind) {
// This is a basic block that we're aborting the program for,
// notably in an `extern` function. These basic blocks are inserted
// so that we assert that `extern` functions do indeed not panic,
// and if they do we abort the process.
//
// On MSVC these are tricky though (where we're doing funclets). If
// we were to do a cleanuppad (like below) the normal functions like
// `longjmp` would trigger the abort logic, terminating the
// program. Instead we insert the equivalent of `catch(...)` for C++
// which magically doesn't trigger when `longjmp` files over this
// frame.
//
// Lots more discussion can be found on #48251 but this codegen is
// modeled after clang's for:
//
// try {
// foo();
// } catch (...) {
// bar();
// }
Some(&mir::TerminatorKind::Abort) => {
let mut cs_bx = bx.build_sibling_block(&format!("cs_funclet{:?}", bb));
let mut cp_bx = bx.build_sibling_block(&format!("cp_funclet{:?}", bb));
ret_llbb = cs_bx.llbb();

let cs = cs_bx.catch_switch(None, None, 1);
cs_bx.add_handler(cs, cp_bx.llbb());

// The "null" here is actually a RTTI type descriptor for the
// C++ personality function, but `catch (...)` has no type so
// it's null. The 64 here is actually a bitfield which
// represents that this is a catch-all block.
let null = bx.const_null(
bx.type_i8p_ext(bx.cx().data_layout().instruction_address_space),
);
let sixty_four = bx.const_i32(64);
funclet = cp_bx.catch_pad(cs, &[null, sixty_four, null]);
cp_bx.br(llbb);
}
_ => {
let mut cleanup_bx = bx.build_sibling_block(&format!("funclet_{:?}", bb));
ret_llbb = cleanup_bx.llbb();
funclet = cleanup_bx.cleanup_pad(None, &[]);
cleanup_bx.br(llbb);
}
};

(Some(ret_llbb), Some(funclet))
})
.unzip()
}

/// Produces, for each argument, a `Value` pointing at the
/// argument's value. As arguments are places, these are always
/// indirect.
Expand Down

0 comments on commit 747a5d2

Please sign in to comment.