Handle full path and archive library names in section placement rules

[dmikulin]

Summary

Currently lld uses base names of files to match against file patterns in SECTION rules. This patch extends it to use a fully specified file name as it appears in --trace output to match agains, i.e, "<path>/<objname>.o" or "<path>/<libname>.a(<objname>.o)".

I had to leave matching against a base name in place not to break existing functionality, specifically PR29115, which added matching of non-wildcard file names.

[ruiu]

lld/ELF/LinkerScript.cpp

187

Does this "or"-behavior match with GNU linker?

[dmikulin]

Replaced the use of toString with InputFile::getQualifiedName.
Removed base name matching to more closely align with GNU ld way of processing file patterns.
Changed tests cases to reflect source changes.

[ruiu]

lld/ELF/InputFiles.cpp

43

I wonder if this is a premature optimization. Can you avoid caching at least for now? You can move this code to filename.

[dmikulin]

lld/ELF/InputFiles.cpp

43

It's done this way in toString(), I was thinking of using the same cache, but wasn't sure how separate these two interfaces need to be.

[ruiu]

lld/ELF/InputFiles.cpp

43

Caching is not part of an interface. It's hidden from users of this function. So, I'm not worried about "separation" of two interfaces.

[dmikulin]

lld/ELF/InputFiles.cpp

43

Are you saying that caching is fine and that I can use the same cache for both toString() and getQualifiedName() ?
Re-creating the same string on every call seems a bit wasteful... I have a modified patch with no caching if that's what you feel is a better alternative.

[ruiu]

lld/ELF/InputFiles.cpp

43

Doing the same thing but without caching is what I want this moment, because I don't want to optimize the code unless it is proved to be needed. Also, you can remove this function entirely and instead add code to filename function in LinkerScript.cpp.

[dmikulin]

Removed string caching.

[ruiu]

lld/ELF/LinkerScript.cpp

180

I'd flip it and return early to reduce indentation.

if (!S->File)
  return "";

183

No else since the last if ends with return.

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

65–67

I applied this patch to verify that this behavior matches ld.bfd. What I found is that this doesn't seem to match with ld.bfd, as it produces FIRSTSECOND instead of SECONDFIRST if I replace ld.lld with ld.bfd.

[dmikulin]

lld/ELF/LinkerScript.cpp

183

I'll update the patch

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

65–67

As far as I can tell, bfd linker does not match against archive library names.
gold produces SECONDFIRST, which makes more sense.

[ruiu]

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

65–67

OK, but I believe ld.bfd is still the gold standard and we want to make our linker compatible with ld.bfd instead of ld.gold.

[dmikulin]

Addressed review comments

[dmikulin]

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

65–67

But what ld.bfd does is not very logically consistent: it matches against directory names when a .o is specified on the link line, but only uses base name if the same file appears in a .a. It's quite possibly unintentional and can be treated as a bug in ld.bfd.

[ruiu]

LGTM

lld/ELF/LinkerScript.cpp

183

Remove this blank line.

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

65–67

If I replace ld.lld with ld.gold, all tests indeed pass. So this is not a good situation; GNU linkers are not compatible each other, and we have to choose one of them. In such situation like this, we usually copy bfd's behavior, but as you said, gold's behavior seems more logical.

This feature doesn't seem to have been tested well. When I fed "SECTIONS { .foo : { *lib2* *lib1* } }" to gold, it crashed. Wow.

Perhaps the fact that ld.gold and ld.bfd are not compatible means that this incompatibility is not important. So I think I'm fine with gold's behavior.

[MikeStump]

The GNU ld documentation says that patterns are archive:file in:

https://sourceware.org/binutils/docs/ld/Input-Section-Basics.html#Input-Section-Basics

and this patch doesn't use the : spelling, so archive:file doesn't work. If archive:file is used, then one could use archive:file in the patterns as documented.

lld/ELF/InputFiles.h

