From e1ee3837acfea876b279f7f00474fdabd5d76cb5 Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@google.com>
Date: Thu, 11 Jul 2019 11:16:51 +0000
Subject: [PATCH] [ELF] Handle non-glob patterns before glob patterns in
 version scripts & fix a corner case of --dynamic-list

This fixes PR38549, which is silently accepted by ld.bfd.
This seems correct because it makes sense to let non-glob patterns take
precedence over glob patterns.

lld issues an error because
`assignWildcardVersion(ver, VER_NDX_LOCAL);` is processed before `assignExactVersion(ver, v.id, v.name);`.

Move all assignWildcardVersion() calls after assignExactVersion() calls
to fix this.

Also, move handleDynamicList() to the bottom. computeBinding() called by
includeInDynsym() has this cryptic rule:

    if (versionId == VER_NDX_LOCAL && isDefined() && !isPreemptible)
      return STB_LOCAL;

Before the change:

* foo's version is set to VER_NDX_LOCAL due to `local: *`
* handleDynamicList() is called
  - foo.computeBinding() is STB_LOCAL
  - foo.includeInDynsym() is false
  - foo.isPreemptible is not set (wrong)
* foo's version is set to V1

After the change:

* foo's version is set to VER_NDX_LOCAL due to `local: *`
* foo's version is set to V1
* handleDynamicList() is called
  - foo.computeBinding() is STB_GLOBAL
  - foo.includeInDynsym() is true
  - foo.isPreemptible is set (correct)

Reviewed By: ruiu

Differential Revision: https://reviews.llvm.org/D64550

llvm-svn: 365760
---
 lld/ELF/SymbolTable.cpp                | 40 ++++++++++++--------------
 lld/ELF/SymbolTable.h                  |  1 -
 lld/test/ELF/dynamic-list-preempt.s    |  7 ++++-
 lld/test/ELF/version-script-reassign.s |  5 ++++
 4 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index 252e1201c0c..fde08064a60 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -153,20 +153,6 @@ std::vector<Symbol *> SymbolTable::findAllByVersion(SymbolVersion ver) {
   return res;
 }
 
