From 370d528a0521f38bef33e985b929609c74ee32ae Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Tue, 22 Mar 2016 21:35:47 +0000 Subject: Drop comdats from the dst module if they are not selected. A really unfortunate design of llvm-link and related libraries is that they operate one module at a time. This means they can copy a GV to the destination module that should not be there in the final result because a later bitcode file takes precedence. We already handled cases like a strong GV replacing a weak for example. One case that is not currently handled is a comdat replacing another. This doesn't happen in ELF, but with COFF largest selection kind it is possible. In "llvm-link a.ll b.ll" if the selected comdat was from a.ll, everything will work and we will not copy the comdat from b.ll. But if we run "llvm-link b.ll a.ll", we fail to delete the already copied comdat from b.ll. This patch fixes that. llvm-svn: 264103 --- llvm/lib/Linker/LinkModules.cpp | 73 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) (limited to 'llvm/lib/Linker/LinkModules.cpp') diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp index 2ee0b9664a3..8eeccb369df 100644 --- a/llvm/lib/Linker/LinkModules.cpp +++ b/llvm/lib/Linker/LinkModules.cpp @@ -102,6 +102,11 @@ class ModuleLinker { return DGV; } + /// Drop GV if it is a member of a comdat that we are dropping. + /// This can happen with COFF's largest selection kind. + void dropReplacedComdat(GlobalValue &GV, + const DenseSet &ReplacedDstComdats); + bool linkIfNeeded(GlobalValue &GV); /// Helper method to check if we are importing from the current source @@ -449,7 +454,45 @@ void ModuleLinker::addLazyFor(GlobalValue &GV, IRMover::ValueAdder Add) { } } +void ModuleLinker::dropReplacedComdat( + GlobalValue &GV, const DenseSet &ReplacedDstComdats) { + Comdat *C = GV.getComdat(); + if (!C) + return; + if (!ReplacedDstComdats.count(C)) + return; + if (GV.use_empty()) { + GV.eraseFromParent(); + return; + } + + if (auto *F = dyn_cast(&GV)) { + F->deleteBody(); + } else if (auto *Var = dyn_cast(&GV)) { + Var->setInitializer(nullptr); + } else { + auto &Alias = cast(GV); + Module &M = *Alias.getParent(); + PointerType &Ty = *cast(Alias.getType()); + GlobalValue *Declaration; + if (auto *FTy = dyn_cast(Alias.getValueType())) { + Declaration = Function::Create(FTy, GlobalValue::ExternalLinkage, "", &M); + } else { + Declaration = + new GlobalVariable(M, Ty.getElementType(), /*isConstant*/ false, + GlobalValue::ExternalLinkage, + /*Initializer*/ nullptr); + } + Declaration->takeName(&Alias); + Alias.replaceAllUsesWith(Declaration); + Alias.eraseFromParent(); + } +} + bool ModuleLinker::run() { + Module &DstM = Mover.getModule(); + DenseSet ReplacedDstComdats; + for (const auto &SMEC : SrcM->getComdatSymbolTable()) { const Comdat &C = SMEC.getValue(); if (ComdatsChosen.count(&C)) @@ -459,6 +502,35 @@ bool ModuleLinker::run() { if (getComdatResult(&C, SK, LinkFromSrc)) return true; ComdatsChosen[&C] = std::make_pair(SK, LinkFromSrc); + + if (!LinkFromSrc) + continue; + + Module::ComdatSymTabType &ComdatSymTab = DstM.getComdatSymbolTable(); + Module::ComdatSymTabType::iterator DstCI = ComdatSymTab.find(C.getName()); + if (DstCI == ComdatSymTab.end()) + continue; + + // The source comdat is replacing the dest one. + const Comdat *DstC = &DstCI->second; + ReplacedDstComdats.insert(DstC); + } + + // Alias have to go first, since we are not able to find their comdats + // otherwise. + for (auto I = DstM.alias_begin(), E = DstM.alias_end(); I != E;) { + GlobalAlias &GV = *I++; + dropReplacedComdat(GV, ReplacedDstComdats); + } + + for (auto I = DstM.global_begin(), E = DstM.global_end(); I != E;) { + GlobalVariable &GV = *I++; + dropReplacedComdat(GV, ReplacedDstComdats); + } + + for (auto I = DstM.begin(), E = DstM.end(); I != E;) { + Function &GV = *I++; + dropReplacedComdat(GV, ReplacedDstComdats); } for (GlobalVariable &GV : SrcM->globals()) @@ -507,7 +579,6 @@ bool ModuleLinker::run() { }, ValIDToTempMDMap, false)) return true; - Module &DstM = Mover.getModule(); for (auto &P : Internalize) { GlobalValue *GV = DstM.getNamedValue(P.first()); GV->setLinkage(GlobalValue::InternalLinkage); -- cgit v1.2.3