Skip to content

Commit

Permalink
Revert "trans: Be a little more picky about dllimport"
Browse files Browse the repository at this point in the history
This reverts commit a0efd3a.
  • Loading branch information
alexcrichton committed Jul 26, 2015
1 parent a5c12f4 commit 316e1b0
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 93 deletions.
38 changes: 1 addition & 37 deletions src/librustc_trans/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ 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
Expand All @@ -239,42 +238,7 @@ pub fn get_extern_const<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, did: ast::DefId,
llvm::set_thread_local(c, true);
}
}

// 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) {
if ccx.use_dll_storage_attrs() {
llvm::SetDLLStorageClass(c, llvm::DLLImportStorageClass);
}
ccx.externs().borrow_mut().insert(name.to_string(), c);
Expand Down
24 changes: 0 additions & 24 deletions src/librustc_trans/trans/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
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;
Expand Down Expand Up @@ -788,29 +787,6 @@ 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
Expand Down
15 changes: 0 additions & 15 deletions src/test/auxiliary/xcrate-static.rs

This file was deleted.

17 changes: 0 additions & 17 deletions src/test/run-pass/xcrate-static.rs

This file was deleted.

0 comments on commit 316e1b0

Please sign in to comment.