Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fd3fecf

Browse filesBrowse files
zmodemzmodem
committed
Revert "[lld] Merge equivalent symbols found during ICF (#134342)"
The change would also merge *non-equivalent* symbols under some circumstances, see comment with a reproducer on the PR. > Fixes a correctness issue for AArch64 when ADRP and LDR instructions are > outlined in separate sections and sections are fed to ICF for > deduplication. > > See test case (based on > #129122) for details. All > rodata.* sections are folded into a single section with ICF. This leads > to all f2_* function sections getting folded into one (as their > relocation target symbols g* belong to .rodata.g* sections that have > already been folded into one). Since relocations still refer original g* > symbols, we end up creating duplicate GOT entry for all such symbols. > This PR addresses that by tracking such folded symbols and create one > GOT entry for all such symbols. > > Fixes #129122 > > Co-authored by: @jyknight This reverts commit 8389d6f.
1 parent 9876343 commit fd3fecf
Copy full SHA for fd3fecf

File tree

5 files changed

+1
-161
lines changed
Filter options

5 files changed

+1
-161
lines changed

‎lld/ELF/ICF.cpp

Copy file name to clipboardExpand all lines: lld/ELF/ICF.cpp
+1-53Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@
8080
#include "SymbolTable.h"
8181
#include "Symbols.h"
8282
#include "SyntheticSections.h"
83-
#include "llvm/ADT/EquivalenceClasses.h"
8483
#include "llvm/BinaryFormat/ELF.h"
8584
#include "llvm/Object/ELF.h"
8685
#include "llvm/Support/Parallel.h"
@@ -334,28 +333,6 @@ bool ICF<ELFT>::equalsConstant(const InputSection *a, const InputSection *b) {
334333
: constantEq(a, ra.relas, b, rb.relas);
335334
}
336335

337-
template <class RelTy>
338-
static SmallVector<Symbol *> getReloc(const InputSection *sec,
339-
Relocs<RelTy> relocs) {
340-
SmallVector<Symbol *> syms;
341-
for (auto ri = relocs.begin(), re = relocs.end(); ri != re; ++ri) {
342-
Symbol &sym = sec->file->getRelocTargetSym(*ri);
343-
syms.push_back(&sym);
344-
}
345-
return syms;
346-
}
347-
348-
template <class ELFT>
349-
static SmallVector<Symbol *> getRelocTargetSyms(const InputSection *sec) {
350-
const RelsOrRelas<ELFT> rel = sec->template relsOrRelas<ELFT>();
351-
if (rel.areRelocsCrel())
352-
return getReloc(sec, rel.crels);
353-
if (rel.areRelocsRel())
354-
return getReloc(sec, rel.rels);
355-
356-
return getReloc(sec, rel.relas);
357-
}
358-
359336
// Compare two lists of relocations. Returns true if all pairs of
360337
// relocations point to the same section in terms of ICF.
361338
template <class ELFT>
@@ -560,28 +537,14 @@ template <class ELFT> void ICF<ELFT>::run() {
560537
auto print = [&ctx = ctx]() -> ELFSyncStream {
561538
return {ctx, ctx.arg.printIcfSections ? DiagLevel::Msg : DiagLevel::None};
562539
};
563-
564-
EquivalenceClasses<Symbol *> symbolEquivalence;
565540
// Merge sections by the equivalence class.
566-
// Merge symbols identified as equivalent during ICF.
567541
forEachClassRange(0, sections.size(), [&](size_t begin, size_t end) {
568542
if (end - begin == 1)
569543
return;
570544
print() << "selected section " << sections[begin];
571-
SmallVector<Symbol *> syms = getRelocTargetSyms<ELFT>(sections[begin]);
572545
for (size_t i = begin + 1; i < end; ++i) {
573546
print() << " removing identical section " << sections[i];
574547
sections[begin]->replace(sections[i]);
575-
SmallVector<Symbol *> replacedSyms =
576-
getRelocTargetSyms<ELFT>(sections[i]);
577-
assert(syms.size() == replacedSyms.size() &&
578-
"Should have same number of syms!");
579-
for (size_t i = 0; i < syms.size(); i++) {
580-
if (syms[i] == replacedSyms[i] || !syms[i]->isGlobal() ||
581-
!replacedSyms[i]->isGlobal())
582-
continue;
583-
symbolEquivalence.unionSets(syms[i], replacedSyms[i]);
584-
}
585548

586549
// At this point we know sections merged are fully identical and hence
587550
// we want to remove duplicate implicit dependencies such as link order
@@ -600,26 +563,11 @@ template <class ELFT> void ICF<ELFT>::run() {
600563
d->folded = true;
601564
}
602565
};
603-
for (Symbol *sym : ctx.symtab->getSymbols()) {
566+
for (Symbol *sym : ctx.symtab->getSymbols())
604567
fold(sym);
605-
auto it = symbolEquivalence.findLeader(sym);
606-
if (it != symbolEquivalence.member_end() && *it != sym) {
607-
print() << "redirecting '" << sym->getName() << "' in symtab to '"
608-
<< (*it)->getName() << "'";
609-
ctx.symtab->redirect(sym, *it);
610-
}
611-
}
612568
parallelForEach(ctx.objectFiles, [&](ELFFileBase *file) {
613569
for (Symbol *sym : file->getLocalSymbols())
614570
fold(sym);
615-
for (Symbol *&sym : file->getMutableGlobalSymbols()) {
616-
auto it = symbolEquivalence.findLeader(sym);
617-
if (it != symbolEquivalence.member_end() && *it != sym) {
618-
print() << "redirecting '" << sym->getName() << "' to '"
619-
<< (*it)->getName() << "'";
620-
sym = *it;
621-
}
622-
}
623571
});
624572

625573
// InputSectionDescription::sections is populated by processSectionCommands().

‎lld/ELF/SymbolTable.cpp

Copy file name to clipboardExpand all lines: lld/ELF/SymbolTable.cpp
-6Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,6 @@ using namespace llvm::ELF;
2929
using namespace lld;
3030
using namespace lld::elf;
3131

32-
void SymbolTable::redirect(Symbol *from, Symbol *to) {
33-
int &fromIdx = symMap[CachedHashStringRef(from->getName())];
34-
const int toIdx = symMap[CachedHashStringRef(to->getName())];
35-
fromIdx = toIdx;
36-
}
37-
3832
void SymbolTable::wrap(Symbol *sym, Symbol *real, Symbol *wrap) {
3933
// Redirect __real_foo to the original foo and foo to the original __wrap_foo.
4034
int &idx1 = symMap[CachedHashStringRef(sym->getName())];

‎lld/ELF/SymbolTable.h

Copy file name to clipboardExpand all lines: lld/ELF/SymbolTable.h
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ class SymbolTable {
4141
SymbolTable(Ctx &ctx) : ctx(ctx) {}
4242
ArrayRef<Symbol *> getSymbols() const { return symVector; }
4343

44-
void redirect(Symbol *from, Symbol *to);
4544
void wrap(Symbol *sym, Symbol *real, Symbol *wrap);
4645

4746
Symbol *insert(StringRef name);

‎lld/test/ELF/aarch64-got-merging-icf.s

Copy file name to clipboardExpand all lines: lld/test/ELF/aarch64-got-merging-icf.s
-95Lines changed: 0 additions & 95 deletions
This file was deleted.

‎lld/test/ELF/icf-preemptible.s

Copy file name to clipboardExpand all lines: lld/test/ELF/icf-preemptible.s
-6Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,6 @@
1717
# EXE-NEXT: selected section {{.*}}:(.text.h1)
1818
# EXE-NEXT: removing identical section {{.*}}:(.text.h2)
1919
# EXE-NEXT: removing identical section {{.*}}:(.text.h3)
20-
# EXE-NEXT: redirecting 'f2' in symtab to 'f1'
21-
# EXE-NEXT: redirecting 'g2' in symtab to 'g1'
22-
# EXE-NEXT: redirecting 'g3' in symtab to 'g1'
23-
# EXE-NEXT: redirecting 'f2' to 'f1'
24-
# EXE-NEXT: redirecting 'g2' to 'g1'
25-
# EXE-NEXT: redirecting 'g3' to 'g1'
2620
# EXE-NOT: {{.}}
2721

2822
## Definitions are preemptible in a DSO. Only leaf functions can be folded.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.