[PATCH] D26977: [LLD][ARM] Add terminating sentinel .ARM.exidx table entry

[Rui Ueyama via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

It's definitely better than the previous one.



================
Comment at: ELF/OutputSections.cpp:91
                                   InputSection<ELFT> *B) {
+  // The synthetic terminating section is always last
+  if (A->kind() == InputSectionData::Synthetic)
----------------
Maybe it should be more descriptive. We have a sentinel for .ARM.exidx as a synthetic section and want to put it at end of the section.

But do we need this? I think we are using stable_sort, so as long as you add a new section at end of Symtab->Sections, it will go at end of sections.


================
Comment at: ELF/SyntheticSections.cpp:1674
+    : SyntheticSection<ELFT>(SHF_ALLOC | SHF_LINK_ORDER, SHT_ARM_EXIDX,
+                             sizeof(typename ELFT::uint), ".ARM.exidx") {}
+
----------------
As you just return `8` from getSize, I'd replace sizeof() with 8 as well.


================
Comment at: ELF/SyntheticSections.cpp:1682
+  // Get the InputSection before us, we are by definition last
+  auto ri = cast<OutputSection<ELFT>>(this->OutSec)->Sections.rbegin();
+  InputSection<ELFT> *LE = *(++ri);
----------------
ri -> RI


================
Comment at: ELF/Writer.cpp:1026
+
+  OutputSection<ELFT> *OS =
+    dyn_cast_or_null<OutputSection<ELFT>>(findSection(".ARM.exidx"));
----------------
You could use `auto`.

  if (auto *Sec = dyn_cast_or_null<OutputSection<ELFT>>(findSection(".ARM.exidx")))
    if (!Sec->Sections.empty() ...


https://reviews.llvm.org/D26977