r240176 caused a regression in reading MIPS elf files.

Aboud, Amjad amjad.aboud at intel.com
Wed Jul 1 12:02:11 PDT 2015


Hi Rafael,

I just noticed that your commit in revision 240176 caused a regression in reading elf file for MIPS architecture.
It seems a copy paste issue, please see attach patch with the proposed fix.

If you agree, please can you take care of the issue?

Thanks,
Amjad

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Rafael Espindola
Sent: Friday, June 19, 2015 23:59
To: llvm-commits at cs.uiuc.edu
Subject: [llvm] r240176 - Improve error handling of getRelocationAddend.

Author: rafael
Date: Fri Jun 19 15:58:43 2015
New Revision: 240176

URL: http://llvm.org/viewvc/llvm-project?rev=240176&view=rev
Log:
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.

Added:
    llvm/trunk/test/Object/Inputs/invalid-bad-rel-type.elf
    llvm/trunk/test/Object/invalid.test
Modified:
    llvm/trunk/include/llvm/Object/ELFObjectFile.h
    llvm/trunk/include/llvm/Object/RelocVisitor.h
    llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
    llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
    llvm/trunk/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp

Modified: llvm/trunk/include/llvm/Object/ELFObjectFile.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/ELFObjectFile.h?rev=240176&r1=240175&r2=240176&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/ELFObjectFile.h (original)
+++ llvm/trunk/include/llvm/Object/ELFObjectFile.h Fri Jun 19 15:58:43 
+++ 2015
@@ -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>::get  }
 
 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
   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();

Modified: llvm/trunk/include/llvm/Object/RelocVisitor.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/RelocVisitor.h?rev=240176&r1=240175&r2=240176&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/RelocVisitor.h (original)
+++ llvm/trunk/include/llvm/Object/RelocVisitor.h Fri Jun 19 15:58:43 
+++ 2015
@@ -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)

Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp?rev=240176&r1=240175&r2=240176&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp Fri 
+++ Jun 19 15:58:43 2015
@@ -718,7 +718,7 @@ void RuntimeDyldELF::applyMIPS64Relocati
 }
 
 // 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
 
 // 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
       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::processSimpleReloca
 }
 
 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

Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h?rev=240176&r1=240175&r2=240176&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h Fri Jun 
+++ 19 15:58:43 2015
@@ -87,10 +87,10 @@ class RuntimeDyldELF : public RuntimeDyl
 
   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);
 

Modified: llvm/trunk/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp?rev=240176&r1=240175&r2=240176&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp (original)
+++ llvm/trunk/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp Fri 
+++ Jun 19 15:58:43 2015
@@ -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.

Added: llvm/trunk/test/Object/Inputs/invalid-bad-rel-type.elf
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/Inputs/invalid-bad-rel-type.elf?rev=240176&view=auto
==============================================================================
Binary files llvm/trunk/test/Object/Inputs/invalid-bad-rel-type.elf (added) and llvm/trunk/test/Object/Inputs/invalid-bad-rel-type.elf Fri Jun 19 15:58:43 2015 differ

Added: llvm/trunk/test/Object/invalid.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/invalid.test?rev=240176&view=auto
==============================================================================
--- llvm/trunk/test/Object/invalid.test (added)
+++ llvm/trunk/test/Object/invalid.test Fri Jun 19 15:58:43 2015
@@ -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


_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: r240176-fix.patch
Type: application/octet-stream
Size: 503 bytes
Desc: r240176-fix.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150701/ad9d0269/attachment.obj>


More information about the llvm-commits mailing list