Emit 'dllimport' attribute for dylib foreign items on Windows.

11 files changed, 220 insertions, 46 deletions

diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs
index 484e2f1535e..9275c01c8b1 100644
--- a/src/librustc/middle/cstore.rs
+++ b/src/librustc/middle/cstore.rs
@@ -131,6 +131,7 @@ pub struct NativeLibrary {
     pub kind: NativeLibraryKind,
     pub name: Symbol,
     pub cfg: Option<ast::MetaItem>,
+    pub foreign_items: Vec<DefIndex>,
 }
 
 /// The data we save and restore about an inlined item or method.  This is not
@@ -305,7 +306,8 @@ pub trait CrateStore<'tcx> {
     fn is_defaulted_trait(&self, did: DefId) -> bool;
     fn is_default_impl(&self, impl_did: DefId) -> bool;
     fn is_foreign_item(&self, did: DefId) -> bool;
-    fn is_statically_included_foreign_item(&self, id: ast::NodeId) -> bool;
+    fn is_dllimport_foreign_item(&self, def: DefId) -> bool;
+    fn is_statically_included_foreign_item(&self, def_id: DefId) -> bool;
 
     // crate metadata
     fn dylib_dependency_formats(&self, cnum: CrateNum)
@@ -462,7 +464,8 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
     fn is_defaulted_trait(&self, did: DefId) -> bool { bug!("is_defaulted_trait") }
     fn is_default_impl(&self, impl_did: DefId) -> bool { bug!("is_default_impl") }
     fn is_foreign_item(&self, did: DefId) -> bool { bug!("is_foreign_item") }
-    fn is_statically_included_foreign_item(&self, id: ast::NodeId) -> bool { false }
+    fn is_dllimport_foreign_item(&self, id: DefId) -> bool { false }
+    fn is_statically_included_foreign_item(&self, def_id: DefId) -> bool { false }
 
     // crate metadata
     fn dylib_dependency_formats(&self, cnum: CrateNum)
@@ -526,9 +529,7 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
     // This is basically a 1-based range of ints, which is a little
     // silly - I may fix that.
     fn crates(&self) -> Vec<CrateNum> { vec![] }
-    fn used_libraries(&self) -> Vec<NativeLibrary> {
-        vec![]
-    }
+    fn used_libraries(&self) -> Vec<NativeLibrary> { vec![] }
     fn used_link_args(&self) -> Vec<String> { vec![] }
 
     // utility functions
diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs
index 2c266068fe8..436b1b3159f 100644
--- a/src/librustc_metadata/creader.rs
+++ b/src/librustc_metadata/creader.rs
@@ -52,7 +52,7 @@ pub struct CrateLoader<'a> {
     pub sess: &'a Session,
     cstore: &'a CStore,
     next_crate_num: CrateNum,
-    foreign_item_map: FxHashMap<String, Vec<ast::NodeId>>,
+    foreign_item_map: FxHashMap<String, Vec<DefIndex>>,
     local_crate_name: Symbol,
 }
 
@@ -310,6 +310,7 @@ impl<'a> CrateLoader<'a> {
                 rlib: rlib,
                 rmeta: rmeta,
             },
+            dllimport_foreign_items: RefCell::new(None),
         });
 
         self.cstore.set_crate_data(cnum, cmeta.clone());
@@ -640,18 +641,36 @@ impl<'a> CrateLoader<'a> {
         }
     }
 
