[PATCH] D59216: [LLD][ELF][ARM] Redesign of .ARM.exidx handling to use a SyntheticSection

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Overall looking good.



================
Comment at: ELF/SyntheticSections.cpp:3111
+  // special EXIDX_CANTUNWIND bit-pattern.
+  auto IsExtabRef = [](uint32_t Unwind) {
+    return (Unwind & 0x80000000) == 0 && Unwind != 0x1;
----------------
This does not capture anything, so I'd define it as a regular function instead of a lambda.


================
Comment at: ELF/SyntheticSections.cpp:3121
+  // nullptr then it will be a synthesized EXIDX_CANTUNWIND entry.
+  ExidxEntry PrevEntry = {ulittle32_t(0), ulittle32_t(1)};
+  if (Prev)
----------------
Do you need to cast to ulittle32_t?


================
Comment at: ELF/SyntheticSections.cpp:3137
+  if (Cur == nullptr)
+    return PrevEntry.Unwind == ulittle32_t(1);
+
----------------
I don't think you need a cast to ulittle32_t.


================
Comment at: ELF/SyntheticSections.cpp:3169
+                   CompareByFilePosition);
+  Sentinel = ExecutableSections[ExecutableSections.size() - 1];
+  // Optionally merge adjacent duplicate entries
----------------
Why don't you use `back`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59216/new/

https://reviews.llvm.org/D59216