This is an archive of the discontinued LLVM Phabricator instance.

Differential D501

[ELF] Fix weak symbols order
ClosedPublic

Authored by shankarke on Mar 6 2013, 11:59 AM.

Details

Reviewers
Bigcheese
Summary

This patch fixes weak symbol order in the executable. This is needed to fix helloworld working with glibc on x86_64 with layout pass.

This makes helloworld pass on x86_64/hexagon with LayoutPass.

Diff Detail

Event Timeline

Bigcheese accepted this revision.Mar 6 2013, 1:35 PM

Looks fine with these changes. Also, you're missing the binary files.

lib/Passes/LayoutPass.cpp
162
.empty()
204–205

No newline.

219–226

This function is getting nested too deeply. It should probably be split into multiple functions.

lib/ReaderWriter/ELF/File.h
464

Drop the extra parens.

if ((*si)->getBinding() == llvm::ELF::STB_WEAK && anonAtom)
shankarke added a comment.Mar 6 2013, 1:48 PM

will add the binary files too.

lib/Passes/LayoutPass.cpp
162

atom doesnot have empty() function.

204–205

Ok.

219–226

Will take this in the upcoming patches.

lib/ReaderWriter/ELF/File.h
464

ok.

Eugene.Zelenko closed this revision.Oct 5 2016, 3:32 PM
Eugene.Zelenko added a subscriber: Eugene.Zelenko.

Committed in rL176583.

Revision Contents

PathSize
lib/
Passes/
51 lines
ReaderWriter/
ELF/
78 lines
test/
elf/
X86_64/
Inputs/
10 lines
20 lines
6 lines
14 lines
15 lines
19 lines
56 lines
34 lines

Diff 1198

lib/Passes/LayoutPass.cpp

Show First 20 Lines • Show All 148 Lines • ▼ Show 20 Lines for (const Reference *r : *ai) {
if (targetFollowOnAtomsIter != _followOnNexts.end()) if (targetFollowOnAtomsIter != _followOnNexts.end())
a = targetFollowOnAtomsIter->second; a = targetFollowOnAtomsIter->second;
else else
break; break;
} // while true } // while true
} else { // the atom could be part of chain already } else { // the atom could be part of chain already
// Get to the root of the chain // Get to the root of the chain
const DefinedAtom *a = _followOnRoots[targetAtom]; const DefinedAtom *a = _followOnRoots[targetAtom];
const DefinedAtom *targetPrevAtom = nullptr;
// If the size of the atom is 0, and the target
// is already part of a chain, lets bring the current
// atom into the chain
size_t currentAtomSize = (*ai).size();
BigcheeseUnsubmitted
Not Done
ReplyInline Actions
.empty()
Bigcheese: .empty()
shankarkeAuthorUnsubmitted
Not Done
ReplyInline Actions

atom doesnot have empty() function.

shankarke: atom doesnot have empty() function.
// Lets add to the chain only if the atoms that // Lets add to the chain only if the atoms that
// appear before the targetAtom in the chain // appear before the targetAtom in the chain
// are of size 0 // are of size 0
bool foundNonZeroSizeAtom = false; bool foundNonZeroSizeAtom = false;
while (true) { while (true) {
targetPrevAtom = a;
// Set all the follow on's for the targetAtom to be // Set all the follow on's for the targetAtom to be
// the current root // the current root
AtomToAtomT::iterator targetFollowOnAtomsIter = AtomToAtomT::iterator targetFollowOnAtomsIter =
_followOnNexts.find(a); _followOnNexts.find(a);
if (targetFollowOnAtomsIter != _followOnNexts.end()) if (targetFollowOnAtomsIter != _followOnNexts.end())
a = targetFollowOnAtomsIter->second; a = targetFollowOnAtomsIter->second;
else else
break; break;
if (a->size() != 0) { if ((a->size() != 0) && (currentAtomSize != 0)) {
foundNonZeroSizeAtom = true; foundNonZeroSizeAtom = true;
break; break;
} }
if (a == targetAtom) if (a == targetAtom)
break; break;
} // while true } // while true
if (foundNonZeroSizeAtom) { if (foundNonZeroSizeAtom) {
// TODO: print warning that an impossible layout // TODO: print warning that an impossible layout
// is being desired by the user // is being desired by the user
// Continue to the next atom // Continue to the next atom
break; break;
} }
// If the atom is a zero sized atom, then make the target
// follow the zero sized atom, as the zero sized atom may be
// a weak symbol
if ((currentAtomSize == 0) && (targetPrevAtom)) {
_followOnNexts[targetPrevAtom] = ai;
_followOnRoots[ai] = _followOnRoots[targetPrevAtom];
_followOnNexts[ai] = targetAtom;
}
else {
BigcheeseUnsubmitted
Not Done
ReplyInline Actions

No newline.

Bigcheese: No newline.
shankarkeAuthorUnsubmitted
Not Done
ReplyInline Actions

Ok.

shankarke: Ok.
_followOnNexts[ai] = _followOnRoots[targetAtom]; _followOnNexts[ai] = _followOnRoots[targetAtom];
// Set the root of all atoms in the // Set the root of all atoms in the
a = _followOnRoots[targetAtom]; a = _followOnRoots[targetAtom];
while (true) { while (true) {
_followOnRoots[a] = _followOnRoots[ai]; _followOnRoots[a] = _followOnRoots[ai];
// Set all the follow on's for the targetAtom to be // Set all the follow on's for the targetAtom to be
// the current root // the current root
AtomToAtomT::iterator targetFollowOnAtomsIter = AtomToAtomT::iterator targetFollowOnAtomsIter =
_followOnNexts.find(a); _followOnNexts.find(a);
if (targetFollowOnAtomsIter != _followOnNexts.end()) if (targetFollowOnAtomsIter != _followOnNexts.end())
a = targetFollowOnAtomsIter->second; a = targetFollowOnAtomsIter->second;
else else
break; break;
} // while true } // while true
} // end else (currentAtomSize != 0)
} // end else } // end else
} // else } // else
} // kindLayoutAfter } // kindLayoutAfter
} // Reference } // Reference
} // range } // range
} }
BigcheeseUnsubmitted
Not Done
ReplyInline Actions

