[lld] lld allow hidden symbols shared with dso (PR #70163)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 24 22:13:05 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lld-elf
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
- [ELF] Suppress --no-allow-shlib-undefined diagnostic when a SharedSymbol is overridden by a hidden visibility Defined which is later discarded
- [ELF] Add --[no-]allow-hidden-symbols-shared-with-dso
---
Full diff: https://github.com/llvm/llvm-project/pull/70163.diff
8 Files Affected:
- (modified) lld/ELF/Config.h (+1)
- (modified) lld/ELF/Driver.cpp (+3)
- (modified) lld/ELF/InputFiles.cpp (+3)
- (modified) lld/ELF/Options.td (+4)
- (modified) lld/ELF/Symbols.h (+2)
- (modified) lld/ELF/Writer.cpp (+18-1)
- (added) lld/test/ELF/allow-hidden-symbols-shared-with-dso.s (+44)
- (modified) lld/test/ELF/allow-shlib-undefined.s (+7-1)
``````````diff
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 56229334f9a44ae..36e8abb0b09eb73 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -331,6 +331,7 @@ struct Config {
StripPolicy strip;
UnresolvedPolicy unresolvedSymbols;
UnresolvedPolicy unresolvedSymbolsInShlib;
+ bool allowHiddenSymbolsSharedWithDso = true;
Target2Policy target2;
bool power10Stubs;
ARMVFPArgKind armVFPArgs = ARMVFPArgKind::Default;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 5f88389a5840824..6e1d51f5463b674 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1215,6 +1215,9 @@ static void readConfigs(opt::InputArgList &args) {
errorHandler().vsDiagnostics =
args.hasArg(OPT_visual_studio_diagnostics_format, false);
+ config->allowHiddenSymbolsSharedWithDso =
+ args.hasFlag(OPT_allow_hidden_symbols_shared_with_dso,
+ OPT_no_allow_hidden_symbols_shared_with_dso, true);
config->allowMultipleDefinition =
args.hasFlag(OPT_allow_multiple_definition,
OPT_no_allow_multiple_definition, false) ||
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index a0d4be8ff9885b0..86e91b88d3aa82d 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -1521,6 +1521,7 @@ template <class ELFT> void SharedFile::parse() {
Symbol *s = symtab.addSymbol(
Undefined{this, name, sym.getBinding(), sym.st_other, sym.getType()});
s->exportDynamic = true;
+ s->setFlags(HAS_SHARED_REF);
if (s->isUndefined() && sym.getBinding() != STB_WEAK &&
config->unresolvedSymbolsInShlib != UnresolvedPolicy::Ignore)
requiredSymbols.push_back(s);
@@ -1545,6 +1546,7 @@ template <class ELFT> void SharedFile::parse() {
auto *s = symtab.addSymbol(
SharedSymbol{*this, name, sym.getBinding(), sym.st_other,
sym.getType(), sym.st_value, sym.st_size, alignment});
+ s->setFlags(HAS_SHARED_DEF);
if (s->file == this)
s->verdefIndex = ver;
}
@@ -1562,6 +1564,7 @@ template <class ELFT> void SharedFile::parse() {
auto *s = symtab.addSymbol(
SharedSymbol{*this, saver().save(name), sym.getBinding(), sym.st_other,
sym.getType(), sym.st_value, sym.st_size, alignment});
+ s->setFlags(HAS_SHARED_DEF);
if (s->file == this)
s->verdefIndex = idx;
}
diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index 9a23f48350644a0..b82b7b70c2bc79d 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -105,6 +105,10 @@ defm Ttext: Eq<"Ttext", "Same as --section-start with .text as the sectionname">
def Ttext_segment: Separate<["-", "--"], "Ttext-segment">;
+defm allow_hidden_symbols_shared_with_dso: BB<"allow-hidden-symbols-shared-with-dso",
+ "Allow hidden visibility symbols that are also defined or referenced by a DSO (default)",
+ "Do not allow hidden visibility symbols that are also defined or referenced by a DSO">;
+
defm allow_multiple_definition: B<"allow-multiple-definition",
"Allow multiple definitions",
"Do not allow multiple definitions (default)">;
diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 4addb79d1257914..02935ac53826c7a 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -54,6 +54,8 @@ enum {
NEEDS_TLSGD_TO_IE = 1 << 6,
NEEDS_GOT_DTPREL = 1 << 7,
NEEDS_TLSIE = 1 << 8,
+ HAS_SHARED_DEF = 1 << 9,
+ HAS_SHARED_REF = 1 << 10,
};
// Some index properties of a symbol are stored separately in this auxiliary
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 57e1aa06c6aa873..82e8b9985b6192d 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -272,8 +272,20 @@ static void demoteDefined(Defined &sym, DenseMap<SectionBase *, size_t> &map) {
static void demoteSymbolsAndComputeIsPreemptible() {
llvm::TimeTraceScope timeScope("Demote symbols");
DenseMap<InputFile *, DenseMap<SectionBase *, size_t>> sectionIndexMap;
+ bool allowHiddenSymbolsSharedWithDso = config->allowHiddenSymbolsSharedWithDso;
for (Symbol *sym : symtab.getSymbols()) {
if (auto *d = dyn_cast<Defined>(sym)) {
+ if (!allowHiddenSymbolsSharedWithDso && d->visibility() == ELF::STV_HIDDEN) {
+ auto flags = d->flags.load(std::memory_order_relaxed);
+ if (flags & HAS_SHARED_DEF) {
+ errorOrWarn("hidden symbol also defined by DSO: " + toString(*sym) +
+ "\n>>> defined by " + toString(d->file));
+ } else if (flags & HAS_SHARED_REF) {
+ errorOrWarn("hidden symbol also referenced by DSO: " + toString(*sym) +
+ "\n>>> defined by " + toString(d->file));
+ }
+ }
+
if (d->section && !d->section->isLive())
demoteDefined(*d, sectionIndexMap[d->file]);
} else {
@@ -2016,6 +2028,11 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
// to catch more cases. That is too much for us. Our approach resembles
// the one used in ld.gold, achieves a good balance to be useful but not
// too smart.
+ //
+ // If a DSO reference is resolved by a SharedSymbol, but the SharedSymbol
+ // is overridden by a hidden visibility Defined (which is later discarded
+ // due to GC), don't report the diagnostic. However, this may indicate an
+ // unintended SharedSymbol.
for (SharedFile *file : ctx.sharedFiles) {
bool allNeededIsKnown =
llvm::all_of(file->dtNeeded, [&](StringRef needed) {
@@ -2024,7 +2041,7 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
if (!allNeededIsKnown)
continue;
for (Symbol *sym : file->requiredSymbols)
- if (sym->isUndefined() && !sym->isWeak())
+ if (sym->isUndefined() && !sym->isWeak() && !sym->hasFlag(HAS_SHARED_DEF))
diagnose("undefined reference due to --no-allow-shlib-undefined: " +
toString(*sym) + "\n>>> referenced by " + toString(file));
}
diff --git a/lld/test/ELF/allow-hidden-symbols-shared-with-dso.s b/lld/test/ELF/allow-hidden-symbols-shared-with-dso.s
new file mode 100644
index 000000000000000..24dfc6c49036ab0
--- /dev/null
+++ b/lld/test/ELF/allow-hidden-symbols-shared-with-dso.s
@@ -0,0 +1,44 @@
+# REQUIRES: x86
+# 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 weak.s -o weak.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 def.s -o def.o
+# RUN: ld.lld -shared -soname=def.so def.o -o def.so
+# RUN: ld.lld a.o def.so -o /dev/null
+# RUN: ld.lld a.o def.so -o /dev/null --no-allow-hidden-symbols-shared-with-dso --allow-hidden-symbols-shared-with-dso
+
+# RUN: not ld.lld a.o def.so --no-allow-hidden-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=CHECK1
+# RUN: not ld.lld --gc-sections weak.o def.so a.o --no-allow-hidden-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=CHECK1
+
+# CHECK1: error: hidden symbol also defined by DSO: foo
+# CHECK1-NEXT: >>> defined by {{.*}}a.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 ref.s -o ref.o
+# RUN: ld.lld -shared -soname=ref.so ref.o -o ref.so
+# RUN: not ld.lld a.o ref.so --no-allow-hidden-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=CHECK2
+
+# CHECK2: error: hidden symbol also referenced by DSO: foo
+# CHECK2-NEXT: >>> defined by {{.*}}a.o
+
+## See also allow-shlib-undefined.s.
+
+#--- a.s
+.globl _start, foo
+_start:
+
+## The check kicks in even if .text.foo is discarded.
+.section .text.foo,"ax"
+.hidden foo
+foo:
+
+#--- weak.s
+.weak foo
+foo:
+
+#--- def.s
+.globl foo
+foo:
+
+#--- ref.s
+call foo
diff --git a/lld/test/ELF/allow-shlib-undefined.s b/lld/test/ELF/allow-shlib-undefined.s
index 03f047b02d75d52..01ef5073a865a75 100644
--- a/lld/test/ELF/allow-shlib-undefined.s
+++ b/lld/test/ELF/allow-shlib-undefined.s
@@ -46,7 +46,10 @@
# RUN: not ld.lld --gc-sections %t.o %t.so %tdef-hidden.o -o /dev/null 2>&1 | FileCheck %s
## The definition %tdef.so is ignored.
# RUN: ld.lld -shared -soname=tdef.so %tdef.o -o %tdef.so
-# RUN: not ld.lld --gc-sections %t.o %t.so %tdef.so %tdef-hidden.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: ld.lld --gc-sections %t.o %t.so %tdef.so %tdef-hidden.o -o /dev/null --fatal-warnings
+# RUN: ld.lld --gc-sections %t.o %t.so %tdef-hidden.o %tdef.so -o /dev/null --fatal-warnings
+
+# RUN: not ld.lld --gc-sections %t.o %t.so %tdef.so %tdef-hidden.o -o /dev/null --no-allow-hidden-symbols-shared-with-dso 2>&1 | FileCheck %s --check-prefix=HIDDEN
.globl _start
_start:
@@ -58,3 +61,6 @@ _start:
# CHECK2-NEXT: >>> referenced by {{.*}}2.so
# WARN: warning: undefined reference due to --no-allow-shlib-undefined: _unresolved
# WARN-NEXT: >>> referenced by {{.*}}.so
+
+# HIDDEN: error: hidden symbol also defined by DSO: _unresolved
+# HIDDEN-NEXT: >>> defined by {{.*}}def-hidden.o
``````````
</details>
https://github.com/llvm/llvm-project/pull/70163
More information about the llvm-commits
mailing list