Skip to content

Commit 864474c

Browse files
fhahnfhahn
committedJul 14, 2019
[BitcodeReader] Use tighter upper bound to validate forward references.
At the moment, bitcode files with invalid forward reference can easily cause the bitcode reader to run out of memory, by creating a forward reference with a very high index. We can use the size of the bitcode file as an upper bound, because a valid bitcode file can never contain more records. This should be sufficient to fail early in most cases. The only exception is large files with invalid forward references close to the file size. There are a couple of clusterfuzz runs that fail with out-of-memory because of very high forward references and they should be fixed by this patch. A concrete example for this is D64507, which causes out-of-memory on systems with low memory, like the hexagon upstream bots. Reviewers: t.p.northover, thegameg, jfb, efriedma, hfinkel Reviewed By: jfb Differential Revision: https://reviews.llvm.org/D64577 llvm-svn: 366017
·
llvmorg-21-initllvmorg-9.0.0-rc1
1 parent f66f5ff commit 864474c

File tree

6 files changed

+38
-11
lines changed

6 files changed

+38
-11
lines changed
 

‎llvm/include/llvm/Bitstream/BitstreamReader.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,9 @@ class SimpleBitstreamCursor {
294294
BitsInCurWord = 0;
295295
}
296296

297+
/// Return the size of the stream in bytes.
298+
size_t SizeInBytes() const { return BitcodeBytes.size(); }
299+
297300
/// Skip to the end of the file.
298301
void skipToEnd() { NextChar = BitcodeBytes.size(); }
299302
};
@@ -364,17 +367,18 @@ class BitstreamCursor : SimpleBitstreamCursor {
364367
explicit BitstreamCursor(MemoryBufferRef BitcodeBytes)
365368
: SimpleBitstreamCursor(BitcodeBytes) {}
366369

367-
using SimpleBitstreamCursor::canSkipToPos;
368370
using SimpleBitstreamCursor::AtEndOfStream;
371+
using SimpleBitstreamCursor::canSkipToPos;
372+
using SimpleBitstreamCursor::fillCurWord;
369373
using SimpleBitstreamCursor::getBitcodeBytes;
370374
using SimpleBitstreamCursor::GetCurrentBitNo;
371375
using SimpleBitstreamCursor::getCurrentByteNo;
372376
using SimpleBitstreamCursor::getPointerToByte;
373377
using SimpleBitstreamCursor::JumpToBit;
374-
using SimpleBitstreamCursor::fillCurWord;
375378
using SimpleBitstreamCursor::Read;
376379
using SimpleBitstreamCursor::ReadVBR;
377380
using SimpleBitstreamCursor::ReadVBR64;
381+
using SimpleBitstreamCursor::SizeInBytes;
378382

379383
/// Return the number of bits used to encode an abbrev #.
380384
unsigned getAbbrevIDWidth() const { return CurCodeSize; }

‎llvm/lib/Bitcode/Reader/BitcodeReader.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ BitcodeReader::BitcodeReader(BitstreamCursor Stream, StringRef Strtab,
858858
StringRef ProducerIdentification,
859859
LLVMContext &Context)
860860
: BitcodeReaderBase(std::move(Stream), Strtab), Context(Context),
861-
ValueList(Context) {
861+
ValueList(Context, Stream.SizeInBytes()) {
862862
this->ProducerIdentification = ProducerIdentification;
863863
}
864864

‎llvm/lib/Bitcode/Reader/MetadataLoader.cpp

+16-4
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,15 @@ class BitcodeReaderMetadataList {
130130

131131
LLVMContext &Context;
132132

133+
/// Maximum number of valid references. Forward references exceeding the
134+
/// maximum must be invalid.
135+
unsigned RefsUpperBound;
136+
133137
public:
134-
BitcodeReaderMetadataList(LLVMContext &C) : Context(C) {}
138+
BitcodeReaderMetadataList(LLVMContext &C, size_t RefsUpperBound)
139+
: Context(C),
140+
RefsUpperBound(std::min((size_t)std::numeric_limits<unsigned>::max(),
141+
RefsUpperBound)) {}
135142

136143
// vector compatibility methods
137144
unsigned size() const { return MetadataPtrs.size(); }
@@ -218,6 +225,10 @@ void BitcodeReaderMetadataList::assignValue(Metadata *MD, unsigned Idx) {
218225
}
219226

220227
Metadata *BitcodeReaderMetadataList::getMetadataFwdRef(unsigned Idx) {
228+
// Bail out for a clearly invalid value.
229+
if (Idx >= RefsUpperBound)
230+
return nullptr;
231+
221232
if (Idx >= size())
222233
resize(Idx + 1);
223234

@@ -625,9 +636,10 @@ class MetadataLoader::MetadataLoaderImpl {
625636
BitcodeReaderValueList &ValueList,
626637
std::function<Type *(unsigned)> getTypeByID,
627638
bool IsImporting)
628-
: MetadataList(TheModule.getContext()), ValueList(ValueList),
629-
Stream(Stream), Context(TheModule.getContext()), TheModule(TheModule),
630-
getTypeByID(std::move(getTypeByID)), IsImporting(IsImporting) {}
639+
: MetadataList(TheModule.getContext(), Stream.SizeInBytes()),
640+
ValueList(ValueList), Stream(Stream), Context(TheModule.getContext()),
641+
TheModule(TheModule), getTypeByID(std::move(getTypeByID)),
642+
IsImporting(IsImporting) {}
631643

632644
Error parseMetadata(bool ModuleLevel);
633645

‎llvm/lib/Bitcode/Reader/ValueList.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ void BitcodeReaderValueList::assignValue(Value *V, unsigned Idx, Type *FullTy) {
9797
}
9898

9999
Constant *BitcodeReaderValueList::getConstantFwdRef(unsigned Idx, Type *Ty) {
100+
// Bail out for a clearly invalid value.
101+
if (Idx >= RefsUpperBound)
102+
return nullptr;
103+
100104
if (Idx >= size())
101105
resize(Idx + 1);
102106

@@ -114,8 +118,8 @@ Constant *BitcodeReaderValueList::getConstantFwdRef(unsigned Idx, Type *Ty) {
114118

115119
Value *BitcodeReaderValueList::getValueFwdRef(unsigned Idx, Type *Ty,
116120
Type **FullTy) {
117-
// Bail out for a clearly invalid value. This would make us call resize(0)
118-
if (Idx == std::numeric_limits<unsigned>::max())
121+
// Bail out for a clearly invalid value.
122+
if (Idx >= RefsUpperBound)
119123
return nullptr;
120124

121125
if (Idx >= size())

‎llvm/lib/Bitcode/Reader/ValueList.h

+8-1
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,15 @@ class BitcodeReaderValueList {
4646
ResolveConstantsTy ResolveConstants;
4747
LLVMContext &Context;
4848

49+
/// Maximum number of valid references. Forward references exceeding the
50+
/// maximum must be invalid.
51+
unsigned RefsUpperBound;
52+
4953
public:
50-
BitcodeReaderValueList(LLVMContext &C) : Context(C) {}
54+
BitcodeReaderValueList(LLVMContext &C, size_t RefsUpperBound)
55+
: Context(C),
56+
RefsUpperBound(std::min((size_t)std::numeric_limits<unsigned>::max(),
57+
RefsUpperBound)) {}
5158

5259
~BitcodeReaderValueList() {
5360
assert(ResolveConstants.empty() && "Constants not resolved?");

‎llvm/test/Bitcode/pr18704.ll

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
; RUN: not llvm-dis < %s.bc 2>&1 | FileCheck %s
22

3-
; CHECK: llvm-dis{{(\.EXE|\.exe)?}}: error: Never resolved value found in function
3+
; CHECK: llvm-dis{{(\.EXE|\.exe)?}}: error: Invalid record
44

55
; pr18704.ll.bc has an instruction referring to invalid type.
66
; The test checks that LLVM reports the error and doesn't access freed memory

0 commit comments

Comments
 (0)
Please sign in to comment.