Skip to content

Commit

Permalink
Auto merge of #26919 - alexcrichton:msvc-turn-off-unwinding, r=brson
Browse files Browse the repository at this point in the history
There are a number of problems with MSVC landing pads today:

* They only work about 80% of the time with optimizations enabled. For example when running the run-pass test suite a failing test will cause `compiletest` to segfault (b/c of a thread panic). There are also a large number of run-fail tests which will simply crash.
* Enabling landing pads caused the regression seen in #26915.

Overall it looks like LLVM's support for MSVC landing pads isn't as robust as we'd like for now, so let's take a little more time before we turn them on by default.


Closes #26915
  • Loading branch information
bors committed Jul 10, 2015
2 parents d0d3707 + 813cfa5 commit cdcce3b
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 31 deletions.
16 changes: 7 additions & 9 deletions src/librustc_trans/trans/base.rs
Expand Up @@ -753,15 +753,13 @@ pub fn invoke<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
}

pub fn need_invoke(bcx: Block) -> bool {
if bcx.sess().no_landing_pads() {
return false
}

// Currently 32-bit MSVC unwinding is not super well implemented in LLVM, so
// we avoid it entirely.
if bcx.sess().target.target.options.is_like_msvc &&
bcx.sess().target.target.arch == "x86" {
return false
// FIXME(#25869) currently unwinding is not implemented for MSVC and our
// normal unwinding infrastructure ends up just causing linker
// errors with the current LLVM implementation, so landing
// pads are disabled entirely for MSVC targets
if bcx.sess().no_landing_pads() ||
bcx.sess().target.target.options.is_like_msvc {
return false;
}

// Avoid using invoke if we are already inside a landing pad.
Expand Down
23 changes: 1 addition & 22 deletions src/librustc_trans/trans/glue.rs
Expand Up @@ -22,9 +22,8 @@ use middle::lang_items::ExchangeFreeFnLangItem;
use middle::subst;
use middle::subst::{Subst, Substs};
use middle::ty::{self, Ty};
use trans::adt::GetDtorType; // for tcx.dtor_type()
use trans::adt;
use trans::attributes;
use trans::adt::GetDtorType; // for tcx.dtor_type()
use trans::base::*;
use trans::build::*;
use trans::callee;
Expand All @@ -44,7 +43,6 @@ use trans::type_::Type;
use arena::TypedArena;
use libc::c_uint;
use syntax::ast;
use syntax::attr::InlineAttr;

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

update_linkage(ccx, llfn, None, OriginalTranslation);

// FIXME: Currently LLVM has a bug where if an SSA value is created in one
// landing pad and then used in another it will abort during
// compilation. The compiler never actually generates nested landing
// pads, but this often arises when destructors are inlined into
// other functions. To prevent this inlining from happening (and thus
// preventing the LLVM abort) we mark all drop glue as inline(never)
// on MSVC.
//
// For more information about the bug, see:
//
// https://llvm.org/bugs/show_bug.cgi?id=23884
//
// This is clearly not the ideal solution to the problem (due to the
// perf hits), so this should be removed once the upstream bug is
// fixed.
if ccx.sess().target.target.options.is_like_msvc {
attributes::inline(llfn, InlineAttr::Never);
}

ccx.stats().n_glues_created.set(ccx.stats().n_glues_created.get() + 1);
// All glue functions take values passed *by alias*; this is a
// requirement since in many contexts glue is invoked indirectly and
Expand Down

0 comments on commit cdcce3b

Please sign in to comment.