From db9a84a1af8bd5cea3738833c0e8cd4e1bee3f7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Miku=C5=82a?= Date: Thu, 7 May 2020 11:52:21 +0200 Subject: [PATCH 1/2] MinGW: emit dllexport/dllimport by rustc This fixes various cases where LD could not guess dllexport correctly and greatly improves compatibility with LLD which is not going to support linker scripts anytime soon --- src/librustc_codegen_llvm/callee.rs | 7 ++++++- src/librustc_codegen_llvm/consts.rs | 2 +- src/librustc_codegen_llvm/context.rs | 15 ++++++++++++--- src/librustc_codegen_ssa/back/linker.rs | 23 +++++++++++++++++++++-- src/librustc_codegen_ssa/back/write.rs | 4 ++-- src/librustc_session/session.rs | 10 +++++----- 6 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/librustc_codegen_llvm/callee.rs b/src/librustc_codegen_llvm/callee.rs index 7b341651adf3d..ee6b28b3d1bb1 100644 --- a/src/librustc_codegen_llvm/callee.rs +++ b/src/librustc_codegen_llvm/callee.rs @@ -168,7 +168,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); } diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index 90887b760fb7d..5b9f131115bda 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -281,7 +281,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) ); diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 21ba97d15a485..191a337cd565c 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -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 @@ -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(); diff --git a/src/librustc_codegen_ssa/back/linker.rs b/src/librustc_codegen_ssa/back/linker.rs index e64aafa599fd8..aba4991c29537 100644 --- a/src/librustc_codegen_ssa/back/linker.rs +++ b/src/librustc_codegen_ssa/back/linker.rs @@ -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:"); @@ -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 { @@ -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); diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index 23e0b9344ec91..b0fae566a5aef 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -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 diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index fcd5dab94a6c2..eb6fcb93002fc 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -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", ); } From 87abd656dabdb97758413b7f4be5bda9be71e26e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Miku=C5=82a?= Date: Fri, 3 Jul 2020 21:00:14 +0200 Subject: [PATCH 2/2] Add test for #50176 --- .../mingw-export-call-convention/Makefile | 9 +++++++++ .../mingw-export-call-convention/foo.rs | 4 ++++ src/test/run-make-fulldeps/tools.mk | 1 + 3 files changed, 14 insertions(+) create mode 100644 src/test/run-make-fulldeps/mingw-export-call-convention/Makefile create mode 100644 src/test/run-make-fulldeps/mingw-export-call-convention/foo.rs diff --git a/src/test/run-make-fulldeps/mingw-export-call-convention/Makefile b/src/test/run-make-fulldeps/mingw-export-call-convention/Makefile new file mode 100644 index 0000000000000..4a60059cc5441 --- /dev/null +++ b/src/test/run-make-fulldeps/mingw-export-call-convention/Makefile @@ -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 diff --git a/src/test/run-make-fulldeps/mingw-export-call-convention/foo.rs b/src/test/run-make-fulldeps/mingw-export-call-convention/foo.rs new file mode 100644 index 0000000000000..1fec00311ef06 --- /dev/null +++ b/src/test/run-make-fulldeps/mingw-export-call-convention/foo.rs @@ -0,0 +1,4 @@ +#![crate_type = "cdylib"] + +#[no_mangle] +pub extern "system" fn bar() {} diff --git a/src/test/run-make-fulldeps/tools.mk b/src/test/run-make-fulldeps/tools.mk index 04bf78ed2105b..381f14a0d382a 100644 --- a/src/test/run-make-fulldeps/tools.mk +++ b/src/test/run-make-fulldeps/tools.mk @@ -48,6 +48,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