[PATCH] D25127: [ARM][LLD] .ARM.exidx support for executables/shared libraries

[Rui Ueyama via llvm-commits]

ruiu added a comment.

Generally looking good, a few minor comments.



> InputSection.cpp:125
> +InputSectionBase<ELFT>::getLinkOrderDep() const {
> +  const typename ELFT::Shdr *Hdr = getSectionHdr();
> +  if ((Hdr->sh_flags & SHF_LINK_ORDER) && Hdr->sh_link != 0)

`typename ELFT::Shdr` -> `ELF_Shdr`

> MarkLive.cpp:257-272
> +  // Mark all .ARM.exidx sections that reference live sections
> +  if (Config->EMachine == EM_ARM) {
> +    for (ObjectFile<ELFT> *F : Symtab<ELFT>::X->getObjectFiles()) {
> +      for (InputSectionBase<ELFT> *Sec : F->getSections()) {
> +        if (Sec && Sec != &InputSection<ELFT>::Discarded &&
> +            Sec->Name.startswith(".ARM.exidx")) {
> +          auto* D = Sec->getLinkOrderDep();

Don't you have to repeat this process until it converges? I wonder if forEachSuccessor() makes more sections live, which in turn makes more .ARM.exidx sections live.

> OutputSections.cpp:882-883
> +  // SHF_LINK_ORDER only has meaning in relocatable objects
> +  if (!Config->Relocatable && (this->Header.sh_flags & SHF_LINK_ORDER))
> +    this->Header.sh_flags &= ~SHF_LINK_ORDER;
>    if (Type != SHT_RELA && Type != SHT_REL)

This is equivalent to this.

  if (!Config->Relocatable)
    this->Header.sh_flags &= ~SHF_LINK_ORDER;

> Writer.cpp:1409
>  
> +// Convert the .ARM.Exidx table entries that use relative PREL31 offsets to
> +// Absolute addresses. This form is internal to LLD and is only used to

Exidx -> exidx

> Writer.cpp:1448
> +static void SortARMExidx(uint8_t *Buf, uint64_t OutSecVA, uint64_t Size) {
> +
> +  struct ARMExidxEntry {

Remove the blank line.

> Writer.cpp:1482
> +  OutputSectionBase<ELFT> *ARMExidx = findSection(".ARM.exidx");
> +  if (!Config->Relocatable) {
> +    if (auto OS = dyn_cast_or_null<OutputSection<ELFT>>(ARMExidx))

Remove {}

> Writer.cpp:1483
> +  if (!Config->Relocatable) {
> +    if (auto OS = dyn_cast_or_null<OutputSection<ELFT>>(ARMExidx))
> +      SortARMExidx(Buf + OS->getFileOff(), OS->getVA(), OS->getSize());

OS -> *OS

https://reviews.llvm.org/D25127