[ELF] - Fix for: PR29093 - version script does not support [chars] wildcards

[grimar]

Summary

From PR29093 description:

GNU ld supports [chars] wildcards in version scripts, to match a single instance of any of the chars.

Here is an excerpt from libstdc++'s version script in FreeBSD:

extern "C++"
{

...

std::locale::_[T-Za-z]*;                                                                                                       
std::[A-Zm]*;                                                            
std::n[^u]*;                                                             
std::nu[^m]*;                                                            
std::num[^e]*;

...

}

Patch adds support for scripts above.

[emaste]

ELF/SymbolTable.cpp

580 ↗

(On Diff #68992)

Should - and ^ be in here? They're only related to wildcards if inside of [].

[grimar]

ELF/SymbolTable.cpp

580 ↗

(On Diff #68992)

I think they can not be a regular part of symbol name. Am I missing something ?

[grimar]

• Simplified.

[grimar]

Something looks wrong with diff2, I`ll update it soon.

[emaste]

ELF/SymbolTable.cpp

580 ↗

(On Diff #68994)

not sure, just that e.g. foo-bar does not have a wildcard

[grimar]

• Last minute fix.
• Improved testcase
• Cosmetic changes.

ELF/SymbolTable.cpp

580 ↗

(On Diff #68994)

This function is used for checking symbol names. Probably we can name it differently to avoid confusing, but what I want to say is that I think foo-bar is not a valid name for symbols and we should not do more complex checks here IMO.

[grimar]

ELF/SymbolTable.cpp

580 ↗

(On Diff #69001)

Though after revisiting, since '-' and '^' can not be used without '[' or ']' we can remove it from here to reduce checks I think.

[emaste]

ELF/SymbolTable.cpp

580 ↗

(On Diff #69001)

Yeah, that's my point. If foo-bar is not valid it'll just be handled as it is today (without the wildcard change).

[grimar]

ELF/SymbolTable.cpp

580 ↗

(On Diff #69001)

I wonder if we need to check ']' then :) It makes no sence without '[',
so we probably can check only for '['..

[emaste]

ELF/SymbolTable.cpp

580 ↗

(On Diff #69001)

Good point, I agree. A version script entry must have one of ?*[ to be a wildcard.

Note that \ is also supported to escape a character, so e.g. foo\*bar is not a wildcard. But this is a complexity we can revisit later.

[ruiu]

I wonder if we should convert glob patterns to regexs and compile them. How hard is it to convert? I believe that compiled regexps are faster than our hand-written, recursion-based glob matcher, so it may be worth doing even if conversion is as hard as our glob matcher.

[ruiu]

Please take a look at https://reviews.llvm.org/D23827. This is a proof-of-concept patch to do glob pattern matching using std::regex. This needs to be improved (creating regex instance is expensive, so you want to do it only once for each glob pattern instead of doing it in globMatch), but I think you can get the idea.

[grimar]

In D23803#523883, @ruiu wrote:

Please take a look at https://reviews.llvm.org/D23827. This is a proof-of-concept patch to do glob pattern matching using std::regex. This needs to be improved (creating regex instance is expensive, so you want to do it only once for each glob pattern instead of doing it in globMatch), but I think you can get the idea.

Looks good IMO. I can prepare initial batch basing on this approach if you do not mind.

[grimar]

In D23803#523883, @ruiu wrote:

Please take a look at https://reviews.llvm.org/D23827. This is a proof-of-concept patch to do glob pattern matching using std::regex. This needs to be improved (creating regex instance is expensive, so you want to do it only once for each glob pattern instead of doing it in globMatch), but I think you can get the idea.

There is one problem with it:
"If the syntax used in the sequence of characters has some syntax error, the constructor throws a regex_error exception.". Not sure how we going to handle this ?

[grimar]

• Rebased (now code is based on llvm::Regex).

[ruiu]

LGTM

ELF/Strings.cpp

33 ↗

(On Diff #70320)

InBracketExpr -> InBracket

lld/trunk/ELF/ScriptParser.cpp

if (S.startswith("\"")) {
       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"
-        "0123456789_.$/\\~=+[]*?-:!<>");
+        "0123456789_.$/\\~=+[]*?-:!<>^");
 
     // A character that cannot start a word (which is usually a
     // punctuation) forms a single character token.
     if (Pos == 0)
       Pos = 1;
     Ret.push_back(S.substr(0, Pos));
     S = S.substr(Pos);
   }

lld/trunk/ELF/Strings.cpp

 #include "llvm/Demangle/Demangle.h"
 #include <algorithm>
 
 using namespace llvm;
 using namespace lld;
 using namespace lld::elf;
 
 bool elf::hasWildcard(StringRef S) {
-  return S.find_first_of("?*") != StringRef::npos;
+  return S.find_first_of("?*[") != StringRef::npos;
 }
 
 // Converts a glob pattern to a regular expression.
 static std::string toRegex(StringRef S) {
-  if (S.find_first_of("[]") != StringRef::npos)
-    warning("unsupported wildcard: " + S);
-
   std::string T;
+  bool InBracket = false;
   while (!S.empty()) {
     char C = S.front();
+    if (InBracket) {
+      InBracket = C != ']';
+      T += C;
+      S = S.drop_front();
+      continue;
+    }
+
     if (C == '*')
       T += ".*";
     else if (C == '?')
       T += '.';
-    else if (StringRef(".+^${}()|/\\[]").find_first_of(C) != StringRef::npos)
+    else if (StringRef(".+^${}()|/\\").find_first_of(C) != StringRef::npos)
       T += std::string("\\") + C;
     else
       T += C;
+
+    InBracket = C == '[';
     S = S.substr(1);
   }
   return T;
 }
 
 // Converts multiple glob patterns to a regular expression.
 Regex elf::compileGlobPatterns(ArrayRef<StringRef> V) {
   std::string T = "^(" + toRegex(V[0]);

lld/trunk/test/ELF/version-script-complex-wildcards.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "LIBSAMPLE_2.0 { \
+# RUN:   global:             \
+# RUN:     extern "C++" {    \
+# RUN:       ab[c]*;          \
+# RUN:   };                  \
+# RUN: }; " > %t.script
+# RUN: ld.lld --version-script %t.script -shared %t.o -o %t.so
+# RUN: llvm-readobj -V %t.so | FileCheck %s --check-prefix=ABC
+# ABC: Name: _Z3abbi@
+# ABC: Name: _Z3abci@@LIBSAMPLE_2.0
+
+# RUN: echo "LIBSAMPLE_2.0 { \
+# RUN:   global:             \
+# RUN:     extern "C++" {    \
+# RUN:       ab[b]*;          \
+# RUN:   };                  \
+# RUN: }; " > %t1.script
+# RUN: ld.lld --version-script %t1.script -shared %t.o -o %t1.so
+# RUN: llvm-readobj -V %t1.so | FileCheck %s --check-prefix=ABB
+# ABB: Name: _Z3abbi@@LIBSAMPLE_2.0
+# ABB: Name: _Z3abci@
+
+# RUN: echo "LIBSAMPLE_2.0 { \
+# RUN:   global:             \
+# RUN:     extern "C++" {    \
+# RUN:       ab[a-b]*;       \
+# RUN:   };                  \
+# RUN: }; " > %t2.script
+# RUN: ld.lld --version-script %t2.script -shared %t.o -o %t2.so
+# RUN: llvm-readobj -V %t2.so | FileCheck %s --check-prefix=ABB
+
+# RUN: echo "LIBSAMPLE_2.0 { \
+# RUN:   global:             \
+# RUN:     extern "C++" {    \
+# RUN:       ab[a-c]*;       \
+# RUN:   };                  \
+# RUN: }; " > %t3.script
+# RUN: ld.lld --version-script %t3.script -shared %t.o -o %t3.so
+# RUN: llvm-readobj -V %t3.so | FileCheck %s --check-prefix=ABBABC
+# ABBABC: Name: _Z3abbi@@LIBSAMPLE_2.0
+# ABBABC: Name: _Z3abci@@LIBSAMPLE_2.0
+
+# RUN: echo "LIBSAMPLE_2.0 { \
+# RUN:   global:             \
+# RUN:     extern "C++" {    \
+# RUN:       ab[a-bc-d]*;       \
+# RUN:   };                  \
+# RUN: }; " > %t4.script
+# RUN: ld.lld --version-script %t4.script -shared %t.o -o %t4.so
+# RUN: llvm-readobj -V %t4.so | FileCheck %s --check-prefix=ABBABC
+
+# RUN: echo "LIBSAMPLE_2.0 { \
+# RUN:   global:             \
+# RUN:     extern "C++" {    \
+# RUN:       ab[a-bd-e]*;       \
+# RUN:   };                  \
+# RUN: }; " > %t5.script
+# RUN: ld.lld --version-script %t5.script -shared %t.o -o %t5.so
+# RUN: llvm-readobj -V %t5.so | FileCheck %s --check-prefix=ABB
+
+# RUN: echo "LIBSAMPLE_2.0 { \
+# RUN:   global:             \
+# RUN:     extern "C++" {    \
+# RUN:       ab[^a-c]*;       \
+# RUN:   };                  \
+# RUN: }; " > %t6.script
+# RUN: ld.lld --version-script %t6.script -shared %t.o -o %t6.so
+# RUN: llvm-readobj -V %t6.so | FileCheck %s --check-prefix=NO
+# NO:  Name: _Z3abbi@
+# NO:  Name: _Z3abci@
+
+# RUN: echo "LIBSAMPLE_2.0 { \
+# RUN:   global:             \
+# RUN:     extern "C++" {    \
+# RUN:       ab[^c-z]*;       \
+# RUN:   };                  \
+# RUN: }; " > %t7.script
+# RUN: ld.lld --version-script %t7.script -shared %t.o -o %t7.so
+# RUN: llvm-readobj -V %t7.so | FileCheck %s --check-prefix=ABB
+
+# RUN: echo "LIBSAMPLE_2.0 { \
+# RUN:   global:             \
+# RUN:     extern "C++" {    \
+# RUN:       a[x-za-b][a-c]*;       \
+# RUN:   };                  \
+# RUN: }; " > %t8.script
+# RUN: ld.lld --version-script %t8.script -shared %t.o -o %t8.so
+# RUN: llvm-readobj -V %t8.so | FileCheck %s --check-prefix=ABBABC
+
+.text
+.globl _Z3abci
+.type _Z3abci,@function
+_Z3abci:
+retq
+
+.globl _Z3abbi
+.type _Z3abbi,@function
+_Z3abbi:
+retq