[PATCH] D21255: Fix cloning GlobalValues with comdats across Modules.

[Justin Lebar via llvm-commits]

jlebar created this revision.
jlebar added reviewers: rnk, majnemer.
jlebar added a subscriber: llvm-commits.

Previously copyAttributesFrom would copy the Comdat pointer from one
GlobalValue to another.  The Comdat is owned by the Module, so if the
two GlobalValues were not in the same module, this would be Bad.

http://reviews.llvm.org/D21255

Files:
  lib/IR/Globals.cpp
  unittests/IR/ValueTest.cpp

Index: unittests/IR/ValueTest.cpp
===================================================================
--- unittests/IR/ValueTest.cpp
+++ unittests/IR/ValueTest.cpp
@@ -89,6 +89,61 @@
   EXPECT_NE(DummyCast0, DummyCast1) << *DummyCast1;
 }
 
+TEST(GlobalTest, CloneWithComdatIntoSameModule) {
+  LLVMContext Ctx;
+  Type *Int32Ty = Type::getInt32Ty(Ctx);
+  auto M = make_unique<Module>("Module", Ctx);
+
+  GlobalVariable *GV1 =
+      new GlobalVariable(*M, Int32Ty, true, GlobalValue::ExternalLinkage,
+                         Constant::getAllOnesValue(Int32Ty), "GV", nullptr,
+                         GlobalVariable::NotThreadLocal, 1);
+  Comdat *Comdat = M->getOrInsertComdat("GV");
+  Comdat->setSelectionKind(Comdat::Largest);
+  GV1->setComdat(Comdat);
+
+  GlobalVariable *GV2 =
+      new GlobalVariable(*M, Int32Ty, true, GlobalValue::ExternalLinkage,
+                         Constant::getAllOnesValue(Int32Ty), "GV", nullptr,
+                         GlobalVariable::NotThreadLocal, 1);
+  GV2->copyAttributesFrom(GV1);
+  ASSERT_EQ(Comdat, GV1->getComdat());
+  ASSERT_TRUE(GV2->getComdat() != nullptr);
+  EXPECT_EQ(Comdat::Largest, GV2->getComdat()->getSelectionKind());
+
+  // GV1 and GV2 should share the same comdat pointers, because they're in the
+  // same module.
+  EXPECT_EQ(GV1->getComdat(), GV2->getComdat());
+}
+
+TEST(GlobalTest, CloneWithComdatIntoSeparateModule) {
+  LLVMContext Ctx;
+  Type *Int32Ty = Type::getInt32Ty(Ctx);
+  auto M1 = make_unique<Module>("Module1", Ctx);
+  auto M2 = make_unique<Module>("Module2", Ctx);
+
+  GlobalVariable *GV1 =
+      new GlobalVariable(*M1, Int32Ty, true, GlobalValue::ExternalLinkage,
+                         Constant::getAllOnesValue(Int32Ty), "GV", nullptr,
+                         GlobalVariable::NotThreadLocal, 1);
+  Comdat *Comdat1 = M1->getOrInsertComdat("GV");
+  Comdat1->setSelectionKind(Comdat::Largest);
+  GV1->setComdat(Comdat1);
+
+  GlobalVariable *GV2 =
+      new GlobalVariable(*M2, Int32Ty, true, GlobalValue::ExternalLinkage,
+                         Constant::getAllOnesValue(Int32Ty), "GV", nullptr,
+                         GlobalVariable::NotThreadLocal, 1);
+  GV2->copyAttributesFrom(GV1);
+  ASSERT_EQ(Comdat1, GV1->getComdat());
+  ASSERT_TRUE(GV2->getComdat() != nullptr);
+  EXPECT_EQ(Comdat::Largest, GV2->getComdat()->getSelectionKind());
+
+  // GV1 and GV2 cannot share the same comdat pointers, because they're in
+  // different Modules.
+  EXPECT_NE(GV1->getComdat(), GV2->getComdat());
+}
+
 #ifdef GTEST_HAS_DEATH_TEST
 #ifndef NDEBUG
 TEST(GlobalTest, AlignDeath) {
Index: lib/IR/Globals.cpp
===================================================================
--- lib/IR/Globals.cpp
+++ lib/IR/Globals.cpp
@@ -96,7 +96,14 @@
   if (const auto *GV = dyn_cast<GlobalObject>(Src)) {
     setAlignment(GV->getAlignment());
     setSection(GV->getSection());
-    setComdat(const_cast<GlobalObject *>(GV)->getComdat());
+    // Src may not be in the same Module as this, and Modules own their Comdats,
+    // so we need to explicitly get a Comdat from our own Module.
+    Comdat *SrcComdat = const_cast<GlobalObject *>(GV)->getComdat();
+    if (SrcComdat != nullptr) {
+      Comdat *C = getParent()->getOrInsertComdat(SrcComdat->getName());
+      C->setSelectionKind(SrcComdat->getSelectionKind());
+      setComdat(C);
+    }
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D21255.60433.patch
Type: text/x-patch
Size: 3368 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160611/bec65872/attachment.bin>