From ea8cbe9782ff78c3d395872dee7f51384c84dc59 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Fri, 20 Feb 2015 03:17:58 +0000 Subject: [PATCH] Bitcode: Stop assuming non-null fields When writing the bitcode serialization for the new debug info hierarchy, I assumed two fields would never be null. Drop that assumption, since it's brittle (and crashes the `BitcodeWriter` if wrong), and is a check better left for the verifier anyway. (No need for a bitcode upgrade here, since the new hierarchy is still not in place.) The fields in question are `MDCompileUnit::getFile()` and `MDDerivedType::getBaseType()`, the latter of which isn't null in test/Transforms/Mem2Reg/ConvertDebugInfo2.ll (see !14, a pointer to nothing). While the testcase might have bitrotted, there's no reason for the bitcode format to rely on non-null for metadata operands. This also fixes a bug in `AsmWriter` where if the `file:` is null it isn't emitted (caught by the double-round trip in the testcase I'm adding) -- this is a required field in `LLParser`. I'll circle back to ConvertDebugInfo2. Once the specialized nodes are in place, I'll be trying to turn the debug info verifier back on by default (in the newer module pass form committed r206300) and throwing more logic in there. If the testcase has bitrotted (as opposed to me not understanding the schema correctly) I'll fix it then. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@229960 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Bitcode/Reader/BitcodeReader.cpp | 19 ++++++++++--------- lib/Bitcode/Writer/BitcodeWriter.cpp | 4 ++-- lib/IR/AsmWriter.cpp | 7 ++----- test/Assembler/metadata-null-operands.ll | 13 +++++++++++++ 4 files changed, 27 insertions(+), 16 deletions(-) create mode 100644 test/Assembler/metadata-null-operands.ll diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 649057dddf0..bb6ba6fc6e1 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -1407,8 +1407,8 @@ std::error_code BitcodeReader::ParseMetadata() { GET_OR_DISTINCT(MDDerivedType, Record[0], (Context, Record[1], getMDString(Record[2]), getMDOrNull(Record[3]), Record[4], - getMDOrNull(Record[5]), getMD(Record[6]), Record[7], - Record[8], Record[9], Record[10], + getMDOrNull(Record[5]), getMDOrNull(Record[6]), + Record[7], Record[8], Record[9], Record[10], getMDOrNull(Record[11]))), NextMDValueNo++); break; @@ -1454,13 +1454,14 @@ std::error_code BitcodeReader::ParseMetadata() { return Error("Invalid record"); MDValueList.AssignValue( - GET_OR_DISTINCT( - MDCompileUnit, Record[0], - (Context, Record[1], getMD(Record[2]), getMDString(Record[3]), - Record[4], getMDString(Record[5]), Record[6], - getMDString(Record[7]), Record[8], getMDOrNull(Record[9]), - getMDOrNull(Record[10]), getMDOrNull(Record[11]), - getMDOrNull(Record[12]), getMDOrNull(Record[13]))), + GET_OR_DISTINCT(MDCompileUnit, Record[0], + (Context, Record[1], getMDOrNull(Record[2]), + getMDString(Record[3]), Record[4], + getMDString(Record[5]), Record[6], + getMDString(Record[7]), Record[8], + getMDOrNull(Record[9]), getMDOrNull(Record[10]), + getMDOrNull(Record[11]), getMDOrNull(Record[12]), + getMDOrNull(Record[13]))), NextMDValueNo++); break; } diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index 932381926c7..ec43035b22e 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -864,7 +864,7 @@ static void WriteMDDerivedType(const MDDerivedType *N, Record.push_back(VE.getMetadataOrNullID(N->getFile())); Record.push_back(N->getLine()); Record.push_back(VE.getMetadataOrNullID(N->getScope())); - Record.push_back(VE.getMetadataID(N->getBaseType())); + Record.push_back(VE.getMetadataOrNullID(N->getBaseType())); Record.push_back(N->getSizeInBits()); Record.push_back(N->getAlignInBits()); Record.push_back(N->getOffsetInBits()); @@ -932,7 +932,7 @@ static void WriteMDCompileUnit(const MDCompileUnit *N, unsigned Abbrev) { Record.push_back(N->isDistinct()); Record.push_back(N->getSourceLanguage()); - Record.push_back(VE.getMetadataID(N->getFile())); + Record.push_back(VE.getMetadataOrNullID(N->getFile())); Record.push_back(VE.getMetadataOrNullID(N->getRawProducer())); Record.push_back(N->isOptimized()); Record.push_back(VE.getMetadataOrNullID(N->getRawFlags())); diff --git a/lib/IR/AsmWriter.cpp b/lib/IR/AsmWriter.cpp index ac86acd1741..47a1b5512a9 100644 --- a/lib/IR/AsmWriter.cpp +++ b/lib/IR/AsmWriter.cpp @@ -1515,11 +1515,8 @@ static void writeMDCompileUnit(raw_ostream &Out, const MDCompileUnit *N, Out << Lang; else Out << N->getSourceLanguage(); - if (N->getFile()) { - Out << FS << "file: "; - writeMetadataAsOperand(Out, N->getFile(), TypePrinter, Machine, - Context); - } + Out << FS << "file: "; + writeMetadataAsOperand(Out, N->getFile(), TypePrinter, Machine, Context); if (!N->getProducer().empty()) Out << FS << "producer: \"" << N->getProducer() << "\""; Out << FS << "isOptimized: " << (N->isOptimized() ? "true" : "false"); diff --git a/test/Assembler/metadata-null-operands.ll b/test/Assembler/metadata-null-operands.ll new file mode 100644 index 00000000000..acae1d4be99 --- /dev/null +++ b/test/Assembler/metadata-null-operands.ll @@ -0,0 +1,13 @@ +; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s +; RUN: verify-uselistorder %s + +; Don't crash on null operands. (If/when we add a verify check for these, we +; should disable the verifier for this test and remove this comment; the test +; is still important.) +!named = !{!0, !1} +!0 = !MDDerivedType(tag: DW_TAG_pointer_type, baseType: null) +!1 = !MDCompileUnit(language: DW_LANG_C, file: null) + +; CHECK: !named = !{!0, !1} +; CHECK: !0 = !MDDerivedType({{.*}}baseType: null{{.*}}) +; CHECK: !1 = !MDCompileUnit({{.*}}file: null{{.*}}) -- 2.34.1