[llvm] r222124 - Object, COFF: Tighten the object file parser

David Majnemer david.majnemer at gmail.com
Mon Nov 17 03:17:17 PST 2014


Author: majnemer
Date: Mon Nov 17 05:17:17 2014
New Revision: 222124

URL: http://llvm.org/viewvc/llvm-project?rev=222124&view=rev
Log:
Object, COFF: Tighten the object file parser

We were a little lax in a few areas:
- We pretended that import libraries were like any old COFF file, they
  are not.  In fact, they aren't really COFF files at all, we should
  probably grow some specialized functionality to handle them smarter.
- Our symbol iterators were more than happy to attempt to go past the
  end of the symbol table if you had a symbol with a bad list of
  auxiliary symbols.

Modified:
    llvm/trunk/include/llvm/Object/COFF.h
    llvm/trunk/lib/Object/COFFObjectFile.cpp
    llvm/trunk/test/tools/llvm-readobj/file-headers.test
    llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
    llvm/trunk/tools/llvm-readobj/COFFDumper.cpp

Modified: llvm/trunk/include/llvm/Object/COFF.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/COFF.h?rev=222124&r1=222123&r2=222124&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/COFF.h (original)
+++ llvm/trunk/include/llvm/Object/COFF.h Mon Nov 17 05:17:17 2014
@@ -508,7 +508,8 @@ public:
   }
   uint16_t getSizeOfOptionalHeader() const {
     if (COFFHeader)
-      return COFFHeader->SizeOfOptionalHeader;
+      return COFFHeader->isImportLibrary() ? 0
+                                           : COFFHeader->SizeOfOptionalHeader;
     // bigobj doesn't have this field.
     if (COFFBigObjHeader)
       return 0;
@@ -516,7 +517,7 @@ public:
   }
   uint16_t getCharacteristics() const {
     if (COFFHeader)
-      return COFFHeader->Characteristics;
+      return COFFHeader->isImportLibrary() ? 0 : COFFHeader->Characteristics;
     // bigobj doesn't have characteristics to speak of,
     // editbin will silently lie to you if you attempt to set any.
     if (COFFBigObjHeader)
@@ -532,21 +533,22 @@ public:
   }
   uint32_t getNumberOfSections() const {
     if (COFFHeader)
-      return COFFHeader->NumberOfSections;
+      return COFFHeader->isImportLibrary() ? 0 : COFFHeader->NumberOfSections;
     if (COFFBigObjHeader)
       return COFFBigObjHeader->NumberOfSections;
     llvm_unreachable("no COFF header!");
   }
   uint32_t getPointerToSymbolTable() const {
     if (COFFHeader)
-      return COFFHeader->PointerToSymbolTable;
+      return COFFHeader->isImportLibrary() ? 0
+                                           : COFFHeader->PointerToSymbolTable;
     if (COFFBigObjHeader)
       return COFFBigObjHeader->PointerToSymbolTable;
     llvm_unreachable("no COFF header!");
   }
   uint32_t getNumberOfSymbols() const {
     if (COFFHeader)
-      return COFFHeader->NumberOfSymbols;
+      return COFFHeader->isImportLibrary() ? 0 : COFFHeader->NumberOfSymbols;
     if (COFFBigObjHeader)
       return COFFBigObjHeader->NumberOfSymbols;
     llvm_unreachable("no COFF header!");
@@ -657,7 +659,7 @@ public:
         return EC;
       return COFFSymbolRef(Symb);
     }
