Skip to content

Commit

Permalink
[BitstreamReader] Handle errors more gracefully
Browse files Browse the repository at this point in the history
Use proper error reporting instead of report_fatal_error().
  • Loading branch information
nikic committed Feb 4, 2022
1 parent f392e9d commit c00ef03
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 14 deletions.
4 changes: 4 additions & 0 deletions llvm/include/llvm/Bitstream/BitCodes.h
Expand Up @@ -106,6 +106,10 @@ class BitCodeAbbrevOp {
Blob = 5 // 32-bit aligned array of 8-bit characters.
};

static bool isValidEncoding(uint64_t E) {
return E >= 1 && E <= 5;
}

explicit BitCodeAbbrevOp(uint64_t V) : Val(V), IsLiteral(true) {}
explicit BitCodeAbbrevOp(Encoding E, uint64_t Data = 0)
: Val(Data), IsLiteral(false), Enc(E) {}
Expand Down
22 changes: 14 additions & 8 deletions llvm/lib/Bitstream/Reader/BitstreamReader.cpp
Expand Up @@ -16,6 +16,10 @@ using namespace llvm;
//===----------------------------------------------------------------------===//
// BitstreamCursor implementation
//===----------------------------------------------------------------------===//
//
static Error error(const char *Message) {
return createStringError(std::errc::illegal_byte_sequence, Message);
}

