Skip to content

Commit ff8b06a

Browse files
committed
[ELF] Add --[no-]allow-non-exported-symbols-shared-with-dso
Commit 1981b1b unexpectedly strengthened --no-allow-shlib-undefined which will be relaxed by llvm#70130. This patch adds --no-allow-non-exported-symbols-shared-with-dso to restore the check and make the check catch more cases: * report errors in the presence of a DSO definition * make the check work whether or not --gc-sections discards the symbol Commit 1981b1b has caught several brittle build issues. For example, ``` libfdio.so: reference to _Znam libclang_rt.asan.so: shared definition of _Znam libc++.a(stdlib_new_delete.cpp.obj): definition of _Znam ``` The executable contains a definition from `libc++.a` while at run-time, libfdio.so's `_Znam` reference gets resolved to `libclang_rt.asan.so`. This scenarios is often undesired. In this case, a possible improvement is to switch to an asan-instrumented libc++.a that does not define `_Znam`. I have also seen problems due to mixing multiple definitions from `libgcc.a` (hidden visibility) and `libclang_rt.builtins.a` (default visibility) and relying on archive member extraction rules to work. --- * I wonder whether this option is useful enough to justify a new option. * Using protected visibility can cause similar multiple definition issues. Is it useful to generalize this option?
1 parent 5f6dd33 commit ff8b06a

File tree

8 files changed

+75
-0
lines changed

8 files changed

+75
-0
lines changed

