Revert "[ELF] Apply version script patterns to non-default version symbols"

This reverts commit 7ed22a6.

buf is not cleared so the commit misses some cases.

Author: MaskRay

lld/ELF/Config.h

@@ -86,8 +86,7 @@ struct SymbolVersion {
 struct VersionDefinition {
   llvm::StringRef name;
   uint16_t id;
-  std::vector<SymbolVersion> nonLocalPatterns;
-  std::vector<SymbolVersion> localPatterns;
+  std::vector<SymbolVersion> patterns;
 };
 
 // This struct contains the global configuration for the linker.

lld/ELF/Driver.cpp

@@ -1351,19 +1351,18 @@ static void readConfigs(opt::InputArgList &args) {
   }
 
   assert(config->versionDefinitions.empty());
+  config->versionDefinitions.push_back({"local", (uint16_t)VER_NDX_LOCAL, {}});
   config->versionDefinitions.push_back(
-      {"local", (uint16_t)VER_NDX_LOCAL, {}, {}});
-  config->versionDefinitions.push_back(
-      {"global", (uint16_t)VER_NDX_GLOBAL, {}, {}});
+      {"global", (uint16_t)VER_NDX_GLOBAL, {}});
 
   // If --retain-symbol-file is used, we'll keep only the symbols listed in
   // the file and discard all others.
   if (auto *arg = args.getLastArg(OPT_retain_symbols_file)) {
-    config->versionDefinitions[VER_NDX_LOCAL].nonLocalPatterns.push_back(
+    config->versionDefinitions[VER_NDX_LOCAL].patterns.push_back(
         {"*", /*isExternCpp=*/false, /*hasWildcard=*/true});
     if (Optional<MemoryBufferRef> buffer = readFile(arg->getValue()))
       for (StringRef s : args::getLines(*buffer))
-        config->versionDefinitions[VER_NDX_GLOBAL].nonLocalPatterns.push_back(
+        config->versionDefinitions[VER_NDX_GLOBAL].patterns.push_back(
             {s, /*isExternCpp=*/false, /*hasWildcard=*/false});
   }
 

lld/ELF/ScriptParser.cpp

@@ -1496,9 +1496,9 @@ void ScriptParser::readAnonymousDeclaration() {
   std::vector<SymbolVersion> globals;
   std::tie(locals, globals) = readSymbols();
   for (const SymbolVersion &pat : locals)
-    config->versionDefinitions[VER_NDX_LOCAL].localPatterns.push_back(pat);
+    config->versionDefinitions[VER_NDX_LOCAL].patterns.push_back(pat);
   for (const SymbolVersion &pat : globals)
-    config->versionDefinitions[VER_NDX_GLOBAL].nonLocalPatterns.push_back(pat);
+    config->versionDefinitions[VER_NDX_GLOBAL].patterns.push_back(pat);
 
   expect(";");
 }
@@ -1510,12 +1510,13 @@ void ScriptParser::readVersionDeclaration(StringRef verStr) {
   std::vector<SymbolVersion> locals;
   std::vector<SymbolVersion> globals;
   std::tie(locals, globals) = readSymbols();
+  for (const SymbolVersion &pat : locals)
+    config->versionDefinitions[VER_NDX_LOCAL].patterns.push_back(pat);
 
   // Create a new version definition and add that to the global symbols.
   VersionDefinition ver;
   ver.name = verStr;
-  ver.nonLocalPatterns = std::move(globals);
-  ver.localPatterns = std::move(locals);
+  ver.patterns = globals;
   ver.id = config->versionDefinitions.size();
   config->versionDefinitions.push_back(ver);
 

lld/ELF/SymbolTable.cpp

@@ -150,24 +150,19 @@ std::vector<Symbol *> SymbolTable::findByVersion(SymbolVersion ver) {
   return {};
 }
 
-std::vector<Symbol *> SymbolTable::findAllByVersion(SymbolVersion ver,
-                                                    bool includeNonDefault) {
+std::vector<Symbol *> SymbolTable::findAllByVersion(SymbolVersion ver) {
   std::vector<Symbol *> res;
   SingleStringMatcher m(ver.name);
 
   if (ver.isExternCpp) {
     for (auto &p : getDemangledSyms())
       if (m.match(p.first()))
-        for (Symbol *sym : p.second)
-          if (includeNonDefault || !sym->getName().contains('@'))
-            res.push_back(sym);
+        res.insert(res.end(), p.second.begin(), p.second.end());
     return res;
   }
 
   for (Symbol *sym : symVector)
-    if (canBeVersioned(*sym) &&
-        (includeNonDefault || !sym->getName().contains('@')) &&
-        m.match(sym->getName()))
+    if (canBeVersioned(*sym) && m.match(sym->getName()))
       res.push_back(sym);
   return res;
 }
@@ -177,7 +172,7 @@ void SymbolTable::handleDynamicList() {
   for (SymbolVersion &ver : config->dynamicList) {
     std::vector<Symbol *> syms;
     if (ver.hasWildcard)
-      syms = findAllByVersion(ver, /*includeNonDefault=*/true);
+      syms = findAllByVersion(ver);
     else
       syms = findByVersion(ver);
 
@@ -186,12 +181,21 @@ void SymbolTable::handleDynamicList() {
   }
 }
 
-// Set symbol versions to symbols. This function handles patterns containing no
-// wildcard characters. Return false if no symbol definition matches ver.
-bool SymbolTable::assignExactVersion(SymbolVersion ver, uint16_t versionId,
+// Set symbol versions to symbols. This function handles patterns
+// containing no wildcard characters.
+void SymbolTable::assignExactVersion(SymbolVersion ver, uint16_t versionId,
                                      StringRef versionName) {
+  if (ver.hasWildcard)
+    return;
+
   // Get a list of symbols which we need to assign the version to.
   std::vector<Symbol *> syms = findByVersion(ver);
+  if (syms.empty()) {
+    if (!config->undefinedVersion)
+      error("version script assignment of '" + versionName + "' to symbol '" +
+            ver.name + "' failed: symbol not defined");
+    return;
+  }
 
   auto getName = [](uint16_t ver) -> std::string {
     if (ver == VER_NDX_LOCAL)
@@ -203,10 +207,10 @@ bool SymbolTable::assignExactVersion(SymbolVersion ver, uint16_t versionId,
 
   // Assign the version.
   for (Symbol *sym : syms) {
-    // For a non-local versionId, skip symbols containing version info because
-    // symbol versions specified by symbol names take precedence over version
-    // scripts. See parseSymbolVersion().
-    if (versionId != VER_NDX_LOCAL && sym->getName().contains('@'))
+    // Skip symbols containing version info because symbol versions
+    // specified by symbol names take precedence over version scripts.
+    // See parseSymbolVersion().
+    if (sym->getName().contains('@'))
       continue;
 
     // If the version has not been assigned, verdefIndex is -1. Use an arbitrary
@@ -221,15 +225,13 @@ bool SymbolTable::assignExactVersion(SymbolVersion ver, uint16_t versionId,
     warn("attempt to reassign symbol '" + ver.name + "' of " +
          getName(sym->versionId) + " to " + getName(versionId));
   }
-  return !syms.empty();
 }
 
-void SymbolTable::assignWildcardVersion(SymbolVersion ver, uint16_t versionId,
-                                        bool includeNonDefault) {
+void SymbolTable::assignWildcardVersion(SymbolVersion ver, uint16_t versionId) {
   // Exact matching takes precedence over fuzzy matching,
   // so we set a version to a symbol only if no version has been assigned
   // to the symbol. This behavior is compatible with GNU.
-  for (Symbol *sym : findAllByVersion(ver, includeNonDefault))
+  for (Symbol *sym : findAllByVersion(ver))
     if (sym->verdefIndex == UINT32_C(-1)) {
       sym->verdefIndex = 0;
       sym->versionId = versionId;
@@ -242,57 +244,26 @@ void SymbolTable::assignWildcardVersion(SymbolVersion ver, uint16_t versionId,
 // script file, the script does not actually define any symbol version,
 // but just specifies symbols visibilities.
 void SymbolTable::scanVersionScript() {
-  SmallString<128> buf;
   // First, we assign versions to exact matching symbols,
   // i.e. version definitions not containing any glob meta-characters.
-  std::vector<Symbol *> syms;
-  for (VersionDefinition &v : config->versionDefinitions) {
-    auto assignExact = [&](SymbolVersion pat, uint16_t id, StringRef ver) {
-      bool found = assignExactVersion(pat, id, ver);
-      found |= assignExactVersion({(pat.name + "@" + v.name).toStringRef(buf),
-                                   pat.isExternCpp, /*hasWildCard=*/false},
-                                  id, ver);
-      if (!found && !config->undefinedVersion)
-        errorOrWarn("version script assignment of '" + ver + "' to symbol '" +
-                    pat.name + "' failed: symbol not defined");
-    };
-    for (SymbolVersion &pat : v.nonLocalPatterns)
-      if (!pat.hasWildcard)
-        assignExact(pat, v.id, v.name);
-    for (SymbolVersion pat : v.localPatterns)
-      if (!pat.hasWildcard)
-        assignExact(pat, VER_NDX_LOCAL, "local");
-  }
+  for (VersionDefinition &v : config->versionDefinitions)
+    for (SymbolVersion &pat : v.patterns)
+      assignExactVersion(pat, v.id, v.name);
 
   // Next, assign versions to wildcards that are not "*". Note that because the
   // last match takes precedence over previous matches, we iterate over the
   // definitions in the reverse order.
-  auto assignWildcard = [&](SymbolVersion pat, uint16_t id, StringRef ver) {
-    assignWildcardVersion(pat, id, /*includeNonDefault=*/false);
-    assignWildcardVersion({(pat.name + "@" + ver).toStringRef(buf),
-                           pat.isExternCpp, /*hasWildCard=*/true},
-                          id,
-                          /*includeNonDefault=*/true);
-  };
-  for (VersionDefinition &v : llvm::reverse(config->versionDefinitions)) {
-    for (SymbolVersion &pat : v.nonLocalPatterns)
+  for (VersionDefinition &v : llvm::reverse(config->versionDefinitions))
+    for (SymbolVersion &pat : v.patterns)
       if (pat.hasWildcard && pat.name != "*")
-        assignWildcard(pat, v.id, v.name);
-    for (SymbolVersion &pat : v.localPatterns)
-      if (pat.hasWildcard && pat.name != "*")
-        assignWildcard(pat, VER_NDX_LOCAL, v.name);
-  }
+        assignWildcardVersion(pat, v.id);
 
   // Then, assign versions to "*". In GNU linkers they have lower priority than
   // other wildcards.
-  for (VersionDefinition &v : config->versionDefinitions) {
-    for (SymbolVersion &pat : v.nonLocalPatterns)
+  for (VersionDefinition &v : config->versionDefinitions)
+    for (SymbolVersion &pat : v.patterns)
       if (pat.hasWildcard && pat.name == "*")
-        assignWildcard(pat, v.id, v.name);
-    for (SymbolVersion &pat : v.localPatterns)
-      if (pat.hasWildcard && pat.name == "*")
-        assignWildcard(pat, VER_NDX_LOCAL, v.name);
-  }
+        assignWildcardVersion(pat, v.id);
 
   // Symbol themselves might know their versions because symbols
   // can contain versions in the form of <name>@<version>.

lld/ELF/SymbolTable.h

@@ -65,14 +65,12 @@ class SymbolTable {
 
 private:
   std::vector<Symbol *> findByVersion(SymbolVersion ver);
-  std::vector<Symbol *> findAllByVersion(SymbolVersion ver,
-                                         bool includeNonDefault);
+  std::vector<Symbol *> findAllByVersion(SymbolVersion ver);
 
   llvm::StringMap<std::vector<Symbol *>> &getDemangledSyms();
-  bool assignExactVersion(SymbolVersion ver, uint16_t versionId,
+  void assignExactVersion(SymbolVersion ver, uint16_t versionId,
                           StringRef versionName);
-  void assignWildcardVersion(SymbolVersion ver, uint16_t versionId,
-                             bool includeNonDefault);
+  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.

lld/ELF/Symbols.cpp

@@ -208,9 +208,6 @@ OutputSection *Symbol::getOutputSection() const {
 // If a symbol name contains '@', the characters after that is
 // a symbol version name. This function parses that.
 void Symbol::parseSymbolVersion() {
-  // Return if localized by a local: pattern in a version script.
-  if (versionId == VER_NDX_LOCAL)
-    return;
   StringRef s = getName();
   size_t pos = s.find('@');
   if (pos == 0 || pos == StringRef::npos)

lld/test/ELF/version-script-extern-exact.s

@@ -0,0 +1,30 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "FOO { global: extern \"C++\" { \"aaa*\"; }; };" > %t.script
+# RUN: ld.lld --version-script %t.script -shared %t.o -o %t.so
+# RUN: llvm-readobj --dyn-syms %t.so | FileCheck %s --check-prefix=NOMATCH
+
+# NOMATCH:     DynamicSymbols [
+# NOMATCH-NOT:   _Z3aaaPi@@FOO
+# NOMATCH-NOT:   _Z3aaaPf@@FOO
+# NOMATCH:     ]
+
+# RUN: echo "FOO { global: extern \"C++\" { \"aaa*\"; aaa*; }; };" > %t2.script
+# RUN: ld.lld --version-script %t2.script -shared %t.o -o %t2.so
+# RUN: llvm-readobj --dyn-syms %t2.so | FileCheck %s --check-prefix=MATCH
+# MATCH:   DynamicSymbols [
+# MATCH:     _Z3aaaPi@@FOO
+# MATCH:     _Z3aaaPf@@FOO
+# MATCH:   ]
+
+.text
+.globl _Z3aaaPi
+.type _Z3aaaPi,@function
+_Z3aaaPi:
+retq
+
+.globl _Z3aaaPf
+.type _Z3aaaPf,@function
+_Z3aaaPf:
+retq

lld/test/ELF/version-script-extern-wildcards.s

@@ -0,0 +1,28 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "FOO { global: extern \"C++\" { foo*; }; };" > %t.script
+# RUN: echo "BAR { global: extern \"C++\" { zed*; bar; }; };" >> %t.script
+# RUN: ld.lld --version-script %t.script -shared %t.o -o %t.so
+# RUN: llvm-readobj -V --dyn-syms %t.so | FileCheck %s
+
+# CHECK:  VersionSymbols [
+# CHECK:    Name: _Z3fooi@@FOO
+# CHECK:    Name: _Z3bari
+# CHECK:    Name: _Z3zedi@@BAR
+
+.text
+.globl _Z3fooi
+.type _Z3fooi,@function
+_Z3fooi:
+retq
+
+.globl _Z3bari
+.type _Z3bari,@function
+_Z3bari:
+retq
+
+.globl _Z3zedi
+.type _Z3zedi,@function
+_Z3zedi:
+retq