[ELF] Do not create copy relocations if dynamic relocations suffice

We used to create copy relocations if we are creating a non-PIC executable and there's a direct reference to imported data. That works for most programs but not for GHC (Glasgow Haskell Compiler). GHC places function machine code and its supplimental data next to each other in the .text section. If we create a copy relocation for such function, the resulting executable won't work because (1) copy-relocated data is not executable and (2) it may separate the function code from its supplemental data. In this patch, I made a change so that mold creates dynamic relocations instead of copy relocations if possible.
rui314 · Dec 2, 2022 · 5866c9e · 5866c9e
1 parent 372f343
commit 5866c9e
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 18 deletions.
diff --git a/elf/input-sections.cc b/elf/input-sections.cc
@@ -128,6 +128,8 @@ template <typename E>
 void InputSection<E>::scan_rel(Context<E> &ctx, Symbol<E> &sym,
                                const ElfRel<E> &rel,
                                const ScanAction table[3][4]) {
+  bool writable = (shdr().sh_flags & SHF_WRITE);
+
   auto error = [&] {
     std::string msg = sym.is_absolute() ? "-fno-PIC" : "-fPIC";
     Error(ctx) << *this << ": " << rel << " relocation at offset 0x"
@@ -136,7 +138,7 @@ void InputSection<E>::scan_rel(Context<E> &ctx, Symbol<E> &sym,
   };
 
   auto check_textrel = [&] {
-    if (this->shdr().sh_flags & SHF_WRITE)
+    if (writable)
       return;
 
     if (ctx.arg.z_text) {
@@ -148,32 +150,53 @@ void InputSection<E>::scan_rel(Context<E> &ctx, Symbol<E> &sym,
     ctx.has_textrel = true;
   };
 
+  auto copyrel = [&] {
+    assert(sym.is_imported);
+    if (sym.esym().st_visibility == STV_PROTECTED) {
+      Error(ctx) << *this
+                 << ": cannot make copy relocation for protected symbol '" << sym
+                 << "', defined in " << *sym.file << "; recompile with -fPIC";
+    }
+    sym.flags |= NEEDS_COPYREL;
+  };
+
+  auto dynrel = [&] {
+    assert(sym.is_imported);
+    check_textrel();
+    this->file.num_dynrel++;
+  };
+
   switch (get_rel_action(ctx, sym, table)) {
   case NONE:
     return;
   case ERROR:
     error();
     return;
   case COPYREL:
-    if (!ctx.arg.z_copyreloc) {
+    if (!ctx.arg.z_copyreloc)
       error();
-    } else if (sym.esym().st_visibility == STV_PROTECTED) {
-      Error(ctx) << *this << ": cannot make copy relocation for protected symbol '"
-                 << sym << "', defined in " << *sym.file
-                 << "; recompile with -fPIC";
-    }
-    sym.flags |= NEEDS_COPYREL;
+    copyrel();
+    return;
+  case DYN_COPYREL:
+    if (writable || !ctx.arg.z_copyreloc)
+      dynrel();
+    else
+      copyrel();
     return;
   case PLT:
     sym.flags |= NEEDS_PLT;
     return;
   case CPLT:
     sym.flags |= NEEDS_CPLT;
     return;
+  case DYN_CPLT:
+    if (writable)
+      dynrel();
+    else
+      sym.flags |= NEEDS_CPLT;
+    return;
   case DYNREL:
-    assert(sym.is_imported);
-    check_textrel();
-    this->file.num_dynrel++;
+    dynrel();
     return;
   case BASEREL:
     check_textrel();
@@ -191,6 +214,14 @@ void InputSection<E>::apply_dyn_absrel(Context<E> &ctx, Symbol<E> &sym,
                                        u64 S, i64 A, u64 P,
                                        ElfRel<E> *&dynrel,
                                        const ScanAction table[3][4]) {
+  bool writable = (shdr().sh_flags & SHF_WRITE);
+
+  auto apply_dynrel = [&] {
+    *dynrel++ = ElfRel<E>(P, E::R_ABS, sym.get_dynsym_idx(ctx), A);
+    if (ctx.arg.apply_dynamic_relocs)
+      *(Word<E> *)loc = A;
+  };
+
   switch (get_rel_action(ctx, sym, table)) {
   case COPYREL:
   case CPLT:
@@ -206,10 +237,20 @@ void InputSection<E>::apply_dyn_absrel(Context<E> &ctx, Symbol<E> &sym,
         *(Word<E> *)loc = S + A;
     }
     break;
+  case DYN_COPYREL:
+    if (writable || !ctx.arg.z_copyreloc)
+      apply_dynrel();
+    else
+      *(Word<E> *)loc = S + A;
+    break;
+  case DYN_CPLT:
+    if (writable)
+      apply_dynrel();
+    else
+      *(Word<E> *)loc = S + A;
+    break;
   case DYNREL:
-    *dynrel++ = ElfRel<E>(P, E::R_ABS, sym.get_dynsym_idx(ctx), A);
-    if (ctx.arg.apply_dynamic_relocs)
-      *(Word<E> *)loc = A;
+    apply_dynrel();
     break;
   default:
     unreachable();

diff --git a/elf/mold.h b/elf/mold.h
@@ -235,7 +235,9 @@ struct InputSectionExtras<E> {
   std::vector<i32> r_deltas;
 };
 
-typedef enum { NONE, ERROR, COPYREL, PLT, CPLT, DYNREL, BASEREL } ScanAction;
+typedef enum {
+  NONE, ERROR, COPYREL, DYN_COPYREL, PLT, CPLT, DYN_CPLT, DYNREL, BASEREL,
+} ScanAction;
 
 // This is a decision table for absolute relocations that is smaller
 // than the word size (e.g. R_X86_64_32). Since the dynamic linker
@@ -254,9 +256,9 @@ constexpr static ScanAction absrel_table[3][4] = {
 // a dynamic relocation if we cannot resolve an address at link-time.
 constexpr static ScanAction dyn_absrel_table[3][4] = {
   // Absolute  Local    Imported data  Imported code
-  {  NONE,     BASEREL, DYNREL,        DYNREL },  // Shared object
-  {  NONE,     BASEREL, DYNREL,        DYNREL },  // Position-independent exec
-  {  NONE,     NONE,    COPYREL,       CPLT   },  // Position-dependent exec
+  {  NONE,     BASEREL, DYNREL,        DYNREL   },  // Shared object
+  {  NONE,     BASEREL, DYNREL,        DYNREL   },  // Position-independent exec
+  {  NONE,     NONE,    DYN_COPYREL,   DYN_CPLT },  // Position-dependent exec
 };
 
 // This is for PC-relative relocations (e.g. R_X86_64_PC32).

diff --git a/test/elf/nocopyreloc.sh b/test/elf/nocopyreloc.sh
@@ -1,6 +1,9 @@
 #!/bin/bash
 . $(dirname $0)/common.inc
 
+[ $MACHINE = i386 ] && skip
+[ $MACHINE = m68k ] && skip
+[ $MACHINE = arm ] && skip
 [ $MACHINE = ppc64 ] && skip
 [ $MACHINE = ppc64le ] && skip