-    fn register_statically_included_foreign_items(&mut self) {
+    fn get_foreign_items_of_kind(&self, kind: cstore::NativeLibraryKind) -> Vec<DefIndex> {
+        let mut items = vec![];
         let libs = self.cstore.get_used_libraries();
+        for lib in libs.borrow().iter() {
+            if lib.kind == kind {
+                items.extend(&lib.foreign_items);
+            }
+        }
         for (foreign_lib, list) in self.foreign_item_map.iter() {
-            let is_static = libs.borrow().iter().any(|lib| {
-                lib.name == &**foreign_lib && lib.kind == cstore::NativeStatic
+            let kind_matches = libs.borrow().iter().any(|lib| {
+                lib.name == &**foreign_lib && lib.kind == kind
             });
-            if is_static {
-                for id in list {
-                    self.cstore.add_statically_included_foreign_item(*id);
-                }
+            if kind_matches {
+                items.extend(list)
             }
         }
+        items
+    }
+
+    fn register_statically_included_foreign_items(&mut self) {
+        for id in self.get_foreign_items_of_kind(cstore::NativeStatic) {
+            self.cstore.add_statically_included_foreign_item(id);
+        }
+    }
+
+    fn register_dllimport_foreign_items(&mut self) {
+        let mut dllimports = self.cstore.dllimport_foreign_items.borrow_mut();
+        for id in self.get_foreign_items_of_kind(cstore::NativeUnknown) {
+            dllimports.insert(id);
+        }
     }
 
     fn inject_panic_runtime(&mut self, krate: &ast::Crate) {
@@ -861,7 +880,8 @@ impl<'a> CrateLoader<'a> {
         }
     }
 
-    fn process_foreign_mod(&mut self, i: &ast::Item, fm: &ast::ForeignMod) {
+    fn process_foreign_mod(&mut self, i: &ast::Item, fm: &ast::ForeignMod,
+                           definitions: &Definitions) {
         if fm.abi == Abi::Rust || fm.abi == Abi::RustIntrinsic || fm.abi == Abi::PlatformIntrinsic {
             return;
         }
@@ -912,10 +932,14 @@ impl<'a> CrateLoader<'a> {
             let cfg = cfg.map(|list| {
                 list[0].meta_item().unwrap().clone()
             });
+            let foreign_items = fm.items.iter()
+                .map(|it| definitions.opt_def_index(it.id).unwrap())
+                .collect();
             let lib = NativeLibrary {
                 name: n,
                 kind: kind,
                 cfg: cfg,
+                foreign_items: foreign_items,
             };
             register_native_lib(self.sess, self.cstore, Some(m.span), lib);
         }
@@ -928,7 +952,7 @@ impl<'a> CrateLoader<'a> {
             };
             let list = self.foreign_item_map.entry(lib_name.to_string())
                                                     .or_insert(Vec::new());
-            list.extend(fm.items.iter().map(|it| it.id));
+            list.extend(fm.items.iter().map(|it| definitions.opt_def_index(it.id).unwrap()));
         }
     }
 }
@@ -947,30 +971,34 @@ impl<'a> middle::cstore::CrateLoader for CrateLoader<'a> {
                 name: Symbol::intern(name),
                 kind: kind,
                 cfg: None,
+                foreign_items: Vec::new(),
             };
             register_native_lib(self.sess, self.cstore, None, lib);
         }
         self.register_statically_included_foreign_items();
+        self.register_dllimport_foreign_items();
     }
 
     fn process_item(&mut self, item: &ast::Item, definitions: &Definitions) {
         match item.node {
-            ast::ItemKind::ExternCrate(_) => {}
-            ast::ItemKind::ForeignMod(ref fm) => return self.process_foreign_mod(item, fm),
-            _ => return,
-        }
-
-        let info = self.extract_crate_info(item).unwrap();
-        let (cnum, ..) = self.resolve_crate(
-            &None, info.ident, info.name, None, item.span, PathKind::Crate, info.dep_kind,
-        );
+            ast::ItemKind::ForeignMod(ref fm) => {
+                self.process_foreign_mod(item, fm, definitions)
+            },
+            ast::ItemKind::ExternCrate(_) => {
+                let info = self.extract_crate_info(item).unwrap();
+                let (cnum, ..) = self.resolve_crate(
+                    &None, info.ident, info.name, None, item.span, PathKind::Crate, info.dep_kind,
+                );
 
-        let def_id = definitions.opt_local_def_id(item.id).unwrap();
-        let len = definitions.def_path(def_id.index).data.len();
+                let def_id = definitions.opt_local_def_id(item.id).unwrap();
+                let len = definitions.def_path(def_id.index).data.len();
 
-        let extern_crate =
-            ExternCrate { def_id: def_id, span: item.span, direct: true, path_len: len };
-        self.update_extern_crate(cnum, extern_crate, &mut FxHashSet());
-        self.cstore.add_extern_mod_stmt_cnum(info.id, cnum);
+                let extern_crate =
+                    ExternCrate { def_id: def_id, span: item.span, direct: true, path_len: len };
+                self.update_extern_crate(cnum, extern_crate, &mut FxHashSet());
+                self.cstore.add_extern_mod_stmt_cnum(info.id, cnum);
+            }
+            _ => {}
+        }
     }
 }
diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs
index 73e03a45196..279ef5bfb72 100644
--- a/src/librustc_metadata/cstore.rs
+++ b/src/librustc_metadata/cstore.rs
@@ -15,13 +15,13 @@ use locator;
 use schema;
 
 use rustc::dep_graph::DepGraph;
