Bitcode: Stop assuming non-null fields
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Fri, 20 Feb 2015 03:17:58 +0000 (03:17 +0000)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Fri, 20 Feb 2015 03:17:58 +0000 (03:17 +0000)
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
lib/Bitcode/Writer/BitcodeWriter.cpp
lib/IR/AsmWriter.cpp
test/Assembler/metadata-null-operands.ll [new file with mode: 0644]

index 649057dddf08d5f478d894f75eee53d5cdb3f1a5..bb6ba6fc6e1c2daa4c2dfd5bb1c72fa720779e6b 100644 (file)
@@ -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;
     }
index 932381926c7766a4c2c857f8187945280f6209f3..ec43035b22ec8a7af91128e458cd6692bc69a1db 100644 (file)
@@ -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()));
index ac86acd17417ac3ce625d8f6e24e4b95fc49eda1..47a1b5512a99496802efe191f18bceb8efef1a3a 100644 (file)
@@ -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 (file)
index 0000000..acae1d4
--- /dev/null
@@ -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{{.*}})