[clang-tools-extra] r364464 - BitStream reader: propagate errors
JF Bastien via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 26 12:50:13 PDT 2019
Author: jfb
Date: Wed Jun 26 12:50:12 2019
New Revision: 364464
URL: http://llvm.org/viewvc/llvm-project?rev=364464&view=rev
Log:
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
<rdar://problem/33159405>
Differential Revision: https://reviews.llvm.org/D63518
Modified:
clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp
Modified: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp Wed Jun 26 12:50:12 2019
@@ -491,24 +491,27 @@ template <typename T>
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<unsigned> 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<unsigned> 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 <typename T>
llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) {
- if (Stream.EnterSubBlock(ID))
- return llvm::make_error<llvm::StringError>("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::readB
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::skipUntilRecordOr
BlockOrRecordID = 0;
while (!Stream.AtEndOfStream()) {
- unsigned Code = Stream.ReadCode();
+ Expected<unsigned> 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<llvm::bitc::FixedAbbrevIDs>(MaybeCode.get());
+
+ switch (Code) {
case llvm::bitc::ENTER_SUBBLOCK:
- BlockOrRecordID = Stream.ReadSubBlockID();
+ if (Expected<unsigned> 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::valid
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<llvm::StringError>("Invalid bitcode signature.\n",
- llvm::inconvertibleErrorCode());
+ for (int Idx = 0; Idx != 4; ++Idx) {
+ Expected<llvm::SimpleBitstreamCursor::word_t> MaybeRead = Stream.Read(8);
+ if (!MaybeRead)
+ return MaybeRead.takeError();
+ else if (MaybeRead.get() != BitCodeConstants::Signature[Idx])
+ return llvm::make_error<llvm::StringError>(
+ "Invalid bitcode signature.\n", llvm::inconvertibleErrorCode());
+ }
return llvm::Error::success();
}
llvm::Error ClangDocBitcodeReader::readBlockInfoBlock() {
- BlockInfo = Stream.ReadBlockInfoBlock();
+ Expected<Optional<llvm::BitstreamBlockInfo>> MaybeBlockInfo =
+ Stream.ReadBlockInfoBlock();
+ if (!MaybeBlockInfo)
+ return MaybeBlockInfo.takeError();
+ else
+ BlockInfo = MaybeBlockInfo.get();
if (!BlockInfo)
return llvm::make_error<llvm::StringError>(
"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<unsigned> MaybeCode = Stream.ReadCode();
+ if (!MaybeCode)
+ return MaybeCode.takeError();
+ if (MaybeCode.get() != llvm::bitc::ENTER_SUBBLOCK)
return llvm::make_error<llvm::StringError>(
"No blocks in input.\n", llvm::inconvertibleErrorCode());
- unsigned ID = Stream.ReadSubBlockID();
+ Expected<unsigned> 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);
Modified: clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp Wed Jun 26 12:50:12 2019
@@ -214,7 +214,7 @@ static const std::vector<std::pair<Block
// AbbreviationMap
-constexpr char BitCodeConstants::Signature[];
+constexpr unsigned char BitCodeConstants::Signature[];
void ClangDocBitcodeWriter::AbbreviationMap::add(RecordId RID,
unsigned AbbrevID) {
Modified: clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/BitcodeWriter.h?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.h (original)
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.h Wed Jun 26 12:50:12 2019
@@ -44,7 +44,7 @@ struct BitCodeConstants {
static constexpr unsigned ReferenceTypeSize = 8U;
static constexpr unsigned USRLengthSize = 6U;
static constexpr unsigned USRBitLengthSize = 8U;
- static constexpr char Signature[4] = {'D', 'O', 'C', 'S'};
+ static constexpr unsigned char Signature[4] = {'D', 'O', 'C', 'S'};
static constexpr int USRHashSize = 20;
};
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Wed Jun 26 12:50:12 2019
@@ -422,8 +422,9 @@ ParsedAST::build(std::unique_ptr<Compile
// Collect tokens of the main file.
syntax::TokenCollector Tokens(Clang->getPreprocessor());
- if (!Action->Execute())
- log("Execute() failed when building AST for {0}", MainInput.getFile());
+ if (llvm::Error Err = Action->Execute())
+ log("Execute() failed when building AST for {0}: {1}", MainInput.getFile(),
+ toString(std::move(Err)));
std::vector<Decl *> ParsedDecls = Action->takeTopLevelDecls();
// AST traversals should exclude the preamble, to avoid performance cliffs.
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Jun 26 12:50:12 2019
@@ -1105,8 +1105,9 @@ bool semaCodeComplete(std::unique_ptr<Co
if (Includes)
Clang->getPreprocessor().addPPCallbacks(
collectIncludeStructureCallback(Clang->getSourceManager(), Includes));
- if (!Action.Execute()) {
- log("Execute() failed when running codeComplete for {0}", Input.FileName);
+ if (llvm::Error Err = Action.Execute()) {
+ log("Execute() failed when running codeComplete for {0}: {1}",
+ Input.FileName, toString(std::move(Err)));
return false;
}
Action.EndSourceFile();
Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Wed Jun 26 12:50:12 2019
@@ -456,9 +456,9 @@ llvm::Error BackgroundIndex::index(tooli
if (!Action->BeginSourceFile(*Clang, Input))
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"BeginSourceFile() failed");
- if (!Action->Execute())
- return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "Execute() failed");
+ if (llvm::Error Err = Action->Execute())
+ return Err;
+
Action->EndSourceFile();
if (Clang->hasDiagnostics() &&
Clang->getDiagnostics().hasUncompilableErrorOccurred()) {
Modified: clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp?rev=364464&r1=364463&r2=364464&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp Wed Jun 26 12:50:12 2019
@@ -68,7 +68,7 @@ protected:
IncludeStructure Includes;
Clang->getPreprocessor().addPPCallbacks(
collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));
- EXPECT_TRUE(Action.Execute());
+ EXPECT_FALSE(Action.Execute());
Action.EndSourceFile();
return Includes;
}
More information about the cfe-commits
mailing list