[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

1801

Use setError.

1804

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

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

ELF/SymbolTable.cpp

592–593

Do you actually need this guard?

707

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

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

1801

Done.

1804

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

ELF/SymbolTable.cpp

592–593

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.

707

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

592–593

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

715

Why symbol version number is of size_t?

[grimar]

• Addressed review comments.

ELF/SymbolTable.cpp

592–593

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

715

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

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

static void setVersionId(SymbolBody *Body, StringRef VersionName,
   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)
     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 *>>
for (SymbolVersion Sym : V.Globals) {
       StringRef N = Sym.Name;
       if (Sym.IsExternCpp) {
         for (SymbolBody *B : findDemangled(Demangled, N))
           setVersionId(B, V.Name, N, V.Id);
         continue;
       }
       setVersionId(find(N), V.Name, N, V.Id);
     }
+    for (SymbolVersion Sym : V.Locals) {
+      if (Sym.HasWildcards)
+        continue;
+      setVersionId(find(Sym.Name), V.Name, Sym.Name, VER_NDX_LOCAL);
+    }
   }
 
   // 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;
+        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