[ELF] - keep alive all non-text sections referenced by .eh_frame

[grimar]

Summary

Patch implements one of suggestions from Rafael Ávila de Espíndola,
to fix segfault after section that contains personality being
garbage collected.

Suggestion was just to keep alive all non executable sections
referenced by .eh_frame.

This fixes PR27529.

[compnerd]

ELF/MarkLive.cpp

102

Unused typedef.

[compnerd]

ELF/MarkLive.cpp

190

Nit: you misspelt referred.

194

Is there a reason to not just merge this into the loops above? It seems unnecessary to re-iterate the objects and sections within the objects.

test/ELF/eh-frame-gc.s

21

Might be able to drop the .long 0 as well. Just thought of that, haven't had a chance to try it.

[ruiu]

This patch seems to be pretty effective for large executables with debug info. r266158 (Rafael's patch to change the way how we apply relocations) caused a temporary performance degradation for such executables, but this patch makes it even faster than before.

Time to link clang with debug info (output size is 1070 MB):

before r266158: 15.312 seconds (0%)
r266158:        17.301 seconds (+13.0%)
Head:           16.484 seconds (+7.7%)
w/patch:        13.166 seconds (-14.0%)

[ruiu]

Oh, I'm sorry, I added a comment to an unrelated patch. Ignore the above message.

[compnerd]

This introduces a new crash on x86_64. Ive not had a chance to reduce this to a test case that is easy to work with, but the following seems to demonstrate the issue:

$ cat /tmp/reduced.cc
struct S { static inline long m() { return 0; } };

#if defined(TU_A)
void f(void) { S::m(); }
#elif defined(TU_B)
void g(void) { S::m(); }
#endif

$ clang++ -target x86_64-unknown-linux-gnu -DTU_A -c /tmp/reduced.cc -o /tmp/reduced-0.o
$ clang++ -target x86_64-unknown-linux-gnu -DTU_B -c /tmp/reduced.cc -o /tmp/reduced-1.o
$ lld -flavor gnu --eh-frame-hdr -m elf_x86_64 -shared -o /tmp/reduced.so --gc-sections /tmp/reduced-0.o /tmp/reduced-1.o

[grimar]

In D19656#415910, @compnerd wrote:

This introduces a new crash on x86_64. Ive not had a chance to reduce this to a test case that is easy to work with, but the following seems to demonstrate the issue:

Reproduced, looking at.

ELF/MarkLive.cpp

190

Thanks !

194

I tried both ways when wrote that and finally decided to split eh_frame processing as special case just to emphasize its exceptional nature. Have no real preference here.

[grimar]

In D19656#415910, @compnerd wrote:

This introduces a new crash on x86_64. Ive not had a chance to reduce this to a test case that is easy to work with, but the following seems to demonstrate the issue:

$ cat /tmp/reduced.cc
struct S { static inline long m() { return 0; } };

#if defined(TU_A)
void f(void) { S::m(); }
#elif defined(TU_B)
void g(void) { S::m(); }
#endif

$ clang++ -target x86_64-unknown-linux-gnu -DTU_A -c /tmp/reduced.cc -o /tmp/reduced-0.o
$ clang++ -target x86_64-unknown-linux-gnu -DTU_B -c /tmp/reduced.cc -o /tmp/reduced-1.o
$ lld -flavor gnu --eh-frame-hdr -m elf_x86_64 -shared -o /tmp/reduced.so --gc-sections /tmp/reduced-0.o /tmp/reduced-1.o

Just in case, this can be reduced to:
reduced-0.s:

.section .text,"G",@progbits,foo
.type foo,@function
foo:

reduced-1.s:

.section .text,"G",@progbits,foo
.type foo,@function
foo:
 .cfi_startproc
 .cfi_endproc

llvm-mc -filetype=obj -triple=x86_64-pc-linux reduced-0.s -o reduced0.o
llvm-mc -filetype=obj -triple=x86_64-pc-linux reduced-1.s -o reduced1.o

[grimar]

• Fixed segfault case reported.
• Addressed review comments.
• Slightly updated testcase (NFC).

ELF/MarkLive.cpp

102

Fixed.

194

After revisiting this, merged into one loop to avoid additional iteration.

test/ELF/eh-frame-gc.s

21

Done.

[• rafael]

LGTM with nits.

I now remember that eh can point to LSDA, but lets get this fixed first.

ELF/MarkLive.cpp

28–1

Single use typedef.

31

Please use std::function<void(ResolvedReloc<ELFT>)> to mach the declaration of forEachSuccessor.

32

These now have a single use each, please inline.

test/ELF/eh-frame-gc.s

10

Give the section a more unique name like test_personality_section.

[grimar]

ELF/MarkLive.cpp

28–1

It was single use, I removed typedef (like was in run()).

31

Done.

32

Done.

test/ELF/eh-frame-gc.s

10

Done.

ELF/MarkLive.cpp

 #include "InputSection.h"
 #include "LinkerScript.h"
 #include "OutputSections.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "Target.h"
 #include "Writer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Object/ELF.h"
 #include <functional>
 #include <vector>
 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;
 
 using namespace lld;
 using namespace lld::elf;

 
 template <class ELFT>
 static typename ELFT::uint getAddend(InputSectionBase<ELFT> *Sec,
                                      const typename ELFT::Rela &Rel) {
   return Rel.r_addend;
 }
 
 template <class ELFT, class RelT>
-static ResolvedReloc<ELFT> resolveReloc(InputSection<ELFT> *Sec, RelT &Rel) {
+static ResolvedReloc<ELFT> resolveReloc(InputSectionBase<ELFT> *Sec,
+                                        RelT &Rel) {
   SymbolBody &B = Sec->getFile()->getRelocTargetSym(Rel);
   auto *D = dyn_cast<DefinedRegular<ELFT>>(&B);
   if (!D || !D->Section)
     return {nullptr, 0};
   typename ELFT::uint Offset = D->Value;
   if (D->isSection())
     Offset += getAddend(Sec, Rel);
   return {D->Section->Repl, Offset};
 }
 
-// Calls Fn for each section that Sec refers to via relocations.
-template <class ELFT>
-static void forEachSuccessor(InputSection<ELFT> *Sec,
-                             std::function<void(ResolvedReloc<ELFT>)> Fn) {
+template <class ELFT, class Elf_Shdr, class Func>
+static void run(ELFFile<ELFT> &Obj, InputSectionBase<ELFT> *Sec,
+                Elf_Shdr *RelSec, Func Fn) {
   typedef typename ELFT::Rel Elf_Rel;
   typedef typename ELFT::Rela Elf_Rela;
-  typedef typename ELFT::Shdr Elf_Shdr;
-
-  ELFFile<ELFT> &Obj = Sec->getFile()->getObj();
-  for (const Elf_Shdr *RelSec : Sec->RelocSections) {
-    if (RelSec->sh_type == SHT_RELA) {
-      for (const Elf_Rela &RI : Obj.relas(RelSec))
-        Fn(resolveReloc(Sec, RI));
-    } else {
-      for (const Elf_Rel &RI : Obj.rels(RelSec))
-        Fn(resolveReloc(Sec, RI));
-    }
-  }
+
+// Calls Fn for each section that Sec refers to via relocations.
+template <class ELFT>
+static void forEachSuccessor(InputSection<ELFT> *Sec,
+                             std::function<void(ResolvedReloc<ELFT>)> Fn) {
+  typedef typename ELFT::Shdr Elf_Shdr;
+
+  ELFFile<ELFT> &Obj = Sec->getFile()->getObj();
+  for (const Elf_Shdr *RelSec : Sec->RelocSections)
+    run(Obj, Sec, RelSec, Fn);
+}
+
+template <class ELFT> static void scanEhFrameSection(EHInputSection<ELFT> &EH) {
+  typedef typename ELFT::Shdr Elf_Shdr;
+  if (!EH.RelocSection)
+    return;
+  ELFFile<ELFT> &EObj = EH.getFile()->getObj();
+  run<ELFT>(EObj, &EH, EH.RelocSection, [&](ResolvedReloc<ELFT> R) {
+    if (!R.Sec)
+      return;
+    if (R.Sec->getSectionHdr()->sh_flags & SHF_EXECINSTR)
+      return;
+    R.Sec->Live = true;
+  });
 }
 
 // Sections listed below are special because they are used by the loader
 // just by being in an ELF file. They should not be garbage-collected.
 template <class ELFT> static bool isReserved(InputSectionBase<ELFT> *Sec) {
   switch (Sec->getSectionHdr()->sh_type) {
   case SHT_FINI_ARRAY:
   case SHT_INIT_ARRAY:
template <class ELFT> void elf::markLive(SymbolTable<ELFT> *Symtab) {
   // Preserve special sections and those which are specified in linker
   // script KEEP command.
   for (const std::unique_ptr<ObjectFile<ELFT>> &F : Symtab->getObjectFiles())
     for (InputSectionBase<ELFT> *Sec : F->getSections())
       if (Sec && Sec != &InputSection<ELFT>::Discarded)
         if (isReserved(Sec) || Script<ELFT>::X->shouldKeep(Sec))
           Enqueue({Sec, 0});
 
+  // .eh_frame is always marked as live now, but also it can reference to
+  // sections that contain personality. We preserve all non-text sections
+  // reffered by .eh_frame here.
+  for (const std::unique_ptr<ObjectFile<ELFT>> &F : Symtab->getObjectFiles())
+    for (InputSectionBase<ELFT> *Sec : F->getSections())
+      if (auto *EH = dyn_cast_or_null<EHInputSection<ELFT>>(Sec))
+        scanEhFrameSection<ELFT>(*EH);
+
   // Mark all reachable sections.
   while (!Q.empty())
     forEachSuccessor<ELFT>(Q.pop_back_val(), Enqueue);
 }
 
 template void elf::markLive<ELF32LE>(SymbolTable<ELF32LE> *);
 template void elf::markLive<ELF32BE>(SymbolTable<ELF32BE> *);
 template void elf::markLive<ELF64LE>(SymbolTable<ELF64LE> *);
 template void elf::markLive<ELF64BE>(SymbolTable<ELF64BE> *);

test/ELF/eh-frame-gc.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=i686-pc-linux %s -o %t.o
+# RUN: ld.lld -shared --gc-sections %t.o -o %t
+# RUN: llvm-readobj  -s %t | FileCheck %s
+
+## Check that data section, containing personality is
+## not garbage collected.
+# CHECK: Sections [
+# CHECK: Name: .data
+
+.text
+.globl f
+.type f,@function
+f:
+ .cfi_startproc
+ .cfi_personality 155, DW.ref.__gxx_personality_v0
+ .cfi_endproc
+
+.data
+DW.ref.__gxx_personality_v0:
+ .long 0