[TBAAVerifier] Be stricter around verifying scalar nodes

This fixes the issue exposed in PR31393, where we weren't trying
sufficiently hard to diagnose bad TBAA metadata.

This does reduce the variety in the error messages we print out, but I
think the tradeoff of verifying more, simply and quickly overrules the
need for more helpful error messags here.

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

diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index 57beb42a4a98b0f62b9012cc399c08a340c4dbe3..5855059a189c66e9f702f6d219b861028af801a2 100644 (file)
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -4555,22 +4555,10 @@ TBAAVerifier::verifyTBAABaseNodeImpl(Instruction &I, const MDNode *BaseNode) {
   const TBAAVerifier::TBAABaseNodeSummary InvalidNode = {true, ~0u};
 
   if (BaseNode->getNumOperands() == 2) {
-    // This is a scalar base node.
-    if (!BaseNode->getOperand(0) || !BaseNode->getOperand(1)) {
-      CheckFailed("Null operands in scalar type nodes!", &I, BaseNode);
-      return InvalidNode;
-    }
-    if (!isa<MDNode>(BaseNode->getOperand(1))) {
-      CheckFailed("Invalid parent operand in scalar TBAA node", &I, BaseNode);
-      return InvalidNode;
-    }
-    if (!isa<MDString>(BaseNode->getOperand(0))) {
-      CheckFailed("Invalid name operand in scalar TBAA node", &I, BaseNode);
-      return InvalidNode;
-    }
-
     // Scalar nodes can only be accessed at offset 0.
-    return {false, 0};
+    return isValidScalarTBAANode(BaseNode)
+               ? TBAAVerifier::TBAABaseNodeSummary({false, 0})
+               : InvalidNode;
   }
 
   if (BaseNode->getNumOperands() % 2 != 1) {
@@ -4579,6 +4567,12 @@ TBAAVerifier::verifyTBAABaseNodeImpl(Instruction &I, const MDNode *BaseNode) {
     return InvalidNode;
   }
 
+  if (!isa<MDString>(BaseNode->getOperand(0))) {
+    CheckFailed("Struct tag nodes have a string as their first operand",
+                BaseNode);
+    return InvalidNode;
+  }
+
   bool Failed = false;
 
   Optional<APInt> PrevOffset;
@@ -4640,18 +4634,20 @@ static bool IsRootTBAANode(const MDNode *MD) {
 
 static bool IsScalarTBAANodeImpl(const MDNode *MD,
                                  SmallPtrSetImpl<const MDNode *> &Visited) {
-  if (MD->getNumOperands() == 2)
-    return true;
-
-  if (MD->getNumOperands() != 3)
+  if (MD->getNumOperands() != 2 && MD->getNumOperands() != 3)
     return false;
 
-  auto *Offset = mdconst::dyn_extract<ConstantInt>(MD->getOperand(2));
-  if (!(Offset && Offset->isZero() && isa<MDString>(MD->getOperand(0))))
+  if (!isa<MDString>(MD->getOperand(0)))
     return false;
 
-  auto *Parent = dyn_cast<MDNode>(MD->getOperand(1));
-  return Visited.insert(Parent).second &&
+  if (MD->getNumOperands() == 3) {
+    auto *Offset = mdconst::dyn_extract<ConstantInt>(MD->getOperand(2));
+    if (!(Offset && Offset->isZero() && isa<MDString>(MD->getOperand(0))))
+      return false;
+  }
+
+  auto *Parent = dyn_cast_or_null<MDNode>(MD->getOperand(1));
+  return Parent && Visited.insert(Parent).second &&
          (IsRootTBAANode(Parent) || IsScalarTBAANodeImpl(Parent, Visited));
 }
 
@@ -4745,7 +4741,8 @@ bool TBAAVerifier::visitTBAAMetadata(Instruction &I, const MDNode *MD) {
              &I, MD, BaseNode, AccessType);
 
   AssertTBAA(isValidScalarTBAANode(AccessType),
-             "Access type node must be scalar", &I, MD, AccessType);
+             "Access type node must be a valid scalar type", &I, MD,
+             AccessType);
 
   auto *OffsetCI = mdconst::dyn_extract_or_null<ConstantInt>(MD->getOperand(2));
   AssertTBAA(OffsetCI, "Offset must be constant integer", &I, MD);

diff --git a/test/Analysis/TypeBasedAliasAnalysis/cyclic.ll b/test/Analysis/TypeBasedAliasAnalysis/cyclic.ll
index 8400b3ce8d7f7ceefcec94f0504a01dbb6af1273..f50f870c53b196d76e9e33fa0f5248e595cc2da0 100644 (file)
--- a/test/Analysis/TypeBasedAliasAnalysis/cyclic.ll
+++ b/test/Analysis/TypeBasedAliasAnalysis/cyclic.ll
@@ -1,5 +1,5 @@
 ; RUN: not opt -instcombine < %s 2>&1 | FileCheck %s
-; CHECK: Access type node must be scalar
+; CHECK: Access type node must be a valid scalar type
 
 define void @test6(i32* %gi) #0 {
 entry:

diff --git a/test/DebugInfo/Generic/incorrect-variable-debugloc.ll b/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
index 44654ba94188e50e9e74b04e24d5defc7089a48d..d86a066ce6e0f43a5117027a4c3f0607cc12dcbe 100644 (file)
--- a/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
+++ b/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
@@ -377,7 +377,7 @@ attributes #3 = { nounwind readnone }
 !39 = !DILocation(line: 6, scope: !32, inlinedAt: !40)
 !40 = !DILocation(line: 18, scope: !22)
 !41 = !{!42, !43, i64 0}
-!42 = !{!14, !43, i64 0}
+!42 = !{!"struct", !43, i64 0}
 !43 = !{!"int", !44, i64 0}
 !44 = !{!"omnipotent char", !45, i64 0}
 !45 = !{!"Simple C/C++ TBAA"}

diff --git a/test/DebugInfo/Generic/recursive_inlining.ll b/test/DebugInfo/Generic/recursive_inlining.ll
index 9afa8aea61a7642fa4c529e1b322dcb217e6cf5a..03ccfed01b635a2c8fba68afabe62e52462fe0ad 100644 (file)
--- a/test/DebugInfo/Generic/recursive_inlining.ll
+++ b/test/DebugInfo/Generic/recursive_inlining.ll
@@ -239,7 +239,7 @@ attributes #3 = { nounwind }
 !35 = !DILocation(line: 9, scope: !36, inlinedAt: !24)
 !36 = distinct !DILexicalBlock(scope: !30, file: !2, line: 9)
 !37 = !{!38, !39, i64 0}
-!38 = !{!4, !39, i64 0}
+!38 = !{!"struct", !39, i64 0}
 !39 = !{!"int", !27, i64 0}
 !40 = !DILocation(line: 9, scope: !41, inlinedAt: !24)
 !41 = distinct !DILexicalBlock(scope: !36, file: !2, line: 9)

diff --git a/test/DebugInfo/X86/nodebug_with_debug_loc.ll b/test/DebugInfo/X86/nodebug_with_debug_loc.ll
index c6d3e64ea0115cd9bc7d32f84524e21abb9468e0..753ece42eef3359f8cfe13b2600e9f75b80176d0 100644 (file)
--- a/test/DebugInfo/X86/nodebug_with_debug_loc.ll
+++ b/test/DebugInfo/X86/nodebug_with_debug_loc.ll
@@ -129,7 +129,7 @@ attributes #3 = { nounwind }
 !30 = !DILocation(line: 17, scope: !11)
 !31 = !DILocation(line: 18, scope: !11)
 !32 = !{!33, !34, i64 0}
-!33 = !{!4, !34, i64 0}
+!33 = !{!"struct", !34, i64 0}
 !34 = !{!"any pointer", !35, i64 0}
 !35 = !{!"omnipotent char", !36, i64 0}
 !36 = !{!"Simple C/C++ TBAA"}

diff --git a/test/Verifier/tbaa.ll b/test/Verifier/tbaa.ll
index 23ed8604dacb8e67a2fc57d9e2a73aa1f484e0de..4939da92b13e2b028889fc3581e8ef6e614cd546 100644 (file)
--- a/test/Verifier/tbaa.ll
+++ b/test/Verifier/tbaa.ll
@@ -22,15 +22,21 @@ define void @f_0(i32* %ptr) {
 ; CHECK: Malformed struct tag metadata:  base and access-type should be non-null and point to Metadata nodes
 ; CHECK-NEXT:  store i32 4, i32* %ptr, !tbaa !{{[0-9]+}}
 
-; CHECK: Access type node must be scalar
+; CHECK: Access type node must be a valid scalar type
 ; CHECK-NEXT:  store i32 5, i32* %ptr, !tbaa !{{[0-9]+}}
 
 ; CHECK: Access bit-width not the same as description bit-width
 ; CHECK-NEXT:  store i32 6, i32* %ptr, !tbaa !{{[0-9]+}}
 
-; CHECK: Access type node must be scalar
+; CHECK: Access type node must be a valid scalar type
 ; CHECK-NEXT:  store i32 7, i32* %ptr, !tbaa !{{[0-9]+}}
 
+; CHECK: Struct tag nodes have a string as their first operand
+; CHECK-NEXT:  !{{[0-9]+}} = !{!{{[0-9]+}}, !{{[0-9]+}}, i64 0}
+
+; CHECK: Access type node must be a valid scalar type
+; CHECK-NEXT:  store i32 9, i32* %ptr, !tbaa !{{[0-9]+}}
+
   store i32 0, i32* %ptr, !tbaa !{!3, !2, i64 40, i64 0, i64 1, i64 2}
   store i32 1, i32* %ptr, !tbaa !{!3, !2, i64 40, !"immutable"}
   store i32 2, i32* %ptr, !tbaa !{!3, !2, i64 40, i64 4}
@@ -39,6 +45,8 @@ define void @f_0(i32* %ptr) {
   store i32 5, i32* %ptr, !tbaa !{!3, !3, !"40", i64 0}
   store i32 6, i32* %ptr, !tbaa !{!3, !2, i32 40, i64 0}
   store i32 7, i32* %ptr, !tbaa !{!3, !12, i32 40, i64 0}, !metadata !42
+  store i32 8, i32* %ptr, !tbaa !{!13, !1, i64 0}
+  store i32 9, i32* %ptr, !tbaa !{!14, !14, i64 0}
   ret void
 }
 !42 = !{!"Do no strip this!"}
@@ -61,13 +69,13 @@ define void @f_1(i32* %ptr) {
 ; CHECK: Did not see access type in access path!
 ; CHECK-NEXT:  store i32 3, i32* %ptr, !tbaa !{{[0-9]+}}
 
-; CHECK: Invalid parent operand in scalar TBAA node
+; CHECK: Access type node must be a valid scalar type
 ; CHECK-NEXT:  store i32 4, i32* %ptr, !tbaa !{{[0-9]+}}
 
-; CHECK: Invalid name operand in scalar TBAA node
+; CHECK: Access type node must be a valid scalar type
 ; CHECK-NEXT:  store i32 5, i32* %ptr, !tbaa !{{[0-9]+}}
 
-; CHECK: Null operands in scalar type nodes!
+; CHECK: Access type node must be a valid scalar type
 ; CHECK-NEXT:  store i32 6, i32* %ptr, !tbaa !{{[0-9]+}}
 
 ; CHECK: Struct tag nodes must have an odd number of operands!
@@ -111,3 +119,5 @@ define void @f_1(i32* %ptr) {
 !10 = !{!"bad-struct-type-2", !1, i64 40, !1, i32 56}
 !11 = !{!"bad-struct-type-2", !1, i64 80, !1, i64 56}
 !12 = !{!"bad-scalar-2", !3, i64 0}
+!13 = !{!1, !1, i64 0}
+!14 = !{!"bad-scalar-2", !13}