bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40)

[belleyb]

Summary

• Linker::LinkOnlyNeeded should always import globals with AppendingLinkage when used in Full-LTO / non-performing-import mode.

• This the patch against the release_40 branch; 4.0.1-rc2 / revision 304242 to be exact. There's also a review for a fix in trunk. See: https://reviews.llvm.org/D34448

See: https://bugs.llvm.org/show_bug.cgi?id=33527

[mehdi_amini]

unittests/Linker/LinkModulesTest.cpp

939

Can this be tested with llvm-lto instead of a gtest? That's the preferred way of testing.

[belleyb]

unittests/Linker/LinkModulesTest.cpp

939

No problem. I'll do that.

[pcc]

unittests/Linker/LinkModulesTest.cpp

939

You mean llvm-link, right?

[belleyb]

Converted tests to use lit, llvm-link and FileCheck instead of being written as unit tests

[belleyb]

lib/Linker/LinkModules.cpp

358

This extra checks at line 357 and 586 are the fix. Without it, the tests below are failing.

The rest of the changes around is only to make the code more readable, allowing me to better document the intent,

test/Linker/only-needed-compiler-used.ll

8 ↗

(On Diff #104142)

The linker emits the references to the global variables in a different ordering when the --only-needed is used. This explains the use of the DAG FileCheck option below.

test/Linker/only-needed-ctors2.ll

4 ↗

(On Diff #104142)

Note that the test case requires at one of the -internalize or -only-needed flag. Else, a link error is generated about func1 being defined twice. This explains why this particular test case has only 3 runs, instead of 4, like most of the other ones.

test/Linker/only-needed-ctors4.ll

4 ↗

(On Diff #104142)

Note that the test case requires at one of the -internalize or -only-needed flag. Else, a link error is generated about func1 being defined twice. This explains why this particular test case has only 3 runs, instead of 4, like most of the other ones.

test/Linker/only-needed-dtors2.ll

4 ↗

(On Diff #104142)

Note that the test case requires at one of the -internalize or -only-needed flag. Else, a link error is generated about func1 being defined twice. This explains why this particular test case has only 3 runs, instead of 4, like most of the other ones.

test/Linker/only-needed-dtors4.ll

4 ↗

(On Diff #104142)

Note that the test case requires at one of the -internalize or -only-needed flag. Else, a link error is generated about func1 being defined twice. This explains why this particular test case has only 3 runs, instead of 4, like most of the other ones.

test/Linker/only-needed-used.ll

8 ↗

(On Diff #104142)

The linker emits the references to the global variables in a different ordering when the --only-needed is used. This explains the use of the DAG FileCheck option below.

10 ↗

(On Diff #104142)

This test gives a different result in the release_40 branch than it does in trunk (5.0). I don't know which is correct. I just noticed the difference.

In trunk, the used variables always have global linkage and never internal global linkage, even when the -internalize flag is used. See: https://reviews.llvm.org/D34448

[tejohnson]

See comments on D34448

[belleyb]

I will update this code review once https://reviews.llvm.org/D34448 is approved and submitted.

[belleyb]

Abandoning this review. The fix has been submitted to the LLVM trunk (6.0 at this time) as r310522. Moreover, we will soon move to LLVM 5.0...

lib/LTO/ThinLTOCodeGenerator.cpp

 #include "llvm/Bitcode/BitcodeWriterPass.h"
 #include "llvm/ExecutionEngine/ObjectMemoryBuffer.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/LTO/LTO.h"
-#include "llvm/Linker/Linker.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Object/IRObjectFile.h"
 #include "llvm/Object/ModuleSummaryIndexObjectFile.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SHA1.h"

lib/Linker/LinkModules.cpp

assert(Dest.hasExternalLinkage() && Src.hasExternalLinkage() &&
          "Unexpected linkage type!");
   return emitError("Linking globals named '" + Src.getName() +
                    "': symbol multiply defined!");
 }
 
 bool ModuleLinker::linkIfNeeded(GlobalValue &GV) {
   GlobalValue *DGV = getLinkedToGlobal(&GV);
 
-  if (shouldLinkOnlyNeeded() && !(DGV && DGV->isDeclaration()))
+  if (shouldLinkOnlyNeeded()) {
+    // Always import variables with appending linkage.
+    if (!GV.hasAppendingLinkage()) {
+      // Don't import globals unless they are referenced by the destination
+      // module.
+      if (!DGV)
+        return false;
+      // Don't import globals that are already defined in the destination module
+      if (!DGV->isDeclaration())
         return false;
+    }
+  }
 
   if (DGV && !GV.hasLocalLinkage() && !GV.hasAppendingLinkage()) {
     auto *DGVar = dyn_cast<GlobalVariable>(DGV);
     auto *SGVar = dyn_cast<GlobalVariable>(&GV);
     if (DGVar && SGVar) {
       if (DGVar->isDeclaration() && SGVar->isDeclaration() &&
           (!DGVar->isConstant() || !SGVar->isConstant())) {
         DGVar->setConstant(false);
for (GlobalValue *GV2 : LazyComdatMembers[SC]) {
         return true;
       if (LinkFromSrc)
         ValuesToLink.insert(GV2);
     }
   }
 
   if (shouldInternalizeLinkedSymbols()) {
     for (GlobalValue *GV : ValuesToLink)
+        if (!GV->hasAppendingLinkage()) {
             Internalize.insert(GV->getName());
         }
+  }
 
   // FIXME: Propagate Errors through to the caller instead of emitting
   // diagnostics.
   bool HasErrors = false;
   if (Error E = Mover.move(std::move(SrcM), ValuesToLink.getArrayRef(),
                            [this](GlobalValue &GV, IRMover::ValueAdder Add) {
                              addLazyFor(GV, Add);
                            },

unittests/Linker/LinkModulesTest.cpp

TEST_F(LinkModuleTest, RemangleIntrinsics) {
 
   // "struct.rtx_def" from Foo and "struct.rtx_def.0" from Bar are isomorphic
   // types, so they must be uniquified by linker. Check that they use the same
   // intrinsic definition.
   Function *F = Foo->getFunction("llvm.memset.p0struct.rtx_def.i32");
   ASSERT_EQ(F->getNumUses(), (unsigned)2);
 }
 
+TEST_F(LinkModuleTest, ImportIntrinsicGlobalVariables_ctors1) {
+  SMDiagnostic Err;
+
+  // Only the source module has global constructors and these are imported!
+  const char *SrcStr =
+      ("define internal void @ctor1() {\n"
+       "  ret void\n"
+       "}"
+       "define internal void @ctor2() {\n"
+       "  ret void\n"
+       "}\n"
+       "@llvm.global_ctors = appending global[2 x{i32, void() *, i8 * }]\n"
+       "  [{i32, void() *, i8 * } { i32 2, void() *@ctor1, i8 *null},\n"
+       "   {i32, void() *, i8 * } { i32 7, void() *@ctor2, i8 *null}]\n");
+
+  std::unique_ptr<Module> Src = parseAssemblyString(SrcStr, Err, Ctx);
+  ASSERT_TRUE(bool(Src));
+  ASSERT_TRUE(Src.get());
+
+  // Link into destination module.
+  auto Dst = llvm::make_unique<Module>("Linked", Ctx);
+  ASSERT_TRUE(Dst.get());
+  Ctx.setDiagnosticHandler(expectNoDiags);
+  Linker L(*Dst);
+
+  ASSERT_FALSE(L.linkInModule(std::move(Src),
+                              Linker::LinkOnlyNeeded |
+                                  Linker::InternalizeLinkedSymbols));
+
+  // Intrisic global variables must be imported since the
+  // ImportIntrinsicGlobalVariables flag has been specified.
+  auto const *ctor1 = Dst->getFunction("ctor1");
+  auto const *ctor2 = Dst->getFunction("ctor2");
+  auto const *gctors = Dst->getNamedGlobal("llvm.global_ctors");
+
+  ASSERT_NE(nullptr, ctor1);
+  ASSERT_NE(nullptr, ctor2);
+  ASSERT_NE(nullptr, gctors);
+
+  EXPECT_TRUE(ctor1->hasInternalLinkage());
+  EXPECT_TRUE(ctor2->hasInternalLinkage());
+  EXPECT_TRUE(gctors->hasAppendingLinkage());
+
+  auto const *init = dyn_cast_or_null<ConstantArray>(gctors->getInitializer());
+  ASSERT_NE(nullptr, init);
+  ASSERT_EQ(2U, init->getNumOperands());
+
+  auto const *ctor1_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(0U));
+  auto const *ctor2_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(1U));
+  ASSERT_NE(nullptr, ctor1_init);
+  ASSERT_NE(nullptr, ctor2_init);
+
+  EXPECT_EQ(3U, ctor1_init->getNumOperands());
+  EXPECT_EQ(2U,
+            dyn_cast<ConstantInt>(ctor1_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor1, dyn_cast<Function>(ctor1_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor1_init->getOperand(2U)));
+
+  EXPECT_EQ(3U, ctor2_init->getNumOperands());
+  EXPECT_EQ(7U,
+            dyn_cast<ConstantInt>(ctor2_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor2, dyn_cast<Function>(ctor2_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor2_init->getOperand(2U)));
+}
+
+TEST_F(LinkModuleTest, ImportIntrinsicGlobalVariables_ctors2) {
+  SMDiagnostic Err;
+
+  // Both the destination and the source modules have global constructors.
+  const char *DstStr =
+      ("define internal void @ctor1() {\n"
+       "  ret void\n"
+       "}"
+       "@llvm.global_ctors = appending global[1 x{i32, void() *, i8 * }]\n"
+       "  [{i32, void() *, i8 * } { i32 4, void() *@ctor1, i8 *null}]\n");
+
+  const char *SrcStr =
+      ("define internal void @ctor1() {\n"
+       "  ret void\n"
+       "}"
+       "define internal void @ctor2() {\n"
+       "  ret void\n"
+       "}\n"
+       "@llvm.global_ctors = appending global[2 x{i32, void() *, i8 * }]\n"
+       "  [{i32, void() *, i8 * } { i32 2, void() *@ctor1, i8 *null},\n"
+       "   {i32, void() *, i8 * } { i32 7, void() *@ctor2, i8 *null}]\n");
+
+  std::unique_ptr<Module> Src = parseAssemblyString(SrcStr, Err, Ctx);
+  ASSERT_TRUE(bool(Src));
+  ASSERT_TRUE(Src.get());
+
+  // Link into destination module.
+   std::unique_ptr<Module> Dst = parseAssemblyString(DstStr, Err, Ctx);
+  ASSERT_TRUE(Dst.get());
+  Ctx.setDiagnosticHandler(expectNoDiags);
+  Linker L(*Dst);
+
+  ASSERT_FALSE(L.linkInModule(std::move(Src),
+                              Linker::LinkOnlyNeeded |
+                                  Linker::InternalizeLinkedSymbols));
+
+  // Intrisic global variables must be imported since the
+  // ImportIntrinsicGlobalVariables flag has been specified.
+  auto const *ctor1 = Dst->getFunction("ctor1");
+  auto const *ctor1_2 = Dst->getFunction("ctor1.2");
+  auto const *ctor2 = Dst->getFunction("ctor2");
+  auto const *gctors = Dst->getNamedGlobal("llvm.global_ctors");
+
+  ASSERT_NE(nullptr, ctor1);
+  ASSERT_NE(nullptr, ctor1_2);
+  ASSERT_NE(nullptr, ctor2);
+  ASSERT_NE(nullptr, gctors);
+
+  EXPECT_TRUE(ctor1->hasInternalLinkage());
+  EXPECT_TRUE(ctor1_2->hasInternalLinkage());
+  EXPECT_TRUE(ctor2->hasInternalLinkage());
+  EXPECT_TRUE(gctors->hasAppendingLinkage());
+
+  auto const *init = dyn_cast_or_null<ConstantArray>(gctors->getInitializer());
+  ASSERT_NE(nullptr, init);
+  ASSERT_EQ(3U, init->getNumOperands());
+
+  auto const *ctor1_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(0U));
+  auto const *ctor1_2_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(1U));
+  auto const *ctor2_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(2U));
+  ASSERT_NE(nullptr, ctor1_init);
+  ASSERT_NE(nullptr, ctor1_2_init);
+  ASSERT_NE(nullptr, ctor2_init);
+
+  EXPECT_EQ(3U, ctor1_init->getNumOperands());
+  EXPECT_EQ(4U,
+            dyn_cast<ConstantInt>(ctor1_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor1, dyn_cast<Function>(ctor1_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor1_init->getOperand(2U)));
+
+  EXPECT_EQ(2U,
+            dyn_cast<ConstantInt>(ctor1_2_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor1_2, dyn_cast<Function>(ctor1_2_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor1_2_init->getOperand(2U)));
+
+  EXPECT_EQ(3U, ctor2_init->getNumOperands());
+  EXPECT_EQ(7U,
+            dyn_cast<ConstantInt>(ctor2_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor2, dyn_cast<Function>(ctor2_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor2_init->getOperand(2U)));
+}
+
+TEST_F(LinkModuleTest, ImportIntrinsicGlobalVariables_ctors3) {
+  SMDiagnostic Err;
+
+  // Destination and source modules have global constructors.
+  const char *DstStr =
+      ("define internal void @ctor1() {\n"
+       "  ret void\n"
+       "}"
+       "@llvm.global_ctors = appending global[1 x{i32, void() *, i8 * }]\n"
+       "  [{i32, void() *, i8 * } { i32 4, void() *@ctor1, i8 *null}]\n");
+
+  const char *Src1Str =
+      ("define internal void @ctor1() {\n"
+       "  ret void\n"
+       "}"
+       "define internal void @ctor2() {\n"
+       "  ret void\n"
+       "}\n"
+       "@llvm.global_ctors = appending global[2 x{i32, void() *, i8 * }]\n"
+       "  [{i32, void() *, i8 * } { i32 2, void() *@ctor1, i8 *null},\n"
+       "   {i32, void() *, i8 * } { i32 7, void() *@ctor2, i8 *null}]\n");
+
+  const char *Src2Str =
+      ("define internal void @ctor3() {\n"
+       "  ret void\n"
+       "}"
+       "define internal void @ctor4() {\n"
+       "  ret void\n"
+       "}\n"
+       "@llvm.global_ctors = appending global[2 x{i32, void() *, i8 * }]\n"
+       "  [{i32, void() *, i8 * } { i32 1, void() *@ctor3, i8 *null},\n"
+       "   {i32, void() *, i8 * } { i32 9, void() *@ctor4, i8 *null}]\n");
+
+  std::unique_ptr<Module> Src1 = parseAssemblyString(Src1Str, Err, Ctx);
+  ASSERT_TRUE(bool(Src1));
+  ASSERT_TRUE(Src1.get());
+
+  std::unique_ptr<Module> Src2 = parseAssemblyString(Src2Str, Err, Ctx);
+  ASSERT_TRUE(bool(Src2));
+  ASSERT_TRUE(Src2.get());
+
+  // Link into destination module.
+  std::unique_ptr<Module> Dst = parseAssemblyString(DstStr, Err, Ctx);
+  ASSERT_TRUE(Dst.get());
+  Ctx.setDiagnosticHandler(expectNoDiags);
+  Linker L(*Dst);
+
+  ASSERT_FALSE(L.linkInModule(std::move(Src1),
+                              Linker::LinkOnlyNeeded |
+                                  Linker::InternalizeLinkedSymbols));
+
+  ASSERT_FALSE(L.linkInModule(std::move(Src2),
+                              Linker::LinkOnlyNeeded |
+                                  Linker::InternalizeLinkedSymbols));
+
+  // Intrisic global variables must be imported since the
+  // ImportIntrinsicGlobalVariables flag has been specified.
+  auto const *ctor1 = Dst->getFunction("ctor1");
+  auto const *ctor1_2 = Dst->getFunction("ctor1.2");
+  auto const *ctor2 = Dst->getFunction("ctor2");
+  auto const *ctor3 = Dst->getFunction("ctor3");
+  auto const *ctor4 = Dst->getFunction("ctor4");
+  auto const *gctors = Dst->getNamedGlobal("llvm.global_ctors");
+
+  ASSERT_NE(nullptr, ctor1);
+  ASSERT_NE(nullptr, ctor1_2);
+  ASSERT_NE(nullptr, ctor2);
+  ASSERT_NE(nullptr, ctor3);
+  ASSERT_NE(nullptr, ctor4);
+  ASSERT_NE(nullptr, gctors);
+
+  EXPECT_TRUE(ctor1->hasInternalLinkage());
+  EXPECT_TRUE(ctor1_2->hasInternalLinkage());
+  EXPECT_TRUE(ctor2->hasInternalLinkage());
+  EXPECT_TRUE(ctor3->hasInternalLinkage());
+  EXPECT_TRUE(ctor4->hasInternalLinkage());
+  EXPECT_TRUE(gctors->hasAppendingLinkage());
+
+  auto const *init = dyn_cast_or_null<ConstantArray>(gctors->getInitializer());
+  ASSERT_NE(nullptr, init);
+  ASSERT_EQ(5U, init->getNumOperands());
+
+  auto const *ctor1_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(0U));
+  auto const *ctor1_2_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(1U));
+  auto const *ctor2_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(2U));
+  auto const *ctor3_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(3U));
+  auto const *ctor4_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(4U));
+  ASSERT_NE(nullptr, ctor1_init);
+  ASSERT_NE(nullptr, ctor1_2_init);
+  ASSERT_NE(nullptr, ctor2_init);
+  ASSERT_NE(nullptr, ctor3_init);
+  ASSERT_NE(nullptr, ctor4_init);
+
+  EXPECT_EQ(3U, ctor1_init->getNumOperands());
+  EXPECT_EQ(4U,
+            dyn_cast<ConstantInt>(ctor1_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor1, dyn_cast<Function>(ctor1_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor1_init->getOperand(2U)));
+
+  EXPECT_EQ(2U,
+            dyn_cast<ConstantInt>(ctor1_2_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor1_2, dyn_cast<Function>(ctor1_2_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor1_2_init->getOperand(2U)));
+
+  EXPECT_EQ(3U, ctor2_init->getNumOperands());
+  EXPECT_EQ(7U,
+            dyn_cast<ConstantInt>(ctor2_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor2, dyn_cast<Function>(ctor2_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor2_init->getOperand(2U)));
+
+  EXPECT_EQ(3U, ctor3_init->getNumOperands());
+  EXPECT_EQ(1U,
+            dyn_cast<ConstantInt>(ctor3_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor3, dyn_cast<Function>(ctor3_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor3_init->getOperand(2U)));
+
+  EXPECT_EQ(3U, ctor4_init->getNumOperands());
+  EXPECT_EQ(9U,
+            dyn_cast<ConstantInt>(ctor4_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor4, dyn_cast<Function>(ctor4_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor4_init->getOperand(2U)));
+}
+
+TEST_F(LinkModuleTest, ImportIntrinsicGlobalVariables_ctors4) {
+  SMDiagnostic Err;
+
+  // Source modules have global constructors, but destination doesn't
+  const char *Src1Str =
+      ("define internal void @ctor1() {\n"
+       "  ret void\n"
+       "}"
+       "define internal void @ctor2() {\n"
+       "  ret void\n"
+       "}\n"
+       "@llvm.global_ctors = appending global[2 x{i32, void() *, i8 * }]\n"
+       "  [{i32, void() *, i8 * } { i32 2, void() *@ctor1, i8 *null},\n"
+       "   {i32, void() *, i8 * } { i32 7, void() *@ctor2, i8 *null}]\n");
+
+  const char *Src2Str =
+      ("define internal void @ctor3() {\n"
+       "  ret void\n"
+       "}"
+       "define internal void @ctor4() {\n"
+       "  ret void\n"
+       "}\n"
+       "@llvm.global_ctors = appending global[2 x{i32, void() *, i8 * }]\n"
+       "  [{i32, void() *, i8 * } { i32 1, void() *@ctor3, i8 *null},\n"
+       "   {i32, void() *, i8 * } { i32 9, void() *@ctor4, i8 *null}]\n");
+
+  std::unique_ptr<Module> Src1 = parseAssemblyString(Src1Str, Err, Ctx);
+  ASSERT_TRUE(bool(Src1));
+  ASSERT_TRUE(Src1.get());
+
+  std::unique_ptr<Module> Src2 = parseAssemblyString(Src2Str, Err, Ctx);
+  ASSERT_TRUE(bool(Src2));
+  ASSERT_TRUE(Src2.get());
+
+  // Link into destination module.
+  auto Dst = llvm::make_unique<Module>("Linked", Ctx);
+  ASSERT_TRUE(Dst.get());
+  Ctx.setDiagnosticHandler(expectNoDiags);
+  Linker L(*Dst);
+
+  ASSERT_FALSE(L.linkInModule(std::move(Src1),
+                              Linker::LinkOnlyNeeded |
+                                  Linker::InternalizeLinkedSymbols));
+
+  ASSERT_FALSE(L.linkInModule(std::move(Src2),
+                              Linker::LinkOnlyNeeded |
+                                  Linker::InternalizeLinkedSymbols));
+
+  // Intrisic global variables must be imported since the
+  // ImportIntrinsicGlobalVariables flag has been specified.
+  auto const *ctor1 = Dst->getFunction("ctor1");
+  auto const *ctor2 = Dst->getFunction("ctor2");
+  auto const *ctor3 = Dst->getFunction("ctor3");
+  auto const *ctor4 = Dst->getFunction("ctor4");
+  auto const *gctors = Dst->getNamedGlobal("llvm.global_ctors");
+
+  ASSERT_NE(nullptr, ctor1);
+  ASSERT_NE(nullptr, ctor2);
+  ASSERT_NE(nullptr, ctor3);
+  ASSERT_NE(nullptr, ctor4);
+  ASSERT_NE(nullptr, gctors);
+
+  EXPECT_TRUE(ctor1->hasInternalLinkage());
+  EXPECT_TRUE(ctor2->hasInternalLinkage());
+  EXPECT_TRUE(ctor3->hasInternalLinkage());
+  EXPECT_TRUE(ctor4->hasInternalLinkage());
+  EXPECT_TRUE(gctors->hasAppendingLinkage());
+
+  auto const *init = dyn_cast_or_null<ConstantArray>(gctors->getInitializer());
+  ASSERT_NE(nullptr, init);
+  ASSERT_EQ(4U, init->getNumOperands());
+
+  auto const *ctor1_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(0U));
+  auto const *ctor2_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(1U));
+  auto const *ctor3_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(2U));
+  auto const *ctor4_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(3U));
+  ASSERT_NE(nullptr, ctor1_init);
+  ASSERT_NE(nullptr, ctor2_init);
+  ASSERT_NE(nullptr, ctor3_init);
+  ASSERT_NE(nullptr, ctor4_init);
+
+  EXPECT_EQ(3U, ctor1_init->getNumOperands());
+  EXPECT_EQ(2U,
+            dyn_cast<ConstantInt>(ctor1_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor1, dyn_cast<Function>(ctor1_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor1_init->getOperand(2U)));
+
+  EXPECT_EQ(3U, ctor2_init->getNumOperands());
+  EXPECT_EQ(7U,
+            dyn_cast<ConstantInt>(ctor2_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor2, dyn_cast<Function>(ctor2_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor2_init->getOperand(2U)));
+
+  EXPECT_EQ(3U, ctor3_init->getNumOperands());
+  EXPECT_EQ(1U,
+            dyn_cast<ConstantInt>(ctor3_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor3, dyn_cast<Function>(ctor3_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor3_init->getOperand(2U)));
+
+  EXPECT_EQ(3U, ctor4_init->getNumOperands());
+  EXPECT_EQ(9U,
+            dyn_cast<ConstantInt>(ctor4_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(ctor4, dyn_cast<Function>(ctor4_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(ctor4_init->getOperand(2U)));
+}
+
+TEST_F(LinkModuleTest, ImportIntrinsicGlobalVariables_dtors1) {
+  SMDiagnostic Err;
+
+  // Only the source module has global destructors and these are imported!
+  const char *SrcStr =
+      ("define internal void @dtor1() {\n"
+       "  ret void\n"
+       "}"
+       "define internal void @dtor2() {\n"
+       "  ret void\n"
+       "}\n"
+       "@llvm.global_dtors = appending global[2 x{i32, void() *, i8 * }]\n"
+       "  [{i32, void() *, i8 * } { i32 2, void() *@dtor1, i8 *null},\n"
+       "   {i32, void() *, i8 * } { i32 7, void() *@dtor2, i8 *null}]\n");
+
+  std::unique_ptr<Module> Src = parseAssemblyString(SrcStr, Err, Ctx);
+  ASSERT_TRUE(bool(Src));
+  ASSERT_TRUE(Src.get());
+
+  // Link into destination module.
+  auto Dst = llvm::make_unique<Module>("Linked", Ctx);
+  ASSERT_TRUE(Dst.get());
+  Ctx.setDiagnosticHandler(expectNoDiags);
+  Linker L(*Dst);
+
+  ASSERT_FALSE(L.linkInModule(std::move(Src),
+                              Linker::LinkOnlyNeeded |
+                                  Linker::InternalizeLinkedSymbols));
+
+  // Intrisic global variables must be imported since the
+  // ImportIntrinsicGlobalVariables flag has been specified.
+  auto const *dtor1 = Dst->getFunction("dtor1");
+  auto const *dtor2 = Dst->getFunction("dtor2");
+  auto const *gdtors = Dst->getNamedGlobal("llvm.global_dtors");
+
+  ASSERT_NE(nullptr, dtor1);
+  ASSERT_NE(nullptr, dtor2);
+  ASSERT_NE(nullptr, gdtors);
+
+  EXPECT_TRUE(dtor1->hasInternalLinkage());
+  EXPECT_TRUE(dtor2->hasInternalLinkage());
+  EXPECT_TRUE(gdtors->hasAppendingLinkage());
+
+  auto const *init = dyn_cast_or_null<ConstantArray>(gdtors->getInitializer());
+  ASSERT_NE(nullptr, init);
+  ASSERT_EQ(2U, init->getNumOperands());
+
+  auto const *dtor1_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(0U));
+  auto const *dtor2_init =
+      dyn_cast_or_null<ConstantStruct>(init->getOperand(1U));
+  ASSERT_NE(nullptr, dtor1_init);
+  ASSERT_NE(nullptr, dtor2_init);
+
+  EXPECT_EQ(3U, dtor1_init->getNumOperands());
+  EXPECT_EQ(2U,
+            dyn_cast<ConstantInt>(dtor1_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(dtor1, dyn_cast<Function>(dtor1_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(dtor1_init->getOperand(2U)));
+
+  EXPECT_EQ(3U, dtor2_init->getNumOperands());
+  EXPECT_EQ(7U,
+            dyn_cast<ConstantInt>(dtor2_init->getOperand(0U))
+                ->getZExtValue());
+  EXPECT_EQ(dtor2, dyn_cast<Function>(dtor2_init->getOperand(1U)));
+  EXPECT_TRUE(isa<ConstantPointerNull>(dtor2_init->getOperand(2U)));
+}
+
+TEST_F(LinkModuleTest, ImportIntrinsicGlobalVariables_used1) {
+  SMDiagnostic Err;
+
+  // Only the source module has llvm.used and these are imported!
+  const char *SrcStr =
+      ("@used1 = global i8 4\n"
+       "@used2 = global i32 123\n"
+       "@llvm.used = appending global [2 x i8*] [\n"
+       "   i8* @used1,\n"
+       "   i8* bitcast (i32* @used2 to i8*)\n"
+       "], section \"llvm.metadata\"\n");
+
+  std::unique_ptr<Module> Src = parseAssemblyString(SrcStr, Err, Ctx);
+  ASSERT_TRUE(bool(Src));
+  ASSERT_TRUE(Src.get());
+
+  // Link into destination module.
+  auto Dst = llvm::make_unique<Module>("Linked", Ctx);
+  ASSERT_TRUE(Dst.get());
+  Ctx.setDiagnosticHandler(expectNoDiags);
+  Linker L(*Dst);
+
+  ASSERT_FALSE(L.linkInModule(std::move(Src),
+                              Linker::LinkOnlyNeeded |
+                                  Linker::InternalizeLinkedSymbols));
+
+  // Intrisic global variables must be imported since the
+  // ImportIntrinsicGlobalVariables flag has been specified.
+  auto const *used1 = Dst->getNamedGlobal("used1");
+  auto const *used2 = Dst->getNamedGlobal("used2");
+  auto const *useds = Dst->getNamedGlobal("llvm.used");
+
+  ASSERT_NE(nullptr, used1);
+  ASSERT_NE(nullptr, used2);
+  ASSERT_NE(nullptr, useds);
+
+  EXPECT_TRUE(used1->hasInternalLinkage());
+  EXPECT_TRUE(used2->hasInternalLinkage());
+  EXPECT_TRUE(useds->hasAppendingLinkage());
+
+  auto const *init = dyn_cast_or_null<ConstantArray>(useds->getInitializer());
+  ASSERT_NE(nullptr, init);
+  ASSERT_EQ(2U, init->getNumOperands());
+
+  auto const *used1_init =
+      dyn_cast_or_null<GlobalVariable>(init->getOperand(0U));
+  auto const *used2_init =
+      dyn_cast_or_null<ConstantExpr>(init->getOperand(1U));
+  EXPECT_EQ(used1, used1_init);
+  EXPECT_EQ(used2, used2_init->getOperand(0U));
+}
+
+TEST_F(LinkModuleTest, ImportIntrinsicGlobalVariables_compiler_used1) {
+  SMDiagnostic Err;
+
+  // Only the source module has llvm.compiler.used and these are imported!
+  const char *SrcStr =
+      ("@used1 = global i8 4\n"
+       "@used2 = global i32 123\n"
+       "@llvm.compiler.used = appending global [2 x i8*] [\n"
+       "   i8* @used1,\n"
+       "   i8* bitcast (i32* @used2 to i8*)\n"
+       "], section \"llvm.metadata\"\n");
+
+  std::unique_ptr<Module> Src = parseAssemblyString(SrcStr, Err, Ctx);
+  ASSERT_TRUE(bool(Src));
+  ASSERT_TRUE(Src.get());
+
+  // Link into destination module.
+  auto Dst = llvm::make_unique<Module>("Linked", Ctx);
+  ASSERT_TRUE(Dst.get());
+  Ctx.setDiagnosticHandler(expectNoDiags);
+  Linker L(*Dst);
+
+  ASSERT_FALSE(L.linkInModule(std::move(Src),
+                              Linker::LinkOnlyNeeded |
+                                  Linker::InternalizeLinkedSymbols));
+
+  // Intrisic global variables must be imported since the
+  // ImportIntrinsicGlobalVariables flag has been specified.
+  auto const *used1 = Dst->getNamedGlobal("used1");
+  auto const *used2 = Dst->getNamedGlobal("used2");
+  auto const *useds = Dst->getNamedGlobal("llvm.compiler.used");
+
+  ASSERT_NE(nullptr, used1);
+  ASSERT_NE(nullptr, used2);
+  ASSERT_NE(nullptr, useds);
+
+  EXPECT_TRUE(used1->hasInternalLinkage());
+  EXPECT_TRUE(used2->hasInternalLinkage());
+  EXPECT_TRUE(useds->hasAppendingLinkage());
+
+  auto const *init = dyn_cast_or_null<ConstantArray>(useds->getInitializer());
+  ASSERT_NE(nullptr, init);
+  ASSERT_EQ(2U, init->getNumOperands());
+
+  auto const *used1_init =
+      dyn_cast_or_null<GlobalVariable>(init->getOperand(0U));
+  auto const *used2_init =
+      dyn_cast_or_null<ConstantExpr>(init->getOperand(1U));
+  EXPECT_EQ(used1, used1_init);
+  EXPECT_EQ(used2, used2_init->getOperand(0U));
+}
+
 } // end anonymous namespace