[lld/ELF] PR44498: Support input filename in double quote

[thopre]

Summary

Linker scripts allow filenames to be put in double quotes to prevent
characters in filenames that are part of the linker script syntax from
having their special meaning. Case in point the * wildcard character.

Availability of double quoting filenames also allows to fix a failure in
ELF/linkerscript/filename-spec.s when the path contain a @ which the
lexer consider as a special characters and thus break up a filename
containing it. This may happens under Jenkins which createspath such as
pipeline@2.

To avoid the need for escaping GlobPattern metacharacters in filename
in double quotes, GlobPattern::create is augmented with a new parameter
to request literal matching instead of relying on the presence of a
wildcard character in the pattern.

[MaskRay]

I think we should add logic to StringMatcher instead of GlobPattern.

lld/test/ELF/linkerscript/filename-spec.s

33

Un-quote this test so that we won't lose coverage for unquoted strings.

[MaskRay]

allows to fix a failure in ELF/linkerscript/filename-spec.s when the path contain a @ which the lexer consider as a special characters and thus break up a filename containing it.

Which one?

[evgeny777]

llvm/include/llvm/Support/GlobPattern.h

31

This looks counter intuitive. Typically I'd use GlobPattern for wildcard matching, not for exact comparison.

[thopre]

• Make exact match at StringMatcher level
• Add possibility to add new pattern to a StringMatcher

[thopre]

In D72517#1814982, @MaskRay wrote:

allows to fix a failure in ELF/linkerscript/filename-spec.s when the path contain a @ which the lexer consider as a special characters and thus break up a filename containing it.

Which one?

See updated description. Jenkins sometimes create workspace with an @2 in them so if the repo is cloned into that path test filename will have an @ which will confuse the lexer on that line. It'll break the filename just before the @ and will create a matcher for that rather than the full path.

lld/test/ELF/linkerscript/filename-spec.s

33

The problem is that this patch was created precisely to deal with this line failing when the path to the test contains a special character, which happens under jenkins (see updated description).

[evgeny777]

I think that adding extra flag to GlobPattern::create was better idea. Just set it to false by default, so that GlobPattern will still act as glob matcher for all other users:

static static Expected<GlobPattern> create(StringRef Pat, bool IgnoreMetaChars = false);

Than add extra constructor to StringMatcher to which you'll pass GlobPattern and use it from InputSectionDescription constructor.

lld/ELF/ScriptParser.cpp

637 ↗