This function is getting nested too deeply. It should probably be split into multiple functions.

Bigcheese: This function is getting nested too deeply. It should probably be split into multiple functions.
shankarkeAuthorUnsubmitted
Not Done
ReplyInline Actions

Will take this in the upcoming patches.

shankarke: Will take this in the upcoming patches.
/// This pass builds the followon tables using InGroup relationships /// This pass builds the followon tables using InGroup relationships
/// The algorithm follows a very simple approach /// The algorithm follows a very simple approach
/// a) If the rootAtom is not part of any root, create a new root with the /// a) If the rootAtom is not part of any root, create a new root with the
/// as the head /// as the head
/// b) If the current Atom root is not found, then make the current atoms root /// b) If the current Atom root is not found, then make the current atoms root
/// point to the rootAtom /// point to the rootAtom
/// c) If the root of the current Atom is itself a root of some other tree /// c) If the root of the current Atom is itself a root of some other tree
▲ Show 20 Lines • Show All 192 Lines • Show Last 20 Lines

lib/ReaderWriter/ELF/File.h

Show First 20 Lines • Show All 340 Lines • ▼ Show 20 Lines for (auto &i : sectionSymbols) {
sectionContents.size()); sectionContents.size());
_definedAtoms._atoms.push_back( _definedAtoms._atoms.push_back(
new (_readerStorage) new (_readerStorage)
ELFDefinedAtom<ELFT>(*this, sectionName, sectionName, sym, i.first, ELFDefinedAtom<ELFT>(*this, sectionName, sectionName, sym, i.first,
content, 0, 0, _references)); content, 0, 0, _references));
} }
ELFDefinedAtom<ELFT> *previous_atom = nullptr; ELFDefinedAtom<ELFT> *previous_atom = nullptr;
// Don't allocate content to a weak symbol, as they may be merged away.
// Create an anonymous atom to hold the data.
ELFDefinedAtom<ELFT> *anonAtom = nullptr;
ELFReference<ELFT> *anonPrecededBy = nullptr;
ELFReference<ELFT> *anonFollowedBy = nullptr;
// i.first is the section the symbol lives in // i.first is the section the symbol lives in
for (auto si = symbols.begin(), se = symbols.end(); si != se; ++si) { for (auto si = symbols.begin(), se = symbols.end(); si != se; ++si) {
StringRef symbolName; StringRef symbolName;
if ((EC = _objFile->getSymbolName(i.first, *si, symbolName))) if ((EC = _objFile->getSymbolName(i.first, *si, symbolName)))
return; return;
const Elf_Shdr *section = _objFile->getSection(*si); const Elf_Shdr *section = _objFile->getSection(*si);
Show All 18 Lines for (auto &i : sectionSymbols) {
uint64_t contentSize; uint64_t contentSize;
if (si + 1 == se) { if (si + 1 == se) {
// if this is the last symbol, take up the remaining data. // if this is the last symbol, take up the remaining data.
contentSize = isCommon ? 0 : i.first->sh_size - (*si)->st_value; contentSize = isCommon ? 0 : i.first->sh_size - (*si)->st_value;
} else { } else {
contentSize = isCommon ? 0 : (*(si + 1))->st_value - (*si)->st_value; contentSize = isCommon ? 0 : (*(si + 1))->st_value - (*si)->st_value;
} }
// Check to see if we need to add the FollowOn Reference
// We dont want to do for symbols that are
// a) common symbols
ELFReference<ELFT> *followOn = nullptr;
if (!isCommon && previous_atom) {
// Replace the followon atom with the anonymous
// atom that we created, so that the next symbol
// that we create is a followon from the anonymous
// atom
if (!anonFollowedBy) {
followOn = new (_readerStorage)
ELFReference<ELFT>(lld::Reference::kindLayoutAfter);
previous_atom->addReference(followOn);
}
else
followOn = anonFollowedBy;
}
// Don't allocate content to a weak symbol, as they may be merged away. // Don't allocate content to a weak symbol, as they may be merged away.
// Create an anonymous atom to hold the data. // Create an anonymous atom to hold the data.
ELFDefinedAtom<ELFT> *anonAtom = nullptr; anonAtom = nullptr;
anonPrecededBy = nullptr;
anonFollowedBy = nullptr;
if ((*si)->getBinding() == llvm::ELF::STB_WEAK && contentSize != 0) { if ((*si)->getBinding() == llvm::ELF::STB_WEAK && contentSize != 0) {
// Create a new non-weak ELF symbol. // Create a new non-weak ELF symbol.
auto sym = new (_readerStorage) Elf_Sym; auto sym = new (_readerStorage) Elf_Sym;
*sym = **si; *sym = **si;
sym->setBinding(llvm::ELF::STB_GLOBAL); sym->setBinding(llvm::ELF::STB_GLOBAL);
anonAtom = createDefinedAtomAndAssignRelocations( anonAtom = createDefinedAtomAndAssignRelocations(
"", sectionName, sym, i.first, "", sectionName, sym, i.first,
ArrayRef<uint8_t>((uint8_t *)sectionContents.data() + ArrayRef<uint8_t>((uint8_t *)sectionContents.data() +
(*si)->st_value, contentSize)); (*si)->st_value, contentSize));
// If this is the last atom, lets not create a followon
// reference
if ((si + 1) != se)
anonFollowedBy = new (_readerStorage)
ELFReference<ELFT>(lld::Reference::kindLayoutAfter);
anonPrecededBy = new (_readerStorage)
ELFReference<ELFT>(lld::Reference::kindLayoutBefore);
// Add the references to the anonymous atom that we created
if (anonFollowedBy)
anonAtom->addReference(anonFollowedBy);
anonAtom->addReference(anonPrecededBy);
if (previous_atom)
anonPrecededBy->setTarget(previous_atom);
contentSize = 0; contentSize = 0;
} }
ArrayRef<uint8_t> symbolData = ArrayRef<uint8_t>( ArrayRef<uint8_t> symbolData = ArrayRef<uint8_t>(
(uint8_t *)sectionContents.data() + (*si)->st_value, contentSize); (uint8_t *)sectionContents.data() + (*si)->st_value, contentSize);
// Check to see if we need to add the FollowOn Reference
// We dont want to do for symbols that are
// a) common symbols
// so, lets add a follow-on reference from the previous atom to the
// current atom as well as lets add a preceded-by reference from the
// current atom to the previous atom, so that the previous atom
// is not removed in any case
ELFReference<ELFT> *followOn = nullptr;
if (!isCommon && previous_atom) {
followOn = new (_readerStorage)
ELFReference<ELFT>(lld::Reference::kindLayoutAfter);
previous_atom->addReference(followOn);
}
// If the linker finds that a section has global atoms that are in a // If the linker finds that a section has global atoms that are in a
// mergeable section, treat them as defined atoms as they shouldnt be // mergeable section, treat them as defined atoms as they shouldnt be
// merged away as well as these symbols have to be part of symbol // merged away as well as these symbols have to be part of symbol
// resolution // resolution
int64_t sectionFlags = 0; int64_t sectionFlags = 0;
if (section) if (section)
sectionFlags = section->sh_flags; sectionFlags = section->sh_flags;
sectionFlags &= ~llvm::ELF::SHF_ALLOC; sectionFlags &= ~llvm::ELF::SHF_ALLOC;
if (doStringsMerge && section && (section->sh_entsize < 2) && if (doStringsMerge && section && (section->sh_entsize < 2) &&
(sectionFlags == (llvm::ELF::SHF_MERGE | llvm::ELF::SHF_STRINGS))) { (sectionFlags == (llvm::ELF::SHF_MERGE | llvm::ELF::SHF_STRINGS))) {
if ((*si)->getBinding() == llvm::ELF::STB_GLOBAL) { if ((*si)->getBinding() == llvm::ELF::STB_GLOBAL) {
auto definedMergeAtom = new (_readerStorage) ELFDefinedAtom<ELFT>( auto definedMergeAtom = new (_readerStorage) ELFDefinedAtom<ELFT>(
*this, symbolName, sectionName, (*si), section, symbolData, *this, symbolName, sectionName, (*si), section, symbolData,
_references.size(), _references.size(), _references); _references.size(), _references.size(), _references);
_definedAtoms._atoms.push_back(definedMergeAtom); _definedAtoms._atoms.push_back(definedMergeAtom);
} }
continue; continue;
} }
auto newAtom = createDefinedAtomAndAssignRelocations( auto newAtom = createDefinedAtomAndAssignRelocations(
symbolName, sectionName, *si, i.first, symbolData); symbolName, sectionName, *si, i.first, symbolData);
// If we are inserting a followOn reference, lets add a precededBy // If the atom was a weak symbol, lets create a followon
// reference too // reference to the anonymous atom that we created
if (((*si)->getBinding() == llvm::ELF::STB_WEAK) && (anonAtom)) {
BigcheeseUnsubmitted
Not Done
ReplyInline Actions

Drop the extra parens.

if ((*si)->getBinding() == llvm::ELF::STB_WEAK && anonAtom)
Bigcheese: Drop the extra parens. if ((*si)->getBinding() == llvm::ELF::STB_WEAK && anonAtom)
shankarkeAuthorUnsubmitted
Not Done
ReplyInline Actions

ok.

shankarke: ok.
ELFReference<ELFT> *wFollowedBy = new (_readerStorage)
ELFReference<ELFT>(lld::Reference::kindLayoutAfter);
wFollowedBy->setTarget(anonAtom);
newAtom->addReference(wFollowedBy);
}
if (followOn) { if (followOn) {
ELFReference<ELFT> *precededby = nullptr; ELFReference<ELFT> *precededby = nullptr;
// Set the followon atom to the weak atom
// that we have created, so that they would
// alias when the file gets written
if (anonAtom)
followOn->setTarget(anonAtom);
else
followOn->setTarget(newAtom); followOn->setTarget(newAtom);
// Add a preceded by reference only if the current atom is not a // Add a preceded by reference only if the current atom is not a
// weak atom // weak atom
if ((*si)->getBinding() != llvm::ELF::STB_WEAK) { if ((*si)->getBinding() != llvm::ELF::STB_WEAK) {
precededby = new (_readerStorage) precededby = new (_readerStorage)
ELFReference<ELFT>(lld::Reference::kindLayoutBefore); ELFReference<ELFT>(lld::Reference::kindLayoutBefore);
precededby->setTarget(previous_atom); precededby->setTarget(previous_atom);
newAtom->addReference(precededby); newAtom->addReference(precededby);
} }
} }
// The previous atom is always the atom created before unless // The previous atom is always the atom created before unless
// the atom is a weak atom // the atom is a weak atom
if ((*si)->getBinding() != llvm::ELF::STB_WEAK) if (anonAtom)
previous_atom = anonAtom;
else
previous_atom = newAtom; previous_atom = newAtom;
_definedAtoms._atoms.push_back(newAtom); _definedAtoms._atoms.push_back(newAtom);
_symbolToAtomMapping.insert(std::make_pair((*si), newAtom)); _symbolToAtomMapping.insert(std::make_pair((*si), newAtom));
if (anonAtom) if (anonAtom)
_definedAtoms._atoms.push_back(anonAtom); _definedAtoms._atoms.push_back(anonAtom);
} }
} }
▲ Show 20 Lines • Show All 139 Lines • Show Last 20 Lines