-// If there's only one anonymous version definition in a version
-// script file, the script does not actually define any symbol version,
-// but just specifies symbols visibilities.
-void SymbolTable::handleAnonymousVersion() {
-  for (SymbolVersion &ver : config->versionScriptGlobals)
-    assignExactVersion(ver, VER_NDX_GLOBAL, "global");
-  for (SymbolVersion &ver : config->versionScriptGlobals)
-    assignWildcardVersion(ver, VER_NDX_GLOBAL);
-  for (SymbolVersion &ver : config->versionScriptLocals)
-    assignExactVersion(ver, VER_NDX_LOCAL, "local");
-  for (SymbolVersion &ver : config->versionScriptLocals)
-    assignWildcardVersion(ver, VER_NDX_LOCAL);
-}
-
 // Handles -dynamic-list.
 void SymbolTable::handleDynamicList() {
   for (SymbolVersion &ver : config->dynamicList) {
@@ -241,23 +227,27 @@ void SymbolTable::assignWildcardVersion(SymbolVersion ver, uint16_t versionId) {
 
 // This function processes version scripts by updating VersionId
 // member of symbols.
+// If there's only one anonymous version definition in a version
+// script file, the script does not actually define any symbol version,
+// but just specifies symbols visibilities.
 void SymbolTable::scanVersionScript() {
-  // Handle edge cases first.
-  handleAnonymousVersion();
-  handleDynamicList();
-
-  // Now we have version definitions, so we need to set version ids to symbols.
-  // Each version definition has a glob pattern, and all symbols that match
-  // with the pattern get that version.
-
   // First, we assign versions to exact matching symbols,
   // i.e. version definitions not containing any glob meta-characters.
+  for (SymbolVersion &ver : config->versionScriptGlobals)
+    assignExactVersion(ver, VER_NDX_GLOBAL, "global");
+  for (SymbolVersion &ver : config->versionScriptLocals)
+    assignExactVersion(ver, VER_NDX_LOCAL, "local");
   for (VersionDefinition &v : config->versionDefinitions)
     for (SymbolVersion &ver : v.globals)
       assignExactVersion(ver, v.id, v.name);
 
   // Next, we assign versions to fuzzy matching symbols,
   // i.e. version definitions containing glob meta-characters.
+  for (SymbolVersion &ver : config->versionScriptGlobals)
+    assignWildcardVersion(ver, VER_NDX_GLOBAL);
+  for (SymbolVersion &ver : config->versionScriptLocals)
+    assignWildcardVersion(ver, VER_NDX_LOCAL);
+
   // Note that because the last match takes precedence over previous matches,
   // we iterate over the definitions in the reverse order.
   for (VersionDefinition &v : llvm::reverse(config->versionDefinitions))
@@ -269,4 +259,10 @@ void SymbolTable::scanVersionScript() {
   // Let them parse and update their names to exclude version suffix.
   for (Symbol *sym : symVector)
     sym->parseSymbolVersion();
+
+  // isPreemptible is false at this point. To correctly compute the binding of a
+  // Defined (which is used by includeInDynsym()), we need to know if it is
+  // VER_NDX_LOCAL or not. If defaultSymbolVersion is VER_NDX_LOCAL, we should
+  // compute symbol versions before handling --dynamic-list.
+  handleDynamicList();
 }
diff --git a/lld/ELF/SymbolTable.h b/lld/ELF/SymbolTable.h
index 26a5adac6e7..b64707f4ab0 100644
--- a/lld/ELF/SymbolTable.h
+++ b/lld/ELF/SymbolTable.h
@@ -64,7 +64,6 @@ private:
   std::vector<Symbol *> findAllByVersion(SymbolVersion ver);
 
   llvm::StringMap<std::vector<Symbol *>> &getDemangledSyms();
-  void handleAnonymousVersion();
   void assignExactVersion(SymbolVersion ver, uint16_t versionId,
                           StringRef versionName);
   void assignWildcardVersion(SymbolVersion ver, uint16_t versionId);
diff --git a/lld/test/ELF/dynamic-list-preempt.s b/lld/test/ELF/dynamic-list-preempt.s
index c19acc891da..024f9302a00 100644
--- a/lld/test/ELF/dynamic-list-preempt.s
+++ b/lld/test/ELF/dynamic-list-preempt.s
@@ -7,9 +7,14 @@
 # RUN: llvm-readobj -r %t.so | FileCheck --check-prefix=RELOCS %s
 # RUN: llvm-readobj --dyn-syms  %t.so | FileCheck --check-prefix=DYNSYMS %s
 
+# RUN: echo "V1 { global: foo; bar; local: *; };" > %t.vers
+# RUN: ld.lld --hash-style=sysv -fatal-warnings -dynamic-list %t.list -version-script %t.vers -shared %t.o -o %t.so
+# RUN: llvm-readobj -r %t.so | FileCheck --check-prefix=RELOCS %s
+# RUN: llvm-readobj --dyn-syms  %t.so | FileCheck --check-prefix=DYNSYMS %s
+
 # RELOCS:      Relocations [
 # RELOCS-NEXT:   Section ({{.*}}) .rela.plt {
-# RELOCS-NEXT:     R_X86_64_JUMP_SLOT foo 0x0
+# RELOCS-NEXT:     R_X86_64_JUMP_SLOT foo{{.*}} 0x0
 # RELOCS-NEXT:     R_X86_64_JUMP_SLOT ext 0x0
 # RELOCS-NEXT:   }
 # RELOCS-NEXT: ]
diff --git a/lld/test/ELF/version-script-reassign.s b/lld/test/ELF/version-script-reassign.s
index e2413295cc4..62b32cba82b 100644
--- a/lld/test/ELF/version-script-reassign.s
+++ b/lld/test/ELF/version-script-reassign.s
@@ -4,6 +4,7 @@
 # RUN: echo '{ global: foo; local: *; };' > %tg.ver
 # RUN: echo 'V1 { global: foo; };' > %t1.ver
 # RUN: echo 'V2 { global: foo; };' > %t2.ver
+# RUN: echo 'V2 { global: notexist; local: f*; };' > %t2w.ver
 
 ## Note, ld.bfd errors on the two cases.
 # RUN: ld.lld -shared %t.o --version-script %tl.ver --version-script %t1.ver \
@@ -18,6 +19,10 @@
 # RUN:   -o %t.so 2>&1 | FileCheck --check-prefix=V1-WARN %s
 # RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=V1-SYM %s
 
+# RUN: ld.lld -shared %t.o --version-script %t1.ver --version-script %t2w.ver \
+# RUN:   -o %t.so --fatal-warnings
+# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=V1-SYM %s
+
 # LOCAL: warning: attempt to reassign symbol 'foo' of VER_NDX_LOCAL to version 'V1'
 # LOCAL-SYM-NOT: foo
 
-- 
2.22.0.657.g960e92d24f-goog