Sort orphan section if --symbol-ordering-file is given

[• espindola]

Summary

This changes the interaction of --symbol-ordering-file with linker scripts, but since we created --symbol-ordering-file I think this is reasonable. The new code is simpler and the behavior less surprising.

[grimar]

LGTM with nits.

ELF/LinkerScript.cpp

252

5th comment's line is outdated after the change.

ELF/Writer.cpp

1021

Its a bit inaccurate now, as sorting with --symbol-ordering-file is always applied with this change.

[jhenderson]

Not sure if this is what motivated your change, but I think this will allow me to make some improvements to D42475.

Personally, I'm not convinced that we should ignore the symbol-ordering-file for non-orphaned sections, since it's more likely, I feel, for people to use linker scripts "by default" than the ordering file, but this is not really part of this change.

ELF/LinkerScript.cpp

256

Order isn't used here after this change, so it can be removed from this function plus a number of functions in the call stack, which in turn allows us to remove the buildSectionOrder call in processSectionCommands.

[• espindola]

r323779

ELF/LinkerScript.cpp

 // line option is respected even if a SORT command is given, the exact
 // behavior we have here is a bit complicated. Here are the rules.
 //
 // 1. If two SORT commands are given, --sort-section is ignored.
 // 2. If one SORT command is given, and if it is not SORT_NONE,
 //    --sort-section is handled as an inner SORT command.
 // 3. If one SORT command is given, and if it is SORT_NONE, don't sort.
 // 4. If no SORT command is given, sort according to --sort-section.
 // 5. If no SORT commands are given and --sort-section is not specified,
 //    apply sorting provided by --symbol-ordering-file if any exist.
 static void sortInputSections(
     MutableArrayRef<InputSection *> Vec, const SectionPattern &Pat,
     const DenseMap<SectionBase *, int> &Order) {
   if (Pat.SortOuter == SortSectionPolicy::None)
     return;
 
-  if (Pat.SortOuter == SortSectionPolicy::Default &&
-      Config->SortSection == SortSectionPolicy::Default) {
-    // If -symbol-ordering-file was given, sort accordingly.
-    // Usually, Order is empty.
-    if (!Order.empty())
-      sortByOrder(Vec, [&](InputSectionBase *S) { return Order.lookup(S); });
-    return;
-  }
-
   if (Pat.SortInner == SortSectionPolicy::Default)
     sortSections(Vec, Config->SortSection);
   else
     sortSections(Vec, Pat.SortInner);
   sortSections(Vec, Pat.SortOuter);
 }
 
 // Compute and remember which sections the InputSectionDescription matches.

ELF/OutputSections.h

 
 } // namespace elf
 } // namespace lld
 
 namespace lld {
 namespace elf {
 
 uint64_t getHeaderSize();
-void sortByOrder(llvm::MutableArrayRef<InputSection *> In,
-                 std::function<int(InputSectionBase *S)> Order);
 
 extern std::vector<OutputSection *> OutputSections;
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/OutputSections.cpp

if (!IS->Assigned) {
     if (SectionCommands.empty() ||
         !isa<InputSectionDescription>(SectionCommands.back()))
       SectionCommands.push_back(make<InputSectionDescription>(""));
     auto *ISD = cast<InputSectionDescription>(SectionCommands.back());
     ISD->Sections.push_back(IS);
   }
 }
 
-void elf::sortByOrder(MutableArrayRef<InputSection *> In,
+static void sortByOrder(MutableArrayRef<InputSection *> In,
                         std::function<int(InputSectionBase *S)> Order) {
   typedef std::pair<int, InputSection *> Pair;
   auto Comp = [](const Pair &A, const Pair &B) { return A.first < B.first; };
 
   std::vector<Pair> V;
   for (InputSection *S : In)
     V.push_back({Order(S), S});
   std::stable_sort(V.begin(), V.end(), Comp);
 

ELF/Writer.cpp

if (NextSec == E)
     return E;
 
   while (I != E && shouldSkip(*I))
     ++I;
   return I;
 }
 
 // If no layout was provided by linker script, we want to apply default
 // sorting for special input sections and handle --symbol-ordering-file.
 template <class ELFT> void Writer<ELFT>::sortInputSections() {
-  assert(!Script->HasSectionsCommand);
-
   // Sort input sections by priority using the list provided
   // by --symbol-ordering-file.
   DenseMap<SectionBase *, int> Order = buildSectionOrder();
   if (!Order.empty())
     for (BaseCommand *Base : Script->SectionCommands)
       if (auto *Sec = dyn_cast<OutputSection>(Base))
         if (Sec->Live)
           Sec->sort([&](InputSectionBase *S) { return Order.lookup(S); });
 
+  if (Script->HasSectionsCommand)
+    return;
+
   // Sort input sections by section name suffixes for
   // __attribute__((init_priority(N))).
   if (OutputSection *Sec = findSection(".init_array"))
     Sec->sortInitFini();
   if (OutputSection *Sec = findSection(".fini_array"))
     Sec->sortInitFini();
 
   // Sort input sections by the special rule for .ctors and .dtors.
template <class ELFT> void Writer<ELFT>::sortSections() {
   // relative order for SHF_LINK_ORDER sections.
   if (Config->Relocatable)
     return;
 
   for (BaseCommand *Base : Script->SectionCommands)
     if (auto *Sec = dyn_cast<OutputSection>(Base))
       Sec->SortRank = getSectionRank(Sec);
 
-  if (!Script->HasSectionsCommand) {
-    sortInputSections();
 
+  if (!Script->HasSectionsCommand) {
     // We know that all the OutputSections are contiguous in this case.
     auto E = Script->SectionCommands.end();
     auto I = Script->SectionCommands.begin();
     auto IsSection = [](BaseCommand *Base) { return isa<OutputSection>(Base); };
     I = std::find_if(I, E, IsSection);
     E = std::find_if(llvm::make_reverse_iterator(E),
                      llvm::make_reverse_iterator(I), IsSection)
             .base();

test/ELF/linkerscript/symbol-ordering-file.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: echo "SECTIONS { .foo : { *(.foo) } }" > %t.script
 
 # RUN: ld.lld %t.o --script %t.script -o %t.out
 # RUN: llvm-objdump -s %t.out| FileCheck %s --check-prefix=BEFORE
 # BEFORE:      Contents of section .foo:
 # BEFORE-NEXT: 1122
 
 # RUN: echo "_foo2" > %t.ord
 # RUN: echo "_foo1" >> %t.ord
 # RUN: ld.lld --symbol-ordering-file %t.ord %t.o --script %t.script -o %t2.out
 # RUN: llvm-objdump -s %t2.out| FileCheck %s --check-prefix=AFTER
 # AFTER:      Contents of section .foo:
 # AFTER-NEXT: 2211
 
+# RUN: echo "SECTIONS { .text : { *(.text) } }" > %t2.script
+# RUN: ld.lld --symbol-ordering-file %t.ord %t.o --script %t2.script -o %t3.out
+# RUN: llvm-objdump -s %t3.out| FileCheck %s --check-prefix=AFTER
+
 .section .foo,"ax",@progbits,unique,1
 _foo1:
  .byte 0x11
 
 .section .foo,"ax",@progbits,unique,2
 _foo2:
  .byte 0x22