-use rustc::hir::def_id::{CRATE_DEF_INDEX, CrateNum, DefIndex, DefId};
+use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, CrateNum, DefIndex, DefId};
 use rustc::hir::map::DefKey;
 use rustc::hir::svh::Svh;
 use rustc::middle::cstore::{DepKind, ExternCrate};
 use rustc_back::PanicStrategy;
 use rustc_data_structures::indexed_vec::IndexVec;
-use rustc::util::nodemap::{FxHashMap, NodeMap, NodeSet, DefIdMap};
+use rustc::util::nodemap::{FxHashMap, FxHashSet, NodeMap, DefIdMap};
 
 use std::cell::{RefCell, Cell};
 use std::rc::Rc;
@@ -31,7 +31,7 @@ use syntax::ext::base::SyntaxExtension;
 use syntax::symbol::Symbol;
 use syntax_pos;
 
-pub use rustc::middle::cstore::{NativeLibrary, LinkagePreference};
+pub use rustc::middle::cstore::{NativeLibrary, NativeLibraryKind, LinkagePreference};
 pub use rustc::middle::cstore::{NativeStatic, NativeFramework, NativeUnknown};
 pub use rustc::middle::cstore::{CrateSource, LinkMeta, LibSource};
 
@@ -84,6 +84,8 @@ pub struct CrateMetadata {
     pub source: CrateSource,
 
     pub proc_macros: Option<Vec<(ast::Name, Rc<SyntaxExtension>)>>,
+    // Foreign items imported from a dylib (Windows only)
+    pub dllimport_foreign_items: RefCell<Option<FxHashSet<DefIndex>>>,
 }
 
 pub struct CachedInlinedItem {
@@ -100,7 +102,8 @@ pub struct CStore {
     extern_mod_crate_map: RefCell<NodeMap<CrateNum>>,
     used_libraries: RefCell<Vec<NativeLibrary>>,
     used_link_args: RefCell<Vec<String>>,
-    statically_included_foreign_items: RefCell<NodeSet>,
+    statically_included_foreign_items: RefCell<FxHashSet<DefIndex>>,
+    pub dllimport_foreign_items: RefCell<FxHashSet<DefIndex>>,
     pub inlined_item_cache: RefCell<DefIdMap<Option<CachedInlinedItem>>>,
     pub defid_for_inlined_node: RefCell<NodeMap<DefId>>,
     pub visible_parent_map: RefCell<DefIdMap<DefId>>,
@@ -114,7 +117,8 @@ impl CStore {
             extern_mod_crate_map: RefCell::new(FxHashMap()),
             used_libraries: RefCell::new(Vec::new()),
             used_link_args: RefCell::new(Vec::new()),
-            statically_included_foreign_items: RefCell::new(NodeSet()),
+            statically_included_foreign_items: RefCell::new(FxHashSet()),
+            dllimport_foreign_items: RefCell::new(FxHashSet()),
             visible_parent_map: RefCell::new(FxHashMap()),
             inlined_item_cache: RefCell::new(FxHashMap()),
             defid_for_inlined_node: RefCell::new(FxHashMap()),
@@ -246,12 +250,13 @@ impl CStore {
         self.extern_mod_crate_map.borrow_mut().insert(emod_id, cnum);
     }
 
-    pub fn add_statically_included_foreign_item(&self, id: ast::NodeId) {
+    pub fn add_statically_included_foreign_item(&self, id: DefIndex) {
         self.statically_included_foreign_items.borrow_mut().insert(id);
     }
 
-    pub fn do_is_statically_included_foreign_item(&self, id: ast::NodeId) -> bool {
-        self.statically_included_foreign_items.borrow().contains(&id)
+    pub fn do_is_statically_included_foreign_item(&self, def_id: DefId) -> bool {
+        assert!(def_id.krate == LOCAL_CRATE);
+        self.statically_included_foreign_items.borrow().contains(&def_id.index)
     }
 
     pub fn do_extern_mod_stmt_cnum(&self, emod_id: ast::NodeId) -> Option<CrateNum> {
diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs
index c41c3afb83e..e3193d322bf 100644
--- a/src/librustc_metadata/cstore_impl.rs
+++ b/src/librustc_metadata/cstore_impl.rs
@@ -19,7 +19,7 @@ use rustc::hir::def::{self, Def};
 use rustc::middle::lang_items;
 use rustc::session::Session;
 use rustc::ty::{self, Ty, TyCtxt};
-use rustc::hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX};
+use rustc::hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE};
 
 use rustc::dep_graph::DepNode;
 use rustc::hir::map as hir_map;
@@ -217,9 +217,17 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore {
         self.get_crate_data(did.krate).is_foreign_item(did.index)
     }
 
-    fn is_statically_included_foreign_item(&self, id: ast::NodeId) -> bool
+    fn is_statically_included_foreign_item(&self, def_id: DefId) -> bool
     {
-        self.do_is_statically_included_foreign_item(id)
+        self.do_is_statically_included_foreign_item(def_id)
+    }
+
+    fn is_dllimport_foreign_item(&self, def_id: DefId) -> bool {
+        if def_id.krate == LOCAL_CRATE {
+            self.dllimport_foreign_items.borrow().contains(&def_id.index)
+        } else {
+            self.get_crate_data(def_id.krate).is_dllimport_foreign_item(def_id.index)
+        }
     }
 
     fn dylib_dependency_formats(&self, cnum: CrateNum)
diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs
index 308cd6a83db..f8f80a60c16 100644
--- a/src/librustc_metadata/decoder.rs
+++ b/src/librustc_metadata/decoder.rs
@@ -11,13 +11,13 @@
 // Decoding metadata from a single crate's metadata
 
 use astencode::decode_inlined_item;
-use cstore::{self, CrateMetadata, MetadataBlob, NativeLibrary};
+use cstore::{self, CrateMetadata, MetadataBlob, NativeLibrary, NativeLibraryKind};
 use index::Index;
 use schema::*;
 
 use rustc::hir::map as hir_map;
 use rustc::hir::map::{DefKey, DefPathData};
-use rustc::util::nodemap::FxHashMap;
+use rustc::util::nodemap::{FxHashMap, FxHashSet};
 use rustc::hir;
 use rustc::hir::intravisit::IdRange;
 
@@ -36,6 +36,7 @@ use rustc::mir::Mir;
 use std::borrow::Cow;
 use std::cell::Ref;
 use std::io;
+use std::iter::FromIterator;
 use std::mem;
 use std::str;
 use std::u32;
@@ -1087,6 +1088,18 @@ impl<'a, 'tcx> CrateMetadata {
         }
     }
 
+    pub fn is_dllimport_foreign_item(&self, id: DefIndex) -> bool {
+        if self.dllimport_foreign_items.borrow().is_none() {
+            *self.dllimport_foreign_items.borrow_mut() = Some(FxHashSet::from_iter(
+                self.get_native_libraries().iter()
+                    .filter(|lib| lib.kind == NativeLibraryKind::NativeUnknown)
+                    .flat_map(|lib| &lib.foreign_items)
+                    .map(|id| *id)
+            ));
+        }
+        self.dllimport_foreign_items.borrow().as_ref().unwrap().contains(&id)
+    }
+
     pub fn is_defaulted_trait(&self, trait_id: DefIndex) -> bool {
         match self.entry(trait_id).kind {
             EntryKind::Trait(data) => data.decode(self).has_default_impl,
diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs
index f1126e6fd25..6a9c81dfd5d 100644
--- a/src/librustc_trans/base.rs
+++ b/src/librustc_trans/base.rs
@@ -1498,7 +1498,8 @@ pub fn filter_reachable_ids(tcx: TyCtxt, reachable: NodeSet) -> NodeSet {
         // let it through if it's included statically.
         match tcx.map.get(id) {
             hir_map::NodeForeignItem(..) => {
-                tcx.sess.cstore.is_statically_included_foreign_item(id)
+                let def_id = tcx.map.local_def_id(id);
+                tcx.sess.cstore.is_statically_included_foreign_item(def_id)
             }
 
             // Only consider nodes that actually have exported symbols.
diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs
index df56e27128c..9fd61caf6fe 100644
--- a/src/librustc_trans/callee.rs
+++ b/src/librustc_trans/callee.rs
@@ -629,7 +629,11 @@ fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
                 llvm::LLVMRustSetLinkage(llfn, llvm::Linkage::ExternalLinkage);
             }
         }
-
+        if ccx.use_dll_storage_attrs() && ccx.sess().cstore.is_dllimport_foreign_item(def_id) {
+            unsafe {
+                llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport);
+            }
+        }
         llfn
     };
 
diff --git a/src/librustc_trans/consts.rs b/src/librustc_trans/consts.rs
index 4186721c122..730a4025a59 100644
--- a/src/librustc_trans/consts.rs
+++ b/src/librustc_trans/consts.rs
@@ -191,7 +191,12 @@ pub fn get_static(ccx: &CrateContext, def_id: DefId) -> ValueRef {
                 llvm::set_thread_local(g, true);
             }
         }
-        if ccx.use_dll_storage_attrs() {
+        if ccx.use_dll_storage_attrs() && !ccx.sess().cstore.is_foreign_item(def_id) {
+            // This item is external but not foreign, i.e. it originates from an external Rust
+            // crate. Since we don't know whether this crate will be linked dynamically or
+            // statically in the final application, we always mark such symbols as 'dllimport'.
+            // If final linkage happens to be static, we rely on compiler-emitted __imp_ stubs to
+            // make things work.
             unsafe {
                 llvm::LLVMSetDLLStorageClass(g, llvm::DLLStorageClass::DllImport);
             }
@@ -199,6 +204,12 @@ pub fn get_static(ccx: &CrateContext, def_id: DefId) -> ValueRef {
         g
     };
 
+    if ccx.use_dll_storage_attrs() && ccx.sess().cstore.is_dllimport_foreign_item(def_id) {
+        // For foreign (native) libs we know the exact storage type to use.
+        unsafe {
+            llvm::LLVMSetDLLStorageClass(g, llvm::DLLStorageClass::DllImport);
+        }
+    }
     ccx.instances().borrow_mut().insert(instance, g);
     ccx.statics().borrow_mut().insert(g, def_id);
     g
diff --git a/src/test/codegen/dllimports/auxiliary/dummy.rs b/src/test/codegen/dllimports/auxiliary/dummy.rs
new file mode 100644
index 00000000000..06001c6b014
--- /dev/null
+++ b/src/test/codegen/dllimports/auxiliary/dummy.rs
@@ -0,0 +1,16 @@
+// 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.
+
+// no-prefer-dynamic
+#![crate_type = "staticlib"]
+
+// Since codegen tests don't actually perform linking, this library doesn't need to export
+// any symbols.  It's here just to satisfy the compiler looking for a .lib file when processing
+// #[link(...)] attributes in wrapper.rs.
diff --git a/src/test/codegen/dllimports/auxiliary/wrapper.rs b/src/test/codegen/dllimports/auxiliary/wrapper.rs
new file mode 100644
index 00000000000..c03f88092e5
--- /dev/null
+++ b/src/test/codegen/dllimports/auxiliary/wrapper.rs
@@ -0,0 +1,24 @@
+// 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.
+
+// no-prefer-dynamic
+#![crate_type = "rlib"]
+
+#[link(name = "dummy", kind="dylib")]
+extern "C" {
+    pub fn dylib_func2(x: i32) -> i32;
+    pub static dylib_global2: i32;
+}
+
+#[link(name = "dummy", kind="static")]
+extern "C" {
+    pub fn static_func2(x: i32) -> i32;
+    pub static static_global2: i32;
+}
diff --git a/src/test/codegen/dllimports/main.rs b/src/test/codegen/dllimports/main.rs
new file mode 100644
index 00000000000..140d95fb7be
--- /dev/null
+++ b/src/test/codegen/dllimports/main.rs
@@ -0,0 +1,63 @@
+// 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 is for Windows only.
+// ignore-android
+// ignore-bitrig
+// ignore-macos
+// ignore-dragonfly
+// ignore-freebsd
+// ignore-haiku
+// ignore-ios
+// ignore-linux
+// ignore-netbsd
+// ignore-openbsd
+// ignore-solaris
+// ignore-emscripten
+
+// aux-build:dummy.rs
+// aux-build:wrapper.rs
+
+extern crate wrapper;
+
+// Check that external symbols coming from foreign dylibs are adorned with 'dllimport',
+// whereas symbols coming from foreign staticlibs are not. (RFC-1717)
+
+// CHECK: @dylib_global1 = external dllimport local_unnamed_addr global i32
+// CHECK: @dylib_global2 = external dllimport local_unnamed_addr global i32
+// CHECK: @static_global1 = external local_unnamed_addr global i32
+// CHECK: @static_global2 = external local_unnamed_addr global i32
+
+// CHECK: declare dllimport i32 @dylib_func1(i32)
+// CHECK: declare dllimport i32 @dylib_func2(i32)
+// CHECK: declare i32 @static_func1(i32)
+// CHECK: declare i32 @static_func2(i32)
+
+#[link(name = "dummy", kind="dylib")]
+extern "C" {
+    pub fn dylib_func1(x: i32) -> i32;
+    pub static dylib_global1: i32;
+}
+
+#[link(name = "dummy", kind="static")]
+extern "C" {
+    pub fn static_func1(x: i32) -> i32;
+    pub static static_global1: i32;
+}
+
+fn main() {
+    unsafe {
+        dylib_func1(dylib_global1);
+        wrapper::dylib_func2(wrapper::dylib_global2);
+
+        static_func1(static_global1);
+        wrapper::static_func2(wrapper::static_global2);
+    }
+}