[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

1807 ↗

(On Diff #77184)

Use setError.

1810 ↗

(On Diff #77184)

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

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

ELF/SymbolTable.cpp

580–581 ↗

(On Diff #77184)

Do you actually need this guard?

697 ↗

(On Diff #77184)

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

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

1807 ↗

(On Diff #77184)

Done.

1810 ↗

(On Diff #77184)

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

ELF/SymbolTable.cpp

580–581 ↗

(On Diff #77184)

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.

697 ↗

(On Diff #77184)

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

725 ↗

(On Diff #77313)

Why symbol version number is of size_t?

580–581 ↗

(On Diff #77184)

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

[grimar]

• Addressed review comments.

ELF/SymbolTable.cpp

725 ↗

(On Diff #77313)

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});

580–581 ↗

(On Diff #77184)

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

[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

lld/trunk/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 {

lld/trunk/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)

lld/trunk/ELF/SymbolTable.cpp

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>;

lld/trunk/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