[llvm-branch-commits] [clang] e2e82c9 - [CodeGenModule] Drop dso_local on function declarations for ELF -fno-pic -fno-direct-access-external-data

[Fangrui Song via llvm-branch-commits]

Author: Fangrui Song
Date: 2021-01-09T16:31:56-08:00
New Revision: e2e82c9983f0dff671dd47d3e256d80faa40e892

URL: https://github.com/llvm/llvm-project/commit/e2e82c9983f0dff671dd47d3e256d80faa40e892
DIFF: https://github.com/llvm/llvm-project/commit/e2e82c9983f0dff671dd47d3e256d80faa40e892.diff

LOG: [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.

Added: 
    

Modified: 
    clang/lib/CodeGen/CodeGenModule.cpp
    clang/test/CodeGen/dso-local-executable.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 85b5009dce7e..da5b03b138bf 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<llvm::GlobalVariable>(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<llvm::GlobalVariable>(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<llvm::Function>(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<llvm::Function>(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 19930b068218..4c282a334b11 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