Auto merge of #27416 - alexcrichton:fix-dll-export, r=brson

These two commits are aimed at "fixing" our usage of `dllexport` in the compiler. Currently we blanket apply this attribute to *everything* public in a crate, but this ends up with a few downsides:

* Executables are larger than the should be as a result of thinking they should export everything
* Native libraries aren't handled correctly because technically a statically included native library should be exported from a DLL in some cases.
* Symbols don't actually need to be exported if they never end up in a DLL.

The first commit adds a new unstable attribute, `#[linked_from]`, which is a way to tell the compiler what native library a block of symbols comes from. This is used to inform the compiler what set of native libraries are statically included in the rlib (or other output). This information is later used to export them from a DLL if necessary. Currently this is only used in a few places (such as the LLVM bindings) to get the compiler to link correctly.

The second commit stops adding `dllexport` to all items in LLVM and instead explicitly telling the linker what symbols should be exported. We only need to do this when building a dynamic library, and otherwise we can avoid adding `dllexport` or telling the linker about exported symbols.

As a testament to this change, the size of "Hello World" on MSVC drops from 1.2MB to 67KB as a result of this patch. This is because the linker can much more aggressively remove unused code.

These commits do not yet attempt to fix our story with `dllimport`, and I'll leave that to a future PR and issue, for now though I'm going to say that this

Closes #7196

Author: bors

mk/platform.mk

@@ -277,10 +277,15 @@ $(foreach target,$(CFG_TARGET), \
 # Fun times!
 #
 # [1]: https://msdn.microsoft.com/en-us/library/28d6s79h.aspx
+#
+# FIXME(stage0): remove this macro and the usage below (and the commments above)
+# 	         when a new snapshot is available. Also remove the
+# 	         RUSTFLAGS$(1)_.._T_ variable in mk/target.mk along with
+# 	         CUSTOM_DEPS (as they were only added for this)
 define ADD_RUSTC_LLVM_DEF_TO_MSVC
 ifeq ($$(findstring msvc,$(1)),msvc)
-RUSTFLAGS_rustc_llvm_T_$(1) += -C link-args="-DEF:$(1)/rt/rustc_llvm.def"
-CUSTOM_DEPS_rustc_llvm_T_$(1) += $(1)/rt/rustc_llvm.def
+RUSTFLAGS0_rustc_llvm_T_$(1) += -C link-args="-DEF:$(1)/rt/rustc_llvm.def"
+CUSTOM_DEPS0_rustc_llvm_T_$(1) += $(1)/rt/rustc_llvm.def
 
 $(1)/rt/rustc_llvm.def: $$(S)src/etc/mklldef.py $$(S)src/librustc_llvm/lib.rs
 	$$(CFG_PYTHON) $$^ $$@ rustc_llvm-$$(CFG_FILENAME_EXTRA)

mk/target.mk

@@ -40,7 +40,7 @@ CRATE_FULLDEPS_$(1)_T_$(2)_H_$(3)_$(4) := \
 		  $$(RT_OUTPUT_DIR_$(2))/$$(dep)) \
 		$$(foreach dep,$$(NATIVE_TOOL_DEPS_$(4)_T_$(2)), \
 		  $$(TBIN$(1)_T_$(3)_H_$(3))/$$(dep)) \
