[ELF] - Add support for locals list in version script.

[grimar]

Summary

Found this when tried to build FreeBSD port jpeg-turbo-1.5.1 which fails to link because of next script:

LIBJPEGTURBO_8.0 {
  
  local:
    jsimd_*;
    jconst_*;
};

LIBJPEG_8.0 {
  global:
    *;
};

Previously we did not support anything except "local: *", patch changes that.

Actually GNU rules of proccessing wildcards are more complex than that (http://www.airs.com/blog/archives/300):
There are 2 iteration for wildcards, at first iteration "*" wildcards are ignored and handled at second iteration.

Since we previously decided not to implement such complex rules,
I suggest solution that is implemented in this patch. So for "local: *" case nothing changes, but if we have wildcarded locals,
they are processed before globals. That adds a chance for such scripts to work.

[ruiu]

ELF/LinkerScript.cpp

1803

Use setError.

1806

Please take a look at other places. We usually write a loop this way to consume tokens until "}".

while (!Error && !consume("}")) {
  ...
}

ELF/SymbolTable.cpp

600–602

Do you actually need this guard?

717

You want to care about the case in which IsExternCpp is true.

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

1803

Done.

1806

I did :) readLocal was written similar to readExtern.
Changed code to use while + peek, consume will not work here.

ELF/SymbolTable.cpp

600–602

I think so. We want to show this warning if assigning version to symbol that is already has one,
(and switch to error one day https://llvm.org/bugs/show_bug.cgi?id=28342)
also we want to show warning if symbol is both in local and global list as it is obvious error.
Actually I even thing we want to error out in latter case, that is how gold do afaik, planned to suggest that in a separate patch to keep this bit simpler for review.

717

Yes, but currently IsExternCpp is never set to true for locals.
Scripts with externs in locals will fail to parse, added testcase to demonstrate.
I suggest to implement them in a separate patch.
(as well as support for locals in anonymous versions declarations).

[ruiu]

ELF/SymbolTable.cpp

600–602

I mean, why the previous condition Sym->VersionId != Config->DefaultSymbolVersion wouldn't work?

725

Why symbol version number is of size_t?

[grimar]

• Addressed review comments.

ELF/SymbolTable.cpp

600–602

Ah, right. It is enough, no need in additional set at all.

725

Because we pass VersionDefinition::Id, which is size_t,
because we use next code:

size_t VersionId = Config->VersionDefinitions.size() + 2;
Config->VersionDefinitions.push_back({VerStr, VersionId});

[grimar]

Few numbers: this patch should fix several FreeBSD ports linkage fails, one of them is graphics/jpeg-turbo which blocks 5014 packages.

[ruiu]

LGTM

ELF/Config.h

 
 // This struct contains symbols version definition that
 // can be found in version script if it is used for link.
 struct VersionDefinition {
   VersionDefinition(llvm::StringRef Name, size_t Id) : Name(Name), Id(Id) {}
   llvm::StringRef Name;
   size_t Id;
   std::vector<SymbolVersion> Globals;
+  std::vector<SymbolVersion> Locals;
   size_t NameOff; // Offset in string table.
 };
 
 // This struct contains the global configuration for the linker.
 // Most fields are direct mapping from the command line options
 // and such fields have the same name as the corresponding options.
 // Most fields are initialized by the driver.
 struct Configuration {

ELF/LinkerScript.cpp

void LinkerScript<ELFT>::assignAddresses(std::vector<PhdrEntry<ELFT>> &Phdrs) {
 
   uintX_t HeaderSize = getHeaderSize();
   auto FirstPTLoad =
       std::find_if(Phdrs.begin(), Phdrs.end(), [](const PhdrEntry<ELFT> &E) {
         return E.H.p_type == PT_LOAD;
       });
 
   if (HeaderSize <= MinVA && FirstPTLoad != Phdrs.end()) {
-    // If linker script specifies program headers and first PT_LOAD doesn't 
+    // If linker script specifies program headers and first PT_LOAD doesn't
     // have both PHDRS and FILEHDR attributes then do nothing
     if (!Opt.PhdrsCommands.empty()) {
       size_t SegNum = std::distance(Phdrs.begin(), FirstPTLoad);
       if (!Opt.PhdrsCommands[SegNum].HasPhdrs ||
           !Opt.PhdrsCommands[SegNum].HasFilehdr)
         return;
     }
     // ELF and Program headers need to be right before the first section in
private:
   Expr readPrimary();
   Expr readTernary(Expr Cond);
   Expr readParenExpr();
 
   // For parsing version script.
   void readExtern(std::vector<SymbolVersion> *Globals);
   void readVersionDeclaration(StringRef VerStr);
   void readGlobal(StringRef VerStr);
-  void readLocal();
+  void readLocal(StringRef VerStr);
 
   ScriptConfiguration &Opt = *ScriptConfig;
   bool IsUnderSysroot;
 };
 
 void ScriptParser::readVersionScript() {
   readVersionScriptCommand();
   if (!atEOF())
void ScriptParser::readVersionDeclaration(StringRef VerStr) {
   // Identifiers start at 2 because 0 and 1 are reserved
   // for VER_NDX_LOCAL and VER_NDX_GLOBAL constants.
   size_t VersionId = Config->VersionDefinitions.size() + 2;
   Config->VersionDefinitions.push_back({VerStr, VersionId});
 
   if (consume("global:") || peek() != "local:")
     readGlobal(VerStr);
   if (consume("local:"))
-    readLocal();
+    readLocal(VerStr);
   expect("}");
 
   // Each version may have a parent version. For example, "Ver2" defined as
   // "Ver2 { global: foo; local: *; } Ver1;" has "Ver1" as a parent. This
   // version hierarchy is, probably against your instinct, purely for human; the
   // runtime doesn't care about them at all. In LLD, we simply skip the token.
   if (!VerStr.empty() && peek() != ";")
     skip();
   expect(";");
 }
 
-void ScriptParser::readLocal() {
+void ScriptParser::readLocal(StringRef VerStr) {
+  if (consume("*")) {
     Config->DefaultSymbolVersion = VER_NDX_LOCAL;
-  expect("*");
     expect(";");
+    return;
+  }
+
+  if (VerStr.empty())
+    setError("locals list for anonymous version is not supported");
+
+  std::vector<SymbolVersion> &Locals = Config->VersionDefinitions.back().Locals;
+  while (!Error && peek() != "}") {
+    StringRef Tok = next();
+    Locals.push_back({unquote(Tok), false, hasWildcard(Tok)});
+    expect(";");
+  }
 }
 
 void ScriptParser::readExtern(std::vector<SymbolVersion> *Globals) {
   expect("\"C++\"");
   expect("{");
 
   for (;;) {
     if (peek() == "}" || Error)

ELF/SymbolTable.cpp

 // This function processes --export-dynamic-symbol and --dynamic-list.
 template <class ELFT> void SymbolTable<ELFT>::scanDynamicList() {
   for (StringRef S : Config->DynamicList)
     if (SymbolBody *B = find(S))
       B->symbol()->ExportDynamic = true;
 }
 
 static void setVersionId(SymbolBody *Body, StringRef VersionName,
-                         StringRef Name, uint16_t Version) {
+                         StringRef Name, uint16_t Version,
+                         DenseSet<SymbolBody *> &Proccessed) {
   if (!Body || Body->isUndefined()) {
     if (Config->NoUndefinedVersion)
       error("version script assignment of " + VersionName + " to symbol " +
             Name + " failed: symbol not defined");
     return;
   }
 
   Symbol *Sym = Body->symbol();
-  if (Sym->VersionId != Config->DefaultSymbolVersion)
+  auto It = Proccessed.insert(Body);
+  if (!It.second)
     warn("duplicate symbol " + Name + " in version script");
   Sym->VersionId = Version;
 }
 
 // Returns a map from demangled symbols to symbol objects.
 // The relationship is 1:N instead of 1:1 because with the symbol
 // versioning, more than one symbol may have the same name.
 template <class ELFT>
 std::map<std::string, std::vector<SymbolBody *>>
if (M.match(P.first))
       for (SymbolBody *Body : P.second)
         if (!Body->isUndefined())
           Res.push_back(Body);
   }
   return Res;
 }
 
 // If there's only one anonymous version definition in a version
-// script file, the script does not actullay define any symbol version,
+// script file, the script does not actually define any symbol version,
 // but just specifies symbols visibilities. We assume that the script was
 // in the form of { global: foo; bar; local *; }. So, local is default.
 // In this function, we make specified symbols global.
 template <class ELFT> void SymbolTable<ELFT>::handleAnonymousVersion() {
   std::vector<StringRef> Patterns;
   for (SymbolVersion &Sym : Config->VersionScriptGlobals) {
     if (hasWildcard(Sym.Name)) {
       Patterns.push_back(Sym.Name);
template <class ELFT> void SymbolTable<ELFT>::scanVersionScript() {
   // C++ symbols. For example, you can write a pattern such as
   // "llvm::*::foo(int, ?)". Obviously, there's no way to handle this
   // other than trying to match a regexp against all demangled symbols.
   // So, if "extern C++" feature is used, we demangle all known symbols.
   std::map<std::string, std::vector<SymbolBody *>> Demangled;
   if (hasExternCpp())
     Demangled = getDemangledSyms();
 
+  DenseSet<SymbolBody *> Proccessed;
   // First, we assign versions to exact matching symbols,
   // i.e. version definitions not containing any glob meta-characters.
   for (VersionDefinition &V : Config->VersionDefinitions) {
     for (SymbolVersion Sym : V.Globals) {
       if (Sym.HasWildcards)
         continue;
 
       StringRef N = Sym.Name;
       if (Sym.IsExternCpp) {
         for (SymbolBody *B : findDemangled(Demangled, N))
-          setVersionId(B, V.Name, N, V.Id);
+          setVersionId(B, V.Name, N, V.Id, Proccessed);
         continue;
       }
-      setVersionId(find(N), V.Name, N, V.Id);
+      setVersionId(find(N), V.Name, N, V.Id, Proccessed);
+    }
+    for (SymbolVersion Sym : V.Locals) {
+      if (Sym.HasWildcards)
+        continue;
+      setVersionId(find(Sym.Name), V.Name, Sym.Name, VER_NDX_LOCAL, Proccessed);
     }
   }
 
   // Next, we assign versions to fuzzy matching symbols,
   // i.e. version definitions containing glob meta-characters.
   // Note that because the last match takes precedence over previous matches,
   // we iterate over the definitions in the reverse order.
-  for (size_t I = Config->VersionDefinitions.size() - 1; I != (size_t)-1; --I) {
-    VersionDefinition &V = Config->VersionDefinitions[I];
-    for (SymbolVersion &Sym : V.Globals) {
-      if (!Sym.HasWildcards)
-        continue;
-      StringMatcher M({Sym.Name});
-      std::vector<SymbolBody *> Syms =
-          Sym.IsExternCpp ? findAllDemangled(Demangled, M) : findAll(M);
-
-      // Exact matching takes precendence 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 (SymbolBody *B : Syms)
-        if (B->symbol()->VersionId == Config->DefaultSymbolVersion)
-          B->symbol()->VersionId = V.Id;
+  auto assignFuzzyVersion = [&](SymbolVersion &Sym, size_t Version) {
+      return;
+    for (SymbolBody *B : Syms) {
+      auto It = Proccessed.insert(B);
+      if (It.second)
+        B->symbol()->VersionId = Version;
     }
+  };
+
+  for (size_t I = Config->VersionDefinitions.size() - 1; I != (size_t)-1; --I) {
+    VersionDefinition &V = Config->VersionDefinitions[I];
+    for (SymbolVersion &Sym : V.Locals)
+      assignFuzzyVersion(Sym, VER_NDX_LOCAL);
+    for (SymbolVersion &Sym : V.Globals)
+      assignFuzzyVersion(Sym, V.Id);
   }
 }
 
 template class elf::SymbolTable<ELF32LE>;
 template class elf::SymbolTable<ELF32BE>;
 template class elf::SymbolTable<ELF64LE>;
 template class elf::SymbolTable<ELF64BE>;

test/ELF/version-script-locals.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+
+# RUN: echo "VERSION_1.0 { local: foo1; };" > %t.script
+# RUN: ld.lld --version-script %t.script -shared %t.o -o %t.so
+# RUN: llvm-readobj -dyn-symbols %t.so | FileCheck --check-prefix=EXACT %s
+# EXACT:  DynamicSymbols [
+# EXACT:      _start
+# EXACT-NOT:  foo1
+# EXACT:      foo2
+# EXACT:      foo3
+
+# RUN: echo "VERSION_1.0 { local: foo*; };" > %t.script
+# RUN: ld.lld --version-script %t.script -shared %t.o -o %t.so
+# RUN: llvm-readobj -dyn-symbols %t.so | FileCheck --check-prefix=WC %s
+# WC:  DynamicSymbols [
+# WC:      _start
+# WC-NOT:  foo1
+# WC-NOT:  foo2
+# WC-NOT:  foo3
+
+# RUN: echo "VERSION_1.0 { global: *; local: foo*; };" > %t.script
+# RUN: ld.lld --version-script %t.script -shared %t.o -o %t.so
+# RUN: llvm-readobj -dyn-symbols %t.so | FileCheck --check-prefix=MIX %s
+# MIX:  DynamicSymbols [
+# MIX:      _start@@VERSION_1.0
+# MIX-NOT:  foo1
+# MIX-NOT:  foo2
+# MIX-NOT:  foo3
+
+# RUN: echo "VERSION_1.0 { global: *; local: extern \"C++\" { foo*; } };" > %t.script
+# RUN: not ld.lld --version-script %t.script -shared %t.o -o %t.so 2>&1 \
+# RUN:   | FileCheck --check-prefix=EXTERNERR %s
+# EXTERNERR: ; expected, but got "C++"
+
+.globl foo1
+foo1:
+  ret
+
+.globl foo2
+foo2:
+  ret
+
+.globl foo3
+foo3:
+  ret
+
+.globl _start
+_start:
+  ret