test/elf/X86_64/Inputs/multi-ovrd.c

#include <stdio.h>
void f(void)
{
printf("overridden f!\n");
}
void g(void)
{
printf("overridden g!\n");
}

test/elf/X86_64/Inputs/multi-weak.c

int fn()
{
return 0;
}
void __attribute__((weak)) f()
{
printf("original f..\n");
}
void __attribute__((weak)) g()
{
printf("original f..\n");
}
int main(void)
{
f();
return 0;
}

test/elf/X86_64/Inputs/ovrd.c

#include <stdio.h>
void f(void)
{
printf("overridden f!\n");
}

test/elf/X86_64/Inputs/weak.c

int fn()
{
return 0;
}
void __attribute__((weak)) f()
{
printf("original f..\n");
}
int main(void)
{
f();
return 0;
}

test/elf/X86_64/multi-weak-override.test

# Test for weak symbol getting overridden
RUN: lld-core -reader ELF -writer ELF -arch x86_64 %p/Inputs/multi-weak.o \
RUN: %p/Inputs/multi-ovrd.o -o %t
RUN: llvm-nm -n %t | FileCheck -check-prefix=WEAKORDER %s
RUN: lld-core -reader ELF -arch x86_64 %p/Inputs/multi-weak.o \
RUN: %p/Inputs/multi-ovrd.o -layout-pass=true -o %t2
RUN: FileCheck -check-prefix=WEAKATOMSORDER %s < %t2
WEAKORDER: 0040011c T f
WEAKORDER: 0040012c T g
WEAKATOMSORDER: - ref-name: L002
WEAKATOMSORDER: - ref-name: L003
WEAKATOMSORDER: - name: f
WEAKATOMSORDER: - name: g

