Skip to content

Commit

Permalink
[ELF] Fix PROVIDE_HIDDEN -shared regression with bitcode file references
Browse files Browse the repository at this point in the history
The inaccurate #111945 condition fixes a PROVIDE regression (#111478)
but introduces another regression: in a DSO link, if a symbol referenced
only by bitcode files is defined as PROVIDE_HIDDEN, lld would not set
the visibility correctly, leading to an assertion failure in
DynamicReloc::getSymIndex (https://reviews.llvm.org/D123985).
This is because `(sym->isUsedInRegularObj || sym->exportDynamic)` is
initially false (bitcode undef does not set `isUsedInRegularObj`) then
true (in `addSymbol`, after LTO compilation).

Fix this by making the condition accurate: use a map to track defined
symbols.

Reviewers: smithp35

Reviewed By: smithp35

Pull Request: #112386
  • Loading branch information
MaskRay authored Oct 15, 2024
1 parent 47a6da2 commit a2359a8
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 13 deletions.
23 changes: 13 additions & 10 deletions lld/ELF/LinkerScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ static bool shouldDefineSym(Ctx &ctx, SymbolAssignment *cmd) {
if (cmd->name == ".")
return false;

return !cmd->provide || LinkerScript::shouldAddProvideSym(ctx, cmd->name);
return !cmd->provide || ctx.script->shouldAddProvideSym(cmd->name);
}

// Called by processSymbolAssignments() to assign definitions to
Expand Down Expand Up @@ -1798,30 +1798,33 @@ void LinkerScript::addScriptReferencedSymbolsToSymTable() {
DenseSet<StringRef> added;
SmallVector<const SmallVector<StringRef, 0> *, 0> symRefsVec;
for (const auto &[name, symRefs] : provideMap)
if (shouldAddProvideSym(ctx, name) && added.insert(name).second)
if (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.
referencedSymbols.push_back(name);
auto it = provideMap.find(name);
if (it != provideMap.end() && shouldAddProvideSym(ctx, name) &&
if (it != provideMap.end() && shouldAddProvideSym(name) &&
added.insert(name).second) {
symRefsVec.push_back(&it->second);
}
}
}
}

bool LinkerScript::shouldAddProvideSym(Ctx &ctx, StringRef symName) {
bool LinkerScript::shouldAddProvideSym(StringRef symName) {
// This function is called before and after garbage collection. To prevent
// undefined references from the RHS, the result of this function for a
// symbol must be the same for each call. We use isUsedInRegularObj to not
// change the return value of a demoted symbol. The exportDynamic condition,
// while not so accurate, allows PROVIDE to define a symbol referenced by a
// DSO.
// symbol must be the same for each call. We use unusedProvideSyms to not
// change the return value of a demoted symbol.
Symbol *sym = ctx.symtab->find(symName);
return sym && !sym->isDefined() && !sym->isCommon() &&
(sym->isUsedInRegularObj || sym->exportDynamic);
if (!sym)
return false;
if (sym->isDefined() || sym->isCommon()) {
unusedProvideSyms.insert(sym);
return false;
}
return !unusedProvideSyms.count(sym);
}
4 changes: 3 additions & 1 deletion lld/ELF/LinkerScript.h
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ class LinkerScript final {
// 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(Ctx &, StringRef symName);
bool shouldAddProvideSym(StringRef symName);

// SECTIONS command list.
SmallVector<SectionCommand *, 0> sectionCommands;
Expand Down Expand Up @@ -433,6 +433,8 @@ class LinkerScript final {
//
// then provideMap should contain the mapping: 'v' -> ['a', 'b', 'c']
llvm::MapVector<StringRef, SmallVector<StringRef, 0>> provideMap;
// Store defined symbols that should ignore PROVIDE commands.
llvm::DenseSet<Symbol *> unusedProvideSyms;

// List of potential spill locations (PotentialSpillSection) for an input
// section.
Expand Down
28 changes: 26 additions & 2 deletions lld/test/ELF/linkerscript/provide-defined.s
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,27 @@
# RUN: rm -rf %t && split-file %s %t && cd %t
# RUN: llvm-mc -filetype=obj -triple=x86_64 a.s -o a.o
# RUN: llvm-mc -filetype=obj -triple=x86_64 b.s -o b.o
# RUN: llvm-as c.ll -o c.bc
# RUN: ld.lld -T a.t --gc-sections a.o b.o -o a
# RUN: llvm-readelf -s a | FileCheck %s

# RUN: ld.lld -T a.t -shared a.o b.o c.bc -o a.so
# RUN: llvm-readelf -s -r a.so | FileCheck %s --check-prefix=DSO

# CHECK: 1: {{.*}} 0 NOTYPE GLOBAL DEFAULT 1 _start
# CHECK-NEXT:2: {{.*}} 0 NOTYPE GLOBAL DEFAULT 2 f3
# CHECK-NEXT:2: {{.*}} 0 NOTYPE WEAK DEFAULT 2 f3
# CHECK-NOT: {{.}}

# DSO: .rela.plt
# DSO-NOT: f5
# DSO: Symbol table '.dynsym'
# DSO-NOT: f5
# DSO: Symbol table '.symtab'
# DSO: {{.*}} 0 NOTYPE LOCAL HIDDEN [[#]] f5

#--- a.s
.global _start, f1, f2, f3, bar
.global _start, f1, f2, bar
.weak f3
_start:
call f3

Expand All @@ -26,11 +38,23 @@ _start:
#--- b.s
call f2

#--- c.ll
target triple = "x86_64-unknown-linux-gnu"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

declare void @f5()

define void @f3() {
call void @f5()
ret void
}

#--- a.t
SECTIONS {
. = . + SIZEOF_HEADERS;
PROVIDE(f1 = bar+1);
PROVIDE(f2 = bar+2);
PROVIDE(f3 = bar+3);
PROVIDE(f4 = comm+4);
PROVIDE_HIDDEN(f5 = bar+5);
}

0 comments on commit a2359a8

Please sign in to comment.