enum Kind {
     ArchiveKind,
     BitcodeKind,
     BinaryKind,
   };
 
   Kind kind() const { return FileKind; }
 
   StringRef getName() const { return MB.getBufferIdentifier(); }
+  StringRef getQualifiedName();
   MemoryBufferRef MB;
 
   // Returns sections. It is a runtime error to call this function
   // on files that don't have the notion of sections.
   ArrayRef<InputSectionBase *> getSections() const {
     assert(FileKind == ObjKind || FileKind == BinaryKind);
     return Sections;
   }

 
 protected:
   InputFile(Kind K, MemoryBufferRef M);
   std::vector<InputSectionBase *> Sections;
   std::vector<SymbolBody *> Symbols;
 
 private:
   const Kind FileKind;
+  mutable std::string FileNameCache;
 };
 
 template <typename ELFT> class ELFFileBase : public InputFile {
 public:
   typedef typename ELFT::Shdr Elf_Shdr;
   typedef typename ELFT::Sym Elf_Sym;
   typedef typename ELFT::Word Elf_Word;
   typedef typename ELFT::SymRange Elf_Sym_Range;

lld/ELF/InputFiles.cpp

 
 using namespace lld;
 using namespace lld::elf;
 
 TarWriter *elf::Tar;
 
 InputFile::InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {}
 
+// Returns "foo.a(bar.o)" or "baz.o".
+StringRef InputFile::getQualifiedName() {
+  if (FileNameCache.empty()) {
+    if (ArchiveName.empty())
+      FileNameCache = getName();
+    else
+      FileNameCache = (ArchiveName + "(" + getName() + ")").str();
+  }
+  return FileNameCache;
+}
+
 Optional<MemoryBufferRef> elf::readFile(StringRef Path) {
   // The --chroot option changes our virtual root directory.
   // This is useful when you are dealing with files created by --reproduce.
   if (!Config->Chroot.empty() && Path.startswith("/"))
     Path = Saver.save(Config->Chroot + Path);
 
   log(Path);
 

lld/ELF/LinkerScript.cpp

 bool AssertCommand::classof(const BaseCommand *C) {
   return C->Kind == AssertKind;
 }
 
 bool BytesDataCommand::classof(const BaseCommand *C) {
   return C->Kind == BytesDataKind;
 }
 
-static StringRef basename(InputSectionBase *S) {
+static StringRef filename(InputSectionBase *S) {
   if (S->File)
-    return sys::path::filename(S->File->getName());
+    return S->File->getQualifiedName();
   return "";
 }
 
 bool LinkerScript::shouldKeep(InputSectionBase *S) {
   for (InputSectionDescription *ID : Opt.KeptSections)
-    if (ID->FilePat.match(basename(S)))
+    if (ID->FilePat.match(filename(S)))
       for (SectionPattern &P : ID->SectionPatterns)
         if (P.SectionPat.match(S->Name))
           return true;
   return false;
 }
 
 // A helper function for the SORT() command.
 static std::function<bool(InputSectionBase *, InputSectionBase *)>
 getComparator(SortSectionPolicy K) {
   switch (K) {
for (InputSectionBase *Sec : InputSections) {
       }
 
       // For -emit-relocs we have to ignore entries like
       //   .rela.dyn : { *(.rela.data) }
       // which are common because they are in the default bfd script.
       if (Sec->Type == SHT_REL || Sec->Type == SHT_RELA)
         continue;
 
-      StringRef Filename = basename(Sec);
+      StringRef Filename = filename(Sec);
       if (!Cmd->FilePat.match(Filename) ||
           Pat.ExcludedFilePat.match(Filename) ||
           !Pat.SectionPat.match(Sec->Name))
         continue;
 
       Ret.push_back(cast<InputSection>(Sec));
       Sec->Assigned = true;
     }

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

 # RUN: llvm-objdump -s %t3 | FileCheck --check-prefix=FIRSTSECOND %s
 
 # RUN: echo "SECTIONS { .foo : { \
 # RUN:   *second.o(.foo) \
 # RUN:   *first.o(.foo) } }" > %t4.script
 # RUN: ld.lld -o %t4 --script %t4.script %tfirst.o %tsecond.o
 # RUN: llvm-objdump -s %t4 | FileCheck --check-prefix=SECONDFIRST %s
 
-# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %T/filename-spec1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o filename-spec1.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux \
-# RUN:   %p/Inputs/filename-spec.s -o %T/filename-spec2.o
+# RUN:   %p/Inputs/filename-spec.s -o filename-spec2.o
 
 # RUN: echo "SECTIONS { .foo : { \
 # RUN:   filename-spec2.o(.foo) \
 # RUN:   filename-spec1.o(.foo) } }" > %t5.script
 # RUN: ld.lld -o %t5 --script %t5.script \
-# RUN:   %T/filename-spec1.o %T/filename-spec2.o
+# RUN:   filename-spec1.o filename-spec2.o
 # RUN: llvm-objdump -s %t5 | FileCheck --check-prefix=SECONDFIRST %s
 
 # RUN: echo "SECTIONS { .foo : { \
 # RUN:   filename-spec1.o(.foo) \
 # RUN:   filename-spec2.o(.foo) } }" > %t6.script
 # RUN: ld.lld -o %t6 --script %t6.script \
-# RUN:   %T/filename-spec1.o %T/filename-spec2.o
+# RUN:   filename-spec1.o filename-spec2.o
 # RUN: llvm-objdump -s %t6 | FileCheck --check-prefix=FIRSTSECOND %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
+# RUN: llvm-ar rsc %t.testdir1/lib1.a %t.testdir1/filename-spec1.o
+# RUN: llvm-ar rsc %t.testdir2/lib2.a %t.testdir2/filename-spec2.o
+
+# Verify matching of archive library names.
+# RUN: echo "SECTIONS { .foo : { \
+# RUN:   *lib2*(.foo) \
+# RUN:   *lib1*(.foo) } }" > %t7.script
+# RUN: ld.lld -o %t7 --script %t7.script --whole-archive \
+# RUN:   %t.testdir1/lib1.a %t.testdir2/lib2.a
+# RUN: llvm-objdump -s %t7 | FileCheck --check-prefix=SECONDFIRST %s
+
+# Verify matching directories.
+# RUN: echo "SECTIONS { .foo : { \
+# RUN:   *testdir2*(.foo) \
+# RUN:   *testdir1*(.foo) } }" > %t8.script
+# RUN: ld.lld -o %t8 --script %t8.script --whole-archive \
+# RUN:   %t.testdir1/lib1.a %t.testdir2/lib2.a
+# RUN: llvm-objdump -s %t8 | FileCheck --check-prefix=SECONDFIRST %s
+
+# Verify matching of archive library names in KEEP.
+# RUN: echo "SECTIONS { .foo : { \
+# RUN:   KEEP(*lib2*(.foo)) \
+# RUN:   KEEP(*lib1*(.foo)) } }" > %t9.script
+# RUN: ld.lld -o %t9 --script %t9.script --whole-archive \
+# RUN:   %t.testdir1/lib1.a %t.testdir2/lib2.a
+# RUN: llvm-objdump -s %t9 | FileCheck --check-prefix=SECONDFIRST %s
+
+# Verify matching directories in KEEP.
+# RUN: echo "SECTIONS { .foo : { \
+# RUN:   KEEP(*testdir2*(.foo)) \
+# RUN:   KEEP(*testdir1*(.foo)) } }" > %t10.script
+# RUN: ld.lld -o %t10 --script %t10.script --whole-archive \
+# RUN:   %t.testdir1/lib1.a %t.testdir2/lib2.a
+# RUN: llvm-objdump -s %t10 | FileCheck --check-prefix=SECONDFIRST %s
+
 .global _start
 _start:
  nop
 
 .section .foo,"a"
  .quad 1