Make SymbolTable::findAll to return only defined symbols.

[ruiu]

Summary

We allowed the function to return a vector that contains nullptrs
which is weird. This change makes the function to return only
defined symbols.

[davide]

I haven't a strong preference, but if you really like this one better, go for it.

[ruiu]

I think that the previous version has a hidden bug that if a returned vector contains two symbols {A, B} where A is undefined and B is undefined, it would report an warning that "version script assignment of foo to B failed: symbol not defined".

[davide]

OK, lgtm.

[davide]

If you can find a way to trigger the bug, and add a testcase, that will be great.

lld/trunk/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) {
   // Fast-path. Fallback to find() if Pattern doesn't contain any wildcard
   // characters.
-  bool HasWildcards = (Pattern.find_first_of("?*") != StringRef::npos);
-  if (!HasWildcards)
-    return {find(Pattern)};
-
-  std::vector<SymbolBody *> Result;
-  for (auto &It : Symtab)
-    if (matchStr(Pattern, It.first.Val))
-      Result.push_back(SymVector[It.second]->body());
-  return Result;
+  if (Pattern.find_first_of("?*") == StringRef::npos) {
+    if (SymbolBody *B = find(Pattern))
+      if (!B->isUndefined())
+        return {B};
+    return {};
+  }
+
+  std::vector<SymbolBody *> Res;
+  for (auto &It : Symtab) {
+    StringRef Name = It.first.Val;
+    SymbolBody *B = SymVector[It.second]->body();
+    if (!B->isUndefined() && matchStr(Pattern, Name))
+      Res.push_back(B);
+  }
+  return Res;
 }
 
 template <class ELFT>
 void SymbolTable<ELFT>::addLazyArchive(
     ArchiveFile *F, const llvm::object::Archive::Symbol Sym) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Sym.getName());
if (!Config->VersionScriptGlobals.empty()) {
     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) {
-      for (SymbolBody *B : findAll(Name)) {
-        if (!B || B->isUndefined()) {
-          if (Config->NoUndefinedVersion)
-            error("version script assignment of " + V.Name + " to symbol " +
-                  Name + " failed: symbol not defined");
-          continue;
-        }
+      std::vector<SymbolBody *> Syms = findAll(Name);
+      if (Syms.empty()) {
 
+      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;
   }