MaskRay created this revision.
MaskRay added reviewers: hjl.tools, rnk, tmsriram.
Herald added subscribers: dexonsmith, dang, pengfei, s.egerton, simoncook.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC r218397 "x86-64: Optimize access to globals in PIE with copy reloc" made
-fpie code emit R_X86_64_PC32 to reference external data symbols by default.
Clang adopted -mpie-copy-relocations D19996 <https://reviews.llvm.org/D19996> 
as a flexible alternative.

The name -mpie-copy-relocations is misleading [1] and does not capture the idea
that this option can actually apply to all of -fno-pic,-fpie,-fpie [2], so this
patch introduces -f[no-]direct-access-external-data and makes
-mpie-copy-relocations their aliases for compatibility.

[1]
For

  extern int var;
  int get() { return var; }

if var is defined in another translation unit in the link unit, there is no copy
relocation.

[2]
While -fno-pic code generally uses an absolute relocation to reference a data
symbol, GOT indirection can still be used.

-fpic can avoid GOT indirection as well, the user should be responsible for
defining var in another translation unit, otherwise the linker will error in a
-shared link (copy relocations are not available in shared objects).

Neither the -fno-pic nor the -fpic behavior is implemented in this patch, 
though.

GCC feature request https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112 (the
description is much more complex than the Clang description because GCC/GNU ld
x86-64 have some STV_PROTECTED issues which originated from the default
HAVE_LD_PIE_COPYRELOC thing).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92633

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/dso-local-executable.c
  clang/test/Driver/fdirect-access-external-data.c

Index: clang/test/Driver/fdirect-access-external-data.c
===================================================================
--- /dev/null
+++ clang/test/Driver/fdirect-access-external-data.c
@@ -0,0 +1,18 @@
+/// -fno-pic code defaults to -fdirect-access-external-data.
+// RUN: %clang -### -c -target x86_64 %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -c -target x86_64 %s -fdirect-access-external-data 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -c -target x86_64 %s -fdirect-access-external-data -fno-direct-access-external-data 2>&1 | FileCheck %s --check-prefix=INDIRECT
+
+/// -fpie/-fpic code defaults to -fdirect-access-external-data.
+// RUN: %clang -### -c -target x86_64 %s -fpie 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -c -target x86_64 %s -fpie -fno-direct-access-external-data -fdirect-access-external-data 2>&1 | FileCheck %s --check-prefix=DIRECT
+// RUN: %clang -### -c -target aarch64 %s -fpic 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -c -target aarch64 %s -fpic -fdirect-access-external-data 2>&1 | FileCheck %s --check-prefix=DIRECT
+
+/// -m[no-]pie-copy-relocations are aliases for compatibility.
+// RUN: %clang -### -c -target riscv64 %s -mno-pie-copy-relocations 2>&1 | FileCheck %s --check-prefix=INDIRECT
+// RUN: %clang -### -c -target riscv64 %s -fpic -mpie-copy-relocations 2>&1 | FileCheck %s --check-prefix=DIRECT
+
+// DEFAULT-NOT: direct-access-external-data"
+// DIRECT:      "-fdirect-access-external-data"
+// INDIRECT:    "-fno-direct-access-external-data"
Index: clang/test/CodeGen/dso-local-executable.c
===================================================================
--- clang/test/CodeGen/dso-local-executable.c
+++ clang/test/CodeGen/dso-local-executable.c
@@ -42,7 +42,7 @@
 // PIE-DAG: define dso_local i32* @zed()
 // PIE-DAG: declare void @import_func()
 
-// RUN: %clang_cc1 -triple x86_64 -emit-llvm -pic-level 1 -pic-is-pie -mpie-copy-relocations %s -o - | FileCheck --check-prefix=PIE-DIRECT %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm -pic-level 1 -pic-is-pie -fdirect-access-external-data %s -o - | FileCheck --check-prefix=PIE-DIRECT %s
 // PIE-DIRECT:      @baz = dso_local global i32 42
 // PIE-DIRECT-NEXT: @import_var = external dso_local global i32
 // PIE-DIRECT-NEXT: @weak_bar = extern_weak global i32
@@ -64,7 +64,7 @@
 // NOPLT-DAG: define dso_local i32* @zed()
 // NOPLT-DAG: declare void @import_func()
 
-// RUN: %clang_cc1 -triple x86_64 -emit-llvm -fno-plt -pic-level 1 -pic-is-pie -mpie-copy-relocations %s -o - | FileCheck --check-prefix=PIE-DIRECT-NOPLT %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm -fno-plt -pic-level 1 -pic-is-pie -fdirect-access-external-data %s -o - | FileCheck --check-prefix=PIE-DIRECT-NOPLT %s
 // PIE-DIRECT-NOPLT:      @baz = dso_local global i32 42
 // PIE-DIRECT-NOPLT-NEXT: @import_var = external dso_local global i32
 // PIE-DIRECT-NOPLT-NEXT: @weak_bar = extern_weak global i32
@@ -75,7 +75,7 @@
 // PIE-DIRECT-NOPLT-DAG: define dso_local i32* @zed()
 // PIE-DIRECT-NOPLT-DAG: declare void @import_func()
 
