From da27cf22e0642b2dbf9ce2de85d0a0ecf9d6263f Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Sun, 24 Jul 2022 06:26:32 -0700 Subject: [PATCH] Revert PR #93718 (use of `#[used(linker)]`) due to breakage on multiple platforms. This reverts commit ceeb5ade201e4181c6d5df2ba96ae5fb2193aadc, reversing changes made to 039a6ad1caa996379f683f2e219ac4f0e34889bf. --- compiler/rustc_codegen_llvm/src/consts.rs | 14 ++------ .../rustc_codegen_ssa/src/traits/statics.rs | 4 +-- compiler/rustc_typeck/src/collect.rs | 32 +------------------ .../used-cdylib-macos/Makefile | 11 ------- .../used-cdylib-macos/dylib_used.rs | 4 --- 5 files changed, 4 insertions(+), 61 deletions(-) delete mode 100644 src/test/run-make-fulldeps/used-cdylib-macos/Makefile delete mode 100644 src/test/run-make-fulldeps/used-cdylib-macos/dylib_used.rs diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 18467e37082d..863f1f09a16d 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -537,20 +537,10 @@ 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, which means we are allowed use `llvm.compiler.used` instead of - // `llvm.used` here. - // + // is dead. As such, use llvm.compiler.used instead of llvm.used. // 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 the handling of `.init_array` (the static constructor list) in versions of - // the gold linker (prior to the one released with binutils 2.36). - // - // 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. + // in some versions of the gold linker. self.add_compiler_used_global(g); } if attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) { diff --git a/compiler/rustc_codegen_ssa/src/traits/statics.rs b/compiler/rustc_codegen_ssa/src/traits/statics.rs index 413d31bb9429..a2a3cb56c780 100644 --- a/compiler/rustc_codegen_ssa/src/traits/statics.rs +++ b/compiler/rustc_codegen_ssa/src/traits/statics.rs @@ -13,9 +13,7 @@ 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 documented semantics of the `#[used]` attribute, although - /// on some targets (non-ELF), we may use `add_used_global` for `#[used]` statics - /// instead. + /// This corresponds to the semantics of the `#[used]` attribute. fn add_compiler_used_global(&self, global: Self::Value); } diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index c562599e2cc5..77724c124f0f 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2818,37 +2818,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { ) .emit(); } - 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 - }; - } + None => codegen_fn_attrs.flags |= CodegenFnAttrFlags::USED, } } else if attr.has_name(sym::cmse_nonsecure_entry) { if !matches!(tcx.fn_sig(did).abi(), abi::Abi::C { .. }) { diff --git a/src/test/run-make-fulldeps/used-cdylib-macos/Makefile b/src/test/run-make-fulldeps/used-cdylib-macos/Makefile deleted file mode 100644 index 4828d9c8aad2..000000000000 --- a/src/test/run-make-fulldeps/used-cdylib-macos/Makefile +++ /dev/null @@ -1,11 +0,0 @@ --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 deleted file mode 100644 index 85f0ff92fe73..000000000000 --- a/src/test/run-make-fulldeps/used-cdylib-macos/dylib_used.rs +++ /dev/null @@ -1,4 +0,0 @@ -#![crate_type = "cdylib"] - -#[used] -static VERY_IMPORTANT_SYMBOL: u32 = 12345;