[ELF] Print file name and section offset in .eh_frame parser

[ruiu]

ELF/EhFrame.cpp

43

Do you need ::?

Ah, so you are defining a new function with the same name. Please avoid overloading because the two functions are semantically different.

154–155

I'd add this function to ReadHelper to eliminate H. from this function. And then probably you want to rename ReadHelper EhReader or something like that.

You can also eliminate ArrayRef<uint_8> & from read* functions if you make the ArrayRef a member of the class.

ELF/EhFrame.h

13

Instead of adding a new #include, declare the class you need.

[evgeny777]

ELF/EhFrame.cpp

154–155

This function is called from outside (OutputSections.cpp), so the caller should construct EhReader then. Is this good?

[ruiu]

ELF/EhFrame.cpp

154–155

You can keep the existing function interface, right?

template <class ELFT> uint8_t elf::getFdeEncoding(EhSectionPiece *Piece) {
  auto *Sec = cast<InputSectionBase<ELFT> *>(Piece->ID);
  return EhReader(Sec, Piece->data()).getFdeEncoding();
}

[evgeny777]

ELF/EhFrame.cpp

154–155

Oh, I see. Now clear.

[evgeny777]

Addressed review comments

[ruiu]

LGTM. This is beautiful!

ELF/EhFrame.h

 //===- EhFrame.h ------------------------------------------------*- C++ -*-===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_ELF_EHFRAME_H
 #define LLD_ELF_EHFRAME_H
 
 #include "lld/Core/LLVM.h"
 
 namespace lld {
 namespace elf {
-template <class ELFT> size_t readEhRecordSize(ArrayRef<uint8_t> Data);
-template <class ELFT> uint8_t getFdeEncoding(ArrayRef<uint8_t> Data);
+template <class ELFT> class InputSectionBase;
+struct EhSectionPiece;
+
+template <class ELFT>
+size_t readEhRecordSize(InputSectionBase<ELFT> *S, size_t Off);
+template <class ELFT> uint8_t getFdeEncoding(EhSectionPiece *P);
 }
 }
 
 #endif

ELF/EhFrame.cpp

 // That means the linker has to understand the format of the section.
 //
 // This file contains a few utility functions to read .eh_frame contents.
 //
 //===----------------------------------------------------------------------===//
 
 #include "EhFrame.h"
 #include "Error.h"
+#include "InputSection.h"
+#include "Relocations.h"
 #include "Strings.h"
 
 #include "llvm/Object/ELF.h"
 #include "llvm/Support/Dwarf.h"
 #include "llvm/Support/Endian.h"
 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::dwarf;
 using namespace llvm::object;
 using namespace llvm::support::endian;
 
-namespace lld {
-namespace elf {
+using namespace lld;
+using namespace lld::elf;
 
+namespace {
+template <class ELFT> class EhReader {
+public:
+  EhReader(InputSectionBase<ELFT> *S, ArrayRef<uint8_t> D) : IS(S), D(D) {}
+  size_t readEhRecordSize();
+  uint8_t getFdeEncoding();
+
+private:
+  template <class P> void failOn(const P *Loc, const Twine &Msg) {
+    fatal(getLocation(*IS, (const uint8_t *)Loc - IS->Data.data()) + ": " +
+          Msg);
+  }
+
+  uint8_t readByte();
+  void skipBytes(size_t Count);
+  StringRef readString();
+  void skipLeb128();
+  void skipAugP();
+
+  InputSectionBase<ELFT> *IS;
+  ArrayRef<uint8_t> D;
+};
+}
+
+template <class ELFT>
+size_t elf::readEhRecordSize(InputSectionBase<ELFT> *S, size_t Off) {
+  return EhReader<ELFT>(S, S->Data.slice(Off)).readEhRecordSize();
+}
 // .eh_frame section is a sequence of records. Each record starts with
 // a 4 byte length field. This function reads the length.
-template <class ELFT> size_t readEhRecordSize(ArrayRef<uint8_t> D) {
+template <class ELFT> size_t EhReader<ELFT>::readEhRecordSize() {
   const endianness E = ELFT::TargetEndianness;
   if (D.size() < 4)
-    fatal("CIE/FDE too small");
+    failOn(D.data(), "CIE/FDE too small");
 
   // First 4 bytes of CIE/FDE is the size of the record.
   // If it is 0xFFFFFFFF, the next 8 bytes contain the size instead,
   // but we do not support that format yet.
   uint64_t V = read32<E>(D.data());
   if (V == UINT32_MAX)
-    fatal("CIE/FDE too large");
+    failOn(D.data(), "CIE/FDE too large");
   uint64_t Size = V + 4;
   if (Size > D.size())
-    fatal("CIE/FDE ends past the end of the section");
+    failOn(D.data(), "CIE/FDE ends past the end of the section");
   return Size;
 }
 
 // Read a byte and advance D by one byte.
-static uint8_t readByte(ArrayRef<uint8_t> &D) {
+template <class ELFT> uint8_t EhReader<ELFT>::readByte() {
   if (D.empty())
-    fatal("corrupted or unsupported CIE information");
+    failOn(D.data(), "unexpected end of CIE");
   uint8_t B = D.front();
   D = D.slice(1);
   return B;
 }
 
+template <class ELFT> void EhReader<ELFT>::skipBytes(size_t Count) {
+  if (D.size() < Count)
+    failOn(D.data(), "CIE is too small");
+  D = D.slice(Count);
+}
+
 // Read a null-terminated string.
-static StringRef readString(ArrayRef<uint8_t> &D) {
+template <class ELFT> StringRef EhReader<ELFT>::readString() {
   const uint8_t *End = std::find(D.begin(), D.end(), '\0');
   if (End == D.end())
-    fatal("corrupted CIE");
+    failOn(D.data(), "corrupted CIE (failed to read string)");
   StringRef S = toStringRef(D.slice(0, End - D.begin()));
   D = D.slice(S.size() + 1);
   return S;
 }
 
 // Skip an integer encoded in the LEB128 format.
 // Actual number is not of interest because only the runtime needs it.
 // But we need to be at least able to skip it so that we can read
 // the field that follows a LEB128 number.
-static void skipLeb128(ArrayRef<uint8_t> &D) {
+template <class ELFT> void EhReader<ELFT>::skipLeb128() {
+  const uint8_t *ErrPos = D.data();
   while (!D.empty()) {
     uint8_t Val = D.front();
     D = D.slice(1);
     if ((Val & 0x80) == 0)
       return;
   }
-  fatal("corrupted or unsupported CIE information");
+  failOn(ErrPos, "corrupted CIE (failed to read LEB128)");
 }
 
 template <class ELFT> static size_t getAugPSize(unsigned Enc) {
   switch (Enc & 0x0f) {
   case DW_EH_PE_absptr:
   case DW_EH_PE_signed:
     return ELFT::Is64Bits ? 8 : 4;
   case DW_EH_PE_udata2:
   case DW_EH_PE_sdata2:
     return 2;
   case DW_EH_PE_udata4:
   case DW_EH_PE_sdata4:
     return 4;
   case DW_EH_PE_udata8:
   case DW_EH_PE_sdata8:
     return 8;
   }
-  fatal("unknown FDE encoding");
+  return 0;
 }
 
-template <class ELFT> static void skipAugP(ArrayRef<uint8_t> &D) {
-  uint8_t Enc = readByte(D);
+template <class ELFT> void EhReader<ELFT>::skipAugP() {
+  uint8_t Enc = readByte();
   if ((Enc & 0xf0) == DW_EH_PE_aligned)
-    fatal("DW_EH_PE_aligned encoding is not supported");
+    failOn(D.data() - 1, "DW_EH_PE_aligned encoding is not supported");
   size_t Size = getAugPSize<ELFT>(Enc);
+  if (Size == 0)
+    failOn(D.data() - 1, "unknown FDE encoding");
   if (Size >= D.size())
-    fatal("corrupted CIE");
+    failOn(D.data() - 1, "corrupted CIE");
   D = D.slice(Size);
 }
 
-template <class ELFT> uint8_t getFdeEncoding(ArrayRef<uint8_t> D) {
-  if (D.size() < 8)
-    fatal("CIE too small");
-  D = D.slice(8);
+template <class ELFT> uint8_t elf::getFdeEncoding(EhSectionPiece *P) {
+  auto *IS = static_cast<InputSectionBase<ELFT> *>(P->ID);
+  return EhReader<ELFT>(IS, P->data()).getFdeEncoding();
+}
 
-  int Version = readByte(D);
+template <class ELFT> uint8_t EhReader<ELFT>::getFdeEncoding() {
+  skipBytes(8);
+  int Version = readByte();
   if (Version != 1 && Version != 3)
-    fatal("FDE version 1 or 3 expected, but got " + Twine(Version));
+    failOn(D.data() - 1,
+           "FDE version 1 or 3 expected, but got " + Twine(Version));
 
-  StringRef Aug = readString(D);
+  StringRef Aug = readString();
 
   // Skip code and data alignment factors.
-  skipLeb128(D);
-  skipLeb128(D);
+  skipLeb128();
+  skipLeb128();
 
   // Skip the return address register. In CIE version 1 this is a single
   // byte. In CIE version 3 this is an unsigned LEB128.
   if (Version == 1)
-    readByte(D);
+    readByte();
   else
-    skipLeb128(D);
+    skipLeb128();
 
   // We only care about an 'R' value, but other records may precede an 'R'
   // record. Unfortunately records are not in TLV (type-length-value) format,
   // so we need to teach the linker how to skip records for each type.
   for (char C : Aug) {
     if (C == 'R')
-      return readByte(D);
+      return readByte();
     if (C == 'z') {
-      skipLeb128(D);
+      skipLeb128();
       continue;
     }
     if (C == 'P') {
-      skipAugP<ELFT>(D);
+      skipAugP();
       continue;
     }
     if (C == 'L') {
-      readByte(D);
+      readByte();
       continue;
     }
-    fatal("unknown .eh_frame augmentation string: " + Aug);
+    failOn(Aug.data(), "unknown .eh_frame augmentation string: " + Aug);
   }
   return DW_EH_PE_absptr;
 }
 
-template size_t readEhRecordSize<ELF32LE>(ArrayRef<uint8_t>);
-template size_t readEhRecordSize<ELF32BE>(ArrayRef<uint8_t>);
-template size_t readEhRecordSize<ELF64LE>(ArrayRef<uint8_t>);
-template size_t readEhRecordSize<ELF64BE>(ArrayRef<uint8_t>);
-
-template uint8_t getFdeEncoding<ELF32LE>(ArrayRef<uint8_t>);
-template uint8_t getFdeEncoding<ELF32BE>(ArrayRef<uint8_t>);
-template uint8_t getFdeEncoding<ELF64LE>(ArrayRef<uint8_t>);
-template uint8_t getFdeEncoding<ELF64BE>(ArrayRef<uint8_t>);
-}
-}
+template size_t elf::readEhRecordSize<ELF32LE>(InputSectionBase<ELF32LE> *S,
+                                               size_t Off);
+template size_t elf::readEhRecordSize<ELF32BE>(InputSectionBase<ELF32BE> *S,
+                                               size_t Off);
+template size_t elf::readEhRecordSize<ELF64LE>(InputSectionBase<ELF64LE> *S,
+                                               size_t Off);
+template size_t elf::readEhRecordSize<ELF64BE>(InputSectionBase<ELF64BE> *S,
+                                               size_t Off);
+
+template uint8_t elf::getFdeEncoding<ELF32LE>(EhSectionPiece *P);
+template uint8_t elf::getFdeEncoding<ELF32BE>(EhSectionPiece *P);
+template uint8_t elf::getFdeEncoding<ELF64LE>(EhSectionPiece *P);
+template uint8_t elf::getFdeEncoding<ELF64BE>(EhSectionPiece *P);

ELF/InputSection.h

private:
 
   mutable llvm::DenseMap<uintX_t, uintX_t> OffsetMap;
   mutable std::once_flag InitOffsetMap;
 
   llvm::DenseSet<uintX_t> LiveOffsets;
 };
 
 struct EhSectionPiece : public SectionPiece {
-  EhSectionPiece(size_t Off, ArrayRef<uint8_t> Data, unsigned FirstRelocation)
-      : SectionPiece(Off, false), Data(Data.data()), Size(Data.size()),
+  EhSectionPiece(size_t Off, InputSectionData *ID, uint32_t Size,
+                 unsigned FirstRelocation)
+      : SectionPiece(Off, false), ID(ID), Size(Size),
         FirstRelocation(FirstRelocation) {}
-  const uint8_t *Data;
+  InputSectionData *ID;
   uint32_t Size;
   uint32_t size() const { return Size; }
 
-  ArrayRef<uint8_t> data() { return {Data, Size}; }
+  ArrayRef<uint8_t> data() { return {ID->Data.data() + this->InputOff, Size}; }
   unsigned FirstRelocation;
 };
 
 // This corresponds to a .eh_frame section of an input file.
 template <class ELFT> class EhInputSection : public InputSectionBase<ELFT> {
 public:
   typedef typename ELFT::Shdr Elf_Shdr;
   typedef typename ELFT::uint uintX_t;

ELF/InputSection.cpp

 }
 
 template <class ELFT>
 template <class RelTy>
 void EhInputSection<ELFT>::split(ArrayRef<RelTy> Rels) {
   ArrayRef<uint8_t> Data = this->Data;
   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),
-                              getReloc(Off, Size, Rels, RelI));
+    size_t Size = readEhRecordSize<ELFT>(this, Off);
+    this->Pieces.emplace_back(Off, this, Size, getReloc(Off, Size, Rels, RelI));
     // The empty record is the end marker.
     if (Size == 4)
       break;
     Off += Size;
   }
 }
 
 static size_t findNull(ArrayRef<uint8_t> A, size_t EntSize) {

ELF/OutputSections.cpp

template <class ELFT> void EhOutputSection<ELFT>::writeTo(uint8_t *Buf) {
   for (EhInputSection<ELFT> *S : Sections)
     S->relocate(Buf, nullptr);
 
   // Construct .eh_frame_hdr. .eh_frame_hdr is a binary search table
   // to get a FDE from an address to which FDE is applied. So here
   // we obtain two addresses and pass them to EhFrameHdr object.
   if (In<ELFT>::EhFrameHdr) {
     for (CieRecord *Cie : Cies) {
-      uint8_t Enc = getFdeEncoding<ELFT>(Cie->Piece->data());
+      uint8_t Enc = getFdeEncoding<ELFT>(Cie->Piece);
       for (SectionPiece *Fde : Cie->FdePieces) {
         uintX_t Pc = getFdePc(Buf, Fde->OutputOff, Enc);
         uintX_t FdeVA = this->Addr + Fde->OutputOff;
         In<ELFT>::EhFrameHdr->addFde(Pc, FdeVA);
       }
     }
   }
 }

test/ELF/invalid-cie-length.s

 // REQUIRES: x86
 
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
 
         .section .eh_frame
         .byte 0
 
-// CHECK: CIE/FDE too small
+// CHECK: {{.*}}:(.eh_frame+0x0): CIE/FDE too small

test/ELF/invalid-cie-length2.s

 // REQUIRES: x86
 
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
 
         .section .eh_frame
         .long 42
 
-// CHECK: CIE/FDE ends past the end of the section
+// CHECK: {{.*}}:(.eh_frame+0x0): CIE/FDE ends past the end of the section

test/ELF/invalid-cie-length3.s

 // REQUIRES: x86
 
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
 
  .section .eh_frame
  .long 0xFFFFFFFC
 
-// CHECK: CIE/FDE ends past the end of the section
+// CHECK: {{.*}}:(.eh_frame+0x0): CIE/FDE ends past the end of the section

test/ELF/invalid-cie-length4.s

 // REQUIRES: x86
 
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
 // RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
 
  .section .eh_frame
  .long 0xFFFFFFFF
  .byte 0
 
-// CHECK: CIE/FDE too large
+// CHECK: {{.*}}:(.eh_frame+0x0): CIE/FDE too large