[CodeGenModule] Drop dso_local on function declarations for ELF -fno-pic -fno-direct-access-external-data

MaskRay · MaskRay · commit e2e82c9983f0 · 2021-01-09T16:31:56.000-08:00
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.
diff --git 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
@@ -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
