From 0e828958264734e60115ba2482437008c822d7db Mon Sep 17 00:00:00 2001 From: JF Bastien Date: Wed, 26 Jun 2019 19:50:12 +0000 Subject: BitStream reader: propagate errors The bitstream reader handles errors poorly. This has two effects: * Bugs in file handling (especially modules) manifest as an "unexpected end of file" crash * Users of clang as a library end up aborting because the code unconditionally calls `report_fatal_error` The bitstream reader should be more resilient and return Expected / Error as soon as an error is encountered, not way late like it does now. This patch starts doing so and adopting the error handling where I think it makes sense. There's plenty more to do: this patch propagates errors to be minimally useful, and follow-ups will propagate them further and improve diagnostics. https://bugs.llvm.org/show_bug.cgi?id=42311 Differential Revision: https://reviews.llvm.org/D63518 llvm-svn: 364464 --- clang-tools-extra/clang-doc/BitcodeReader.cpp | 86 +++++++++++++++++++-------- 1 file changed, 60 insertions(+), 26 deletions(-) (limited to 'clang-tools-extra/clang-doc/BitcodeReader.cpp') diff --git a/clang-tools-extra/clang-doc/BitcodeReader.cpp b/clang-tools-extra/clang-doc/BitcodeReader.cpp index 4489daf3d92..77e67e896c2 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.cpp +++ b/clang-tools-extra/clang-doc/BitcodeReader.cpp @@ -491,24 +491,27 @@ template llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID, T I) { Record R; llvm::StringRef Blob; - unsigned RecID = Stream.readRecord(ID, R, &Blob); - return parseRecord(R, RecID, Blob, I); + llvm::Expected MaybeRecID = Stream.readRecord(ID, R, &Blob); + if (!MaybeRecID) + return MaybeRecID.takeError(); + return parseRecord(R, MaybeRecID.get(), Blob, I); } template <> llvm::Error ClangDocBitcodeReader::readRecord(unsigned ID, Reference *I) { Record R; llvm::StringRef Blob; - unsigned RecID = Stream.readRecord(ID, R, &Blob); - return parseRecord(R, RecID, Blob, I, CurrentReferenceField); + llvm::Expected MaybeRecID = Stream.readRecord(ID, R, &Blob); + if (!MaybeRecID) + return MaybeRecID.takeError(); + return parseRecord(R, MaybeRecID.get(), Blob, I, CurrentReferenceField); } // Read a block of records into a single info. template llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) { - if (Stream.EnterSubBlock(ID)) - return llvm::make_error("Unable to enter subblock.\n", - llvm::inconvertibleErrorCode()); + if (llvm::Error Err = Stream.EnterSubBlock(ID)) + return Err; while (true) { unsigned BlockOrCode = 0; @@ -521,9 +524,9 @@ llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) { case Cursor::BlockEnd: return llvm::Error::success(); case Cursor::BlockBegin: - if (auto Err = readSubBlock(BlockOrCode, I)) { - if (!Stream.SkipBlock()) - continue; + if (llvm::Error Err = readSubBlock(BlockOrCode, I)) { + if (llvm::Error Skipped = Stream.SkipBlock()) + return joinErrors(std::move(Err), std::move(Skipped)); return Err; } continue; @@ -605,18 +608,34 @@ ClangDocBitcodeReader::skipUntilRecordOrBlock(unsigned &BlockOrRecordID) { BlockOrRecordID = 0; while (!Stream.AtEndOfStream()) { - unsigned Code = Stream.ReadCode(); + Expected MaybeCode = Stream.ReadCode(); + if (!MaybeCode) { + // FIXME this drops the error on the floor. + consumeError(MaybeCode.takeError()); + return Cursor::BadBlock; + } - switch ((llvm::bitc::FixedAbbrevIDs)Code) { + // FIXME check that the enum is in range. + auto Code = static_cast(MaybeCode.get()); + + switch (Code) { case llvm::bitc::ENTER_SUBBLOCK: - BlockOrRecordID = Stream.ReadSubBlockID(); + if (Expected MaybeID = Stream.ReadSubBlockID()) + BlockOrRecordID = MaybeID.get(); + else { + // FIXME this drops the error on the floor. + consumeError(MaybeID.takeError()); + } return Cursor::BlockBegin; case llvm::bitc::END_BLOCK: if (Stream.ReadBlockEnd()) return Cursor::BadBlock; return Cursor::BlockEnd; case llvm::bitc::DEFINE_ABBREV: - Stream.ReadAbbrevRecord(); + if (llvm::Error Err = Stream.ReadAbbrevRecord()) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); + } continue; case llvm::bitc::UNABBREV_RECORD: return Cursor::BadBlock; @@ -634,17 +653,24 @@ llvm::Error ClangDocBitcodeReader::validateStream() { llvm::inconvertibleErrorCode()); // Sniff for the signature. - if (Stream.Read(8) != BitCodeConstants::Signature[0] || - Stream.Read(8) != BitCodeConstants::Signature[1] || - Stream.Read(8) != BitCodeConstants::Signature[2] || - Stream.Read(8) != BitCodeConstants::Signature[3]) - return llvm::make_error("Invalid bitcode signature.\n", - llvm::inconvertibleErrorCode()); + for (int Idx = 0; Idx != 4; ++Idx) { + Expected MaybeRead = Stream.Read(8); + if (!MaybeRead) + return MaybeRead.takeError(); + else if (MaybeRead.get() != BitCodeConstants::Signature[Idx]) + return llvm::make_error( + "Invalid bitcode signature.\n", llvm::inconvertibleErrorCode()); + } return llvm::Error::success(); } llvm::Error ClangDocBitcodeReader::readBlockInfoBlock() { - BlockInfo = Stream.ReadBlockInfoBlock(); + Expected> MaybeBlockInfo = + Stream.ReadBlockInfoBlock(); + if (!MaybeBlockInfo) + return MaybeBlockInfo.takeError(); + else + BlockInfo = MaybeBlockInfo.get(); if (!BlockInfo) return llvm::make_error( "Unable to parse BlockInfoBlock.\n", llvm::inconvertibleErrorCode()); @@ -687,11 +713,16 @@ ClangDocBitcodeReader::readBitcode() { // Read the top level blocks. while (!Stream.AtEndOfStream()) { - unsigned Code = Stream.ReadCode(); - if (Code != llvm::bitc::ENTER_SUBBLOCK) + Expected MaybeCode = Stream.ReadCode(); + if (!MaybeCode) + return MaybeCode.takeError(); + if (MaybeCode.get() != llvm::bitc::ENTER_SUBBLOCK) return llvm::make_error( "No blocks in input.\n", llvm::inconvertibleErrorCode()); - unsigned ID = Stream.ReadSubBlockID(); + Expected MaybeID = Stream.ReadSubBlockID(); + if (!MaybeID) + return MaybeID.takeError(); + unsigned ID = MaybeID.get(); switch (ID) { // NamedType and Comment blocks should not appear at the top level case BI_TYPE_BLOCK_ID: @@ -720,8 +751,11 @@ ClangDocBitcodeReader::readBitcode() { return std::move(Err); continue; default: - if (!Stream.SkipBlock()) - continue; + if (llvm::Error Err = Stream.SkipBlock()) { + // FIXME this drops the error on the floor. + consumeError(std::move(Err)); + } + continue; } } return std::move(Infos); -- cgit v1.2.3