Skip to content

trans: Avoid weak linkage for closures when linking with MinGW. #34830

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

Merged
merged 2 commits into from
Aug 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,11 @@ impl Options {
self.debugging_opts.dump_dep_graph ||
self.debugging_opts.query_dep_graph
}

pub fn single_codegen_unit(&self) -> bool {
self.incremental.is_none() ||
self.cg.codegen_units == 1
}
}

// The type of entry function, so
Expand Down Expand Up @@ -655,7 +660,6 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options,
"panic strategy to compile crate with"),
}


options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
build_debugging_options, "Z", "debugging",
DB_OPTIONS, db_type_desc, dbsetters,
Expand Down
10 changes: 10 additions & 0 deletions src/librustc_back/target/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,13 @@ pub struct TargetOptions {
pub is_like_android: bool,
/// Whether the linker support GNU-like arguments such as -O. Defaults to false.
pub linker_is_gnu: bool,
/// The MinGW toolchain has a known issue that prevents it from correctly
/// handling COFF object files with more than 2^15 sections. Since each weak
/// symbol needs its own COMDAT section, weak linkage implies a large
/// number sections that easily exceeds the given limit for larger
/// codebases. Consequently we want a way to disallow weak linkage on some
/// platforms.
pub allows_weak_linkage: bool,
/// Whether the linker support rpaths or not. Defaults to false.
pub has_rpath: bool,
/// Whether to disable linking to compiler-rt. Defaults to false, as LLVM
Expand Down Expand Up @@ -367,6 +374,7 @@ impl Default for TargetOptions {
is_like_android: false,
is_like_msvc: false,
linker_is_gnu: false,
allows_weak_linkage: true,
has_rpath: false,
no_compiler_rt: false,
no_default_libraries: true,
Expand Down Expand Up @@ -509,6 +517,7 @@ impl Target {
key!(is_like_msvc, bool);
key!(is_like_android, bool);
key!(linker_is_gnu, bool);
key!(allows_weak_linkage, bool);
key!(has_rpath, bool);
key!(no_compiler_rt, bool);
key!(no_default_libraries, bool);
Expand Down Expand Up @@ -651,6 +660,7 @@ impl ToJson for Target {
target_option_val!(is_like_msvc);
target_option_val!(is_like_android);
target_option_val!(linker_is_gnu);
target_option_val!(allows_weak_linkage);
target_option_val!(has_rpath);
target_option_val!(no_compiler_rt);
target_option_val!(no_default_libraries);
Expand Down
1 change: 1 addition & 0 deletions src/librustc_back/target/windows_base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub fn opts() -> TargetOptions {
staticlib_suffix: ".lib".to_string(),
no_default_libraries: true,
is_like_windows: true,
allows_weak_linkage: false,
pre_link_args: vec!(
// And here, we see obscure linker flags #45. On windows, it has been
// found to be necessary to have this flag to compile liblibc.
Expand Down
77 changes: 75 additions & 2 deletions src/librustc_trans/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,41 @@ fn get_or_create_closure_declaration<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
llfn
}

fn translating_closure_body_via_mir_will_fail(ccx: &CrateContext,
closure_def_id: DefId)
-> bool {
let default_to_mir = ccx.sess().opts.debugging_opts.orbit;
let invert = if default_to_mir { "rustc_no_mir" } else { "rustc_mir" };
let use_mir = default_to_mir ^ ccx.tcx().has_attr(closure_def_id, invert);

!use_mir
}

pub fn trans_closure_body_via_mir<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
closure_def_id: DefId,
closure_substs: ty::ClosureSubsts<'tcx>) {
use syntax::ast::DUMMY_NODE_ID;
use syntax_pos::DUMMY_SP;
use syntax::ptr::P;

trans_closure_expr(Dest::Ignore(ccx),
&hir::FnDecl {
inputs: P::new(),
output: hir::NoReturn(DUMMY_SP),
variadic: false
},
&hir::Block {
stmts: P::new(),
expr: None,
id: DUMMY_NODE_ID,
rules: hir::DefaultBlock,
span: DUMMY_SP
},
DUMMY_NODE_ID,
closure_def_id,
closure_substs);
}

pub enum Dest<'a, 'tcx: 'a> {
SaveIn(Block<'a, 'tcx>, ValueRef),
Ignore(&'a CrateContext<'a, 'tcx>)
Expand Down Expand Up @@ -213,8 +248,13 @@ pub fn trans_closure_expr<'a, 'tcx>(dest: Dest<'a, 'tcx>,
// If we have not done so yet, translate this closure's body
if !ccx.instances().borrow().contains_key(&instance) {
let llfn = get_or_create_closure_declaration(ccx, closure_def_id, closure_substs);
llvm::SetLinkage(llfn, llvm::WeakODRLinkage);
llvm::SetUniqueComdat(ccx.llmod(), llfn);

if ccx.sess().target.target.options.allows_weak_linkage {
llvm::SetLinkage(llfn, llvm::WeakODRLinkage);
llvm::SetUniqueComdat(ccx.llmod(), llfn);
} else {
llvm::SetLinkage(llfn, llvm::InternalLinkage);
}

// set an inline hint for all closures
attributes::inline(llfn, attributes::InlineAttr::Hint);
Expand Down Expand Up @@ -296,6 +336,39 @@ pub fn trans_closure_method<'a, 'tcx>(ccx: &'a CrateContext<'a, 'tcx>,
// If this is a closure, redirect to it.
let llfn = get_or_create_closure_declaration(ccx, closure_def_id, substs);

// If weak linkage is not allowed, we have to make sure that a local,
// private copy of the closure is available in this codegen unit
if !ccx.sess().target.target.options.allows_weak_linkage &&
!ccx.sess().opts.single_codegen_unit() {

if let Some(node_id) = ccx.tcx().map.as_local_node_id(closure_def_id) {
// If the closure is defined in the local crate, we can always just
// translate it.
let (decl, body) = match ccx.tcx().map.expect_expr(node_id).node {
hir::ExprClosure(_, ref decl, ref body, _) => (decl, body),
_ => { unreachable!() }
};

trans_closure_expr(Dest::Ignore(ccx),
decl,
body,
node_id,
closure_def_id,
substs);
} else {
// If the closure is defined in an upstream crate, we can only
// translate it if MIR-trans is active.

if translating_closure_body_via_mir_will_fail(ccx, closure_def_id) {
ccx.sess().fatal("You have run into a known limitation of the \
MingW toolchain. Either compile with -Zorbit or \
with -Ccodegen-units=1 to work around it.");
}

trans_closure_body_via_mir(ccx, closure_def_id, substs);
}
}

// If the closure is a Fn closure, but a FnOnce is needed (etc),
// then adapt the self type
let llfn_closure_kind = ccx.tcx().closure_kind(closure_def_id);
Expand Down
22 changes: 3 additions & 19 deletions src/librustc_trans/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,26 +530,10 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> {

// FIXME Shouldn't need to manually trigger closure instantiations.
if let mir::AggregateKind::Closure(def_id, substs) = *kind {
use rustc::hir;
use syntax::ast::DUMMY_NODE_ID;
use syntax::ptr::P;
use closure;

closure::trans_closure_expr(closure::Dest::Ignore(self.ccx),
&hir::FnDecl {
inputs: P::new(),
output: hir::NoReturn(DUMMY_SP),
variadic: false
},
&hir::Block {
stmts: P::new(),
expr: None,
id: DUMMY_NODE_ID,
rules: hir::DefaultBlock,
span: DUMMY_SP
},
DUMMY_NODE_ID, def_id,
self.monomorphize(&substs));
closure::trans_closure_body_via_mir(self.ccx,
def_id,
self.monomorphize(&substs));
}

let val = if let mir::AggregateKind::Adt(adt_def, index, _) = *kind {
Expand Down
22 changes: 3 additions & 19 deletions src/librustc_trans/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,27 +131,11 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
_ => {
// FIXME Shouldn't need to manually trigger closure instantiations.
if let mir::AggregateKind::Closure(def_id, substs) = *kind {
use rustc::hir;
use syntax::ast::DUMMY_NODE_ID;
use syntax::ptr::P;
use syntax_pos::DUMMY_SP;
use closure;

closure::trans_closure_expr(closure::Dest::Ignore(bcx.ccx()),
&hir::FnDecl {
inputs: P::new(),
output: hir::NoReturn(DUMMY_SP),
variadic: false
},
&hir::Block {
stmts: P::new(),
expr: None,
id: DUMMY_NODE_ID,
rules: hir::DefaultBlock,
span: DUMMY_SP
},
DUMMY_NODE_ID, def_id,
bcx.monomorphize(&substs));
closure::trans_closure_body_via_mir(bcx.ccx(),
def_id,
bcx.monomorphize(&substs));
}

for (i, operand) in operands.iter().enumerate() {
Expand Down
48 changes: 48 additions & 0 deletions src/test/run-pass/myriad-closures.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// This test case tests whether we can handle code bases that contain a high
// number of closures, something that needs special handling in the MingGW
// toolchain.
// See https://github.com/rust-lang/rust/issues/34793 for more information.

// Expand something exponentially
macro_rules! go_bacterial {
($mac:ident) => ($mac!());
($mac:ident 1 $($t:tt)*) => (
go_bacterial!($mac $($t)*);
go_bacterial!($mac $($t)*);
)
}

macro_rules! mk_closure {
() => ({
let c = |a: u32| a + 4;
let _ = c(2);
})
}

macro_rules! mk_fn {
() => {
{
fn function() {
// Make 16 closures
go_bacterial!(mk_closure 1 1 1 1);
}
let _ = function();
}
}
}

fn main() {
// Make 2^12 functions, each containing 16 closures,
// resulting in 2^16 closures overall.
go_bacterial!(mk_fn 1 1 1 1 1 1 1 1 1 1 1 1);
}