Split the creation of program headers in a few steps

[• rafael]

Summary

IMHO this makes the code easier to read and should help with linker scripts.

This is strongly based on D16575. The main differences are

• We record a range of sections, not every section in a program header.
• ScanHeaders takes case of deciding what goes in every program header, including PT_GNU_RELRO
• We create dummy sections for the start of the file

With this, program header creation has 3 isolated stages:

1. Map sections to program headers.
2. Assign addresses to *sections*
3. Looking at sections find the address and size of each program header.

[emaste]

ELF/Writer.cpp

45

extra 'm'

[grimar]

That looks better then my approach I think, thanks ! Have a consern about dummies output sections thought.

ELF/Writer.cpp

96

"create"

103

Honestly, I dont like the approach to have dummies like that. Thats looks like a hack (more like a hack in compare with 'special' first load).
Linkerscript allows to put elf header and phdr anywhere afaik. But the question is do we really need to support that ?
In my patch about PHDRS (http://reviews.llvm.org/D16771) I introduced the restriction about that:
first phdr is always PT_PHDR and first load must contain elf and phdr headers. It seems your implementation is really close or the same to mine here (at least it assumes them are at start always), but do we really need to have dummies logic then ?
IMO special first load and less entities looks better here.

187

Let's place them before all other sections as they are usually always go before all other ones ?

[• rafael]

Honestly, I dont like the approach to have dummies like that. Thats looks like a hack (more like a hack in compare with 'special' first load).
Linkerscript allows to put elf header and phdr anywhere afaik. But the question is do we really need to support that ?
In my patch about PHDRS (http://reviews.llvm.org/D16771) I introduced the restriction about that:
first phdr is always PT_PHDR and first load must contain elf and phdr headers. It seems your implementation is really close or the same to mine here (at least it assumes them are at start always), but do we really need to have dummies logic then ?
IMO special first load and less entities looks better here.

I remember seeing that used in a case where it was desired that the
elf and program headers were not part of a PT_LOAD (NaCl). It is
unclear if it needs to be supported, but if it is an "Inventor's
paradox" situation, why not support the more general case?

The problem with having PT_LOAD that is before any section is that we
need special cases in two places:

• The first section in it doesn't need to be page aligned, since it is

not the start of the pt_load.

• Computing its actual position.

Not having a section for the program headers also means PT_PHDR is a
special case. Implementing this patch is how I found the bug I fixed
in r260102.

Comment at: ELF/Writer.cpp:186
@@ -162,2 +185,3 @@
+ Out<ELFT>::ProgramHeaders = &ProgramHeaders;

Writer<ELFT>(*Symtab).run();

---

Let's place them before all other sections as they are usually always go before all other ones ?

Not sure I follow. They are already at the start.

I will upload a rebased/fixed patch.

Cheers,
Rafael

[• rafael]

Rebased and comment fixes.

[ruiu]

ELF/Writer.cpp

53

You can "Elf_Phdr H = {}" here, can't you?

54–55

I think I like this idea. I was thinking to make Phdr a list of output sections, but that was too flexible. This two-pointer approach is on the other hand directly reflects what we actually have in an ELF file.

95

s/a a/a/
s/elf/ELF/

96

I think we usually omit int.

139

Move this to beginning of the function.

1200–1202

You may want to return a pointer instead of a reference because in a few places in this function you wrote "&AddHdr".

1255

// Add the TLS segment unless it's empty.

1287–1288

I think this function call worth to be moved to Writer::run because now PHDRs construction and address assignments are nicely separated. You may want to rename scanHeaders to createPhdrs or something like that.

1297–1298

Why don't you use a range-based for loop?

1299–1305

Can you add a comment for this piece of code? It is not obvious.

1325

Why process only needsPhdr sections?

1354

I wouldn't use auto here.

1447–1451

Instead of uint8_t *, you can cast to Elf_Phdr *.

[grimar]

ELF/Writer.cpp

1200–1202

I agree here, that also eliminates the need of direct specify of return type for functor (it usually doesn't look good, here as well).

1299–1305

If you're not using range-loop (but I think it would be ok here),
could you not use the variable name that replaces the name in global scope ? I mean you already have "I" above.

[• rafael]

Comment at: ELF/Writer.cpp:1324
@@ +1323,3 @@
+
+ if (needsPhdr<ELFT>(Sec)) {

+ // Don't allocate VA space for TLS NOBITS sections. The PT_TLS PHDR is

Why process only needsPhdr sections?

Other sections don't have a VA, which is what we set in the if body.

I will upload a patch with the other suggestions.

Cheers,
Rafael

[• rafael]

Address Rui's and George's comments.

[grimar]

LGTM. But that is just mine opinion, lets wait to see Rui's one. Btw, thanks, that looks better than mine version I think !

[ruiu]

LGTM

ELF/Writer.cpp

1261

nit: Can it fit in 80 cols if you rename H?

1325

Please leave a comment that sections that are not in any PHDRs don't have VA (because they are not mapped to anywhere at runtime).

1448–1451

I think you can do

for (Phdr & P : Phdrs)
  *HBuf++ = P.H;

[grimar]

Was commited as r260453

ELF/OutputSections.h

public:
   }
   uint32_t getType() { return Header.sh_type; }
   void updateAlign(uintX_t Align) {
     if (Align > Header.sh_addralign)
       Header.sh_addralign = Align;
   }
 
   virtual void finalize() {}
-  virtual void writeTo(uint8_t *Buf) = 0;
+  virtual void writeTo(uint8_t *Buf) {}
   virtual ~OutputSectionBase() = default;
 
 protected:
   StringRef Name;
   Elf_Shdr Header;
 };
 
 template <class ELFT> class GotSection final : public OutputSectionBase<ELFT> {
template <class ELFT> struct Out {
   static RelocationSection<ELFT> *RelaDyn;
   static RelocationSection<ELFT> *RelaPlt;
   static StringTableSection<ELFT> *DynStrTab;
   static StringTableSection<ELFT> *ShStrTab;
   static StringTableSection<ELFT> *StrTab;
   static SymbolTableSection<ELFT> *DynSymTab;
   static SymbolTableSection<ELFT> *SymTab;
   static Elf_Phdr *TlsPhdr;
+  static OutputSectionBase<ELFT> *ElfHeader;
+  static OutputSectionBase<ELFT> *ProgramHeaders;
 };
 
 template <class ELFT> DynamicSection<ELFT> *Out<ELFT>::Dynamic;
 template <class ELFT> EhFrameHeader<ELFT> *Out<ELFT>::EhFrameHdr;
 template <class ELFT> GnuHashTableSection<ELFT> *Out<ELFT>::GnuHashTab;
 template <class ELFT> GotPltSection<ELFT> *Out<ELFT>::GotPlt;
 template <class ELFT> GotSection<ELFT> *Out<ELFT>::Got;
 template <class ELFT> HashTableSection<ELFT> *Out<ELFT>::HashTab;
 template <class ELFT> InterpSection<ELFT> *Out<ELFT>::Interp;
 template <class ELFT> OutputSection<ELFT> *Out<ELFT>::Bss;
 template <class ELFT> OutputSection<ELFT> *Out<ELFT>::MipsRldMap;
 template <class ELFT> OutputSectionBase<ELFT> *Out<ELFT>::Opd;
 template <class ELFT> uint8_t *Out<ELFT>::OpdBuf;
 template <class ELFT> PltSection<ELFT> *Out<ELFT>::Plt;
 template <class ELFT> RelocationSection<ELFT> *Out<ELFT>::RelaDyn;
 template <class ELFT> RelocationSection<ELFT> *Out<ELFT>::RelaPlt;
 template <class ELFT> StringTableSection<ELFT> *Out<ELFT>::DynStrTab;
 template <class ELFT> StringTableSection<ELFT> *Out<ELFT>::ShStrTab;
 template <class ELFT> StringTableSection<ELFT> *Out<ELFT>::StrTab;
 template <class ELFT> SymbolTableSection<ELFT> *Out<ELFT>::DynSymTab;
 template <class ELFT> SymbolTableSection<ELFT> *Out<ELFT>::SymTab;
 template <class ELFT> typename Out<ELFT>::Elf_Phdr *Out<ELFT>::TlsPhdr;
+template <class ELFT> OutputSectionBase<ELFT> *Out<ELFT>::ElfHeader;
+template <class ELFT> OutputSectionBase<ELFT> *Out<ELFT>::ProgramHeaders;
 
 } // namespace elf2
 } // namespace lld
 
 #endif // LLD_ELF_OUTPUT_SECTIONS_H

ELF/Writer.cpp

 //===- Writer.cpp ---------------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Writer.h"
 #include "Config.h"
 #include "OutputSections.h"
 #include "SymbolTable.h"
 #include "Target.h"
 
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 using namespace llvm::ELF;
public:
   typedef typename ELFFile<ELFT>::Elf_Phdr Elf_Phdr;
   typedef typename ELFFile<ELFT>::Elf_Sym Elf_Sym;
   typedef typename ELFFile<ELFT>::Elf_Sym_Range Elf_Sym_Range;
   typedef typename ELFFile<ELFT>::Elf_Rela Elf_Rela;
   Writer(SymbolTable<ELFT> &S) : Symtab(S) {}
   void run();
 
 private:
+  // This describes a program header entry.
+  // Each contains type, access flags and range of output sections that will be
+  // placed in it.
+  struct Phdr {
+    Phdr(unsigned Type, unsigned Flags) {
+      H.p_type = Type;
+      H.p_flags = Flags;
+    }
+    Elf_Phdr H = {};
+    OutputSectionBase<ELFT> *First = nullptr;
+    OutputSectionBase<ELFT> *Last = nullptr;
+  };
+
   void copyLocalSymbols();
   void addReservedSymbols();
   bool createSections();
   void addPredefinedSections();
 
   template <bool isRela>
   void scanRelocs(InputSectionBase<ELFT> &C,
                   iterator_range<const Elf_Rel_Impl<ELFT, isRela> *> Rels);
 
   void scanRelocs(InputSection<ELFT> &C);
   void scanRelocs(InputSectionBase<ELFT> &S, const Elf_Shdr &RelSec);
-  void updateRelro(Elf_Phdr *Cur, Elf_Phdr *GnuRelroPhdr, uintX_t VA);
+  void createPhdrs();
   void assignAddresses();
   void buildSectionMap();
   void fixAbsoluteSymbols();
   bool openFile();
   void writeHeader();
   void writeSections();
   bool isDiscarded(InputSectionBase<ELFT> *IS) const;
   StringRef getOutputSectionName(StringRef S) const;
   bool needsInterpSection() const {
     return !Symtab.getSharedFiles().empty() && !Config->DynamicLinker.empty();
   }
   bool isOutputDynamic() const {
     return !Symtab.getSharedFiles().empty() || Config->Shared;
   }
-  int getPhdrsNum() const;
 
   OutputSection<ELFT> *getBss();
   void addCommonSymbols(std::vector<DefinedCommon *> &Syms);
   void addCopyRelSymbols(std::vector<SharedSymbol<ELFT> *> &Syms);
 
   std::unique_ptr<llvm::FileOutputBuffer> Buffer;
 
   BumpPtrAllocator Alloc;
   std::vector<OutputSectionBase<ELFT> *> OutputSections;
   std::vector<std::unique_ptr<OutputSectionBase<ELFT>>> OwningSections;
-  unsigned getNumSections() const { return OutputSections.size() + 1; }
+
+  // We create a section for the ELF header and one for the program headers.
+  const unsigned NumDummySections = 2;
+  ArrayRef<OutputSectionBase<ELFT> *> getSections() const {
+    return makeArrayRef(OutputSections).slice(NumDummySections);
+  }
+  unsigned getNumSections() const {
+    return OutputSections.size() + 1 - NumDummySections;
+  }
 
   void addRelIpltSymbols();
   void addStartEndSymbols();
   void addStartStopSymbols(OutputSectionBase<ELFT> *Sec);
-  void setPhdr(Elf_Phdr *PH, uint32_t Type, uint32_t Flags, uintX_t FileOff,
-               uintX_t VA, uintX_t Size, uintX_t Align);
-  void copyPhdr(Elf_Phdr *PH, OutputSectionBase<ELFT> *From);
 
-  bool HasRelro = false;
   SymbolTable<ELFT> &Symtab;
-  std::vector<Elf_Phdr> Phdrs;
+  std::vector<Phdr> Phdrs;
 
   uintX_t FileSize;
   uintX_t SectionHeaderOff;
 
   llvm::StringMap<llvm::StringRef> InputToOutputSection;
 
   // Flag to force GOT to be in output if we have relocations
   // that relies on its address.
   bool HasGotOffRel = false;
 };
 } // anonymous namespace
 
 template <class ELFT> static bool shouldUseRela() { return ELFT::Is64Bits; }
 
 template <class ELFT> void elf2::writeResult(SymbolTable<ELFT> *Symtab) {
+  typedef typename ELFFile<ELFT>::uintX_t uintX_t;
+
   // Create singleton output sections.
   bool IsRela = shouldUseRela<ELFT>();
   DynamicSection<ELFT> Dynamic(*Symtab);
   EhFrameHeader<ELFT> EhFrameHdr;
   GotSection<ELFT> Got;
   InterpSection<ELFT> Interp;
   PltSection<ELFT> Plt;
   RelocationSection<ELFT> RelaDyn(IsRela ? ".rela.dyn" : ".rel.dyn", IsRela);
   StringTableSection<ELFT> DynStrTab(".dynstr", true);
   StringTableSection<ELFT> ShStrTab(".shstrtab", false);
   SymbolTableSection<ELFT> DynSymTab(*Symtab, DynStrTab);
 
+  OutputSectionBase<ELFT> ElfHeader("", 0, SHF_ALLOC);
+  OutputSectionBase<ELFT> ProgramHeaders("", 0, SHF_ALLOC);
+  ProgramHeaders.updateAlign(sizeof(uintX_t));
+
   // Instantiate optional output sections if they are needed.
   std::unique_ptr<GnuHashTableSection<ELFT>> GnuHashTab;
   std::unique_ptr<GotPltSection<ELFT>> GotPlt;
   std::unique_ptr<HashTableSection<ELFT>> HashTab;
   std::unique_ptr<RelocationSection<ELFT>> RelaPlt;
   std::unique_ptr<StringTableSection<ELFT>> StrTab;
   std::unique_ptr<SymbolTableSection<ELFT>> SymTabSec;
 
template <class ELFT> void elf2::writeResult(SymbolTable<ELFT> *Symtab) {
   Out<ELFT>::ShStrTab = &ShStrTab;
   Out<ELFT>::StrTab = StrTab.get();
   Out<ELFT>::SymTab = SymTabSec.get();
   Out<ELFT>::Bss = nullptr;
   Out<ELFT>::MipsRldMap = nullptr;
   Out<ELFT>::Opd = nullptr;
   Out<ELFT>::OpdBuf = nullptr;
   Out<ELFT>::TlsPhdr = nullptr;
+  Out<ELFT>::ElfHeader = &ElfHeader;
+  Out<ELFT>::ProgramHeaders = &ProgramHeaders;
 
   Writer<ELFT>(*Symtab).run();
 }
 
 // The main function of the writer.
 template <class ELFT> void Writer<ELFT>::run() {
   buildSectionMap();
   if (!Config->DiscardAll)
     copyLocalSymbols();
   addReservedSymbols();
   if (!createSections())
     return;
+  createPhdrs();
   assignAddresses();
   fixAbsoluteSymbols();
   if (!openFile())
     return;
   writeHeader();
   writeSections();
   if (HasError)
     return;
return StringSwitch<int>(SectionName)
            .Case(".branch_lt", 2)
            .Case(".toc", 3)
            .Case(".toc1", 4)
            .Case(".opd", 5)
            .Default(1);
 }
 
 template <class ELFT> static bool isRelroSection(OutputSectionBase<ELFT> *Sec) {
+  if (!Config->ZRelro)
+    return false;
   typename OutputSectionBase<ELFT>::uintX_t Flags = Sec->getFlags();
   if (!(Flags & SHF_ALLOC) || !(Flags & SHF_WRITE))
     return false;
   if (Flags & SHF_TLS)
     return true;
   uint32_t Type = Sec->getType();
   if (Type == SHT_INIT_ARRAY || Type == SHT_FINI_ARRAY ||
       Type == SHT_PREINIT_ARRAY)
template <class ELFT> void Writer<ELFT>::addReservedSymbols() {
   // break programs that uses "end" as a regular symbol.
   if (SymbolBody *B = Symtab.find("end"))
     if (B->isUndefined())
       Symtab.addAbsolute("end", ElfSym<ELFT>::End);
 }
 
 // Create output section objects and add them to OutputSections.
 template <class ELFT> bool Writer<ELFT>::createSections() {
+  OutputSections.push_back(Out<ELFT>::ElfHeader);
+  OutputSections.push_back(Out<ELFT>::ProgramHeaders);
+
   // Add .interp first because some loaders want to see that section
   // on the first page of the executable file when loaded into memory.
   if (needsInterpSection())
     OutputSections.push_back(Out<ELFT>::Interp);
 
   // Create output sections for input object file sections.
   std::vector<OutputSectionBase<ELFT> *> RegularSections;
   OutputSectionFactory<ELFT> Factory;
template <class ELFT> bool Writer<ELFT>::createSections() {
 
   // So far we have added sections from input object files.
   // This function adds linker-created Out<ELFT>::* sections.
   addPredefinedSections();
 
   std::stable_sort(OutputSections.begin(), OutputSections.end(),
                    compareSections<ELFT>);
 
-  for (unsigned I = 0, N = OutputSections.size(); I < N; ++I) {
-    OutputSections[I]->SectionIndex = I + 1;
-    HasRelro |= (Config->ZRelro && isRelroSection(OutputSections[I]));
-  }
+  for (unsigned I = NumDummySections, N = OutputSections.size(); I < N; ++I)
+    OutputSections[I]->SectionIndex = I + 1 - NumDummySections;
 
-  for (OutputSectionBase<ELFT> *Sec : OutputSections)
+  for (OutputSectionBase<ELFT> *Sec : getSections())
     Sec->setSHName(Out<ELFT>::ShStrTab->addString(Sec->getName()));
 
   // Finalizers fix each section's size.
   // .dynsym is finalized early since that may fill up .gnu.hash.
   if (isOutputDynamic())
     Out<ELFT>::DynSymTab->finalize();
 
   // Fill other section headers. The dynamic table is finalized
static uint32_t getAmdgpuPhdr(OutputSectionBase<ELFT> *Sec) {
   uint32_t Flags = Sec->getFlags();
   if (Flags & SHF_AMDGPU_HSA_CODE)
     return PT_AMDGPU_HSA_LOAD_CODE_AGENT;
   if ((Flags & SHF_AMDGPU_HSA_GLOBAL) && !(Flags & SHF_AMDGPU_HSA_AGENT))
     return PT_AMDGPU_HSA_LOAD_GLOBAL_PROGRAM;
   return PT_LOAD;
 }
 
-template <class ELFT>
-void Writer<ELFT>::updateRelro(Elf_Phdr *Cur, Elf_Phdr *GnuRelroPhdr,
-                               uintX_t VA) {
-  if (!GnuRelroPhdr->p_type)
-    setPhdr(GnuRelroPhdr, PT_GNU_RELRO, PF_R, Cur->p_offset, Cur->p_vaddr,
-            VA - Cur->p_vaddr, 1 /*p_align*/);
-  GnuRelroPhdr->p_filesz = VA - Cur->p_vaddr;
-  GnuRelroPhdr->p_memsz = VA - Cur->p_vaddr;
-}
-
-// Visits all sections to create PHDRs and to assign incremental,
-// non-overlapping addresses to output sections.
-template <class ELFT> void Writer<ELFT>::assignAddresses() {
-  uintX_t VA = Target->getVAStart() + sizeof(Elf_Ehdr);
-  uintX_t FileOff = sizeof(Elf_Ehdr);
+// Decide which program headers to create and which sections to include in each
+// one.
+template <class ELFT> void Writer<ELFT>::createPhdrs() {
+  auto AddHdr = [this](unsigned Type, unsigned Flags) {
+    return &*Phdrs.emplace(Phdrs.end(), Type, Flags);
+  };
 
-  // Calculate and reserve the space for the program header first so that
-  // the first section can start right after the program header.
-  Phdrs.resize(getPhdrsNum());
-  size_t PhdrSize = sizeof(Elf_Phdr) * Phdrs.size();
+  auto AddSec = [](Phdr &Hdr, OutputSectionBase<ELFT> *Sec) {
+    Hdr.Last = Sec;
+    if (!Hdr.First)
+      Hdr.First = Sec;
+    Hdr.H.p_align = std::max<uintX_t>(Hdr.H.p_align, Sec->getAlign());
+  };
 
   // The first phdr entry is PT_PHDR which describes the program header itself.
-  setPhdr(&Phdrs[0], PT_PHDR, PF_R, FileOff, VA, PhdrSize,
-          /*Align=*/sizeof(uintX_t));
-  FileOff += PhdrSize;
-  VA += PhdrSize;
+  Phdr &Hdr = *AddHdr(PT_PHDR, PF_R);
+  AddSec(Hdr, Out<ELFT>::ProgramHeaders);
 
   // PT_INTERP must be the second entry if exists.
-  int PhdrIdx = 0;
-  Elf_Phdr *Interp = nullptr;
-  if (needsInterpSection())
-    Interp = &Phdrs[++PhdrIdx];
+  if (needsInterpSection()) {
+    Phdr &Hdr = *AddHdr(PT_INTERP, toPhdrFlags(Out<ELFT>::Interp->getFlags()));
+    AddSec(Hdr, Out<ELFT>::Interp);
+  }
 
   // Add the first PT_LOAD segment for regular output sections.
-  setPhdr(&Phdrs[++PhdrIdx], PT_LOAD, PF_R, 0, Target->getVAStart(), FileOff,
-          Target->PageSize);
+  uintX_t Flags = PF_R;
+  Phdr *Load = AddHdr(PT_LOAD, Flags);
+  AddSec(*Load, Out<ELFT>::ElfHeader);
 
-  Elf_Phdr GnuRelroPhdr = {};
-  Elf_Phdr TlsPhdr{};
-  bool RelroAligned = false;
-  uintX_t ThreadBssOffset = 0;
-  // Create phdrs as we assign VAs and file offsets to all output sections.
+  Phdr TlsHdr(PT_TLS, PF_R);
+  Phdr RelRo(PT_GNU_RELRO, PF_R);
   for (OutputSectionBase<ELFT> *Sec : OutputSections) {
-    Elf_Phdr *PH = &Phdrs[PhdrIdx];
-    if (needsPhdr<ELFT>(Sec)) {
-      uintX_t Flags = toPhdrFlags(Sec->getFlags());
-      bool InRelRo = Config->ZRelro && (Flags & PF_W) && isRelroSection(Sec);
-      bool FirstNonRelRo = GnuRelroPhdr.p_type && !InRelRo && !RelroAligned;
-      if (FirstNonRelRo || PH->p_flags != Flags) {
-        VA = alignTo(VA, Target->PageSize);
-        FileOff = alignTo(FileOff, Target->PageSize);
-        if (FirstNonRelRo)
-          RelroAligned = true;
-      }
-
-      if (PH->p_flags != Flags) {
-        // Flags changed. Create a new PT_LOAD.
-        PH = &Phdrs[++PhdrIdx];
-        uint32_t PTType = (Config->EMachine != EM_AMDGPU) ? (uint32_t)PT_LOAD
-                                                          : getAmdgpuPhdr(Sec);
-        setPhdr(PH, PTType, Flags, FileOff, VA, 0, Target->PageSize);
-      }
+    if (!needsPhdr<ELFT>(Sec))
+      break;
 
-      if (Sec->getFlags() & SHF_TLS) {
-        if (!TlsPhdr.p_vaddr)
-          setPhdr(&TlsPhdr, PT_TLS, PF_R, FileOff, VA, 0, Sec->getAlign());
-        if (Sec->getType() != SHT_NOBITS)
-          VA = alignTo(VA, Sec->getAlign());
-        uintX_t TVA = alignTo(VA + ThreadBssOffset, Sec->getAlign());
-        Sec->setVA(TVA);
-        TlsPhdr.p_memsz += Sec->getSize();
-        if (Sec->getType() == SHT_NOBITS) {
-          ThreadBssOffset = TVA - VA + Sec->getSize();
-        } else {
-          TlsPhdr.p_filesz += Sec->getSize();
-          VA += Sec->getSize();
-        }
-        TlsPhdr.p_align = std::max<uintX_t>(TlsPhdr.p_align, Sec->getAlign());
-      } else {
-        VA = alignTo(VA, Sec->getAlign());
-        Sec->setVA(VA);
-        VA += Sec->getSize();
-      }
-      if (InRelRo)
-        updateRelro(PH, &GnuRelroPhdr, VA);
+    // If flags changed then we want new load segment.
+    uintX_t NewFlags = toPhdrFlags(Sec->getFlags());
+    if (Flags != NewFlags) {
+      uint32_t LoadType = (Config->EMachine == EM_AMDGPU) ? getAmdgpuPhdr(Sec)
+                                                          : (uint32_t)PT_LOAD;
+      Load = AddHdr(LoadType, NewFlags);
+      Flags = NewFlags;
+    }
+    // If we meet TLS section then we create TLS header
+    // and put all TLS sections inside for futher use when
+    // assign addresses.
+      AddSec(TlsHdr, Sec);
+      if (Sec->getType() == SHT_NOBITS)
+        continue;
     }
 
-    FileOff = alignTo(FileOff, Sec->getAlign());
-    Sec->setFileOffset(FileOff);
-    if (Sec->getType() != SHT_NOBITS)
-      FileOff += Sec->getSize();
-    if (needsPhdr<ELFT>(Sec)) {
-      PH->p_filesz = FileOff - PH->p_offset;
-      PH->p_memsz = VA - PH->p_vaddr;
-    }
-  }
+    AddSec(*Load, Sec);
 
-  if (TlsPhdr.p_vaddr) {
-    // 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.
-    TlsPhdr.p_memsz = alignTo(TlsPhdr.p_memsz, TlsPhdr.p_align);
-    Phdrs[++PhdrIdx] = TlsPhdr;
-    Out<ELFT>::TlsPhdr = &Phdrs[PhdrIdx];
+    if (isRelroSection(Sec))
+      AddSec(RelRo, Sec);
   }
 
+  // Add the TLS segment unless it's empty.
+  if (TlsHdr.First)
+    Phdrs.push_back(std::move(TlsHdr));
+
   // Add an entry for .dynamic.
   if (isOutputDynamic()) {
-    Elf_Phdr *PH = &Phdrs[++PhdrIdx];
-    PH->p_type = PT_DYNAMIC;
-    copyPhdr(PH, Out<ELFT>::Dynamic);
+    Phdr &Hdr =
+        *AddHdr(PT_DYNAMIC, toPhdrFlags(Out<ELFT>::Dynamic->getFlags()));
+    AddSec(Hdr, Out<ELFT>::Dynamic);
   }
 
-  if (HasRelro) {
-    Elf_Phdr *PH = &Phdrs[++PhdrIdx];
-    *PH = GnuRelroPhdr;
-  }
+  // PT_GNU_RELRO includes all sections that should be marked as
+  // read-only by dynamic linker after proccessing relocations.
+  if (RelRo.First)
+    Phdrs.push_back(std::move(RelRo));
 
+  // PT_GNU_EH_FRAME is a special section pointing on .eh_frame_hdr.
   if (Out<ELFT>::EhFrameHdr->Live) {
-    Elf_Phdr *PH = &Phdrs[++PhdrIdx];
-    PH->p_type = PT_GNU_EH_FRAME;
-    copyPhdr(PH, Out<ELFT>::EhFrameHdr);
+    Phdr &Hdr = *AddHdr(PT_GNU_EH_FRAME,
+                        toPhdrFlags(Out<ELFT>::EhFrameHdr->getFlags()));
+    AddSec(Hdr, Out<ELFT>::EhFrameHdr);
   }
 
   // PT_GNU_STACK is a special section to tell the loader to make the
   // pages for the stack non-executable.
-  if (!Config->ZExecStack) {
-    Elf_Phdr *PH = &Phdrs[++PhdrIdx];
-    PH->p_type = PT_GNU_STACK;
-    PH->p_flags = PF_R | PF_W;
+  if (!Config->ZExecStack)
+    AddHdr(PT_GNU_STACK, PF_R | PF_W);
+}
+
+// Visits all headers in PhdrTable and assigns the adresses to
+// the output sections. Also creates common and special headers.
+template <class ELFT> void Writer<ELFT>::assignAddresses() {
+  Out<ELFT>::ElfHeader->setSize(sizeof(Elf_Ehdr));
+  size_t PhdrSize = sizeof(Elf_Phdr) * Phdrs.size();
+  Out<ELFT>::ProgramHeaders->setSize(PhdrSize);
+
+  // The first section of each PT_LOAD and the first section after PT_GNU_RELRO
+  // have to be page aligned so that the dynamic linker can set the permissions.
+  SmallPtrSet<OutputSectionBase<ELFT> *, 4> PageAlign;
+  for (const Phdr &P : Phdrs) {
+    if (P.H.p_type == PT_GNU_RELRO) {
+      // Find the first section after PT_GNU_RELRO. If it is in a PT_LOAD
+      // and is not tls, we have to align it to a page. We don't have to
+      // align tls since TLS NOBITS takes no space.
+      auto I = std::find(OutputSections.begin(), OutputSections.end(), P.Last);
+      ++I;
+      if (I != OutputSections.end() && needsPhdr(*I) &&
+          !((*I)->getFlags() & SHF_TLS))
+        PageAlign.insert(*I);
     }
 
-  // Fix up PT_INTERP as we now know the address of .interp section.
-  if (Interp) {
-    Interp->p_type = PT_INTERP;
-    copyPhdr(Interp, Out<ELFT>::Interp);
+    // FIXME: why create empty PT_LOAD?
+    if (P.H.p_type == PT_LOAD && P.First)
+      PageAlign.insert(P.First);
+  }
+
+  uintX_t ThreadBssOffset = 0;
+  uintX_t VA = Target->getVAStart();
+  uintX_t FileOff = 0;
+
+  for (OutputSectionBase<ELFT> *Sec : OutputSections) {
+    uintX_t Align = Sec->getAlign();
+    if (PageAlign.count(Sec))
+      Align = std::max<uintX_t>(Align, Target->PageSize);
+
+    FileOff = alignTo(FileOff, Align);
+    Sec->setFileOffset(FileOff);
+    if (Sec->getType() != SHT_NOBITS)
+      FileOff += Sec->getSize();
+
+    if (needsPhdr<ELFT>(Sec)) {
+      // Don't allocate VA space for TLS NOBITS sections. The PT_TLS PHDR is
+      // responsible for allocating space for them, not the PT_LOAD that
+      // contains the TLS initialization image.
+      bool IsTls = Sec->getFlags() & SHF_TLS;
+      if (IsTls && Sec->getType() == SHT_NOBITS) {
+        uintX_t TVA = VA + ThreadBssOffset;
+        TVA = alignTo(TVA, Align);
+        Sec->setVA(TVA);
+        ThreadBssOffset = TVA - VA + Sec->getSize();
+      } else {
+        VA = alignTo(VA, Align);
+        Sec->setVA(VA);
+        VA += Sec->getSize();
+      }
+    }
   }
 
   // Add space for section headers.
   SectionHeaderOff = alignTo(FileOff, sizeof(uintX_t));
   FileSize = SectionHeaderOff + getNumSections() * sizeof(Elf_Shdr);
 
   // Update "_end" and "end" symbols so that they
   // point to the end of the data segment.
   ElfSym<ELFT>::End.st_value = VA;
-}
 
-// Returns the number of PHDR entries.
-template <class ELFT> int Writer<ELFT>::getPhdrsNum() const {
-  bool Tls = false;
-  int I = 2; // 2 for PT_PHDR and first PT_LOAD
-  if (needsInterpSection())
-    ++I;
-  if (isOutputDynamic())
-    ++I;
-  if (!Config->ZExecStack)
-    ++I;
-  uintX_t Last = PF_R;
-  for (OutputSectionBase<ELFT> *Sec : OutputSections) {
-    if (!needsPhdr<ELFT>(Sec))
-      continue;
-    if (Sec->getFlags() & SHF_TLS)
-      Tls = true;
-    uintX_t Flags = toPhdrFlags(Sec->getFlags());
-    if (Last != Flags) {
-      Last = Flags;
-      ++I;
+  for (Phdr &PHdr : Phdrs) {
+    Elf_Phdr &H = PHdr.H;
+    if (PHdr.First) {
+      OutputSectionBase<ELFT> *Last = PHdr.Last;
+      H.p_filesz = Last->getFileOff() - PHdr.First->getFileOff();
+      if (Last->getType() != SHT_NOBITS)
+        H.p_filesz += Last->getSize();
+      H.p_memsz = Last->getVA() + Last->getSize() - PHdr.First->getVA();
+      H.p_offset = PHdr.First->getFileOff();
+      H.p_vaddr = PHdr.First->getVA();
+    }
+    if (PHdr.H.p_type == PT_LOAD)
+      H.p_align = Target->PageSize;
+    else if (PHdr.H.p_type == PT_GNU_RELRO)
+      H.p_align = 1;
+    H.p_paddr = H.p_vaddr;
+
+    // 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 (PHdr.H.p_type == PT_TLS) {
+      Out<ELFT>::TlsPhdr = &H;
+      H.p_memsz = alignTo(H.p_memsz, H.p_align);
     }
   }
-  if (Tls)
-    ++I;
-  if (HasRelro)
-    ++I;
-  if (Out<ELFT>::EhFrameHdr->Live)
-    ++I;
-  return I;
 }
 
 static uint32_t getELFFlags() {
   if (Config->EMachine != EM_MIPS)
     return 0;
   // FIXME: In fact ELF flags depends on ELF flags of input object files
   // and selected emulation. For now just use hard coded values.
   uint32_t V = EF_MIPS_ABI_O32 | EF_MIPS_CPIC | EF_MIPS_ARCH_32R2;
template <class ELFT> void Writer<ELFT>::writeHeader() {
   EHdr->e_ehsize = sizeof(Elf_Ehdr);
   EHdr->e_phentsize = sizeof(Elf_Phdr);
   EHdr->e_phnum = Phdrs.size();
   EHdr->e_shentsize = sizeof(Elf_Shdr);
   EHdr->e_shnum = getNumSections();
   EHdr->e_shstrndx = Out<ELFT>::ShStrTab->SectionIndex;
 
   // Write the program header table.
-  memcpy(Buf + EHdr->e_phoff, &Phdrs[0], Phdrs.size() * sizeof(Phdrs[0]));
+  auto *HBuf = reinterpret_cast<Elf_Phdr *>(Buf + EHdr->e_phoff);
+  for (Phdr &P : Phdrs) {
+    memcpy(HBuf, &P.H, sizeof(Phdrs[0].H));
+    ++HBuf;
+  }
 
   // Write the section header table. Note that the first table entry is null.
   auto SHdrs = reinterpret_cast<Elf_Shdr *>(Buf + EHdr->e_shoff);
-  for (OutputSectionBase<ELFT> *Sec : OutputSections)
+  for (OutputSectionBase<ELFT> *Sec : getSections())
     Sec->writeHeaderTo(++SHdrs);
 }
 
 template <class ELFT> bool Writer<ELFT>::openFile() {
   ErrorOr<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
       FileOutputBuffer::create(Config->OutputFile, FileSize,
                                FileOutputBuffer::F_executable);
   if (error(BufferOrErr, "failed to open " + Config->OutputFile))
if (OutputSectionBase<ELFT> *Sec = Out<ELFT>::Opd) {
     Sec->writeTo(Buf + Sec->getFileOff());
   }
 
   for (OutputSectionBase<ELFT> *Sec : OutputSections)
     if (Sec != Out<ELFT>::Opd)
       Sec->writeTo(Buf + Sec->getFileOff());
 }
 
-template <class ELFT>
-void Writer<ELFT>::setPhdr(Elf_Phdr *PH, uint32_t Type, uint32_t Flags,
-                           uintX_t FileOff, uintX_t VA, uintX_t Size,
-                           uintX_t Align) {
-  PH->p_type = Type;
-  PH->p_flags = Flags;
-  PH->p_offset = FileOff;
-  PH->p_vaddr = VA;
-  PH->p_paddr = VA;
-  PH->p_filesz = Size;
-  PH->p_memsz = Size;
-  PH->p_align = Align;
-}
-
-template <class ELFT>
-void Writer<ELFT>::copyPhdr(Elf_Phdr *PH, OutputSectionBase<ELFT> *From) {
-  PH->p_flags = toPhdrFlags(From->getFlags());
-  PH->p_offset = From->getFileOff();
-  PH->p_vaddr = From->getVA();
-  PH->p_paddr = From->getVA();
-  PH->p_filesz = From->getSize();
-  PH->p_memsz = From->getSize();
-  PH->p_align = From->getAlign();
-}
-
 template <class ELFT> void Writer<ELFT>::buildSectionMap() {
   for (const std::pair<StringRef, std::vector<StringRef>> &OutSec :
        Config->OutputSections)
     for (StringRef Name : OutSec.second)
       InputToOutputSection[Name] = OutSec.first;
 }
 
 template void elf2::writeResult<ELF32LE>(SymbolTable<ELF32LE> *Symtab);
 template void elf2::writeResult<ELF32BE>(SymbolTable<ELF32BE> *Symtab);
 template void elf2::writeResult<ELF64LE>(SymbolTable<ELF64LE> *Symtab);
 template void elf2::writeResult<ELF64BE>(SymbolTable<ELF64BE> *Symtab);