From e2e82c9983f0dff671dd47d3e256d80faa40e892 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Sat, 9 Jan 2021 16:31:55 -0800 Subject: [PATCH] [CodeGenModule] Drop dso_local on function declarations for ELF -fno-pic -fno-direct-access-external-data ELF -fno-pic sets dso_local on a function declaration to allow direct accesses when taking its address (similar to a data symbol). The emitted code follows the traditional GCC/Clang -fno-pic behavior: an absolute relocation is produced. If the function is not defined in the executable, a canonical PLT entry will be needed at link time. This is similar to a copy relocation and is incompatible with (-Bsymbolic or --dynamic-list linked shared objects / protected symbols in a shared object). This patch gives -fno-pic code a way to avoid such a canonical PLT entry. The FIXME was about a generalization for -fpie -mpie-copy-relocations (now -fpie -fdirect-access-external-data). While we could set dso_local to avoid GOT when taking the address of a function declaration (there is an ignorable difference about R_386_PC32 vs R_386_PLT32 on i386), it likely does not provide any benefit and can just cause trouble, so we don't make the generalization. --- clang/lib/CodeGen/CodeGenModule.cpp | 27 +++++++++++++++++++-------- clang/test/CodeGen/dso-local-executable.c | 4 ++-- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 85b5009d..da5b03b 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -983,16 +983,27 @@ static bool shouldAssumeDSOLocal(const CodeGenModule &CGM, if (TT.isPPC64()) return false; - // If we can use copy relocations we can assume it is local. - if (auto *Var = dyn_cast(GV)) - if (!Var->isThreadLocal() && CGOpts.DirectAccessExternalData) + if (CGOpts.DirectAccessExternalData) { + // If -fdirect-access-external-data (default for -fno-pic), set dso_local + // for non-thread-local variables. If the symbol is not defined in the + // executable, a copy relocation will be needed at link time. dso_local is + // excluded for thread-local variables because they generally don't support + // copy relocations. + if (auto *Var = dyn_cast(GV)) + if (!Var->isThreadLocal()) + return true; + + // -fno-pic sets dso_local on a function declaration to allow direct + // accesses when taking its address (similar to a data symbol). If the + // function is not defined in the executable, a canonical PLT entry will be + // needed at link time. -fno-direct-access-external-data can avoid the + // canonical PLT entry. We don't generalize this condition to -fpie/-fpic as + // it could just cause trouble without providing perceptible benefits. + if (isa(GV) && !CGOpts.NoPLT && RM == llvm::Reloc::Static) return true; + } - // If we can use a plt entry as the symbol address we can assume it - // is local. - // FIXME: This should work for PIE, but the gold linker doesn't support it. - if (isa(GV) && !CGOpts.NoPLT && RM == llvm::Reloc::Static) - return true; + // If we can use copy relocations we can assume it is local. // Otherwise don't assume it is local. return false; diff --git a/clang/test/CodeGen/dso-local-executable.c b/clang/test/CodeGen/dso-local-executable.c index 19930b0..4c282a3 100644 --- a/clang/test/CodeGen/dso-local-executable.c +++ b/clang/test/CodeGen/dso-local-executable.c @@ -43,9 +43,9 @@ // STATIC-INDIRECT-NEXT: @bar = external global i32 // STATIC-INDIRECT-NEXT: @local_thread_var = dso_local thread_local global i32 42 // STATIC-INDIRECT-NEXT: @thread_var = external thread_local global i32 -// STATIC-INDIRECT-DAG: declare dso_local void @import_func() +// STATIC-INDIRECT-DAG: declare void @import_func() // STATIC-INDIRECT-DAG: define dso_local i32* @zed() -// STATIC-INDIRECT-DAG: declare dso_local void @foo() +// STATIC-INDIRECT-DAG: declare void @foo() // RUN: %clang_cc1 -triple x86_64 -emit-llvm -pic-level 1 -pic-is-pie %s -o - | FileCheck --check-prefix=PIE %s // PIE: @baz = dso_local global i32 42 -- 2.7.4