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

michaelwoerister · michaelwoerister · commit 59cfe904dcfb · 2016-08-01T04:33:39.000-04:00
diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs
@@ -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
@@ -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,
diff --git a/src/librustc_back/target/mod.rs b/src/librustc_back/target/mod.rs
@@ -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
@@ -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,
@@ -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);
@@ -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);
diff --git a/src/librustc_back/target/windows_base.rs b/src/librustc_back/target/windows_base.rs
@@ -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.
diff --git a/src/librustc_trans/closure.rs b/src/librustc_trans/closure.rs
@@ -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>)
@@ -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);
@@ -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);
diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs
@@ -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 {
diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs
@@ -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() {

Original file line number	Diff line number	Diff line change
`@@ -330,6 +330,11 @@ impl Options {`
`330`	`330`	`self.debugging_opts.dump_dep_graph \|\|`
`331`	`331`	`self.debugging_opts.query_dep_graph`
`332`	`332`	`}`
	`333`	`+`
	`334`	`+ pub fn single_codegen_unit(&self) -> bool {`
	`335`	`+ self.incremental.is_none() \|\|`
	`336`	`+ self.cg.codegen_units == 1`
	`337`	`+ }`
`333`	`338`	`}`
`334`	`339`
`335`	`340`	`// The type of entry function, so`
`@@ -655,7 +660,6 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options,`
`655`	`660`	`"panic strategy to compile crate with"),`
`656`	`661`	`}`
`657`	`662`
`658`		`-`
`659`	`663`	`options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,`
`660`	`664`	`build_debugging_options, "Z", "debugging",`
`661`	`665`	`DB_OPTIONS, db_type_desc, dbsetters,`