Fix PR28575

[ruiu]

ELF/InputSection.cpp

459–461

I think you want to remove "That means ..." part because it doesn't describe the behavior of this function.

502

Why don't you store relocation indices to section pieces? It would be better than having parallel arrays.

ELF/MarkLive.cpp

99

Please write a function comment why you needed this function (e.g. why .eh_frame needs special treament).

ELF/InputSection.h

   EhInputSection(ObjectFile<ELFT> *F, const Elf_Shdr *Header);
   static bool classof(const InputSectionBase<ELFT> *S);
   void split();
+  template <class RelTy> void split(ArrayRef<RelTy> Rels);
 
   // Translate an offset in the input section to an offset in the output
   // section.

 
   // Relocation section that refer to this one.
   const Elf_Shdr *RelocSection = nullptr;
+
+  std::vector<unsigned> FirstRelocOfPiece;
 };
 
 // This corresponds to a non SHF_MERGE section of an input file.

ELF/InputSection.cpp

   return S->SectionKind == InputSectionBase<ELFT>::EHFrame;
 }
 
+// Returns the index of the first relocation that points to a region between
+// Begin and Begin+Size.
+template <class IntTy, class RelTy>
+static unsigned getReloc(IntTy Begin, IntTy Size, const ArrayRef<RelTy> &Rels,
+                         unsigned &RelocI) {
+  // Start search from RelocI for fast access. That means we expect that the
+  // relocations are sorted and we are looking up symbols in sequential order.
+  // It is naturally satisfied for .eh_frame.
+  for (unsigned N = Rels.size(); RelocI < N; ++RelocI) {
+    const RelTy &Rel = Rels[RelocI];
+    if (Rel.r_offset < Begin)
+      continue;
+
+    if (Rel.r_offset < Begin + Size)
+      return RelocI;
+    return -1;
+  }
+  return -1;
+}
+
 // .eh_frame is a sequence of CIE or FDE records.
 // This function splits an input section into records and returns them.
 template <class ELFT>
 void EhInputSection<ELFT>::split() {
+  // Early exit if already split.
+  if (!this->Pieces.empty())
+    return;
+
+  if (RelocSection) {
+    ELFFile<ELFT> &Obj = this->File->getObj();
+    if (RelocSection->sh_type == SHT_RELA)
+      split(Obj.relas(RelocSection));
+    else
+      split(Obj.rels(RelocSection));
+    return;
+  }
+  split(makeArrayRef<typename ELFT::Rela>(nullptr, nullptr));
+}
+
+template <class ELFT>
+template <class RelTy>
+void EhInputSection<ELFT>::split(ArrayRef<RelTy> Rels) {
   ArrayRef<uint8_t> Data = this->getSectionData();
+  unsigned RelI = 0;
   for (size_t Off = 0, End = Data.size(); Off != End;) {
     size_t Size = readEhRecordSize<ELFT>(Data.slice(Off));
     this->Pieces.emplace_back(Off, Data.slice(Off, Size));
+    SectionPiece &Piece = this->Pieces.back();
+    FirstRelocOfPiece.push_back(
+        getReloc(Piece.InputOff, Piece.size(), Rels, RelI));
     // The empty record is the end marker.
     if (Size == 4)
       break;

ELF/MarkLive.cpp

 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;
+using namespace llvm::support::endian;
 
 using namespace lld;
 using namespace lld::elf;

     run(Obj, Sec, RelSec, Fn);
 }
 
+template <class ELFT, class RelTy>
+static void scanEhFrameSection(EhInputSection<ELFT> &EH, ArrayRef<RelTy> Rels,
+                               std::function<void(ResolvedReloc<ELFT>)> Fn) {
+  const endianness E = ELFT::TargetEndianness;
+  for (unsigned I = 0, N = EH.Pieces.size(); I < N; ++I) {
+    unsigned FirstRelI = EH.FirstRelocOfPiece[I];
+    if (FirstRelI == (unsigned)-1)
+      continue;
+    SectionPiece &Piece = EH.Pieces[I];
+    if (read32<E>(Piece.data().data() + 4) == 0) {
+      // This is a CIE, we only need to worry about the first relocation. It is
+      // known to point to the personality function.
+      Fn(resolveReloc(EH, Rels[FirstRelI]));
+      continue;
+    }
+    // This is a FDE. The relocations point to the described function or to
+    // a LSDA. We only need to keep the LSDA alive, so ignore anything that
+    // points to executable sections.
+    typename ELFT::uint PieceEnd = Piece.InputOff + Piece.size();
+    for (unsigned I2 = FirstRelI, N2 = Rels.size(); I2 < N2; ++I2) {
+      const RelTy &RI = Rels[I2];
+      if (RI.r_offset >= PieceEnd)
+        break;
+      ResolvedReloc<ELFT> R = resolveReloc(EH, Rels[I2]);
+      if (!R.Sec || R.Sec == &InputSection<ELFT>::Discarded)
+        continue;
+      if (R.Sec->getSectionHdr()->sh_flags & SHF_EXECINSTR)
+        continue;
+      Fn({R.Sec, 0});
+    }
+  }
+}
+
 template <class ELFT>
 static void scanEhFrameSection(EhInputSection<ELFT> &EH,
                                std::function<void(ResolvedReloc<ELFT>)> Fn) {
   if (!EH.RelocSection)
     return;
+
+  // Unfortunately we need to split .eh_frame early since some relocations in
+  // .eh_frame keep other section alive and some don't.
+  EH.split();
+
   ELFFile<ELFT> &EObj = EH.getFile()->getObj();
-  run<ELFT>(EObj, EH, EH.RelocSection, [&](ResolvedReloc<ELFT> R) {
-    if (!R.Sec || R.Sec == &InputSection<ELFT>::Discarded)
-      return;
-    if (R.Sec->getSectionHdr()->sh_flags & SHF_EXECINSTR)
-      return;
-    Fn({R.Sec, 0});
-  });
+  if (EH.RelocSection->sh_type == SHT_RELA)
+    scanEhFrameSection(EH, EObj.relas(EH.RelocSection), Fn);
+  else
+    scanEhFrameSection(EH, EObj.rels(EH.RelocSection), Fn);
 }
 
 // Sections listed below are special because they are used by the loader

ELF/OutputSections.h

   void addSectionAux(EhInputSection<ELFT> *S, llvm::ArrayRef<RelTy> Rels);
 
   template <class RelTy>
-  CieRecord *addCie(SectionPiece &Piece, EhInputSection<ELFT> *Sec,
-                    ArrayRef<RelTy> &Rels);
+  CieRecord *addCie(SectionPiece &Piece, unsigned FirstRelI,
+                    EhInputSection<ELFT> *Sec, ArrayRef<RelTy> Rels);
 
   template <class RelTy>
-  bool isFdeLive(SectionPiece &Piece, EhInputSection<ELFT> *Sec,
-                 ArrayRef<RelTy> &Rels);
+  bool isFdeLive(SectionPiece &Piece, unsigned FirstRelI,
+                 EhInputSection<ELFT> *Sec, ArrayRef<RelTy> Rels);
 
   uintX_t getFdePc(uint8_t *Buf, size_t Off, uint8_t Enc);
 

ELF/OutputSections.cpp

 EhOutputSection<ELFT>::EhOutputSection()
     : OutputSectionBase<ELFT>(".eh_frame", SHT_PROGBITS, SHF_ALLOC) {}
 
-// Returns the first relocation that points to a region
-// between Begin and Begin+Size.
-template <class IntTy, class RelTy>
-static const RelTy *getReloc(IntTy Begin, IntTy Size, ArrayRef<RelTy> &Rels) {
-  for (auto I = Rels.begin(), E = Rels.end(); I != E; ++I) {
-    if (I->r_offset < Begin)
-      continue;
-
-    // Truncate Rels for fast access. That means we expect that the
-    // relocations are sorted and we are looking up symbols in
-    // sequential order. It is naturally satisfied for .eh_frame.
-    Rels = Rels.slice(I - Rels.begin());
-    if (I->r_offset < Begin + Size)
-      return I;
-    return nullptr;
-  }
-  Rels = ArrayRef<RelTy>();
-  return nullptr;
-}
-
 // Search for an existing CIE record or create a new one.
 // CIE records from input object files are uniquified by their contents
 // and where their relocations point to.
 template <class ELFT>
 template <class RelTy>
-CieRecord *EhOutputSection<ELFT>::addCie(SectionPiece &Piece,
-                                         EhInputSection<ELFT> *Sec,
-                                         ArrayRef<RelTy> &Rels) {
+CieRecord *
+EhOutputSection<ELFT>::addCie(SectionPiece &Piece, unsigned FirstRelI,
+                              EhInputSection<ELFT> *Sec, ArrayRef<RelTy> Rels) {
   const endianness E = ELFT::TargetEndianness;
   if (read32<E>(Piece.data().data() + 4) != 0)
     fatal("CIE expected at beginning of .eh_frame: " + Sec->getSectionName());
 
   SymbolBody *Personality = nullptr;
-  if (const RelTy *Rel = getReloc(Piece.InputOff, Piece.size(), Rels))
-    Personality = &Sec->getFile()->getRelocTargetSym(*Rel);
+  if (FirstRelI != (unsigned)-1)
+    Personality = &Sec->getFile()->getRelocTargetSym(Rels[FirstRelI]);
 
   // Search for an existing CIE by CIE contents/relocation target pair.
   CieRecord *Cie = &CieMap[{Piece.data(), Personality}];

 // points to a live function.
 template <class ELFT>
 template <class RelTy>
-bool EhOutputSection<ELFT>::isFdeLive(SectionPiece &Piece,
+bool EhOutputSection<ELFT>::isFdeLive(SectionPiece &Piece, unsigned FirstRelI,
                                       EhInputSection<ELFT> *Sec,
-                                      ArrayRef<RelTy> &Rels) {
-  const RelTy *Rel = getReloc(Piece.InputOff, Piece.size(), Rels);
-  if (!Rel)
+                                      ArrayRef<RelTy> Rels) {
+  if (FirstRelI == (unsigned)-1)
     fatal("FDE doesn't reference another section");
-  SymbolBody &B = Sec->getFile()->getRelocTargetSym(*Rel);
+  const RelTy &Rel = Rels[FirstRelI];
+  SymbolBody &B = Sec->getFile()->getRelocTargetSym(Rel);
   auto *D = dyn_cast<DefinedRegular<ELFT>>(&B);
   if (!D || !D->Section)
     return false;

   const endianness E = ELFT::TargetEndianness;
 
   DenseMap<size_t, CieRecord *> OffsetToCie;
-  for (SectionPiece &Piece : Sec->Pieces) {
+  for (unsigned I = 0, N = Sec->Pieces.size(); I < N; ++I) {
+    SectionPiece &Piece = Sec->Pieces[I];
+    unsigned FirstRelI = Sec->FirstRelocOfPiece[I];
     // The empty record is the end marker.
     if (Piece.size() == 4)
       return;

     size_t Offset = Piece.InputOff;
     uint32_t ID = read32<E>(Piece.data().data() + 4);
     if (ID == 0) {
-      OffsetToCie[Offset] = addCie(Piece, Sec, Rels);
+      OffsetToCie[Offset] = addCie(Piece, FirstRelI, Sec, Rels);
       continue;
     }
 

     if (!Cie)
       fatal("invalid CIE reference");
 
-    if (!isFdeLive(Piece, Sec, Rels))
+    if (!isFdeLive(Piece, FirstRelI, Sec, Rels))
       continue;
     Cie->FdePieces.push_back(&Piece);
     NumFdes++;

test/ELF/eh-frame-gc2.s

+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=i686-pc-linux %s -o %t.o
+// RUN: ld.lld --gc-sections %t.o -o %t
+// RUN: llvm-readobj -s %t | FileCheck %s
+
+// Test that the we don't gc the personality function.
+// CHECK: Name: .foobar
+
+  .globl  _start
+_start:
+  .cfi_startproc
+  .cfi_personality 3, foobar
+  .cfi_endproc
+        .section .foobar,"ax"
+foobar: