[ELF] - Simplify output section creation.

[grimar]

Summary

Our code that creates output sections is very similar to code
that handles orphans. Earlier we did integration of LLD scripted/non-scripted logic
so that even fabricate script commands in createSections() and with fabricateDefaultCommands
when no layout specified in script. And I think when there is no SECTION
commands given, all sections are technically orphans, but now we handle script orphans
sections and regular "orphans" sections for non-scripted case differently,
though we can handle them at one place.

That IMO makes code cleaner and simpler. Patch shows the change I am suggesting.

[ruiu]

ELF/LinkerScript.cpp

464–468 ↗

(On Diff #119457)

I do not understand this particular change, but I guess this will be removed by your other change.

487 ↗

(On Diff #119457)

any others -> otheers

490–492 ↗

(On Diff #119457)

I think

if (HasSectionsCommand)
  SectionCommands.insert(SectionsCommand.begin(), V.begin(), V.end());
else
  SectionCommands.insert(SectionsCommand.end(), V.begin(), V.end());

is more straightforward.

ELF/Writer.cpp

162 ↗

(On Diff #119457)

is given

163 ↗

(On Diff #119457)

, -> .

"Otherwise ..." part of the comment doesn't seem to describe this code. Please move it to the right place.

168–170 ↗

(On Diff #119457)

// Linker scripts controls how input sections are assigned to output sections. Input sections that were not handled by scripts are called "orphans", and they are assigned to output sections by the default rule. Process that.

173–176 ↗

(On Diff #119457)

Hmm, at this point, I started wondering if the changes to this function actually improves the function. Now, it isn't easy to point out which code is executed when linker script is given/not givne.

[grimar]

ELF/Writer.cpp

173–176 ↗

(On Diff #119457)

It is not good place to apply default sorting I think.
I prepared patch to cleanup this place: D39371.

It should relax this place if we land it, what do you think ?

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

464–468 ↗

(On Diff #119457)

Yep, it's gone.

ELF/Writer.cpp

173–176 ↗

(On Diff #119457)

With help of D39371 and one more change, this code was completly removed.

[ruiu]

LGTM

ELF/LinkerScript.cpp

802 ↗

(On Diff #120799)

This comment doesn't make sense even though it's not new. Please remove instead of copy&paste.

lld/trunk/ELF/LinkerScript.h

public:
   OutputSection *getOrCreateOutputSection(StringRef Name);
 
   bool hasPhdrsCommands() { return !PhdrsCommands.empty(); }
   uint64_t getDot() { return Dot; }
   void discard(ArrayRef<InputSection *> V);
 
   ExprValue getSymbolValue(StringRef Name, const Twine &Loc);
 
-  void fabricateDefaultCommands();
   void addOrphanSections(OutputSectionFactory &Factory);
   void removeEmptyCommands();
   void adjustSectionsBeforeSorting();
   void adjustSectionsAfterSorting();
 
   std::vector<PhdrEntry *> createPhdrs();
   bool needsInterpSection();
 

lld/trunk/ELF/LinkerScript.cpp

if (!Sec)
       continue;
     assert(Sec->SectionIndex == INT_MAX);
     Sec->SectionIndex = I;
     if (Sec->Noload)
       Sec->Type = SHT_NOBITS;
   }
 }
 
-// If no SECTIONS command was given, we create simple SectionCommands
-// as if a minimum SECTIONS command were given. This function does that.
-void LinkerScript::fabricateDefaultCommands() {
-  // Define start address
-  uint64_t StartAddr = UINT64_MAX;
-
-  // The Sections with -T<section> have been sorted in order of ascending
-  // address. We must lower StartAddr if the lowest -T<section address> as
-  // calls to setDot() must be monotonically increasing.
-  for (auto &KV : Config->SectionStartMap)
-    StartAddr = std::min(StartAddr, KV.second);
-
-  auto Expr = [=] {
-    return std::min(StartAddr, Target->getImageBase() + elf::getHeaderSize());
-  };
-  SectionCommands.insert(SectionCommands.begin(),
-                         make<SymbolAssignment>(".", Expr, ""));
-}
-
 static OutputSection *findByName(ArrayRef<BaseCommand *> Vec,
                                  StringRef Name) {
   for (BaseCommand *Base : Vec)
     if (auto *Sec = dyn_cast<OutputSection>(Base))
       if (Sec->Name == Name)
         return Sec;
   return nullptr;
 }
 
 static void reportOrphan(InputSectionBase *IS, StringRef Name) {
   if (Config->OrphanHandling == OrphanHandlingPolicy::Error)
     error(toString(IS) + " is being placed in '" + Name + "'");
   else if (Config->OrphanHandling == OrphanHandlingPolicy::Warn)
     warn(toString(IS) + " is being placed in '" + Name + "'");
 }
 
 // Add sections that didn't match any sections command.
 void LinkerScript::addOrphanSections(OutputSectionFactory &Factory) {
   unsigned End = SectionCommands.size();
 
+  std::vector<OutputSection *> V;
   for (InputSectionBase *S : InputSections) {
     if (!S->Live || S->Parent)
       continue;
 
     StringRef Name = getOutputSectionName(S->Name);
     reportOrphan(S, Name);
 
     if (OutputSection *Sec =
             findByName(makeArrayRef(SectionCommands).slice(0, End), Name)) {
       Sec->addSection(cast<InputSection>(S));
       continue;
     }
 
     if (OutputSection *OS = Factory.addInputSec(S, Name))
-      SectionCommands.push_back(OS);
+      V.push_back(OS);
     assert(S->getOutputSection()->SectionIndex == INT_MAX);
   }
+
+  // If no SECTIONS command was given, we should insert sections commands
+  // before others, so that we can handle scripts which refers them,
+  // for example: "foo = ABSOLUTE(ADDR(.text)));".
+  // When SECTIONS command is present we just add all orphans to the end.
+  if (HasSectionsCommand)
+    SectionCommands.insert(SectionCommands.end(), V.begin(), V.end());
+  else
+    SectionCommands.insert(SectionCommands.begin(), V.begin(), V.end());
 }
 
 uint64_t LinkerScript::advance(uint64_t Size, unsigned Alignment) {
   bool IsTbss =
       (Ctx->OutSec->Flags & SHF_TLS) && Ctx->OutSec->Type == SHT_NOBITS;
   uint64_t Start = IsTbss ? Dot + Ctx->ThreadBssOffset : Dot;
   Start = alignTo(Start, Alignment);
   uint64_t End = Start + Size;

 
 LinkerScript::AddressState::AddressState() {
   for (auto &MRI : Script->MemoryRegions) {
     const MemoryRegion *MR = MRI.second;
     MemRegionOffset[MR] = MR->Origin;
   }
 }
 
-// Assign addresses as instructed by linker script SECTIONS sub-commands.
+static uint64_t getInitialDot() {
+  // By default linker scripts use an initial value of 0 for '.',
+  // but prefer -image-base if set.
+  if (Script->HasSectionsCommand)
+    return Config->ImageBase ? *Config->ImageBase : 0;
+
+  uint64_t StartAddr = UINT64_MAX;
+  // The Sections with -T<section> have been sorted in order of ascending
+  // address. We must lower StartAddr if the lowest -T<section address> as
+  // calls to setDot() must be monotonically increasing.
+  for (auto &KV : Config->SectionStartMap)
+    StartAddr = std::min(StartAddr, KV.second);
+  return std::min(StartAddr, Target->getImageBase() + elf::getHeaderSize());
+}
+
+// Here we assign addresses as instructed by linker script SECTIONS
+// sub-commands. Doing that allows us to use final VA values, so here
+// we also handle rest commands like symbol assignments and ASSERTs.
 void LinkerScript::assignAddresses() {
-  // By default linker scripts use an initial value of 0 for '.', but prefer
-  // -image-base if set.
-  Dot = Config->ImageBase ? *Config->ImageBase : 0;
+  Dot = getInitialDot();
 
   auto Deleter = make_unique<AddressState>();
   Ctx = Deleter.get();
   ErrorOnMissingSection = true;
   switchTo(Aether);
 
   for (BaseCommand *Base : SectionCommands) {
     if (auto *Cmd = dyn_cast<SymbolAssignment>(Base)) {

lld/trunk/ELF/Writer.cpp

public:
 
   void run();
 
 private:
   void createSyntheticSections();
   void copyLocalSymbols();
   void addSectionSymbols();
   void addReservedSymbols();
-  void createSections();
   void forEachRelSec(std::function<void(InputSectionBase &)> Fn);
   void sortSections();
   void sortInputSections();
   void finalizeSections();
   void addPredefinedSections();
   void setReservedSymbolSections();
 
   std::vector<PhdrEntry *> createPhdrs();
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();
 
-  // Create output sections.
-  if (Script->HasSectionsCommand) {
-    // If linker script contains SECTIONS commands, let it create sections.
-    Script->processSectionCommands();
+  // We want to process linker script commands. When SECTIONS command
+  // is given we let it create sections.
 
-    // Linker scripts may have left some input sections unassigned.
-    // Assign such sections using the default rule.
-    Script->addOrphanSections(Factory);
-  } else {
-    // If linker script does not contain SECTIONS commands, create
-    // output sections by default rules. We still need to give the
-    // linker script a chance to run, because it might contain
-    // non-SECTIONS commands such as ASSERT.
-    Script->processSectionCommands();
-    createSections();
-  }
+  // Linker scripts controls how input sections are assigned to output sections.
+  // Input sections that were not handled by scripts are called "orphans", and
+  // they are assigned to output sections by the default rule. Process that.
 
   if (Config->Discard != DiscardPolicy::All)
     copyLocalSymbols();
 
   if (Config->CopyRelocs)
     addSectionSymbols();
 
   // Now that we have a complete set of output sections. This function
void Writer<ELFT>::forEachRelSec(std::function<void(InputSectionBase &)> Fn) {
   // processed by InputSection::relocateNonAlloc.
   for (InputSectionBase *IS : InputSections)
     if (IS->Live && isa<InputSection>(IS) && (IS->Flags & SHF_ALLOC))
       Fn(*IS);
   for (EhInputSection *ES : InX::EhFrame->Sections)
     Fn(*ES);
 }
 
-template <class ELFT> void Writer<ELFT>::createSections() {
-  std::vector<OutputSection *> Vec;
-  for (InputSectionBase *IS : InputSections)
-    if (IS && IS->Live)
-      if (OutputSection *Sec =
-              Factory.addInputSec(IS, getOutputSectionName(IS->Name)))
-        Vec.push_back(Sec);
-
-  Script->SectionCommands.insert(Script->SectionCommands.begin(), Vec.begin(),
-                                 Vec.end());
-
-  Script->fabricateDefaultCommands();
-}
-
 // This function generates assignments for predefined symbols (e.g. _end or
 // _etext) and inserts them into the commands sequence to be processed at the
 // appropriate time. This ensures that the value is going to be correct by the
 // time any references to these symbols are processed and is equivalent to
 // defining these symbols explicitly in the linker script.
 template <class ELFT> void Writer<ELFT>::setReservedSymbolSections() {
   PhdrEntry *Last = nullptr;
   PhdrEntry *LastRO = nullptr;