[llvm] r263742 - Bitcode: Error out instead of crashing on corrupt metadata
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 17 15:11:28 PDT 2016
Yeah, if we're going to get into the business of making bitcode robust to
arbitrary errors, we really should talk about a testing strategy for it. I
imagine that'll probably look like a fuzzer (as mentioned in review) and
we'll just pull out cases from the fuzzer to check in as test cases & keep
as part of the fuzzer's seed library (whatever the right word is for that).
On Thu, Mar 17, 2016 at 1:12 PM, Justin Bogner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: bogner
> Date: Thu Mar 17 15:12:06 2016
> New Revision: 263742
>
> URL: http://llvm.org/viewvc/llvm-project?rev=263742&view=rev
> Log:
> 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.
>
> Modified:
> llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
>
> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=263742&r1=263741&r2=263742&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Thu Mar 17 15:12:06
> 2016
> @@ -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::assignVa
> --NumFwdRefs;
> }
>
> -Metadata *BitcodeReaderMetadataList::getValueFwdRef(unsigned Idx) {
> +Metadata *BitcodeReaderMetadataList::getMetadataFwdRef(unsigned Idx) {
> if (Idx >= size())
> resize(Idx + 1);
>
> @@ -1091,6 +1092,10 @@ Metadata *BitcodeReaderMetadataList::get
> return MD;
> }
>
> +MDNode *BitcodeReaderMetadataList::getMDNodeFwdRefOrNull(unsigned Idx) {
> + return dyn_cast_or_null<MDNode>(getMetadataFwdRef(Idx));
> +}
> +
> void BitcodeReaderMetadataList::tryToResolveCycles() {
> if (!AnyFwdRefs)
> // Nothing to do.
> @@ -1907,7 +1912,7 @@ std::error_code BitcodeReader::parseMeta
> SmallVector<uint64_t, 64> 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::parseMeta
> unsigned Size = Record.size();
> NamedMDNode *NMD = TheModule->getOrInsertNamedMetadata(Name);
> for (unsigned i = 0; i != Size; ++i) {
> - MDNode *MD =
> -
> dyn_cast_or_null<MDNode>(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::parseMeta
> 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::parseMeta
> SmallVector<Metadata *, 8> 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::parseMeta
>
> unsigned Line = Record[1];
> unsigned Column = Record[2];
> - MDNode *Scope =
> cast<MDNode>(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::parseMeta
> auto *Header = getMDString(Record[3]);
> SmallVector<Metadata *, 8> 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::parseMeta
> 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<MDNode>(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::parseMeta
> 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<LocalAsMetadata>(Node))
> // Drop the attachment. This used to be legal, but there's no
> // upgrade path.
> break;
> - Inst->setMetadata(I->second, cast<MDNode>(Node));
> + MDNode *MD = dyn_cast_or_null<MDNode>(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::parseFunc
> unsigned ScopeID = Record[2], IAID = Record[3];
>
> MDNode *Scope = nullptr, *IA = nullptr;
> - if (ScopeID)
> - Scope = cast<MDNode>(MetadataList.getValueFwdRef(ScopeID - 1));
> - if (IAID)
> - IA = cast<MDNode>(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;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160317/0b30fe46/attachment.html>
More information about the llvm-commits
mailing list