From 54133cfcf49c68886a7cd8046007ea14e3d3944d Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Sun, 6 Feb 2022 13:51:11 -0800 Subject: [PATCH 1/4] Only compile #[used] as llvm.compiler.used for ELF targets --- compiler/rustc_codegen_llvm/src/consts.rs | 36 +++++++++++++++++-- .../rustc_codegen_ssa/src/traits/statics.rs | 4 ++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 4d3f3f318b8..98b412f9397 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -535,11 +535,41 @@ impl<'ll> StaticMethods for CodegenCx<'ll, '_> { // The semantics of #[used] in Rust only require the symbol to make it into the // object file. It is explicitly allowed for the linker to strip the symbol if it - // is dead. As such, use llvm.compiler.used instead of llvm.used. + // is dead, which means we are allowed use `llvm.compiler.used` instead of + // `llvm.used` here. + // // Additionally, https://reviews.llvm.org/D97448 in LLVM 13 started emitting unique // sections with SHF_GNU_RETAIN flag for llvm.used symbols, which may trigger bugs - // in some versions of the gold linker. - self.add_compiler_used_global(g); + // in the handling of `.init_array` (the static constructor list) in versions of + // the gold linker (prior to the one released with binutils 2.36). + // + // However, unconditional use of `llvm.compiler.used` caused a nontrivial amount of + // ecosystem breakage, especially on Mach-O targets. To resolve this, we compile it + // as llvm.used on ELF targets and llvm.compiler.used elsewhere, which and should be + // equivalent to how we compiled `#[used]` before LLVM 13, as `llvm.used` and + // `llvm.compiler.used` were treated the same on ELF targets prior in earlier LLVM + // versions (additionally, it seems to be how Clang handles `__attribute__((used))`, + // perhaps for similar compatibility-motivated reasons). + // + // See https://github.com/rust-lang/rust/issues/47384#issuecomment-1019080146 and + // following comments for some discussion of this. + // + // The final wrinkle is it's not really clear how to tell if we're going to output + // ELF, so it's been approximated as "not like wasm, osx, or windows", which is + // not exactly correct, but is pretty close and hopefully handles all the platforms + // platforms where old versions of `ld.gold` are likely to show up. + // + // All this is subject to change in the future. Which is a good thing, because this + // probably should be firmed up somehow! + let seems_like_elf = !(self.tcx.sess.target.is_like_osx + || self.tcx.sess.target.is_like_windows + || self.tcx.sess.target.is_like_wasm); + + if seems_like_elf { + self.add_compiler_used_global(g); + } else { + self.add_used_global(g); + } } if attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) { // `USED` and `USED_LINKER` can't be used together. diff --git a/compiler/rustc_codegen_ssa/src/traits/statics.rs b/compiler/rustc_codegen_ssa/src/traits/statics.rs index a2a3cb56c78..413d31bb942 100644 --- a/compiler/rustc_codegen_ssa/src/traits/statics.rs +++ b/compiler/rustc_codegen_ssa/src/traits/statics.rs @@ -13,7 +13,9 @@ pub trait StaticMethods: BackendTypes { /// Same as add_used_global(), but only prevent the compiler from potentially removing an /// otherwise unused symbol. The linker is still permitted to drop it. /// - /// This corresponds to the semantics of the `#[used]` attribute. + /// This corresponds to the documented semantics of the `#[used]` attribute, although + /// on some targets (non-ELF), we may use `add_used_global` for `#[used]` statics + /// instead. fn add_compiler_used_global(&self, global: Self::Value); } From 16c2b39e1c5029913daf7b780c2cbea7477e077c Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Sun, 6 Feb 2022 17:16:47 -0800 Subject: [PATCH 2/4] Add a test that `#[used]` makes it through to the linker on macos --- src/test/run-make-fulldeps/used-cdylib-macos/Makefile | 11 +++++++++++ .../run-make-fulldeps/used-cdylib-macos/dylib_used.rs | 4 ++++ 2 files changed, 15 insertions(+) create mode 100644 src/test/run-make-fulldeps/used-cdylib-macos/Makefile create mode 100644 src/test/run-make-fulldeps/used-cdylib-macos/dylib_used.rs diff --git a/src/test/run-make-fulldeps/used-cdylib-macos/Makefile b/src/test/run-make-fulldeps/used-cdylib-macos/Makefile new file mode 100644 index 00000000000..4828d9c8aad --- /dev/null +++ b/src/test/run-make-fulldeps/used-cdylib-macos/Makefile @@ -0,0 +1,11 @@ +-include ../tools.mk + +# only-macos +# +# This checks that `#[used]` passes through to the linker on +# darwin. This is subject to change in the future, see +# https://github.com/rust-lang/rust/pull/93718 for discussion + +all: + $(RUSTC) -Copt-level=3 dylib_used.rs + nm $(TMPDIR)/libdylib_used.dylib | $(CGREP) VERY_IMPORTANT_SYMBOL diff --git a/src/test/run-make-fulldeps/used-cdylib-macos/dylib_used.rs b/src/test/run-make-fulldeps/used-cdylib-macos/dylib_used.rs new file mode 100644 index 00000000000..85f0ff92fe7 --- /dev/null +++ b/src/test/run-make-fulldeps/used-cdylib-macos/dylib_used.rs @@ -0,0 +1,4 @@ +#![crate_type = "cdylib"] + +#[used] +static VERY_IMPORTANT_SYMBOL: u32 = 12345; From da72295411491b2b64270f5b050955859e743fcd Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Thu, 24 Mar 2022 08:42:14 -0700 Subject: [PATCH 3/4] Fix mixup between `llvm.compiler.used` and `llvm.used` in comment --- compiler/rustc_codegen_llvm/src/consts.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 98b412f9397..a1ab590335a 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -545,7 +545,7 @@ impl<'ll> StaticMethods for CodegenCx<'ll, '_> { // // However, unconditional use of `llvm.compiler.used` caused a nontrivial amount of // ecosystem breakage, especially on Mach-O targets. To resolve this, we compile it - // as llvm.used on ELF targets and llvm.compiler.used elsewhere, which and should be + // as llvm.compiler.used on ELF targets and llvm.used elsewhere, which should be // equivalent to how we compiled `#[used]` before LLVM 13, as `llvm.used` and // `llvm.compiler.used` were treated the same on ELF targets prior in earlier LLVM // versions (additionally, it seems to be how Clang handles `__attribute__((used))`, From a64b2a95ff3155fd6dc85312cb4ceac232d28d02 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 11 May 2022 01:55:00 -0700 Subject: [PATCH 4/4] Move `#[used]` check for Mach-O to `rustc_typeck` from `rustc_codegen_llvm` --- compiler/rustc_codegen_llvm/src/consts.rs | 34 +++++------------------ compiler/rustc_typeck/src/collect.rs | 32 ++++++++++++++++++++- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index a1ab590335a..ea5df912438 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -543,33 +543,13 @@ impl<'ll> StaticMethods for CodegenCx<'ll, '_> { // in the handling of `.init_array` (the static constructor list) in versions of // the gold linker (prior to the one released with binutils 2.36). // - // However, unconditional use of `llvm.compiler.used` caused a nontrivial amount of - // ecosystem breakage, especially on Mach-O targets. To resolve this, we compile it - // as llvm.compiler.used on ELF targets and llvm.used elsewhere, which should be - // equivalent to how we compiled `#[used]` before LLVM 13, as `llvm.used` and - // `llvm.compiler.used` were treated the same on ELF targets prior in earlier LLVM - // versions (additionally, it seems to be how Clang handles `__attribute__((used))`, - // perhaps for similar compatibility-motivated reasons). - // - // See https://github.com/rust-lang/rust/issues/47384#issuecomment-1019080146 and - // following comments for some discussion of this. - // - // The final wrinkle is it's not really clear how to tell if we're going to output - // ELF, so it's been approximated as "not like wasm, osx, or windows", which is - // not exactly correct, but is pretty close and hopefully handles all the platforms - // platforms where old versions of `ld.gold` are likely to show up. - // - // All this is subject to change in the future. Which is a good thing, because this - // probably should be firmed up somehow! - let seems_like_elf = !(self.tcx.sess.target.is_like_osx - || self.tcx.sess.target.is_like_windows - || self.tcx.sess.target.is_like_wasm); - - if seems_like_elf { - self.add_compiler_used_global(g); - } else { - self.add_used_global(g); - } + // That said, we only ever emit these when compiling for ELF targets, unless + // `#[used(compiler)]` is explicitly requested. This is to avoid similar breakage + // on other targets, in particular MachO targets have *their* static constructor + // lists broken if `llvm.compiler.used` is emitted rather than llvm.used. However, + // that check happens when assigning the `CodegenFnAttrFlags` in `rustc_typeck`, + // so we don't need to take care of it here. + self.add_compiler_used_global(g); } if attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) { // `USED` and `USED_LINKER` can't be used together. diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 2e0e026631b..e13c8322e33 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2850,7 +2850,37 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { ) .emit(); } - None => codegen_fn_attrs.flags |= CodegenFnAttrFlags::USED, + None => { + // Unfortunately, unconditionally using `llvm.used` causes + // issues in handling `.init_array` with the gold linker, + // but using `llvm.compiler.used` caused a nontrival amount + // of unintentional ecosystem breakage -- particularly on + // Mach-O targets. + // + // As a result, we emit `llvm.compiler.used` only on ELF + // targets. This is somewhat ad-hoc, but actually follows + // our pre-LLVM 13 behavior (prior to the ecosystem + // breakage), and seems to match `clang`'s behavior as well + // (both before and after LLVM 13), possibly because they + // have similar compatibility concerns to us. See + // https://github.com/rust-lang/rust/issues/47384#issuecomment-1019080146 + // and following comments for some discussion of this, as + // well as the comments in `rustc_codegen_llvm` where these + // flags are handled. + // + // Anyway, to be clear: this is still up in the air + // somewhat, and is subject to change in the future (which + // is a good thing, because this would ideally be a bit + // more firmed up). + let is_like_elf = !(tcx.sess.target.is_like_osx + || tcx.sess.target.is_like_windows + || tcx.sess.target.is_like_wasm); + codegen_fn_attrs.flags = if is_like_elf { + CodegenFnAttrFlags::USED + } else { + CodegenFnAttrFlags::USED_LINKER + }; + } } } else if attr.has_name(sym::cmse_nonsecure_entry) { if !matches!(tcx.fn_sig(did).abi(), abi::Abi::C { .. }) {