Skip to content

Commit 3a17f74

Browse files
committed
Revert "[ELF] Fix unnecessary inclusion of unreferenced provide symbols"
This reverts commit ebb326a.
1 parent 676fd91 commit 3a17f74

File tree

8 files changed

+34
-188
lines changed

8 files changed

+34
-188
lines changed

lld/ELF/Driver.cpp

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2470,6 +2470,12 @@ static void readSymbolPartitionSection(InputSectionBase *s) {
24702470
sym->partition = newPart.getNumber();
24712471
}
24722472

2473+
static Symbol *addUnusedUndefined(StringRef name,
2474+
uint8_t binding = STB_GLOBAL) {
2475+
return symtab.addSymbol(
2476+
Undefined{ctx.internalFile, name, binding, STV_DEFAULT, 0});
2477+
}
2478+
24732479
static void markBuffersAsDontNeed(bool skipLinkedOutput) {
24742480
// With --thinlto-index-only, all buffers are nearly unused from now on
24752481
// (except symbol/section names used by infrequent passes). Mark input file
@@ -2556,19 +2562,19 @@ static std::vector<WrappedSymbol> addWrappedSymbols(opt::InputArgList &args) {
25562562
continue;
25572563

25582564
Symbol *wrap =
2559-
symtab.addUnusedUndefined(saver().save("__wrap_" + name), sym->binding);
2565+
addUnusedUndefined(saver().save("__wrap_" + name), sym->binding);
25602566

25612567
// If __real_ is referenced, pull in the symbol if it is lazy. Do this after
25622568
// processing __wrap_ as that may have referenced __real_.
25632569
StringRef realName = saver().save("__real_" + name);
25642570
if (Symbol *real = symtab.find(realName)) {
2565-
symtab.addUnusedUndefined(name, sym->binding);
2571+
addUnusedUndefined(name, sym->binding);
25662572
// Update sym's binding, which will replace real's later in
25672573
// SymbolTable::wrap.
25682574
sym->binding = real->binding;
25692575
}
25702576

2571-
Symbol *real = symtab.addUnusedUndefined(realName);
2577+
Symbol *real = addUnusedUndefined(realName);
25722578
v.push_back({sym, real, wrap});
25732579

25742580
// We want to tell LTO not to inline symbols to be overwritten
@@ -2845,14 +2851,20 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
28452851
// Handle -u/--undefined before input files. If both a.a and b.so define foo,
28462852
// -u foo a.a b.so will extract a.a.
28472853
for (StringRef name : config->undefined)
2848-
symtab.addUnusedUndefined(name)->referenced = true;
2854+
addUnusedUndefined(name)->referenced = true;
28492855

28502856
parseFiles(files, armCmseImpLib);
28512857

28522858
// Create dynamic sections for dynamic linking and static PIE.
28532859
config->hasDynSymTab = !ctx.sharedFiles.empty() || config->isPic;
28542860

2855-
script->addScriptReferencedSymbolsToSymTable();
2861+
// Some symbols (such as __ehdr_start) are defined lazily only when there
2862+
// are undefined symbols for them, so we add these to trigger that logic.
2863+
for (StringRef name : script->referencedSymbols) {
2864+
Symbol *sym = addUnusedUndefined(name);
2865+
sym->isUsedInRegularObj = true;
2866+
sym->referenced = true;
2867+
}
28562868

28572869
// Prevent LTO from removing any definition referenced by -u.
28582870
for (StringRef name : config->undefined)

lld/ELF/LinkerScript.cpp

Lines changed: 9 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,15 @@ static bool shouldDefineSym(SymbolAssignment *cmd) {
199199
if (cmd->name == ".")
200200
return false;
201201

202-
return !cmd->provide || LinkerScript::shouldAddProvideSym(cmd->name);
202+
if (!cmd->provide)
203+
return true;
204+
205+
// If a symbol was in PROVIDE(), we need to define it only
206+
// when it is a referenced undefined symbol.
207+
Symbol *b = symtab.find(cmd->name);
208+
if (b && !b->isDefined() && !b->isCommon())
209+
return true;
210+
return false;
203211
}
204212

205213
// Called by processSymbolAssignments() to assign definitions to
@@ -1683,41 +1691,3 @@ void LinkerScript::checkFinalScriptConditions() const {
16831691
checkMemoryRegion(lmaRegion, sec, sec->getLMA());
16841692
}
16851693
}
1686-
1687-
void LinkerScript::addScriptReferencedSymbolsToSymTable() {
1688-
// Some symbols (such as __ehdr_start) are defined lazily only when there
1689-
// are undefined symbols for them, so we add these to trigger that logic.
1690-
auto reference = [](StringRef name) {
1691-
Symbol *sym = symtab.addUnusedUndefined(name);
1692-
sym->isUsedInRegularObj = true;
1693-
sym->referenced = true;
1694-
};
1695-
for (StringRef name : referencedSymbols)
1696-
reference(name);
1697-
1698-
// Keeps track of references from which PROVIDE symbols have been added to the
1699-
// symbol table.
1700-
DenseSet<StringRef> added;
1701-
SmallVector<const SmallVector<StringRef, 0> *, 0> symRefsVec;
1702-
for (const auto &[name, symRefs] : provideMap)
1703-
if (LinkerScript::shouldAddProvideSym(name) && added.insert(name).second)
1704-
symRefsVec.push_back(&symRefs);
1705-
while (symRefsVec.size()) {
1706-
for (StringRef name : *symRefsVec.pop_back_val()) {
1707-
reference(name);
1708-
// Prevent the symbol from being discarded by --gc-sections.
1709-
script->referencedSymbols.push_back(name);
1710-
auto it = script->provideMap.find(name);
1711-
if (it != script->provideMap.end() &&
1712-
LinkerScript::shouldAddProvideSym(name) &&
1713-
added.insert(name).second) {
1714-
symRefsVec.push_back(&it->second);
1715-
}
1716-
}
1717-
}
1718-
}
1719-
1720-
bool LinkerScript::shouldAddProvideSym(StringRef symName) {
1721-
Symbol *sym = symtab.find(symName);
1722-
return sym && !sym->isDefined() && !sym->isCommon();
1723-
}

lld/ELF/LinkerScript.h

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#include "llvm/ADT/ArrayRef.h"
1818
#include "llvm/ADT/DenseMap.h"
1919
#include "llvm/ADT/MapVector.h"
20-
#include "llvm/ADT/SmallVector.h"
2120
#include "llvm/ADT/StringRef.h"
2221
#include "llvm/Support/Compiler.h"
2322
#include <cstddef>
@@ -366,18 +365,6 @@ class LinkerScript final {
366365
// Check backward location counter assignment and memory region/LMA overflows.
367366
void checkFinalScriptConditions() const;
368367

369-
// Add symbols that are referenced in the linker script to the symbol table.
370-
// Symbols referenced in a PROVIDE command are only added to the symbol table
371-
// if the PROVIDE command actually provides the symbol.
372-
// It also adds the symbols referenced by the used PROVIDE symbols to the
373-
// linker script referenced symbols list.
374-
void addScriptReferencedSymbolsToSymTable();
375-
376-
// Returns true if the PROVIDE symbol should be added to the link.
377-
// A PROVIDE symbol is added to the link only if it satisfies an
378-
// undefined reference.
379-
static bool shouldAddProvideSym(StringRef symName);
380-
381368
// SECTIONS command list.
382369
SmallVector<SectionCommand *, 0> sectionCommands;
383370

@@ -413,14 +400,6 @@ class LinkerScript final {
413400
// Sections that will be warned/errored by --orphan-handling.
414401
SmallVector<const InputSectionBase *, 0> orphanSections;
415402

416-
// Stores the mapping: PROVIDE symbol -> symbols referred in the PROVIDE
417-
// expression. For example, if the PROVIDE command is:
418-
//
419-
// PROVIDE(v = a + b + c);
420-
//
421-
// then provideMap should contain the mapping: 'v' -> ['a', 'b', 'c']
422-
llvm::MapVector<StringRef, SmallVector<StringRef, 0>> provideMap;
423-
424403
// List of potential spill locations (PotentialSpillSection) for an input
425404
// section.
426405
struct PotentialSpillList {

lld/ELF/ScriptParser.cpp

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
#include "llvm/Support/TimeProfiler.h"
3737
#include <cassert>
3838
#include <limits>
39-
#include <optional>
4039
#include <vector>
4140

4241
using namespace llvm;
@@ -140,10 +139,6 @@ class ScriptParser final : ScriptLexer {
140139

141140
// A set to detect an INCLUDE() cycle.
142141
StringSet<> seen;
143-
144-
// If we are currently parsing a PROVIDE|PROVIDE_HIDDEN command,
145-
// then this member is set to the PROVIDE symbol name.
146-
std::optional<llvm::StringRef> activeProvideSym;
147142
};
148143
} // namespace
149144

@@ -1088,9 +1083,6 @@ SymbolAssignment *ScriptParser::readProvideHidden(bool provide, bool hidden) {
10881083
;
10891084
return nullptr;
10901085
}
1091-
llvm::SaveAndRestore saveActiveProvideSym(activeProvideSym);
1092-
if (provide)
1093-
activeProvideSym = name;
10941086
SymbolAssignment *cmd = readSymbolAssignment(name);
10951087
cmd->provide = provide;
10961088
cmd->hidden = hidden;
@@ -1604,10 +1596,7 @@ Expr ScriptParser::readPrimary() {
16041596
tok = unquote(tok);
16051597
else if (!isValidSymbolName(tok))
16061598
setError("malformed number: " + tok);
1607-
if (activeProvideSym)
1608-
script->provideMap[*activeProvideSym].push_back(tok);
1609-
else
1610-
script->referencedSymbols.push_back(tok);
1599+
script->referencedSymbols.push_back(tok);
16111600
return [=] { return script->getSymbolValue(tok, location); };
16121601
}
16131602

lld/ELF/SymbolTable.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,3 @@ void SymbolTable::scanVersionScript() {
333333
// --dynamic-list.
334334
handleDynamicList();
335335
}
336-
337-
Symbol *SymbolTable::addUnusedUndefined(StringRef name, uint8_t binding) {
338-
return addSymbol(Undefined{ctx.internalFile, name, binding, STV_DEFAULT, 0});
339-
}

lld/ELF/SymbolTable.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,6 @@ class SymbolTable {
5757

5858
void handleDynamicList();
5959

60-
Symbol *addUnusedUndefined(StringRef name,
61-
uint8_t binding = llvm::ELF::STB_GLOBAL);
62-
6360
// Set of .so files to not link the same shared object file more than once.
6461
llvm::DenseMap<llvm::CachedHashStringRef, SharedFile *> soNames;
6562

lld/test/ELF/gc-sections-with-provide.s

Lines changed: 0 additions & 60 deletions
This file was deleted.

lld/test/ELF/linkerscript/symbolreferenced.s

Lines changed: 7 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -21,31 +21,11 @@
2121
# RUN: ld.lld -o chain -T chain.t a.o
2222
# RUN: llvm-nm chain | FileCheck %s
2323

24-
# CHECK-NOT: another_unused
25-
# CHECK: 0000000000007000 a f1
26-
# CHECK-NEXT: 0000000000007000 A f2
27-
# CHECK-NEXT: 0000000000007000 A f3
28-
# CHECK-NEXT: 0000000000007000 A f4
29-
# CHECK-NEXT: 0000000000006000 A f5
30-
# CHECK-NEXT: 0000000000003000 A f6
31-
# CHECK-NEXT: 0000000000001000 A f7
32-
# CHECK-NOT: g1
33-
# CHECK-NOT: g2
34-
# CHECK-NEXT: 0000000000007500 A newsym
35-
# CHECK: 0000000000002000 A u
36-
# CHECK-NOT: unused
37-
# CHECK-NEXT: 0000000000002000 A v
38-
# CHECK-NEXT: 0000000000002000 A w
39-
40-
41-
# RUN: ld.lld -o chain_with_cycle -T chain_with_cycle.t a.o
42-
# RUN: llvm-nm chain_with_cycle | FileCheck %s --check-prefix=CHAIN_WITH_CYCLE
43-
44-
# CHAIN_WITH_CYCLE: 000 A f1
45-
# CHAIN_WITH_CYCLE: 000 A f2
46-
# CHAIN_WITH_CYCLE: 000 A f3
47-
# CHAIN_WITH_CYCLE: 000 A f4
48-
# CHAIN_WITH_CYCLE: 000 A newsym
24+
# CHECK: 0000000000001000 a f1
25+
# CHECK-NEXT: 0000000000001000 A f2
26+
# CHECK-NEXT: 0000000000001000 a g1
27+
# CHECK-NEXT: 0000000000001000 A g2
28+
# CHECK-NEXT: 0000000000001000 A newsym
4929

5030
# RUN: not ld.lld -T chain2.t a.o 2>&1 | FileCheck %s --check-prefix=ERR --implicit-check-not=error:
5131
# ERR-COUNT-3: error: chain2.t:1: symbol not found: undef
@@ -60,30 +40,13 @@ patatino:
6040
movl newsym, %eax
6141

6242
#--- chain.t
63-
PROVIDE(f7 = 0x1000);
64-
PROVIDE(f5 = f6 + 0x3000);
65-
PROVIDE(f6 = f7 + 0x2000);
66-
PROVIDE(f4 = f5 + 0x1000);
67-
PROVIDE(f3 = f4);
68-
PROVIDE(f2 = f3);
43+
PROVIDE(f2 = 0x1000);
6944
PROVIDE_HIDDEN(f1 = f2);
70-
PROVIDE(newsym = f1 + 0x500);
71-
72-
u = v;
73-
PROVIDE(w = 0x2000);
74-
PROVIDE(v = w);
45+
PROVIDE(newsym = f1);
7546

7647
PROVIDE(g2 = 0x1000);
7748
PROVIDE_HIDDEN(g1 = g2);
7849
PROVIDE(unused = g1);
79-
PROVIDE_HIDDEN(another_unused = g1);
80-
81-
#--- chain_with_cycle.t
82-
PROVIDE(f1 = f2 + f3);
83-
PROVIDE(f2 = f3 + f4);
84-
PROVIDE(f3 = f4);
85-
PROVIDE(f4 = f1);
86-
PROVIDE(newsym = f1);
8750

8851
#--- chain2.t
8952
PROVIDE(f2 = undef);

0 commit comments

Comments
 (0)