[lld][elf] Warn if '*' pattern is used multiple times in version scripts

[igorkudrin]

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.

[llvmbot]

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Igor Kudrin (igorkudrin)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/102669.diff

3 Files Affected:

• (modified) lld/ELF/SymbolTable.cpp (+33-2)
• (modified) lld/test/ELF/version-script-reassign-glob.s (+3-1)
• (added) lld/test/ELF/version-script-warn.s (+35)

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:

[MaskRay]

We don't capitalize warn/error messages

[igorkudrin]

Thanks! Fixed.

[MaskRay]

These variables might look mouthful as asterisk is a wildcard character.
Perhaps AsteriskWildcard => Asterisk

[igorkudrin]

Done

[igorkudrin]

Ping

[igorkudrin]

Ping

[igorkudrin]

Ping

[MaskRay]

Sorry. This is early in my /pulls/review-requested queue and I haven't noticed it...

[igorkudrin]

Thanks!