Improve error handling of getRelocationAddend.
authorRafael Espindola <rafael.espindola@gmail.com>
Fri, 19 Jun 2015 20:58:43 +0000 (20:58 +0000)
committerRafael Espindola <rafael.espindola@gmail.com>
Fri, 19 Jun 2015 20:58:43 +0000 (20:58 +0000)
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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@240176 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Object/ELFObjectFile.h
include/llvm/Object/RelocVisitor.h
lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp
test/Object/Inputs/invalid-bad-rel-type.elf [new file with mode: 0644]
test/Object/invalid.test [new file with mode: 0644]

index d9bcbfa5f3857a1448dae48dbe15bc631c9d312f..7fc56adff1dfa485111e9f0ed89c49540dc7d13d 100644 (file)
@@ -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();
index bc2002d2d31de39951b37ab0d7d238d4d2b909e6..f80ee0a8a7a20699ffcbc1c6416e003bb42c9da9 100644 (file)
@@ -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)
index 5c39c5944e4a1df2bab28feb89b4c5bf8f3c541f..967d7c07bc8adc3bf8e8b799c4705deeab0cab95 100644 (file)
@@ -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
index 3a377a2e162c82d099269bd4edf709a090d07e37..1a2552deed95ed5d0826b140f4eec9f7bd884284 100644 (file)
@@ -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);
 
index 2943dd383efadf2d471987d80bfbc42cca01bfaa..7c09e5d59580493b6975b184f8435a932737143a 100644 (file)
@@ -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/test/Object/Inputs/invalid-bad-rel-type.elf b/test/Object/Inputs/invalid-bad-rel-type.elf
new file mode 100644 (file)
index 0000000..2caebcd
Binary files /dev/null and b/test/Object/Inputs/invalid-bad-rel-type.elf differ
diff --git a/test/Object/invalid.test b/test/Object/invalid.test
new file mode 100644 (file)
index 0000000..73bf62a
--- /dev/null
@@ -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