trans: Be a little more picky about dllimport

Currently you can hit a link error on MSVC by only referencing static items from
a crate (no functions for example) and then link to the crate statically (as all
Rust crates do 99% of the time). A detailed investigation can be found [on
github][details], but the tl;dr is that we need to stop applying dllimport so
aggressively.

This commit alters the application of dllimport on constants to only cases where
the crate the constant originated from will be linked as a dylib in some output
crate type. That way if we're just linking rlibs (like the motivation for this
issue) we won't use dllimport. For the compiler, however, (which has lots of
dylibs) we'll use dllimport.

[details]: #26591 (comment)

cc #26591

Author: alexcrichton

src/librustc_trans/trans/base.rs

@@ -228,6 +228,7 @@ pub fn get_extern_const<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, did: ast::DefId,
     // FIXME(nagisa): perhaps the map of externs could be offloaded to llvm somehow?
     // FIXME(nagisa): investigate whether it can be changed into define_global
     let c = declare::declare_global(ccx, &name[..], ty);
+
     // Thread-local statics in some other crate need to *always* be linked
     // against in a thread-local fashion, so we need to be sure to apply the
     // thread-local attribute locally if it was present remotely. If we
@@ -239,7 +240,42 @@ pub fn get_extern_const<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, did: ast::DefId,
             llvm::set_thread_local(c, true);
         }
     }
-    if ccx.use_dll_storage_attrs() {
+
+    // MSVC is a little ornery about how items are imported across dlls, and for
+    // lots more info on dllimport/dllexport see the large comment in
+    // SharedCrateContext::new. Unfortunately, unlike functions, statics
+    // imported from dlls *must* be tagged with dllimport (if you forget
+    // dllimport on a function then the linker fixes it up with an injected
+    // shim). This means that to link correctly to an upstream Rust dynamic
+    // library we need to make sure its statics are tagged with dllimport.
+    //
+    // Hence, if this translation is using dll storage attributes and the crate
+    // that this const originated from is being imported as a dylib at some
+    // point we tag this with dllimport.
+    //
+    // Note that this is not 100% correct for a variety of reasons:
+    //
+    // 1. If we are producing an rlib and linking to an upstream rlib, we'll
+    //    omit the dllimport. It's a possibility, though, that some later
+    //    downstream compilation will link the same upstream dependency as a
+    //    dylib and use our rlib, causing linker errors because we didn't use
+    //    dllimport.
+    // 2. We may have multiple crate output types. For example if we are
+    //    emitting a statically linked binary as well as a dynamic library we'll
+    //    want to omit dllimport for the binary but we need to have it for the
+    //    dylib.
+    //
+    // For most every day uses, however, this should suffice. During the
+    // bootstrap we're almost always linking upstream to a dylib for some crate
+    // type output, so most imports will be tagged with dllimport (somewhat
+    // appropriately). Otherwise rust dylibs linking against rust dylibs is
+    // pretty rare in Rust so this will likely always be `false` and we'll never
+    // tag with dllimport.
+    //
+    // Note that we can't just blindly tag all constants with dllimport as can
+    // cause linkage errors when we're not actually linking against a dll. For
+    // more info on this see rust-lang/rust#26591.
+    if ccx.use_dll_storage_attrs() && ccx.upstream_dylib_used(did.krate) {
         llvm::SetDLLStorageClass(c, llvm::DLLImportStorageClass);
     }
     ccx.externs().borrow_mut().insert(name.to_string(), c);

src/librustc_trans/trans/context.rs

@@ -11,6 +11,7 @@
 use llvm;
 use llvm::{ContextRef, ModuleRef, ValueRef, BuilderRef};
 use metadata::common::LinkMeta;
+use metadata::cstore;
 use middle::def::ExportMap;
 use middle::traits;
 use trans::adt;
@@ -778,6 +779,29 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
     pub fn use_dll_storage_attrs(&self) -> bool {
         self.shared.use_dll_storage_attrs()
     }
+
+    /// Tests whether the given `krate` (an upstream crate) is ever used as a
+    /// dynamic library for the final linkage of this crate.
+    pub fn upstream_dylib_used(&self, krate: ast::CrateNum) -> bool {
+        let tcx = self.tcx();
+        let formats = tcx.dependency_formats.borrow();
+        tcx.sess.crate_types.borrow().iter().any(|ct| {
+            match formats[ct].get(krate as usize - 1) {
+                // If a crate is explicitly linked dynamically then we're
+                // definitely using it dynamically. If it's not being linked
+                // then currently that means it's being included through another
+                // dynamic library, so we're including it dynamically.
+                Some(&Some(cstore::RequireDynamic)) |
+                Some(&None) => true,
+
+                // Static linkage isn't included dynamically and if there's not
+                // an entry in the array then this crate type isn't actually
+                // doing much linkage so there's nothing dynamic going on.
+                Some(&Some(cstore::RequireStatic)) |
+                None => false,
+            }
+        })
+    }
 }
 
 /// Declare any llvm intrinsics that you might need

src/test/auxiliary/xcrate-static.rs

@@ -0,0 +1,15 @@
+// Copyright 2015 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"]
+
+pub static FOO: u8 = 8;

src/test/run-pass/xcrate-static.rs

@@ -0,0 +1,17 @@
+// Copyright 2015 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.
+
+// aux-build:xcrate-static.rs
+
+extern crate xcrate_static;
+
+fn main() {
+    println!("{}", xcrate_static::FOO);
+}