[llvm] r222124 - Object, COFF: Tighten the object file parser
Rui Ueyama
ruiu at google.com
Mon Nov 17 15:06:23 PST 2014
On Mon, Nov 17, 2014 at 3:17 AM, David Majnemer <david.majnemer at gmail.com>
wrote:
> 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;
> + }
> + }
>
nit: else { if { could be written as else if {
>
> // 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);
> }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141117/4c92ba99/attachment.html>
More information about the llvm-commits
mailing list