[PATCH] D34956: [ELF] - Store pointer to PT_LOAD instead of pointer to first section (FirstInPtLoad) in OutputSection

[Rafael Avila de Espindola via llvm-commits]

So, regardless of any desire to change when lld allocates the headers,
this, patch is an improvement, no?

LGTM, Rui, what do you think?

Cheers,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 108432.
> grimar added a comment.
>
> - Rebased.
>
>
> https://reviews.llvm.org/D34956
>
> Files:
>   ELF/InputSection.cpp
>   ELF/LinkerScript.cpp
>   ELF/OutputSections.h
>   ELF/Writer.cpp
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -751,7 +751,7 @@
>      First = Sec;
>    p_align = std::max(p_align, Sec->Alignment);
>    if (p_type == PT_LOAD)
> -    Sec->FirstInPtLoad = First;
> +    Sec->Load = this;
>  }
>  
>  template <class ELFT>
> @@ -1592,7 +1592,7 @@
>  // virtual address (modulo the page size) so that the loader can load
>  // executables without any address adjustment.
>  static uint64_t getFileAlignment(uint64_t Off, OutputSection *Sec) {
> -  OutputSection *First = Sec->FirstInPtLoad;
> +  OutputSection *First = Sec->Load ? Sec->Load->First : nullptr;
>    // If the section is not in a PT_LOAD, we just have to align it.
>    if (!First)
>      return alignTo(Off, Sec->Alignment);
> Index: ELF/OutputSections.h
> ===================================================================
> --- ELF/OutputSections.h
> +++ ELF/OutputSections.h
> @@ -59,13 +59,14 @@
>        Alignment = Val;
>    }
>  
> -  // Pointer to the first section in PT_LOAD segment, which this section
> -  // also resides in. This field is used to correctly compute file offset
> -  // of a section. When two sections share the same load segment, difference
> -  // between their file offsets should be equal to difference between their
> -  // virtual addresses. To compute some section offset we use the following
> -  // formula: Off = Off_first + VA - VA_first.
> -  OutputSection *FirstInPtLoad = nullptr;
> +  // Pointer to the PT_LOAD segment, which this section resides in. This field
> +  // is used to correctly compute file offset of a section. When two sections
> +  // share the same load segment, difference between their file offsets should
> +  // be equal to difference between their virtual addresses. To compute some
> +  // section offset we use the following formula: Off = Off_first + VA -
> +  // VA_first, where Off_first and VA_first is file offset and VA of first
> +  // section in PT_LOAD.
> +  PhdrEntry *Load = nullptr;
>  
>    // Pointer to a relocation section for this section. Usually nullptr because
>    // we consume relocations, but if --emit-relocs is specified (which is rare),
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -761,6 +761,13 @@
>    removeEmptyCommands();
>  }
>  
> +static OutputSection *getFirstSection(PhdrEntry *Load) {
> +  for (OutputSectionCommand *Cmd : OutputSectionCommands)
> +    if (Cmd->Sec->Load == Load)
> +      return Cmd->Sec;
> +  return nullptr;
> +}
> +
>  void LinkerScript::allocateHeaders(std::vector<PhdrEntry *> &Phdrs) {
>    uint64_t Min = std::numeric_limits<uint64_t>::max();
>    for (OutputSectionCommand *Cmd : OutputSectionCommands) {
> @@ -784,24 +791,9 @@
>    }
>  
>    assert(FirstPTLoad->First == Out::ElfHeader);
> -  OutputSection *ActualFirst = nullptr;
> -  for (OutputSectionCommand *Cmd : OutputSectionCommands) {
> -    OutputSection *Sec = Cmd->Sec;
> -    if (Sec->FirstInPtLoad == Out::ElfHeader) {
> -      ActualFirst = Sec;
> -      break;
> -    }
> -  }
> -  if (ActualFirst) {
> -    for (OutputSectionCommand *Cmd : OutputSectionCommands) {
> -      OutputSection *Sec = Cmd->Sec;
> -      if (Sec->FirstInPtLoad == Out::ElfHeader)
> -        Sec->FirstInPtLoad = ActualFirst;
> -    }
> -    FirstPTLoad->First = ActualFirst;
> -  } else {
> -    Phdrs.erase(It);
> -  }
> +  Out::ElfHeader->Load = nullptr;
> +  Out::ProgramHeaders->Load = nullptr;
> +  FirstPTLoad->First = getFirstSection(FirstPTLoad);
>  
>    auto PhdrI = llvm::find_if(
>        Phdrs, [](const PhdrEntry *E) { return E->p_type == PT_PHDR; });
> Index: ELF/InputSection.cpp
> ===================================================================
> --- ELF/InputSection.cpp
> +++ ELF/InputSection.cpp
> @@ -467,9 +467,9 @@
>  // of the RW segment.
>  static uint64_t getARMStaticBase(const SymbolBody &Body) {
>    OutputSection *OS = Body.getOutputSection();
> -  if (!OS || !OS->FirstInPtLoad)
> +  if (!OS || !OS->Load || !OS->Load->First)
>      fatal("SBREL relocation to " + Body.getName() + " without static base");
> -  return OS->FirstInPtLoad->Addr;
> +  return OS->Load->First->Addr;
>  }
>  
>  static uint64_t getRelocTargetVA(uint32_t Type, int64_t A, uint64_t P,