From d9c611c77ee56ea436dfa6adae5309001a286de1 Mon Sep 17 00:00:00 2001 From: Sanjoy Das Date: Thu, 29 Dec 2016 15:47:05 +0000 Subject: [PATCH] [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 --- lib/IR/Verifier.cpp | 45 +++++++++---------- .../Analysis/TypeBasedAliasAnalysis/cyclic.ll | 2 +- .../Generic/incorrect-variable-debugloc.ll | 2 +- test/DebugInfo/Generic/recursive_inlining.ll | 2 +- test/DebugInfo/X86/nodebug_with_debug_loc.ll | 2 +- test/Verifier/tbaa.ll | 20 ++++++--- 6 files changed, 40 insertions(+), 33 deletions(-) diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index 57beb42a4a9..5855059a189 100644 --- 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(BaseNode->getOperand(1))) { - CheckFailed("Invalid parent operand in scalar TBAA node", &I, BaseNode); - return InvalidNode; - } - if (!isa(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(BaseNode->getOperand(0))) { + CheckFailed("Struct tag nodes have a string as their first operand", + BaseNode); + return InvalidNode; + } + bool Failed = false; Optional PrevOffset; @@ -4640,18 +4634,20 @@ static bool IsRootTBAANode(const MDNode *MD) { static bool IsScalarTBAANodeImpl(const MDNode *MD, SmallPtrSetImpl &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(MD->getOperand(2)); - if (!(Offset && Offset->isZero() && isa(MD->getOperand(0)))) + if (!isa(MD->getOperand(0))) return false; - auto *Parent = dyn_cast(MD->getOperand(1)); - return Visited.insert(Parent).second && + if (MD->getNumOperands() == 3) { + auto *Offset = mdconst::dyn_extract(MD->getOperand(2)); + if (!(Offset && Offset->isZero() && isa(MD->getOperand(0)))) + return false; + } + + auto *Parent = dyn_cast_or_null(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(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 8400b3ce8d7..f50f870c53b 100644 --- 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 44654ba9418..d86a066ce6e 100644 --- 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 9afa8aea61a..03ccfed01b6 100644 --- 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 c6d3e64ea01..753ece42eef 100644 --- 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 23ed8604dac..4939da92b13 100644 --- 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} -- 2.49.0