Skip to content

Commit

Permalink
Auto merge of #72049 - mati865:mingw-lld, r=petrochenkov
Browse files Browse the repository at this point in the history
MinGW: enable dllexport/dllimport

Fixes (only when using LLD) #50176
Fixes #72319

This makes `windows-gnu` on pair with `windows-msvc` when it comes to symbol exporting.
For MinGW it means both good things like correctly working dllimport/dllexport, ability to link with LLD and bad things like #27438.

Not sure but maybe this should land behind unstable compiler option (`-Z`) or environment variable?
  • Loading branch information
bors committed Jul 29, 2020
2 parents 06e7b93 + 87abd65 commit 584e83d
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 14 deletions.
7 changes: 6 additions & 1 deletion src/librustc_codegen_llvm/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,12 @@ pub fn get_fn(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) -> &'ll Value
}
}

if cx.use_dll_storage_attrs && tcx.is_dllimport_foreign_item(instance_def_id) {
// MinGW: For backward compatibility we rely on the linker to decide whether it
// should use dllimport for functions.
if cx.use_dll_storage_attrs
&& tcx.is_dllimport_foreign_item(instance_def_id)
&& tcx.sess.target.target.target_env != "gnu"
{
unsafe {
llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport);
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ impl CodegenCx<'ll, 'tcx> {
// argument validation.
debug_assert!(
!(self.tcx.sess.opts.cg.linker_plugin_lto.enabled()
&& self.tcx.sess.target.target.options.is_like_msvc
&& self.tcx.sess.target.target.options.is_like_windows
&& self.tcx.sess.opts.cg.prefer_dynamic)
);

Expand Down
15 changes: 12 additions & 3 deletions src/librustc_codegen_llvm/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,16 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
// attributes in LLVM IR as well as native dependencies (in C these
// correspond to `__declspec(dllimport)`).
//
// Whenever a dynamic library is built by MSVC it must have its public
// LD (BFD) in MinGW mode can often correctly guess `dllexport` but
// relying on that can result in issues like #50176.
// LLD won't support that and expects symbols with proper attributes.
// Because of that we make MinGW target emit dllexport just like MSVC.
// When it comes to dllimport we use it for constants but for functions
// rely on the linker to do the right thing. Opposed to dllexport this
// task is easy for them (both LD and LLD) and allows us to easily use
// symbols from static libraries in shared libraries.
//
// Whenever a dynamic library is built on Windows it must have its public
// interface specified by functions tagged with `dllexport` or otherwise
// they're not available to be linked against. This poses a few problems
// for the compiler, some of which are somewhat fundamental, but we use
Expand Down Expand Up @@ -254,8 +263,8 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
// this effect) by marking very little as `dllimport` and praying the
// linker will take care of everything. Fixing this problem will likely
// require adding a few attributes to Rust itself (feature gated at the
// start) and then strongly recommending static linkage on MSVC!
let use_dll_storage_attrs = tcx.sess.target.target.options.is_like_msvc;
// start) and then strongly recommending static linkage on Windows!
let use_dll_storage_attrs = tcx.sess.target.target.options.is_like_windows;

let check_overflow = tcx.sess.overflow_checks();

Expand Down
23 changes: 21 additions & 2 deletions src/librustc_codegen_ssa/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,9 @@ impl<'a> Linker for GccLinker<'a> {
return;
}

let is_windows = self.sess.target.target.options.is_like_windows;
let mut arg = OsString::new();
let path = tmpdir.join("list");
let path = tmpdir.join(if is_windows { "list.def" } else { "list" });

debug!("EXPORTED SYMBOLS:");

Expand All @@ -540,6 +541,21 @@ impl<'a> Linker for GccLinker<'a> {
if let Err(e) = res {
self.sess.fatal(&format!("failed to write lib.def file: {}", e));
}
} else if is_windows {
let res: io::Result<()> = try {
let mut f = BufWriter::new(File::create(&path)?);

// .def file similar to MSVC one but without LIBRARY section
// because LD doesn't like when it's empty
writeln!(f, "EXPORTS")?;
for symbol in self.info.exports[&crate_type].iter() {
debug!(" _{}", symbol);
writeln!(f, " {}", symbol)?;
}
};
if let Err(e) = res {
self.sess.fatal(&format!("failed to write list.def file: {}", e));
}
} else {
// Write an LD version script
let res: io::Result<()> = try {
Expand Down Expand Up @@ -573,7 +589,10 @@ impl<'a> Linker for GccLinker<'a> {
if !self.is_ld {
arg.push("-Wl,")
}
arg.push("--version-script=");
// Both LD and LLD accept export list in *.def file form, there are no flags required
if !is_windows {
arg.push("--version-script=")
}
}

arg.push(&path);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_codegen_ssa/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1846,11 +1846,11 @@ fn msvc_imps_needed(tcx: TyCtxt<'_>) -> bool {
// something is wrong with commandline arg validation.
assert!(
!(tcx.sess.opts.cg.linker_plugin_lto.enabled()
&& tcx.sess.target.target.options.is_like_msvc
&& tcx.sess.target.target.options.is_like_windows
&& tcx.sess.opts.cg.prefer_dynamic)
);

tcx.sess.target.target.options.is_like_msvc &&
tcx.sess.target.target.options.is_like_windows &&
tcx.sess.crate_types().iter().any(|ct| *ct == CrateType::Rlib) &&
// ThinLTO can't handle this workaround in all cases, so we don't
// emit the `__imp_` symbols. Instead we make them unnecessary by disallowing
Expand Down
10 changes: 5 additions & 5 deletions src/librustc_session/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1294,19 +1294,19 @@ pub fn build_session(
// commandline argument, you can do so here.
fn validate_commandline_args_with_session_available(sess: &Session) {
// Since we don't know if code in an rlib will be linked to statically or
// dynamically downstream, rustc generates `__imp_` symbols that help the
// MSVC linker deal with this lack of knowledge (#27438). Unfortunately,
// dynamically downstream, rustc generates `__imp_` symbols that help linkers
// on Windows deal with this lack of knowledge (#27438). Unfortunately,
// these manually generated symbols confuse LLD when it tries to merge
// bitcode during ThinLTO. Therefore we disallow dynamic linking on MSVC
// bitcode during ThinLTO. Therefore we disallow dynamic linking on Windows
// when compiling for LLD ThinLTO. This way we can validly just not generate
// the `dllimport` attributes and `__imp_` symbols in that case.
if sess.opts.cg.linker_plugin_lto.enabled()
&& sess.opts.cg.prefer_dynamic
&& sess.target.target.options.is_like_msvc
&& sess.target.target.options.is_like_windows
{
sess.err(
"Linker plugin based LTO is not supported together with \
`-C prefer-dynamic` when targeting MSVC",
`-C prefer-dynamic` when targeting Windows-like targets",
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
include ../tools.mk

# only-windows-gnu

all:
$(RUSTC) foo.rs
# FIXME: we should make sure __stdcall calling convention is used here
# but that only works with LLD right now
nm -g "$(call IMPLIB,foo)" | $(CGREP) bar
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#![crate_type = "cdylib"]

#[no_mangle]
pub extern "system" fn bar() {}
1 change: 1 addition & 0 deletions src/test/run-make-fulldeps/tools.mk
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ ifdef IS_MSVC
STATICLIB = $(TMPDIR)/$(1).lib
STATICLIB_GLOB = $(1)*.lib
else
IMPLIB = $(TMPDIR)/lib$(1).dll.a
STATICLIB = $(TMPDIR)/lib$(1).a
STATICLIB_GLOB = lib$(1)*.a
endif
Expand Down

0 comments on commit 584e83d

Please sign in to comment.