From ae341c6e9b996cbbe8631ce5f0e26168487f71bf Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Thu, 17 Mar 2016 20:12:06 +0000 Subject: [PATCH] Bitcode: Error out instead of crashing on corrupt metadata I hit a crash in the bitcode reader on some corrupt input where an MDString had somehow been attached to an instruction instead of an MDNode. This input is pretty bogus, but we shouldn't be crashing on bad input here. This change adds error handling in all of the places where we currently have unchecked casts from Metadata to MDNode, which means we'll error out instead of crashing for that sort of input. Unfortunately, I don't have tests. Hitting this requires flipping bits in the input bitcode, and committing corrupt binary files to catch these cases is a bit too opaque and unmaintainable. llvm-svn: 263742 --- llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 58 +++++++++++++++-------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index fd9a5e5c5ef9..dcaaa7d29753 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -126,7 +126,8 @@ public: MetadataPtrs.resize(N); } - Metadata *getValueFwdRef(unsigned Idx); + Metadata *getMetadataFwdRef(unsigned Idx); + MDNode *getMDNodeFwdRefOrNull(unsigned Idx); void assignValue(Metadata *MD, unsigned Idx); void tryToResolveCycles(); }; @@ -295,7 +296,7 @@ private: return ValueList.getValueFwdRef(ID, Ty); } Metadata *getFnMetadataByID(unsigned ID) { - return MetadataList.getValueFwdRef(ID); + return MetadataList.getMetadataFwdRef(ID); } BasicBlock *getBasicBlock(unsigned ID) const { if (ID >= FunctionBBs.size()) return nullptr; // Invalid ID @@ -1068,7 +1069,7 @@ void BitcodeReaderMetadataList::assignValue(Metadata *MD, unsigned Idx) { --NumFwdRefs; } -Metadata *BitcodeReaderMetadataList::getValueFwdRef(unsigned Idx) { +Metadata *BitcodeReaderMetadataList::getMetadataFwdRef(unsigned Idx) { if (Idx >= size()) resize(Idx + 1); @@ -1091,6 +1092,10 @@ Metadata *BitcodeReaderMetadataList::getValueFwdRef(unsigned Idx) { return MD; } +MDNode *BitcodeReaderMetadataList::getMDNodeFwdRefOrNull(unsigned Idx) { + return dyn_cast_or_null(getMetadataFwdRef(Idx)); +} + void BitcodeReaderMetadataList::tryToResolveCycles() { if (!AnyFwdRefs) // Nothing to do. @@ -1907,7 +1912,7 @@ std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) { SmallVector Record; auto getMD = [&](unsigned ID) -> Metadata * { - return MetadataList.getValueFwdRef(ID); + return MetadataList.getMetadataFwdRef(ID); }; auto getMDOrNull = [&](unsigned ID) -> Metadata *{ if (ID) @@ -1963,8 +1968,7 @@ std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) { unsigned Size = Record.size(); NamedMDNode *NMD = TheModule->getOrInsertNamedMetadata(Name); for (unsigned i = 0; i != Size; ++i) { - MDNode *MD = - dyn_cast_or_null(MetadataList.getValueFwdRef(Record[i])); + MDNode *MD = MetadataList.getMDNodeFwdRefOrNull(Record[i]); if (!MD) return error("Invalid record"); NMD->addOperand(MD); @@ -2011,7 +2015,7 @@ std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) { if (!Ty) return error("Invalid record"); if (Ty->isMetadataTy()) - Elts.push_back(MetadataList.getValueFwdRef(Record[i + 1])); + Elts.push_back(MetadataList.getMetadataFwdRef(Record[i + 1])); else if (!Ty->isVoidTy()) { auto *MD = ValueAsMetadata::get(ValueList.getValueFwdRef(Record[i + 1], Ty)); @@ -2044,7 +2048,7 @@ std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) { SmallVector Elts; Elts.reserve(Record.size()); for (unsigned ID : Record) - Elts.push_back(ID ? MetadataList.getValueFwdRef(ID - 1) : nullptr); + Elts.push_back(ID ? MetadataList.getMetadataFwdRef(ID - 1) : nullptr); MetadataList.assignValue(IsDistinct ? MDNode::getDistinct(Context, Elts) : MDNode::get(Context, Elts), NextMetadataNo++); @@ -2056,9 +2060,11 @@ std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) { unsigned Line = Record[1]; unsigned Column = Record[2]; - MDNode *Scope = cast(MetadataList.getValueFwdRef(Record[3])); + MDNode *Scope = MetadataList.getMDNodeFwdRefOrNull(Record[3]); + if (!Scope) + return error("Invalid record"); Metadata *InlinedAt = - Record[4] ? MetadataList.getValueFwdRef(Record[4] - 1) : nullptr; + Record[4] ? MetadataList.getMetadataFwdRef(Record[4] - 1) : nullptr; MetadataList.assignValue( GET_OR_DISTINCT(DILocation, Record[0], (Context, Line, Column, Scope, InlinedAt)), @@ -2078,8 +2084,9 @@ std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) { auto *Header = getMDString(Record[3]); SmallVector DwarfOps; for (unsigned I = 4, E = Record.size(); I != E; ++I) - DwarfOps.push_back( - Record[I] ? MetadataList.getValueFwdRef(Record[I] - 1) : nullptr); + DwarfOps.push_back(Record[I] + ? MetadataList.getMetadataFwdRef(Record[I] - 1) + : nullptr); MetadataList.assignValue( GET_OR_DISTINCT(GenericDINode, Record[0], (Context, Tag, Header, DwarfOps)), @@ -3925,8 +3932,10 @@ std::error_code BitcodeReader::parseMetadataAttachment(Function &F) { auto K = MDKindMap.find(Record[I]); if (K == MDKindMap.end()) return error("Invalid ID"); - Metadata *MD = MetadataList.getValueFwdRef(Record[I + 1]); - F.setMetadata(K->second, cast(MD)); + MDNode *MD = MetadataList.getMDNodeFwdRefOrNull(Record[I + 1]); + if (!MD) + return error("Invalid metadata attachment"); + F.setMetadata(K->second, MD); } continue; } @@ -3939,12 +3948,15 @@ std::error_code BitcodeReader::parseMetadataAttachment(Function &F) { MDKindMap.find(Kind); if (I == MDKindMap.end()) return error("Invalid ID"); - Metadata *Node = MetadataList.getValueFwdRef(Record[i + 1]); + Metadata *Node = MetadataList.getMetadataFwdRef(Record[i + 1]); if (isa(Node)) // Drop the attachment. This used to be legal, but there's no // upgrade path. break; - Inst->setMetadata(I->second, cast(Node)); + MDNode *MD = dyn_cast_or_null(Node); + if (!MD) + return error("Invalid metadata attachment"); + Inst->setMetadata(I->second, MD); if (I->second == LLVMContext::MD_tbaa) InstsWithTBAATag.push_back(Inst); } @@ -4105,10 +4117,16 @@ std::error_code BitcodeReader::parseFunctionBody(Function *F) { unsigned ScopeID = Record[2], IAID = Record[3]; MDNode *Scope = nullptr, *IA = nullptr; - if (ScopeID) - Scope = cast(MetadataList.getValueFwdRef(ScopeID - 1)); - if (IAID) - IA = cast(MetadataList.getValueFwdRef(IAID - 1)); + if (ScopeID) { + Scope = MetadataList.getMDNodeFwdRefOrNull(ScopeID - 1); + if (!Scope) + return error("Invalid record"); + } + if (IAID) { + IA = MetadataList.getMDNodeFwdRefOrNull(IAID - 1); + if (!IA) + return error("Invalid record"); + } LastLoc = DebugLoc::get(Line, Col, Scope, IA); I->setDebugLoc(LastLoc); I = nullptr;