[llvm-link] Fix for an assertion when linking global with appending linkage

This patch fixes llvm-link assertion when linking external variable
declaration with a definition with appending linkage.

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D95126

llvm/lib/Linker/IRMover.cpp

@@ -843,6 +843,38 @@ static void getArrayElements(const Constant *C,
 Expected<Constant *>
 IRLinker::linkAppendingVarProto(GlobalVariable *DstGV,
                                 const GlobalVariable *SrcGV) {
+  // Check that both variables have compatible properties.
+  if (DstGV && !DstGV->isDeclaration() && !SrcGV->isDeclaration()) {
+    if (!SrcGV->hasAppendingLinkage() || !DstGV->hasAppendingLinkage())
+      return stringErr(
+          "Linking globals named '" + SrcGV->getName() +
+          "': can only link appending global with another appending "
+          "global!");
+
+    if (DstGV->isConstant() != SrcGV->isConstant())
+      return stringErr("Appending variables linked with different const'ness!");
+
+    if (DstGV->getAlignment() != SrcGV->getAlignment())
+      return stringErr(
+          "Appending variables with different alignment need to be linked!");
+
+    if (DstGV->getVisibility() != SrcGV->getVisibility())
+      return stringErr(
+          "Appending variables with different visibility need to be linked!");
+
+    if (DstGV->hasGlobalUnnamedAddr() != SrcGV->hasGlobalUnnamedAddr())
+      return stringErr(
+          "Appending variables with different unnamed_addr need to be linked!");
+
+    if (DstGV->getSection() != SrcGV->getSection())
+      return stringErr(
+          "Appending variables with different section name need to be linked!");
+  }
+
+  // Do not need to do anything if source is a declaration.
+  if (SrcGV->isDeclaration())
+    return DstGV;
+
   Type *EltTy = cast<ArrayType>(TypeMap.get(SrcGV->getValueType()))
                     ->getElementType();
 
@@ -868,37 +900,13 @@ IRLinker::linkAppendingVarProto(GlobalVariable *DstGV,
   }
 
   uint64_t DstNumElements = 0;
-  if (DstGV) {
+  if (DstGV && !DstGV->isDeclaration()) {
     ArrayType *DstTy = cast<ArrayType>(DstGV->getValueType());
     DstNumElements = DstTy->getNumElements();
 
-    if (!SrcGV->hasAppendingLinkage() || !DstGV->hasAppendingLinkage())
-      return stringErr(
-          "Linking globals named '" + SrcGV->getName() +
-          "': can only link appending global with another appending "
-          "global!");
-
     // Check to see that they two arrays agree on type.
     if (EltTy != DstTy->getElementType())
       return stringErr("Appending variables with different element types!");
-    if (DstGV->isConstant() != SrcGV->isConstant())
-      return stringErr("Appending variables linked with different const'ness!");
-
-    if (DstGV->getAlignment() != SrcGV->getAlignment())
-      return stringErr(
-          "Appending variables with different alignment need to be linked!");
-
-    if (DstGV->getVisibility() != SrcGV->getVisibility())
-      return stringErr(
-          "Appending variables with different visibility need to be linked!");
-
-    if (DstGV->hasGlobalUnnamedAddr() != SrcGV->hasGlobalUnnamedAddr())
-      return stringErr(
-          "Appending variables with different unnamed_addr need to be linked!");
-
-    if (DstGV->getSection() != SrcGV->getSection())
-      return stringErr(
-          "Appending variables with different section name need to be linked!");
   }
 
   SmallVector<Constant *, 16> SrcElements;
@@ -928,9 +936,10 @@ IRLinker::linkAppendingVarProto(GlobalVariable *DstGV,
 
   Constant *Ret = ConstantExpr::getBitCast(NG, TypeMap.get(SrcGV->getType()));
 
-  Mapper.scheduleMapAppendingVariable(*NG,
-                                      DstGV ? DstGV->getInitializer() : nullptr,
-                                      IsOldStructor, SrcElements);
+  Mapper.scheduleMapAppendingVariable(
+      *NG,
+      (DstGV && !DstGV->isDeclaration()) ? DstGV->getInitializer() : nullptr,
+      IsOldStructor, SrcElements);
 
   // Replace any uses of the two global variables with uses of the new
   // global.
@@ -983,8 +992,7 @@ Expected<Constant *> IRLinker::linkGlobalValueProto(GlobalValue *SGV,
     DGV = nullptr;
 
   // Handle the ultra special appending linkage case first.
-  assert(!DGV || SGV->hasAppendingLinkage() == DGV->hasAppendingLinkage());
-  if (SGV->hasAppendingLinkage())
+  if (SGV->hasAppendingLinkage() || (DGV && DGV->hasAppendingLinkage()))
     return linkAppendingVarProto(cast_or_null<GlobalVariable>(DGV),
                                  cast<GlobalVariable>(SGV));
 

llvm/lib/Linker/LinkModules.cpp

@@ -248,7 +248,7 @@ bool ModuleLinker::shouldLinkFromSource(bool &LinkFromSrc,
   }
 
   // We always have to add Src if it has appending linkage.
-  if (Src.hasAppendingLinkage()) {
+  if (Src.hasAppendingLinkage() || Dest.hasAppendingLinkage()) {
     LinkFromSrc = true;
     return false;
   }

llvm/test/Linker/Inputs/appending-global.ll

@@ -0,0 +1 @@
+@var = appending global [1 x i8* ] undef

llvm/test/Linker/appending-global-err1.ll

@@ -0,0 +1,9 @@
+; RUN: not llvm-link %s %p/Inputs/appending-global.ll -S -o - 2>&1 | FileCheck %s
+; RUN: not llvm-link %p/Inputs/appending-global.ll %s -S -o - 2>&1 | FileCheck %s
+
+; Negative test to check that global variable with appending linkage can only be
+; linked with another global variable with appending linkage.
+
+; CHECK: can only link appending global with another appending global
+
+@var = global i8* undef

llvm/test/Linker/appending-global-err2.ll

@@ -0,0 +1,9 @@
+; RUN: not llvm-link %s %p/Inputs/appending-global.ll -S -o - 2>&1 | FileCheck %s
+; RUN: not llvm-link %p/Inputs/appending-global.ll %s -S -o - 2>&1 | FileCheck %s
+
+; Negative test to check that global variables with appending linkage and
+; different constness cannot be linked.
+
+; CHECK: Appending variables linked with different const'ness
+
+@var = appending constant [1 x i8* ] undef

llvm/test/Linker/appending-global-err3.ll

@@ -0,0 +1,9 @@
+; RUN: not llvm-link %s %p/Inputs/appending-global.ll -S -o - 2>&1 | FileCheck %s
+; RUN: not llvm-link %p/Inputs/appending-global.ll %s -S -o - 2>&1 | FileCheck %s
+
+; Negative test to check that global variables with appending linkage and
+; different element types cannot be linked.
+
+; CHECK: Appending variables with different element types
+
+@var = appending global [1 x i32* ] undef

llvm/test/Linker/appending-global-err4.ll

@@ -0,0 +1,9 @@
+; RUN: not llvm-link %s %p/Inputs/appending-global.ll -S -o - 2>&1 | FileCheck %s
+; RUN: not llvm-link %p/Inputs/appending-global.ll %s -S -o - 2>&1 | FileCheck %s
+
+; Negative test to check that global variables with appending linkage and
+; different visibility cannot be linked.
+
+; CHECK: Appending variables with different visibility need to be linked
+
+@var = appending hidden global [1 x i32* ] undef

llvm/test/Linker/appending-global-err5.ll

@@ -0,0 +1,9 @@
+; RUN: not llvm-link %s %p/Inputs/appending-global.ll -S -o - 2>&1 | FileCheck %s
+; RUN: not llvm-link %p/Inputs/appending-global.ll %s -S -o - 2>&1 | FileCheck %s
+
+; Negative test to check that global variables with appending linkage and
+; different sections cannot be linked.
+
+; CHECK: Appending variables with different section name need to be linked
+
+@var = appending global [1 x i32* ] undef, section "dummy"

llvm/test/Linker/appending-global-proto.ll

@@ -0,0 +1,11 @@
+; RUN: llvm-link %s %p/Inputs/appending-global.ll -S -o - | FileCheck %s
+; RUN: llvm-link %p/Inputs/appending-global.ll %s -S -o - | FileCheck %s
+
+; Checks that we can link global variable with appending linkage with the
+; existing external declaration.
+
+; CHECK-DAG: @var = appending global [1 x i8*] undef
+; CHECK-DAG: @use = global [1 x i8*] [i8* bitcast ([1 x i8*]* @var to i8*)]
+
+@var = external global i8*
+@use = global [1 x i8*] [i8* bitcast (i8** @var to i8*)]