[llvm] r263180 - [gold] Fix common symbols handling.
Evgeniy Stepanov via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 10 16:51:57 PST 2016
Author: eugenis
Date: Thu Mar 10 18:51:57 2016
New Revision: 263180
URL: http://llvm.org/viewvc/llvm-project?rev=263180&view=rev
Log:
[gold] Fix common symbols handling.
LLVM Gold plugin decides which instance of a common symbol it wants
based on the symbol size in claim_file_hook. If the file that
contains the chosen instance is later dropped from the link, we end
up with an undefined reference.
This change delays this decision until the set of the included files
is known.
Added:
llvm/trunk/test/tools/gold/X86/Inputs/start-lib-common.ll
llvm/trunk/test/tools/gold/X86/start-lib-common.ll
Modified:
llvm/trunk/tools/gold/gold-plugin.cpp
Added: llvm/trunk/test/tools/gold/X86/Inputs/start-lib-common.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/gold/X86/Inputs/start-lib-common.ll?rev=263180&view=auto
==============================================================================
--- llvm/trunk/test/tools/gold/X86/Inputs/start-lib-common.ll (added)
+++ llvm/trunk/test/tools/gold/X86/Inputs/start-lib-common.ll Thu Mar 10 18:51:57 2016
@@ -0,0 +1 @@
+ at x = common global i32 0, align 8
Added: llvm/trunk/test/tools/gold/X86/start-lib-common.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/gold/X86/start-lib-common.ll?rev=263180&view=auto
==============================================================================
--- llvm/trunk/test/tools/gold/X86/start-lib-common.ll (added)
+++ llvm/trunk/test/tools/gold/X86/start-lib-common.ll Thu Mar 10 18:51:57 2016
@@ -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
+
+ at 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
Modified: llvm/trunk/tools/gold/gold-plugin.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/gold/gold-plugin.cpp?rev=263180&r1=263179&r2=263180&view=diff
==============================================================================
--- llvm/trunk/tools/gold/gold-plugin.cpp (original)
+++ llvm/trunk/tools/gold/gold-plugin.cpp Thu Mar 10 18:51:57 2016
@@ -101,14 +101,13 @@ struct PluginInputFile {
};
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 @@ static ld_plugin_status claim_file_hook(
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 const void *getSymbolsAndView(cla
static std::unique_ptr<FunctionInfoIndex>
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 @@ static std::unique_ptr<Module>
getModuleForFile(LLVMContext &Context, claimed_file &F, const void *View,
ld_plugin_input_file &Info, raw_fd_ostream *ApiFile,
StringSet<> &Internalize, StringSet<> &Maybe,
- std::vector<GlobalValue *> &Keep) {
+ std::vector<GlobalValue *> &Keep,
+ StringMap<unsigned> &Realign) {
MemoryBufferRef BufferRef(StringRef((const char *)View, Info.filesize),
Info.name);
ErrorOr<std::unique_ptr<object::IRObjectFile>> ObjOrErr =
@@ -725,7 +718,6 @@ getModuleForFile(LLVMContext &Context, c
// Override gold's resolution for common symbols. We want the largest
// one to win.
if (GV->hasCommonLinkage()) {
- cast<GlobalVariable>(GV)->setAlignment(Res.CommonAlign);
if (Resolution == LDPR_PREVAILING_DEF_IRONLY)
Res.CommonInternal = true;
@@ -733,14 +725,29 @@ getModuleForFile(LLVMContext &Context, c
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<GlobalVariable>(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 @@ static bool linkInModule(LLVMContext &Co
raw_fd_ostream *ApiFile, StringSet<> &Internalize,
StringSet<> &Maybe) {
std::vector<GlobalValue *> Keep;
- std::unique_ptr<Module> M = getModuleForFile(Context, F, View, File, ApiFile,
- Internalize, Maybe, Keep);
+ StringMap<unsigned> Realign;
+ std::unique_ptr<Module> M = getModuleForFile(
+ Context, F, View, File, ApiFile, Internalize, Maybe, Keep, Realign);
if (!M.get())
return false;
if (!options::triple.empty())
@@ -1068,9 +1076,17 @@ static bool linkInModule(LLVMContext &Co
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<GlobalVariable>(Dst)->setAlignment(I.second);
+ }
+
+ return true;
}
/// Perform the ThinLTO backend on a single module, invoking the LTO and codegen
@@ -1116,6 +1132,8 @@ static void thinLTOBackends(raw_fd_ostre
// 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 @@ static ld_plugin_status allSymbolsReadHo
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");
More information about the llvm-commits
mailing list