[ELF][ARM] Fix /DISCARD/ of section with .ARM.exidx section

[peter.smith]

Summary

The combineEhSections runs, by design, before processSectionCommands so that input exception sections like .ARM.exidx and .eh_frame are not assigned to OutputSections. Unfortunately if /DISCARD/ removes InputSections that have associated .ARM.exidx sections without discarding the .ARM.exidx synthetic section then we will end up crashing when trying to sort the InputSections in ascending address order.

We fix this by filtering out the sections that have been discarded prior to processing the InputSections in finalizeContents()

fixes pr42890

Note that we cannot discard a subset of the .ARM.exidx table in LLD as by the time the /DISCARD/ has been evaluated it is a single synthetic section called .ARM.exidx. I'm not too concerned about this as a partial discard doesn't make a lot of sense as LLD creates synthetic entries to fill any gaps in the table.

I don't think that this affects .eh_frame as we don't have the dependent section and ordering requirements that we do for .ARM.exidx.

[MaskRay]

Haven't gone through the logic yet (and I'm still learning the .ARM.exidx mechanism), but the solution looks good.

In the description, several .ehframe occurrences should be replaced by .eh_frame. Some nits inlined.

ELF/SyntheticSections.cpp

3209 ↗

(On Diff #213402)

with a -> with a dependent?

3212 ↗

(On Diff #213402)

std::remove_if(excutableSections.begin(), excutableSections.end(), ...) -> llvm::remove_if(excutableSections, ...)

3217 ↗

(On Diff #213402)

llvm::remove_if

[grimar]

ELF/SyntheticSections.cpp

3212 ↗

(On Diff #213402)

The following should work here (and below):
llvm::erase_if(executableSections, isDiscarded);

test/ELF/arm-exidx-partial-discard.s

6 ↗

(On Diff #213402)

Seems you could combine this llvm-readobj calls?

[MaskRay]

test/ELF/arm-exidx-partial-discard.s

6 ↗

(On Diff #213402)

Alternatively, | FileCheck --implicit-check-not=.exit.text /dev/null

[peter.smith]

Thanks very much for the comments. I've updated the description, comment, used llvm::erase_if and combined the llvm-readobj into one call.

The full details are in https://static.docs.arm.com/ihi0038/a/IHI0038A_ehabi.pdf although only a small part is relevant to linking. The biggest complications that lead to us using a SyntheticSection were:

• Adjacent table entries with identical unwind entries can be merged.
• The address range covered by table entry N is terminated by table entry N + 1. This means that if the .ARM.exidx information is incomplete (executable sections don't have any unwinding information) then an exception thrown through a function without any unwinding information can match a table entry with unwinding information. We fix this by generating CANT_UNWIND table entries for sections without any unwind information.
• A terminating CANT_UNWIND sentinel entry is required to terminate the address range of the last table entry.

Let me know if you have any questions, I've glossed over a lot of detail.

[ruiu]

LGTM

lld/trunk/ELF/SyntheticSections.cpp

static bool isDuplicateArmExidxSec(InputSection *prev, InputSection *cur) {
 
   // All table entries in this .ARM.exidx Section can be merged into the
   // previous Section.
   return true;
 }
 
 // The .ARM.exidx table must be sorted in ascending order of the address of the
 // functions the table describes. Optionally duplicate adjacent table entries
-// can be removed. At the end of the function the ExecutableSections must be
+// can be removed. At the end of the function the executableSections must be
 // sorted in ascending order of address, Sentinel is set to the InputSection
 // with the highest address and any InputSections that have mergeable
 // .ARM.exidx table entries are removed from it.
 void ARMExidxSyntheticSection::finalizeContents() {
+  if (script->hasSectionsCommand) {
+    // The executableSections and exidxSections that we use to derive the
+    // final contents of this SyntheticSection are populated before the
+    // linker script assigns InputSections to OutputSections. The linker script
+    // SECTIONS command may have a /DISCARD/ entry that removes executable
+    // InputSections and their dependent .ARM.exidx section that we recorded
+    // earlier.
+    auto isDiscarded = [](const InputSection *isec) { return !isec->isLive(); };
+    llvm::erase_if(executableSections, isDiscarded);
+    llvm::erase_if(exidxSections, isDiscarded);
+  }
+
   // Sort the executable sections that may or may not have associated
   // .ARM.exidx sections by order of ascending address. This requires the
   // relative positions of InputSections to be known.
   auto compareByFilePosition = [](const InputSection *a,
                                   const InputSection *b) {
     OutputSection *aOut = a->getParent();
     OutputSection *bOut = b->getParent();
 

lld/trunk/test/ELF/arm-exidx-partial-discard.s

+// REQUIRES: arm
+// RUN: llvm-mc -filetype=obj -triple arm-gnu-linux-eabi -mcpu cortex-a7 -arm-add-build-attributes %s -o %t.o
+// RUN: echo "SECTIONS { . = 0x10000; .text : { *(.text) } /DISCARD/ : { *(.exit.text) } }" > %t.script
+// RUN: ld.lld -T %t.script %t.o -o %t.elf
+// RUN: llvm-readobj -x .ARM.exidx --sections %t.elf | FileCheck %s
+
+// CHECK-NOT: .exit.text
+/// Expect 2 entries both CANTUNWIND as the .ARM.exidx.exit.text
+// should have been removed.
+// CHECK: Hex dump of section '.ARM.exidx':
+// CHECK-NEXT: 0x00010000 10000000 01000000 10000000 01000000
+
+/// The /DISCARD/ is evaluated after sections have been assigned to the
+/// .ARM.exidx synthetic section. We must account for the /DISCARD/
+ .section .exit.text, "ax", %progbits
+ .globl foo
+ .type foo, %function
+foo:
+ .fnstart
+ bx lr
+ .save {r7, lr}
+ .setfp r7, sp, #0
+ .fnend
+
+ .text
+ .globl _start
+ .type _start, %function
+_start:
+ .fnstart
+ bx lr
+ .cantunwind
+ .fnend
+
+ .section .text.__aeabi_unwind_cpp_pr0, "ax", %progbits
+ .global __aeabi_unwind_cpp_pr0
+__aeabi_unwind_cpp_pr0:
+ bx lr