test/elf/X86_64/multi-weak-syms-order.test

# Test for weak symbol getting overridden
RUN: lld-core -reader ELF -writer ELF -arch x86_64 %p/Inputs/multi-weak.o \
RUN: -o %t
RUN: llvm-nm -n %t | FileCheck -check-prefix=WEAKORDER %s
RUN: lld-core -reader ELF -arch x86_64 %p/Inputs/multi-weak.o \
RUN: -layout-pass=true -o %t2
RUN: FileCheck -check-prefix=WEAKATOMSORDER %s < %t2
WEAKORDER: 004000c0 T fn
WEAKORDER: 004000cb T .text
WEAKORDER: 004000cb T f
WEAKORDER: 004000db T .text
WEAKORDER: 004000db T g
WEAKORDER: 004000eb T main
WEAKATOMSORDER: - name: f
WEAKATOMSORDER: - ref-name: L004
WEAKATOMSORDER: - name: g
WEAKATOMSORDER: - ref-name: L005

test/elf/X86_64/weak-override.test

# Test for weak symbol getting overridden
RUN: lld-core -reader ELF -writer ELF -arch x86_64 %p/Inputs/weak.o \
RUN: %p/Inputs/ovrd.o -o %t
RUN: llvm-nm %t | FileCheck -check-prefix=WEAKORDER %s
RUN: lld-core -reader ELF -arch x86_64 %p/Inputs/weak.o \
RUN: %p/Inputs/ovrd.o -layout-pass=true -o %t2
RUN: FileCheck -check-prefix=WEAKATOMSORDER %s < %t2
WEAKORDER: 004000fc T f
WEAKATOMSORDER: - name: .text
WEAKATOMSORDER: references:
WEAKATOMSORDER: - kind: layout-after
WEAKATOMSORDER: offset: 0
WEAKATOMSORDER: target: fn
WEAKATOMSORDER: - name: fn
WEAKATOMSORDER: references:
WEAKATOMSORDER: - kind: layout-after
WEAKATOMSORDER: offset: 0
WEAKATOMSORDER: target: L001
WEAKATOMSORDER: - ref-name: L001
WEAKATOMSORDER: scope: global
WEAKATOMSORDER: content: [ 55, 48, 89, E5, BF, 00, 00, 00, 00, E8, 00, 00,
WEAKATOMSORDER: 00, 00, 5D, C3 ]
WEAKATOMSORDER: references:
WEAKATOMSORDER: - kind: layout-after
WEAKATOMSORDER: offset: 0
WEAKATOMSORDER: target: main
WEAKATOMSORDER: - kind: layout-before
WEAKATOMSORDER: offset: 0
WEAKATOMSORDER: target: fn
WEAKATOMSORDER: - name: main
WEAKATOMSORDER: scope: global
WEAKATOMSORDER: content: [ 55, 48, 89, E5, B8, 00, 00, 00, 00, E8, 00, 00,
WEAKATOMSORDER: 00, 00, B8, 00, 00, 00, 00, 5D, C3 ]
WEAKATOMSORDER: references:
WEAKATOMSORDER: - kind: call32
WEAKATOMSORDER: offset: 10
WEAKATOMSORDER: target: f
WEAKATOMSORDER: addend: -4
WEAKATOMSORDER: - kind: layout-before
WEAKATOMSORDER: offset: 0
WEAKATOMSORDER: target: L001
WEAKATOMSORDER: - name: .text
WEAKATOMSORDER: section-name: .text
WEAKATOMSORDER: references:
WEAKATOMSORDER: - kind: layout-after
WEAKATOMSORDER: offset: 0
WEAKATOMSORDER: target: f
WEAKATOMSORDER: - name: f
WEAKATOMSORDER: scope: global
WEAKATOMSORDER: content: [ 55, 48, 89, E5, BF, 00, 00, 00, 00, E8, 00, 00,
WEAKATOMSORDER: 00, 00, 5D, C3 ]
WEAKATOMSORDER: references:
WEAKATOMSORDER: - kind: layout-before
WEAKATOMSORDER: offset: 0

