[PATCH] D148033: [LLD][ARM] Handle .ARM.exidx sections at non-zero output sec offset

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Thanks for the patch. I prefer the current approach than changing `PT_ARM_EXIDX` to `PT_NULL`. The code change looks good.

> This does not seem to be the case for GNU and LLVM readelf which seems to look for the SHT_ARM_EXIDX section.

Confirmed that they just check the section type SHT_ARM_EXIDX, not the program header type. Perhaps `readelf -D -u` can be made to check `PT_ARM_EXIDX`, but the change is likely over-engineering..



================
Comment at: lld/ELF/Writer.cpp:2631
+    // SyntheticSection.
+    if (config->emachine == EM_ARM && p->p_type == PT_ARM_EXIDX) {
+      p->p_filesz = part.armExidx->getSize();
----------------
Perhaps use `if (part.armExidx && p->p_type == PT_ARM_EXIDX)`. The two conditions are equivalent, but checking `part.armExidx` is safer in case `part.armExidx` is null.


================
Comment at: lld/test/ELF/arm-exidx-nonzero-offset.s:9
+// RUN: llvm-readelf --program-headers --symbols -x .exceptions %t/zero | FileCheck %s
+
+/// On platforms that load ELF files directly the ARM.exidx sections
----------------
Shall we use `llvm-readelf --unwind` output as well? The output can be more readable, though it doesn't use the GNU readelf style...


================
Comment at: lld/test/ELF/arm-exidx-nonzero-offset.s:28
+/// same.
+// CHECK: EXIDX 0x010080 0x00000080 0x00000080 0x00028 0x00028 R   0x4
+
----------------



================
Comment at: lld/test/ELF/arm-exidx-nonzero-offset.s:41
+  /* Addition of thunk in .text changes alignment padding */
+   .exceptions : {
+   /* Alignment padding within .exceptions */
----------------
`.exceptions` has different alignment than the previous line.


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

https://reviews.llvm.org/D148033