(On Diff #238037)

Add empty() method to StringMatcher and use !SectionMatcher.empty() instead of this flag

849 ↗

(On Diff #238037)

What's the purpose of this change?

[thopre]

In D72517#1823830, @evgeny777 wrote:

I think that adding extra flag to GlobPattern::create was better idea. Just set it to false by default, so that GlobPattern will still act as glob matcher for all other users:

static static Expected<GlobPattern> create(StringRef Pat, bool IgnoreMetaChars = false);

Than add extra constructor to StringMatcher to which you'll pass GlobPattern and use it from InputSectionDescription constructor.

Actually I agree with Fangrui. Having GlobPattern do any match other than globbing would be counter-intuitive given the name. A user who doesn't want globbing just use an exact match. I'm not opposed to create a new wrapper class around GlobPattern that would allow exact match with a name that reflect that matching duality. Maybe simply PatternMatcher?

lld/ELF/ScriptParser.cpp

849 ↗

(On Diff #238037)

I've changed StringMatcher to only take a single pattern to avoid changing the array of pattern for an array of pair (pattern + exact match boolean) which would make the interface more cumbersome to use *and* implement. All callers passing an array were constructing it in a loop with push_back which I've changed for a call to a new addPattern.

[thopre]

Introduce StringMatcher.empty()

[MaskRay]

LG. It may be worth waiting for other opinions.

lld/Common/Strings.cpp

std::string lld::demangleItanium(StringRef name) {
   // does not look like a C++ symbol name to avoid getting unexpected
   // result for a C symbol that happens to match a mangled type name.
   if (!name.startswith("_Z"))
     return name;
 
   return demangle(name);
 }
 
+StringMatcher::StringMatcher(ArrayRef<std::pair<StringRef, bool>> pat) {
+  for (std::pair<StringRef, bool> p : pat) {
+    Expected<GlobPattern> pat = GlobPattern::create(p.first, p.second);
+    if (!pat)
+      error(toString(pat.takeError()));
+    else
+      patterns.push_back(*pat);
+  }
+}
+
 StringMatcher::StringMatcher(ArrayRef<StringRef> pat) {
   for (StringRef s : pat) {
     Expected<GlobPattern> pat = GlobPattern::create(s);
     if (!pat)
       error(toString(pat.takeError()));
     else
       patterns.push_back(*pat);
   }

lld/ELF/LinkerScript.h

struct SectionPattern {
   StringMatcher excludedFilePat;
   StringMatcher sectionPat;
   SortSectionPolicy sortOuter;
   SortSectionPolicy sortInner;
 };
 
 struct InputSectionDescription : BaseCommand {
   InputSectionDescription(StringRef filePattern)
-      : BaseCommand(InputSectionKind), filePat(filePattern) {}
+      : BaseCommand(InputSectionKind), filePat(filePattern) {
+    bool ForceExactMatch = false;
+    // Do not glob filename inside double quotes.
+    if (filePattern.startswith("\"")) {
+      filePattern = filePattern.substr(1, filePattern.size() - 2);
+      ForceExactMatch = true;
+    }
+    filePat = StringMatcher(std::make_pair(filePattern, ForceExactMatch));
+  }
 
   static bool classof(const BaseCommand *c) {
     return c->kind == InputSectionKind;
   }
 
   StringMatcher filePat;
 
   // Input sections that matches at least one of SectionPatterns

lld/include/lld/Common/Strings.h

 // Write the contents of the a buffer to a file
 void saveBuffer(llvm::StringRef buffer, const llvm::Twine &path);
 
 // This class represents multiple glob patterns.
 class StringMatcher {
 public:
   StringMatcher() = default;
   explicit StringMatcher(llvm::ArrayRef<llvm::StringRef> pat);
+  explicit StringMatcher(llvm::ArrayRef<std::pair<llvm::StringRef, bool>> pat);
 
   bool match(llvm::StringRef s) const;
 
 private:
   std::vector<llvm::GlobPattern> patterns;
 };
 
 } // namespace lld
 
 #endif

lld/test/ELF/linkerscript/filename-spec.s

 # RUN: ld.lld -o %t4 --script %t4.script %tx.o %ty.o
 # RUN: llvm-objdump -s %t4 | FileCheck --check-prefix=SECONDFIRST %s
 
 # RUN: rm -rf %t.dir && mkdir -p %t.dir
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.dir/filename-spec1.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux \
 # RUN:   %p/Inputs/filename-spec.s -o %t.dir/filename-spec2.o
 
-# RUN: echo "SECTIONS{.foo :{ %/t.dir/filename-spec2.o(.foo) %/t.dir/filename-spec1.o(.foo) }}" > %t5.script
+# RUN: echo "SECTIONS{.foo :{ \"%/t.dir/filename-spec2.o\"(.foo) \"%/t.dir/filename-spec1.o\"(.foo) }}" > %t5.script
 # RUN: ld.lld -o %t5 --script %t5.script \
 # RUN:   %/t.dir/filename-spec1.o %/t.dir/filename-spec2.o
 # RUN: llvm-objdump -s %t5 | FileCheck --check-prefix=SECONDFIRST %s
 
-# RUN: echo "SECTIONS{.foo :{ %/t.dir/filename-spec1.o(.foo) %/t.dir/filename-spec2.o(.foo) }}" > %t6.script
+# RUN: echo "SECTIONS{.foo :{ \"%/t.dir/filename-spec1.o\"(.foo) \"%/t.dir/filename-spec2.o\"(.foo) }}" > %t6.script
 # RUN: ld.lld -o %t6 --script %t6.script \
 # RUN:   %/t.dir/filename-spec1.o %/t.dir/filename-spec2.o
 # RUN: llvm-objdump -s %t6 | FileCheck --check-prefix=FIRSTY %s
 
 # RUN: mkdir -p %t.testdir1 %t.testdir2
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.testdir1/filename-spec1.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux \
 # RUN:   %p/Inputs/filename-spec.s -o %t.testdir2/filename-spec2.o

llvm/include/llvm/Support/GlobPattern.h

 // This class represents a glob pattern. Supported metacharacters
 // are "*", "?", "\", "[<chars>]", "[^<chars>]", and "[!<chars>]".
 namespace llvm {
 class BitVector;
 template <typename T> class ArrayRef;
 
 class GlobPattern {
 public:
-  static Expected<GlobPattern> create(StringRef Pat);
+  static Expected<GlobPattern> create(StringRef Pat, bool ForceExactMatch = true);
   bool match(StringRef S) const;
 
 private:
   bool matchOne(ArrayRef<BitVector> Pat, StringRef S) const;
 
   // Parsed glob pattern.
   std::vector<BitVector> Tokens;
 
   // The following members are for optimization.
   Optional<StringRef> Exact;
   Optional<StringRef> Prefix;
   Optional<StringRef> Suffix;
 };
 }
 
 #endif // LLVM_SUPPORT_GLOB_PATTERN_H

llvm/lib/Support/GlobPattern.cpp

static Expected<BitVector> scan(StringRef &S, StringRef Original) {
   default:
     BitVector BV(256, false);
     BV[(uint8_t)S[0]] = true;
     S = S.substr(1);
     return BV;
   }
 }
 
-Expected<GlobPattern> GlobPattern::create(StringRef S) {
+Expected<GlobPattern> GlobPattern::create(StringRef S, bool ForceExactMatch) {
   GlobPattern Pat;
 
-  // S doesn't contain any metacharacter,
+  // S should be matched "as is" or doesn't contain any metacharacter,
   // so the regular string comparison should work.
-  if (!hasWildcard(S)) {
+  if (ForceExactMatch || !hasWildcard(S)) {
     Pat.Exact = S;
     return Pat;
   }
 
   // S is something like "foo*", and the "* is not escaped. We can use
   // startswith().
   if (S.endswith("*") && !S.endswith("\\*") && !hasWildcard(S.drop_back())) {
     Pat.Prefix = S.drop_back();