[PATCH] D24728: [ARM][LLD] Add support for .ARM.exidx sections

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/OutputSections.cpp:894
@@ -880,1 +893,3 @@
+      this->Header.sh_link = D->OutSec->SectionIndex;
+  }
   if (Type != SHT_RELA && Type != SHT_REL)
----------------
```
if (auto *D = getLinkOrderDependency(this->Sections.front()))
  this->Header.sh_link = D->OutSec->SectionIndex;
```

================
Comment at: ELF/Writer.cpp:1006
@@ -1003,1 +1005,3 @@
+    if (Config->EMachine == EM_ARM && Sec->getType() == SHT_ARM_EXIDX)
+      ARMExidx.add(Sec);
   }
----------------
First check looks excessive for me, I guess only EM_ARM can have SHT_ARM_EXIDX anyways.

================
Comment at: ELF/Writer.cpp:1342
@@ +1341,3 @@
+  };
+  auto *Start = (ARMExidxEntry*)Buf;
+  size_t NumEnt = Size / sizeof(ARMExidxEntry);
----------------
We probably more often do not use auto in such short expressions for casting pointers,
though no general rule I think, but I would write without.

================
Comment at: ELF/Writer.cpp:1347
@@ +1346,3 @@
+  std::stable_sort(Start, Start + NumEnt,
+                   [](ARMExidxEntry A, ARMExidxEntry B) {
+                     return A.Target < B.Target;
----------------
```
(ARMExidxEntry &A, ARMExidxEntry &B)
```

================
Comment at: ELF/Writer.cpp:1372
@@ +1371,3 @@
+  if (ARMExidx && !Config->Relocatable) {
+    auto OS = dyn_cast<OutputSection<ELFT>>(ARMExidx);
+    if (OS)
----------------
ruiu wrote:
>   if (auto *OS = dyn_cast<OutputSection<ELFT>>(ARMEidx))
I would probably rewrite this as

```
if (!Config->Relocatable)
    if (auto *OS = dyn_cast_or_null<OutputSection<ELFT>>(findSection(".ARM.exidx")))
      SortARMExidx(...)
```

================
Comment at: test/ELF/arm-exidx-order.s:15
@@ +14,3 @@
+// Check that relocatable link when relinked produces same output as a full
+// link
+// RUN: ld.lld -r %t %tcantunwind -o %t4 2>&1
----------------
one line for comment ?

================
Comment at: test/ELF/arm-exidx-order.s:27
@@ +26,3 @@
+// InputSections in the same order that it has combined the executable section,
+// such that the combined .ARM.exidx OutputSection can be used as  a binary
+// search table.
----------------
"as a" (double space)


https://reviews.llvm.org/D24728