[ELF] - Hide LinkerScript::removeEmptyCommands().

[grimar]

Summary

With little code reordering it is possible to make
removeEmptyCommands to be private. Does not seem
it worth to allow it leak to public interface and also
simplifies code a bit.

[ruiu]

Basically this seems like a good change, but why did you move the existing code?

[grimar]

In D39047#907279, @ruiu wrote:

Basically this seems like a good change, but why did you move the existing code?

It's for case from orphan-phdrs.s test:

PHDRS {
  exec PT_LOAD FLAGS(0x4 | 0x1);
  rw   PT_LOAD FLAGS(0x4 | 0x2);
}
SECTIONS {
 .text : { *(.text) } :exec
 .empty : { *(.empty) } :rw
 .rw : { *(.rw) }
}

if we have no .empty input sections, this command will be removed from output,
though we still want to assign output section .rw to rw PT_LOAD.
So we want to remove empty commands after sections are assigned to headers and not before.

[ruiu]

LGTM

[grimar]

Not sure what exactly changed, but this one breaks 276 tests now. Will revisit it later.

ELF/LinkerScript.h

class LinkerScript final {
   std::vector<size_t> getPhdrIndices(OutputSection *Sec);
 
   MemoryRegion *findMemoryRegion(OutputSection *Sec);
 
   void switchTo(OutputSection *Sec);
   uint64_t advance(uint64_t Size, unsigned Align);
   void output(InputSection *Sec);
 
+  void removeEmptyCommands();
+
   std::unique_ptr<AddressState> Ctx;
   OutputSection *Aether;
 
   uint64_t Dot;
 
 public:
   OutputSection *createOutputSection(StringRef Name, StringRef Location);
   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();
 
   bool shouldKeep(InputSectionBase *S);
   void assignOffsets(OutputSection *Sec);

ELF/LinkerScript.cpp

if (isAllSectionDescription(*Sec))
       continue;
 
     Sec->Live = true;
     Sec->Flags = Flags;
   }
 }
 
 void LinkerScript::adjustSectionsAfterSorting() {
-  // Try and find an appropriate memory region to assign offsets in.
-  for (BaseCommand *Base : SectionCommands) {
-    if (auto *Sec = dyn_cast<OutputSection>(Base)) {
-      if (!Sec->Live)
-        continue;
-      Sec->MemRegion = findMemoryRegion(Sec);
-      // Handle align (e.g. ".foo : ALIGN(16) { ... }").
-      if (Sec->AlignExpr)
-        Sec->Alignment =
-            std::max<uint32_t>(Sec->Alignment, Sec->AlignExpr().getValue());
-    }
-  }
-
   // If output section command doesn't specify any segments,
   // and we haven't previously assigned any section to segment,
   // then we simply assign section to the very first load segment.
   // Below is an example of such linker script:
   // PHDRS { seg PT_LOAD; }
   // SECTIONS { .aaa : { *(.aaa) } }
   std::vector<StringRef> DefPhdrs;
   auto FirstPtLoad =
if (Sec->Phdrs.empty()) {
       // To match the bfd linker script behaviour, only propagate program
       // headers to sections that are allocated.
       if (Sec->Flags & SHF_ALLOC)
         Sec->Phdrs = DefPhdrs;
     } else {
       DefPhdrs = Sec->Phdrs;
     }
   }
+
+  removeEmptyCommands();
+
+  // Try and find an appropriate memory region to assign offsets in.
+  for (BaseCommand *Base : SectionCommands) {
+    if (auto *Sec = dyn_cast<OutputSection>(Base)) {
+      Sec->MemRegion = findMemoryRegion(Sec);
+      // Handle align (e.g. ".foo : ALIGN(16) { ... }").
+      if (Sec->AlignExpr)
+        Sec->Alignment =
+            std::max<uint32_t>(Sec->Alignment, Sec->AlignExpr().getValue());
+    }
+  }
 }
 
 static OutputSection *findFirstSection(PhdrEntry *Load) {
   for (OutputSection *Sec : OutputSections)
     if (Sec->PtLoad == Load)
       return Sec;
   return nullptr;
 }

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::finalizeSections() {
   // Do not proceed if there was an undefined symbol.
   if (ErrorCount)
     return;
 
   addPredefinedSections();
   removeUnusedSyntheticSections();
 
   sortSections();
-  Script->removeEmptyCommands();
 
   // Now that we have the final list, create a list of all the
   // OutputSections for convenience.
   for (BaseCommand *Base : Script->SectionCommands)
     if (auto *Sec = dyn_cast<OutputSection>(Base))
       OutputSections.push_back(Sec);
 
   // Prefer command line supplied address over other constraints.