[ELF] - Stop removing sections in removeUnusedSyntheticSections().

[grimar]

Summary

That makes code a bit more consistent. Instead of removing sections there
we can just mark them as dead. So that removeEmptyCommands() will
handle the rest.

[ruiu]

LGTM

[ruiu]

On second thought, I think removeUnusedSyntehticSections is a source of confusion. This mechanism seems too hacky, and that's why you need to write many pieces of tricky code. I feels we could design a better mechanism. Can you visit this again to design something new?

[grimar]

In D38393#886125, @ruiu wrote:

On second thought, I think removeUnusedSyntehticSections is a source of confusion. This mechanism seems too hacky, and that's why you need to write many pieces of tricky code. I feels we could design a better mechanism. Can you visit this again to design something new?

I'll take a look what can I suggest here.

[grimar]

In D38393#886125, @ruiu wrote:

On second thought, I think removeUnusedSyntehticSections is a source of confusion. This mechanism seems too hacky, and that's why you need to write many pieces of tricky code. I feels we could design a better mechanism. Can you visit this again to design something new?

I suggest to do following change: https://reviews.llvm.org/D38613

lld/trunk/ELF/LinkerScript.cpp

for (BaseCommand * Cmd : Opt.Commands) {
     Sec->Flags = Flags;
   }
 }
 
 void LinkerScript::adjustSectionsAfterSorting() {
   // Try and find an appropriate memory region to assign offsets in.
   for (BaseCommand *Base : Opt.Commands) {
     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->updateAlignment(Sec->AlignExpr().getValue());
     }
   }
 
   // If output section command doesn't specify any segments,
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();
 }
 
 static OutputSection *findFirstSection(PhdrEntry *Load) {
   for (OutputSection *Sec : OutputSections)
     if (Sec->PtLoad == Load)
       return Sec;
   return nullptr;
 }

lld/trunk/ELF/Writer.cpp

if (NextSec == E)
     return E;
 
   while (I != E && shouldSkip(*I))
     ++I;
   return I;
 }
 
 template <class ELFT> void Writer<ELFT>::sortSections() {
-  if (Script->Opt.HasSections)
-    Script->adjustSectionsBeforeSorting();
 
   // Don't sort if using -r. It is not necessary and we want to preserve the
   // relative order for SHF_LINK_ORDER sections.
   if (Config->Relocatable)
     return;
 
   for (BaseCommand *Base : Script->Opt.Commands)
     if (auto *Sec = dyn_cast<OutputSection>(Base))
for (auto I = OS->Commands.begin(), E = OS->Commands.end(); I != E; ++I) {
       }
     }
     if (Empty != OS->Commands.end())
       OS->Commands.erase(Empty);
 
     // If there are no other sections in the output section, remove it from the
     // output.
     if (OS->Commands.empty())
-      llvm::erase_if(Script->Opt.Commands,
-                     [&](BaseCommand *Cmd) { return Cmd == OS; });
+      OS->Live = false;
   }
 }
 
 // Returns true if a symbol can be replaced at load-time by a symbol
 // with the same name defined in other ELF executable or DSO.
 static bool computeIsPreemptible(const SymbolBody &B) {
   assert(!B.isLocal());
   // Only symbols that appear in dynsym can be preempted.
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->Opt.Commands)
     if (auto *Sec = dyn_cast<OutputSection>(Base))
       OutputSections.push_back(Sec);
 
   // Prefer command line supplied address over other constraints.