Index: llvm/trunk/include/llvm/DebugInfo/CodeView/StringTable.h =================================================================== --- llvm/trunk/include/llvm/DebugInfo/CodeView/StringTable.h +++ llvm/trunk/include/llvm/DebugInfo/CodeView/StringTable.h @@ -34,9 +34,11 @@ public: StringTableRef(); - Error initialize(BinaryStreamReader &Stream); + Error initialize(BinaryStreamRef Contents); - StringRef getString(uint32_t Offset) const; + Expected getString(uint32_t Offset) const; + + bool valid() const { return Stream.valid(); } private: BinaryStreamRef Stream; Index: llvm/trunk/include/llvm/DebugInfo/CodeView/SymbolVisitorDelegate.h =================================================================== --- llvm/trunk/include/llvm/DebugInfo/CodeView/SymbolVisitorDelegate.h +++ llvm/trunk/include/llvm/DebugInfo/CodeView/SymbolVisitorDelegate.h @@ -19,13 +19,15 @@ namespace codeview { +class StringTableRef; + class SymbolVisitorDelegate { public: virtual ~SymbolVisitorDelegate() = default; virtual uint32_t getRecordOffset(BinaryStreamReader Reader) = 0; virtual StringRef getFileNameForFileOffset(uint32_t FileOffset) = 0; - virtual StringRef getStringTable() = 0; + virtual StringTableRef getStringTable() = 0; }; } // end namespace codeview Index: llvm/trunk/include/llvm/DebugInfo/PDB/Native/PDBStringTable.h =================================================================== --- llvm/trunk/include/llvm/DebugInfo/PDB/Native/PDBStringTable.h +++ llvm/trunk/include/llvm/DebugInfo/PDB/Native/PDBStringTable.h @@ -1,5 +1,4 @@ -//===- PDBStringTable.h - PDB String Table -------------------------*- C++ -//-*-===// +//===- PDBStringTable.h - PDB String Table -----------------------*- C++-*-===// // // The LLVM Compiler Infrastructure // @@ -41,8 +40,8 @@ uint32_t getHashVersion() const; uint32_t getSignature() const; - StringRef getStringForID(uint32_t ID) const; - uint32_t getIDForString(StringRef Str) const; + Expected getStringForID(uint32_t ID) const; + Expected getIDForString(StringRef Str) const; FixedStreamArray name_ids() const; Index: llvm/trunk/include/llvm/Support/BinaryStreamRef.h =================================================================== --- llvm/trunk/include/llvm/Support/BinaryStreamRef.h +++ llvm/trunk/include/llvm/Support/BinaryStreamRef.h @@ -98,6 +98,9 @@ BinaryStreamRef(BinaryStreamRef &S, uint32_t Offset, uint32_t Length) = delete; + /// Check if a Stream is valid. + bool valid() const { return Stream != nullptr; } + /// Given an Offset into this StreamRef and a Size, return a reference to a /// buffer owned by the stream. /// Index: llvm/trunk/lib/DebugInfo/CodeView/StringTable.cpp =================================================================== --- llvm/trunk/lib/DebugInfo/CodeView/StringTable.cpp +++ llvm/trunk/lib/DebugInfo/CodeView/StringTable.cpp @@ -18,17 +18,17 @@ StringTableRef::StringTableRef() {} -Error StringTableRef::initialize(BinaryStreamReader &Reader) { - return Reader.readStreamRef(Stream, Reader.bytesRemaining()); +Error StringTableRef::initialize(BinaryStreamRef Contents) { + Stream = Contents; + return Error::success(); } -StringRef StringTableRef::getString(uint32_t Offset) const { +Expected StringTableRef::getString(uint32_t Offset) const { BinaryStreamReader Reader(Stream); Reader.setOffset(Offset); StringRef Result; - Error EC = Reader.readCString(Result); - assert(!EC); - consumeError(std::move(EC)); + if (auto EC = Reader.readCString(Result)) + return std::move(EC); return Result; } Index: llvm/trunk/lib/DebugInfo/CodeView/SymbolDumper.cpp =================================================================== --- llvm/trunk/lib/DebugInfo/CodeView/SymbolDumper.cpp +++ llvm/trunk/lib/DebugInfo/CodeView/SymbolDumper.cpp @@ -13,6 +13,7 @@ #include "llvm/DebugInfo/CodeView/CVSymbolVisitor.h" #include "llvm/DebugInfo/CodeView/CVTypeDumper.h" #include "llvm/DebugInfo/CodeView/EnumTables.h" +#include "llvm/DebugInfo/CodeView/StringTable.h" #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h" #include "llvm/DebugInfo/CodeView/SymbolDumpDelegate.h" #include "llvm/DebugInfo/CodeView/SymbolRecord.h" @@ -369,14 +370,14 @@ DictScope S(W, "DefRangeSubfield"); if (ObjDelegate) { - StringRef StringTable = ObjDelegate->getStringTable(); - auto ProgramStringTableOffset = DefRangeSubfield.Program; - if (ProgramStringTableOffset >= StringTable.size()) + StringTableRef Strings = ObjDelegate->getStringTable(); + auto ExpectedProgram = Strings.getString(DefRangeSubfield.Program); + if (!ExpectedProgram) { + consumeError(ExpectedProgram.takeError()); return llvm::make_error( "String table offset outside of bounds of String Table!"); - StringRef Program = - StringTable.drop_front(ProgramStringTableOffset).split('\0').first; - W.printString("Program", Program); + } + W.printString("Program", *ExpectedProgram); } W.printNumber("OffsetInParent", DefRangeSubfield.OffsetInParent); printLocalVariableAddrRange(DefRangeSubfield.Range, @@ -390,14 +391,14 @@ DictScope S(W, "DefRange"); if (ObjDelegate) { - StringRef StringTable = ObjDelegate->getStringTable(); - auto ProgramStringTableOffset = DefRange.Program; - if (ProgramStringTableOffset >= StringTable.size()) + StringTableRef Strings = ObjDelegate->getStringTable(); + auto ExpectedProgram = Strings.getString(DefRange.Program); + if (!ExpectedProgram) { + consumeError(ExpectedProgram.takeError()); return llvm::make_error( "String table offset outside of bounds of String Table!"); - StringRef Program = - StringTable.drop_front(ProgramStringTableOffset).split('\0').first; - W.printString("Program", Program); + } + W.printString("Program", *ExpectedProgram); } printLocalVariableAddrRange(DefRange.Range, DefRange.getRelocationOffset()); printLocalVariableAddrGap(DefRange.Gaps); Index: llvm/trunk/lib/DebugInfo/PDB/Native/PDBStringTable.cpp =================================================================== --- llvm/trunk/lib/DebugInfo/PDB/Native/PDBStringTable.cpp +++ llvm/trunk/lib/DebugInfo/PDB/Native/PDBStringTable.cpp @@ -42,7 +42,11 @@ } Error PDBStringTable::readStrings(BinaryStreamReader &Reader) { - if (auto EC = Strings.initialize(Reader)) { + BinaryStreamRef Stream; + if (auto EC = Reader.readStreamRef(Stream)) + return EC; + + if (auto EC = Strings.initialize(Stream)) { return joinErrors(std::move(EC), make_error(raw_error_code::corrupt_file, "Invalid hash table byte length")); @@ -99,11 +103,11 @@ return Error::success(); } -StringRef PDBStringTable::getStringForID(uint32_t ID) const { +Expected PDBStringTable::getStringForID(uint32_t ID) const { return Strings.getString(ID); } -uint32_t PDBStringTable::getIDForString(StringRef Str) const { +Expected PDBStringTable::getIDForString(StringRef Str) const { uint32_t Hash = (Header->HashVersion == 1) ? hashStringV1(Str) : hashStringV2(Str); size_t Count = IDs.size(); @@ -115,12 +119,14 @@ uint32_t Index = (Start + I) % Count; uint32_t ID = IDs[Index]; - StringRef S = getStringForID(ID); - if (S == Str) + auto ExpectedStr = getStringForID(ID); + if (!ExpectedStr) + return ExpectedStr.takeError(); + + if (*ExpectedStr == Str) return ID; } - // IDs[0] contains the ID of the "invalid" entry. - return IDs[0]; + return make_error(raw_error_code::no_entry); } FixedStreamArray PDBStringTable::name_ids() const { Index: llvm/trunk/tools/llvm-pdbdump/Diff.cpp =================================================================== --- llvm/trunk/tools/llvm-pdbdump/Diff.cpp +++ llvm/trunk/tools/llvm-pdbdump/Diff.cpp @@ -394,11 +394,17 @@ StringRef S1, S2; if (I < IdList1.size()) { Id1 = IdList1[I]; - S1 = ST1.getStringForID(*Id1); + if (auto Result = ST1.getStringForID(*Id1)) + S1 = *Result; + else + return Result.takeError(); } if (I < IdList2.size()) { Id2 = IdList2[I]; - S2 = ST2.getStringForID(*Id2); + if (auto Result = ST2.getStringForID(*Id2)) + S2 = *Result; + else + return Result.takeError(); } if (Id1 == Id2 && S1 == S2) continue; @@ -418,10 +424,18 @@ std::vector Strings1, Strings2; Strings1.reserve(IdList1.size()); Strings2.reserve(IdList2.size()); - for (auto ID : IdList1) - Strings1.push_back(ST1.getStringForID(ID)); - for (auto ID : IdList2) - Strings2.push_back(ST2.getStringForID(ID)); + for (auto ID : IdList1) { + auto S = ST1.getStringForID(ID); + if (!S) + return S.takeError(); + Strings1.push_back(*S); + } + for (auto ID : IdList2) { + auto S = ST2.getStringForID(ID); + if (!S) + return S.takeError(); + Strings2.push_back(*S); + } SmallVector OnlyP; SmallVector OnlyQ; Index: llvm/trunk/tools/llvm-pdbdump/LLVMOutputStyle.cpp =================================================================== --- llvm/trunk/tools/llvm-pdbdump/LLVMOutputStyle.cpp +++ llvm/trunk/tools/llvm-pdbdump/LLVMOutputStyle.cpp @@ -525,14 +525,17 @@ DictScope D(P, "String Table"); for (uint32_t I : IS->name_ids()) { - StringRef S = IS->getStringForID(I); - if (!S.empty()) { - llvm::SmallString<32> Str; - Str.append("'"); - Str.append(S); - Str.append("'"); - P.printString(Str); - } + auto ES = IS->getStringForID(I); + if (!ES) + return ES.takeError(); + + if (ES->empty()) + continue; + llvm::SmallString<32> Str; + Str.append("'"); + Str.append(*ES); + Str.append("'"); + P.printString(Str); } return Error::success(); } @@ -688,8 +691,11 @@ const auto &ST = *ExpectedST; for (const auto &E : Tpi->getHashAdjusters()) { DictScope DHA(P); - StringRef Name = ST.getStringForID(E.first); - P.printString("Type", Name); + auto Name = ST.getStringForID(E.first); + if (!Name) + return Name.takeError(); + + P.printString("Type", *Name); P.printHex("TI", E.second); } } Index: llvm/trunk/tools/llvm-pdbdump/YAMLOutputStyle.cpp =================================================================== --- llvm/trunk/tools/llvm-pdbdump/YAMLOutputStyle.cpp +++ llvm/trunk/tools/llvm-pdbdump/YAMLOutputStyle.cpp @@ -233,9 +233,12 @@ const auto &ST = ExpectedST.get(); for (auto ID : ST.name_ids()) { - StringRef S = ST.getStringForID(ID); - if (!S.empty()) - Obj.StringTable->push_back(S); + auto S = ST.getStringForID(ID); + if (!S) + return S.takeError(); + if (S->empty()) + continue; + Obj.StringTable->push_back(*S); } return Error::success(); } Index: llvm/trunk/tools/llvm-readobj/COFFDumper.cpp =================================================================== --- llvm/trunk/tools/llvm-readobj/COFFDumper.cpp +++ llvm/trunk/tools/llvm-readobj/COFFDumper.cpp @@ -29,6 +29,7 @@ #include "llvm/DebugInfo/CodeView/ModuleDebugInlineeLinesFragment.h" #include "llvm/DebugInfo/CodeView/ModuleDebugLineFragment.h" #include "llvm/DebugInfo/CodeView/RecordSerialization.h" +#include "llvm/DebugInfo/CodeView/StringTable.h" #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h" #include "llvm/DebugInfo/CodeView/SymbolDumpDelegate.h" #include "llvm/DebugInfo/CodeView/SymbolDumper.h" @@ -124,7 +125,7 @@ StringRef SectionContents, StringRef Block); /// Given a .debug$S section, find the string table and file checksum table. - void initializeFileAndStringTables(StringRef Data); + void initializeFileAndStringTables(BinaryStreamReader &Reader); void cacheRelocations(); @@ -145,8 +146,12 @@ const llvm::object::COFFObjectFile *Obj; bool RelocCached = false; RelocMapTy RelocMap; - StringRef CVFileChecksumTable; - StringRef CVStringTable; + + BinaryByteStream ChecksumContents; + VarStreamArray CVFileChecksumTable; + + BinaryByteStream StringTableContents; + StringTableRef CVStringTable; ScopedPrinter &Writer; TypeDatabase TypeDB; @@ -186,7 +191,7 @@ return CD.getFileNameForFileOffset(FileOffset); } - StringRef getStringTable() override { return CD.CVStringTable; } + StringTableRef getStringTable() override { return CD.CVStringTable; } private: COFFDumper &CD; @@ -725,30 +730,35 @@ } } -void COFFDumper::initializeFileAndStringTables(StringRef Data) { - while (!Data.empty() && (CVFileChecksumTable.data() == nullptr || - CVStringTable.data() == nullptr)) { +void COFFDumper::initializeFileAndStringTables(BinaryStreamReader &Reader) { + while (Reader.bytesRemaining() > 0 && + (!CVFileChecksumTable.valid() || !CVStringTable.valid())) { // The section consists of a number of subsection in the following format: // |SubSectionType|SubSectionSize|Contents...| uint32_t SubType, SubSectionSize; - error(consume(Data, SubType)); - error(consume(Data, SubSectionSize)); - if (SubSectionSize > Data.size()) - return error(object_error::parse_failed); + error(Reader.readInteger(SubType)); + error(Reader.readInteger(SubSectionSize)); + + StringRef Contents; + error(Reader.readFixedString(Contents, SubSectionSize)); + switch (ModuleDebugFragmentKind(SubType)) { - case ModuleDebugFragmentKind::FileChecksums: - CVFileChecksumTable = Data.substr(0, SubSectionSize); - break; - case ModuleDebugFragmentKind::StringTable: - CVStringTable = Data.substr(0, SubSectionSize); + case ModuleDebugFragmentKind::FileChecksums: { + ChecksumContents = BinaryByteStream(Contents, support::little); + BinaryStreamReader CSR(ChecksumContents); + error(CSR.readArray(CVFileChecksumTable, CSR.getLength())); break; + } + case ModuleDebugFragmentKind::StringTable: { + StringTableContents = BinaryByteStream(Contents, support::little); + error(CVStringTable.initialize(StringTableContents)); + } break; default: break; } + uint32_t PaddedSize = alignTo(SubSectionSize, 4); - if (PaddedSize > Data.size()) - error(object_error::parse_failed); - Data = Data.drop_front(PaddedSize); + error(Reader.skip(PaddedSize - SubSectionSize)); } } @@ -771,7 +781,9 @@ if (Magic != COFF::DEBUG_SECTION_MAGIC) return error(object_error::parse_failed); - initializeFileAndStringTables(Data); + BinaryByteStream FileAndStrings(Data, support::little); + BinaryStreamReader FSReader(FileAndStrings); + initializeFileAndStringTables(FSReader); // TODO: Convert this over to using ModuleSubstreamVisitor. while (!Data.empty()) { @@ -861,11 +873,7 @@ const FrameData *FD; error(SR.readObject(FD)); - if (FD->FrameFunc >= CVStringTable.size()) - error(object_error::parse_failed); - - StringRef FrameFunc = - CVStringTable.drop_front(FD->FrameFunc).split('\0').first; + StringRef FrameFunc = error(CVStringTable.getString(FD->FrameFunc)); DictScope S(W, "FrameData"); W.printHex("RvaStart", FD->RvaStart); @@ -971,10 +979,7 @@ for (auto &FC : Checksums) { DictScope S(W, "FileChecksum"); - if (FC.FileNameOffset >= CVStringTable.size()) - error(object_error::parse_failed); - StringRef Filename = - CVStringTable.drop_front(FC.FileNameOffset).split('\0').first; + StringRef Filename = error(CVStringTable.getString(FC.FileNameOffset)); W.printHex("Filename", Filename, FC.FileNameOffset); W.printHex("ChecksumSize", FC.Checksum.size()); W.printEnum("ChecksumKind", uint8_t(FC.Kind), @@ -1008,23 +1013,16 @@ StringRef COFFDumper::getFileNameForFileOffset(uint32_t FileOffset) { // The file checksum subsection should precede all references to it. - if (!CVFileChecksumTable.data() || !CVStringTable.data()) - error(object_error::parse_failed); - // Check if the file checksum table offset is valid. - if (FileOffset >= CVFileChecksumTable.size()) + if (!CVFileChecksumTable.valid() || !CVStringTable.valid()) error(object_error::parse_failed); - // The string table offset comes first before the file checksum. - StringRef Data = CVFileChecksumTable.drop_front(FileOffset); - uint32_t StringOffset; - error(consume(Data, StringOffset)); + auto Iter = CVFileChecksumTable.at(FileOffset); - // Check if the string table offset is valid. - if (StringOffset >= CVStringTable.size()) + // Check if the file checksum table offset is valid. + if (Iter == CVFileChecksumTable.end()) error(object_error::parse_failed); - // Return the null-terminated string. - return CVStringTable.drop_front(StringOffset).split('\0').first; + return error(CVStringTable.getString(Iter->FileNameOffset)); } void COFFDumper::printFileNameForOffset(StringRef Label, uint32_t FileOffset) { Index: llvm/trunk/tools/llvm-readobj/llvm-readobj.h =================================================================== --- llvm/trunk/tools/llvm-readobj/llvm-readobj.h +++ llvm/trunk/tools/llvm-readobj/llvm-readobj.h @@ -25,6 +25,11 @@ LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg); void error(std::error_code EC); void error(llvm::Error EC); + template T error(llvm::Expected &&E) { + error(E.takeError()); + return std::move(*E); + } + template T unwrapOrError(ErrorOr EO) { if (EO) return *EO; Index: llvm/trunk/unittests/DebugInfo/PDB/ErrorChecking.h =================================================================== --- llvm/trunk/unittests/DebugInfo/PDB/ErrorChecking.h +++ llvm/trunk/unittests/DebugInfo/PDB/ErrorChecking.h @@ -36,6 +36,18 @@ } \ } +#define EXPECT_EXPECTED_EQ(Val, Exp) \ + { \ + auto Result = Exp; \ + auto E = Result.takeError(); \ + EXPECT_FALSE(static_cast(E)); \ + if (E) { \ + consumeError(std::move(E)); \ + return; \ + } \ + EXPECT_EQ(Val, *Result); \ + } + #define EXPECT_UNEXPECTED(Exp) \ { \ auto E = Exp.takeError(); \ Index: llvm/trunk/unittests/DebugInfo/PDB/StringTableBuilderTest.cpp =================================================================== --- llvm/trunk/unittests/DebugInfo/PDB/StringTableBuilderTest.cpp +++ llvm/trunk/unittests/DebugInfo/PDB/StringTableBuilderTest.cpp @@ -25,6 +25,12 @@ class StringTableBuilderTest : public ::testing::Test {}; } +template +static void ExpectExpected(Expected &&E, const T &Value) { + EXPECT_EXPECTED(E); + EXPECT_EQ(Value, *E); +} + TEST_F(StringTableBuilderTest, Simple) { // Create /names table contents. PDBStringTableBuilder Builder; @@ -46,10 +52,11 @@ EXPECT_EQ(3U, Table.getNameCount()); EXPECT_EQ(1U, Table.getHashVersion()); - EXPECT_EQ("foo", Table.getStringForID(1)); - EXPECT_EQ("bar", Table.getStringForID(5)); - EXPECT_EQ("baz", Table.getStringForID(9)); - EXPECT_EQ(1U, Table.getIDForString("foo")); - EXPECT_EQ(5U, Table.getIDForString("bar")); - EXPECT_EQ(9U, Table.getIDForString("baz")); + + EXPECT_EXPECTED_EQ("foo", Table.getStringForID(1)); + EXPECT_EXPECTED_EQ("bar", Table.getStringForID(5)); + EXPECT_EXPECTED_EQ("baz", Table.getStringForID(9)); + EXPECT_EXPECTED_EQ(1U, Table.getIDForString("foo")); + EXPECT_EXPECTED_EQ(5U, Table.getIDForString("bar")); + EXPECT_EXPECTED_EQ(9U, Table.getIDForString("baz")); }