[ELF] Prevent crash in writing an .ARM.exidx sentinel entry.

[ikudrin]

Summary

We might crash in ARMExidxSentinelSection::writeTo() because it expects that
the sentinel entry is put in the same InputSectionDescription as the last real entry,
but if a linker script has anything but an input section description entry at the end
of the output section description for .ARM.exidx, OutputSection::addSection()
puts the sentinel entry into a new bucket.

[ruiu]

As the new code is not obvious, please write comment.

[ikudrin]

• Added a comment.

[peter.smith]

I think that this is the right thing to do for .ARM.exidx sections. I'm pretty sure the original implementation did do something like that, but it must have been refactored sometime later to use addSection(). I don't think that this is expected for orphans in general though. For example consider the silly example: SECTIONS { .text : { t.o(.text) foo = .; } }
If I provide an object t.o with a .text section and an object t2.o with a .text section, the t2.o will be an orphan that will match in .text. The bfd linker places this after the assignment to foo = .

.text          0x0000000000000000        0x4 t.o
                0x0000000000000000                _start
                0x0000000000000004                foo = .
 .text          0x0000000000000004        0x4 t2.o
                0x0000000000000004                func

At first glance it looks like the patch might change this behaviour for orphan sections? This might lead to incompatibility problems with bfd. If I'm right perhaps addSection can be made to optionally choose to find the last InputSectionDescription for cases like .ARM.exidx. Alternatively some custom code could be used for .ARM.exidx as that case is much more specialised.

[ikudrin]

Thanks for the example, Peter! It looks like I have to think on these corner cases a bit more.

[ikudrin]

• Limited the patch to fix only the crash in ARMExidxSentinelSection::writeTo().

[peter.smith]

That looks correct to me. In the test I think it would be useful to check the contents of the table llvm-objdump -s or llvm-readobj -u to check that the sentinel is written in the correct place.

ELF/SyntheticSections.cpp

 // This section will have been sorted last in the .ARM.exidx table.
 // This table entry will have the form:
 // | PREL31 upper bound of code that has exception tables | EXIDX_CANTUNWIND |
 // The sentinel must have the PREL31 value of an address higher than any
 // address described by any other table entry.
 void ARMExidxSentinelSection::writeTo(uint8_t *Buf) {
   // The Sections are sorted in order of ascending PREL31 address with the
   // sentinel last. We need to find the InputSection that precedes the
-  // sentinel. By construction the Sentinel is in the last
-  // InputSectionDescription as the InputSection that precedes it.
+  // sentinel.
   OutputSection *C = getParent();
-  auto ISD =
-      std::find_if(C->SectionCommands.rbegin(), C->SectionCommands.rend(),
-                   [](const BaseCommand *Base) {
-                     return isa<InputSectionDescription>(Base);
-                   });
-  auto L = cast<InputSectionDescription>(*ISD);
-  InputSection *Highest = L->Sections[L->Sections.size() - 2];
+  InputSection *Highest = nullptr;
+  int Skip = 1;
+  for (const BaseCommand *Base : llvm::reverse(C->SectionCommands)) {
+    if (!isa<InputSectionDescription>(Base))
+      continue;
+    auto L = cast<InputSectionDescription>(Base);
+    if (Skip >= L->Sections.size()) {
+      Skip -= L->Sections.size();
+      continue;
+    }
+    Highest = L->Sections[L->Sections.size() - Skip - 1];
+    break;
+  }
+  assert(Highest);
   InputSection *LS = Highest->getLinkOrderDep();
   uint64_t S = LS->getParent()->Addr + LS->getOffset(LS->getSize());
   uint64_t P = getVA();
   Target->relocateOne(Buf, R_ARM_PREL31, S - P);
   write32le(Buf + 4, 1);
 }
 
 ThunkSection::ThunkSection(OutputSection *OS, uint64_t Off)

test/ELF/linkerscript/arm-exidx-sentinel-and-assignment.s

+# REQUIRES: arm
+# RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %s -o %t.o
+# RUN: echo "SECTIONS {                                 \
+# RUN:         .ARM.exidx : { *(.ARM.exidx*) foo = .; } \
+# RUN:       }" > %t.script
+## We used to crash in the case when the last output section command
+## for .ARM.exidx was anything but an input section description.
+# RUN: ld.lld -T %t.script %t.o -shared -o %t.so
+
+.fnstart
+.cantunwind
+.fnend