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

[Rafael Avila de Espindola via llvm-commits]

LGTM. Lets get the crash fixed and iterate.

With this the sentinel is after the symbol assignment, right? I wonder
if you can in a followup patch move the sentinel creation to
resolveShfLinkOrder. In there you already have the ISD in which to
insert in and you can pass the pointer to the last real section to the
constructor so that writeTo doesn't have to search.

Cheers,
Rafael

Igor Kudrin via Phabricator <reviews at reviews.llvm.org> writes:

> ikudrin updated this revision to Diff 126704.
> ikudrin retitled this revision from "[ELF] Fix placement of a sentinel entry in the .ARM.exidx section." to "[ELF] Prevent crash in writing an .ARM.exidx sentinel entry.".
> ikudrin edited the summary of this revision.
> ikudrin added a comment.
>
> - Limited the patch to fix only the crash in `ARMExidxSentinelSection::writeTo()`.
>
>
> https://reviews.llvm.org/D41105
>
> Files:
>   ELF/SyntheticSections.cpp
>   test/ELF/linkerscript/arm-exidx-sentinel-and-assignment.s
>
>
> Index: test/ELF/linkerscript/arm-exidx-sentinel-and-assignment.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/linkerscript/arm-exidx-sentinel-and-assignment.s
> @@ -0,0 +1,12 @@
> +# 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
> Index: ELF/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -2569,16 +2569,22 @@
>  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();
>
>
> Index: test/ELF/linkerscript/arm-exidx-sentinel-and-assignment.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/linkerscript/arm-exidx-sentinel-and-assignment.s
> @@ -0,0 +1,12 @@
> +# 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
> Index: ELF/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -2569,16 +2569,22 @@
>  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();