Verifier: Move checks over from DIDescriptor::Verify()

Move over some more checks from `DIDescriptor::Verify()`, and change
`LLParser` to require non-null `file:` fields in compile units.

I've ignored the comment in test/Assembler/metadata-null-operands.ll
since I disagree with it.  At the time that test was written (r229960),
the debug info verifier wasn't on by default, so my comment there is in
the context of not expecting the verifier to be useful.  It is now, and
besides that, since r233394 we can check when parsing textual IR whether
an operand is null that shouldn't be.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@233654 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp
index dc348e4d4b73b9aef43f8a2850defb3bd1674cf6..6e79960e3db8f2e873736d6e441bb3d6b1b08006 100644 (file)
--- a/lib/AsmParser/LLParser.cpp
+++ b/lib/AsmParser/LLParser.cpp
@@ -3506,7 +3506,7 @@ bool LLParser::ParseMDFile(MDNode *&Result, bool IsDistinct) {
 bool LLParser::ParseMDCompileUnit(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   REQUIRED(language, DwarfLangField, );                                        \
-  REQUIRED(file, MDField, );                                                   \
+  REQUIRED(file, MDField, (/* AllowNull */ false));                            \
   OPTIONAL(producer, MDStringField, );                                         \
   OPTIONAL(isOptimized, MDBoolField, );                                        \
   OPTIONAL(flags, MDStringField, );                                            \

diff --git a/lib/IR/DebugInfo.cpp b/lib/IR/DebugInfo.cpp
index bc40907ba18ce66fa673bf89f79ad6d872065c59..4e5b5f1f97561b0f453a57938e68cff329bfe2a8 100644 (file)
--- a/lib/IR/DebugInfo.cpp
+++ b/lib/IR/DebugInfo.cpp
@@ -220,15 +220,7 @@ void DIDescriptor::replaceAllUsesWith(MDNode *D) {
   Node->replaceAllUsesWith(D);
 }
 
-bool DICompileUnit::Verify() const {
-  if (!isCompileUnit())
-    return false;
-
-  // Don't bother verifying the compilation directory or producer string
-  // as those could be empty.
-  return !getFilename().empty();
-}
-
+bool DICompileUnit::Verify() const { return isCompileUnit(); }
 bool DIObjCProperty::Verify() const { return isObjCProperty(); }
 
 /// \brief Check if a value can be a reference to a type.
@@ -264,61 +256,18 @@ bool DIType::Verify() const {
   auto *N = dyn_cast_or_null<MDType>(DbgNode);
   if (!N)
     return false;
-  if (!isScopeRef(N->getScope()))
-    return false;
-
-  // DIType is abstract, it should be a BasicType, a DerivedType or
-  // a CompositeType.
-  if (isBasicType())
-    return DIBasicType(DbgNode).Verify();
-
-  // FIXME: Sink this into the various subclass verifies.
-  if (getFilename().empty()) {
-    // Check whether the filename is allowed to be empty.
-    uint16_t Tag = getTag();
-    if (Tag != dwarf::DW_TAG_const_type && Tag != dwarf::DW_TAG_volatile_type &&
-        Tag != dwarf::DW_TAG_pointer_type &&
-        Tag != dwarf::DW_TAG_ptr_to_member_type &&
-        Tag != dwarf::DW_TAG_reference_type &&
-        Tag != dwarf::DW_TAG_rvalue_reference_type &&
-        Tag != dwarf::DW_TAG_restrict_type && Tag != dwarf::DW_TAG_array_type &&
-        Tag != dwarf::DW_TAG_enumeration_type &&
-        Tag != dwarf::DW_TAG_subroutine_type &&
-        Tag != dwarf::DW_TAG_inheritance && Tag != dwarf::DW_TAG_friend &&
-        Tag != dwarf::DW_TAG_structure_type && Tag != dwarf::DW_TAG_member &&
-        Tag != dwarf::DW_TAG_typedef)
-      return false;
-  }
 
   if (isCompositeType())
     return DICompositeType(DbgNode).Verify();
-  if (isDerivedType())
-    return DIDerivedType(DbgNode).Verify();
-  return false;
-}
-
-bool DIBasicType::Verify() const {
-  return dyn_cast_or_null<MDBasicType>(DbgNode);
+  return true;
 }
 
-bool DIDerivedType::Verify() const {
-  auto *N = dyn_cast_or_null<MDDerivedTypeBase>(DbgNode);
-  if (!N)
-    return false;
-  if (getTag() == dwarf::DW_TAG_ptr_to_member_type) {
-    auto *D = dyn_cast<MDDerivedType>(N);
-    if (!D)
-      return false;
-    if (!isTypeRef(D->getExtraData()))
-      return false;
-  }
-  return isTypeRef(N->getBaseType());
-}
+bool DIBasicType::Verify() const { return isBasicType(); }
+bool DIDerivedType::Verify() const { return isDerivedType(); }
 
 bool DICompositeType::Verify() const {
   auto *N = dyn_cast_or_null<MDCompositeTypeBase>(DbgNode);
-  return N && isTypeRef(N->getBaseType()) && isTypeRef(N->getVTableHolder()) &&
-         !(isLValueReference() && isRValueReference());
+  return N && !(isLValueReference() && isRValueReference());
 }
 
 bool DISubprogram::Verify() const {

diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index e5488593b8fced4165a7e89e0d6e9fa406753c7f..4bf24211963868aca4b704acf7c6e76f0e4bbe9f 100644 (file)
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -752,6 +752,26 @@ void Verifier::visitMDDerivedTypeBase(const MDDerivedTypeBase &N) {
 
   Assert(isScopeRef(N.getScope()), "invalid scope", &N, N.getScope());
   Assert(isTypeRef(N.getBaseType()), "invalid base type", &N, N.getBaseType());
+
+  // FIXME: Sink this into the subclass verifies.
+  if (!N.getFile() || N.getFile()->getFilename().empty()) {
+    // Check whether the filename is allowed to be empty.
+    uint16_t Tag = N.getTag();
+    Assert(
+        Tag == dwarf::DW_TAG_const_type || Tag == dwarf::DW_TAG_volatile_type ||
+            Tag == dwarf::DW_TAG_pointer_type ||
+            Tag == dwarf::DW_TAG_ptr_to_member_type ||
+            Tag == dwarf::DW_TAG_reference_type ||
+            Tag == dwarf::DW_TAG_rvalue_reference_type ||
+            Tag == dwarf::DW_TAG_restrict_type ||
+            Tag == dwarf::DW_TAG_array_type ||
+            Tag == dwarf::DW_TAG_enumeration_type ||
+            Tag == dwarf::DW_TAG_subroutine_type ||
+            Tag == dwarf::DW_TAG_inheritance || Tag == dwarf::DW_TAG_friend ||
+            Tag == dwarf::DW_TAG_structure_type ||
+            Tag == dwarf::DW_TAG_member || Tag == dwarf::DW_TAG_typedef,
+        "derived/composite type requires a filename", &N, N.getFile());
+  }
 }
 
 void Verifier::visitMDDerivedType(const MDDerivedType &N) {
@@ -770,6 +790,10 @@ void Verifier::visitMDDerivedType(const MDDerivedType &N) {
              N.getTag() == dwarf::DW_TAG_inheritance ||
              N.getTag() == dwarf::DW_TAG_friend,
          "invalid tag", &N);
+  if (N.getTag() == dwarf::DW_TAG_ptr_to_member_type) {
+    Assert(isTypeRef(N.getExtraData()), "invalid pointer to member type",
+           &N, N.getExtraData());
+  }
 }
 
 void Verifier::visitMDCompositeType(const MDCompositeType &N) {
@@ -809,6 +833,13 @@ void Verifier::visitMDFile(const MDFile &N) {
 void Verifier::visitMDCompileUnit(const MDCompileUnit &N) {
   Assert(N.getTag() == dwarf::DW_TAG_compile_unit, "invalid tag", &N);
 
+  // Don't bother verifying the compilation directory or producer string
+  // as those could be empty.
+  Assert(N.getRawFile() && isa<MDFile>(N.getRawFile()),
+         "invalid file", &N, N.getRawFile());
+  Assert(!N.getFile()->getFilename().empty(), "invalid filename", &N,
+         N.getFile());
+
   if (auto *Array = N.getRawEnumTypes()) {
     Assert(isa<MDTuple>(Array), "invalid enum list", &N, Array);
     for (Metadata *Op : N.getEnumTypes()->operands()) {

diff --git a/test/Assembler/invalid-mdcompileunit-null-file.ll b/test/Assembler/invalid-mdcompileunit-null-file.ll
new file mode 100644 (file)
index 0000000..613948f
--- /dev/null
+++ b/test/Assembler/invalid-mdcompileunit-null-file.ll
@@ -0,0 +1,4 @@
+; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
+
+; CHECK: <stdin>:[[@LINE+1]]:27: error: 'file' cannot be null
+!0 = !MDCompileUnit(file: null)

diff --git a/test/Assembler/metadata-null-operands.ll b/test/Assembler/metadata-null-operands.ll
index acae1d4be99042f13c31359fae21de9342faa2b2..7e27eba45fe73c47c378bc56cd52e8fda1f08a5d 100644 (file)
--- a/test/Assembler/metadata-null-operands.ll
+++ b/test/Assembler/metadata-null-operands.ll
@@ -1,13 +1,11 @@
 ; 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}
+; Don't crash on null operands.  When we add a verify check for this, also
+; require non-null in the assembler and rework this test to check for that ala
+; test/Assembler/invalid-mdcompileunit-null-file.ll.
+!named = !{!0}
 !0 = !MDDerivedType(tag: DW_TAG_pointer_type, baseType: null)
-!1 = !MDCompileUnit(language: DW_LANG_C, file: null)
 
-; CHECK: !named = !{!0, !1}
+; CHECK: !named = !{!0}
 ; CHECK: !0 = !MDDerivedType({{.*}}baseType: null{{.*}})
-; CHECK: !1 = !MDCompileUnit({{.*}}file: null{{.*}})