Skip to content

Commit 503cb3a

Browse files
MaskRayDanielCChen
authored andcommitted
[ELF] Fix PROVIDE_HIDDEN -shared regression with bitcode file references
The inaccurate llvm#111945 condition fixes a PROVIDE regression (llvm#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: llvm#112386
1 parent ea2e1cd commit 503cb3a

File tree

3 files changed

+42
-13
lines changed

3 files changed

+42
-13
lines changed

lld/ELF/LinkerScript.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ static bool shouldDefineSym(Ctx &ctx, SymbolAssignment *cmd) {
200200
if (cmd->name == ".")
201201
return false;
202202

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

206206
// Called by processSymbolAssignments() to assign definitions to
@@ -1798,30 +1798,33 @@ void LinkerScript::addScriptReferencedSymbolsToSymTable() {
17981798
DenseSet<StringRef> added;
17991799
SmallVector<const SmallVector<StringRef, 0> *, 0> symRefsVec;
18001800
for (const auto &[name, symRefs] : provideMap)
1801-
if (shouldAddProvideSym(ctx, name) && added.insert(name).second)
1801+
if (shouldAddProvideSym(name) && added.insert(name).second)
18021802
symRefsVec.push_back(&symRefs);
18031803
while (symRefsVec.size()) {
18041804
for (StringRef name : *symRefsVec.pop_back_val()) {
18051805
reference(name);
18061806
// Prevent the symbol from being discarded by --gc-sections.
18071807
referencedSymbols.push_back(name);
18081808
auto it = provideMap.find(name);
1809-
if (it != provideMap.end() && shouldAddProvideSym(ctx, name) &&
1809+
if (it != provideMap.end() && shouldAddProvideSym(name) &&
18101810
added.insert(name).second) {
18111811
symRefsVec.push_back(&it->second);
18121812
}
18131813
}
18141814
}
18151815
}
18161816

1817-
bool LinkerScript::shouldAddProvideSym(Ctx &ctx, StringRef symName) {
1817+
bool LinkerScript::shouldAddProvideSym(StringRef symName) {
18181818
// This function is called before and after garbage collection. To prevent
18191819
// undefined references from the RHS, the result of this function for a
1820-
// symbol must be the same for each call. We use isUsedInRegularObj to not
1821-
// change the return value of a demoted symbol. The exportDynamic condition,
1822-
// while not so accurate, allows PROVIDE to define a symbol referenced by a
1823-
// DSO.
1820+
// symbol must be the same for each call. We use unusedProvideSyms to not
1821+
// change the return value of a demoted symbol.
18241822
Symbol *sym = ctx.symtab->find(symName);
1825-
return sym && !sym->isDefined() && !sym->isCommon() &&
1826-
(sym->isUsedInRegularObj || sym->exportDynamic);
1823+
if (!sym)
1824+
return false;
1825+
if (sym->isDefined() || sym->isCommon()) {
1826+
unusedProvideSyms.insert(sym);
1827+
return false;
1828+
}
1829+
return !unusedProvideSyms.count(sym);
18271830
}

lld/ELF/LinkerScript.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ class LinkerScript final {
389389
// Returns true if the PROVIDE symbol should be added to the link.
390390
// A PROVIDE symbol is added to the link only if it satisfies an
391391
// undefined reference.
392-
static bool shouldAddProvideSym(Ctx &, StringRef symName);
392+
bool shouldAddProvideSym(StringRef symName);
393393

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

437439
// List of potential spill locations (PotentialSpillSection) for an input
438440
// section.

lld/test/ELF/linkerscript/provide-defined.s

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,27 @@
44
# RUN: rm -rf %t && split-file %s %t && cd %t
55
# RUN: llvm-mc -filetype=obj -triple=x86_64 a.s -o a.o
66
# RUN: llvm-mc -filetype=obj -triple=x86_64 b.s -o b.o
7+
# RUN: llvm-as c.ll -o c.bc
78
# RUN: ld.lld -T a.t --gc-sections a.o b.o -o a
89
# RUN: llvm-readelf -s a | FileCheck %s
910

11+
# RUN: ld.lld -T a.t -shared a.o b.o c.bc -o a.so
12+
# RUN: llvm-readelf -s -r a.so | FileCheck %s --check-prefix=DSO
13+
1014
# CHECK: 1: {{.*}} 0 NOTYPE GLOBAL DEFAULT 1 _start
11-
# CHECK-NEXT:2: {{.*}} 0 NOTYPE GLOBAL DEFAULT 2 f3
15+
# CHECK-NEXT:2: {{.*}} 0 NOTYPE WEAK DEFAULT 2 f3
1216
# CHECK-NOT: {{.}}
1317

18+
# DSO: .rela.plt
19+
# DSO-NOT: f5
20+
# DSO: Symbol table '.dynsym'
21+
# DSO-NOT: f5
22+
# DSO: Symbol table '.symtab'
23+
# DSO: {{.*}} 0 NOTYPE LOCAL HIDDEN [[#]] f5
24+
1425
#--- a.s
15-
.global _start, f1, f2, f3, bar
26+
.global _start, f1, f2, bar
27+
.weak f3
1628
_start:
1729
call f3
1830

@@ -26,11 +38,23 @@ _start:
2638
#--- b.s
2739
call f2
2840

41+
#--- c.ll
42+
target triple = "x86_64-unknown-linux-gnu"
43+
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
44+
45+
declare void @f5()
46+
47+
define void @f3() {
48+
call void @f5()
49+
ret void
50+
}
51+
2952
#--- a.t
3053
SECTIONS {
3154
. = . + SIZEOF_HEADERS;
3255
PROVIDE(f1 = bar+1);
3356
PROVIDE(f2 = bar+2);
3457
PROVIDE(f3 = bar+3);
3558
PROVIDE(f4 = comm+4);
59+
PROVIDE_HIDDEN(f5 = bar+5);
3660
}

0 commit comments

Comments
 (0)