<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Nov 17, 2014 at 3:17 AM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: majnemer<br>
Date: Mon Nov 17 05:17:17 2014<br>
New Revision: 222124<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=222124&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=222124&view=rev</a><br>
Log:<br>
Object, COFF: Tighten the object file parser<br>
<br>
We were a little lax in a few areas:<br>
- We pretended that import libraries were like any old COFF file, they<br>
are not. In fact, they aren't really COFF files at all, we should<br>
probably grow some specialized functionality to handle them smarter.<br>
- Our symbol iterators were more than happy to attempt to go past the<br>
end of the symbol table if you had a symbol with a bad list of<br>
auxiliary symbols.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/Object/COFF.h<br>
llvm/trunk/lib/Object/COFFObjectFile.cpp<br>
llvm/trunk/test/tools/llvm-readobj/file-headers.test<br>
llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp<br>
llvm/trunk/tools/llvm-readobj/COFFDumper.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Object/COFF.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/COFF.h?rev=222124&r1=222123&r2=222124&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/COFF.h?rev=222124&r1=222123&r2=222124&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Object/COFF.h (original)<br>
+++ llvm/trunk/include/llvm/Object/COFF.h Mon Nov 17 05:17:17 2014<br>
@@ -508,7 +508,8 @@ public:<br>
}<br>
uint16_t getSizeOfOptionalHeader() const {<br>
if (COFFHeader)<br>
- return COFFHeader->SizeOfOptionalHeader;<br>
+ return COFFHeader->isImportLibrary() ? 0<br>
+ : COFFHeader->SizeOfOptionalHeader;<br>
// bigobj doesn't have this field.<br>
if (COFFBigObjHeader)<br>
return 0;<br>
@@ -516,7 +517,7 @@ public:<br>
}<br>
uint16_t getCharacteristics() const {<br>
if (COFFHeader)<br>
- return COFFHeader->Characteristics;<br>
+ return COFFHeader->isImportLibrary() ? 0 : COFFHeader->Characteristics;<br>
// bigobj doesn't have characteristics to speak of,<br>
// editbin will silently lie to you if you attempt to set any.<br>
if (COFFBigObjHeader)<br>
@@ -532,21 +533,22 @@ public:<br>
}<br>
uint32_t getNumberOfSections() const {<br>
if (COFFHeader)<br>
- return COFFHeader->NumberOfSections;<br>
+ return COFFHeader->isImportLibrary() ? 0 : COFFHeader->NumberOfSections;<br>
if (COFFBigObjHeader)<br>
return COFFBigObjHeader->NumberOfSections;<br>
llvm_unreachable("no COFF header!");<br>
}<br>
uint32_t getPointerToSymbolTable() const {<br>
if (COFFHeader)<br>
- return COFFHeader->PointerToSymbolTable;<br>
+ return COFFHeader->isImportLibrary() ? 0<br>
+ : COFFHeader->PointerToSymbolTable;<br>
if (COFFBigObjHeader)<br>
return COFFBigObjHeader->PointerToSymbolTable;<br>
llvm_unreachable("no COFF header!");<br>
}<br>
uint32_t getNumberOfSymbols() const {<br>
if (COFFHeader)<br>
- return COFFHeader->NumberOfSymbols;<br>
+ return COFFHeader->isImportLibrary() ? 0 : COFFHeader->NumberOfSymbols;<br>
if (COFFBigObjHeader)<br>
return COFFBigObjHeader->NumberOfSymbols;<br>
llvm_unreachable("no COFF header!");<br>
@@ -657,7 +659,7 @@ public:<br>
return EC;<br>
return COFFSymbolRef(Symb);<br>
}<br>
- llvm_unreachable("no symbol table pointer!");<br>
+ return object_error::parse_failed;<br>
}<br>
template <typename T><br>
std::error_code getAuxSymbol(uint32_t index, const T *&Res) const {<br>
<br>
Modified: llvm/trunk/lib/Object/COFFObjectFile.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/COFFObjectFile.cpp?rev=222124&r1=222123&r2=222124&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/COFFObjectFile.cpp?rev=222124&r1=222123&r2=222124&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Object/COFFObjectFile.cpp (original)<br>
+++ llvm/trunk/lib/Object/COFFObjectFile.cpp Mon Nov 17 05:17:17 2014<br>
@@ -54,7 +54,7 @@ static std::error_code checkOffset(Memor<br>
template <typename T><br>
static std::error_code getObject(const T *&Obj, MemoryBufferRef M,<br>
const void *Ptr,<br>
- const size_t Size = sizeof(T)) {<br>
+ const uint64_t Size = sizeof(T)) {<br>
uintptr_t Addr = uintptr_t(Ptr);<br>
if (std::error_code EC = checkOffset(M, Addr, Size))<br>
return EC;<br>
@@ -101,13 +101,10 @@ const coff_symbol_type *COFFObjectFile::<br>
const coff_symbol_type *Addr =<br>
reinterpret_cast<const coff_symbol_type *>(Ref.p);<br>
<br>
+ assert(!checkOffset(Data, uintptr_t(Addr), sizeof(*Addr)));<br>
#ifndef NDEBUG<br>
// Verify that the symbol points to a valid entry in the symbol table.<br>
uintptr_t Offset = uintptr_t(Addr) - uintptr_t(base());<br>
- if (Offset < getPointerToSymbolTable() ||<br>
- Offset >= getPointerToSymbolTable() +<br>
- (getNumberOfSymbols() * sizeof(coff_symbol_type)))<br>
- report_fatal_error("Symbol was outside of symbol table.");<br>
<br>
assert((Offset - getPointerToSymbolTable()) % sizeof(coff_symbol_type) == 0 &&<br>
"Symbol did not point to the beginning of a symbol");<br>
@@ -133,14 +130,15 @@ const coff_section *COFFObjectFile::toSe<br>
}<br>
<br>
void COFFObjectFile::moveSymbolNext(DataRefImpl &Ref) const {<br>
+ auto End = reinterpret_cast<uintptr_t>(StringTable);<br>
if (SymbolTable16) {<br>
const coff_symbol16 *Symb = toSymb<coff_symbol16>(Ref);<br>
Symb += 1 + Symb->NumberOfAuxSymbols;<br>
- Ref.p = reinterpret_cast<uintptr_t>(Symb);<br>
+ Ref.p = std::min(reinterpret_cast<uintptr_t>(Symb), End);<br>
} else if (SymbolTable32) {<br>
const coff_symbol32 *Symb = toSymb<coff_symbol32>(Ref);<br>
Symb += 1 + Symb->NumberOfAuxSymbols;<br>
- Ref.p = reinterpret_cast<uintptr_t>(Symb);<br>
+ Ref.p = std::min(reinterpret_cast<uintptr_t>(Symb), End);<br>
} else {<br>
llvm_unreachable("no symbol table pointer!");<br>
}<br>
@@ -365,8 +363,12 @@ bool COFFObjectFile::isSectionBSS(DataRe<br>
}<br>
<br>
bool COFFObjectFile::isSectionRequiredForExecution(DataRefImpl Ref) const {<br>
- // FIXME: Unimplemented<br>
- return true;<br>
+ // Sections marked 'Info', 'Remove', or 'Discardable' aren't required for<br>
+ // execution.<br>
+ const coff_section *Sec = toSec(Ref);<br>
+ return !(Sec->Characteristics &<br>
+ (COFF::IMAGE_SCN_LNK_INFO | COFF::IMAGE_SCN_LNK_REMOVE |<br>
+ COFF::IMAGE_SCN_MEM_DISCARDABLE));<br>
}<br>
<br>
bool COFFObjectFile::isSectionVirtual(DataRefImpl Ref) const {<br>
@@ -375,13 +377,22 @@ bool COFFObjectFile::isSectionVirtual(Da<br>
}<br>
<br>
bool COFFObjectFile::isSectionZeroInit(DataRefImpl Ref) const {<br>
- // FIXME: Unimplemented.<br>
- return false;<br>
+ const coff_section *Sec = toSec(Ref);<br>
+ return Sec->Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA;<br>
}<br>
<br>
bool COFFObjectFile::isSectionReadOnlyData(DataRefImpl Ref) const {<br>
- // FIXME: Unimplemented.<br>
- return false;<br>
+ const coff_section *Sec = toSec(Ref);<br>
+ // Check if it's any sort of data section.<br>
+ if (!(Sec->Characteristics & (COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA |<br>
+ COFF::IMAGE_SCN_CNT_INITIALIZED_DATA)))<br>
+ return false;<br>
+ // If it's writable or executable or contains code, it isn't read-only data.<br>
+ if (Sec->Characteristics &<br>
+ (COFF::IMAGE_SCN_CNT_CODE | COFF::IMAGE_SCN_MEM_EXECUTE |<br>
+ COFF::IMAGE_SCN_MEM_WRITE))<br>
+ return false;<br>
+ return true;<br>
}<br>
<br>
bool COFFObjectFile::sectionContainsSymbol(DataRefImpl SecRef,<br>
@@ -446,15 +457,15 @@ relocation_iterator COFFObjectFile::sect<br>
// Initialize the pointer to the symbol table.<br>
std::error_code COFFObjectFile::initSymbolTablePtr() {<br>
if (COFFHeader)<br>
- if (std::error_code EC =<br>
- getObject(SymbolTable16, Data, base() + getPointerToSymbolTable(),<br>
- getNumberOfSymbols() * getSymbolTableEntrySize()))<br>
+ if (std::error_code EC = getObject(<br>
+ SymbolTable16, Data, base() + getPointerToSymbolTable(),<br>
+ (uint64_t)getNumberOfSymbols() * getSymbolTableEntrySize()))<br>
return EC;<br>
<br>
if (COFFBigObjHeader)<br>
- if (std::error_code EC =<br>
- getObject(SymbolTable32, Data, base() + getPointerToSymbolTable(),<br>
- getNumberOfSymbols() * getSymbolTableEntrySize()))<br>
+ if (std::error_code EC = getObject(<br>
+ SymbolTable32, Data, base() + getPointerToSymbolTable(),<br>
+ (uint64_t)getNumberOfSymbols() * getSymbolTableEntrySize()))<br>
return EC;<br>
<br>
// Find string table. The first four byte of the string table contains the<br>
@@ -681,13 +692,20 @@ COFFObjectFile::COFFObjectFile(MemoryBuf<br>
}<br>
<br>
if ((EC = getObject(SectionTable, Data, base() + CurPtr,<br>
- getNumberOfSections() * sizeof(coff_section))))<br>
+ (uint64_t)getNumberOfSections() * sizeof(coff_section))))<br>
return;<br>
<br>
// Initialize the pointer to the symbol table.<br>
- if (getPointerToSymbolTable() != 0)<br>
+ if (getPointerToSymbolTable() != 0) {<br>
if ((EC = initSymbolTablePtr()))<br>
return;<br>
+ } else {<br>
+ // We had better not have any symbols if we don't have a symbol table.<br>
+ if (getNumberOfSymbols() != 0) {<br>
+ EC = object_error::parse_failed;<br>
+ return;<br>
+ }<br>
+ }<br></blockquote><div><br></div><div>nit: else { if { could be written as else if {</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
// Initialize the pointer to the beginning of the import table.<br>
if ((EC = initImportTablePtr()))<br>
@@ -843,15 +861,15 @@ COFFObjectFile::getDataDirectory(uint32_<br>
<br>
std::error_code COFFObjectFile::getSection(int32_t Index,<br>
const coff_section *&Result) const {<br>
- // Check for special index values.<br>
+ Result = nullptr;<br>
if (COFF::isReservedSectionNumber(Index))<br>
- Result = nullptr;<br>
- else if (Index > 0 && static_cast<uint32_t>(Index) <= getNumberOfSections())<br>
+ return object_error::success;<br>
+ if (static_cast<uint32_t>(Index) <= getNumberOfSections()) {<br>
// We already verified the section table data, so no need to check again.<br>
Result = SectionTable + (Index - 1);<br>
- else<br>
- return object_error::parse_failed;<br>
- return object_error::success;<br>
+ return object_error::success;<br>
+ }<br>
+ return object_error::parse_failed;<br>
}<br>
<br>
std::error_code COFFObjectFile::getString(uint32_t Offset,<br>
@@ -1001,6 +1019,8 @@ std::error_code COFFObjectFile::getReloc<br>
symbol_iterator COFFObjectFile::getRelocationSymbol(DataRefImpl Rel) const {<br>
const coff_relocation *R = toRel(Rel);<br>
DataRefImpl Ref;<br>
+ if (R->SymbolTableIndex >= getNumberOfSymbols())<br>
+ return symbol_end();<br>
if (SymbolTable16)<br>
Ref.p = reinterpret_cast<uintptr_t>(SymbolTable16 + R->SymbolTableIndex);<br>
else if (SymbolTable32)<br>
<br>
Modified: llvm/trunk/test/tools/llvm-readobj/file-headers.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-readobj/file-headers.test?rev=222124&r1=222123&r2=222124&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-readobj/file-headers.test?rev=222124&r1=222123&r2=222124&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/tools/llvm-readobj/file-headers.test (original)<br>
+++ llvm/trunk/test/tools/llvm-readobj/file-headers.test Mon Nov 17 05:17:17 2014<br>
@@ -335,12 +335,11 @@ COFF-IMPORTLIB-NEXT: Arch: unknown<br>
COFF-IMPORTLIB-NEXT: AddressSize: 32bit<br>
COFF-IMPORTLIB-NEXT: ImageFileHeader {<br>
COFF-IMPORTLIB-NEXT: Machine: IMAGE_FILE_MACHINE_UNKNOWN (0x0)<br>
-COFF-IMPORTLIB-NEXT: SectionCount: 65535<br>
+COFF-IMPORTLIB-NEXT: SectionCount: 0<br>
COFF-IMPORTLIB-NEXT: TimeDateStamp: 1970-09-09 19:52:32 (0x14C0000)<br>
-COFF-IMPORTLIB-NEXT: PointerToSymbolTable: 0x528542EB<br>
-COFF-IMPORTLIB-NEXT: SymbolCount: 20<br>
+COFF-IMPORTLIB-NEXT: PointerToSymbolTable: 0x0<br>
+COFF-IMPORTLIB-NEXT: SymbolCount: 0<br>
COFF-IMPORTLIB-NEXT: OptionalHeaderSize: 0<br>
-COFF-IMPORTLIB-NEXT: Characteristics [ (0x8)<br>
-COFF-IMPORTLIB-NEXT: IMAGE_FILE_LOCAL_SYMS_STRIPPED (0x8)<br>
+COFF-IMPORTLIB-NEXT: Characteristics [ (0x0)<br>
COFF-IMPORTLIB-NEXT: ]<br>
COFF-IMPORTLIB-NEXT: }<br>
<br>
Modified: llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp?rev=222124&r1=222123&r2=222124&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp?rev=222124&r1=222123&r2=222124&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp (original)<br>
+++ llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp Mon Nov 17 05:17:17 2014<br>
@@ -307,8 +307,7 @@ static void DisassembleObject(const Obje<br>
}<br>
<br>
for (const SectionRef &Section : Obj->sections()) {<br>
- bool Text = Section.isText();<br>
- if (!Text)<br>
+ if (!Section.isText() || Section.isVirtual())<br>
continue;<br>
<br>
uint64_t SectionAddr = Section.getAddress();<br>
<br>
Modified: llvm/trunk/tools/llvm-readobj/COFFDumper.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/COFFDumper.cpp?rev=222124&r1=222123&r2=222124&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/COFFDumper.cpp?rev=222124&r1=222123&r2=222124&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/tools/llvm-readobj/COFFDumper.cpp (original)<br>
+++ llvm/trunk/tools/llvm-readobj/COFFDumper.cpp Mon Nov 17 05:17:17 2014<br>
@@ -825,22 +825,22 @@ void COFFDumper::printSymbols() {<br>
<br>
void COFFDumper::printDynamicSymbols() { ListScope Group(W, "DynamicSymbols"); }<br>
<br>
-static StringRef getSectionName(const llvm::object::COFFObjectFile *Obj,<br>
- COFFSymbolRef Symbol,<br>
- const coff_section *Section) {<br>
+static ErrorOr<StringRef><br>
+getSectionName(const llvm::object::COFFObjectFile *Obj, int32_t SectionNumber,<br>
+ const coff_section *Section) {<br>
if (Section) {<br>
StringRef SectionName;<br>
- Obj->getSectionName(Section, SectionName);<br>
+ if (std::error_code EC = Obj->getSectionName(Section, SectionName))<br>
+ return EC;<br>
return SectionName;<br>
}<br>
- int32_t SectionNumber = Symbol.getSectionNumber();<br>
if (SectionNumber == llvm::COFF::IMAGE_SYM_DEBUG)<br>
- return "IMAGE_SYM_DEBUG";<br>
+ return StringRef("IMAGE_SYM_DEBUG");<br>
if (SectionNumber == llvm::COFF::IMAGE_SYM_ABSOLUTE)<br>
- return "IMAGE_SYM_ABSOLUTE";<br>
+ return StringRef("IMAGE_SYM_ABSOLUTE");<br>
if (SectionNumber == llvm::COFF::IMAGE_SYM_UNDEFINED)<br>
- return "IMAGE_SYM_UNDEFINED";<br>
- return "";<br>
+ return StringRef("IMAGE_SYM_UNDEFINED");<br>
+ return StringRef("");<br>
}<br>
<br>
void COFFDumper::printSymbol(const SymbolRef &Sym) {<br>
@@ -858,7 +858,11 @@ void COFFDumper::printSymbol(const Symbo<br>
if (Obj->getSymbolName(Symbol, SymbolName))<br>
SymbolName = "";<br>
<br>
- StringRef SectionName = getSectionName(Obj, Symbol, Section);<br>
+ StringRef SectionName = "";<br>
+ ErrorOr<StringRef> Res =<br>
+ getSectionName(Obj, Symbol.getSectionNumber(), Section);<br>
+ if (Res)<br>
+ SectionName = *Res;<br>
<br>
W.printString("Name", SymbolName);<br>
W.printNumber("Value", Symbol.getValue());<br>
@@ -929,10 +933,14 @@ void COFFDumper::printSymbol(const Symbo<br>
if (Section && Section->Characteristics & COFF::IMAGE_SCN_LNK_COMDAT<br>
&& Aux->Selection == COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE) {<br>
const coff_section *Assoc;<br>
- StringRef AssocName;<br>
- std::error_code EC;<br>
- if ((EC = Obj->getSection(AuxNumber, Assoc)) ||<br>
- (EC = Obj->getSectionName(Assoc, AssocName))) {<br>
+ StringRef AssocName = "";<br>
+ std::error_code EC = Obj->getSection(AuxNumber, Assoc);<br>
+ ErrorOr<StringRef> Res = getSectionName(Obj, AuxNumber, Assoc);<br>
+ if (Res)<br>
+ AssocName = *Res;<br>
+ if (!EC)<br>
+ EC = Res.getError();<br>
+ if (EC) {<br>
AssocName = "";<br>
error(EC);<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>