Skip to content

Commit f9de964

Browse files
committed
msvc: Enable landing pads by default
This commit turns on landing pads for MSVC by default, which means that we'll now be running cleanups for values on the stack when an exception is thrown. This commit "fixes" the previously seen LLVM abort by attaching the `noinline` attribute to all generated drop glue to prevent landing pads from being inlined into other landing pads. The performance of MSVC is highly likely to decrease from this commit, but there are various routes we can taken in the future if this ends up staying for quite a while, such as generating a shim function only called from landing pads which calls the actual drop glue, and this shim is marked noinline. For now, however, this patch enables MSVC to successfully bootstrap itself!
1 parent d4fe2a0 commit f9de964

File tree

2 files changed

+23
-7
lines changed

2 files changed

+23
-7
lines changed

src/librustc_trans/trans/base.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -751,12 +751,7 @@ pub fn invoke<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
751751
}
752752

753753
pub fn need_invoke(bcx: Block) -> bool {
754-
// FIXME(#25869) currently unwinding is not implemented for MSVC and our
755-
// normal unwinding infrastructure ends up just causing linker
756-
// errors with the current LLVM implementation, so landing
757-
// pads are disabled entirely for MSVC targets
758-
if bcx.sess().no_landing_pads() ||
759-
bcx.sess().target.target.options.is_like_msvc {
754+
if bcx.sess().no_landing_pads() {
760755
return false;
761756
}
762757

src/librustc_trans/trans/glue.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ use middle::lang_items::ExchangeFreeFnLangItem;
2222
use middle::subst;
2323
use middle::subst::{Subst, Substs};
2424
use middle::ty::{self, Ty};
25-
use trans::adt;
2625
use trans::adt::GetDtorType; // for tcx.dtor_type()
26+
use trans::adt;
27+
use trans::attributes;
2728
use trans::base::*;
2829
use trans::build::*;
2930
use trans::callee;
@@ -43,6 +44,7 @@ use trans::type_::Type;
4344
use arena::TypedArena;
4445
use libc::c_uint;
4546
use syntax::ast;
47+
use syntax::attr::InlineAttr;
4648

4749
pub fn trans_exchange_free_dyn<'blk, 'tcx>(cx: Block<'blk, 'tcx>,
4850
v: ValueRef,
@@ -250,6 +252,25 @@ fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
250252

251253
update_linkage(ccx, llfn, None, OriginalTranslation);
252254

255+
// FIXME: Currently LLVM has a bug where if an SSA value is created in one
256+
// landing pad and then used in another it will abort during
257+
// compilation. The compiler never actually generates nested landing
258+
// pads, but this often arises when destructors are inlined into
259+
// other functions. To prevent this inlining from happening (and thus
260+
// preventing the LLVM abort) we mark all drop glue as inline(never)
261+
// on MSVC.
262+
//
263+
// For more information about the bug, see:
264+
//
265+
// https://llvm.org/bugs/show_bug.cgi?id=23884
266+
//
267+
// This is clearly not the ideal solution to the problem (due to the
268+
// perf hits), so this should be removed once the upstream bug is
269+
// fixed.
270+
if ccx.sess().target.target.options.is_like_msvc {
271+
attributes::inline(llfn, InlineAttr::Never);
272+
}
273+
253274
ccx.stats().n_glues_created.set(ccx.stats().n_glues_created.get() + 1);
254275
// All glue functions take values passed *by alias*; this is a
255276
// requirement since in many contexts glue is invoked indirectly and

0 commit comments

Comments
 (0)