[llvm] r233654 - Verifier: Move checks over from DIDescriptor::Verify()

Duncan P. N. Exon Smith dexonsmith at apple.com
Mon Mar 30 17:47:16 PDT 2015


Author: dexonsmith
Date: Mon Mar 30 19:47:15 2015
New Revision: 233654

URL: http://llvm.org/viewvc/llvm-project?rev=233654&view=rev
Log:
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.

Added:
    llvm/trunk/test/Assembler/invalid-mdcompileunit-null-file.ll
Modified:
    llvm/trunk/lib/AsmParser/LLParser.cpp
    llvm/trunk/lib/IR/DebugInfo.cpp
    llvm/trunk/lib/IR/Verifier.cpp
    llvm/trunk/test/Assembler/metadata-null-operands.ll

Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=233654&r1=233653&r2=233654&view=diff
==============================================================================
--- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
+++ llvm/trunk/lib/AsmParser/LLParser.cpp Mon Mar 30 19:47:15 2015
@@ -3506,7 +3506,7 @@ bool LLParser::ParseMDFile(MDNode *&Resu
 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, );                                            \

Modified: llvm/trunk/lib/IR/DebugInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=233654&r1=233653&r2=233654&view=diff
==============================================================================
--- llvm/trunk/lib/IR/DebugInfo.cpp (original)
+++ llvm/trunk/lib/IR/DebugInfo.cpp Mon Mar 30 19:47:15 2015
@@ -220,15 +220,7 @@ void DIDescriptor::replaceAllUsesWith(MD
   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;
+  return true;
 }
 
-bool DIBasicType::Verify() const {
-  return dyn_cast_or_null<MDBasicType>(DbgNode);
-}
-
-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 {

Modified: llvm/trunk/lib/IR/Verifier.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=233654&r1=233653&r2=233654&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Verifier.cpp (original)
+++ llvm/trunk/lib/IR/Verifier.cpp Mon Mar 30 19:47:15 2015
@@ -752,6 +752,26 @@ void Verifier::visitMDDerivedTypeBase(co
 
   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
              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
 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()) {

Added: llvm/trunk/test/Assembler/invalid-mdcompileunit-null-file.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/invalid-mdcompileunit-null-file.ll?rev=233654&view=auto
==============================================================================
--- llvm/trunk/test/Assembler/invalid-mdcompileunit-null-file.ll (added)
+++ llvm/trunk/test/Assembler/invalid-mdcompileunit-null-file.ll Mon Mar 30 19:47:15 2015
@@ -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)

Modified: llvm/trunk/test/Assembler/metadata-null-operands.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/metadata-null-operands.ll?rev=233654&r1=233653&r2=233654&view=diff
==============================================================================
--- llvm/trunk/test/Assembler/metadata-null-operands.ll (original)
+++ llvm/trunk/test/Assembler/metadata-null-operands.ll Mon Mar 30 19:47:15 2015
@@ -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{{.*}})





More information about the llvm-commits mailing list