[ELF] - Linkerscript: reimplemented output sections constrains matching functionality.

[grimar]

Summary

Previously filtering that was used worked incorrectly.
For example for next script it would just remove both sections completely:

SECTIONS {
. = 0x1000;
.aaa : ONLY_IF_RW { *(.aaa.*) }
. = 0x2000;
.aaa : ONLY_IF_RO { *(.aaa.*) }
}

Patch fixes above issues and adds testcase showing the issue. Testcase is a subset of
FreeBSD script which has:

.eh_frame       : ONLY_IF_RO { KEEP (*(.eh_frame)) }
...
.eh_frame       : ONLY_IF_RW { KEEP (*(.eh_frame)) }

As a side result of a change, behavior of these commands is looks to be equal with ld now.

[grimar]

• Removed dead code and unrelated changes in testcase.
• Cosmetic changes.

[ruiu]

ELF/LinkerScript.cpp

271–272

I cannot infer P.first's class from this local context. Please create a temporary varaible

OutputSectionCommand *Cmd = P.first;

to make it obvious.

[grimar]

• Rebased.

ELF/LinkerScript.cpp

271–272

Gone after rebasing.

[ruiu]

LGTM

ELF/LinkerScript.cpp

263–264

Hoist RO and RW because they are actually constants.

bool RO = (Kind == ConstraintKind::ReadOnly);
bool RW = (Kind == ConstraintKind:: ReadWriter);
return !llvm::any_of(Sections, [=](InputSectionBase<ELFT> *Sec) {
  bool Writable = ...;
  return (RO && Writable) || (RW && !Writable);
});

ELF/LinkerScript.h

 
 private:
   std::vector<InputSectionBase<ELFT> *>
   getInputSections(const InputSectionDescription *);
 
   // "ScriptConfig" is a bit too long, so define a short name for it.
   ScriptConfiguration &Opt = *ScriptConfig;
 
-  void filter();
-
   int getSectionIndex(StringRef Name);
   std::vector<size_t> getPhdrIndices(StringRef SectionName);
   size_t getPhdrIndex(StringRef PhdrName);
 
   uintX_t Dot;
 };
 
 // Variable template is a C++14 feature, so we can't template
 // a global variable. Use a struct to workaround.
 template <class ELFT> struct Script { static LinkerScript<ELFT> *X; };
 template <class ELFT> LinkerScript<ELFT> *Script<ELFT>::X;
 
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/LinkerScript.cpp

 static std::function<bool(InputSectionBase<ELFT> *, InputSectionBase<ELFT> *)>
 getComparator(SortKind K) {
   if (K == SortByName)
     return compareName<ELFT>;
   return compareAlignment<ELFT>;
 }
 
 template <class ELFT>
+static bool matchConstraints(ArrayRef<InputSectionBase<ELFT> *> Sections,
+  ConstraintKind Kind) {
+  return !llvm::any_of(Sections, [=](InputSectionBase<ELFT> *Sec) {
+    bool Writable = Sec->getSectionHdr()->sh_flags & SHF_WRITE;
+    bool RO = (Kind == ConstraintKind::ReadOnly);
+    bool RW = (Kind == ConstraintKind::ReadWrite);
+    return (RO && Writable) || (RW && !Writable);
+  });
+}
+
+template <class ELFT>
 void LinkerScript<ELFT>::createSections(
     OutputSectionFactory<ELFT> &Factory) {
   OutputSectionBuilder<ELFT> Builder(Factory, OutputSections);
 
-  auto Add = [&](StringRef OutputName, const InputSectionDescription *Cmd) {
-    std::vector<InputSectionBase<ELFT> *> Sections = getInputSections(Cmd);
-    if (OutputName == "/DISCARD/") {
+  auto Add = [&](const OutputSectionCommand *OutCmd,
+                 const InputSectionDescription *InCmd) {
+    std::vector<InputSectionBase<ELFT> *> Sections = getInputSections(InCmd);
+    if (OutCmd->Name == "/DISCARD/") {
       for (InputSectionBase<ELFT> *S : Sections) {
         S->Live = false;
         reportDiscarded(S);
       }
       return;
     }
-    if (Cmd->SortInner)
+    if (!matchConstraints<ELFT>(Sections, OutCmd->Constraint))
+      return;
+
+    if (InCmd->SortInner)
       std::stable_sort(Sections.begin(), Sections.end(),
-                       getComparator<ELFT>(Cmd->SortInner));
-    if (Cmd->SortOuter)
+                       getComparator<ELFT>(InCmd->SortInner));
+    if (InCmd->SortOuter)
       std::stable_sort(Sections.begin(), Sections.end(),
-                       getComparator<ELFT>(Cmd->SortOuter));
+                       getComparator<ELFT>(InCmd->SortOuter));
     for (InputSectionBase<ELFT> *S : Sections)
-      Builder.addSection(OutputName, S);
+      Builder.addSection(OutCmd->Name, S);
   };
 
   for (const std::unique_ptr<BaseCommand> &Base1 : Opt.Commands)
     if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base1.get())) {
       for (const std::unique_ptr<BaseCommand> &Base2 : Cmd->Commands)
         if (auto *Assignment = dyn_cast<SymbolAssignment>(Base2.get()))
           Builder.addSymbol(Assignment);
         else
-          Add(Cmd->Name, cast<InputSectionDescription>(Base2.get()));
+          Add(Cmd, cast<InputSectionDescription>(Base2.get()));
 
       Builder.flushSection();
     } else if (auto *Cmd2 = dyn_cast<SymbolAssignment>(Base1.get())) {
       addSymbolToSymtab<ELFT>(*Cmd2, STV_DEFAULT);
     }
 
   // Add all other input sections, which are not listed in script.
   for (const std::unique_ptr<ObjectFile<ELFT>> &F :
        Symtab<ELFT>::X->getObjectFiles())
     for (InputSectionBase<ELFT> *S : F->getSections())
       if (!isDiscarded(S) && !S->OutSec)
         Builder.addSection(getOutputSectionName(S), S);
-
-  // Remove from the output all the sections which did not meet
-  // the optional constraints.
-  filter();
   Builder.finalize();
 }
 
