[PATCH] D26635: [TBAA] Don't generate invalid TBAA when merging nodes

[Sanjoy Das via llvm-commits]

sanjoy created this revision.
sanjoy added reviewers: manmanren, chandlerc, hfinkel, mehdi_amini.
sanjoy added a subscriber: llvm-commits.
Herald added a subscriber: mcrosier.

Fix a corner case in `MDNode::getMostGenericTBAA` where we can sometimes
generate invalid TBAA metadata.


https://reviews.llvm.org/D26635

Files:
  lib/Analysis/TypeBasedAliasAnalysis.cpp
  unittests/Analysis/TBAATest.cpp


Index: unittests/Analysis/TBAATest.cpp
===================================================================
--- unittests/Analysis/TBAATest.cpp
+++ unittests/Analysis/TBAATest.cpp
@@ -31,18 +31,25 @@
   MDBuilder MD;
 };
 
-TEST_F(OldTBAATest, checkVerifierBehavior) {
-  // C++ unit test case to avoid going through the auto upgrade logic.
-
+static StoreInst *getFunctionWithSingleStore(Module *M, StringRef Name) {
+  auto &C = M->getContext();
   FunctionType *FTy = FunctionType::get(Type::getVoidTy(C), {});
-  auto *F = cast<Function>(M.getOrInsertFunction("f", FTy));
+  auto *F = cast<Function>(M->getOrInsertFunction(Name, FTy));
   auto *BB = BasicBlock::Create(C, "entry", F);
   auto *IntType = Type::getInt32Ty(C);
   auto *PtrType = Type::getInt32PtrTy(C);
   auto *SI = new StoreInst(ConstantInt::get(IntType, 42),
                            ConstantPointerNull::get(PtrType), BB);
   ReturnInst::Create(C, nullptr, BB);
 
+  return SI;
+}
+
+TEST_F(OldTBAATest, checkVerifierBehavior) {
+  auto *SI = getFunctionWithSingleStore(&M, "f1");
+  auto *F = SI->getFunction();
+
+  // C++ unit test case to avoid going through the auto upgrade logic.
   auto *RootMD = MD.createTBAARoot("Simple C/C++ TBAA");
   auto *MD1 = MD.createTBAANode("omnipotent char", RootMD);
   auto *MD2 = MD.createTBAANode("int", MD1);
@@ -59,5 +66,27 @@
                   .startswith(ExpectedFailureMsg));
 }
 
+TEST_F(OldTBAATest, checkTBAAMerging) {
+  auto *SI = getFunctionWithSingleStore(&M, "f2");
+  auto *F = SI->getFunction();
+
+  // C++ unit test case to avoid going through the auto upgrade logic.
+  auto *RootMD = MD.createTBAARoot("tbaa-root");
+  auto *MD1 = MD.createTBAANode("scalar-a", RootMD);
+  auto *StructTag1 = MD.createTBAAStructTagNode(MD1, MD1, 0);
+  auto *MD2 = MD.createTBAANode("scalar-b", RootMD);
+  auto *StructTag2 = MD.createTBAAStructTagNode(MD2, MD2, 0);
+
+  auto *GenericMD = MDNode::getMostGenericTBAA(StructTag1, StructTag2);
+
+  EXPECT_EQ(GenericMD, nullptr);
+
+  // Despite GenericMD, we expect the setMetadata call to be well defined, and
+  // produce a verifiable function.
+  SI->setMetadata(LLVMContext::MD_tbaa, GenericMD);
+
+  EXPECT_TRUE(!verifyFunction(*F));
+}
+
 } // end anonymous namspace
 } // end llvm namespace
Index: lib/Analysis/TypeBasedAliasAnalysis.cpp
===================================================================
--- lib/Analysis/TypeBasedAliasAnalysis.cpp
+++ lib/Analysis/TypeBasedAliasAnalysis.cpp
@@ -457,8 +457,12 @@
     --IB;
   }
 
-  if (!Ret)
+  // We either did not find a match, or the only common base "type" is
+  // the root node.  In either case, we don't have any useful TBAA
+  // metadata to attach.
+  if (!Ret || Ret->getNumOperands() < 2)
     return nullptr;
+
   // We need to convert from a type node to a tag node.
   Type *Int64 = IntegerType::get(A->getContext(), 64);
   Metadata *Ops[3] = {Ret, Ret,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26635.77879.patch
Type: text/x-patch
Size: 2900 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161114/f6544ca9/attachment.bin>