[ELF] - Fix "--symbol-ordering-file doesn't work with linker scripts"

[grimar]

Summary

This is PR33889,

Patch adds support of combination of linkerscript and
-symbol-ordering-file option.

If no sorting commands are present in script inside section declaration
and no --sort-section option specified, code uses sorting from ordering
file if any exist.

[grimar]

• Cosmetic changes.

[ruiu]

ELF/Config.h

80

SectionsOrder -> SectionOrder

I don't think it is a good idea to make this available everywhere. Please just keep SymbolOrderingFile like before. When you need SectionOrder, compute it and store it to a local variable.

ELF/LinkerScript.cpp

303

SymbolsOrder -> SymbolOrder

[grimar]

• Addressed review comments.

[ruiu]

This can be simplified, but I'll do that later. Please submit. LGTM

ELF/Config.h

 #include "llvm/Support/Endian.h"
 
 #include <vector>
 
 namespace lld {
 namespace elf {
 
 class InputFile;
+class SectionBase;
 struct Symbol;
 
 enum ELFKind {
   ELFNoneKind,
   ELF32LEKind,
   ELF32BEKind,
   ELF64LEKind,
   ELF64BEKind

 };
 
 // Structure for mapping renamed symbols
 struct RenamedSymbol {
   Symbol *Target;
   uint8_t OriginalBinding;
 };
 
+// For handling --symbol-ordering-file.
+struct SymbolOrdering {
+  // A map from section to it's priority.
+  llvm::DenseMap<SectionBase *, int> SectionsOrder;
+  std::vector<llvm::StringRef> Symbols;
+};
+
 // This struct contains the global configuration for the linker.
 // Most fields are direct mapping from the command line options
 // and such fields have the same name as the corresponding options.
 // Most fields are initialized by the driver.
 struct Configuration {
   InputFile *FirstElf = nullptr;
   uint8_t OSABI = 0;
   llvm::CachePruningPolicy ThinLTOCachePolicy;
struct Configuration {
   llvm::StringRef Sysroot;
   llvm::StringRef ThinLTOCacheDir;
   std::string Rpath;
   std::vector<VersionDefinition> VersionDefinitions;
   std::vector<llvm::StringRef> Argv;
   std::vector<llvm::StringRef> AuxiliaryList;
   std::vector<llvm::StringRef> FilterList;
   std::vector<llvm::StringRef> SearchPaths;
-  std::vector<llvm::StringRef> SymbolOrderingFile;
   std::vector<llvm::StringRef> Undefined;
   std::vector<SymbolVersion> VersionScriptGlobals;
   std::vector<SymbolVersion> VersionScriptLocals;
   std::vector<uint8_t> BuildIdVector;
   llvm::MapVector<Symbol *, RenamedSymbol> RenamedSymbols;
   bool AllowMultipleDefinition;
   bool AsNeeded = false;
   bool Bsymbolic;
struct Configuration {
   bool ZRelro;
   bool ZRodynamic;
   bool ZText;
   bool ExitEarly;
   bool ZWxneeded;
   DiscardPolicy Discard;
   SortSectionPolicy SortSection;
   StripPolicy Strip;
+  SymbolOrdering SymbolOrderingFile;
   UnresolvedPolicy UnresolvedSymbols;
   Target2Policy Target2;
   BuildIdKind BuildId = BuildIdKind::None;
   ELFKind EKind = ELFNoneKind;
   uint16_t DefaultSymbolVersion = llvm::ELF::VER_NDX_GLOBAL;
   uint16_t EMachine = llvm::ELF::EM_NONE;
   uint64_t ErrorLimit = 20;
   uint64_t ImageBase;

ELF/Driver.cpp

void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   if (Config->Omagic)
     Config->ZRelro = false;
 
   std::tie(Config->SysvHash, Config->GnuHash) = getHashStyle(Args);
   std::tie(Config->BuildId, Config->BuildIdVector) = getBuildId(Args);
 
   if (auto *Arg = Args.getLastArg(OPT_symbol_ordering_file))
     if (Optional<MemoryBufferRef> Buffer = readFile(Arg->getValue()))
-      Config->SymbolOrderingFile = getLines(*Buffer);
+      Config->SymbolOrderingFile.Symbols = getLines(*Buffer);
 
   // If --retain-symbol-file is used, we'll keep only the symbols listed in
   // the file and discard all others.
   if (auto *Arg = Args.getLastArg(OPT_retain_symbols_file)) {
     Config->DefaultSymbolVersion = VER_NDX_LOCAL;
     if (Optional<MemoryBufferRef> Buffer = readFile(Arg->getValue()))
       for (StringRef S : getLines(*Buffer))
         Config->VersionScriptGlobals.push_back(

ELF/LinkerScript.cpp

 }
 
 static void sortSections(InputSection **Begin, InputSection **End,
                          SortSectionPolicy K) {
   if (K != SortSectionPolicy::Default && K != SortSectionPolicy::None)
     std::stable_sort(Begin, End, getComparator(K));
 }
 
+static void sortByOrder(MutableArrayRef<InputSection *> In,
+                        std::function<int(InputSectionBase *S)> Order) {
+  typedef std::pair<unsigned, 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);
+
+  for (size_t I = 0; I < V.size(); ++I)
+    In[I] = V[I].second;
+}
+
+static void sortBySymbolsOrder(InputSection **Begin, InputSection **End) {
+  if (Config->SymbolOrderingFile.SectionsOrder.empty())
+    return;
+  MutableArrayRef<InputSection *> In(Begin, End - Begin);
+  sortByOrder(In, [&](InputSectionBase *S) {
+    return Config->SymbolOrderingFile.SectionsOrder.lookup(S);
+  });
+}
+
 // Compute and remember which sections the InputSectionDescription matches.
 std::vector<InputSection *>
 LinkerScript::computeInputSections(const InputSectionDescription *Cmd) {
   std::vector<InputSection *> Ret;
 
   // Collects all sections that satisfy constraints of Cmd.
   for (const SectionPattern &Pat : Cmd->SectionPatterns) {
     size_t SizeBefore = Ret.size();
for (const SectionPattern &Pat : Cmd->SectionPatterns) {
     // 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.
     InputSection **Begin = Ret.data() + SizeBefore;
     InputSection **End = Ret.data() + Ret.size();
+    if (Pat.SortOuter == SortSectionPolicy::Default &&
+        Config->SortSection == SortSectionPolicy::Default) {
+      sortBySymbolsOrder(Begin, End);
+      continue;
+    }
     if (Pat.SortOuter != SortSectionPolicy::None) {
       if (Pat.SortInner == SortSectionPolicy::Default)
         sortSections(Begin, End, Config->SortSection);
       else
         sortSections(Begin, End, Pat.SortInner);
       sortSections(Begin, End, Pat.SortOuter);
     }
   }

 OutputSectionCommand *LinkerScript::getCmd(OutputSection *Sec) const {
   auto I = SecToCommand.find(Sec);
   if (I == SecToCommand.end())
     return nullptr;
   return I->second;
 }
 
 void OutputSectionCommand::sort(std::function<int(InputSectionBase *S)> Order) {
-  typedef std::pair<unsigned, InputSection *> Pair;
-  auto Comp = [](const Pair &A, const Pair &B) { return A.first < B.first; };
-
-  std::vector<Pair> V;
   assert(Commands.size() == 1);
-  auto *ISD = cast<InputSectionDescription>(Commands[0]);
-  for (InputSection *S : ISD->Sections)
-    V.push_back({Order(S), S});
-  std::stable_sort(V.begin(), V.end(), Comp);
-  ISD->Sections.clear();
-  for (Pair &P : V)
-    ISD->Sections.push_back(P.second);
+  sortByOrder(cast<InputSectionDescription>(Commands[0])->Sections, Order);
 }
 
 // Returns true if S matches /Filename.?\.o$/.
 static bool isCrtBeginEnd(StringRef S, StringRef Filename) {
   if (!S.endswith(".o"))
     return false;
   S = S.drop_back(2);
   if (S.endswith(Filename))

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::run() {
 
   if (!Config->Relocatable)
     combineEhFrameSections<ELFT>();
 
   // We need to create some reserved symbols such as _end. Create them.
   if (!Config->Relocatable)
     addReservedSymbols();
 
+  if (!Config->SymbolOrderingFile.Symbols.empty())
+    scanSectionOrderFile<ELFT>();
+
   // Create output sections.
   if (Script->Opt.HasSections) {
     // If linker script contains SECTIONS commands, let it create sections.
     Script->processCommands(Factory);
 
     // Linker scripts may have left some input sections unassigned.
     // Assign such sections using the default rule.
     Script->addOrphanSections(Factory);
if (InX::DynSymTab)
     return;
   StringRef S = Config->IsRela ? "__rela_iplt_start" : "__rel_iplt_start";
   addOptionalRegular<ELFT>(S, In<ELFT>::RelaIplt, 0, STV_HIDDEN, STB_WEAK);
 
   S = Config->IsRela ? "__rela_iplt_end" : "__rel_iplt_end";
   addOptionalRegular<ELFT>(S, In<ELFT>::RelaIplt, -1, STV_HIDDEN, STB_WEAK);
 }
 
+template <class ELFT> static void scanSectionOrderFile() {
+  // Build a map from symbols to their priorities. Symbols that didn't
+  // appear in the symbol ordering file have the lowest priority 0.
+  // All explicitly mentioned symbols have negative (higher) priorities.
+  DenseMap<StringRef, int> SymbolOrder;
+  int Priority = -Config->SymbolOrderingFile.Symbols.size();
+  for (StringRef S : Config->SymbolOrderingFile.Symbols)
+    SymbolOrder.insert({S, Priority++});
+
+  // Build a map from sections to their priorities.
+  for (elf::ObjectFile<ELFT> *File : Symtab<ELFT>::X->getObjectFiles()) {
+    for (SymbolBody *Body : File->getSymbols()) {
+      auto *D = dyn_cast<DefinedRegular>(Body);
+      if (!D || !D->Section)
+        continue;
+      int &Priority = Config->SymbolOrderingFile.SectionsOrder[D->Section];
+      Priority = std::min(Priority, SymbolOrder.lookup(D->getName()));
+    }
+  }
+}
+
 // The linker is expected to define some symbols depending on
 // the linking result. This function defines such symbols.
 template <class ELFT> void Writer<ELFT>::addReservedSymbols() {
   if (Config->EMachine == EM_MIPS) {
     // Define _gp for MIPS. st_value of _gp symbol will be updated by Writer
     // so that it points to an absolute address which by default is relative
     // to GOT. Default offset is 0x7ff0.
     // See "Global Data Symbols" in Chapter 6 in the following document:

 // Sort input sections by the special rule for .ctors and .dtors.
 static void sortCtorsDtors(OutputSectionCommand *Cmd) {
   if (Cmd)
     Cmd->sortCtorsDtors();
 }
 
 // Sort input sections using the list provided by --symbol-ordering-file.
 template <class ELFT> static void sortBySymbolsOrder() {
-  if (Config->SymbolOrderingFile.empty())
+  if (Config->SymbolOrderingFile.SectionsOrder.empty())
     return;
-
-  // Build a map from symbols to their priorities. Symbols that didn't
-  // appear in the symbol ordering file have the lowest priority 0.
-  // All explicitly mentioned symbols have negative (higher) priorities.
-  DenseMap<StringRef, int> SymbolOrder;
-  int Priority = -Config->SymbolOrderingFile.size();
-  for (StringRef S : Config->SymbolOrderingFile)
-    SymbolOrder.insert({S, Priority++});
-
-  // Build a map from sections to their priorities.
-  DenseMap<SectionBase *, int> SectionOrder;
-  for (elf::ObjectFile<ELFT> *File : Symtab<ELFT>::X->getObjectFiles()) {
-    for (SymbolBody *Body : File->getSymbols()) {
-      auto *D = dyn_cast<DefinedRegular>(Body);
-      if (!D || !D->Section)
-        continue;
-      int &Priority = SectionOrder[D->Section];
-      Priority = std::min(Priority, SymbolOrder.lookup(D->getName()));
-    }
-  }
-
   // Sort sections by priority.
   for (BaseCommand *Base : Script->Opt.Commands)
     if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base))
-      Cmd->sort([&](InputSectionBase *S) { return SectionOrder.lookup(S); });
+      Cmd->sort([&](InputSectionBase *S) {
+        return Config->SymbolOrderingFile.SectionsOrder.lookup(S);
+      });
 }
 
 template <class ELFT>
 void Writer<ELFT>::forEachRelSec(std::function<void(InputSectionBase &)> Fn) {
   for (InputSectionBase *IS : InputSections) {
     if (!IS->Live)
       continue;
     // Scan all relocations. Each relocation goes through a series

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
+
+.section .foo,"ax",@progbits,unique,1
+_foo1:
+ .byte 0x11
+
+.section .foo,"ax",@progbits,unique,2
+_foo2:
+ .byte 0x22