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

[ruiu]

ELF/EhFrame.cpp

42 ↗

(On Diff #78719)

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.

143 ↗

(On Diff #78719)

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 ↗

(On Diff #78719)

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

[evgeny777]

ELF/EhFrame.cpp

143 ↗

(On Diff #78719)

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

[ruiu]

ELF/EhFrame.cpp

143 ↗

(On Diff #78719)

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

143 ↗

(On Diff #78719)

Oh, I see. Now clear.

[evgeny777]

Addressed review comments

[ruiu]

LGTM. This is beautiful!

lld/trunk/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

lld/trunk/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);

lld/trunk/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;

lld/trunk/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) {

lld/trunk/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);
       }
     }
   }
 }

lld/trunk/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

lld/trunk/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

lld/trunk/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

lld/trunk/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