Index: test/tools/gold/X86/Inputs/start-lib-common.ll =================================================================== --- /dev/null +++ test/tools/gold/X86/Inputs/start-lib-common.ll @@ -0,0 +1 @@ +@x = common global i32 0, align 8 Index: test/tools/gold/X86/start-lib-common.ll =================================================================== --- /dev/null +++ test/tools/gold/X86/start-lib-common.ll @@ -0,0 +1,22 @@ +; Test the case when the preferred (larger / more aligned) version of a common +; symbol is located in a module that's not included in the link. + +; RUN: llvm-as %s -o %t1.o +; RUN: llvm-as %p/Inputs/start-lib-common.ll -o %t2.o + +; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \ +; RUN: --plugin-opt=emit-llvm \ +; RUN: -shared %t1.o --start-lib %t2.o --end-lib -o %t3.o +; RUN: llvm-dis %t3.o -o - | FileCheck %s + +@x = common global i32 0, align 4 + +; ToT gold (as of 03/2016) honors --start-lib/--end-lib, drops %t2.o and ends up +; with (i32 align 4) symbol. +; Older gold does not drop %t2.o and ends up with (i32 align 8) symbol. This is +; incorrect behavior, but this test does not verify this in order to support +; both old and new gold. + +; Check that the common symbol is not dropped completely, which was a regression +; in r262676. +; CHECK: @x = common global i32 0 Index: tools/gold/gold-plugin.cpp =================================================================== --- tools/gold/gold-plugin.cpp +++ tools/gold/gold-plugin.cpp @@ -101,14 +101,13 @@ }; struct ResolutionInfo { + uint64_t CommonSize = 0; + unsigned CommonAlign = 0; bool IsLinkonceOdr = true; bool UnnamedAddr = true; GlobalValue::VisibilityTypes Visibility = GlobalValue::DefaultVisibility; bool CommonInternal = false; bool UseCommon = false; - unsigned CommonSize = 0; - unsigned CommonAlign = 0; - claimed_file *CommonFile = nullptr; }; /// Class to own information used by a task or during its cleanup for a @@ -515,15 +514,6 @@ if (GV) { Res.UnnamedAddr &= GV->hasUnnamedAddr(); Res.IsLinkonceOdr &= GV->hasLinkOnceLinkage(); - if (GV->hasCommonLinkage()) { - Res.CommonAlign = std::max(Res.CommonAlign, GV->getAlignment()); - const DataLayout &DL = GV->getParent()->getDataLayout(); - uint64_t Size = DL.getTypeAllocSize(GV->getType()->getElementType()); - if (Size >= Res.CommonSize) { - Res.CommonSize = Size; - Res.CommonFile = &cf; - } - } Res.Visibility = getMinVisibility(Res.Visibility, GV->getVisibility()); switch (GV->getVisibility()) { case GlobalValue::DefaultVisibility: @@ -637,6 +627,8 @@ static std::unique_ptr getFunctionIndexForFile(claimed_file &F, ld_plugin_input_file &Info) { const void *View = getSymbolsAndView(F); + if (!View) + return nullptr; MemoryBufferRef BufferRef(StringRef((const char *)View, Info.filesize), Info.name); @@ -663,7 +655,8 @@ getModuleForFile(LLVMContext &Context, claimed_file &F, const void *View, ld_plugin_input_file &Info, raw_fd_ostream *ApiFile, StringSet<> &Internalize, StringSet<> &Maybe, - std::vector &Keep) { + std::vector &Keep, + StringMap &Realign) { MemoryBufferRef BufferRef(StringRef((const char *)View, Info.filesize), Info.name); ErrorOr> ObjOrErr = @@ -725,7 +718,6 @@ // Override gold's resolution for common symbols. We want the largest // one to win. if (GV->hasCommonLinkage()) { - cast(GV)->setAlignment(Res.CommonAlign); if (Resolution == LDPR_PREVAILING_DEF_IRONLY) Res.CommonInternal = true; @@ -733,14 +725,29 @@ Resolution == LDPR_PREVAILING_DEF) Res.UseCommon = true; - if (Res.CommonFile == &F && Res.UseCommon) { + const DataLayout &DL = GV->getParent()->getDataLayout(); + uint64_t Size = DL.getTypeAllocSize(GV->getType()->getElementType()); + unsigned Align = GV->getAlignment(); + + if (Res.UseCommon && Size >= Res.CommonSize) { + // Take GV. if (Res.CommonInternal) Resolution = LDPR_PREVAILING_DEF_IRONLY; else Resolution = LDPR_PREVAILING_DEF; + cast(GV)->setAlignment( + std::max(Res.CommonAlign, Align)); } else { + // Do not take GV, it's smaller than what we already have in the + // combined module. Resolution = LDPR_PREEMPTED_IR; + if (Align > Res.CommonAlign) + // Need to raise the alignment though. + Realign[Sym.name] = Align; } + + Res.CommonSize = std::max(Res.CommonSize, Size); + Res.CommonAlign = std::max(Res.CommonAlign, Align); } switch (Resolution) { @@ -1058,8 +1065,9 @@ raw_fd_ostream *ApiFile, StringSet<> &Internalize, StringSet<> &Maybe) { std::vector Keep; - std::unique_ptr M = getModuleForFile(Context, F, View, File, ApiFile, - Internalize, Maybe, Keep); + StringMap Realign; + std::unique_ptr M = getModuleForFile( + Context, F, View, File, ApiFile, Internalize, Maybe, Keep, Realign); if (!M.get()) return false; if (!options::triple.empty()) @@ -1068,9 +1076,17 @@ M->setTargetTriple(DefaultTriple); } - if (L.move(std::move(M), Keep, [](GlobalValue &, IRMover::ValueAdder) {})) - return true; - return false; + if (!L.move(std::move(M), Keep, [](GlobalValue &, IRMover::ValueAdder) {})) + return false; + + for (const auto &I : Realign) { + GlobalValue *Dst = L.getModule().getNamedValue(I.first()); + if (!Dst) + continue; + cast(Dst)->setAlignment(I.second); + } + + return true; } /// Perform the ThinLTO backend on a single module, invoking the LTO and codegen @@ -1116,6 +1132,8 @@ // safe by default. PluginInputFile InputFile(F.handle); const void *View = getSymbolsAndView(F); + if (!View) + continue; SmallString<128> Filename; if (!options::obj_path.empty()) @@ -1211,6 +1229,8 @@ for (claimed_file &F : Modules) { PluginInputFile InputFile(F.handle); const void *View = getSymbolsAndView(F); + if (!View) + continue; if (linkInModule(Context, L, F, View, InputFile.file(), ApiFile, Internalize, Maybe)) message(LDPL_FATAL, "Failed to link module");