diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 4aeba2fe06287..258051357b105 100644 --- a/src/librustc_trans/trans/base.rs +++ b/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); diff --git a/src/librustc_trans/trans/context.rs b/src/librustc_trans/trans/context.rs index 5a4bd7ff3a184..82428eebcce97 100644 --- a/src/librustc_trans/trans/context.rs +++ b/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 diff --git a/src/test/auxiliary/xcrate-static.rs b/src/test/auxiliary/xcrate-static.rs new file mode 100644 index 0000000000000..85093869ba21a --- /dev/null +++ b/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 or the MIT license +// , 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; diff --git a/src/test/run-pass/xcrate-static.rs b/src/test/run-pass/xcrate-static.rs new file mode 100644 index 0000000000000..d1f08e726bc1f --- /dev/null +++ b/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 or the MIT license +// , 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); +}