-		$$(CUSTOM_DEPS_$(4)_T_$(2))
+		$$(CUSTOM_DEPS$(1)_$(4)_T_$(2))
 endef
 
 $(foreach host,$(CFG_HOST), \
@@ -92,7 +92,7 @@ $$(TLIB$(1)_T_$(2)_H_$(3))/stamp.$(4): \
 		$$(LLVM_LIBDIR_RUSTFLAGS_$(2)) \
 		$$(LLVM_STDCPP_RUSTFLAGS_$(2)) \
 		$$(RUSTFLAGS_$(4)) \
-		$$(RUSTFLAGS_$(4)_T_$(2)) \
+		$$(RUSTFLAGS$(1)_$(4)_T_$(2)) \
 		--out-dir $$(@D) \
 		-C extra-filename=-$$(CFG_FILENAME_EXTRA) \
 		$$<

src/compiletest/procsrv.rs

@@ -26,8 +26,7 @@ fn add_target_env(cmd: &mut Command, lib_path: &str, aux_path: Option<&str>) {
     // Add the new dylib search path var
     let var = DynamicLibrary::envvar();
     let newpath = DynamicLibrary::create_path(&path);
-    let newpath = newpath.to_str().unwrap().to_string();
-    cmd.env(var, &newpath);
+    cmd.env(var, newpath);
 }
 
 pub struct Result {pub status: ExitStatus, pub out: String, pub err: String}

src/doc/reference.md

@@ -1924,10 +1924,16 @@ On an `extern` block, the following attributes are interpreted:
   name and type. This is feature gated and the exact behavior is
   implementation-defined (due to variety of linker invocation syntax).
 - `link` - indicate that a native library should be linked to for the
-  declarations in this block to be linked correctly. `link` supports an optional `kind`
-  key with three possible values: `dylib`, `static`, and `framework`. See [external blocks](#external-blocks) for more about external blocks. Two
+  declarations in this block to be linked correctly. `link` supports an optional
+  `kind` key with three possible values: `dylib`, `static`, and `framework`. See
+  [external blocks](#external-blocks) for more about external blocks. Two
   examples: `#[link(name = "readline")]` and
   `#[link(name = "CoreFoundation", kind = "framework")]`.
+- `linked_from` - indicates what native library this block of FFI items is
+  coming from. This attribute is of the form `#[linked_from = "foo"]` where
+  `foo` is the name of a library in either `#[link]` or a `-l` flag. This
+  attribute is currently required to export symbols from a Rust dynamic library
+  on Windows, and it is feature gated behind the `linked_from` feature.
 
 On declarations inside an `extern` block, the following attributes are
 interpreted:

src/librustc/metadata/common.rs

@@ -205,8 +205,8 @@ pub const tag_plugin_registrar_fn: usize = 0x10b; // top-level only
 pub const tag_method_argument_names: usize = 0x85;
 pub const tag_method_argument_name: usize = 0x86;
 
-pub const tag_reachable_extern_fns: usize = 0x10c; // top-level only
-pub const tag_reachable_extern_fn_id: usize = 0x87;
+pub const tag_reachable_ids: usize = 0x10c; // top-level only
+pub const tag_reachable_id: usize = 0x87;
 
 pub const tag_items_data_item_stability: usize = 0x88;
 

src/librustc/metadata/creader.rs

@@ -20,6 +20,7 @@ use metadata::cstore::{CStore, CrateSource, MetadataBlob};
 use metadata::decoder;
 use metadata::loader;
 use metadata::loader::CratePaths;
+use util::nodemap::FnvHashMap;
 
 use std::cell::RefCell;
 use std::path::PathBuf;
@@ -47,6 +48,7 @@ pub struct LocalCrateReader<'a, 'b:'a> {
 pub struct CrateReader<'a> {
     sess: &'a Session,
     next_crate_num: ast::CrateNum,
+    foreign_item_map: FnvHashMap<String, Vec<ast::NodeId>>,
 }
 
 impl<'a, 'b, 'v> visit::Visitor<'v> for LocalCrateReader<'a, 'b> {
@@ -157,6 +159,7 @@ impl<'a> CrateReader<'a> {
         CrateReader {
             sess: sess,
             next_crate_num: sess.cstore.next_crate_num(),
+            foreign_item_map: FnvHashMap(),
         }
     }
 
@@ -490,6 +493,20 @@ impl<'a> CrateReader<'a> {
             _ => None,
         }
     }
+
+    fn register_statically_included_foreign_items(&mut self) {
+        let libs = self.sess.cstore.get_used_libraries();
+        for (lib, list) in self.foreign_item_map.iter() {
+            let is_static = libs.borrow().iter().any(|&(ref name, kind)| {
+                lib == name && kind == cstore::NativeStatic
+            });
+            if is_static {
+                for id in list {
+                    self.sess.cstore.add_statically_included_foreign_item(*id);
+                }
+            }
+        }
+    }
 }
 
 impl<'a, 'b> LocalCrateReader<'a, 'b> {
@@ -515,6 +532,7 @@ impl<'a, 'b> LocalCrateReader<'a, 'b> {
         for &(ref name, kind) in &self.sess.opts.libs {
             register_native_lib(self.sess, None, name.clone(), kind);
         }
+        self.creader.register_statically_included_foreign_items();
     }
 
     fn process_crate(&self, c: &ast::Crate) {
@@ -541,87 +559,73 @@ impl<'a, 'b> LocalCrateReader<'a, 'b> {
                                                               None,
                                                               i.span,
                                                               PathKind::Crate);
-                        self.ast_map.with_path(i.id, |path|
-                            cmeta.update_local_path(path));
+                        self.ast_map.with_path(i.id, |path| {
+                            cmeta.update_local_path(path)
+                        });
                         self.sess.cstore.add_extern_mod_stmt_cnum(info.id, cnum);
                     }
                     None => ()
                 }
             }
