Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upFix dllimports of static data from rlibs #28646
Conversation
rust-highfive
assigned
nikomatsakis
Sep 25, 2015
This comment has been minimized.
This comment has been minimized.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
rust-highfive
assigned
alexcrichton
and unassigned
nikomatsakis
Sep 25, 2015
vadimcn
force-pushed the
vadimcn:imps
branch
from
38e6ad7
to
e82bb91
Sep 25, 2015
alexcrichton
referenced this pull request
Sep 25, 2015
Open
Correctly handle dllimport on Windows/MSVC #27438
alexcrichton
reviewed
Sep 25, 2015
| extern crate foo; | ||
|
|
||
| fn main() { | ||
| println!("The answer is {} !", foo::FOO); |
This comment has been minimized.
This comment has been minimized.
alexcrichton
Sep 25, 2015
Member
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.
alexcrichton
reviewed
Sep 25, 2015
| // 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>) { |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
vadimcn
Sep 25, 2015
Author
Contributor
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?
This comment has been minimized.
This comment has been minimized.
alexcrichton
Sep 25, 2015
Member
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
alexcrichton
reviewed
Sep 25, 2015
| @@ -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) { | |||
This comment has been minimized.
This comment has been minimized.
alexcrichton
Sep 25, 2015
Member
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?
This comment has been minimized.
This comment has been minimized.
vadimcn
added some commits
Sep 26, 2015
This comment has been minimized.
This comment has been minimized.
|
r? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Sep 26, 2015
This comment has been minimized.
This comment has been minimized.
bors
merged commit 38f1c47
into
rust-lang:master
Sep 27, 2015
This comment has been minimized.
This comment has been minimized.
|
Am I right, this PR should fix link errors on msvc platforms that look like #28169? |
This comment has been minimized.
This comment has been minimized.
|
Hm yeah this should fix issues like that, what version of the compiler are you using? I just downloaded the latest nightly and tested it out and it worked for me, so perhaps a stale download link was used? Or maybe the wrong compiler? |
This comment has been minimized.
This comment has been minimized.
|
ah, also nominating for a backport to beta |
alexcrichton
added
the
beta-nominated
label
Sep 29, 2015
This comment has been minimized.
This comment has been minimized.
|
I tried the
|
This comment has been minimized.
This comment has been minimized.
|
Oh dear, that is indeed bad! Looks like the naming conventions are different for 32-bit than they are for 64. Who knew! I'm working on a patch, thanks for letting us know @alexbool! |
vadimcn
referenced this pull request
Sep 30, 2015
Merged
trans: Fix __imp_ creation for i686 MSVC #28741
alexcrichton
referenced this pull request
Oct 1, 2015
Closed
Linker Error on nightly msvc 32bit, maybe related to FnBox #28799
This comment has been minimized.
This comment has been minimized.
|
Accepting for backport to beta |
vadimcn commentedSep 25, 2015
As discussed in the referenced issues, this PR makes rustc emit
__imp_<symbol>stubs for all public static data to ensure smooth linking in on-windows-msvctargets.Resolves #26591, cc #27438