test/elf/X86_64/weak-zero-sized.test

# Test for zero sized weak atoms, there is only a single weak atom
RUN: lld-core -reader ELF -writer ELF -arch x86_64 %p/Inputs/weak-zero-sized.o \
RUN: -o %t
RUN: llvm-nm %t | FileCheck -check-prefix=WEAKORDER %s
RUN: lld-core -reader ELF -arch x86_64 %p/Inputs/weak-zero-sized.o \
RUN: -layout-pass=true -o %t2
RUN: FileCheck -check-prefix=WEAKATOMSORDER %s < %t2
WEAKORDER: 004000b0 T _start
WEAKATOMSORDER: - name: .text
WEAKATOMSORDER: alignment: 2^2
WEAKATOMSORDER: section-name: .text
WEAKATOMSORDER: - kind: layout-after
WEAKATOMSORDER: offset: 0
WEAKATOMSORDER: target: L001
WEAKATOMSORDER: - name: _start
WEAKATOMSORDER: scope: global
WEAKATOMSORDER: merge: as-weak
WEAKATOMSORDER: alignment: 2^2
WEAKATOMSORDER: section-name: .text
WEAKATOMSORDER: references:
WEAKATOMSORDER: - kind: layout-after
WEAKATOMSORDER: offset: 0
WEAKATOMSORDER: target: L001
WEAKATOMSORDER: - ref-name: L001
WEAKATOMSORDER: scope: global
WEAKATOMSORDER: content: [ C3 ]
WEAKATOMSORDER: alignment: 2^2
WEAKATOMSORDER: section-name: .text
WEAKATOMSORDER: references:
WEAKATOMSORDER: - kind: layout-before
WEAKATOMSORDER: offset: 0
WEAKATOMSORDER: target: .text