-            ast::ItemForeignMod(ref fm) => {
-                if fm.abi == abi::Rust || fm.abi == abi::RustIntrinsic {
-                    return;
-                }
+            ast::ItemForeignMod(ref fm) => self.process_foreign_mod(i, fm),
+            _ => { }
+        }
+    }
 
-                // First, add all of the custom link_args attributes
-                let link_args = i.attrs.iter()
-                    .filter_map(|at| if at.name() == "link_args" {
-                        Some(at)
-                    } else {
-                        None
-                    })
-                    .collect::<Vec<&ast::Attribute>>();
-                for m in &link_args {
-                    match m.value_str() {
-                        Some(linkarg) => self.sess.cstore.add_used_link_args(&linkarg),
-                        None => { /* fallthrough */ }
-                    }
-                }
+    fn process_foreign_mod(&mut self, i: &ast::Item, fm: &ast::ForeignMod) {
+        if fm.abi == abi::Rust || fm.abi == abi::RustIntrinsic {
+            return;
+        }
 
-                // Next, process all of the #[link(..)]-style arguments
-                let link_args = i.attrs.iter()
-                    .filter_map(|at| if at.name() == "link" {
-                        Some(at)
-                    } else {
-                        None
-                    })
-                    .collect::<Vec<&ast::Attribute>>();
-                for m in &link_args {
-                    match m.meta_item_list() {
-                        Some(items) => {
-                            let kind = items.iter().find(|k| {
-                                k.name() == "kind"
-                            }).and_then(|a| a.value_str());
-                            let kind = match kind {
-                                Some(k) => {
-                                    if k == "static" {
-                                        cstore::NativeStatic
-                                    } else if self.sess.target.target.options.is_like_osx
-                                              && k == "framework" {
-                                        cstore::NativeFramework
-                                    } else if k == "framework" {
-                                        cstore::NativeFramework
-                                    } else if k == "dylib" {
-                                        cstore::NativeUnknown
-                                    } else {
-                                        self.sess.span_err(m.span,
-                                            &format!("unknown kind: `{}`",
-                                                    k));
-                                        cstore::NativeUnknown
-                                    }
-                                }
-                                None => cstore::NativeUnknown
-                            };
-                            let n = items.iter().find(|n| {
-                                n.name() == "name"
-                            }).and_then(|a| a.value_str());
-                            let n = match n {
-                                Some(n) => n,
-                                None => {
-                                    self.sess.span_err(m.span,
-                                        "#[link(...)] specified without \
-                                         `name = \"foo\"`");
-                                    InternedString::new("foo")
-                                }
-                            };
-                            register_native_lib(self.sess, Some(m.span),
-                                                n.to_string(), kind);
-                        }
-                        None => {}
-                    }
-                }
+        // First, add all of the custom #[link_args] attributes
+        for m in i.attrs.iter().filter(|a| a.check_name("link_args")) {
+            if let Some(linkarg) = m.value_str() {
+                self.sess.cstore.add_used_link_args(&linkarg);
             }
-            _ => { }
+        }
+
+        // Next, process all of the #[link(..)]-style arguments
+        for m in i.attrs.iter().filter(|a| a.check_name("link")) {
+            let items = match m.meta_item_list() {
+                Some(item) => item,
+                None => continue,
+            };
+            let kind = items.iter().find(|k| {
+                k.check_name("kind")
+            }).and_then(|a| a.value_str());
+            let kind = match kind.as_ref().map(|s| &s[..]) {
+                Some("static") => cstore::NativeStatic,
+                Some("dylib") => cstore::NativeUnknown,
+                Some("framework") => cstore::NativeFramework,
+                Some(k) => {
+                    self.sess.span_err(m.span, &format!("unknown kind: `{}`", k));
+                    cstore::NativeUnknown
+                }
+                None => cstore::NativeUnknown
+            };
+            let n = items.iter().find(|n| {
+                n.check_name("name")
+            }).and_then(|a| a.value_str());
+            let n = match n {
+                Some(n) => n,
+                None => {
+                    self.sess.span_err(m.span, "#[link(...)] specified without \
+                                                `name = \"foo\"`");
+                    InternedString::new("foo")
+                }
+            };
+            register_native_lib(self.sess, Some(m.span), n.to_string(), kind);
+        }
+
+        // Finally, process the #[linked_from = "..."] attribute
+        for m in i.attrs.iter().filter(|a| a.check_name("linked_from")) {
+            let lib_name = match m.value_str() {
+                Some(name) => name,
+                None => continue,
+            };
+            let list = self.creader.foreign_item_map.entry(lib_name.to_string())
+                                                    .or_insert(Vec::new());
+            list.extend(fm.items.iter().map(|it| it.id));
         }
     }
 }

