From d1fec24fb5de8fe9ff65258a79989b7d7694c441 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Tue, 10 Nov 2015 21:09:06 +0000 Subject: [PATCH] Ensure ModuleLinker materializes complete comdat groups 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. Reviewers: rafael Subscribers: dexonsmith, davidxl, llvm-commits Differential Revision: http://reviews.llvm.org/D14516 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@252647 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Linker/LinkModules.cpp | 16 +++++++++++++++ .../Inputs/only-needed-named-metadata.ll | 3 --- test/Linker/comdat_group.ll | 20 +++++++++++++++++++ test/Linker/only-needed-named-metadata.ll | 14 +------------ 4 files changed, 37 insertions(+), 16 deletions(-) create mode 100644 test/Linker/comdat_group.ll diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp index 8968377663e..71af9d0d61e 100644 --- a/lib/Linker/LinkModules.cpp +++ b/lib/Linker/LinkModules.cpp @@ -509,6 +509,8 @@ private: ComdatsChosen; bool getComdatResult(const Comdat *SrcC, Comdat::SelectionKind &SK, bool &LinkFromSrc); + // Keep track of the global value members of each comdat in source. + DenseMap> ComdatMembers; /// Given a global in the source module, return the global in the /// destination module that is being linked to, if any. @@ -1386,6 +1388,7 @@ bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) { 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; @@ -1589,6 +1592,19 @@ void ModuleLinker::linkAliasBody(GlobalAlias &Dst, GlobalAlias &Src) { 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(Dst)) DGV->setLinkage(GlobalValue::InternalLinkage); diff --git a/test/Linker/Inputs/only-needed-named-metadata.ll b/test/Linker/Inputs/only-needed-named-metadata.ll index 95151692d3b..fa7bc2e3cc8 100644 --- a/test/Linker/Inputs/only-needed-named-metadata.ll +++ b/test/Linker/Inputs/only-needed-named-metadata.ll @@ -2,11 +2,8 @@ declare i32 @foo() -declare void @c1_a() - define void @bar() { load i32, i32* @X call i32 @foo() - call void @c1_a() ret void } diff --git a/test/Linker/comdat_group.ll b/test/Linker/comdat_group.ll new file mode 100644 index 00000000000..efaa94f2858 --- /dev/null +++ b/test/Linker/comdat_group.ll @@ -0,0 +1,20 @@ +; 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 +} diff --git a/test/Linker/only-needed-named-metadata.ll b/test/Linker/only-needed-named-metadata.ll index 36b42590628..a0df5bf2238 100644 --- a/test/Linker/only-needed-named-metadata.ll +++ b/test/Linker/only-needed-named-metadata.ll @@ -25,17 +25,6 @@ ; 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 @@ -64,7 +53,7 @@ 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} @@ -72,4 +61,3 @@ entry: !4 = !{void (...)* @weakalias} !5 = !{void (...)* @analias} !6 = !{void (...)* @linkoncealias} -!7 = !{void ()* @c1} -- 2.34.1