lld/ELF/Config.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,7 @@ struct Config {
331331
StripPolicy strip;
332332
UnresolvedPolicy unresolvedSymbols;
333333
UnresolvedPolicy unresolvedSymbolsInShlib;
334+
bool allowNonExportedSymbolsSharedWithDso = true;
334335
Target2Policy target2;
335336
bool power10Stubs;
336337
ARMVFPArgKind armVFPArgs = ARMVFPArgKind::Default;

lld/ELF/Driver.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,6 +1215,9 @@ static void readConfigs(opt::InputArgList &args) {
12151215
errorHandler().vsDiagnostics =
12161216
args.hasArg(OPT_visual_studio_diagnostics_format, false);
12171217

1218+
config->allowNonExportedSymbolsSharedWithDso =
1219+
args.hasFlag(OPT_allow_non_exported_symbols_shared_with_dso,
1220+
OPT_no_allow_non_exported_symbols_shared_with_dso, true);
12181221
config->allowMultipleDefinition =
12191222
args.hasFlag(OPT_allow_multiple_definition,
12201223
OPT_no_allow_multiple_definition, false) ||

lld/ELF/InputFiles.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,6 +1521,7 @@ template <class ELFT> void SharedFile::parse() {
15211521
Symbol *s = symtab.addSymbol(
15221522
Undefined{this, name, sym.getBinding(), sym.st_other, sym.getType()});
15231523
s->exportDynamic = true;
1524+
s->setFlags(HAS_SHARED_REF);
15241525
if (s->isUndefined() && sym.getBinding() != STB_WEAK &&
15251526
config->unresolvedSymbolsInShlib != UnresolvedPolicy::Ignore)
15261527
requiredSymbols.push_back(s);

lld/ELF/Options.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ defm Ttext: Eq<"Ttext", "Same as --section-start with .text as the sectionname">
105105

106106
def Ttext_segment: Separate<["-", "--"], "Ttext-segment">;
107107

108+
defm allow_non_exported_symbols_shared_with_dso: BB<"allow-non-exported-symbols-shared-with-dso",
109+
"Allow non-exported symbols that are also defined or referenced by a DSO (default)",
110+
"Do not allow non-exported symbols that are also defined or referenced by a DSO">;
111+
108112
defm allow_multiple_definition: B<"allow-multiple-definition",
109113
"Allow multiple definitions",
110114
"Do not allow multiple definitions (default)">;

lld/ELF/Symbols.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ enum {
5555
NEEDS_GOT_DTPREL = 1 << 7,
5656
NEEDS_TLSIE = 1 << 8,
5757
HAS_SHARED_DEF = 1 << 9,
58+
HAS_SHARED_REF = 1 << 10,
5859
};
5960

6061
// Some index properties of a symbol are stored separately in this auxiliary

lld/ELF/Writer.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,23 @@ static void demoteDefined(Defined &sym, DenseMap<SectionBase *, size_t> &map) {
272272
static void demoteSymbolsAndComputeIsPreemptible() {
273273
llvm::TimeTraceScope timeScope("Demote symbols");
274274
DenseMap<InputFile *, DenseMap<SectionBase *, size_t>> sectionIndexMap;
275+
bool allowNonExportedSymbolsSharedWithDso =
276+
config->allowNonExportedSymbolsSharedWithDso;
275277
for (Symbol *sym : symtab.getSymbols()) {
276278
if (auto *d = dyn_cast<Defined>(sym)) {
279+
if (!allowNonExportedSymbolsSharedWithDso &&
280+
d->visibility() != ELF::STV_DEFAULT &&
281+
d->visibility() != ELF::STV_PROTECTED) {
282+
auto flags = d->flags.load(std::memory_order_relaxed);
283+
if (flags & HAS_SHARED_DEF) {
284+
errorOrWarn("non-exported symbol also defined by DSO: " + toString(*sym) +
285+
"\n>>> defined by " + toString(d->file));
286+
} else if (flags & HAS_SHARED_REF) {
287+
errorOrWarn("non-exported symbol also referenced by DSO: " +
288+
toString(*sym) + "\n>>> defined by " + toString(d->file));
289+
}
290+
}
291+
277292
if (d->section && !d->section->isLive())
278293
demoteDefined(*d, sectionIndexMap[d->file]);
279294
} else {
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# REQUIRES: x86
2+
# RUN: rm -rf %t && split-file %s %t && cd %t
3+
# RUN: llvm-mc -filetype=obj -triple=x86_64 a.s -o a.o
4+
# RUN: llvm-mc -filetype=obj -triple=x86_64 weak.s -o weak.o
5+
6+
# RUN: llvm-mc -filetype=obj -triple=x86_64 def.s -o def.o
7+
# RUN: ld.lld -shared -soname=def.so def.o -o def.so
8+
# RUN: ld.lld a.o def.so -o /dev/null
9+
# RUN: ld.lld a.o def.so -o /dev/null --no-allow-non-exported-symbols-shared-with-dso --allow-non-exported-symbols-shared-with-dso
10+
11+
# RUN: not ld.lld a.o def.so --no-allow-non-exported-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=CHECK1
12+
# RUN: not ld.lld --gc-sections weak.o def.so a.o --no-allow-non-exported-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=CHECK1
13+
14+
# CHECK1: error: non-exported symbol also defined by DSO: foo
15+
# CHECK1-NEXT: >>> defined by {{.*}}a.o
16+
17+
# RUN: llvm-mc -filetype=obj -triple=x86_64 ref.s -o ref.o
18+
# RUN: ld.lld -shared -soname=ref.so ref.o -o ref.so
19+
# RUN: not ld.lld a.o ref.so --no-allow-non-exported-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=CHECK2
20+
21+
# CHECK2: error: non-exported symbol also referenced by DSO: foo
22+
# CHECK2-NEXT: >>> defined by {{.*}}a.o
23+
24+
## See also allow-shlib-undefined.s.
25+
26+
#--- a.s
27+
.globl _start, foo
28+
_start:
29+
30+
## The check kicks in even if .text.foo is discarded.
31+
.section .text.foo,"ax"
32+
.hidden foo
33+
foo:
34+
35+
#--- weak.s
36+
.weak foo
37+
foo:
38+
39+
#--- def.s
40+
.globl foo
41+
foo:
42+
43+
#--- ref.s
44+
call foo

lld/test/ELF/allow-shlib-undefined.s

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
## The definition def.so is ignored.
4242
# RUN: ld.lld -shared def.o -o def.so
4343
# RUN: ld.lld --gc-sections main.o a.so def.so def-hidden.o --fatal-warnings -o /dev/null
44+
# RUN: not ld.lld --gc-sections main.o a.so def.so def-hidden.o -o /dev/null --no-allow-non-exported-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=HIDDEN
4445

4546
# CHECK-NOT: error:
4647
# CHECK: error: undefined reference due to --no-allow-shlib-undefined: x1{{$}}
@@ -61,6 +62,11 @@
6162
# NONEXPORTED: error: non-exported symbol 'x1' in 'def-hidden.o' is referenced by DSO 'a.so'
6263
# NONEXPORTED-NOT: {{.}}
6364

65+
# HIDDEN-NOT: error:
66+
# HIDDEN: error: non-exported symbol also defined by DSO: x1
67+
# HIDDEN-NEXT: >>> defined by def-hidden.o
68+
# HIDDEN-NOT: {{.}}
69+
6470
#--- main.s
6571
.globl _start
6672
_start:

0 commit comments

Comments
 (0)