[ELF] Handle non-glob patterns before glob patterns in version scripts & fix a corner case of --dynamic-list

[MaskRay]

Summary

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)

[ruiu]

LGTM

[MaskRay]

Fix the markdown bullet points in the description

lld/trunk/ELF/SymbolTable.h

public:
   // is used to uniquify them.
   llvm::DenseMap<llvm::CachedHashStringRef, const InputFile *> comdatGroups;
 
 private:
   std::vector<Symbol *> findByVersion(SymbolVersion ver);
   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);
 
   // The order the global symbols are in is not defined. We can use an arbitrary
   // order, but it has to be reproducible. That is true even when cross linking.
   // The default hashing of StringRef produces different results on 32 and 64
   // bit systems so we use a map to a vector. That is arbitrary, deterministic

lld/trunk/ELF/SymbolTable.cpp

std::vector<Symbol *> SymbolTable::findAllByVersion(SymbolVersion ver) {
   }
 
   for (Symbol *sym : symVector)
     if ((sym->isDefined() || sym->isCommon()) && m.match(sym->getName()))
       res.push_back(sym);
   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) {
     std::vector<Symbol *> syms;
     if (ver.hasWildcard)
       syms = findAllByVersion(ver);
     else
       syms = findByVersion(ver);
void SymbolTable::assignWildcardVersion(SymbolVersion ver, uint16_t versionId) {
   // to the symbol. This behavior is compatible with GNU.
   for (Symbol *b : findAllByVersion(ver))
     if (b->versionId == config->defaultSymbolVersion)
       b->versionId = 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))
     for (SymbolVersion &ver : v.globals)
       assignWildcardVersion(ver, v.id);
 
   // Symbol themselves might know their versions because symbols
   // can contain versions in the form of <name>@<version>.
   // 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();
 }

lld/trunk/test/ELF/dynamic-list-preempt.s

 # REQUIRES: x86
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: echo "{ foo; zed; };" > %t.list
 # RUN: echo "{ 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
 
+# 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: ]
 
 # DYNSYMS:      DynamicSymbols [
 # DYNSYMS-NEXT:   Symbol {
 # DYNSYMS-NEXT:     Name:
 # DYNSYMS-NEXT:     Value: 0x0

lld/trunk/test/ELF/version-script-reassign.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
 # RUN: echo '{ local: foo; };' > %tl.ver
 # 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 \
 # RUN:   -o %t.so 2>&1 | FileCheck --check-prefix=LOCAL %s
 # RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=LOCAL-SYM %s
 # RUN: ld.lld -shared %t.o --version-script %tg.ver --version-script %t1.ver \
 # RUN:   -o %t.so 2>&1 | FileCheck --check-prefix=GLOBAL %s
 # RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=GLOBAL-SYM %s
 
 ## Note, ld.bfd silently accepts this case.
 # RUN: ld.lld -shared %t.o --version-script %t1.ver --version-script %t2.ver \
 # 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
 
 # GLOBAL: warning: attempt to reassign symbol 'foo' of VER_NDX_GLOBAL to version 'V1'
 # GLOBAL-SYM: foo{{$}}
 
 # V1-WARN: warning: attempt to reassign symbol 'foo' of version 'V1' to version 'V2'
 # V1-SYM: foo@@V1
 
 .globl foo
 foo: