[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

11–1

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.

12–13

Pass InputSectionDescription instead of the two arguments.

13

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.

ELF/LinkerScript.h

2

I'd name FilePattern and rename Patterns SectionPatterns.

[grimar]

• Addressed review comments + refactored.

ELF/LinkerScript.cpp

11–1

Now it includes KEEP.

12–13

Done.

13

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.

ELF/LinkerScript.h

2

Done.

[ruiu]

ELF/LinkerScript.cpp

96

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

108–109

Flip the condition and remove continue.

451

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.

756–757

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

Done.

108–109

Done.

451

Fixed.

756–757

Done.

[ruiu]

LGTM

ELF/LinkerScript.cpp

700

Use auto.

708–710

} else {
  readInpu... 
}

750–752

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

ELF/LinkerScript.h

 //===- LinkerScript.h -------------------------------------------*- C++ -*-===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_ELF_LINKER_SCRIPT_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;

ELF/LinkerScript.cpp

 //===- LinkerScript.cpp ---------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains the parser/evaluator of the linker script.
 // It parses a linker script and write the result to Config or ScriptConfig
 // objects.
 //
 // If SECTIONS command is used, a ScriptConfig contains an AST
 // of the command which will later be consumed by createSections() and
 // assignAddresses().
 //
 //===----------------------------------------------------------------------===//
 
 #include "LinkerScript.h"
 #include "Config.h"
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() {
+  std::unique_ptr<InputSectionDescription> 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::readSort() {
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") {
+      std::unique_ptr<InputSectionDescription> InCmd =
+          readInputSectionDescription();
+      Cmd->Commands.push_back(std::move(InCmd));
+      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);
     }

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

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

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