Auto merge of #34830 - michaelwoerister:internal-closures, r=nikomats…

…akis

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

This PR proposes one possible solution to #34793, the problem that prevents servo/servo#12393 from landing. It applies the same strategy, that we already use for monomorphizations, to closures, that is, instead of emitting symbols with `weak_odr` linkage in order to avoid symbol conflicts, we emit them with `internal` linkage, with the side effect that we have to copy code instead of just linking to it, if more than one codegen unit is involved.
With this PR, the compiler will only apply this strategy for targets where we would actually run into a problem when using `weak_odr` linkage, in other words nothing will change for platforms except for MinGW.

The solution implemented here has one restriction that could be lifted with some more effort, but it does not seem to be worth the trouble since it will go away once we use only MIR-trans: If someone compiles code

1. on MinGW,
2. with more than one codegen unit,
3. *not* using MIR-trans,
4. and runs into a closure inlined from another crate

then the compiler will abort and suggest to compile either with just one codegen unit or `-Zorbit`.

What's nice about this is that I lays a foundation for also doing the same for generics: using weak linkage where possible and thus enabling some more space optimizations that the linker can do.

~~This PR also contains a test case for compiling a program that contains more than 2^15 closures. It's a huge, generated file with almost 100K LOCs. I did not commit the script for generating the file but could do so. Alternatively, maybe someone wants to come up with a way of doing this with macros.~~
The test file is implemented via macros now (thanks @alexcrichton!)

Opinions?

Fixes #34793.

cc @rust-lang/compiler

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,

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);

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.

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);

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 {

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() {

src/test/run-pass/myriad-closures.rs

@@ -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);
+}