ELF: Perform per-section .ARM.exidx processing during combineEhFrameSections(). NFCI.

[pcc]

Summary

And rename the function to combineEhSections(). This makes the processing
of .ARM.exidx even more similar to .eh_frame and means that we can avoid an
additional loop over InputSections.

[grimar]

I like this approach and see no major issues with this patch probably.

(The way combineEhSections works with sections is not very consistent (e.g. the behavior of 'addSection' calls),
but I did not find a nice way to improve it atm, we can change it later I think)

lld/ELF/Writer.cpp

472 ↗

(On Diff #192959)

I think it worth to add a comment about what this method does now.
(or perhaps it would be better to add comments to combineEhSections body though)

[peter.smith]

I'm happy to make the change. One minor reservation is that from combineEHSections to ARMExidxSections::finalize() the section will report its size as 0 and each individual ARMExidx InputSection won't have a parent. To the best of my knowledge this doesn't matter right now as nothing depends on these being set during that time. It may give someone a surprise in the future though.

[ruiu]

LGTM

[pcc]

In D60026#1449098, @peter.smith wrote:

I'm happy to make the change. One minor reservation is that from combineEHSections to ARMExidxSections::finalize() the section will report its size as 0 and each individual ARMExidx InputSection won't have a parent. To the best of my knowledge this doesn't matter right now as nothing depends on these being set during that time. It may give someone a surprise in the future though.

It's normal for synthetic section sizes to not be fixed until the call to finalizeContents(), e.g. EhFrameSection: http://llvm-cs.pcc.me.uk/tools/lld/ELF/SyntheticSections.cpp#505

I believe that the sections already don't have parents until the call to finalizeContents() because the parents are created during processSectionCommands/addOrphanSections.

lld/trunk/ELF/SyntheticSections.h

 //
 // Instead of storing pointers to the .ARM.exidx InputSections from
 // InputObjects, we store pointers to the executable sections that need
 // .ARM.exidx sections. We can then use the dependentSections of these to
 // either find the .ARM.exidx section or know that we need to generate one.
 class ARMExidxSyntheticSection : public SyntheticSection {
 public:
   ARMExidxSyntheticSection();
+
+  // Add an input section to the ARMExidxSyntheticSection. Returns whether the
+  // section needs to be removed from the main input section list.
+  bool addSection(InputSection *IS);
+
   size_t getSize() const override { return Size; }
   void writeTo(uint8_t *Buf) override;
   bool isNeeded() const override { return !Empty; }
   // Sort and remove duplicate entries.
   void finalizeContents() override;
   InputSection *getLinkOrderDep() const;
 
   static bool classof(const SectionBase *D);
 
   // Links to the ARMExidxSections so we can transfer the relocations once the
   // layout is known.
   std::vector<InputSection *> ExidxSections;
 
 private:
-  // Derive Size from contents of ExecutableSections, including any linker
-  // generated sentinels. Also set the OutSecOff of the ExidxSections.
-  void setSizeAndOffsets();
   size_t Size;
 
   // Empty if ExecutableSections contains no dependent .ARM.exidx sections.
   bool Empty = true;
 
   // Instead of storing pointers to the .ARM.exidx InputSections from
   // InputObjects, we store pointers to the executable sections that need
   // .ARM.exidx sections. We can then use the dependentSections of these to

lld/trunk/ELF/SyntheticSections.cpp

 }
 
 MipsRldMapSection::MipsRldMapSection()
     : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_PROGBITS, Config->Wordsize,
                        ".rld_map") {}
 
 ARMExidxSyntheticSection::ARMExidxSyntheticSection()
     : SyntheticSection(SHF_ALLOC | SHF_LINK_ORDER, SHT_ARM_EXIDX,
-                       Config->Wordsize, ".ARM.exidx") {
-  for (InputSectionBase *&IS : InputSections) {
-    if (isa<InputSection>(IS) && IS->Type == SHT_ARM_EXIDX) {
-      ExidxSections.push_back(cast<InputSection>(IS));
-      IS = nullptr;
-    } else if (IS->Live && isa<InputSection>(IS) &&
-               IS->kind() != SectionBase::Synthetic &&
-               (IS->Flags & SHF_ALLOC) && (IS->Flags & SHF_EXECINSTR) &&
-               IS->getSize() > 0) {
-      ExecutableSections.push_back(cast<InputSection>(IS));
-    }
-  }
-  setSizeAndOffsets();
-
-  // FIXME: we do not output a relocation section when --emit-relocs is used
-  // as we do not have relocation sections for linker generated table entries
-  // and we would have to erase at a late stage relocations from merged entries.
-  // Given that exception tables are already position independent and a binary
-  // analyzer could derive the relocations we choose to erase the relocations.
-  if (Config->EmitRelocs)
-    for (InputSectionBase *&IS : InputSections)
-      if (IS && isa<InputSection>(IS) && IS->Type == SHT_REL) {
-        InputSection *RS = cast<InputSection>(IS);
-        if (InputSectionBase *EX = RS->getRelocatedSection())
-          if (isa<InputSection>(EX) && EX->Type == SHT_ARM_EXIDX)
-            IS = nullptr;
-      }
-
-  std::vector<InputSectionBase *> &V = InputSections;
-  V.erase(std::remove(V.begin(), V.end(), nullptr), V.end());
-}
+                       Config->Wordsize, ".ARM.exidx") {}
 
 static InputSection *findExidxSection(InputSection *IS) {
   for (InputSection *D : IS->DependentSections)
     if (D->Type == SHT_ARM_EXIDX)
       return D;
   return nullptr;
 }
 
