Skip to content

Commit

Permalink
Revert "[ELF] Fix unnecessary inclusion of unreferenced provide symbols"
Browse files Browse the repository at this point in the history
This reverts commit ebb326a.
  • Loading branch information
DianQK committed Oct 9, 2024
1 parent 676fd91 commit 3a17f74
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 188 deletions.
22 changes: 17 additions & 5 deletions lld/ELF/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2470,6 +2470,12 @@ static void readSymbolPartitionSection(InputSectionBase *s) {
sym->partition = newPart.getNumber();
}

static Symbol *addUnusedUndefined(StringRef name,
uint8_t binding = STB_GLOBAL) {
return symtab.addSymbol(
Undefined{ctx.internalFile, name, binding, STV_DEFAULT, 0});
}

static void markBuffersAsDontNeed(bool skipLinkedOutput) {
// With --thinlto-index-only, all buffers are nearly unused from now on
// (except symbol/section names used by infrequent passes). Mark input file
Expand Down Expand Up @@ -2556,19 +2562,19 @@ static std::vector<WrappedSymbol> addWrappedSymbols(opt::InputArgList &args) {
continue;

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

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

Symbol *real = symtab.addUnusedUndefined(realName);
Symbol *real = addUnusedUndefined(realName);
v.push_back({sym, real, wrap});

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

parseFiles(files, armCmseImpLib);

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

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

// Prevent LTO from removing any definition referenced by -u.
for (StringRef name : config->undefined)
Expand Down
48 changes: 9 additions & 39 deletions lld/ELF/LinkerScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,15 @@ static bool shouldDefineSym(SymbolAssignment *cmd) {
if (cmd->name == ".")
return false;

return !cmd->provide || LinkerScript::shouldAddProvideSym(cmd->name);
if (!cmd->provide)
return true;

// If a symbol was in PROVIDE(), we need to define it only
// when it is a referenced undefined symbol.
Symbol *b = symtab.find(cmd->name);
if (b && !b->isDefined() && !b->isCommon())
return true;
return false;
}

// Called by processSymbolAssignments() to assign definitions to
Expand Down Expand Up @@ -1683,41 +1691,3 @@ void LinkerScript::checkFinalScriptConditions() const {
checkMemoryRegion(lmaRegion, sec, sec->getLMA());
}
}

void LinkerScript::addScriptReferencedSymbolsToSymTable() {
// Some symbols (such as __ehdr_start) are defined lazily only when there
// are undefined symbols for them, so we add these to trigger that logic.
auto reference = [](StringRef name) {
Symbol *sym = symtab.addUnusedUndefined(name);
sym->isUsedInRegularObj = true;
sym->referenced = true;
};
for (StringRef name : referencedSymbols)
reference(name);

// Keeps track of references from which PROVIDE symbols have been added to the
// symbol table.
DenseSet<StringRef> added;
SmallVector<const SmallVector<StringRef, 0> *, 0> symRefsVec;
for (const auto &[name, symRefs] : provideMap)
if (LinkerScript::shouldAddProvideSym(name) && added.insert(name).second)
symRefsVec.push_back(&symRefs);
while (symRefsVec.size()) {
for (StringRef name : *symRefsVec.pop_back_val()) {
reference(name);
// Prevent the symbol from being discarded by --gc-sections.
script->referencedSymbols.push_back(name);
auto it = script->provideMap.find(name);
if (it != script->provideMap.end() &&
LinkerScript::shouldAddProvideSym(name) &&
added.insert(name).second) {
symRefsVec.push_back(&it->second);
}
}
}
}

bool LinkerScript::shouldAddProvideSym(StringRef symName) {
Symbol *sym = symtab.find(symName);
return sym && !sym->isDefined() && !sym->isCommon();
}
21 changes: 0 additions & 21 deletions lld/ELF/LinkerScript.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Compiler.h"
#include <cstddef>
Expand Down Expand Up @@ -366,18 +365,6 @@ class LinkerScript final {
// Check backward location counter assignment and memory region/LMA overflows.
void checkFinalScriptConditions() const;

// Add symbols that are referenced in the linker script to the symbol table.
// Symbols referenced in a PROVIDE command are only added to the symbol table
// if the PROVIDE command actually provides the symbol.
// It also adds the symbols referenced by the used PROVIDE symbols to the
// linker script referenced symbols list.
void addScriptReferencedSymbolsToSymTable();

// Returns true if the PROVIDE symbol should be added to the link.
// A PROVIDE symbol is added to the link only if it satisfies an
// undefined reference.
static bool shouldAddProvideSym(StringRef symName);

// SECTIONS command list.
SmallVector<SectionCommand *, 0> sectionCommands;

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

// Stores the mapping: PROVIDE symbol -> symbols referred in the PROVIDE
// expression. For example, if the PROVIDE command is:
//
// PROVIDE(v = a + b + c);
//
// then provideMap should contain the mapping: 'v' -> ['a', 'b', 'c']
llvm::MapVector<StringRef, SmallVector<StringRef, 0>> provideMap;

// List of potential spill locations (PotentialSpillSection) for an input
// section.
struct PotentialSpillList {
Expand Down
13 changes: 1 addition & 12 deletions lld/ELF/ScriptParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include "llvm/Support/TimeProfiler.h"
#include <cassert>
#include <limits>
#include <optional>
#include <vector>

using namespace llvm;
Expand Down Expand Up @@ -140,10 +139,6 @@ class ScriptParser final : ScriptLexer {

// A set to detect an INCLUDE() cycle.
StringSet<> seen;

// If we are currently parsing a PROVIDE|PROVIDE_HIDDEN command,
// then this member is set to the PROVIDE symbol name.
std::optional<llvm::StringRef> activeProvideSym;
};
} // namespace

Expand Down Expand Up @@ -1088,9 +1083,6 @@ SymbolAssignment *ScriptParser::readProvideHidden(bool provide, bool hidden) {
;
return nullptr;
}
llvm::SaveAndRestore saveActiveProvideSym(activeProvideSym);
if (provide)
activeProvideSym = name;
SymbolAssignment *cmd = readSymbolAssignment(name);
cmd->provide = provide;
cmd->hidden = hidden;
Expand Down Expand Up @@ -1604,10 +1596,7 @@ Expr ScriptParser::readPrimary() {
tok = unquote(tok);
else if (!isValidSymbolName(tok))
setError("malformed number: " + tok);
if (activeProvideSym)
script->provideMap[*activeProvideSym].push_back(tok);
else
script->referencedSymbols.push_back(tok);
script->referencedSymbols.push_back(tok);
return [=] { return script->getSymbolValue(tok, location); };
}

Expand Down
4 changes: 0 additions & 4 deletions lld/ELF/SymbolTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,3 @@ void SymbolTable::scanVersionScript() {
// --dynamic-list.
handleDynamicList();
}

Symbol *SymbolTable::addUnusedUndefined(StringRef name, uint8_t binding) {
return addSymbol(Undefined{ctx.internalFile, name, binding, STV_DEFAULT, 0});
}
3 changes: 0 additions & 3 deletions lld/ELF/SymbolTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ class SymbolTable {

void handleDynamicList();

Symbol *addUnusedUndefined(StringRef name,
uint8_t binding = llvm::ELF::STB_GLOBAL);

// Set of .so files to not link the same shared object file more than once.
llvm::DenseMap<llvm::CachedHashStringRef, SharedFile *> soNames;

Expand Down
60 changes: 0 additions & 60 deletions lld/test/ELF/gc-sections-with-provide.s

This file was deleted.

51 changes: 7 additions & 44 deletions lld/test/ELF/linkerscript/symbolreferenced.s
Original file line number Diff line number Diff line change
Expand Up @@ -21,31 +21,11 @@
# RUN: ld.lld -o chain -T chain.t a.o
# RUN: llvm-nm chain | FileCheck %s

# CHECK-NOT: another_unused
# CHECK: 0000000000007000 a f1
# CHECK-NEXT: 0000000000007000 A f2
# CHECK-NEXT: 0000000000007000 A f3
# CHECK-NEXT: 0000000000007000 A f4
# CHECK-NEXT: 0000000000006000 A f5
# CHECK-NEXT: 0000000000003000 A f6
# CHECK-NEXT: 0000000000001000 A f7
# CHECK-NOT: g1
# CHECK-NOT: g2
# CHECK-NEXT: 0000000000007500 A newsym
# CHECK: 0000000000002000 A u
# CHECK-NOT: unused
# CHECK-NEXT: 0000000000002000 A v
# CHECK-NEXT: 0000000000002000 A w


# RUN: ld.lld -o chain_with_cycle -T chain_with_cycle.t a.o
# RUN: llvm-nm chain_with_cycle | FileCheck %s --check-prefix=CHAIN_WITH_CYCLE

# CHAIN_WITH_CYCLE: 000 A f1
# CHAIN_WITH_CYCLE: 000 A f2
# CHAIN_WITH_CYCLE: 000 A f3
# CHAIN_WITH_CYCLE: 000 A f4
# CHAIN_WITH_CYCLE: 000 A newsym
# CHECK: 0000000000001000 a f1
# CHECK-NEXT: 0000000000001000 A f2
# CHECK-NEXT: 0000000000001000 a g1
# CHECK-NEXT: 0000000000001000 A g2
# CHECK-NEXT: 0000000000001000 A newsym

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

#--- chain.t
PROVIDE(f7 = 0x1000);
PROVIDE(f5 = f6 + 0x3000);
PROVIDE(f6 = f7 + 0x2000);
PROVIDE(f4 = f5 + 0x1000);
PROVIDE(f3 = f4);
PROVIDE(f2 = f3);
PROVIDE(f2 = 0x1000);
PROVIDE_HIDDEN(f1 = f2);
PROVIDE(newsym = f1 + 0x500);

u = v;
PROVIDE(w = 0x2000);
PROVIDE(v = w);
PROVIDE(newsym = f1);

PROVIDE(g2 = 0x1000);
PROVIDE_HIDDEN(g1 = g2);
PROVIDE(unused = g1);
PROVIDE_HIDDEN(another_unused = g1);

#--- chain_with_cycle.t
PROVIDE(f1 = f2 + f3);
PROVIDE(f2 = f3 + f4);
PROVIDE(f3 = f4);
PROVIDE(f4 = f1);
PROVIDE(newsym = f1);

#--- chain2.t
PROVIDE(f2 = undef);
Expand Down

0 comments on commit 3a17f74

Please sign in to comment.