[lld] [lld][elf] Warn if '*' pattern is used multiple times in version scripts (PR #102669)
Igor Kudrin via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 13 12:54:50 PDT 2024
https://github.com/igorkudrin updated https://github.com/llvm/llvm-project/pull/102669
>From e1a204e0219fe5ae3b914bbb8ee4eaab28a3c491 Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Thu, 8 Aug 2024 22:50:42 -0700
Subject: [PATCH 1/3] [lld][elf] Warn if '*' pattern is used multiple times in
version scripts
If this pattern is used more than once in version script(s), only one
will have an effect, so it's probably a user error and can be diagnosed.
---
lld/ELF/SymbolTable.cpp | 35 +++++++++++++++++++--
lld/test/ELF/version-script-reassign-glob.s | 4 ++-
lld/test/ELF/version-script-warn.s | 35 +++++++++++++++++++++
3 files changed, 71 insertions(+), 3 deletions(-)
create mode 100644 lld/test/ELF/version-script-warn.s
diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index 258a78ab40bb57..f7465cd5d097fc 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -311,13 +311,44 @@ void SymbolTable::scanVersionScript() {
// Then, assign versions to "*". In GNU linkers they have lower priority than
// other wildcards.
+ bool globalAsteriskWildcardFound = false;
+ bool localAsteriskWildcardFound = false;
+ bool asteriskWildcardReported = false;
+ auto assignAsteriskWildcard = [&](SymbolVersion &pat, VersionDefinition *ver,
+ bool isLocal) {
+ // Avoid issuing a warning if both '--retain-symbol-file' and a version
+ // script with `global: *` are used.
+ //
+ // '--retain-symbol-file' adds a "*" pattern to
+ // 'config->versionDefinitions[VER_NDX_LOCAL].nonLocalPatterns', see
+ // 'readConfigs()' in 'Driver.cpp'. Note that it is '.nonLocalPatterns', not
+ // '.localPatterns', which may seem counterintuitive, but still works as
+ // expected. Here we can exploit that and skip analyzing the pattern added
+ // for this option.
+ if (!asteriskWildcardReported && (isLocal || ver->id > VER_NDX_LOCAL)) {
+ if ((isLocal && globalAsteriskWildcardFound) ||
+ (!isLocal && localAsteriskWildcardFound)) {
+ warn("Wildcard pattern '*' is used for both 'local' and 'global' "
+ "scopes in version script");
+ asteriskWildcardReported = true;
+ } else if (!isLocal && globalAsteriskWildcardFound) {
+ warn("Wildcard pattern '*' is used for multiple version definitions in "
+ "version script");
+ asteriskWildcardReported = true;
+ } else {
+ localAsteriskWildcardFound = isLocal;
+ globalAsteriskWildcardFound = !isLocal;
+ }
+ }
+ assignWildcard(pat, isLocal ? VER_NDX_LOCAL : ver->id, ver->name);
+ };
for (VersionDefinition &v : llvm::reverse(config->versionDefinitions)) {
for (SymbolVersion &pat : v.nonLocalPatterns)
if (pat.hasWildcard && pat.name == "*")
- assignWildcard(pat, v.id, v.name);
+ assignAsteriskWildcard(pat, &v, false);
for (SymbolVersion &pat : v.localPatterns)
if (pat.hasWildcard && pat.name == "*")
- assignWildcard(pat, VER_NDX_LOCAL, v.name);
+ assignAsteriskWildcard(pat, &v, true);
}
// Symbol themselves might know their versions because symbols
diff --git a/lld/test/ELF/version-script-reassign-glob.s b/lld/test/ELF/version-script-reassign-glob.s
index 39d19a26fc4498..6470b67b5a1667 100644
--- a/lld/test/ELF/version-script-reassign-glob.s
+++ b/lld/test/ELF/version-script-reassign-glob.s
@@ -10,7 +10,8 @@
# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=BAR %s
# RUN: echo 'bar1 { *; }; bar2 { *; };' > %t2.ver
-# RUN: ld.lld --version-script %t2.ver %t.o -shared -o %t2.so --fatal-warnings
+# RUN: ld.lld --version-script %t2.ver %t.o -shared -o %t2.so 2>&1 | \
+# RUN: FileCheck --check-prefix=DUPWARN %s
# RUN: llvm-readelf --dyn-syms %t2.so | FileCheck --check-prefix=BAR2 %s
## If both a non-* glob and a * match, non-* wins.
@@ -21,6 +22,7 @@
## When there are multiple * patterns, the last wins.
# BAR2: GLOBAL DEFAULT 7 foo@@bar2
+# DUPWARN: warning: Wildcard pattern '*' is used for multiple version definitions in version script
.globl foo
foo:
diff --git a/lld/test/ELF/version-script-warn.s b/lld/test/ELF/version-script-warn.s
new file mode 100644
index 00000000000000..43ed97d04eeea3
--- /dev/null
+++ b/lld/test/ELF/version-script-warn.s
@@ -0,0 +1,35 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+
+# RUN: echo 'foo { *; }; bar { *; };' > %t.ver
+# RUN: ld.lld --version-script %t.ver %t.o -shared -o %t.so 2>&1 | \
+# RUN: FileCheck --check-prefix=MULTVER %s
+
+# RUN: echo '{ global: *; local: *;};' > %t.ver
+# RUN: ld.lld --version-script %t.ver %t.o -shared -o %t.so 2>&1 | \
+# RUN: FileCheck --check-prefix=LOCGLOB %s
+
+# RUN: echo 'V1 { global: *; }; V2 { local: *;};' > %t.ver
+# RUN: ld.lld --version-script %t.ver %t.o -shared -o %t.so 2>&1 | \
+# RUN: FileCheck --check-prefix=LOCGLOB %s
+
+# RUN: echo 'V1 { local: *; }; V2 { global: *;};' > %t.ver
+# RUN: ld.lld --version-script %t.ver %t.o -shared -o %t.so 2>&1 | \
+# RUN: FileCheck --check-prefix=LOCGLOB %s
+
+# RUN: echo 'V1 { local: *; }; V2 { local: *;};' > %t.ver
+# RUN: ld.lld --version-script %t.ver %t.o -shared -o %t.so --fatal-warnings
+
+## --retain-symbols-file uses the same internal infrastructure as the support
+## for version scripts. Do not show the warings if they both are used.
+# RUN: echo 'foo' > %t_retain.txt
+# RUN: echo '{ local: *; };' > %t_local.ver
+# RUN: echo '{ global: *; };' > %t_global.ver
+# RUN: ld.lld --retain-symbols-file=%t_retain.txt --version-script %t_local.ver %t.o -shared -o %t.so --fatal-warnings
+# RUN: ld.lld --retain-symbols-file=%t_retain.txt --version-script %t_global.ver %t.o -shared -o %t.so --fatal-warnings
+
+# MULTVER: warning: Wildcard pattern '*' is used for multiple version definitions in version script
+# LOCGLOB: warning: Wildcard pattern '*' is used for both 'local' and 'global' scopes in version script
+
+.globl foo
+foo:
>From 20f2710a2c5da4aeb9cc2998f38e0cc8546de420 Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Mon, 12 Aug 2024 21:05:12 -0700
Subject: [PATCH 2/3] Fix capitalization
---
lld/ELF/SymbolTable.cpp | 4 ++--
lld/test/ELF/version-script-reassign-glob.s | 2 +-
lld/test/ELF/version-script-warn.s | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index f7465cd5d097fc..3b94839fa92207 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -328,11 +328,11 @@ void SymbolTable::scanVersionScript() {
if (!asteriskWildcardReported && (isLocal || ver->id > VER_NDX_LOCAL)) {
if ((isLocal && globalAsteriskWildcardFound) ||
(!isLocal && localAsteriskWildcardFound)) {
- warn("Wildcard pattern '*' is used for both 'local' and 'global' "
+ warn("wildcard pattern '*' is used for both 'local' and 'global' "
"scopes in version script");
asteriskWildcardReported = true;
} else if (!isLocal && globalAsteriskWildcardFound) {
- warn("Wildcard pattern '*' is used for multiple version definitions in "
+ warn("wildcard pattern '*' is used for multiple version definitions in "
"version script");
asteriskWildcardReported = true;
} else {
diff --git a/lld/test/ELF/version-script-reassign-glob.s b/lld/test/ELF/version-script-reassign-glob.s
index 6470b67b5a1667..8de36467bd8ee6 100644
--- a/lld/test/ELF/version-script-reassign-glob.s
+++ b/lld/test/ELF/version-script-reassign-glob.s
@@ -22,7 +22,7 @@
## When there are multiple * patterns, the last wins.
# BAR2: GLOBAL DEFAULT 7 foo@@bar2
-# DUPWARN: warning: Wildcard pattern '*' is used for multiple version definitions in version script
+# DUPWARN: warning: wildcard pattern '*' is used for multiple version definitions in version script
.globl foo
foo:
diff --git a/lld/test/ELF/version-script-warn.s b/lld/test/ELF/version-script-warn.s
index 43ed97d04eeea3..9aba596165796b 100644
--- a/lld/test/ELF/version-script-warn.s
+++ b/lld/test/ELF/version-script-warn.s
@@ -28,8 +28,8 @@
# RUN: ld.lld --retain-symbols-file=%t_retain.txt --version-script %t_local.ver %t.o -shared -o %t.so --fatal-warnings
# RUN: ld.lld --retain-symbols-file=%t_retain.txt --version-script %t_global.ver %t.o -shared -o %t.so --fatal-warnings
-# MULTVER: warning: Wildcard pattern '*' is used for multiple version definitions in version script
-# LOCGLOB: warning: Wildcard pattern '*' is used for both 'local' and 'global' scopes in version script
+# MULTVER: warning: wildcard pattern '*' is used for multiple version definitions in version script
+# LOCGLOB: warning: wildcard pattern '*' is used for both 'local' and 'global' scopes in version script
.globl foo
foo:
>From bfd43c4b79366f0ab989f516a9200965fac5f66a Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Tue, 13 Aug 2024 12:54:28 -0700
Subject: [PATCH 3/3] AsteriskWildcard -> Asterisk, update the comment
---
lld/ELF/SymbolTable.cpp | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index 3b94839fa92207..7cc800fc664cbf 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -311,33 +311,32 @@ void SymbolTable::scanVersionScript() {
// Then, assign versions to "*". In GNU linkers they have lower priority than
// other wildcards.
- bool globalAsteriskWildcardFound = false;
- bool localAsteriskWildcardFound = false;
- bool asteriskWildcardReported = false;
- auto assignAsteriskWildcard = [&](SymbolVersion &pat, VersionDefinition *ver,
- bool isLocal) {
+ bool globalAsteriskFound = false;
+ bool localAsteriskFound = false;
+ bool asteriskReported = false;
+ auto assignAsterisk = [&](SymbolVersion &pat, VersionDefinition *ver,
+ bool isLocal) {
// Avoid issuing a warning if both '--retain-symbol-file' and a version
// script with `global: *` are used.
//
// '--retain-symbol-file' adds a "*" pattern to
// 'config->versionDefinitions[VER_NDX_LOCAL].nonLocalPatterns', see
- // 'readConfigs()' in 'Driver.cpp'. Note that it is '.nonLocalPatterns', not
- // '.localPatterns', which may seem counterintuitive, but still works as
- // expected. Here we can exploit that and skip analyzing the pattern added
- // for this option.
- if (!asteriskWildcardReported && (isLocal || ver->id > VER_NDX_LOCAL)) {
- if ((isLocal && globalAsteriskWildcardFound) ||
- (!isLocal && localAsteriskWildcardFound)) {
+ // 'readConfigs()' in 'Driver.cpp'. Note that it is not '.localPatterns',
+ // and may seem counterintuitive, but still works as expected. Here we can
+ // exploit that and skip analyzing the pattern added for this option.
+ if (!asteriskReported && (isLocal || ver->id > VER_NDX_LOCAL)) {
+ if ((isLocal && globalAsteriskFound) ||
+ (!isLocal && localAsteriskFound)) {
warn("wildcard pattern '*' is used for both 'local' and 'global' "
"scopes in version script");
- asteriskWildcardReported = true;
- } else if (!isLocal && globalAsteriskWildcardFound) {
+ asteriskReported = true;
+ } else if (!isLocal && globalAsteriskFound) {
warn("wildcard pattern '*' is used for multiple version definitions in "
"version script");
- asteriskWildcardReported = true;
+ asteriskReported = true;
} else {
- localAsteriskWildcardFound = isLocal;
- globalAsteriskWildcardFound = !isLocal;
+ localAsteriskFound = isLocal;
+ globalAsteriskFound = !isLocal;
}
}
assignWildcard(pat, isLocal ? VER_NDX_LOCAL : ver->id, ver->name);
@@ -345,10 +344,10 @@ void SymbolTable::scanVersionScript() {
for (VersionDefinition &v : llvm::reverse(config->versionDefinitions)) {
for (SymbolVersion &pat : v.nonLocalPatterns)
if (pat.hasWildcard && pat.name == "*")
- assignAsteriskWildcard(pat, &v, false);
+ assignAsterisk(pat, &v, false);
for (SymbolVersion &pat : v.localPatterns)
if (pat.hasWildcard && pat.name == "*")
- assignAsteriskWildcard(pat, &v, true);
+ assignAsterisk(pat, &v, true);
}
// Symbol themselves might know their versions because symbols
More information about the llvm-commits
mailing list