[PATCH] D67848: [LLD][ELF][ARM] Fix crash when discarding all of the InputSections that have .ARM.exidx sections

[Peter Smith via Phabricator via llvm-commits]

peter.smith marked 2 inline comments as done.
peter.smith added inline comments.


================
Comment at: lld/ELF/SyntheticSections.cpp:3228
   if (isec->type == SHT_ARM_EXIDX) {
-    exidxSections.push_back(isec);
-    return true;
+    if (InputSection* dep = isec->getLinkOrderDep())
+      if (isValidExidxSectionDep(dep)) {
----------------
MaskRay wrote:
> ```
>   if (isec->type == SHT_ARM_EXIDX) {
>     exidxSections.push_back(isec);
>     return true;
>   }
> ```
> 
> probably also works. finalizeContents() has logic to filter dead InputSections, so I'm thinking whether we should simplify the logic here.
I tried that initially but got test failures (crashes) for the case where there was a zero sized .text section with a non-zero size .ARM.exidx section (garbage collection off). The .ARM.exidx section is added, but the executable section isn't, isNeeded() returns true. This leaves the executableSections and exidxSections out of synch.

It is possible to account for this case by making isNeeded() more complex by making it search for both executable and exidx sections, but I thought it better to keep the executableSections and exidxSections synchronised at the start.

I can make the change if you prefer.


================
Comment at: lld/test/ELF/arm-exidx-discard-all.s:1
+// REQUIRES: arm
+// RUN: llvm-mc -filetype=obj -triple arm-gnu-linux-eabi -mcpu cortex-a7 -arm-add-build-attributes %s -o %t.o
----------------
MaskRay wrote:
> How about moving this to linkerscript/? There are 2 arm-exidx* tests.
Sure I'll update the patch.


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

https://reviews.llvm.org/D67848