-template <class R, class T>
-static inline void removeElementsIf(R &Range, const T &Pred) {
-  Range.erase(std::remove_if(Range.begin(), Range.end(), Pred), Range.end());
-}
-
-// Process ONLY_IF_RO and ONLY_IF_RW.
-template <class ELFT> void LinkerScript<ELFT>::filter() {
-  // In this loop, we remove output sections if they don't satisfy
-  // requested properties.
-  for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
-    auto *Cmd = dyn_cast<OutputSectionCommand>(Base.get());
-    if (!Cmd || Cmd->Name == "/DISCARD/")
-      continue;
-
-    if (Cmd->Constraint == ConstraintKind::NoConstraint)
-      continue;
-
-    bool RO = (Cmd->Constraint == ConstraintKind::ReadOnly);
-    bool RW = (Cmd->Constraint == ConstraintKind::ReadWrite);
-
-    removeElementsIf(*OutputSections, [&](OutputSectionBase<ELFT> *S) {
-      bool Writable = (S->getFlags() & SHF_WRITE);
-      return S->getName() == Cmd->Name &&
-             ((RO && Writable) || (RW && !Writable));
-    });
-  }
-}
-
 template <class ELFT> void assignOffsets(OutputSectionBase<ELFT> *Sec) {
   // Non-zero size means we have assigned offsets earlier in
   // OutputSectionBuilder<ELFT>::finalize
   auto *OutSec = dyn_cast<OutputSection<ELFT>>(Sec);
   if (Sec->getSize() || !OutSec)
     return;
 
   typedef typename ELFT::uint uintX_t;
   uintX_t Off = 0;
 
   for (InputSection<ELFT> *I : OutSec->Sections) {
     if (auto *L = dyn_cast<LayoutInputSection<ELFT>>(I)) {
       uintX_t Value = L->Cmd->Expression(Sec->getVA() + Off) - Sec->getVA();
       if (L->Cmd->Name == ".")
         Off = Value;
       else
         cast<DefinedSynthetic<ELFT>>(L->Cmd->Sym)->Value = Value;
     } else {
       Off = alignTo(Off, I->Alignment);
       I->OutSecOff = Off;
       Off += I->getSize();
     }
-    // Update section size inside for-loop, so that SIZEOF 
+    // Update section size inside for-loop, so that SIZEOF
     // works correctly in the case below:
     // .foo { *(.aaa) a = SIZEOF(.foo); *(.bbb) }
     Sec->setSize(Off);
   }
 }
 
 template <class ELFT> void LinkerScript<ELFT>::assignAddresses() {
   ArrayRef<OutputSectionBase<ELFT> *> Sections = *OutputSections;

test/ELF/linkerscript/linkerscript-multi-sections-constraint.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "SECTIONS { \
+# RUN:  . = 0x1000; .aaa : ONLY_IF_RO { *(.aaa.*) } \
+# RUN:  . = 0x2000; .aaa : ONLY_IF_RW { *(.aaa.*) } } " > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %t
+# RUN: llvm-objdump -section-headers %t1 | FileCheck %s
+
+# CHECK:      Sections:
+# CHECK-NEXT: Idx Name          Size      Address          Type
+# CHECK-NEXT:   0               00000000 0000000000000000
+# CHECK-NEXT:   1 .aaa          00000010 0000000000002000 DATA
+# CHECK-NEXT:   2 .text         00000001 0000000000002010 TEXT DATA
+# CHECK-NEXT:   3 .symtab       00000030 0000000000000000
+# CHECK-NEXT:   4 .shstrtab     00000026 0000000000000000
+# CHECK-NEXT:   5 .strtab       00000008 0000000000000000
+
+.global _start
+_start:
+ nop
+
+.section .aaa.1, "aw"
+.quad 1
+
+.section .aaa.2, "aw"
+.quad 1

test/ELF/linkerscript/linkerscript-sections-constraint.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 # RUN: echo "SECTIONS { \
 # RUN:  .writable : ONLY_IF_RW { *(.writable) } \
 # RUN:  .readable : ONLY_IF_RO { *(.readable) }}" > %t.script
 # RUN: ld.lld -o %t1 --script %t.script %t
 # RUN: llvm-objdump -section-headers %t1 | \
 # RUN:   FileCheck -check-prefix=BASE %s
 # BASE: Sections:
 # BASE-NEXT: Idx Name          Size      Address          Type
 # BASE-NEXT:   0               00000000 0000000000000000
 # BASE-NEXT:   1 .writable     00000004 0000000000000200 DATA
 # BASE-NEXT:   2 .readable     00000004 0000000000000204 DATA
 
 # RUN: echo "SECTIONS { \
-# RUN:  .writable : ONLY_IF_RO { *(.writable) } \
-# RUN:  .readable : ONLY_IF_RW { *(.readable) }}" > %t.script
-# RUN: ld.lld -o %t1 --script %t.script %t
-# RUN: llvm-objdump -section-headers %t1 | \
-# RUN:   FileCheck -check-prefix=NOSECTIONS %s
-# NOSECTIONS: Sections:
-# NOSECTIONS-NOT: .writable
-# NOSECTIONS-NOT: .readable
-
-# RUN: echo "SECTIONS { \
-# RUN:  .foo : ONLY_IF_RO { *(.foo.*) }}" > %t.script
-# RUN: ld.lld -o %t1 --script %t.script %t
-# RUN: llvm-objdump -section-headers %t1 | \
-# RUN:   FileCheck -check-prefix=NOSECTIONS2 %s
-# NOSECTIONS2: Sections:
-# NOSECTIONS2-NOT: .foo
+# RUN:  .foo : ONLY_IF_RO { *(.foo.*) } \
+# RUN:  .writable : ONLY_IF_RW { *(.writable) } \
+# RUN:  .readable : ONLY_IF_RO { *(.readable) }}" > %t2.script
+# RUN: ld.lld -o %t2 --script %t2.script %t
+# RUN: llvm-objdump -section-headers %t2 | \
+# RUN:   FileCheck -check-prefix=NO1 %s
+# NO1: Sections:
+# NO1-NEXT: Idx Name          Size      Address          Type
+# NO1-NEXT: 0               00000000 0000000000000000
+# NO1-NEXT: 1 .writable     00000004 0000000000000200 DATA
+# NO1-NEXT: 2 .readable     00000004 0000000000000204 DATA
+# NO1-NEXT: 3 .text         00000001 0000000000000208 TEXT DATA
+# NO1-NEXT: 4 .foo.2        00000004 0000000000000209 DATA
+# NO1-NEXT: 5 .foo.1        00000004 000000000000020d TEXT DATA
 
 .global _start
 _start:
   nop
 
 .section .writable, "aw"
 writable:
  .long 1