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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
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)) {
----------------
peter.smith wrote:
> 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.
OK. I think having the check here is fine.

> I thought it better to keep the executableSections and exidxSections synchronised at the start.

Keeping them synchronized is a nice property to have.


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

https://reviews.llvm.org/D67848