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.

llvm-svn: 222124

diff --git a/llvm/include/llvm/Object/COFF.h b/llvm/include/llvm/Object/COFF.h
index 100f419..80078a4 100644 (file)
--- a/llvm/include/llvm/Object/COFF.h
+++ b/llvm/include/llvm/Object/COFF.h
@@ -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 {

diff --git a/llvm/lib/Object/COFFObjectFile.cpp b/llvm/lib/Object/COFFObjectFile.cpp
index a1fc51a..e25b63b 100644 (file)
--- a/llvm/lib/Object/COFFObjectFile.cpp
+++ b/llvm/lib/Object/COFFObjectFile.cpp
@@ -54,7 +54,7 @@ static std::error_code checkOffset(MemoryBufferRef M, uintptr_t Addr,
 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::toSymb(DataRefImpl Ref) const {
   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::toSec(DataRefImpl Ref) const {
 }
 
 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(DataRefImpl Ref) const {
 }
 
 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(DataRefImpl Ref) const {
 }
 
 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::section_rel_end(DataRefImpl Ref) const {
 // 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(MemoryBufferRef Object, std::error_code &EC)
   }
 
   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_t Index,
 
 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::getRelocationOffset(DataRefImpl Rel,
 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)

diff --git a/llvm/test/tools/llvm-readobj/file-headers.test b/llvm/test/tools/llvm-readobj/file-headers.test
index 1627eb7..a19cb4b 100644 (file)
--- a/llvm/test/tools/llvm-readobj/file-headers.test
+++ b/llvm/test/tools/llvm-readobj/file-headers.test
@@ -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: }

diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index da5c275..8903bff 100644 (file)
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -307,8 +307,7 @@ static void DisassembleObject(const ObjectFile *Obj, bool InlineRelocs) {
   }
 
   for (const SectionRef &Section : Obj->sections()) {
-    bool Text = Section.isText();
-    if (!Text)
+    if (!Section.isText() || Section.isVirtual())
       continue;
 
     uint64_t SectionAddr = Section.getAddress();

diff --git a/llvm/tools/llvm-readobj/COFFDumper.cpp b/llvm/tools/llvm-readobj/COFFDumper.cpp
index 1506f06..c5469db 100644 (file)
--- a/llvm/tools/llvm-readobj/COFFDumper.cpp
+++ b/llvm/tools/llvm-readobj/COFFDumper.cpp
@@ -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 SymbolRef &Sym) {
   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 SymbolRef &Sym) {
       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);
         }