Simplify removing empty output sections

[• espindola]

Summary

With this the meaning of the Live bit in output sections is clear: we have at some point added a input section into it.

A followup patch could probably replace it by making the Flags field Optional.

[ruiu]

LGTM

ELF/LinkerScript.cpp

775

It feels like this function name is not right because this function does more than checking if all members are section descriptions. Can you rename?

[• espindola]

r326401.

ELF/LinkerScript.cpp

for (BaseCommand *Base : SectionCommands) {
 
     if (auto *Sec = dyn_cast<OutputSection>(Base)) {
       std::vector<InputSection *> V = createInputSectionList(*Sec);
 
       // The output section name `/DISCARD/' is special.
       // Any input section assigned to it is discarded.
       if (Sec->Name == "/DISCARD/") {
         discard(V);
+        Sec->SectionCommands.clear();
         continue;
       }
 
       // This is for ONLY_IF_RO and ONLY_IF_RW. An output section directive
       // ".foo : ONLY_IF_R[OW] { ... }" is handled only if all member input
       // sections satisfy a given constraint. If not, a directive is handled
       // as if it wasn't present from the beginning.
       //
for (InputSection *Sec : Cmd->Sections) {
       if (!Sec->Live)
         continue;
       assert(Ctx->OutSec == Sec->getParent());
       output(Sec);
     }
   }
 }
 
+static bool isAllSectionDescription(const OutputSection &Cmd) {
+  // We do not remove empty sections that are explicitly
+  // assigned to any segment.
+  if (!Cmd.Phdrs.empty())
+    return false;
+
+  // We do not want to remove sections that have custom address or align
+  // expressions set even if them are empty. We keep them because we
+  // want to be sure that any expressions can be evaluated and report
+  // an error otherwise.
+  if (Cmd.AddrExpr || Cmd.AlignExpr || Cmd.LMAExpr)
+    return false;
+
+  for (BaseCommand *Base : Cmd.SectionCommands)
+    if (!isa<InputSectionDescription>(*Base))
+      return false;
+  return true;
+}
+
 void LinkerScript::adjustSectionsBeforeSorting() {
   // If the output section contains only symbol assignments, create a
   // corresponding output section. The issue is what to do with linker script
   // like ".foo : { symbol = 42; }". One option would be to convert it to
   // "symbol = 42;". That is, move the symbol out of the empty section
   // description. That seems to be what bfd does for this simple case. The
   // problem is that this is not completely general. bfd will give up and
   // create a dummy section too if there is a ". = . + 1" inside the section
void LinkerScript::adjustSectionsBeforeSorting() {
   // will have on the rest of the linker. That is why we copy the flags from
   // the previous sections. Only a few flags are needed to keep the impact low.
   uint64_t Flags = SHF_ALLOC;
 
   for (BaseCommand *&Cmd : SectionCommands) {
     auto *Sec = dyn_cast<OutputSection>(Cmd);
     if (!Sec)
       continue;
-    if (Sec->Live) {
-      Flags = Sec->Flags & (SHF_ALLOC | SHF_WRITE | SHF_EXECINSTR);
-      continue;
-    }
 
-    if (!Sec->isAllSectionDescription())
-      Sec->Flags = Flags;
+    // A live output section means that some input section was added to it. It
+    // might have been removed (gc, or empty synthetic section), but we at least
+    // know the flags.
+    if (Sec->Live)
+      Flags = Sec->Flags & (SHF_ALLOC | SHF_WRITE | SHF_EXECINSTR);
     else
+      Sec->Flags = Flags;
+
+    if (getInputSections(Sec).empty() && isAllSectionDescription(*Sec)) {
+      Sec->Live = false;
       Cmd = nullptr;
     }
+  }
 
   // It is common practice to use very generic linker scripts. So for any
   // given run some of the output sections in the script will be empty.
   // We could create corresponding empty output sections, but that would
   // clutter the output.
   // We instead remove trivially empty sections. The bfd linker seems even
   // more aggressive at removing them.
   llvm::erase_if(SectionCommands, [&](BaseCommand *Base) { return !Base; });

ELF/OutputSections.h

 // This represents a section in an output file.
 // It is composed of multiple InputSections.
 // The writer creates multiple OutputSections and assign them unique,
 // non-overlapping file offsets and VAs.
 class OutputSection final : public BaseCommand, public SectionBase {
 public:
   OutputSection(StringRef Name, uint32_t Type, uint64_t Flags);
 
-  bool isAllSectionDescription() const;
-
   static bool classof(const SectionBase *S) {
     return S->kind() == SectionBase::Output;
   }
 
   static bool classof(const BaseCommand *C);
 
   uint64_t getLMA() const { return PtLoad ? Addr + PtLoad->LMAOffset : Addr; }
   template <typename ELFT> void writeHeaderTo(typename ELFT::Shdr *SHdr);

ELF/OutputSections.cpp

OutputSection::OutputSection(StringRef Name, uint32_t Type, uint64_t Flags)
     : BaseCommand(OutputSectionKind),
       SectionBase(Output, Name, Flags, /*Entsize*/ 0, /*Alignment*/ 1, Type,
                   /*Info*/ 0,
                   /*Link*/ 0),
       SectionIndex(INT_MAX) {
   Live = false;
 }
 
-bool OutputSection::isAllSectionDescription() const {
-  // We do not remove empty sections that are explicitly
-  // assigned to any segment.
-  if (!Phdrs.empty())
-    return false;
-
-  // We do not want to remove sections that have custom address or align
-  // expressions set even if them are empty. We keep them because we
-  // want to be sure that any expressions can be evaluated and report
-  // an error otherwise.
-  if (AddrExpr || AlignExpr || LMAExpr)
-    return false;
-
-  for (BaseCommand *Base : SectionCommands)
-    if (!isa<InputSectionDescription>(*Base))
-      return false;
-  return true;
-}
-
 // We allow sections of types listed below to merged into a
 // single progbits section. This is typically done by linker
 // scripts. Merging nobits and progbits will force disk space
 // to be allocated for nobits sections. Other ones don't require
 // any special treatment on top of progbits, so there doesn't
 // seem to be a harm in merging them.
 static bool canMergeToProgbits(unsigned Type) {
   return Type == SHT_NOBITS || Type == SHT_PROGBITS || Type == SHT_INIT_ARRAY ||

ELF/Writer.cpp

if (!OS || !SS->empty())
       continue;
 
     // If we reach here, then SS is an unused synthetic section and we want to
     // remove it from corresponding input section description of output section.
     for (BaseCommand *B : OS->SectionCommands)
       if (auto *ISD = dyn_cast<InputSectionDescription>(B))
         llvm::erase_if(ISD->Sections,
                        [=](InputSection *IS) { return IS == SS; });
-
-    // If there are no other alive sections or commands left in the output
-    // section description, we remove it from the output.
-    if (getInputSections(OS).empty() && OS->isAllSectionDescription())
-      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 Symbol &B) {
   assert(!B.isLocal());
   // Only symbols that appear in dynsym can be preempted.