/// Having read the ENTER_SUBBLOCK abbrevid, enter the block.
Error BitstreamCursor::EnterSubBlock(unsigned BlockID, unsigned *NumWordsP) {
Expand Down Expand Up @@ -152,7 +156,7 @@ Expected<unsigned> BitstreamCursor::skipRecord(unsigned AbbrevID) {
// Decode the value as we are commanded.
switch (EltEnc.getEncoding()) {
default:
report_fatal_error("Array element type can't be an Array or a Blob");
return error("Array element type can't be an Array or a Blob");
case BitCodeAbbrevOp::Fixed:
assert((unsigned)EltEnc.getEncodingData() <= MaxChunkSize);
if (Error Err =
Expand Down Expand Up @@ -235,7 +239,7 @@ Expected<unsigned> BitstreamCursor::readRecord(unsigned AbbrevID,
else {
if (CodeOp.getEncoding() == BitCodeAbbrevOp::Array ||
CodeOp.getEncoding() == BitCodeAbbrevOp::Blob)
report_fatal_error("Abbreviation starts with an Array or a Blob");
return error("Abbreviation starts with an Array or a Blob");
if (Expected<uint64_t> MaybeCode = readAbbreviatedField(*this, CodeOp))
Code = MaybeCode.get();
else
Expand Down Expand Up @@ -268,16 +272,16 @@ Expected<unsigned> BitstreamCursor::readRecord(unsigned AbbrevID,

// Get the element encoding.
if (i + 2 != e)
report_fatal_error("Array op not second to last");
return error("Array op not second to last");
const BitCodeAbbrevOp &EltEnc = Abbv->getOperandInfo(++i);
if (!EltEnc.isEncoding())
report_fatal_error(
return error(
"Array element type has to be an encoding of a type");

// Read all the elements.
switch (EltEnc.getEncoding()) {
default:
report_fatal_error("Array element type can't be an Array or a Blob");
return error("Array element type can't be an Array or a Blob");
case BitCodeAbbrevOp::Fixed:
for (; NumElts; --NumElts)
if (Expected<SimpleBitstreamCursor::word_t> MaybeVal =
Expand Down Expand Up @@ -366,6 +370,9 @@ Error BitstreamCursor::ReadAbbrevRecord() {
Expected<word_t> MaybeEncoding = Read(3);
if (!MaybeEncoding)
return MaybeEncoding.takeError();
if (!BitCodeAbbrevOp::isValidEncoding(MaybeEncoding.get()))
return error("Invalid encoding");

BitCodeAbbrevOp::Encoding E =
(BitCodeAbbrevOp::Encoding)MaybeEncoding.get();
if (BitCodeAbbrevOp::hasEncodingData(E)) {
Expand All @@ -385,16 +392,15 @@ Error BitstreamCursor::ReadAbbrevRecord() {

if ((E == BitCodeAbbrevOp::Fixed || E == BitCodeAbbrevOp::VBR) &&
Data > MaxChunkSize)
report_fatal_error(
"Fixed or VBR abbrev record with size > MaxChunkData");
return error("Fixed or VBR abbrev record with size > MaxChunkData");

Abbv->Add(BitCodeAbbrevOp(E, Data));
} else
Abbv->Add(BitCodeAbbrevOp(E));
}

if (Abbv->getNumOperandInfos() == 0)
report_fatal_error("Abbrev record with no operands");
return error("Abbrev record with no operands");
CurAbbrevs.push_back(std::move(Abbv));

return Error::success();
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/Bitcode/invalid.test
@@ -1,6 +1,6 @@
RUN: not llvm-dis -disable-output %p/Inputs/invalid-empty.bc 2>&1 | \
RUN: FileCheck --check-prefix=INVALID-EMPTY %s
RUN: not --crash llvm-dis -disable-output %p/Inputs/invalid-pr20485.bc 2>&1 | \
RUN: not llvm-dis -disable-output %p/Inputs/invalid-pr20485.bc 2>&1 | \
RUN: FileCheck --check-prefix=INVALID-ENCODING %s
RUN: not llvm-dis -disable-output %p/Inputs/invalid-abbrev.bc 2>&1 | \
RUN: FileCheck --check-prefix=BAD-ABBREV %s
Expand Down Expand Up @@ -71,14 +71,14 @@ RUN: FileCheck --check-prefix=FP-SHIFT %s

FP-SHIFT: Invalid record

RUN: not --crash llvm-dis -disable-output %p/Inputs/invalid-abbrev-vbr-size-too-big.bc 2>&1 | \
RUN: not llvm-dis -disable-output %p/Inputs/invalid-abbrev-vbr-size-too-big.bc 2>&1 | \
RUN: FileCheck --check-prefix=HUGE-ABBREV-OP %s
RUN: not --crash llvm-dis -disable-output %p/Inputs/invalid-abbrev-fixed-size-too-big.bc 2>&1 | \
RUN: not llvm-dis -disable-output %p/Inputs/invalid-abbrev-fixed-size-too-big.bc 2>&1 | \
RUN: FileCheck --check-prefix=HUGE-ABBREV-OP %s

HUGE-ABBREV-OP: Fixed or VBR abbrev record with size > MaxChunkData

RUN: not --crash llvm-dis -disable-output %p/Inputs/invalid-array-type.bc 2>&1 | \
RUN: not llvm-dis -disable-output %p/Inputs/invalid-array-type.bc 2>&1 | \
RUN: FileCheck --check-prefix=ARRAY-TYPE %s

ARRAY-TYPE: Array element type can't be an Array or a Blob
Expand Down Expand Up @@ -116,7 +116,7 @@ RUN: FileCheck --check-prefix=INVALID-CAST %s

INVALID-CAST: Invalid cast

RUN: not --crash llvm-dis -disable-output %p/Inputs/invalid-array-op-not-2nd-to-last.bc 2>&1 | \
RUN: not llvm-dis -disable-output %p/Inputs/invalid-array-op-not-2nd-to-last.bc 2>&1 | \
RUN: FileCheck --check-prefix=ARRAY-NOT-2LAST %s

ARRAY-NOT-2LAST: Array op not second to last
Expand Down Expand Up @@ -176,7 +176,7 @@ RUN: FileCheck --check-prefix=INVALID-GVCOMDAT-ID %s

INVALID-GVCOMDAT-ID: Invalid global variable comdat ID

RUN: not --crash llvm-dis -disable-output %p/Inputs/invalid-abbrev-no-operands.bc 2>&1 | \
RUN: not llvm-dis -disable-output %p/Inputs/invalid-abbrev-no-operands.bc 2>&1 | \
RUN: FileCheck --check-prefix=ABBREV-NO-OPS %s

ABBREV-NO-OPS: Abbrev record with no operands
Expand Down

0 comments on commit c00ef03

Please sign in to comment.