[PATCH] D60613: Make setInitializer() assert that the entire initializer is usable.

[Arnt Gulbrandsen via Phabricator via llvm-commits]

arnt updated this revision to Diff 207257.
arnt added a comment.

Add unit test, plus some nonfunctional changes.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60613/new/

https://reviews.llvm.org/D60613

Files:
  llvm/lib/IR/Globals.cpp
  llvm/unittests/IR/VerifierTest.cpp


Index: llvm/unittests/IR/VerifierTest.cpp
===================================================================
--- llvm/unittests/IR/VerifierTest.cpp
+++ llvm/unittests/IR/VerifierTest.cpp
@@ -190,5 +190,53 @@
   }
 }
 
+TEST(VerifierTest, CrossModuleInitialization) {
+  // Setup: provide a module containing a global variable that can be
+  // referenced...
+  LLVMContext C;
+  Module M1("M1", C);
+
+  StructType *ST = StructType::create(C, "selfref");
+  ST->setBody({ST->getPointerTo()}, false);
+  GlobalVariable *GV1 = new GlobalVariable(
+      M1, ST, true, GlobalVariable::ExternalLinkage, nullptr);
+
+  // ... and a module, that absolutely cannot refer to GV1 as part of any
+  // initializer.
+  Module M2("M2", C);
+
+  // No test is complete without a little plumbing.
+  std::string Error;
+  raw_string_ostream ErrorOS(Error);
+
+  // Test 1. Is it okay for another variable in M1 to refer to GV1? It should
+  // be.
+  new GlobalVariable(M1, ST, true, GlobalVariable::ExternalLinkage,
+                     ConstantStruct::get(ST, {GV1}));
+  EXPECT_FALSE(verifyModule(M1, &ErrorOS));
+
+  // Test 2. Is it okay for a variable in M2 to refer to GV1? It should not be.
+  new GlobalVariable(M2, ST, true, GlobalVariable::ExternalLinkage,
+                     ConstantStruct::get(ST, {GV1}));
+  Error.clear();
+  EXPECT_TRUE(verifyModule(M2, &ErrorOS));
+  EXPECT_TRUE(ErrorOS.str().substr(0, 36) ==
+              "Referencing global in another module");
+
+  // Test 3. Like 2, except that here a ConstantExpr is used to mask the
+  // reference.
+  auto GV1again = ConstantExpr::getPointerCast(GV1, GV1->getType());
+  new GlobalVariable(M2, ST, true, GlobalVariable::ExternalLinkage,
+                     ConstantStruct::get(ST, {GV1again}));
+  Error.clear();
+  EXPECT_TRUE(verifyModule(M2, &ErrorOS));
+  EXPECT_TRUE(ErrorOS.str().substr(0, 36) ==
+              "Referencing global in another module");
+
+  // Minor addendum at the end: M1 isn't made bad by the references from M2,
+  // right?
+  EXPECT_FALSE(verifyModule(M1, &ErrorOS));
+}
+
 } // end anonymous namespace
 } // end namespace llvm
Index: llvm/lib/IR/Globals.cpp
===================================================================
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -381,6 +381,18 @@
   getParent()->getGlobalList().erase(getIterator());
 }
 
+static void assertThatAllGlobalsAreIn(Constant *Value, Module *Correct) {
+#ifndef NDEBUG
+  if (ConstantAggregate *V = dyn_cast<ConstantAggregate>(Value)) {
+    for (auto &O : V->operands())
+      assertThatAllGlobalsAreIn(cast<Constant>(O.getUser()), Correct);
+  } else if (GlobalValue *V = dyn_cast<GlobalValue>(Value)) {
+    assert((!V->getParent() || V->getParent() == Correct) &&
+           "A Constant and its initializer cannot be in different modules");
+  }
+#endif
+}
+
 void GlobalVariable::setInitializer(Constant *InitVal) {
   if (!InitVal) {
     if (hasInitializer()) {
@@ -399,6 +411,7 @@
     if (!hasInitializer())
       setGlobalVariableNumOperands(1);
     Op<0>().set(InitVal);
+    assertThatAllGlobalsAreIn(InitVal, getParent());
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60613.207257.patch
Type: text/x-patch
Size: 3136 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190701/b66762e0/attachment-0001.bin>