src/librustc/metadata/csearch.rs

@@ -352,11 +352,11 @@ pub fn get_method_arg_names(cstore: &cstore::CStore, did: ast::DefId)
     decoder::get_method_arg_names(&*cdata, did.node)
 }
 
-pub fn get_reachable_extern_fns(cstore: &cstore::CStore, cnum: ast::CrateNum)
+pub fn get_reachable_ids(cstore: &cstore::CStore, cnum: ast::CrateNum)
     -> Vec<ast::DefId>
 {
     let cdata = cstore.get_crate_data(cnum);
-    decoder::get_reachable_extern_fns(&*cdata)
+    decoder::get_reachable_ids(&*cdata)
 }
 
 pub fn is_typedef(cstore: &cstore::CStore, did: ast::DefId) -> bool {
@@ -400,3 +400,9 @@ pub fn is_default_impl(cstore: &cstore::CStore, impl_did: ast::DefId) -> bool {
     let cdata = cstore.get_crate_data(impl_did.krate);
     decoder::is_default_impl(&*cdata, impl_did.node)
 }
+
+pub fn is_extern_fn(cstore: &cstore::CStore, did: ast::DefId,
+                    tcx: &ty::ctxt) -> bool {
+    let cdata = cstore.get_crate_data(did.krate);
+    decoder::is_extern_fn(&*cdata, did.node, tcx)
+}

src/librustc/metadata/cstore.rs

@@ -20,7 +20,7 @@ pub use self::NativeLibraryKind::*;
 use back::svh::Svh;
 use metadata::{creader, decoder, loader};
 use session::search_paths::PathKind;
-use util::nodemap::{FnvHashMap, NodeMap};
+use util::nodemap::{FnvHashMap, NodeMap, NodeSet};
 
 use std::cell::{RefCell, Ref};
 use std::rc::Rc;
@@ -97,6 +97,7 @@ pub struct CStore {
     used_crate_sources: RefCell<Vec<CrateSource>>,
     used_libraries: RefCell<Vec<(String, NativeLibraryKind)>>,
     used_link_args: RefCell<Vec<String>>,
+    statically_included_foreign_items: RefCell<NodeSet>,
     pub intr: Rc<IdentInterner>,
 }
 
@@ -108,7 +109,8 @@ impl CStore {
             used_crate_sources: RefCell::new(Vec::new()),
             used_libraries: RefCell::new(Vec::new()),
             used_link_args: RefCell::new(Vec::new()),
-            intr: intr
+            intr: intr,
+            statically_included_foreign_items: RefCell::new(NodeSet()),
         }
     }
 
