[ELF] - Simplify reporting of garbage collected sections.

[grimar]

Summary

I suggest following change.
It allows to simplify code.

[ruiu]

I think I like the direction of this patch.

ELF/LinkerScript.cpp

286

Question: is it possible that a section is assigned to some output section but is not live? It sounds like a bad state to me.

[grimar]

ELF/LinkerScript.cpp

286

Yes, it is possible for following case:
/DISCARD/ : {*(.text*)}
Here we set Assigned flag for .text. It is probably OK it seems ?

(I would probably try to completely get rid of 'Assigned' flag, I think we should be able to do that).

[ruiu]

ELF/LinkerScript.cpp

286

No, I don't think that is OK. /DISCARD/ is a keyword and not a real section. Sections discarded by /DISCARD/ should be not Assigned and not Live.

[grimar]

ELF/LinkerScript.cpp

286

I think that is fine because specification explicitly mentions that input sections are assigned:

The special output section name ‘/DISCARD/’ may be used to discard input sections. Any
input sections which are assigned to an output section named ‘/DISCARD/’ are not included
in the output file.

(https://sourceware.org/binutils/docs-2.16/ld/Output-Section-Discarding.html)

Anyways I believe nice way to solve this would be to remove Assigned flag,
what should be possible and should be done in different patch.

[ruiu]

ELF/LinkerScript.cpp

286

I don't think that is fine, please read my commit message for r316622. Can you rebase this patch?

[grimar]

• Rebased.

ELF/LinkerScript.cpp

286

Rebased, but we still need to check both conditions here:

1. We need to check that section is not dead, I think it is obvious.
2. We still need to check that it is not assigned. We could avoid that in theory and check Sec->Parent instead of Sec->Assigned, but it will not work so simple, because we can have script like: SECTIONS {.foo : { *(.foo) *(.foo) } } (input-sec-dup.s) and do not want to add .foo twice. We could assign Sec->Parent to some dummy non-zero value probably and get rid of Assigned, but that is story for another patch.

[ruiu]

LGTM

ELF/MarkLive.cpp

293

collected -> garbage-collected

ELF/LinkerScript.cpp

LinkerScript::computeInputSections(const InputSectionDescription *Cmd,
                                    const DenseMap<SectionBase *, int> &Order) {
   std::vector<InputSection *> Ret;
 
   // Collects all sections that satisfy constraints of Cmd.
   for (const SectionPattern &Pat : Cmd->SectionPatterns) {
     size_t SizeBefore = Ret.size();
 
     for (InputSectionBase *Sec : InputSections) {
-      if (Sec->Assigned)
+      if (!Sec->Live || Sec->Assigned)
         continue;
 
-      if (!Sec->Live) {
-        reportDiscarded(Sec);
-        continue;
-      }
-
       // For -emit-relocs we have to ignore entries like
       //   .rela.dyn : { *(.rela.data) }
       // which are common because they are in the default bfd script.
       if (Sec->Type == SHT_REL || Sec->Type == SHT_RELA)
         continue;
 
       std::string Filename = getFilename(Sec->File);
       if (!Cmd->FilePat.match(Filename) ||

ELF/MarkLive.cpp

for (InputSectionBase *Sec : InputSections) {
     bool IsAlloc = (Sec->Flags & SHF_ALLOC);
     bool IsRel = (Sec->Type == SHT_REL || Sec->Type == SHT_RELA);
     if (!IsAlloc && !IsRel)
       Sec->Live = true;
   }
 
   // Follow the graph to mark all live sections.
   doGcSections<ELFT>();
+
+  // Report collected sections.
+  if (Config->PrintGcSections)
+    for (InputSectionBase *Sec : InputSections)
+      if (!Sec->Live)
+        message("removing unused section from '" + Sec->Name + "' in file '" +
+                Sec->File->getName() + "'");
 }
 
 template void elf::markLive<ELF32LE>();
 template void elf::markLive<ELF32BE>();
 template void elf::markLive<ELF64LE>();
 template void elf::markLive<ELF64BE>();

ELF/OutputSections.h

public:
 
   OutputSection *addInputSec(InputSectionBase *IS, StringRef OutsecName);
 
 private:
   llvm::SmallDenseMap<SectionKey, OutputSection *> Map;
 };
 
 uint64_t getHeaderSize();
-void reportDiscarded(InputSectionBase *IS);
 void sortByOrder(llvm::MutableArrayRef<InputSection *> In,
                  std::function<int(InputSectionBase *S)> Order);
 
 extern std::vector<OutputSection *> OutputSections;
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/OutputSections.cpp

 // seem to be a harm in merging them.
 static bool canMergeToProgbits(unsigned Type) {
   return Type == SHT_NOBITS || Type == SHT_PROGBITS || Type == SHT_INIT_ARRAY ||
          Type == SHT_PREINIT_ARRAY || Type == SHT_FINI_ARRAY ||
          Type == SHT_NOTE;
 }
 
 void OutputSection::addSection(InputSection *IS) {
-  if (!IS->Live) {
-    reportDiscarded(IS);
-    return;
-  }
-
   if (!Live) {
     // If IS is the first section to be added to this section,
     // initialize Type by IS->Type.
     Live = true;
     Type = IS->Type;
   } else {
     // Otherwise, check if new type or flags are compatible with existing ones.
     if ((Flags & (SHF_ALLOC | SHF_TLS)) != (IS->Flags & (SHF_ALLOC | SHF_TLS)))
void elf::sortByOrder(MutableArrayRef<InputSection *> In,
   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;
 }
 
-void elf::reportDiscarded(InputSectionBase *IS) {
-  if (!Config->PrintGcSections)
-    return;
-  message("removing unused section from '" + IS->Name + "' in file '" +
-          IS->File->getName() + "'");
-}
-
 static OutputSection *createSection(InputSectionBase *IS, StringRef OutsecName) {
   OutputSection *Sec = Script->createOutputSection(OutsecName, "<internal>");
   Sec->Type = IS->Type;
   Sec->Flags = IS->Flags;
   Sec->addSection(cast<InputSection>(IS));
   return Sec;
 }
 
 OutputSection *OutputSectionFactory::addInputSec(InputSectionBase *IS,
                                                  StringRef OutsecName) {
-  if (!IS->Live) {
-    reportDiscarded(IS);
-    return nullptr;
-  }
 
   // Sections with SHT_GROUP or SHF_GROUP attributes reach here only when the -r
   // option is given. A section with SHT_GROUP defines a "section group", and
   // its members have SHF_GROUP attribute. Usually these flags have already been
   // stripped by InputFiles.cpp as section groups are processed and uniquified.
   // However, for the -r option, we want to pass through all section groups
   // as-is because adding/removing members or merging them with other groups
   // change their semantics.

ELF/Writer.cpp

if (IS->Live && isa<InputSection>(IS) && (IS->Flags & SHF_ALLOC))
       Fn(*IS);
   for (EhInputSection *ES : In<ELFT>::EhFrame->Sections)
     Fn(*ES);
 }
 
 template <class ELFT> void Writer<ELFT>::createSections() {
   std::vector<OutputSection *> Vec;
   for (InputSectionBase *IS : InputSections)
-    if (IS)
+    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();