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

llvm-svn: 364464
This commit is contained in:
JF Bastien
2019-06-26 19:50:12 +00:00
parent d34bff7f41
commit ec871141b2
9 changed files with 936 additions and 420 deletions
+136 -53
View File
@@ -674,8 +674,12 @@ MetadataLoader::MetadataLoaderImpl::lazyLoadModuleMetadataBlock() {
SmallVector<uint64_t, 64> Record;
// Get the abbrevs, and preload record positions to make them lazy-loadable.
while (true) {
BitstreamEntry Entry = IndexCursor.advanceSkippingSubblocks(
Expected<BitstreamEntry> MaybeEntry = IndexCursor.advanceSkippingSubblocks(
BitstreamCursor::AF_DontPopBlockAtEnd);
if (!MaybeEntry)
return MaybeEntry.takeError();
BitstreamEntry Entry = MaybeEntry.get();
switch (Entry.Kind) {
case BitstreamEntry::SubBlock: // Handled for us already.
case BitstreamEntry::Error:
@@ -687,14 +691,22 @@ MetadataLoader::MetadataLoaderImpl::lazyLoadModuleMetadataBlock() {
// The interesting case.
++NumMDRecordLoaded;
uint64_t CurrentPos = IndexCursor.GetCurrentBitNo();
auto Code = IndexCursor.skipRecord(Entry.ID);
Expected<unsigned> MaybeCode = IndexCursor.skipRecord(Entry.ID);
if (!MaybeCode)
return MaybeCode.takeError();
unsigned Code = MaybeCode.get();
switch (Code) {
case bitc::METADATA_STRINGS: {
// Rewind and parse the strings.
IndexCursor.JumpToBit(CurrentPos);
if (Error Err = IndexCursor.JumpToBit(CurrentPos))
return std::move(Err);
StringRef Blob;
Record.clear();
IndexCursor.readRecord(Entry.ID, Record, &Blob);
if (Expected<unsigned> MaybeRecord =
IndexCursor.readRecord(Entry.ID, Record, &Blob))
;
else
return MaybeRecord.takeError();
unsigned NumStrings = Record[0];
MDStringRef.reserve(NumStrings);
auto IndexNextMDString = [&](StringRef Str) {
@@ -707,26 +719,37 @@ MetadataLoader::MetadataLoaderImpl::lazyLoadModuleMetadataBlock() {
case bitc::METADATA_INDEX_OFFSET: {
// This is the offset to the index, when we see this we skip all the
// records and load only an index to these.
IndexCursor.JumpToBit(CurrentPos);
if (Error Err = IndexCursor.JumpToBit(CurrentPos))
return std::move(Err);
Record.clear();
IndexCursor.readRecord(Entry.ID, Record);
if (Expected<unsigned> MaybeRecord =
IndexCursor.readRecord(Entry.ID, Record))
;
else
return MaybeRecord.takeError();
if (Record.size() != 2)
return error("Invalid record");
auto Offset = Record[0] + (Record[1] << 32);
auto BeginPos = IndexCursor.GetCurrentBitNo();
IndexCursor.JumpToBit(BeginPos + Offset);
Entry = IndexCursor.advanceSkippingSubblocks(
BitstreamCursor::AF_DontPopBlockAtEnd);
if (Error Err = IndexCursor.JumpToBit(BeginPos + Offset))
return std::move(Err);
Expected<BitstreamEntry> MaybeEntry =
IndexCursor.advanceSkippingSubblocks(
BitstreamCursor::AF_DontPopBlockAtEnd);
if (!MaybeEntry)
return MaybeEntry.takeError();
Entry = MaybeEntry.get();
assert(Entry.Kind == BitstreamEntry::Record &&
"Corrupted bitcode: Expected `Record` when trying to find the "
"Metadata index");
Record.clear();
auto Code = IndexCursor.readRecord(Entry.ID, Record);
(void)Code;
assert(Code == bitc::METADATA_INDEX && "Corrupted bitcode: Expected "
"`METADATA_INDEX` when trying "
"to find the Metadata index");
if (Expected<unsigned> MaybeCode =
IndexCursor.readRecord(Entry.ID, Record))
assert(MaybeCode.get() == bitc::METADATA_INDEX &&
"Corrupted bitcode: Expected `METADATA_INDEX` when trying to "
"find the Metadata index");
else
return MaybeCode.takeError();
// Delta unpack
auto CurrentValue = BeginPos;
GlobalMetadataBitPosIndex.reserve(Record.size());
@@ -742,21 +765,33 @@ MetadataLoader::MetadataLoaderImpl::lazyLoadModuleMetadataBlock() {
return error("Corrupted Metadata block");
case bitc::METADATA_NAME: {
// Named metadata need to be materialized now and aren't deferred.
IndexCursor.JumpToBit(CurrentPos);
if (Error Err = IndexCursor.JumpToBit(CurrentPos))
return std::move(Err);
Record.clear();
unsigned Code = IndexCursor.readRecord(Entry.ID, Record);
assert(Code == bitc::METADATA_NAME);
unsigned Code;
if (Expected<unsigned> MaybeCode =
IndexCursor.readRecord(Entry.ID, Record)) {
Code = MaybeCode.get();
assert(Code == bitc::METADATA_NAME);
} else
return MaybeCode.takeError();
// Read name of the named metadata.
SmallString<8> Name(Record.begin(), Record.end());
Code = IndexCursor.ReadCode();
if (Expected<unsigned> MaybeCode = IndexCursor.ReadCode())
Code = MaybeCode.get();
else
return MaybeCode.takeError();
// Named Metadata comes in two parts, we expect the name to be followed
// by the node
Record.clear();
unsigned NextBitCode = IndexCursor.readRecord(Code, Record);
assert(NextBitCode == bitc::METADATA_NAMED_NODE);
(void)NextBitCode;
if (Expected<unsigned> MaybeNextBitCode =
IndexCursor.readRecord(Code, Record))
assert(MaybeNextBitCode.get() == bitc::METADATA_NAMED_NODE);
else
return MaybeNextBitCode.takeError();
// Read named metadata elements.
unsigned Size = Record.size();
@@ -775,9 +810,14 @@ MetadataLoader::MetadataLoaderImpl::lazyLoadModuleMetadataBlock() {
case bitc::METADATA_GLOBAL_DECL_ATTACHMENT: {
// FIXME: we need to do this early because we don't materialize global
// value explicitly.
IndexCursor.JumpToBit(CurrentPos);
if (Error Err = IndexCursor.JumpToBit(CurrentPos))
return std::move(Err);
Record.clear();
IndexCursor.readRecord(Entry.ID, Record);
if (Expected<unsigned> MaybeRecord =
IndexCursor.readRecord(Entry.ID, Record))
;
else
return MaybeRecord.takeError();
if (Record.size() % 2 == 0)
return error("Invalid record");
unsigned ValueID = Record[0];
@@ -845,8 +885,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
// skip the whole block in case we lazy-load.
auto EntryPos = Stream.GetCurrentBitNo();
if (Stream.EnterSubBlock(bitc::METADATA_BLOCK_ID))
return error("Invalid record");
if (Error Err = Stream.EnterSubBlock(bitc::METADATA_BLOCK_ID))
return Err;
SmallVector<uint64_t, 64> Record;
PlaceholderQueue Placeholders;
@@ -871,9 +911,14 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
// Return at the beginning of the block, since it is easy to skip it
// entirely from there.
Stream.ReadBlockEnd(); // Pop the abbrev block context.
Stream.JumpToBit(EntryPos);
if (Stream.SkipBlock())
return error("Invalid record");
if (Error Err = IndexCursor.JumpToBit(EntryPos))
return Err;
if (Error Err = Stream.SkipBlock()) {
// FIXME this drops the error on the floor, which
// ThinLTO/X86/debuginfo-cu-import.ll relies on.
consumeError(std::move(Err));
return Error::success();
}
return Error::success();
}
// Couldn't load an index, fallback to loading all the block "old-style".
@@ -883,7 +928,10 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
// Read all the records.
while (true) {
BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
if (!MaybeEntry)
return MaybeEntry.takeError();
BitstreamEntry Entry = MaybeEntry.get();
switch (Entry.Kind) {
case BitstreamEntry::SubBlock: // Handled for us already.
@@ -902,10 +950,13 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
Record.clear();
StringRef Blob;
++NumMDRecordLoaded;
unsigned Code = Stream.readRecord(Entry.ID, Record, &Blob);
if (Error Err =
parseOneMetadata(Record, Code, Placeholders, Blob, NextMetadataNo))
return Err;
if (Expected<unsigned> MaybeCode =
Stream.readRecord(Entry.ID, Record, &Blob)) {
if (Error Err = parseOneMetadata(Record, MaybeCode.get(), Placeholders,
Blob, NextMetadataNo))
return Err;
} else
return MaybeCode.takeError();
}
}
@@ -930,12 +981,25 @@ void MetadataLoader::MetadataLoaderImpl::lazyLoadOneMetadata(
}
SmallVector<uint64_t, 64> Record;
StringRef Blob;
IndexCursor.JumpToBit(GlobalMetadataBitPosIndex[ID - MDStringRef.size()]);
auto Entry = IndexCursor.advanceSkippingSubblocks();
if (Error Err = IndexCursor.JumpToBit(
GlobalMetadataBitPosIndex[ID - MDStringRef.size()]))
report_fatal_error("lazyLoadOneMetadata failed jumping: " +
toString(std::move(Err)));
Expected<BitstreamEntry> MaybeEntry = IndexCursor.advanceSkippingSubblocks();
if (!MaybeEntry)
// FIXME this drops the error on the floor.
report_fatal_error("lazyLoadOneMetadata failed advanceSkippingSubblocks: " +
toString(MaybeEntry.takeError()));
BitstreamEntry Entry = MaybeEntry.get();
++NumMDRecordLoaded;
unsigned Code = IndexCursor.readRecord(Entry.ID, Record, &Blob);
if (Error Err = parseOneMetadata(Record, Code, Placeholders, Blob, ID))
report_fatal_error("Can't lazyload MD");
if (Expected<unsigned> MaybeCode =
IndexCursor.readRecord(Entry.ID, Record, &Blob)) {
if (Error Err =
parseOneMetadata(Record, MaybeCode.get(), Placeholders, Blob, ID))
report_fatal_error("Can't lazyload MD, parseOneMetadata: " +
toString(std::move(Err)));
} else
report_fatal_error("Can't lazyload MD: " + toString(MaybeCode.takeError()));
}
/// Ensure that all forward-references and placeholders are resolved.
@@ -1032,12 +1096,17 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
// Read name of the named metadata.
SmallString<8> Name(Record.begin(), Record.end());
Record.clear();
Code = Stream.ReadCode();
Expected<unsigned> MaybeCode = Stream.ReadCode();
if (!MaybeCode)
return MaybeCode.takeError();
Code = MaybeCode.get();
++NumMDRecordLoaded;
unsigned NextBitCode = Stream.readRecord(Code, Record);
if (NextBitCode != bitc::METADATA_NAMED_NODE)
return error("METADATA_NAME not followed by METADATA_NAMED_NODE");
if (Expected<unsigned> MaybeNextBitCode = Stream.readRecord(Code, Record)) {
if (MaybeNextBitCode.get() != bitc::METADATA_NAMED_NODE)
return error("METADATA_NAME not followed by METADATA_NAMED_NODE");
} else
return MaybeNextBitCode.takeError();
// Read named metadata elements.
unsigned Size = Record.size();
@@ -1863,7 +1932,10 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadataStrings(
if (R.AtEndOfStream())
return error("Invalid record: metadata strings bad length");
unsigned Size = R.ReadVBR(6);
Expected<uint32_t> MaybeSize = R.ReadVBR(6);
if (!MaybeSize)
return MaybeSize.takeError();
uint32_t Size = MaybeSize.get();
if (Strings.size() < Size)
return error("Invalid record: metadata strings truncated chars");
@@ -1892,14 +1964,17 @@ Error MetadataLoader::MetadataLoaderImpl::parseGlobalObjectAttachment(
/// Parse metadata attachments.
Error MetadataLoader::MetadataLoaderImpl::parseMetadataAttachment(
Function &F, const SmallVectorImpl<Instruction *> &InstructionList) {
if (Stream.EnterSubBlock(bitc::METADATA_ATTACHMENT_ID))
return error("Invalid record");
if (Error Err = Stream.EnterSubBlock(bitc::METADATA_ATTACHMENT_ID))
return Err;
SmallVector<uint64_t, 64> Record;
PlaceholderQueue Placeholders;
while (true) {
BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
if (!MaybeEntry)
return MaybeEntry.takeError();
BitstreamEntry Entry = MaybeEntry.get();
switch (Entry.Kind) {
case BitstreamEntry::SubBlock: // Handled for us already.
@@ -1916,7 +1991,10 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadataAttachment(
// Read a metadata attachment record.
Record.clear();
++NumMDRecordLoaded;
switch (Stream.readRecord(Entry.ID, Record)) {
Expected<unsigned> MaybeRecord = Stream.readRecord(Entry.ID, Record);
if (!MaybeRecord)
return MaybeRecord.takeError();
switch (MaybeRecord.get()) {
default: // Default behavior: ignore.
break;
case bitc::METADATA_ATTACHMENT: {
@@ -1990,14 +2068,17 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadataKindRecord(
/// Parse the metadata kinds out of the METADATA_KIND_BLOCK.
Error MetadataLoader::MetadataLoaderImpl::parseMetadataKinds() {
if (Stream.EnterSubBlock(bitc::METADATA_KIND_BLOCK_ID))
return error("Invalid record");
if (Error Err = Stream.EnterSubBlock(bitc::METADATA_KIND_BLOCK_ID))
return Err;
SmallVector<uint64_t, 64> Record;
// Read all the records.
while (true) {
BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
Expected<BitstreamEntry> MaybeEntry = Stream.advanceSkippingSubblocks();
if (!MaybeEntry)
return MaybeEntry.takeError();
BitstreamEntry Entry = MaybeEntry.get();
switch (Entry.Kind) {
case BitstreamEntry::SubBlock: // Handled for us already.
@@ -2013,8 +2094,10 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadataKinds() {
// Read a record.
Record.clear();
++NumMDRecordLoaded;
unsigned Code = Stream.readRecord(Entry.ID, Record);
switch (Code) {
Expected<unsigned> MaybeCode = Stream.readRecord(Entry.ID, Record);
if (!MaybeCode)
return MaybeCode.takeError();
switch (MaybeCode.get()) {
default: // Default behavior: ignore.
break;
case bitc::METADATA_KIND: {