Improve error handling of getRelocationAddend.

This patch changes getRelocationAddend to use ErrorOr and considers it an error
to try to get the addend of a REL section.

If, for example, a x86_64 file has a REL section, that file is corrupted and
we should reject it.

Using ErrorOr is not ideal since we check the section type once per relocation
instead of once per section.

Checking once per section would involve getRelocationAddend just asserting and
callers checking the section before iterating over the relocations.

In any case, this is an improvement and includes a test.

llvm-svn: 240176

7 files changed, 61 insertions, 99 deletions

diff --git a/llvm/include/llvm/Object/ELFObjectFile.h b/llvm/include/llvm/Object/ELFObjectFile.h
index d9bcbfa5f38..7fc56adff1d 100644
--- a/llvm/include/llvm/Object/ELFObjectFile.h
+++ b/llvm/include/llvm/Object/ELFObjectFile.h
@@ -40,8 +40,12 @@ protected:
   ELFObjectFileBase(unsigned int Type, MemoryBufferRef Source);
 
 public:
-  virtual std::error_code getRelocationAddend(DataRefImpl Rel,
-                                              int64_t &Res) const = 0;
+  virtual ErrorOr<int64_t> getRelocationAddend(DataRefImpl Rel) const = 0;
+
+  // FIXME: This is a bit of a hack. Every caller should know if it expecting
+  // and addend or not.
+  virtual bool hasRelocationAddend(DataRefImpl Rel) const = 0;
+
   virtual std::pair<symbol_iterator, symbol_iterator>
   getELFDynamicSymbolIterators() const = 0;
 
@@ -204,8 +208,8 @@ public:
   section_iterator section_begin() const override;
   section_iterator section_end() const override;
 
-  std::error_code getRelocationAddend(DataRefImpl Rel,
-                                      int64_t &Res) const override;
+  ErrorOr<int64_t> getRelocationAddend(DataRefImpl Rel) const override;
+  bool hasRelocationAddend(DataRefImpl Rel) const override;
 
   uint64_t getSectionFlags(SectionRef Sec) const override;
   uint32_t getSectionType(SectionRef Sec) const override;
@@ -652,22 +656,16 @@ std::error_code ELFObjectFile<ELFT>::getRelocationTypeName(
 }
 
 template <class ELFT>
