Auto merge of #27304 - alexcrichton:revert-picky-dllimport, r=brson

This reverts commit a0efd3a3d9.

This commit caused a lot of unintended breakage for many Cargo builds. The problem is that Cargo compiles build scripts with `-C prefer-dynamic`, so the standard library is always dynamically linked and hence any imports need to be marked with `dllimport`. Dependencies of build scripts, however, were compiled as rlibs and did not have their imports tagged with `dllimport`, so build scripts would fail to link.

While known that this situation would break, it was unknown that it was a common scenario in the wild. As a result I'm just reverting these heuristics for now.
This commit is contained in:
bors 2015-07-27 18:09:22 +00:00
commit bbda6f9cca
4 changed files with 1 additions and 93 deletions

View file

@ -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
@ -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);

View file

@ -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;
@ -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

View file

@ -1,15 +0,0 @@
// 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;

View file

@ -1,17 +0,0 @@
// 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);
}