[lld] r208797 - Fix regression introduced in r205566.
Rui Ueyama
ruiu at google.com
Wed May 14 10:29:27 PDT 2014
Author: ruiu
Date: Wed May 14 12:29:27 2014
New Revision: 208797
URL: http://llvm.org/viewvc/llvm-project?rev=208797&view=rev
Log:
Fix regression introduced in r205566.
In r205566, I made a change to Resolver so that Resolver revisit
only archive files in --start-group and --end-group pair. That's
not correct, as it also has to revisit DSO files.
This patch is to fix the issue.
Added a test to demonstrate the fix. I confirmed that it succeeded
before r205566, failed after r205566, and is ok with this patch.
Differential Revision: http://reviews.llvm.org/D3734
Added:
lld/trunk/test/elf/X86_64/Inputs/group/libfn2.so (with props)
Modified:
lld/trunk/include/lld/Core/Resolver.h
lld/trunk/include/lld/Core/SymbolTable.h
lld/trunk/include/lld/Driver/GnuLdInputGraph.h
lld/trunk/lib/Core/Resolver.cpp
lld/trunk/lib/Core/SymbolTable.cpp
lld/trunk/test/elf/X86_64/Inputs/group/group.sh
lld/trunk/test/elf/X86_64/startGroupEndGroup.test
Modified: lld/trunk/include/lld/Core/Resolver.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Core/Resolver.h?rev=208797&r1=208796&r2=208797&view=diff
==============================================================================
--- lld/trunk/include/lld/Core/Resolver.h (original)
+++ lld/trunk/include/lld/Core/Resolver.h Wed May 14 12:29:27 2014
@@ -33,7 +33,7 @@ public:
// InputFiles::Handler methods
void doDefinedAtom(const DefinedAtom&);
- void doUndefinedAtom(const UndefinedAtom&);
+ bool doUndefinedAtom(const UndefinedAtom &);
void doSharedLibraryAtom(const SharedLibraryAtom &);
void doAbsoluteAtom(const AbsoluteAtom &);
Modified: lld/trunk/include/lld/Core/SymbolTable.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Core/SymbolTable.h?rev=208797&r1=208796&r2=208797&view=diff
==============================================================================
--- lld/trunk/include/lld/Core/SymbolTable.h (original)
+++ lld/trunk/include/lld/Core/SymbolTable.h Wed May 14 12:29:27 2014
@@ -39,16 +39,16 @@ public:
explicit SymbolTable(const LinkingContext &);
/// @brief add atom to symbol table
- void add(const DefinedAtom &);
+ bool add(const DefinedAtom &);
/// @brief add atom to symbol table
- void add(const UndefinedAtom &);
+ bool add(const UndefinedAtom &);
/// @brief add atom to symbol table
- void add(const SharedLibraryAtom &);
+ bool add(const SharedLibraryAtom &);
/// @brief add atom to symbol table
- void add(const AbsoluteAtom &);
+ bool add(const AbsoluteAtom &);
/// @brief checks if name is in symbol table and if so atom is not
/// UndefinedAtom
@@ -102,8 +102,8 @@ private:
};
typedef llvm::DenseSet<const DefinedAtom*, AtomMappingInfo> AtomContentSet;
- void addByName(const Atom &);
- void addByContent(const DefinedAtom &);
+ bool addByName(const Atom &);
+ bool addByContent(const DefinedAtom &);
const LinkingContext &_context;
AtomToAtom _replacedAtoms;
Modified: lld/trunk/include/lld/Driver/GnuLdInputGraph.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Driver/GnuLdInputGraph.h?rev=208797&r1=208796&r2=208797&view=diff
==============================================================================
--- lld/trunk/include/lld/Driver/GnuLdInputGraph.h (original)
+++ lld/trunk/include/lld/Driver/GnuLdInputGraph.h Wed May 14 12:29:27 2014
@@ -80,8 +80,9 @@ public:
/// to start processing files as part of the input element from beginning.
/// Reset the next file index to 0 only if the node is an archive library.
void resetNextIndex() override {
- if (_files[0]->kind() == File::kindArchiveLibrary &&
- !_attributes._isWholeArchive) {
+ auto kind = _files[0]->kind();
+ if (kind == File::kindSharedLibrary ||
+ (kind == File::kindArchiveLibrary && !_attributes._isWholeArchive)) {
_nextFileIndex = 0;
}
}
Modified: lld/trunk/lib/Core/Resolver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/Resolver.cpp?rev=208797&r1=208796&r2=208797&view=diff
==============================================================================
--- lld/trunk/lib/Core/Resolver.cpp (original)
+++ lld/trunk/lib/Core/Resolver.cpp Wed May 14 12:29:27 2014
@@ -48,15 +48,12 @@ private:
} // namespace
void Resolver::handleFile(const File &file) {
- bool isEmpty = file.defined().empty() && file.undefined().empty() &&
- file.sharedLibrary().empty() && file.absolute().empty();
- if (isEmpty)
- return;
-
+ bool undefAdded = false;
for (const DefinedAtom *atom : file.defined())
doDefinedAtom(*atom);
for (const UndefinedAtom *atom : file.undefined())
- doUndefinedAtom(*atom);
+ if (doUndefinedAtom(*atom))
+ undefAdded = true;
for (const SharedLibraryAtom *atom : file.sharedLibrary())
doSharedLibraryAtom(*atom);
for (const AbsoluteAtom *atom : file.absolute())
@@ -65,7 +62,9 @@ void Resolver::handleFile(const File &fi
// Notify the input file manager of the fact that we have made some progress
// on linking using the current input file. It may want to know the fact for
// --start-group/--end-group.
- _context.getInputGraph().notifyProgress();
+ if (undefAdded) {
+ _context.getInputGraph().notifyProgress();
+ }
}
void Resolver::forEachUndefines(bool searchForOverrides,
@@ -124,7 +123,7 @@ void Resolver::handleSharedLibrary(const
});
}
-void Resolver::doUndefinedAtom(const UndefinedAtom &atom) {
+bool Resolver::doUndefinedAtom(const UndefinedAtom &atom) {
DEBUG_WITH_TYPE("resolver", llvm::dbgs()
<< " UndefinedAtom: "
<< llvm::format("0x%09lX", &atom)
@@ -134,7 +133,7 @@ void Resolver::doUndefinedAtom(const Und
_atoms.push_back(&atom);
// tell symbol table
- _symbolTable.add(atom);
+ bool newUndefAdded = _symbolTable.add(atom);
// If the undefined symbol has an alternative name, try to resolve the
// symbol with the name to give it a second chance. This feature is used
@@ -145,6 +144,7 @@ void Resolver::doUndefinedAtom(const Und
_symbolTable.addReplacement(&atom, fallbackAtom);
}
}
+ return newUndefAdded;
}
/// \brief Add the section group and the group-child reference members.
@@ -178,7 +178,8 @@ bool Resolver::maybeAddSectionGroupOrGnu
return true;
}
-// called on each atom when a file is added
+// Called on each atom when a file is added. Returns true if a given
+// atom is added to the symbol table.
void Resolver::doDefinedAtom(const DefinedAtom &atom) {
DEBUG_WITH_TYPE("resolver", llvm::dbgs()
<< " DefinedAtom: "
Modified: lld/trunk/lib/Core/SymbolTable.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/SymbolTable.cpp?rev=208797&r1=208796&r2=208797&view=diff
==============================================================================
--- lld/trunk/lib/Core/SymbolTable.cpp (original)
+++ lld/trunk/lib/Core/SymbolTable.cpp Wed May 14 12:29:27 2014
@@ -32,26 +32,26 @@
namespace lld {
SymbolTable::SymbolTable(const LinkingContext &context) : _context(context) {}
-void SymbolTable::add(const UndefinedAtom &atom) { addByName(atom); }
+bool SymbolTable::add(const UndefinedAtom &atom) { return addByName(atom); }
-void SymbolTable::add(const SharedLibraryAtom &atom) { addByName(atom); }
+bool SymbolTable::add(const SharedLibraryAtom &atom) { return addByName(atom); }
-void SymbolTable::add(const AbsoluteAtom &atom) { addByName(atom); }
+bool SymbolTable::add(const AbsoluteAtom &atom) { return addByName(atom); }
-void SymbolTable::add(const DefinedAtom &atom) {
+bool SymbolTable::add(const DefinedAtom &atom) {
if (!atom.name().empty() &&
atom.scope() != DefinedAtom::scopeTranslationUnit) {
// Named atoms cannot be merged by content.
assert(atom.merge() != DefinedAtom::mergeByContent);
// Track named atoms that are not scoped to file (static).
- addByName(atom);
- return;
+ return addByName(atom);
}
if (atom.merge() == DefinedAtom::mergeByContent) {
// Named atoms cannot be merged by content.
assert(atom.name().empty());
- addByContent(atom);
+ return addByContent(atom);
}
+ return false;
}
const Atom *SymbolTable::findGroup(StringRef sym) {
@@ -162,16 +162,20 @@ static uint64_t sectionSize(const Define
+ getSizeFollowReferences(atom, lld::Reference::kindLayoutAfter);
}
-void SymbolTable::addByName(const Atom &newAtom) {
+bool SymbolTable::addByName(const Atom &newAtom) {
StringRef name = newAtom.name();
assert(!name.empty());
const Atom *existing = findByName(name);
if (existing == nullptr) {
// Name is not in symbol table yet, add it associate with this atom.
_nameTable[name] = &newAtom;
- return;
+ return true;
}
+ // Do nothing if the same object is added more than once.
+ if (existing == &newAtom)
+ return false;
+
// Name is already in symbol table and associated with another atom.
bool useNew = true;
switch (collide(existing->definition(), newAtom.definition())) {
@@ -301,6 +305,7 @@ void SymbolTable::addByName(const Atom &
// New atom is not being used. Add it to replacement table.
_replacedAtoms[&newAtom] = existing;
}
+ return false;
}
unsigned SymbolTable::AtomMappingInfo::getHashValue(const DefinedAtom *atom) {
@@ -332,17 +337,18 @@ bool SymbolTable::AtomMappingInfo::isEqu
return memcmp(lc.data(), rc.data(), lc.size()) == 0;
}
-void SymbolTable::addByContent(const DefinedAtom & newAtom) {
+bool SymbolTable::addByContent(const DefinedAtom &newAtom) {
// Currently only read-only constants can be merged.
assert(newAtom.permissions() == DefinedAtom::permR__);
AtomContentSet::iterator pos = _contentTable.find(&newAtom);
if (pos == _contentTable.end()) {
_contentTable.insert(&newAtom);
- return;
+ return true;
}
const Atom* existing = *pos;
// New atom is not being used. Add it to replacement table.
_replacedAtoms[&newAtom] = existing;
+ return false;
}
const Atom *SymbolTable::findByName(StringRef sym) {
Modified: lld/trunk/test/elf/X86_64/Inputs/group/group.sh
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf/X86_64/Inputs/group/group.sh?rev=208797&r1=208796&r2=208797&view=diff
==============================================================================
--- lld/trunk/test/elf/X86_64/Inputs/group/group.sh (original)
+++ lld/trunk/test/elf/X86_64/Inputs/group/group.sh Wed May 14 12:29:27 2014
@@ -31,6 +31,7 @@ fn2();
gcc -c 1.c fn.c fn2.c fn1.c
ar cr libfn.a fn.o fn2.o
ar cr libfn1.a fn1.o
+lld -flavor gnu -target x86_64 -shared -o libfn2.so fn2.o
lld -flavor gnu -target x86_64 1.o libfn.a libfn1.a -o x
lld -flavor gnu -target x86_64 1.o --start-group libfn.a libfn1.a --end-group -o x
lld -flavor gnu -target x86_64 1.o --start-group fn.o fn2.o fn1.o --end-group -o x
Added: lld/trunk/test/elf/X86_64/Inputs/group/libfn2.so
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf/X86_64/Inputs/group/libfn2.so?rev=208797&view=auto
==============================================================================
Binary files lld/trunk/test/elf/X86_64/Inputs/group/libfn2.so (added) and lld/trunk/test/elf/X86_64/Inputs/group/libfn2.so Wed May 14 12:29:27 2014 differ
Propchange: lld/trunk/test/elf/X86_64/Inputs/group/libfn2.so
------------------------------------------------------------------------------
svn:executable = *
Modified: lld/trunk/test/elf/X86_64/startGroupEndGroup.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf/X86_64/startGroupEndGroup.test?rev=208797&r1=208796&r2=208797&view=diff
==============================================================================
--- lld/trunk/test/elf/X86_64/startGroupEndGroup.test (original)
+++ lld/trunk/test/elf/X86_64/startGroupEndGroup.test Wed May 14 12:29:27 2014
@@ -18,6 +18,11 @@ RUN: lld -flavor gnu -target x86_64 %p/I
RUN: --whole-archive %p/Inputs/group/libfn.a --no-whole-archive \
RUN: %p/Inputs/group/libfn1.a --end-group -o %t3
+# Defined symbols in a shared library.
+RUN: lld -flavor gnu -target x86_64 %p/Inputs/group/1.o --start-group \
+RUN: %p/Inputs/group/libfn2.so %p/Inputs/group/fn1.o %p/Inputs/group/fn.o \
+RUN: --end-group -o %t4
+
# Test alias options too, as they are more widely used
# Test group
RUN: lld -flavor gnu -target x86_64 %p/Inputs/group/1.o '-(' \
More information about the llvm-commits
mailing list