[ELF] - Linkerscript: implemented filename specification.

[grimar]

Summary

Scripts can contain something like:
KEEP (*crtbegin.o(.ctors))

What means that "*crtbegin.o" is a wildcard of file to take the sections from.
This is some kind of opposite to EXCLUDE_FILE and used in FreeBSD script:
https://svnweb.freebsd.org/base/head/sys/conf/ldscript.amd64?revision=284870&view=markup#l122

Patch implements this.

Patch partially relies on changes from D22749 that changed the way of parsing
LS script a bit. So I would suggest to start reviewing from D22749 and
then I can rebase this one.

[ruiu]

ELF/LinkerScript.cpp

95–96 ↗

(On Diff #65704)

Pass InputSectionDescription instead of the two arguments.

450 ↗

(On Diff #65704)

This name seems odd. Everything is named readXXX where XXX is a name which the function is expected to read. This function name is not consistent.

So this function reads XXX where XXX appears

*(XXX)
KEEP(*(XXX))
EXCLUDED_FILES(foo.o, XXX)

right? Maybe I'd call it readInputFilePattern.

705 ↗

(On Diff #65704)

This name is no longer correct. Input section description is everything that is described in https://sourceware.org/binutils/docs/ld/Input-Section.html. So it includes KEEP. You got to give it a new name.

ELF/LinkerScript.h

86 ↗

(On Diff #65704)

I'd name FilePattern and rename Patterns SectionPatterns.

[grimar]

• Addressed review comments + refactored.

ELF/LinkerScript.cpp

95–96 ↗

(On Diff #65704)

Done.

450 ↗

(On Diff #65704)

No. It read everything except KEEP which I thought would be reasonable to leave at higher level as everyhing in input section description can be surrounded by keep, so readInputFilePattern would not work I think.
I refactored that place.

705 ↗

(On Diff #65704)

Now it includes KEEP.

ELF/LinkerScript.h

86 ↗

(On Diff #65704)

Done.

[ruiu]

ELF/LinkerScript.cpp

96 ↗

(On Diff #65891)

I think we used filename as one word in other places, so Filename.

109–111 ↗

(On Diff #65891)

Flip the condition and remove continue.

447 ↗

(On Diff #65891)

This looks weird. If this function reads an input section description, why do we have to pass an input section description to the function? If we can pass an input section description to the function, it implies that we have already read an input section description.

742–744 ↗

(On Diff #65891)

Huh, so you created an empty input section description and pass it to the function. You want to make readInputSectionDescription to return a new InputSectionDescription instead.

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

96 ↗

(On Diff #65891)

Done.

109–111 ↗

(On Diff #65891)

Done.

447 ↗

(On Diff #65891)

Fixed.

742–744 ↗

(On Diff #65891)

Done.

[ruiu]

LGTM

ELF/LinkerScript.cpp

700 ↗

(On Diff #66014)

Use auto.

708–710 ↗

(On Diff #66014)

} else {
  readInpu... 
}

750–752 ↗

(On Diff #66014)

Cmd->Command.push_back(readInputSectionDescription());

lld/trunk/ELF/LinkerScript.h

struct OutputSectionCommand : BaseCommand {
   std::vector<StringRef> Phdrs;
   std::vector<uint8_t> Filler;
   ConstraintKind Constraint = ConstraintKind::NoConstraint;
 };
 
 struct InputSectionDescription : BaseCommand {
   InputSectionDescription() : BaseCommand(InputSectionKind) {}
   static bool classof(const BaseCommand *C);
+  StringRef FilePattern;
   std::vector<StringRef> ExcludedFiles;
-  std::vector<StringRef> Patterns;
+  std::vector<StringRef> SectionPatterns;
 };
 
 struct PhdrsCommand {
   StringRef Name;
   unsigned Type;
   bool HasFilehdr;
   bool HasPhdrs;
   unsigned Flags;

lld/trunk/ELF/LinkerScript.cpp

for (const std::unique_ptr<BaseCommand> &Base1 : Opt.Commands)
     if (auto *Cmd1 = dyn_cast<OutputSectionCommand>(Base1.get()))
       for (const std::unique_ptr<BaseCommand> &Base2 : Cmd1->Commands)
         if (auto *Cmd2 = dyn_cast<InputSectionDescription>(Base2.get()))
           Ret.emplace_back(Cmd1->Name, Cmd2);
 
   return Ret;
 }
 
+static bool fileMatches(const InputSectionDescription *Desc,
+                        StringRef Filename) {
+  if (!globMatch(Desc->FilePattern, Filename))
+    return false;
+  return Desc->ExcludedFiles.empty() || !match(Desc->ExcludedFiles, Filename);
+}
+
 // Returns input sections filtered by given glob patterns.
 template <class ELFT>
 std::vector<InputSectionBase<ELFT> *>
 LinkerScript<ELFT>::getInputSections(const InputSectionDescription *I) {
-  ArrayRef<StringRef> Patterns = I->Patterns;
-  ArrayRef<StringRef> ExcludedFiles = I->ExcludedFiles;
+  ArrayRef<StringRef> Patterns = I->SectionPatterns;
   std::vector<InputSectionBase<ELFT> *> Ret;
   for (const std::unique_ptr<ObjectFile<ELFT>> &F :
-       Symtab<ELFT>::X->getObjectFiles())
+       Symtab<ELFT>::X->getObjectFiles()) {
+    if (fileMatches(I, sys::path::filename(F->getName())))
       for (InputSectionBase<ELFT> *S : F->getSections())
-      if (!isDiscarded(S) && !S->OutSec && match(Patterns, S->getSectionName()))
-        if (ExcludedFiles.empty() ||
-            !match(ExcludedFiles, sys::path::filename(F->getName())))
+        if (!isDiscarded(S) && !S->OutSec &&
+            match(Patterns, S->getSectionName()))
           Ret.push_back(S);
+  }
 
   if ((llvm::find(Patterns, "COMMON") != Patterns.end()))
     Ret.push_back(CommonInputSection<ELFT>::X);
 
   return Ret;
 }
 
 template <class ELFT>
public:
   void run();
 
 private:
   void addFile(StringRef Path);
 
   void readAsNeeded();
   void readEntry();
   void readExtern();
-  std::unique_ptr<InputSectionDescription> readFilePattern();
   void readGroup();
-  void readKeep(OutputSectionCommand *Cmd);
   void readInclude();
   void readNothing() {}
   void readOutput();
   void readOutputArch();
   void readOutputFormat();
   void readPhdrs();
   void readSearchDir();
   void readSections();
 
   SymbolAssignment *readAssignment(StringRef Name);
   void readOutputSectionDescription(StringRef OutSec);
   std::vector<StringRef> readOutputSectionPhdrs();
+  std::unique_ptr<InputSectionDescription> readInputSectionDescription();
+  void readInputSectionRules(InputSectionDescription *InCmd, bool Keep);
   unsigned readPhdrType();
   void readProvide(bool Hidden);
   void readAlign(OutputSectionCommand *Cmd);
   void readSort();
 
   Expr readExpr();
   Expr readExpr1(Expr Lhs, int MinPrec);
   Expr readPrimary();
return StringSwitch<int>(Op)
       .Case(">=", 2)
       .Case("<=", 2)
       .Case("==", 2)
       .Case("!=", 2)
       .Case("&", 1)
       .Default(-1);
 }
 
-std::unique_ptr<InputSectionDescription> ScriptParser::readFilePattern() {
-  expect("*");
+void ScriptParser::readInputSectionRules(InputSectionDescription *InCmd, bool Keep) {
+  InCmd->FilePattern = next();
   expect("(");
 
-  auto InCmd = llvm::make_unique<InputSectionDescription>();
-
   if (skip("EXCLUDE_FILE")) {
     expect("(");
     while (!Error && !skip(")"))
       InCmd->ExcludedFiles.push_back(next());
-    InCmd->Patterns.push_back(next());
-    expect(")");
-  } else {
-    while (!Error && !skip(")"))
-      InCmd->Patterns.push_back(next());
   }
-  return InCmd;
+
+  while (!Error && !skip(")")) {
+    if (Keep)
+      Opt.KeptSections.push_back(peek());
+    InCmd->SectionPatterns.push_back(next());
+  }
 }
 
-void ScriptParser::readKeep(OutputSectionCommand *Cmd) {
+std::unique_ptr<InputSectionDescription>
+ScriptParser::readInputSectionDescription() {
+  auto InCmd = std::make_unique<InputSectionDescription>();
+
+  // Input section wildcard can be surrounded by KEEP.
+  // https://sourceware.org/binutils/docs/ld/Input-Section-Keep.html#Input-Section-Keep
+  if (skip("KEEP")) {
     expect("(");
-  std::unique_ptr<InputSectionDescription> InCmd = readFilePattern();
-  Opt.KeptSections.insert(Opt.KeptSections.end(), InCmd->Patterns.begin(),
-                          InCmd->Patterns.end());
-  Cmd->Commands.push_back(std::move(InCmd));
+    readInputSectionRules(InCmd.get(), true);
     expect(")");
+  } else {
+    readInputSectionRules(InCmd.get(), false);
+  }
+
+  return InCmd;
 }
 
 void ScriptParser::readAlign(OutputSectionCommand *Cmd) {
   expect("(");
   Cmd->AlignExpr = readExpr();
   expect(")");
 }
 
void ScriptParser::readOutputSectionDescription(StringRef OutSec) {
   // Parse constraints.
   if (skip("ONLY_IF_RO"))
     Cmd->Constraint = ConstraintKind::ReadOnly;
   if (skip("ONLY_IF_RW"))
     Cmd->Constraint = ConstraintKind::ReadWrite;
   expect("{");
 
   while (!Error && !skip("}")) {
+    if ((!peek().empty() && peek()[0] == '*') || peek() == "KEEP") {
+      Cmd->Commands.push_back(readInputSectionDescription());
+      continue;
+    }
+
     StringRef Tok = next();
-    if (Tok == "*") {
-      auto *InCmd = new InputSectionDescription();
-      Cmd->Commands.emplace_back(InCmd);
-      expect("(");
-      while (!Error && !skip(")"))
-        InCmd->Patterns.push_back(next());
-    } else if (Tok == "KEEP") {
-      readKeep(Cmd);
-    } else if (Tok == "PROVIDE") {
+    if (Tok == "PROVIDE") {
       readProvide(false);
     } else if (Tok == "PROVIDE_HIDDEN") {
       readProvide(true);
     } else if (Tok == "SORT") {
       readSort();
     } else {
       setError("unknown command " + Tok);
     }

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

+.section .foo,"a"
+  .quad 0x11

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

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %tfirst.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux \
+# RUN:   %p/Inputs/linkerscript-filename-spec.s -o %tsecond.o
+
+# RUN: echo "SECTIONS { .foo : { \
+# RUN:   KEEP(*first.o(.foo)) \
+# RUN:   KEEP(*second.o(.foo)) } }" > %t1.script
+# RUN: ld.lld -o %t1 --script %t1.script %tfirst.o %tsecond.o
+# RUN: llvm-objdump -s %t1 | FileCheck --check-prefix=FIRSTSECOND %s
+# FIRSTSECOND:      Contents of section .foo:
+# FIRSTSECOND-NEXT:   0120 01000000 00000000 11000000 00000000
+
+# RUN: echo "SECTIONS { .foo : { \
+# RUN:   KEEP(*second.o(.foo)) \
+# RUN:   KEEP(*first.o(.foo)) } }" > %t2.script
+# RUN: ld.lld -o %t2 --script %t2.script %tfirst.o %tsecond.o
+# RUN: llvm-objdump -s %t2 | FileCheck --check-prefix=SECONDFIRST %s
+# SECONDFIRST:      Contents of section .foo:
+# SECONDFIRST-NEXT:   0120 11000000 00000000 01000000 00000000
+
+## Now the same tests but without KEEP. Checking that file name inside
+## KEEP is parsed fine.
+# RUN: echo "SECTIONS { .foo : { \
+# RUN:   *first.o(.foo) \
+# RUN:   *second.o(.foo) } }" > %t3.script
+# RUN: ld.lld -o %t3 --script %t3.script %tfirst.o %tsecond.o
+# 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
+
+.global _start
+_start:
+ nop
+
+.section .foo,"a"
+ .quad 1