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

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 3 10:59:13 PDT 2017


LGTM

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

> grimar created this revision.
> Herald added a subscriber: emaste.
>
> I found that it is probably a bit more convinent and helps to simplify logic 
> of program headers allocation a little.
>
>
> https://reviews.llvm.org/D34956
>
> Files:
>   ELF/InputSection.cpp
>   ELF/LinkerScript.cpp
>   ELF/LinkerScript.h
>   ELF/OutputSections.h
>   ELF/Writer.cpp
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -57,9 +57,9 @@
>    void finalizeSections();
>    void addPredefinedSections();
>  
> -  std::vector<PhdrEntry> createPhdrs();
> +  std::vector<PhdrEntry *> createPhdrs();
>    void removeEmptyPTLoad();
> -  void addPtArmExid(std::vector<PhdrEntry> &Phdrs);
> +  void addPtArmExid(std::vector<PhdrEntry *> &Phdrs);
>    void assignFileOffsets();
>    void assignFileOffsetsBinary();
>    void setPhdrs();
> @@ -83,7 +83,7 @@
>    OutputSection *findSectionInScript(StringRef Name);
>    OutputSectionCommand *findSectionCommand(StringRef Name);
>  
> -  std::vector<PhdrEntry> Phdrs;
> +  std::vector<PhdrEntry *> Phdrs;
>  
>    uint64_t FileSize;
>    uint64_t SectionHeaderOff;
> @@ -126,12 +126,12 @@
>  template <class ELFT> void elf::writeResult() { Writer<ELFT>().run(); }
>  
>  template <class ELFT> void Writer<ELFT>::removeEmptyPTLoad() {
> -  auto I = std::remove_if(Phdrs.begin(), Phdrs.end(), [&](const PhdrEntry &P) {
> -    if (P.p_type != PT_LOAD)
> +  auto I = llvm::remove_if(Phdrs, [&](const PhdrEntry *P) {
> +    if (P->p_type != PT_LOAD)
>        return false;
> -    if (!P.First)
> +    if (!P->First)
>        return true;
> -    uint64_t Size = P.Last->Addr + P.Last->Size - P.First->Addr;
> +    uint64_t Size = P->Last->Addr + P->Last->Size - P->First->Addr;
>      return Size == 0;
>    });
>    Phdrs.erase(I, Phdrs.end());
> @@ -748,7 +748,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>
> @@ -1420,11 +1420,11 @@
>  
>  // Decide which program headers to create and which sections to include in each
>  // one.
> -template <class ELFT> std::vector<PhdrEntry> Writer<ELFT>::createPhdrs() {
> -  std::vector<PhdrEntry> Ret;
> +template <class ELFT> std::vector<PhdrEntry *> Writer<ELFT>::createPhdrs() {
> +  std::vector<PhdrEntry *> Ret;
>    auto AddHdr = [&](unsigned Type, unsigned Flags) -> PhdrEntry * {
> -    Ret.emplace_back(Type, Flags);
> -    return &Ret.back();
> +    Ret.push_back(make<PhdrEntry>(Type, Flags));
> +    return Ret.back();
>    };
>  
>    // The first phdr entry is PT_PHDR which describes the program header itself.
> @@ -1464,30 +1464,30 @@
>    }
>  
>    // Add a TLS segment if any.
> -  PhdrEntry TlsHdr(PT_TLS, PF_R);
> +  PhdrEntry *TlsHdr = make<PhdrEntry>(PT_TLS, PF_R);
>    for (OutputSectionCommand *Cmd : OutputSectionCommands) {
>      OutputSection *Sec = Cmd->Sec;
>      if (Sec->Flags & SHF_TLS)
> -      TlsHdr.add(Sec);
> +      TlsHdr->add(Sec);
>    }
> -  if (TlsHdr.First)
> -    Ret.push_back(std::move(TlsHdr));
> +  if (TlsHdr->First)
> +    Ret.push_back(TlsHdr);
>  
>    // Add an entry for .dynamic.
>    if (InX::DynSymTab)
>      AddHdr(PT_DYNAMIC, InX::Dynamic->getParent()->getPhdrFlags())
>          ->add(InX::Dynamic->getParent());
>  
>    // PT_GNU_RELRO includes all sections that should be marked as
>    // read-only by dynamic linker after proccessing relocations.
> -  PhdrEntry RelRo(PT_GNU_RELRO, PF_R);
> +  PhdrEntry *RelRo = make<PhdrEntry>(PT_GNU_RELRO, PF_R);
>    for (OutputSectionCommand *Cmd : OutputSectionCommands) {
>      OutputSection *Sec = Cmd->Sec;
>      if (needsPtLoad(Sec) && isRelroSection(Sec))
> -      RelRo.add(Sec);
> +      RelRo->add(Sec);
>    }
> -  if (RelRo.First)
> -    Ret.push_back(std::move(RelRo));
> +  if (RelRo->First)
> +    Ret.push_back(RelRo);
>  
>    // PT_GNU_EH_FRAME is a special section pointing on .eh_frame_hdr.
>    if (!In<ELFT>::EhFrame->empty() && In<ELFT>::EhFrameHdr &&
> @@ -1534,7 +1534,7 @@
>  }
>  
>  template <class ELFT>
> -void Writer<ELFT>::addPtArmExid(std::vector<PhdrEntry> &Phdrs) {
> +void Writer<ELFT>::addPtArmExid(std::vector<PhdrEntry *> &Phdrs) {
>    if (Config->EMachine != EM_ARM)
>      return;
>    auto I =
> @@ -1546,8 +1546,8 @@
>      return;
>  
>    // PT_ARM_EXIDX is the ARM EHABI equivalent of PT_GNU_EH_FRAME
> -  PhdrEntry ARMExidx(PT_ARM_EXIDX, PF_R);
> -  ARMExidx.add((*I)->Sec);
> +  PhdrEntry *ARMExidx = make<PhdrEntry>(PT_ARM_EXIDX, PF_R);
> +  ARMExidx->add((*I)->Sec);
>    Phdrs.push_back(ARMExidx);
>  }
>  
> @@ -1563,20 +1563,20 @@
>        };
>    };
>  
> -  for (const PhdrEntry &P : Phdrs)
> -    if (P.p_type == PT_LOAD && P.First)
> -      PageAlign(P.First);
> +  for (const PhdrEntry *P : Phdrs)
> +    if (P->p_type == PT_LOAD && P->First)
> +      PageAlign(P->First);
>  
> -  for (const PhdrEntry &P : Phdrs) {
> -    if (P.p_type != PT_GNU_RELRO)
> +  for (const PhdrEntry *P : Phdrs) {
> +    if (P->p_type != PT_GNU_RELRO)
>        continue;
> -    if (P.First)
> -      PageAlign(P.First);
> +    if (P->First)
> +      PageAlign(P->First);
>      // Find the first section after PT_GNU_RELRO. If it is in a PT_LOAD we
>      // have to align it to a page.
>      auto End = OutputSectionCommands.end();
>      auto I =
> -        std::find(OutputSectionCommands.begin(), End, Script->getCmd(P.Last));
> +        std::find(OutputSectionCommands.begin(), End, Script->getCmd(P->Last));
>      if (I == End || (I + 1) == End)
>        continue;
>      OutputSection *Sec = (*(I + 1))->Sec;
> @@ -1590,7 +1590,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);
> @@ -1643,35 +1643,35 @@
>  // Finalize the program headers. We call this function after we assign
>  // file offsets and VAs to all sections.
>  template <class ELFT> void Writer<ELFT>::setPhdrs() {
> -  for (PhdrEntry &P : Phdrs) {
> -    OutputSection *First = P.First;
> -    OutputSection *Last = P.Last;
> +  for (PhdrEntry *P : Phdrs) {
> +    OutputSection *First = P->First;
> +    OutputSection *Last = P->Last;
>      if (First) {
> -      P.p_filesz = Last->Offset - First->Offset;
> +      P->p_filesz = Last->Offset - First->Offset;
>        if (Last->Type != SHT_NOBITS)
> -        P.p_filesz += Last->Size;
> -      P.p_memsz = Last->Addr + Last->Size - First->Addr;
> -      P.p_offset = First->Offset;
> -      P.p_vaddr = First->Addr;
> -      if (!P.HasLMA)
> -        P.p_paddr = First->getLMA();
> +        P->p_filesz += Last->Size;
> +      P->p_memsz = Last->Addr + Last->Size - First->Addr;
> +      P->p_offset = First->Offset;
> +      P->p_vaddr = First->Addr;
> +      if (!P->HasLMA)
> +        P->p_paddr = First->getLMA();
>      }
> -    if (P.p_type == PT_LOAD)
> -      P.p_align = Config->MaxPageSize;
> -    else if (P.p_type == PT_GNU_RELRO) {
> -      P.p_align = 1;
> +    if (P->p_type == PT_LOAD)
> +      P->p_align = Config->MaxPageSize;
> +    else if (P->p_type == PT_GNU_RELRO) {
> +      P->p_align = 1;
>        // The glibc dynamic loader rounds the size down, so we need to round up
>        // to protect the last page. This is a no-op on FreeBSD which always
>        // rounds up.
> -      P.p_memsz = alignTo(P.p_memsz, Target->PageSize);
> +      P->p_memsz = alignTo(P->p_memsz, Target->PageSize);
>      }
>  
>      // The TLS pointer goes after PT_TLS. At least glibc will align it,
>      // so round up the size to make sure the offsets are correct.
> -    if (P.p_type == PT_TLS) {
> -      Out::TlsPhdr = &P;
> -      if (P.p_memsz)
> -        P.p_memsz = alignTo(P.p_memsz, P.p_align);
> +    if (P->p_type == PT_TLS) {
> +      Out::TlsPhdr = P;
> +      if (P->p_memsz)
> +        P->p_memsz = alignTo(P->p_memsz, P->p_align);
>      }
>    }
>  }
> @@ -1725,14 +1725,14 @@
>    PhdrEntry *Last = nullptr;
>    PhdrEntry *LastRO = nullptr;
>    PhdrEntry *LastRW = nullptr;
> -  for (PhdrEntry &P : Phdrs) {
> -    if (P.p_type != PT_LOAD)
> +  for (PhdrEntry *P : Phdrs) {
> +    if (P->p_type != PT_LOAD)
>        continue;
> -    Last = &P;
> -    if (P.p_flags & PF_W)
> -      LastRW = &P;
> +    Last = P;
> +    if (P->p_flags & PF_W)
> +      LastRW = P;
>      else
> -      LastRO = &P;
> +      LastRO = P;
>    }
>  
>    auto Set = [](DefinedRegular *S, OutputSection *Sec, uint64_t Value) {
> @@ -1809,15 +1809,15 @@
>  
>    // Write the program header table.
>    auto *HBuf = reinterpret_cast<Elf_Phdr *>(Buf + EHdr->e_phoff);
> -  for (PhdrEntry &P : Phdrs) {
> -    HBuf->p_type = P.p_type;
> -    HBuf->p_flags = P.p_flags;
> -    HBuf->p_offset = P.p_offset;
> -    HBuf->p_vaddr = P.p_vaddr;
> -    HBuf->p_paddr = P.p_paddr;
> -    HBuf->p_filesz = P.p_filesz;
> -    HBuf->p_memsz = P.p_memsz;
> -    HBuf->p_align = P.p_align;
> +  for (PhdrEntry *P : Phdrs) {
> +    HBuf->p_type = P->p_type;
> +    HBuf->p_flags = P->p_flags;
> +    HBuf->p_offset = P->p_offset;
> +    HBuf->p_vaddr = P->p_vaddr;
> +    HBuf->p_paddr = P->p_paddr;
> +    HBuf->p_filesz = P->p_filesz;
> +    HBuf->p_memsz = P->p_memsz;
> +    HBuf->p_align = P->p_align;
>      ++HBuf;
>    }
>  
> 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.h
> ===================================================================
> --- ELF/LinkerScript.h
> +++ ELF/LinkerScript.h
> @@ -273,15 +273,15 @@
>    void adjustSectionsBeforeSorting();
>    void adjustSectionsAfterSorting();
>  
> -  std::vector<PhdrEntry> createPhdrs();
> +  std::vector<PhdrEntry *> createPhdrs();
>    bool ignoreInterpSection();
>  
>    bool hasLMA(OutputSection *Sec);
>    bool shouldKeep(InputSectionBase *S);
>    void assignOffsets(OutputSectionCommand *Cmd);
>    void createOrphanCommands();
>    void processNonSectionCommands();
> -  void assignAddresses(std::vector<PhdrEntry> &Phdrs);
> +  void assignAddresses(std::vector<PhdrEntry *> &Phdrs);
>  
>    void addSymbol(SymbolAssignment *Cmd);
>    void processCommands(OutputSectionFactory &Factory);
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -784,13 +784,20 @@
>    }
>  }
>  
> +static OutputSection *getFirstSection(PhdrEntry *Load) {
> +  for (OutputSectionCommand *Cmd : OutputSectionCommands)
> +    if (Cmd->Sec->Load == Load)
> +      return Cmd->Sec;
> +  return nullptr;
> +}
> +
>  static void
> -allocateHeaders(std::vector<PhdrEntry> &Phdrs,
> +allocateHeaders(std::vector<PhdrEntry *> &Phdrs,
>                  ArrayRef<OutputSectionCommand *> OutputSectionCommands,
>                  uint64_t Min) {
> -  auto FirstPTLoad = llvm::find_if(
> -      Phdrs, [](const PhdrEntry &E) { return E.p_type == PT_LOAD; });
> -  if (FirstPTLoad == Phdrs.end())
> +  auto LoadI = llvm::find_if(
> +      Phdrs, [](const PhdrEntry *E) { return E->p_type == PT_LOAD; });
> +  if (LoadI == Phdrs.end())
>      return;
>  
>    uint64_t HeaderSize = getHeaderSize();
> @@ -801,33 +808,18 @@
>      return;
>    }
>  
> -  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(FirstPTLoad);
> -  }
> +  assert((*LoadI)->First == Out::ElfHeader);
> +  Out::ElfHeader->Load = nullptr;
> +  Out::ProgramHeaders->Load = nullptr;
> +  (*LoadI)->First = getFirstSection(*LoadI);
>  
>    auto PhdrI = llvm::find_if(
> -      Phdrs, [](const PhdrEntry &E) { return E.p_type == PT_PHDR; });
> +      Phdrs, [](const PhdrEntry *E) { return E->p_type == PT_PHDR; });
>    if (PhdrI != Phdrs.end())
>      Phdrs.erase(PhdrI);
>  }
>  
> -void LinkerScript::assignAddresses(std::vector<PhdrEntry> &Phdrs) {
> +void LinkerScript::assignAddresses(std::vector<PhdrEntry *> &Phdrs) {
>    // Assign addresses as instructed by linker script SECTIONS sub-commands.
>    Dot = 0;
>    ErrorOnMissingSection = true;
> @@ -859,35 +851,36 @@
>  }
>  
>  // Creates program headers as instructed by PHDRS linker script command.
> -std::vector<PhdrEntry> LinkerScript::createPhdrs() {
> -  std::vector<PhdrEntry> Ret;
> +std::vector<PhdrEntry *> LinkerScript::createPhdrs() {
> +  std::vector<PhdrEntry *> Ret;
>  
>    // Process PHDRS and FILEHDR keywords because they are not
>    // real output sections and cannot be added in the following loop.
>    for (const PhdrsCommand &Cmd : Opt.PhdrsCommands) {
> -    Ret.emplace_back(Cmd.Type, Cmd.Flags == UINT_MAX ? PF_R : Cmd.Flags);
> -    PhdrEntry &Phdr = Ret.back();
> +    PhdrEntry *Phdr =
> +        make<PhdrEntry>(Cmd.Type, Cmd.Flags == UINT_MAX ? PF_R : Cmd.Flags);
>  
>      if (Cmd.HasFilehdr)
> -      Phdr.add(Out::ElfHeader);
> +      Phdr->add(Out::ElfHeader);
>      if (Cmd.HasPhdrs)
> -      Phdr.add(Out::ProgramHeaders);
> +      Phdr->add(Out::ProgramHeaders);
>  
>      if (Cmd.LMAExpr) {
> -      Phdr.p_paddr = Cmd.LMAExpr().getValue();
> -      Phdr.HasLMA = true;
> +      Phdr->p_paddr = Cmd.LMAExpr().getValue();
> +      Phdr->HasLMA = true;
>      }
> +    Ret.push_back(Phdr);
>    }
>  
>    // Add output sections to program headers.
>    for (OutputSectionCommand *Cmd : OutputSectionCommands) {
>      OutputSection *Sec = Cmd->Sec;
>  
>      // Assign headers specified by linker script
>      for (size_t Id : getPhdrIndices(Sec)) {
> -      Ret[Id].add(Sec);
> +      Ret[Id]->add(Sec);
>        if (Opt.PhdrsCommands[Id].Flags == UINT_MAX)
> -        Ret[Id].p_flags |= Sec->getPhdrFlags();
> +        Ret[Id]->p_flags |= Sec->getPhdrFlags();
>      }
>    }
>    return Ret;
> Index: ELF/InputSection.cpp
> ===================================================================
> --- ELF/InputSection.cpp
> +++ ELF/InputSection.cpp
> @@ -465,9 +465,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\n");
> -  return OS->FirstInPtLoad->Addr;
> +  return OS->Load->First->Addr;
>  }
>  
>  static uint64_t getRelocTargetVA(uint32_t Type, int64_t A, uint64_t P,


More information about the llvm-commits mailing list