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

[Justin Lebar via llvm-commits]

jlebar updated this revision to Diff 60554.
jlebar added a comment.

Add missing null check.


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 && getParent() != nullptr) {
+      Comdat *C = getParent()->getOrInsertComdat(SrcComdat->getName());
+      C->setSelectionKind(SrcComdat->getSelectionKind());
+      setComdat(C);
+    }
   }
 }
 


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