[ELF] - Implemented --no-undefined-version flag

[grimar]

Summary

The GNU ld option --no-undefined-version is defined as follows:

Normally when a symbol has an undefined version, the linker will
ignore it.  This option disallows symbols with undefined version
and a fatal error will be issued instead.

Description in not very clear in my opinion about what it should do.
Fortunately, Ian Lance Tailor leaved a message in binutils maillist
(https://sourceware.org/ml/binutils/2010-01/msg00092.html)

From there:
"what the option actually does is this:

For each version defined in a version script:

for each global symbol pattern in that version listed in the script:
  if the pattern is a literal string--not a wildcard pattern:
    if the pattern does not match any symbol defined in the link:
      give an error.

In other words, the option does not check for symbols with undefined
versions. It checks for cases where a version script explicitly lists
a symbol, but the symbol is not defined."

Patch implements that behavior. This fixes PR27448.

[ruiu]

ELF/Options.td

112 ↗

(On Diff #61950)

"Report version scripts that refer nonexistent symbols."

ELF/SymbolTable.cpp

619–622

This can be

if (!B || B->isUndefined()) {
  if (Config->NoUndefinedVersion)
    error("version script refers an undefined symbol: " + Name);
  continue;
}

?

(is "version script lists not defined symbol" the error message that gold uses?)

[grimar]

ELF/Options.td

112 ↗

(On Diff #61950)

nonexistent != undefined, but we also report about undefined ones (that is also what gold do).

"Report version scripts that refer non defines symbols." ?

ELF/SymbolTable.cpp

619–622

Yes, I think this can be, there is no point to assign version to undefined symbol I think.

gold uses something like "gold: error: version script assignment of LIBSAMPLE_3.0 to symbol d failed: symbol not defined".

[ruiu]

ELF/Options.td

112 ↗

(On Diff #61950)

How about "Report version scripts that refer undefined symbols." Strictly speaking, we may not have an ELF undefined symbol here, but the fact that a symbol is not defined is true.

[grimar]

ELF/Options.td

112 ↗

(On Diff #61950)

Ok.

[grimar]

• Addressed review comments.

[ruiu]

LGTM with a nit.

ELF/SymbolTable.cpp

621

Please change the error message to be the same as gold. I think gold's error message is better.

[grimar]

That probably looks a bit better. May be a wrong place for Reverser,
or may be we have something like that in LLVM already ? I did not find.

[grimar]

Uploaded diff to wrong page, sorry.

ELF/Config.h

enum ELFKind {
   ELF64LEKind,
   ELF64BEKind
 };
 
 enum class BuildIdKind { None, Fnv1, Md5, Sha1, Hexstring };
 
 enum class UnresolvedPolicy { NoUndef, Error, Warn, Ignore };
 
+struct VersionSymbol {
+  llvm::StringRef Name;
+  bool HasWildcard;
+};
+
 // This struct contains symbols version definition that
 // can be found in version script if it is used for link.
 struct Version {
   Version(llvm::StringRef Name) : Name(Name) {}
   llvm::StringRef Name;
   llvm::StringRef Parent;
-  std::vector<llvm::StringRef> Globals;
+  std::vector<VersionSymbol> Globals;
   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 {
struct Configuration {
   llvm::StringRef SoName;
   llvm::StringRef Sysroot;
   llvm::StringSet<> TraceSymbol;
   std::string RPath;
   std::vector<Version> SymbolVersions;
   std::vector<llvm::StringRef> DynamicList;
   std::vector<llvm::StringRef> SearchPaths;
   std::vector<llvm::StringRef> Undefined;
-  std::vector<llvm::StringRef> VersionScriptGlobals;
+  std::vector<VersionSymbol> VersionScriptGlobals;
   std::vector<uint8_t> BuildIdVector;
   bool AllowMultipleDefinition;
   bool AsNeeded = false;
   bool Bsymbolic;
   bool BsymbolicFunctions;
   bool Demangle = true;
   bool DisableVerify;
   bool DiscardAll;

ELF/SymbolListFile.cpp

 void VersionScriptParser::parseLocal() {
   expect("local:");
   expect("*");
   expect(";");
   Config->VersionScriptGlobalByDefault = false;
 }
 
 void VersionScriptParser::parseVersionSymbols(StringRef Version) {
-  std::vector<StringRef> *Globals;
+  std::vector<VersionSymbol> *Globals;
   if (Version.empty())
     Globals = &Config->VersionScriptGlobals;
   else
     Globals = &Config->SymbolVersions.back().Globals;
 
   for (;;) {
     StringRef Cur = peek();
     if (Cur == "extern")
       setError("extern keyword is not supported");
     if (Cur == "}" || Cur == "local:" || Error)
       return;
     next();
-    Globals->push_back(Cur);
+    Globals->push_back({Cur, Cur.find_first_of("?*") != StringRef::npos});
     expect(";");
   }
 }
 
 void VersionScriptParser::run() {
   StringRef Msg = "anonymous version definition is used in "
                   "combination with other version definitions";
   if (peek() == "{") {

ELF/SymbolTable.h

public:
   void scanDynamicList();
   void scanVersionScript();
   void traceDefined();
 
   SymbolBody *find(StringRef Name);
   void wrap(StringRef Name);
 
 private:
-  std::vector<SymbolBody *> findAll(StringRef Pattern);
+  std::vector<SymbolBody *> findAll(StringRef Pattern, bool NoWildcard);
   std::pair<Symbol *, bool> insert(StringRef Name);
   std::pair<Symbol *, bool> insert(StringRef Name, uint8_t Type,
                                    uint8_t Visibility, bool CanOmitFromDynSym,
                                    bool IsUsedInRegularObj, InputFile *File);
 
   std::string conflictMsg(SymbolBody *Existing, InputFile *NewFile);
   void reportDuplicate(SymbolBody *Existing, InputFile *NewFile);
 
+  void applySymbolVersion(VersionSymbol &Sym, size_t Version);
+
   // 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
   // but a bit inefficient.
   // FIXME: Experiment with passing in a custom hashing or sorting the symbols
   // once symbol resolution is finished.
   llvm::DenseMap<SymName, unsigned> Symtab;

ELF/SymbolTable.cpp

template <class ELFT> SymbolBody *SymbolTable<ELFT>::find(StringRef Name) {
   auto It = Symtab.find(Name);
   if (It == Symtab.end())
     return nullptr;
   return SymVector[It->second]->body();
 }
 
 // Returns a list of defined symbols that match with a given glob pattern.
 template <class ELFT>
-std::vector<SymbolBody *> SymbolTable<ELFT>::findAll(StringRef Pattern) {
+std::vector<SymbolBody *> SymbolTable<ELFT>::findAll(StringRef Pattern,
+                                                     bool NoWildcard) {
   // Fast-path. Fallback to find() if Pattern doesn't contain any wildcard
   // characters.
-  if (Pattern.find_first_of("?*") == StringRef::npos) {
+  if (NoWildcard) {
     if (SymbolBody *B = find(Pattern))
       if (!B->isUndefined())
         return {B};
     return {};
   }
 
   std::vector<SymbolBody *> Res;
   for (auto &It : Symtab) {

 // This function process the dynamic list option by marking all the symbols
 // to be exported in the dynamic table.
 template <class ELFT> void SymbolTable<ELFT>::scanDynamicList() {
   for (StringRef S : Config->DynamicList)
     if (SymbolBody *B = find(S))
       B->symbol()->ExportDynamic = true;
 }
 
+template <class ELFT>
+void SymbolTable<ELFT>::applySymbolVersion(VersionSymbol &Sym, size_t Version) {
+  std::vector<SymbolBody *> Syms = findAll(Sym.Name, !Sym.HasWildcard);
+  if (Syms.empty()) {
+    if (Config->NoUndefinedVersion)
+      error("version script assignment of " +
+            Config->SymbolVersions[Version].Name + " to symbol " + Sym.Name +
+            " failed: symbol not defined");
+    return;
+  }
+
+  for (SymbolBody *B : Syms) {
+    if (B->symbol()->VersionId == VER_NDX_GLOBAL ||
+        B->symbol()->VersionId == VER_NDX_LOCAL) {
+      B->symbol()->VersionId = Version + 2;
+      continue;
+    }
+
+    // It is ok that wildcard matches the symbol that already was assigned
+    // some version. Just skip it.
+    if (Sym.HasWildcard)
+      continue;
+
+    warning("duplicate symbol " + Sym.Name + " in version script");
+  }
+}
+
+template <typename T> struct Reverser {
+  T &Rev;
+  Reverser(T &R) : Rev(R) {}
+  typename T::reverse_iterator begin() { return Rev.rbegin(); }
+  typename T::reverse_iterator end() { return Rev.rend(); }
+};
+
+template <typename T> Reverser<T> Reverse(T &R) {
+  return Reverser<T>(R);
+}
+
 // This function processes the --version-script option by marking all global
 // symbols with the VersionScriptGlobal flag, which acts as a filter on the
 // dynamic symbol table.
 template <class ELFT> void SymbolTable<ELFT>::scanVersionScript() {
   // If version script does not contain versions declarations,
   // we just should mark global symbols.
   if (!Config->VersionScriptGlobals.empty()) {
-    for (StringRef S : Config->VersionScriptGlobals)
-      if (SymbolBody *B = find(S))
+    for (VersionSymbol &S : Config->VersionScriptGlobals)
+      if (SymbolBody *B = find(S.Name))
         B->symbol()->VersionId = VER_NDX_GLOBAL;
     return;
   }
 
   // If we have symbols version declarations, we should
   // assign version references for each symbol.
-  size_t I = 2;
-  for (Version &V : Config->SymbolVersions) {
-    for (StringRef Name : V.Globals) {
-      std::vector<SymbolBody *> Syms = findAll(Name);
-      if (Syms.empty()) {
-        if (Config->NoUndefinedVersion)
-          error("version script assignment of " + V.Name + " to symbol " +
-                Name + " failed: symbol not defined");
-        continue;
-      }
-
-      for (SymbolBody *B : Syms) {
-        if (B->symbol()->VersionId != VER_NDX_GLOBAL &&
-            B->symbol()->VersionId != VER_NDX_LOCAL)
-          warning("duplicate symbol " + Name + " in version script");
-        B->symbol()->VersionId = I;
-      }
-    }
-    ++I;
-  }
+  // Current rules are:
+  // * If there is an exact match for the mangled name, we use it.
+  // * Otherwise, we look through the wildcard patterns. We look through the
+  //   version tags in reverse order. We use the first match we find (the last
+  //   matching version tag in the file).
+  for (Version &V : Config->SymbolVersions)
+    for (VersionSymbol &Sym : V.Globals)
+      if (!Sym.HasWildcard)
+        applySymbolVersion(Sym, &V - Config->SymbolVersions.data());
+
+  for (Version &V : Reverse(Config->SymbolVersions))
+    for (VersionSymbol &Sym : V.Globals)
+      if (Sym.HasWildcard)
+        applySymbolVersion(Sym, &V - Config->SymbolVersions.data());
 }
 
 // Print the module names which define the notified
 // symbols provided through -y or --trace-symbol option.
 template <class ELFT> void SymbolTable<ELFT>::traceDefined() {
   for (const auto &Symbol : Config->TraceSymbol)
     if (SymbolBody *B = find(Symbol.getKey()))
       if (B->isDefined() || B->isCommon())

test/ELF/version-wildcard.test

 # CHECK-NEXT:     Size: 0
 # CHECK-NEXT:     Binding: Global
 # CHECK-NEXT:     Type: None
 # CHECK-NEXT:     Other: 0
 # CHECK-NEXT:     Section: .text
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: echo "VERSION_1.0{      \
+# RUN:          global: foo2;  \
+# RUN:          local: *; };   \
+# RUN:       VERSION_2.0{      \
+# RUN:          global: foo*;  \
+# RUN:       }; " > %t2.script
+# RUN: ld.lld --version-script %t2.script -shared %t.o -o %t2.so
+# RUN: llvm-readobj -dyn-symbols %t2.so | FileCheck --check-prefix=MIX %s
+
+# MIX:      DynamicSymbols [
+# MIX-NEXT:   Symbol {
+# MIX-NEXT:     Name: @
+# MIX-NEXT:     Value: 0x0
+# MIX-NEXT:     Size: 0
+# MIX-NEXT:     Binding: Local
+# MIX-NEXT:     Type: None
+# MIX-NEXT:     Other: 0
+# MIX-NEXT:     Section: Undefined
+# MIX-NEXT:   }
+# MIX-NEXT:   Symbol {
+# MIX-NEXT:     Name: foo1@@VERSION_2.0
+# MIX-NEXT:     Value: 0x1000
+# MIX-NEXT:     Size: 0
+# MIX-NEXT:     Binding: Global
+# MIX-NEXT:     Type: None
+# MIX-NEXT:     Other: 0
+# MIX-NEXT:     Section: .text
+# MIX-NEXT:   }
+# MIX-NEXT:   Symbol {
+# MIX-NEXT:     Name: foo2@@VERSION_1.0
+# MIX-NEXT:     Value: 0x1001
+# MIX-NEXT:     Size: 0
+# MIX-NEXT:     Binding: Global
+# MIX-NEXT:     Type: None
+# MIX-NEXT:     Other: 0
+# MIX-NEXT:     Section: .text
+# MIX-NEXT:   }
+# MIX-NEXT:   Symbol {
+# MIX-NEXT:     Name: foo3@@VERSION_2.0
+# MIX-NEXT:     Value: 0x1007
+# MIX-NEXT:     Size: 0
+# MIX-NEXT:     Binding: Global
+# MIX-NEXT:     Type: None
+# MIX-NEXT:     Other: 0
+# MIX-NEXT:     Section: .text
+# MIX-NEXT:   }
+# MIX-NEXT: ]
+
 .globl foo1
 foo1:
   ret
 
 .globl foo2
 foo2:
   call foo1@PLT
   ret