Ensure ModuleLinker materializes complete comdat groups

[tejohnson]

Summary

The module linker lazy links some "discardable if unused" global
values (e.g. linkonce), materializing and linking them only
if they are referenced in the module. If a comdat group contains a
linkonce member that is not referenced, however, it would not be
materialized and linked, leading to an incomplete comdat group.

If there are other object files not part of the same LTO link that also
define and use that comdat group, the linker may select the incomplete
group leading to link time unsats.

To solve this, whenever a global value body is linked, make sure we
materialize any other members of the same comdat group that are not yet
materialized. This ensures they are in the lazy link list and get linked
as well.

Added new test and adjusted old test to remove parts that didn't
make sense with fix.

[• rafael]

LGTM

llvm/trunk/lib/Linker/LinkModules.cpp

bool computeResultingSelectionKind(StringRef ComdatName,
                                      Comdat::SelectionKind Src,
                                      Comdat::SelectionKind Dst,
                                      Comdat::SelectionKind &Result,
                                      bool &LinkFromSrc);
   std::map<const Comdat *, std::pair<Comdat::SelectionKind, bool>>
       ComdatsChosen;
   bool getComdatResult(const Comdat *SrcC, Comdat::SelectionKind &SK,
                        bool &LinkFromSrc);
+  // Keep track of the global value members of each comdat in source.
+  DenseMap<const Comdat *, std::vector<GlobalValue *>> ComdatMembers;
 
   /// Given a global in the source module, return the global in the
   /// destination module that is being linked to, if any.
   GlobalValue *getLinkedToGlobal(const GlobalValue *SrcGV) {
     // If the source has no name it can't link.  If it has local linkage,
     // there is no name match-up going on.
     if (!SrcGV->hasName() || SrcGV->hasLocalLinkage())
       return nullptr;
bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) {
   Comdat *C = nullptr;
   bool HasUnnamedAddr = SGV->hasUnnamedAddr();
 
   if (const Comdat *SC = SGV->getComdat()) {
     Comdat::SelectionKind SK;
     std::tie(SK, LinkFromSrc) = ComdatsChosen[SC];
     C = DstM->getOrInsertComdat(SC->getName());
     C->setSelectionKind(SK);
+    ComdatMembers[SC].push_back(SGV);
   } else if (DGV) {
     if (shouldLinkFromSource(LinkFromSrc, *DGV, *SGV))
       return true;
   }
 
   if (!LinkFromSrc) {
     // Track the source global so that we don't attempt to copy it over when
     // processing global initializers.
void ModuleLinker::linkAliasBody(GlobalAlias &Dst, GlobalAlias &Src) {
   Constant *Val = MapValue(Aliasee, ValueMap, RF_MoveDistinctMDs, &TypeMap,
                            &ValMaterializer);
   Dst.setAliasee(Val);
 }
 
 bool ModuleLinker::linkGlobalValueBody(GlobalValue &Src) {
   Value *Dst = ValueMap[&Src];
   assert(Dst);
+  if (const Comdat *SC = Src.getComdat()) {
+    // To ensure that we don't generate an incomplete comdat group,
+    // we must materialize and map in any other members that are not
+    // yet materialized in Dst, which also ensures their definitions
+    // are linked in. Otherwise, linkonce and other lazy linked GVs will
+    // not be materialized if they aren't referenced.
+    for (auto *SGV : ComdatMembers[SC]) {
+      if (ValueMap[SGV])
+        continue;
+      Value *NewV = ValMaterializer.materializeValueFor(SGV);
+      ValueMap[SGV] = NewV;
+    }
+  }
   if (shouldInternalizeLinkedSymbols())
     if (auto *DGV = dyn_cast<GlobalValue>(Dst))
       DGV->setLinkage(GlobalValue::InternalLinkage);
   if (auto *F = dyn_cast<Function>(&Src))
     return linkFunctionBody(cast<Function>(*Dst), *F);
   if (auto *GVar = dyn_cast<GlobalVariable>(&Src)) {
     linkGlobalInit(cast<GlobalVariable>(*Dst), *GVar);
     return false;

llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll

 @X = external global i32
 
 declare i32 @foo()
 
-declare void @c1_a()
-
 define void @bar() {
   load i32, i32* @X
   call i32 @foo()
-  call void @c1_a()
   ret void
 }

llvm/trunk/test/Linker/comdat_group.ll

+; RUN: llvm-as -function-summary %s -o %t.bc
+
+; Ensure complete comdat group is materialized
+; RUN: llvm-link %t.bc -S | FileCheck %s
+; CHECK: $linkoncecomdat = comdat any
+; CHECK: @linkoncecomdat = linkonce global i32 2
+; CHECK: @linkoncecomdat_unref_var = linkonce global i32 2, comdat($linkoncecomdat)
+; CHECK: define linkonce void @linkoncecomdat_unref_func() comdat($linkoncecomdat)
+
+$linkoncecomdat = comdat any
+@linkoncecomdat = linkonce global i32 2, comdat($linkoncecomdat)
+@linkoncecomdat_unref_var = linkonce global i32 2, comdat($linkoncecomdat)
+define linkonce void @linkoncecomdat_unref_func() comdat($linkoncecomdat) {
+  ret void
+}
+; Reference one member of comdat so that comdat is generated.
+define void @ref_linkoncecomdat() {
+  load i32, i32* @linkoncecomdat, align 4
+  ret void
+}

llvm/trunk/test/Linker/only-needed-named-metadata.ll

 ; ONLYNEEDED-NOT:@unused()
 ; ONLYNEEDED-NOT:@unused_linkonce()
 ; ONLYNEEDED-NOT:@linkoncealias
 ; ONLYNEEDED-NOT:@linkoncefunc2()
 ; ONLYNEEDED-NOT:@weakalias
 ; ONLYNEEDED-NOT:@globalfunc1()
 ; ONLYNEEDED-NOT:@analias
 ; ONLYNEEDED-NOT:@globalfunc2()
-; ONLYNEEDED-NOT:@c1_c
-; ONLYNEEDED-NOT:@c1()
-
-$c1 = comdat any
-@c1_c = global i32 0, comdat($c1)
-define void @c1() comdat {
-  ret void
-}
-define void @c1_a() comdat($c1) {
-  ret void
-}
 
 @X = global i32 5
 @U = global i32 6
 @U_linkonce = linkonce_odr hidden global i32 6
 define i32 @foo() { ret i32 7 }
 define i32 @unused() { ret i32 8 }
 define linkonce_odr hidden i32 @unused_linkonce() { ret i32 8 }
 @linkoncealias = alias void (...), bitcast (void ()* @linkoncefunc2 to void (...)*)

 }
 
 $linkoncefunc2 = comdat any
 define linkonce_odr void @linkoncefunc2() #0 comdat {
 entry:
   ret void
 }
 
-!llvm.named = !{!0, !1, !2, !3, !4, !5, !6, !7}
+!llvm.named = !{!0, !1, !2, !3, !4, !5, !6}
 !0 = !{i32 ()* @unused}
 !1 = !{i32* @U}
 !2 = !{i32 ()* @unused_linkonce}
 !3 = !{i32* @U_linkonce}
 !4 = !{void (...)* @weakalias}
 !5 = !{void (...)* @analias}
 !6 = !{void (...)* @linkoncealias}
-!7 = !{void ()* @c1}