[llvm] r290713 - [TBAAVerifier] Be stricter around verifying scalar nodes

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 29 07:47:06 PST 2016


Author: sanjoy
Date: Thu Dec 29 09:47:05 2016
New Revision: 290713

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

Modified:
    llvm/trunk/lib/IR/Verifier.cpp
    llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/cyclic.ll
    llvm/trunk/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
    llvm/trunk/test/DebugInfo/Generic/recursive_inlining.ll
    llvm/trunk/test/DebugInfo/X86/nodebug_with_debug_loc.ll
    llvm/trunk/test/Verifier/tbaa.ll

Modified: llvm/trunk/lib/IR/Verifier.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=290713&r1=290712&r2=290713&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Verifier.cpp (original)
+++ llvm/trunk/lib/IR/Verifier.cpp Thu Dec 29 09:47:05 2016
@@ -4555,22 +4555,10 @@ TBAAVerifier::verifyTBAABaseNodeImpl(Ins
   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(Ins
     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
 
 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(Ins
              &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);

Modified: llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/cyclic.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/cyclic.ll?rev=290713&r1=290712&r2=290713&view=diff
==============================================================================
--- llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/cyclic.ll (original)
+++ llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/cyclic.ll Thu Dec 29 09:47:05 2016
@@ -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:

Modified: llvm/trunk/test/DebugInfo/Generic/incorrect-variable-debugloc.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/Generic/incorrect-variable-debugloc.ll?rev=290713&r1=290712&r2=290713&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/Generic/incorrect-variable-debugloc.ll (original)
+++ llvm/trunk/test/DebugInfo/Generic/incorrect-variable-debugloc.ll Thu Dec 29 09:47:05 2016
@@ -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"}

Modified: llvm/trunk/test/DebugInfo/Generic/recursive_inlining.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/Generic/recursive_inlining.ll?rev=290713&r1=290712&r2=290713&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/Generic/recursive_inlining.ll (original)
+++ llvm/trunk/test/DebugInfo/Generic/recursive_inlining.ll Thu Dec 29 09:47:05 2016
@@ -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)

Modified: llvm/trunk/test/DebugInfo/X86/nodebug_with_debug_loc.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/nodebug_with_debug_loc.ll?rev=290713&r1=290712&r2=290713&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/X86/nodebug_with_debug_loc.ll (original)
+++ llvm/trunk/test/DebugInfo/X86/nodebug_with_debug_loc.ll Thu Dec 29 09:47:05 2016
@@ -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"}

Modified: llvm/trunk/test/Verifier/tbaa.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Verifier/tbaa.ll?rev=290713&r1=290712&r2=290713&view=diff
==============================================================================
--- llvm/trunk/test/Verifier/tbaa.ll (original)
+++ llvm/trunk/test/Verifier/tbaa.ll Thu Dec 29 09:47:05 2016
@@ -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}




More information about the llvm-commits mailing list