Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dllimports of static data from rlibs #28646

Merged
merged 3 commits into from
Sep 27, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 69 additions & 31 deletions src/librustc_trans/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2570,20 +2570,6 @@ fn internalize_symbols(cx: &SharedCrateContext, reachable: &HashSet<&str>) {
unsafe {
let mut declared = HashSet::new();

let iter_globals = |llmod| {
ValueIter {
cur: llvm::LLVMGetFirstGlobal(llmod),
step: llvm::LLVMGetNextGlobal,
}
};

let iter_functions = |llmod| {
ValueIter {
cur: llvm::LLVMGetFirstFunction(llmod),
step: llvm::LLVMGetNextFunction,
}
};

// Collect all external declarations in all compilation units.
for ccx in cx.iter() {
for val in iter_globals(ccx.llmod()).chain(iter_functions(ccx.llmod())) {
Expand Down Expand Up @@ -2623,28 +2609,74 @@ fn internalize_symbols(cx: &SharedCrateContext, reachable: &HashSet<&str>) {
}
}
}
}

// Create a `__imp_<symbol> = &symbol` global for every public static `symbol`.
// This is required to satisfy `dllimport` references to static data in .rlibs
// when using MSVC linker. We do this only for data, as linker can fix up
// code references on its own.
// See #26591, #27438
fn create_imps(cx: &SharedCrateContext, _reachable: &HashSet<&str>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this _reachable parameter be dropped?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, I was not sure if I should filter them through reachable, like internalize_symbols does. Can a symbol have external linkage while not being reachable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a safe assumption that any static with external linkage at this point was a reachable static. Either it was taken care of during emission or internalize_symbols internalized all the non-reachable ones I believe

unsafe {

struct ValueIter {
cur: ValueRef,
step: unsafe extern "C" fn(ValueRef) -> ValueRef,
for ccx in cx.iter() {
let exported: Vec<_> = iter_globals(ccx.llmod())
.filter(|&val| llvm::LLVMGetLinkage(val) == llvm::ExternalLinkage as c_uint &&
llvm::LLVMIsDeclaration(val) == 0)
.collect();

let i8p_ty = Type::i8p(&ccx);
for val in exported {
let name = CStr::from_ptr(llvm::LLVMGetValueName(val));
let imp_name = String::from("__imp_") +
str::from_utf8(name.to_bytes()).unwrap();
let imp_name = CString::new(imp_name).unwrap();
let imp = llvm::LLVMAddGlobal(ccx.llmod(), i8p_ty.to_ref(),
imp_name.as_ptr() as *const _);
llvm::LLVMSetInitializer(imp, llvm::LLVMConstBitCast(val, i8p_ty.to_ref()));
llvm::SetLinkage(imp, llvm::ExternalLinkage);
}
}
}
}

impl Iterator for ValueIter {
type Item = ValueRef;
struct ValueIter {
cur: ValueRef,
step: unsafe extern "C" fn(ValueRef) -> ValueRef,
}

fn next(&mut self) -> Option<ValueRef> {
let old = self.cur;
if !old.is_null() {
self.cur = unsafe {
let step: unsafe extern "C" fn(ValueRef) -> ValueRef =
mem::transmute_copy(&self.step);
step(old)
};
Some(old)
} else {
None
}
impl Iterator for ValueIter {
type Item = ValueRef;

fn next(&mut self) -> Option<ValueRef> {
let old = self.cur;
if !old.is_null() {
self.cur = unsafe {
let step: unsafe extern "C" fn(ValueRef) -> ValueRef =
mem::transmute_copy(&self.step);
step(old)
};
Some(old)
} else {
None
}
}
}

fn iter_globals(llmod: llvm::ModuleRef) -> ValueIter {
unsafe {
ValueIter {
cur: llvm::LLVMGetFirstGlobal(llmod),
step: llvm::LLVMGetNextGlobal,
}
}
}

fn iter_functions(llmod: llvm::ModuleRef) -> ValueIter {
unsafe {
ValueIter {
cur: llvm::LLVMGetFirstFunction(llmod),
step: llvm::LLVMGetNextFunction,
}
}
}
Expand Down Expand Up @@ -2824,6 +2856,12 @@ pub fn trans_crate(tcx: &ty::ctxt, analysis: ty::CrateAnalysis) -> CrateTranslat
&reachable_symbols.iter().map(|x| &x[..]).collect());
}

if sess.target.target.options.is_like_msvc &&
sess.crate_types.borrow().iter().any(|ct| *ct == config::CrateTypeRlib ||
*ct == config::CrateTypeStaticlib) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can only do this for rlibs, right? If you're producing a staticlib then on one else is going to be referencing this crate's globals via dllimport (not correctly, at least), right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yeah, that's probably right.

create_imps(&shared_ccx, &reachable_symbols.iter().map(|x| &x[..]).collect());
}

let metadata_module = ModuleTranslation {
llcx: shared_ccx.metadata_llcx(),
llmod: shared_ccx.metadata_llmod(),
Expand Down
8 changes: 8 additions & 0 deletions src/test/run-make/msvc-data-only/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Test that on *-pc-windows-msvc we can link to a rlib containing only data.
# See #26591, #27438

-include ../tools.mk

all:
$(RUSTC) foo.rs
$(RUSTC) bar.rs
15 changes: 15 additions & 0 deletions src/test/run-make/msvc-data-only/bar.rs
Original file line number Diff line number Diff line change
@@ -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.

extern crate foo;

fn main() {
println!("The answer is {} !", foo::FOO);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be a run-pass instead of a run-make test? Without any C dependencies it should be fine for inclusion there. You'll need to tag the dependency as // no-prefer-dynamic as well as #![crate_type = "rlib"] to ensure it's not built as a DLL, however.

}
13 changes: 13 additions & 0 deletions src/test/run-make/msvc-data-only/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// 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.

#![crate_type = "rlib"]

pub static FOO: i32 = 42;