@@ -167,6 +169,7 @@ impl CStore {
         self.used_crate_sources.borrow_mut().clear();
         self.used_libraries.borrow_mut().clear();
         self.used_link_args.borrow_mut().clear();
+        self.statically_included_foreign_items.borrow_mut().clear();
     }
 
     // This method is used when generating the command line to pass through to
@@ -240,6 +243,14 @@ impl CStore {
                                      -> Option<ast::CrateNum> {
         self.extern_mod_crate_map.borrow().get(&emod_id).cloned()
     }
+
+    pub fn add_statically_included_foreign_item(&self, id: ast::NodeId) {
+        self.statically_included_foreign_items.borrow_mut().insert(id);
+    }
+
+    pub fn is_statically_included_foreign_item(&self, id: ast::NodeId) -> bool {
+        self.statically_included_foreign_items.borrow().contains(&id)
+    }
 }
 
 impl crate_metadata {

src/librustc/metadata/decoder.rs

@@ -45,6 +45,7 @@ use std::str;
 use rbml::reader;
 use rbml;
 use serialize::Decodable;
+use syntax::abi;
 use syntax::attr;
 use syntax::parse::token::{IdentInterner, special_idents};
 use syntax::parse::token;
@@ -1418,10 +1419,10 @@ pub fn get_method_arg_names(cdata: Cmd, id: ast::NodeId) -> Vec<String> {
     }
 }
 
-pub fn get_reachable_extern_fns(cdata: Cmd) -> Vec<ast::DefId> {
+pub fn get_reachable_ids(cdata: Cmd) -> Vec<ast::DefId> {
     let items = reader::get_doc(rbml::Doc::new(cdata.data()),
-                                tag_reachable_extern_fns);
-    reader::tagged_docs(items, tag_reachable_extern_fn_id).map(|doc| {
+                                tag_reachable_ids);
+    reader::tagged_docs(items, tag_reachable_id).map(|doc| {
         ast::DefId {
             krate: cdata.cnum,
             node: reader::doc_as_u32(doc),
@@ -1543,3 +1544,21 @@ pub fn get_imported_filemaps(metadata: &[u8]) -> Vec<codemap::FileMap> {
         Decodable::decode(&mut decoder).unwrap()
     }).collect()
 }
+
+pub fn is_extern_fn(cdata: Cmd, id: ast::NodeId, tcx: &ty::ctxt) -> bool {
+    let root_doc = rbml::Doc::new(cdata.data());
+    let items = reader::get_doc(root_doc, tag_items);
+    let item_doc = match maybe_find_item(id, items) {
+        Some(doc) => doc,
+        None => return false,
+    };
+    if let Fn = item_family(item_doc) {
+        let ty::TypeScheme { generics, ty } = get_type(cdata, id, tcx);
+        generics.types.is_empty() && match ty.sty {
+            ty::TyBareFn(_, fn_ty) => fn_ty.abi != abi::Rust,
+            _ => false,
+        }
+    } else {
+        false
+    }
+}