-// RUN: %clang_cc1 -triple x86_64 -emit-llvm -pic-is-pie -fno-plt %s -o - | FileCheck --check-prefix=PIE-NO-PLT %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm -pic-level 1 -pic-is-pie -fno-plt %s -o - | FileCheck --check-prefix=PIE-NO-PLT %s
 // RUN: %clang_cc1 -triple powerpc64le -emit-llvm -mrelocation-model static %s -o - | FileCheck --check-prefix=PIE-NO-PLT %s
 // PIE-NO-PLT:      @baz = dso_local global i32 42
 // PIE-NO-PLT-NEXT: @import_var = external global i32
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1073,8 +1073,10 @@
   Opts.RelaxAll = Args.hasArg(OPT_mrelax_all);
   Opts.IncrementalLinkerCompatible =
       Args.hasArg(OPT_mincremental_linker_compatible);
-  Opts.PIECopyRelocations =
-      Args.hasArg(OPT_mpie_copy_relocations);
+  Opts.DirectAccessExternalData =
+      Args.hasArg(OPT_fdirect_access_external_data) ||
+      (!Args.hasArg(OPT_fno_direct_access_external_data) &&
+       getLastArgIntValue(Args, OPT_pic_level, 0, Diags) == 0);
   Opts.NoPLT = Args.hasArg(OPT_fno_plt);
   Opts.SaveTempLabels = Args.hasArg(OPT_msave_temp_labels);
   Opts.NoDwarfDirectoryAsm = Args.hasArg(OPT_fno_dwarf_directory_asm);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4811,11 +4811,14 @@
     CmdArgs.push_back("-mms-bitfields");
   }
 
-  if (Args.hasFlag(options::OPT_mpie_copy_relocations,
-                   options::OPT_mno_pie_copy_relocations,
-                   false)) {
-    CmdArgs.push_back("-mpie-copy-relocations");
-  }
+  // Non-PIC code defaults to -fdirect-access-external-data while PIC code
+  // defaults to -fno-direct-access-external-data. Pass the option if different
+  // from the default.
+  if (Arg *A = Args.getLastArg(options::OPT_fdirect_access_external_data,
+                               options::OPT_fno_direct_access_external_data))
+    if (A->getOption().matches(options::OPT_fdirect_access_external_data) !=
+        (PICLevel == 0))
+      A->render(Args, CmdArgs);
 
   if (Args.hasFlag(options::OPT_fno_plt, options::OPT_fplt, false)) {
     CmdArgs.push_back("-fno-plt");
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -976,7 +976,7 @@
   // If we can use copy relocations we can assume it is local.
   if (auto *Var = dyn_cast<llvm::GlobalVariable>(GV))
     if (!Var->isThreadLocal() &&
-        (RM == llvm::Reloc::Static || CGOpts.PIECopyRelocations))
+        (RM == llvm::Reloc::Static || CGOpts.DirectAccessExternalData))
       return true;
 
   // If we can use a plt entry as the symbol address we can assume it
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1863,6 +1863,10 @@
 def fno_pic : Flag<["-"], "fno-pic">, Group<f_Group>;
 def fpie : Flag<["-"], "fpie">, Group<f_Group>;
 def fno_pie : Flag<["-"], "fno-pie">, Group<f_Group>;
+def fdirect_access_external_data : Flag<["-"], "fdirect-access-external-data">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Don't use GOT indirection to reference external data symbols">;
+def fno_direct_access_external_data : Flag<["-"], "fno-direct-access-external-data">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Use GOT indirection to reference external data symbols">;
 defm plt : OptOutFFlag<"plt", "",
   "Use GOT indirection instead of PLT to make external function calls (x86 only)">;
 defm ropi : OptInFFlag<"ropi", "Generate read-only position independent code (ARM only)">;
@@ -2719,10 +2723,10 @@
   HelpText<"Use the given offset for addressing the stack-protector guard">;
 def mstack_protector_guard_reg_EQ : Joined<["-"], "mstack-protector-guard-reg=">, Group<m_Group>, Flags<[CC1Option]>,
   HelpText<"Use the given reg for addressing the stack-protector guard">;
-def mpie_copy_relocations : Flag<["-"], "mpie-copy-relocations">, Group<m_Group>,
-  Flags<[CC1Option]>,
-  HelpText<"Use copy relocations support for PIE builds">;
-def mno_pie_copy_relocations : Flag<["-"], "mno-pie-copy-relocations">, Group<m_Group>;
+def mpie_copy_relocations : Flag<["-"], "mpie-copy-relocations">,
+  Alias<fdirect_access_external_data>, Group<m_Group>;
+def mno_pie_copy_relocations : Flag<["-"], "mno-pie-copy-relocations">,
+  Alias<fno_direct_access_external_data>, Group<m_Group>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at function entry (x86/SystemZ only)">,
   Flags<[CC1Option]>, Group<m_Group>;
 def mnop_mcount : Flag<["-"], "mnop-mcount">, HelpText<"Generate mcount/__fentry__ calls as nops. To activate they need to be patched in.">,
Index: clang/include/clang/Basic/CodeGenOptions.def
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -366,8 +366,8 @@
 /// Whether to report the hotness of the code region for optimization remarks.
 CODEGENOPT(DiagnosticsWithHotness, 1, 0)
 
-/// Whether copy relocations support is available when building as PIE.
-CODEGENOPT(PIECopyRelocations, 1, 0)
+/// Whether to use direct access relocations (instead of GOT) to reference external data symbols.
+CODEGENOPT(DirectAccessExternalData, 1, 0)
 
 /// Whether we should use the undefined behaviour optimization for control flow
 /// paths that reach the end of a function without executing a required return.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to