-    llvm_unreachable("no symbol table pointer!");
+    return object_error::parse_failed;
   }
   template <typename T>
   std::error_code getAuxSymbol(uint32_t index, const T *&Res) const {

Modified: llvm/trunk/lib/Object/COFFObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/COFFObjectFile.cpp?rev=222124&r1=222123&r2=222124&view=diff
==============================================================================
--- llvm/trunk/lib/Object/COFFObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/COFFObjectFile.cpp Mon Nov 17 05:17:17 2014
@@ -54,7 +54,7 @@ static std::error_code checkOffset(Memor
 template <typename T>
 static std::error_code getObject(const T *&Obj, MemoryBufferRef M,
                                  const void *Ptr,
-                                 const size_t Size = sizeof(T)) {
+                                 const uint64_t Size = sizeof(T)) {
   uintptr_t Addr = uintptr_t(Ptr);
   if (std::error_code EC = checkOffset(M, Addr, Size))
     return EC;
@@ -101,13 +101,10 @@ const coff_symbol_type *COFFObjectFile::
   const coff_symbol_type *Addr =
       reinterpret_cast<const coff_symbol_type *>(Ref.p);
 
+  assert(!checkOffset(Data, uintptr_t(Addr), sizeof(*Addr)));
 #ifndef NDEBUG
   // Verify that the symbol points to a valid entry in the symbol table.
   uintptr_t Offset = uintptr_t(Addr) - uintptr_t(base());
-  if (Offset < getPointerToSymbolTable() ||
-      Offset >= getPointerToSymbolTable() +
-                    (getNumberOfSymbols() * sizeof(coff_symbol_type)))
-    report_fatal_error("Symbol was outside of symbol table.");
 
   assert((Offset - getPointerToSymbolTable()) % sizeof(coff_symbol_type) == 0 &&
          "Symbol did not point to the beginning of a symbol");
@@ -133,14 +130,15 @@ const coff_section *COFFObjectFile::toSe
 }
 
 void COFFObjectFile::moveSymbolNext(DataRefImpl &Ref) const {
+  auto End = reinterpret_cast<uintptr_t>(StringTable);
   if (SymbolTable16) {
     const coff_symbol16 *Symb = toSymb<coff_symbol16>(Ref);
     Symb += 1 + Symb->NumberOfAuxSymbols;
-    Ref.p = reinterpret_cast<uintptr_t>(Symb);
+    Ref.p = std::min(reinterpret_cast<uintptr_t>(Symb), End);
   } else if (SymbolTable32) {
     const coff_symbol32 *Symb = toSymb<coff_symbol32>(Ref);
     Symb += 1 + Symb->NumberOfAuxSymbols;
-    Ref.p = reinterpret_cast<uintptr_t>(Symb);
+    Ref.p = std::min(reinterpret_cast<uintptr_t>(Symb), End);
   } else {
     llvm_unreachable("no symbol table pointer!");
   }
@@ -365,8 +363,12 @@ bool COFFObjectFile::isSectionBSS(DataRe
 }
 
 bool COFFObjectFile::isSectionRequiredForExecution(DataRefImpl Ref) const {
-  // FIXME: Unimplemented
-  return true;
+  // Sections marked 'Info', 'Remove', or 'Discardable' aren't required for
+  // execution.
+  const coff_section *Sec = toSec(Ref);
+  return !(Sec->Characteristics &
+           (COFF::IMAGE_SCN_LNK_INFO | COFF::IMAGE_SCN_LNK_REMOVE |
+            COFF::IMAGE_SCN_MEM_DISCARDABLE));
 }
 
 bool COFFObjectFile::isSectionVirtual(DataRefImpl Ref) const {
@@ -375,13 +377,22 @@ bool COFFObjectFile::isSectionVirtual(Da
 }
 
 bool COFFObjectFile::isSectionZeroInit(DataRefImpl Ref) const {
-  // FIXME: Unimplemented.
-  return false;
+  const coff_section *Sec = toSec(Ref);
+  return Sec->Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA;
 }
 
 bool COFFObjectFile::isSectionReadOnlyData(DataRefImpl Ref) const {
-  // FIXME: Unimplemented.
-  return false;
+  const coff_section *Sec = toSec(Ref);
+  // Check if it's any sort of data section.
+  if (!(Sec->Characteristics & (COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA |
+                                COFF::IMAGE_SCN_CNT_INITIALIZED_DATA)))
+    return false;
+  // If it's writable or executable or contains code, it isn't read-only data.
+  if (Sec->Characteristics &
+      (COFF::IMAGE_SCN_CNT_CODE | COFF::IMAGE_SCN_MEM_EXECUTE |
+       COFF::IMAGE_SCN_MEM_WRITE))
+    return false;
+  return true;
 }
 
 bool COFFObjectFile::sectionContainsSymbol(DataRefImpl SecRef,
@@ -446,15 +457,15 @@ relocation_iterator COFFObjectFile::sect
 // Initialize the pointer to the symbol table.
 std::error_code COFFObjectFile::initSymbolTablePtr() {
   if (COFFHeader)
-    if (std::error_code EC =
-            getObject(SymbolTable16, Data, base() + getPointerToSymbolTable(),
-                      getNumberOfSymbols() * getSymbolTableEntrySize()))
+    if (std::error_code EC = getObject(
+            SymbolTable16, Data, base() + getPointerToSymbolTable(),
+            (uint64_t)getNumberOfSymbols() * getSymbolTableEntrySize()))
       return EC;
 
   if (COFFBigObjHeader)
-    if (std::error_code EC =
-            getObject(SymbolTable32, Data, base() + getPointerToSymbolTable(),
-                      getNumberOfSymbols() * getSymbolTableEntrySize()))
+    if (std::error_code EC = getObject(
+            SymbolTable32, Data, base() + getPointerToSymbolTable(),
+            (uint64_t)getNumberOfSymbols() * getSymbolTableEntrySize()))
       return EC;
 
   // Find string table. The first four byte of the string table contains the
@@ -681,13 +692,20 @@ COFFObjectFile::COFFObjectFile(MemoryBuf
   }
 
   if ((EC = getObject(SectionTable, Data, base() + CurPtr,
-                      getNumberOfSections() * sizeof(coff_section))))
+                      (uint64_t)getNumberOfSections() * sizeof(coff_section))))
     return;
 
   // Initialize the pointer to the symbol table.
-  if (getPointerToSymbolTable() != 0)
+  if (getPointerToSymbolTable() != 0) {
     if ((EC = initSymbolTablePtr()))
       return;
+  } else {
+    // We had better not have any symbols if we don't have a symbol table.
+    if (getNumberOfSymbols() != 0) {
+      EC = object_error::parse_failed;
+      return;
+    }
+  }
 
   // Initialize the pointer to the beginning of the import table.
   if ((EC = initImportTablePtr()))
@@ -843,15 +861,15 @@ COFFObjectFile::getDataDirectory(uint32_
 
 std::error_code COFFObjectFile::getSection(int32_t Index,
                                            const coff_section *&Result) const {
-  // Check for special index values.
+  Result = nullptr;
   if (COFF::isReservedSectionNumber(Index))
-    Result = nullptr;
-  else if (Index > 0 && static_cast<uint32_t>(Index) <= getNumberOfSections())
+    return object_error::success;
+  if (static_cast<uint32_t>(Index) <= getNumberOfSections()) {
     // We already verified the section table data, so no need to check again.
     Result = SectionTable + (Index - 1);
-  else
-    return object_error::parse_failed;
-  return object_error::success;
+    return object_error::success;
+  }
+  return object_error::parse_failed;
 }
 
 std::error_code COFFObjectFile::getString(uint32_t Offset,
@@ -1001,6 +1019,8 @@ std::error_code COFFObjectFile::getReloc
 symbol_iterator COFFObjectFile::getRelocationSymbol(DataRefImpl Rel) const {
   const coff_relocation *R = toRel(Rel);
   DataRefImpl Ref;
+  if (R->SymbolTableIndex >= getNumberOfSymbols())
+    return symbol_end();
   if (SymbolTable16)
     Ref.p = reinterpret_cast<uintptr_t>(SymbolTable16 + R->SymbolTableIndex);
   else if (SymbolTable32)

Modified: llvm/trunk/test/tools/llvm-readobj/file-headers.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-readobj/file-headers.test?rev=222124&r1=222123&r2=222124&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-readobj/file-headers.test (original)
+++ llvm/trunk/test/tools/llvm-readobj/file-headers.test Mon Nov 17 05:17:17 2014
@@ -335,12 +335,11 @@ COFF-IMPORTLIB-NEXT: Arch: unknown
 COFF-IMPORTLIB-NEXT: AddressSize: 32bit
 COFF-IMPORTLIB-NEXT: ImageFileHeader {
 COFF-IMPORTLIB-NEXT:   Machine: IMAGE_FILE_MACHINE_UNKNOWN (0x0)
-COFF-IMPORTLIB-NEXT:   SectionCount: 65535
+COFF-IMPORTLIB-NEXT:   SectionCount: 0
 COFF-IMPORTLIB-NEXT:   TimeDateStamp: 1970-09-09 19:52:32 (0x14C0000)
-COFF-IMPORTLIB-NEXT:   PointerToSymbolTable: 0x528542EB
-COFF-IMPORTLIB-NEXT:   SymbolCount: 20
+COFF-IMPORTLIB-NEXT:   PointerToSymbolTable: 0x0
+COFF-IMPORTLIB-NEXT:   SymbolCount: 0
 COFF-IMPORTLIB-NEXT:   OptionalHeaderSize: 0
-COFF-IMPORTLIB-NEXT:   Characteristics [ (0x8)
-COFF-IMPORTLIB-NEXT:     IMAGE_FILE_LOCAL_SYMS_STRIPPED (0x8)
+COFF-IMPORTLIB-NEXT:   Characteristics [ (0x0)
 COFF-IMPORTLIB-NEXT:   ]
 COFF-IMPORTLIB-NEXT: }

Modified: llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp?rev=222124&r1=222123&r2=222124&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp (original)
+++ llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp Mon Nov 17 05:17:17 2014
@@ -307,8 +307,7 @@ static void DisassembleObject(const Obje
   }
 
   for (const SectionRef &Section : Obj->sections()) {
-    bool Text = Section.isText();
-    if (!Text)
+    if (!Section.isText() || Section.isVirtual())
       continue;
 
     uint64_t SectionAddr = Section.getAddress();

Modified: llvm/trunk/tools/llvm-readobj/COFFDumper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/COFFDumper.cpp?rev=222124&r1=222123&r2=222124&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-readobj/COFFDumper.cpp (original)
+++ llvm/trunk/tools/llvm-readobj/COFFDumper.cpp Mon Nov 17 05:17:17 2014
@@ -825,22 +825,22 @@ void COFFDumper::printSymbols() {
 
 void COFFDumper::printDynamicSymbols() { ListScope Group(W, "DynamicSymbols"); }
 
-static StringRef getSectionName(const llvm::object::COFFObjectFile *Obj,
-                                COFFSymbolRef Symbol,
-                                const coff_section *Section) {
+static ErrorOr<StringRef>
+getSectionName(const llvm::object::COFFObjectFile *Obj, int32_t SectionNumber,
+               const coff_section *Section) {
   if (Section) {
     StringRef SectionName;
-    Obj->getSectionName(Section, SectionName);
+    if (std::error_code EC = Obj->getSectionName(Section, SectionName))
+      return EC;
     return SectionName;
   }
-  int32_t SectionNumber = Symbol.getSectionNumber();
   if (SectionNumber == llvm::COFF::IMAGE_SYM_DEBUG)
-    return "IMAGE_SYM_DEBUG";
+    return StringRef("IMAGE_SYM_DEBUG");
   if (SectionNumber == llvm::COFF::IMAGE_SYM_ABSOLUTE)
-    return "IMAGE_SYM_ABSOLUTE";
+    return StringRef("IMAGE_SYM_ABSOLUTE");
   if (SectionNumber == llvm::COFF::IMAGE_SYM_UNDEFINED)
-    return "IMAGE_SYM_UNDEFINED";
-  return "";
+    return StringRef("IMAGE_SYM_UNDEFINED");
+  return StringRef("");
 }
 
 void COFFDumper::printSymbol(const SymbolRef &Sym) {
@@ -858,7 +858,11 @@ void COFFDumper::printSymbol(const Symbo
   if (Obj->getSymbolName(Symbol, SymbolName))
     SymbolName = "";
 
-  StringRef SectionName = getSectionName(Obj, Symbol, Section);
+  StringRef SectionName = "";
+  ErrorOr<StringRef> Res =
+      getSectionName(Obj, Symbol.getSectionNumber(), Section);
+  if (Res)
+    SectionName = *Res;
 
   W.printString("Name", SymbolName);
   W.printNumber("Value", Symbol.getValue());
@@ -929,10 +933,14 @@ void COFFDumper::printSymbol(const Symbo
       if (Section && Section->Characteristics & COFF::IMAGE_SCN_LNK_COMDAT
           && Aux->Selection == COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE) {
         const coff_section *Assoc;
-        StringRef AssocName;
-        std::error_code EC;
-        if ((EC = Obj->getSection(AuxNumber, Assoc)) ||
-            (EC = Obj->getSectionName(Assoc, AssocName))) {
+        StringRef AssocName = "";
+        std::error_code EC = Obj->getSection(AuxNumber, Assoc);
+        ErrorOr<StringRef> Res = getSectionName(Obj, AuxNumber, Assoc);
+        if (Res)
+          AssocName = *Res;
+        if (!EC)
+          EC = Res.getError();
+        if (EC) {
           AssocName = "";
           error(EC);
         }





More information about the llvm-commits mailing list