[ELF] - Versionscript: do not treat non-wildcarded names as wildcards.

[grimar]

Summary

Previously we incorrectly handled cases when symbol name in extern c++ tag
was enclosed in quotes. Next case was treated as wildcard:

GLIBCXX_3.4 {                                                                   
    extern "C++" {  
    "aaa*"
   }

But it should have not. Quotes around aaa here means that we should have do exact
name matching.
That is PR30268 which has name with pointer is interpreted as wildcard by lld:

extern "C++" {                                                              
    "operator delete[](void*)";

[ruiu]

With this patch, the existence of double quotes affects how strings will be interpreted later. Such grammar seems really weird to me. Are you sure that the grammar is correct? You are making

foo::*

to match foo::bar, while

"foo::*"

does not match foo::bar.

test/ELF/version-script-extern-exact.s

6 ↗

(On Diff #70317)

You need to escape '"' in this line too.

[grimar]

In D24229#535364, @ruiu wrote:

With this patch, the existence of double quotes affects how strings will be interpreted later. Such grammar seems really weird to me. Are you sure that the grammar is correct?

Yes, I believe so. From GNU Liner specs
(p.66 https://web.eecs.umich.edu/~prabal/teaching/resources/eecs373/Linker.pdf):

"Demangled names may contains spaces and other special characters. As described above,
you can use a glob pattern to match demangled names, or you can use a double-quoted
string to match the string exactly. In the latter case, be aware that minor differences (such
as differing whitespace) between the version script and the demangler output will cause a
mismatch."

Behavior is expected, I re-checked both gold and ld:

Used version script:

LIBSAMPLE_1.0 {
   global:            
     extern "C++" {  
       "foo*";   
   };                  
 };                                     

Code:
.text
.globl _Z3fooi
.type _Z3fooi,@function
_Z3fooi:
retq

llvm-mc -filetype=obj -triple=x86_64-pc-linux test1.s -o test1.o
ld --version-script test.script -shared test1.o -o out
llvm-readobj -V out

foo is unversioned:

Symbol {
  Version: 1
  Name: _Z3fooi@
}

But if I remove quotes then it has version LIBSAMPLE_1.0:

LIBSAMPLE_1.0 {
   global:            
     extern "C++" {  
       foo*;   
   };                  
 };    

  Symbol {
    Version: 2
    Name: _Z3fooi@@LIBSAMPLE_1.0
  }

That actually looks as simple and nice rule for me. Value in quotes is always exact match only and never a wildcard.

[grimar]

• Addressed review comment.

test/ELF/version-script-extern-exact.s

6 ↗

(On Diff #70317)

Right. This bug we also have in other testcases, e.g version-script-extern-wildcards.s. I`ll fix.

[ruiu]

If you think of this, that grammar looks very weird because it lacks a way to write a glob pattern containing a whitespace character. For example, if you want to write a pattern that matches strings starting with "foo " followed by any characters, you need this glob pattern: "foo *". However, if you write that in a linker script, it means the literal string "foo *" instead of a glob pattern, because the token is quoted. That means there's no way to express that in the linker script.

Being said that, this is what GNU does, and linker scripts are written that way, so unfortunately there's no way other than implementing the same thing.

ELF/ScriptParser.cpp

63 ↗

(On Diff #70512)

Please add a comment why we leave double quotes in a resulting token.

// Quoted token. Note that double-quote characters are parts of a token
// because, in a glob match context, only unquoted tokens are interpreted
// as glob patterns. Double-quoted tokens are literal patterns in that context.

test/ELF/version-script-extern-exact.s

7 ↗

(On Diff #70512)

echo "FOO { global: extern \"C++\" \"aaa*\"; }; };" > %t.script

[grimar]

• Addressed review comments.
• Make checks in tests to be explicit.

ELF/ScriptParser.cpp

63 ↗

(On Diff #70512)

Done.

test/ELF/version-script-extern-exact.s

7 ↗

(On Diff #70512)

Done.

[ruiu]

LGTM

lld/trunk/ELF/Config.h

 enum class StripPolicy { None, All, Debug };
 
 // For --unresolved-symbols.
 enum class UnresolvedPolicy { NoUndef, ReportError, Warn, Ignore };
 
 struct SymbolVersion {
   llvm::StringRef Name;
   bool IsExternCpp;
+  bool HasWildcards;
 };
 
 // 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;

lld/trunk/ELF/LinkerScript.cpp

if (sys::path::is_absolute(S)) {
     std::string Path = findFromSearchPaths(S);
     if (Path.empty())
       setError("unable to find " + S);
     else
       Driver->addFile(Saver.save(Path));
   }
 }
 
+static StringRef unquote(StringRef S) {
+  if (!S.startswith("\""))
+    return S;
+  return S.substr(1, S.size() - 2);
+}
+
 void ScriptParser::readAsNeeded() {
   expect("(");
   bool Orig = Config->AsNeeded;
   Config->AsNeeded = true;
   while (!Error && !skip(")"))
-    addFile(next());
+    addFile(unquote(next()));
   Config->AsNeeded = Orig;
 }
 
 void ScriptParser::readEntry() {
   // -e <symbol> takes predecence over ENTRY(<symbol>).
   expect("(");
   StringRef Tok = next();
   if (Config->Entry.empty())

 
 void ScriptParser::readGroup() {
   expect("(");
   while (!Error && !skip(")")) {
     StringRef Tok = next();
     if (Tok == "AS_NEEDED")
       readAsNeeded();
     else
-      addFile(Tok);
+      addFile(unquote(Tok));
   }
 }
 
 void ScriptParser::readInclude() {
   StringRef Tok = next();
   auto MBOrErr = MemoryBuffer::getFile(Tok);
   if (!MBOrErr) {
     setError("cannot open " + Tok);
void ScriptParser::readSort() {
   expect("CONSTRUCTORS");
   expect(")");
 }
 
 Expr ScriptParser::readAssert() {
   expect("(");
   Expr E = readExpr();
   expect(",");
-  StringRef Msg = next();
+  StringRef Msg = unquote(next());
   expect(")");
   return [=](uint64_t Dot) {
     uint64_t V = E(Dot);
     if (!V)
       error(Msg);
     return V;
   };
 }

 
 void ScriptParser::readLocal() {
   Config->DefaultSymbolVersion = VER_NDX_LOCAL;
   expect("*");
   expect(";");
 }
 
 void ScriptParser::readExtern(std::vector<SymbolVersion> *Globals) {
-  expect("C++");
+  expect("\"C++\"");
   expect("{");
 
   for (;;) {
     if (peek() == "}" || Error)
       break;
-    Globals->push_back({next(), true});
+    bool HasWildcard = !peek().startswith("\"") && hasWildcard(peek());
+    Globals->push_back({unquote(next()), true, HasWildcard});
     expect(";");
   }
 
   expect("}");
   expect(";");
 }
 
 void ScriptParser::readGlobal(StringRef VerStr) {
   std::vector<SymbolVersion> *Globals;
   if (VerStr.empty())
     Globals = &Config->VersionScriptGlobals;
   else
     Globals = &Config->VersionDefinitions.back().Globals;
 
   for (;;) {
     if (skip("extern"))
       readExtern(Globals);
 
     StringRef Cur = peek();
     if (Cur == "}" || Cur == "local:" || Error)
       return;
     next();
-    Globals->push_back({Cur, false});
+    Globals->push_back({unquote(Cur), false, hasWildcard(Cur)});
     expect(";");
   }
 }
 
 static bool isUnderSysroot(StringRef Path) {
   if (Config->Sysroot == "")
     return false;
   for (; !Path.empty(); Path = sys::path::parent_path(Path))

lld/trunk/ELF/ScriptParser.cpp

 // Split S into linker script tokens.
 std::vector<StringRef> ScriptParserBase::tokenize(StringRef S) {
   std::vector<StringRef> Ret;
   for (;;) {
     S = skipSpace(S);
     if (S.empty())
       return Ret;
 
-    // Quoted token.
+    // Quoted token. Note that double-quote characters are parts of a token
+    // because, in a glob match context, only unquoted tokens are interpreted
+    // as glob patterns. Double-quoted tokens are literal patterns in that
+    // context.
     if (S.startswith("\"")) {
       size_t E = S.find("\"", 1);
       if (E == StringRef::npos) {
         error("unclosed quote");
         return {};
       }
-      Ret.push_back(S.substr(1, E - 1));
+      Ret.push_back(S.take_front(E + 1));
       S = S.substr(E + 1);
       continue;
     }
 
     // Unquoted token. This is more relaxed than tokens in C-like language,
     // so that you can write "file-name.cpp" as one bare token, for example.
     size_t Pos = S.find_first_not_of(
         "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"

lld/trunk/ELF/SymbolTable.cpp

 // the symbol types.
 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolTable.h"
 #include "Config.h"
 #include "Error.h"
 #include "LinkerScript.h"
-#include "Strings.h"
 #include "SymbolListFile.h"
 #include "Symbols.h"
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/Support/StringSaver.h"
 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::ELF;
template <class ELFT> void SymbolTable<ELFT>::scanVersionScript() {
   std::map<std::string, SymbolBody *> Demangled;
   if (hasExternCpp())
     Demangled = getDemangledSyms();
 
   // 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 (hasWildcard(Sym.Name))
+      if (Sym.HasWildcards)
         continue;
       StringRef N = Sym.Name;
       SymbolBody *B = Sym.IsExternCpp ? findDemangled(Demangled, N) : find(N);
       setVersionId(B, V.Name, N, V.Id);
     }
   }
 
   // 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 (!hasWildcard(Sym.Name))
+      if (!Sym.HasWildcards)
         continue;
       Regex Re = compileGlobPatterns({Sym.Name});
       std::vector<SymbolBody *> Syms =
           Sym.IsExternCpp ? findAllDemangled(Demangled, Re) : findAll(Re);
 
       // 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.

lld/trunk/test/ELF/version-script-extern-exact.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "FOO { global: extern \"C++\" { \"aaa*\"; }; };" > %t.script
+# RUN: ld.lld --version-script %t.script -shared %t.o -o %t.so
+# RUN: llvm-readobj -V -dyn-symbols %t.so | FileCheck %s
+
+# CHECK: Symbol {
+# CHECK:   Name: _Z3aaaPf@ (1)
+# CHECK: Symbol {
+# CHECK:   Name: _Z3aaaPi@ (10)
+
+.text
+.globl _Z3aaaPi
+.type _Z3aaaPi,@function
+_Z3aaaPi:
+retq
+
+.globl _Z3aaaPf
+.type _Z3aaaPf,@function
+_Z3aaaPf:
+retq