-std::error_code
-ELFObjectFile<ELFT>::getRelocationAddend(DataRefImpl Rel,
-                                         int64_t &Result) const {
-  const Elf_Shdr *sec = getRelSection(Rel);
-  switch (sec->sh_type) {
-  default:
-    report_fatal_error("Invalid section type in Rel!");
-  case ELF::SHT_REL: {
-    Result = 0;
-    return std::error_code();
-  }
-  case ELF::SHT_RELA: {
-    Result = getRela(Rel)->r_addend;
-    return std::error_code();
-  }
-  }
+ErrorOr<int64_t>
+ELFObjectFile<ELFT>::getRelocationAddend(DataRefImpl Rel) const {
+  if (getRelSection(Rel)->sh_type != ELF::SHT_RELA)
+    return object_error::parse_failed;
+  return (int64_t)getRela(Rel)->r_addend;
+}
+
+template <class ELFT>
+bool ELFObjectFile<ELFT>::hasRelocationAddend(DataRefImpl Rel) const {
+  return getRelSection(Rel)->sh_type == ELF::SHT_RELA;
 }
 
 template <class ELFT>
@@ -845,13 +843,6 @@ template <class ELFT> bool ELFObjectFile<ELFT>::isRelocatableObject() const {
   return EF.getHeader()->e_type == ELF::ET_REL;
 }
 
-inline std::error_code getELFRelocationAddend(const RelocationRef R,
-                                              int64_t &Addend) {
-  const ObjectFile *Obj = R.getObjectFile();
-  DataRefImpl DRI = R.getRawDataRefImpl();
-  return cast<ELFObjectFileBase>(Obj)->getRelocationAddend(DRI, Addend);
-}
-
 inline std::pair<symbol_iterator, symbol_iterator>
 getELFDynamicSymbolIterators(const SymbolicFile *Obj) {
   return cast<ELFObjectFileBase>(Obj)->getELFDynamicSymbolIterators();
diff --git a/llvm/include/llvm/Object/RelocVisitor.h b/llvm/include/llvm/Object/RelocVisitor.h
index bc2002d2d31..f80ee0a8a7a 100644
--- a/llvm/include/llvm/Object/RelocVisitor.h
+++ b/llvm/include/llvm/Object/RelocVisitor.h
@@ -239,36 +239,13 @@ private:
     return RelocToApply();
   }
 
-  int64_t getELFAddend32LE(RelocationRef R) {
-    const ELF32LEObjectFile *Obj = cast<ELF32LEObjectFile>(R.getObjectFile());
+  int64_t getELFAddend(RelocationRef R) {
+    const auto *Obj = cast<ELFObjectFileBase>(R.getObjectFile());
     DataRefImpl DRI = R.getRawDataRefImpl();
-    int64_t Addend;
-    Obj->getRelocationAddend(DRI, Addend);
-    return Addend;
-  }
-
-  int64_t getELFAddend64LE(RelocationRef R) {
-    const ELF64LEObjectFile *Obj = cast<ELF64LEObjectFile>(R.getObjectFile());
-    DataRefImpl DRI = R.getRawDataRefImpl();
-    int64_t Addend;
-    Obj->getRelocationAddend(DRI, Addend);
-    return Addend;
-  }
-
-  int64_t getELFAddend32BE(RelocationRef R) {
-    const ELF32BEObjectFile *Obj = cast<ELF32BEObjectFile>(R.getObjectFile());
-    DataRefImpl DRI = R.getRawDataRefImpl();
-    int64_t Addend;
-    Obj->getRelocationAddend(DRI, Addend);
-    return Addend;
-  }
-
-  int64_t getELFAddend64BE(RelocationRef R) {
-    const ELF64BEObjectFile *Obj = cast<ELF64BEObjectFile>(R.getObjectFile());
-    DataRefImpl DRI = R.getRawDataRefImpl();
-    int64_t Addend;
-    Obj->getRelocationAddend(DRI, Addend);
-    return Addend;
+    ErrorOr<int64_t> AddendOrErr = Obj->getRelocationAddend(DRI);
+    if (std::error_code EC = AddendOrErr.getError())
+      report_fatal_error(EC.message());
+    return *AddendOrErr;
   }
 
   uint8_t getLengthMachO64(RelocationRef R) {
@@ -286,15 +263,13 @@ private:
   // Ideally the Addend here will be the addend in the data for
   // the relocation. It's not actually the case for Rel relocations.
   RelocToApply visitELF_386_32(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend32LE(R);
-    return RelocToApply(Value + Addend, 4);
+    return RelocToApply(Value, 4);
   }
 
   RelocToApply visitELF_386_PC32(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend32LE(R);
     uint64_t Address;
     R.getOffset(Address);
-    return RelocToApply(Value + Addend - Address, 4);
+    return RelocToApply(Value - Address, 4);
   }
 
   /// X86-64 ELF
@@ -302,65 +277,59 @@ private:
     return RelocToApply(0, 0);
   }
   RelocToApply visitELF_X86_64_64(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend64LE(R);
+    int64_t Addend = getELFAddend(R);
     return RelocToApply(Value + Addend, 8);
   }
   RelocToApply visitELF_X86_64_PC32(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend64LE(R);
+    int64_t Addend = getELFAddend(R);
     uint64_t Address;
     R.getOffset(Address);
     return RelocToApply(Value + Addend - Address, 4);
   }
   RelocToApply visitELF_X86_64_32(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend64LE(R);
+    int64_t Addend = getELFAddend(R);
     uint32_t Res = (Value + Addend) & 0xFFFFFFFF;
     return RelocToApply(Res, 4);
   }
   RelocToApply visitELF_X86_64_32S(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend64LE(R);
+    int64_t Addend = getELFAddend(R);
     int32_t Res = (Value + Addend) & 0xFFFFFFFF;
     return RelocToApply(Res, 4);
   }
 
   /// PPC64 ELF
   RelocToApply visitELF_PPC64_ADDR32(RelocationRef R, uint64_t Value) {
-    int64_t Addend;
-    getELFRelocationAddend(R, Addend);
+    int64_t Addend = getELFAddend(R);
     uint32_t Res = (Value + Addend) & 0xFFFFFFFF;
     return RelocToApply(Res, 4);
   }
   RelocToApply visitELF_PPC64_ADDR64(RelocationRef R, uint64_t Value) {
-    int64_t Addend;
-    getELFRelocationAddend(R, Addend);
+    int64_t Addend = getELFAddend(R);
     return RelocToApply(Value + Addend, 8);
   }
 
   /// PPC32 ELF
   RelocToApply visitELF_PPC_ADDR32(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend32BE(R);
+    int64_t Addend = getELFAddend(R);
     uint32_t Res = (Value + Addend) & 0xFFFFFFFF;
     return RelocToApply(Res, 4);
   }
 
   /// MIPS ELF
   RelocToApply visitELF_MIPS_32(RelocationRef R, uint64_t Value) {
-    int64_t Addend;
-    getELFRelocationAddend(R, Addend);
-    uint32_t Res = (Value + Addend) & 0xFFFFFFFF;
+    uint32_t Res = (Value)&0xFFFFFFFF;
     return RelocToApply(Res, 4);
   }
 
   RelocToApply visitELF_MIPS_64(RelocationRef R, uint64_t Value) {
-    int64_t Addend;
-    getELFRelocationAddend(R, Addend);
+    int64_t Addend = getELFAddend(R);
     uint64_t Res = (Value + Addend);
     return RelocToApply(Res, 8);
   }
 
   // AArch64 ELF
   RelocToApply visitELF_AARCH64_ABS32(RelocationRef R, uint64_t Value) {
-    int64_t Addend;
-    getELFRelocationAddend(R, Addend);
+    int64_t Addend = getELFAddend(R);
     int64_t Res =  Value + Addend;
 
     // Overflow check allows for both signed and unsigned interpretation.
@@ -371,14 +340,13 @@ private:
   }
 
   RelocToApply visitELF_AARCH64_ABS64(RelocationRef R, uint64_t Value) {
-    int64_t Addend;
-    getELFRelocationAddend(R, Addend);
+    int64_t Addend = getELFAddend(R);
     return RelocToApply(Value + Addend, 8);
   }
 
   // SystemZ ELF
   RelocToApply visitELF_390_32(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend64BE(R);
+    int64_t Addend = getELFAddend(R);
     int64_t Res = Value + Addend;
 
     // Overflow check allows for both signed and unsigned interpretation.
@@ -389,29 +357,27 @@ private:
   }
 
   RelocToApply visitELF_390_64(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend64BE(R);
+    int64_t Addend = getELFAddend(R);
     return RelocToApply(Value + Addend, 8);
   }
 
   RelocToApply visitELF_SPARC_32(RelocationRef R, uint32_t Value) {
-    int32_t Addend = getELFAddend32BE(R);
+    int32_t Addend = getELFAddend(R);
     return RelocToApply(Value + Addend, 4);
   }
 
   RelocToApply visitELF_SPARCV9_32(RelocationRef R, uint64_t Value) {
-    int32_t Addend = getELFAddend64BE(R);
+    int32_t Addend = getELFAddend(R);
     return RelocToApply(Value + Addend, 4);
   }
 
   RelocToApply visitELF_SPARCV9_64(RelocationRef R, uint64_t Value) {
-    int64_t Addend = getELFAddend64BE(R);
+    int64_t Addend = getELFAddend(R);
     return RelocToApply(Value + Addend, 8);
   }
 
   RelocToApply visitELF_ARM_ABS32(RelocationRef R, uint64_t Value) {
-    int64_t Addend;
-    getELFRelocationAddend(R, Addend);
-    int64_t Res = Value + Addend;
+    int64_t Res = Value;
 
     // Overflow check allows for both signed and unsigned interpretation.
     if (Res < INT32_MIN || Res > UINT32_MAX)
diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
index 5c39c5944e4..967d7c07bc8 100644
--- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
+++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
@@ -718,7 +718,7 @@ void RuntimeDyldELF::applyMIPS64Relocation(uint8_t *TargetPtr,
 }
 
 // Return the .TOC. section and offset.
-void RuntimeDyldELF::findPPC64TOCSection(const ObjectFile &Obj,
+void RuntimeDyldELF::findPPC64TOCSection(const ELFObjectFileBase &Obj,
                                          ObjSectionToIDMap &LocalSections,
                                          RelocationValueRef &Rel) {
   // Set a default SectionID in case we do not find a TOC section below.
@@ -751,7 +751,7 @@ void RuntimeDyldELF::findPPC64TOCSection(const ObjectFile &Obj,
 
 // Returns the sections and offset associated with the ODP entry referenced
 // by Symbol.
-void RuntimeDyldELF::findOPDEntrySection(const ObjectFile &Obj,
+void RuntimeDyldELF::findOPDEntrySection(const ELFObjectFileBase &Obj,
                                          ObjSectionToIDMap &LocalSections,
                                          RelocationValueRef &Rel) {
   // Get the ELF symbol value (st_value) to compare with Relocation offset in
@@ -782,8 +782,10 @@ void RuntimeDyldELF::findOPDEntrySection(const ObjectFile &Obj,
       uint64_t TargetSymbolOffset;
       symbol_iterator TargetSymbol = i->getSymbol();
       check(i->getOffset(TargetSymbolOffset));
-      int64_t Addend;
-      check(getELFRelocationAddend(*i, Addend));
+      ErrorOr<int64_t> AddendOrErr =
+          Obj.getRelocationAddend(i->getRawDataRefImpl());
+      Check(AddendOrErr.getError());
+      int64_t Addend = *AddendOrErr;
 
       ++i;
       if (i == e)
@@ -1056,14 +1058,14 @@ void RuntimeDyldELF::processSimpleRelocation(unsigned SectionID, uint64_t Offset
 }
 
 relocation_iterator RuntimeDyldELF::processRelocationRef(
-    unsigned SectionID, relocation_iterator RelI,
-    const ObjectFile &Obj,
-    ObjSectionToIDMap &ObjSectionToID,
-    StubMap &Stubs) {
+    unsigned SectionID, relocation_iterator RelI, const ObjectFile &O,
+    ObjSectionToIDMap &ObjSectionToID, StubMap &Stubs) {
+  const auto &Obj = cast<ELFObjectFileBase>(O);
   uint64_t RelType;
   Check(RelI->getType(RelType));
-  int64_t Addend;
-  Check(getELFRelocationAddend(*RelI, Addend));
+  int64_t Addend = 0;
+  if (Obj.hasRelocationAddend(RelI->getRawDataRefImpl()))
+    Addend = *Obj.getRelocationAddend(RelI->getRawDataRefImpl());
   symbol_iterator Symbol = RelI->getSymbol();
 
   // Obtain the symbol name which is referenced in the relocation
diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
index 3a377a2e162..1a2552deed9 100644
--- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
+++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
@@ -87,10 +87,10 @@ class RuntimeDyldELF : public RuntimeDyldImpl {
 
   void setMipsABI(const ObjectFile &Obj) override;
 
-  void findPPC64TOCSection(const ObjectFile &Obj,
+  void findPPC64TOCSection(const ELFObjectFileBase &Obj,
                            ObjSectionToIDMap &LocalSections,
                            RelocationValueRef &Rel);
-  void findOPDEntrySection(const ObjectFile &Obj,
+  void findOPDEntrySection(const ELFObjectFileBase &Obj,
                            ObjSectionToIDMap &LocalSections,
                            RelocationValueRef &Rel);
 
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp
index 2943dd383ef..7c09e5d5958 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp
@@ -32,7 +32,8 @@ public:
     StringRef SymName; SymI->getName(SymName);
     uint64_t  SymAddr; SymI->getAddress(SymAddr);
     uint64_t SymSize = SymI->getSize();
-    int64_t  Addend;  getELFRelocationAddend(Rel, Addend);
+    auto *Obj = cast<ELFObjectFileBase>(Rel.getObjectFile());
+    int64_t Addend = *Obj->getRelocationAddend(Rel.getRawDataRefImpl());
 
     MCSymbol *Sym = Ctx.getOrCreateSymbol(SymName);
     // FIXME: check that the value is actually the same.
diff --git a/llvm/test/Object/Inputs/invalid-bad-rel-type.elf b/llvm/test/Object/Inputs/invalid-bad-rel-type.elf
new file mode 100644
index 00000000000..2caebcdd1e5
--- /dev/null
+++ b/llvm/test/Object/Inputs/invalid-bad-rel-type.elf
diff --git a/llvm/test/Object/invalid.test b/llvm/test/Object/invalid.test
new file mode 100644
index 00000000000..73bf62a1ed4
--- /dev/null
+++ b/llvm/test/Object/invalid.test
@@ -0,0 +1,2 @@
+RUN: not llvm-dwarfdump %p/Inputs/invalid-bad-rel-type.elf 2>&1 | FileCheck %s
+CHECK: Invalid data was encountered while parsing the file