-void ARMExidxSyntheticSection::setSizeAndOffsets() {
-  size_t Offset = 0;
-  Size = 0;
-  for (InputSection *IS : ExecutableSections) {
-    if (InputSection *D = findExidxSection(IS)) {
-      D->OutSecOff = Offset;
-      D->Parent = getParent();
-      Offset += D->getSize();
-      Empty = false;
-    } else {
-      Offset += 8;
-    }
+bool ARMExidxSyntheticSection::addSection(InputSection *IS) {
+  if (IS->Type == SHT_ARM_EXIDX) {
+    ExidxSections.push_back(IS);
+    return true;
+
+  if ((IS->Flags & SHF_ALLOC) && (IS->Flags & SHF_EXECINSTR) &&
+      IS->getSize() > 0) {
+    ExecutableSections.push_back(IS);
+    if (Empty && findExidxSection(IS))
+      Empty = false;
+    return false;
   }
-  // Size includes Sentinel.
-  Size = Offset + 8;
+
+  // FIXME: we do not output a relocation section when --emit-relocs is used
+  // as we do not have relocation sections for linker generated table entries
+  // and we would have to erase at a late stage relocations from merged entries.
+  // Given that exception tables are already position independent and a binary
+  // analyzer could derive the relocations we choose to erase the relocations.
+  if (Config->EmitRelocs && IS->Type == SHT_REL)
+    if (InputSectionBase *EX = IS->getRelocatedSection())
+      if (isa<InputSection>(EX) && EX->Type == SHT_ARM_EXIDX)
+        return true;
+
+  return false;
 }
 
 // References to .ARM.Extab Sections have bit 31 clear and are not the
 // special EXIDX_CANTUNWIND bit-pattern.
 static bool isExtabRef(uint32_t Unwind) {
   return (Unwind & 0x80000000) == 0 && Unwind != 0x1;
 }
 
for (size_t I = 1; I < ExecutableSections.size(); ++I) {
       InputSection *EX2 = findExidxSection(ExecutableSections[I]);
       if (!isDuplicateArmExidxSec(EX1, EX2)) {
         SelectedSections.push_back(ExecutableSections[I]);
         Prev = I;
       }
     }
     ExecutableSections = std::move(SelectedSections);
   }
-  setSizeAndOffsets();
+
+  size_t Offset = 0;
+  Size = 0;
+  for (InputSection *IS : ExecutableSections) {
+    if (InputSection *D = findExidxSection(IS)) {
+      D->OutSecOff = Offset;
+      D->Parent = getParent();
+      Offset += D->getSize();
+    } else {
+      Offset += 8;
+    }
+  }
+  // Size includes Sentinel.
+  Size = Offset + 8;
 }
 
 InputSection *ARMExidxSyntheticSection::getLinkOrderDep() const {
   return ExecutableSections.front();
 }
 
 // To write the .ARM.exidx table from the ExecutableSections we have three cases
 // 1.) The InputSection has a .ARM.exidx InputSection in its dependent sections.

lld/trunk/ELF/Writer.cpp

if (P->p_type != PT_LOAD)
       return false;
     if (!P->FirstSec)
       return true;
     uint64_t Size = P->LastSec->Addr + P->LastSec->Size - P->FirstSec->Addr;
     return Size == 0;
   });
 }
 
-template <class ELFT> static void combineEhFrameSections() {
+template <class ELFT> static void combineEhSections() {
   for (InputSectionBase *&S : InputSections) {
-    EhInputSection *ES = dyn_cast<EhInputSection>(S);
-    if (!ES || !ES->Live)
+    if (!S->Live)
       continue;
 
+    if (auto *ES = dyn_cast<EhInputSection>(S)) {
       In.EhFrame->addSection<ELFT>(ES);
       S = nullptr;
+    } else if (S->kind() == SectionBase::Regular && In.ARMExidx &&
+               In.ARMExidx->addSection(cast<InputSection>(S))) {
+      S = nullptr;
+    }
   }
 
   std::vector<InputSectionBase *> &V = InputSections;
   V.erase(std::remove(V.begin(), V.end(), nullptr), V.end());
 }
 
 static Defined *addOptionalRegular(StringRef Name, SectionBase *Sec,
                                    uint64_t Val, uint8_t StOther = STV_HIDDEN,

 }
 
 // The main function of the writer.
 template <class ELFT> void Writer<ELFT>::run() {
   // Create linker-synthesized sections such as .got or .plt.
   // Such sections are of type input section.
   createSyntheticSections<ELFT>();
 
+  // Some input sections that are used for exception handling need to be moved
+  // into synthetic sections. Do that now so that they aren't assigned to
+  // output sections in the usual way.
   if (!Config->Relocatable)
-    combineEhFrameSections<ELFT>();
+    combineEhSections<ELFT>();
 
   // We want to process linker script commands. When SECTIONS command
   // is given we let it create sections.
   Script->processSectionCommands();
 
   // 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.