[PATCH] D12718: [elf2] Assign output sections to PHDRs.

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 9 07:28:52 PDT 2015


+ static const uint64_t BaseAddress = 0x400000;

Any reason to change this in this patch? It can be done in another
patch if there is any advantage to it, no?

+  FileOff = RoundUpToAlignment(FileOff, ELFT::Is64Bits ? 8 : 4);

That is not needed. The file header size is already a multiple of 8 or
4. You can assert it if you want.

+  for (OutputSectionBase<ELFT::Is64Bits> *Sec : OutputSections)
+    if (outputSectionHasPHDR<ELFT>(Sec)) {
+      FileOff += sizeof(Elf_Phdr_Impl<ELFT>);

Use Elf_Phdr instead of Elf_Phdr_Impl

+      ++NumPhdrs;
+    }

Include {} around the for. Alternatively, write it as


  for (OutputSectionBase<ELFT::Is64Bits> *Sec : OutputSections)
    if (outputSectionHasPHDR<ELFT>(Sec))
      ++NumPhdrs;

 FileOff += NumPhdrs * sizeof(Elf_Phdr);

Even better, we can merge the loop with the following one since all
the program headers should fit in the first page (which is plenty of
space, specially once we create one per flag combination instead of
one per output section):

https://github.com/freebsd/freebsd/blob/a8fae65d69bf46d8a4aa6351092235ee691df503/sys/kern/imgact_elf.c#L674


+  FileSize = RoundUpToAlignment(FileOff, 8);

This can just be

FileSize = FileOff;

no? Why do we have to pad the end of the file?

An attached patch with most of these comments is attached.


On 8 September 2015 at 22:06, Michael Spencer <bigcheesegs at gmail.com> wrote:
> Bigcheese created this revision.
> Bigcheese added a reviewer: rafael.
> Bigcheese added a subscriber: llvm-commits.
>
> This is a minimal implementation to produce legal output. Future patches will combine multiple compatible PT_LOADs.
>
> It uses the term PHDR instead of segment as not all PHDRs represent segments.
>
> http://reviews.llvm.org/D12718
>
> Files:
>   ELF/Writer.cpp
>   test/elf2/basic.s
>   test/elf2/basic32.s
>   test/elf2/basic32be.s
>   test/elf2/basic64be.s
>   test/elf2/bss.s
>   test/elf2/common.s
>   test/elf2/relocation.s
>   test/elf2/shared.s
>   test/elf2/string-table.s
>   test/elf2/symbols.s
>   test/elf2/visibility.s
>
-------------- next part --------------
diff --git a/ELF/Writer.cpp b/ELF/Writer.cpp
index 8f0c7f2..d916722 100644
--- a/ELF/Writer.cpp
+++ b/ELF/Writer.cpp
@@ -203,8 +203,9 @@ private:
   unsigned getNumSections() const { return OutputSections.size() + 1; }
 
   uintX_t FileSize;
-  uintX_t SizeOfHeaders;
+  uintX_t ProgramHeaderOff;
   uintX_t SectionHeaderOff;
+  unsigned NumPhdrs;
 
   StringTableSection<ELFT::Is64Bits> StringTable;
 
@@ -528,14 +529,31 @@ template <class ELFT> void Writer<ELFT>::createSections() {
     OutputSections[I]->setSectionIndex(I + 1);
 }
 
+template <class ELFT>
+static bool outputSectionHasPHDR(OutputSectionBase<ELFT::Is64Bits> *Sec) {
+  return (Sec->getSize() != 0) && (Sec->getFlags() & SHF_ALLOC);
+}
+
 // Visits all sections to assign incremental, non-overlapping RVAs and
 // file offsets.
 template <class ELFT> void Writer<ELFT>::assignAddresses() {
-  SizeOfHeaders = RoundUpToAlignment(sizeof(Elf_Ehdr), PageSize);
   uintX_t VA = 0x1000; // The first page is kept unmapped.
-  uintX_t FileOff = SizeOfHeaders;
+  uintX_t FileOff = sizeof(Elf_Ehdr);
+
+  // Reserve space for PHDRs.
+  ProgramHeaderOff = FileOff;
+  FileOff = RoundUpToAlignment(FileOff, PageSize);
 
+  NumPhdrs = 0;
   for (OutputSectionBase<ELFT::Is64Bits> *Sec : OutputSections) {
+    // Since each output section gets its own PHDR, align each output section to
+    // a page.
+    if (outputSectionHasPHDR<ELFT>(Sec)) {
+      ++NumPhdrs;
+      VA = RoundUpToAlignment(VA, PageSize);
+      FileOff = RoundUpToAlignment(FileOff, PageSize);
+    }
+
     StringTable.add(Sec->getName());
     Sec->finalize();
 
@@ -550,15 +568,19 @@ template <class ELFT> void Writer<ELFT>::assignAddresses() {
       FileOff += RoundUpToAlignment(Size, Align);
   }
 
+  // Add a PHDR for the dynamic table.
+  if (!SymTable.getSymTable().getSharedFiles().empty())
+    ++NumPhdrs;
+
   FileOff += OffsetToAlignment(FileOff, ELFT::Is64Bits ? 8 : 4);
 
   // Add space for section headers.
   SectionHeaderOff = FileOff;
   FileOff += getNumSections() * sizeof(Elf_Shdr);
-  FileSize = SizeOfHeaders + RoundUpToAlignment(FileOff - SizeOfHeaders, 8);
+  FileSize = FileOff;
 }
 
-static uint32_t convertSectionFlagsToSegmentFlags(uint64_t Flags) {
+static uint32_t convertSectionFlagsToPHDRFlags(uint64_t Flags) {
   uint32_t Ret = PF_R;
   if (Flags & SHF_WRITE)
     Ret |= PF_W;
@@ -585,42 +607,41 @@ template <class ELFT> void Writer<ELFT>::writeHeader() {
 
   // FIXME: Generalize the segment construction similar to how we create
   // output sections.
-  unsigned NumSegments = 1;
   const SymbolTable &Symtab = SymTable.getSymTable();
-  const std::vector<std::unique_ptr<SharedFileBase>> &SharedFiles =
-      Symtab.getSharedFiles();
-  bool HasDynamicSegment = !SharedFiles.empty();
-  if (HasDynamicSegment)
-    NumSegments++;
+  bool HasDynamicSegment = !Symtab.getSharedFiles().empty();
 
   EHdr->e_type = ET_EXEC;
   auto &FirstObj = cast<ObjectFile<ELFT>>(*Symtab.getFirstELF());
   EHdr->e_machine = FirstObj.getEMachine();
   EHdr->e_version = EV_CURRENT;
   EHdr->e_entry = getSymVA(cast<DefinedRegular<ELFT>>(Symtab.getEntrySym()));
-  EHdr->e_phoff = sizeof(Elf_Ehdr);
+  EHdr->e_phoff = ProgramHeaderOff;
   EHdr->e_shoff = SectionHeaderOff;
   EHdr->e_ehsize = sizeof(Elf_Ehdr);
   EHdr->e_phentsize = sizeof(Elf_Phdr);
-  EHdr->e_phnum = NumSegments;
+  EHdr->e_phnum = NumPhdrs;
   EHdr->e_shentsize = sizeof(Elf_Shdr);
   EHdr->e_shnum = getNumSections();
   EHdr->e_shstrndx = StringTable.getSectionIndex();
 
   auto PHdrs = reinterpret_cast<Elf_Phdr *>(Buf + EHdr->e_phoff);
-  PHdrs->p_type = PT_LOAD;
-  PHdrs->p_flags = PF_R | PF_X;
-  PHdrs->p_offset = 0x0000;
-  PHdrs->p_vaddr = 0x400000;
-  PHdrs->p_paddr = PHdrs->p_vaddr;
-  PHdrs->p_filesz = FileSize;
-  PHdrs->p_memsz = FileSize;
-  PHdrs->p_align = 0x4000;
+  for (OutputSectionBase<ELFT::Is64Bits> *Sec : OutputSections) {
+    if (!outputSectionHasPHDR<ELFT>(Sec))
+      continue;
+    PHdrs->p_type = PT_LOAD;
+    PHdrs->p_flags = convertSectionFlagsToPHDRFlags(Sec->getFlags());
+    PHdrs->p_offset = Sec->getFileOff();
+    PHdrs->p_vaddr = Sec->getVA();
+    PHdrs->p_paddr = PHdrs->p_vaddr;
+    PHdrs->p_filesz = Sec->getType() == SHT_NOBITS ? 0 : Sec->getSize();
+    PHdrs->p_memsz = Sec->getSize();
+    PHdrs->p_align = PageSize;
+    ++PHdrs;
+  }
 
   if (HasDynamicSegment) {
-    PHdrs++;
     PHdrs->p_type = PT_DYNAMIC;
-    PHdrs->p_flags = convertSectionFlagsToSegmentFlags(DynamicSec.getFlags());
+    PHdrs->p_flags = convertSectionFlagsToPHDRFlags(DynamicSec.getFlags());
     PHdrs->p_offset = DynamicSec.getFileOff();
     PHdrs->p_vaddr = DynamicSec.getVA();
     PHdrs->p_paddr = PHdrs->p_vaddr;
diff --git a/test/elf2/basic.s b/test/elf2/basic.s
index 384bc46..004cf7d 100644
--- a/test/elf2/basic.s
+++ b/test/elf2/basic.s
@@ -151,16 +151,16 @@ _start:
 # CHECK-NEXT: ProgramHeaders [
 # CHECK-NEXT:   ProgramHeader {
 # CHECK-NEXT:     Type: PT_LOAD (0x1)
-# CHECK-NEXT:     Offset: 0x0
-# CHECK-NEXT:     VirtualAddress: 0x400000
-# CHECK-NEXT:     PhysicalAddress: 0x400000
-# CHECK-NEXT:     FileSize: 4592
-# CHECK-NEXT:     MemSize: 4592
+# CHECK-NEXT:     Offset: 0x1000
+# CHECK-NEXT:     VirtualAddress: 0x1000
+# CHECK-NEXT:     PhysicalAddress: 0x1000
+# CHECK-NEXT:     FileSize: 16
+# CHECK-NEXT:     MemSize: 16
 # CHECK-NEXT:     Flags [ (0x5)
 # CHECK-NEXT:       PF_R (0x4)
 # CHECK-NEXT:       PF_X (0x1)
 # CHECK-NEXT:     ]
-# CHECK-NEXT:     Alignment: 16384
+# CHECK-NEXT:     Alignment: 4096
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 
diff --git a/test/elf2/basic32.s b/test/elf2/basic32.s
index 7194871..7c477ef 100644
--- a/test/elf2/basic32.s
+++ b/test/elf2/basic32.s
@@ -130,15 +130,15 @@ _start:
 # CHECK-NEXT: ProgramHeaders [
 # CHECK-NEXT:   ProgramHeader {
 # CHECK-NEXT:     Type: PT_LOAD (0x1)
-# CHECK-NEXT:     Offset: 0x0
-# CHECK-NEXT:     VirtualAddress: 0x400000
-# CHECK-NEXT:     PhysicalAddress: 0x400000
-# CHECK-NEXT:     FileSize: 4424
-# CHECK-NEXT:     MemSize: 4424
+# CHECK-NEXT:     Offset: 0x1000
+# CHECK-NEXT:     VirtualAddress: 0x1000
+# CHECK-NEXT:     PhysicalAddress: 0x1000
+# CHECK-NEXT:     FileSize: 12
+# CHECK-NEXT:     MemSize: 12
 # CHECK-NEXT:     Flags [ (0x5)
 # CHECK-NEXT:       PF_R (0x4)
 # CHECK-NEXT:       PF_X (0x1)
 # CHECK-NEXT:     ]
-# CHECK-NEXT:     Alignment: 16384
+# CHECK-NEXT:     Alignment: 4096
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
diff --git a/test/elf2/basic32be.s b/test/elf2/basic32be.s
index 149ab18..be6d55a 100644
--- a/test/elf2/basic32be.s
+++ b/test/elf2/basic32be.s
@@ -130,15 +130,15 @@ _start:
 # CHECK-NEXT: ProgramHeaders [
 # CHECK-NEXT:   ProgramHeader {
 # CHECK-NEXT:     Type: PT_LOAD (0x1)
-# CHECK-NEXT:     Offset: 0x0
-# CHECK-NEXT:     VirtualAddress: 0x400000
-# CHECK-NEXT:     PhysicalAddress: 0x400000
-# CHECK-NEXT:     FileSize: 4424
-# CHECK-NEXT:     MemSize: 4424
+# CHECK-NEXT:     Offset: 0x1000
+# CHECK-NEXT:     VirtualAddress: 0x1000
+# CHECK-NEXT:     PhysicalAddress: 0x1000
+# CHECK-NEXT:     FileSize: 12
+# CHECK-NEXT:     MemSize: 12
 # CHECK-NEXT:     Flags [ (0x5)
 # CHECK-NEXT:       PF_R (0x4)
 # CHECK-NEXT:       PF_X (0x1)
 # CHECK-NEXT:     ]
-# CHECK-NEXT:     Alignment: 16384
+# CHECK-NEXT:     Alignment: 4096
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
diff --git a/test/elf2/basic64be.s b/test/elf2/basic64be.s
index 5fcf3f2..1fe6181 100644
--- a/test/elf2/basic64be.s
+++ b/test/elf2/basic64be.s
@@ -28,14 +28,14 @@ _start:
 # CHECK-NEXT:   Type: Executable (0x2)
 # CHECK-NEXT:   Machine: EM_PPC64 (0x15)
 # CHECK-NEXT:   Version: 1
-# CHECK-NEXT:   Entry: 0x100C
+# CHECK-NEXT:   Entry: 0x2000
 # CHECK-NEXT:   ProgramHeaderOffset: 0x40
-# CHECK-NEXT:   SectionHeaderOffset: 0x1088
+# CHECK-NEXT:   SectionHeaderOffset: 0x2078
 # CHECK-NEXT:   Flags [ (0x0)
 # CHECK-NEXT:   ]
 # CHECK-NEXT:   HeaderSize: 64
 # CHECK-NEXT:   ProgramHeaderEntrySize: 56
-# CHECK-NEXT:   ProgramHeaderCount: 1
+# CHECK-NEXT:   ProgramHeaderCount: 2
 # CHECK-NEXT:   SectionHeaderEntrySize: 64
 # CHECK-NEXT:   SectionHeaderCount: 7
 # CHECK-NEXT:    StringTableSectionIndex: 6
@@ -111,8 +111,8 @@ _start:
 # CHECK-NEXT:       SHF_ALLOC (0x2)
 # CHECK-NEXT:       SHF_WRITE (0x1)
 # CHECK-NEXT:     ]
-# CHECK-NEXT:     Address: 0x100C
-# CHECK-NEXT:     Offset: 0x100C
+# CHECK-NEXT:     Address: 0x2000
+# CHECK-NEXT:     Offset: 0x2000
 # CHECK-NEXT:     Size: 24
 # CHECK-NEXT:     Link: 0
 # CHECK-NEXT:     Info: 0
@@ -126,7 +126,7 @@ _start:
 # CHECK-NEXT:     Flags [
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Address: 0x0
-# CHECK-NEXT:     Offset: 0x1024
+# CHECK-NEXT:     Offset: 0x2018
 # CHECK-NEXT:     Size: 48
 # CHECK-NEXT:     Link: 6
 # CHECK-NEXT:     Info: 1
@@ -140,7 +140,7 @@ _start:
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Address: 0x0
-# CHECK-NEXT:     Offset: 0x1054
+# CHECK-NEXT:     Offset: 0x2048
 # CHECK-NEXT:     Size: 46
 # CHECK-NEXT:     Link: 0
 # CHECK-NEXT:     Info: 0
@@ -149,17 +149,30 @@ _start:
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 # CHECK-NEXT: ProgramHeaders [
-# CHECK-NEXT:   ProgramHeader {
-# CHECK-NEXT:     Type: PT_LOAD (0x1)
-# CHECK-NEXT:     Offset: 0x0
-# CHECK-NEXT:     VirtualAddress: 0x400000
-# CHECK-NEXT:     PhysicalAddress: 0x400000
-# CHECK-NEXT:     FileSize: 4680
-# CHECK-NEXT:     MemSize: 4680
-# CHECK-NEXT:     Flags [ (0x5)
-# CHECK-NEXT:       PF_R (0x4)
-# CHECK-NEXT:       PF_X (0x1)
-# CHECK-NEXT:     ]
-# CHECK-NEXT:     Alignment: 16384
-# CHECK-NEXT:   }
-# CHECK-NEXT: ]
+# CHECK-NEXT:  ProgramHeader {
+# CHECK-NEXT:    Type: PT_LOAD (0x1)
+# CHECK-NEXT:    Offset: 0x1000
+# CHECK-NEXT:    VirtualAddress: 0x1000
+# CHECK-NEXT:    PhysicalAddress: 0x1000
+# CHECK-NEXT:    FileSize: 12
+# CHECK-NEXT:    MemSize: 12
+# CHECK-NEXT:    Flags [ (0x5)
+# CHECK-NEXT:      PF_R (0x4)
+# CHECK-NEXT:      PF_X (0x1)
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Alignment: 4096
+# CHECK-NEXT:  }
+# CHECK-NEXT:  ProgramHeader {
+# CHECK-NEXT:    Type: PT_LOAD (0x1)
+# CHECK-NEXT:    Offset: 0x2000
+# CHECK-NEXT:    VirtualAddress: 0x2000
+# CHECK-NEXT:    PhysicalAddress: 0x2000
+# CHECK-NEXT:    FileSize: 24
+# CHECK-NEXT:    MemSize: 24
+# CHECK-NEXT:    Flags [ (0x6)
+# CHECK-NEXT:      PF_R (0x4)
+# CHECK-NEXT:      PF_W (0x2)
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Alignment: 4096
+# CHECK-NEXT:  }
+# CHECK-NEXT:]
diff --git a/test/elf2/relocation.s b/test/elf2/relocation.s
index 054846e..7c1e920 100644
--- a/test/elf2/relocation.s
+++ b/test/elf2/relocation.s
@@ -26,4 +26,4 @@ bar:
 // CHECK: e8 04 00 00 00  callq   4
 
 // Also check that symbols match.
-// CHECK: 0000000000001000         .text           00000000 bar
+// CHECK: 000000000001000         .text           00000000 bar
diff --git a/test/elf2/symbols.s b/test/elf2/symbols.s
index eb8a549..8fb9a0c 100644
--- a/test/elf2/symbols.s
+++ b/test/elf2/symbols.s
@@ -59,8 +59,8 @@ internal:
 // CHECK-NEXT:   SHF_ALLOC (0x2)
 // CHECK-NEXT:   SHF_WRITE (0x1)
 // CHECK-NEXT: ]
-// CHECK-NEXT: Address: 0x1004
-// CHECK-NEXT: Offset: 0x1004
+// CHECK-NEXT: Address: 0x2000
+// CHECK-NEXT: Offset: 0x2000
 // CHECK-NEXT: Size: 4
 
 // CHECK:      Name: foobar
@@ -68,7 +68,7 @@ internal:
 // CHECK-NEXT: Flags [
 // CHECK-NEXT:   SHF_ALLOC
 // CHECK-NEXT: ]
-// CHECK-NEXT: Address: 0x1008
+// CHECK-NEXT: Address: 0x3000
 
 // CHECK:      Symbols [
 // CHECK-NEXT:   Symbol {
@@ -118,7 +118,7 @@ internal:
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: common (34)
-// CHECK-NEXT:     Value: 0x1004
+// CHECK-NEXT:     Value: 0x2000
 // CHECK-NEXT:     Size: 4
 // CHECK-NEXT:     Binding: Global (0x1)
 // CHECK-NEXT:     Type: Object (0x1)
@@ -127,7 +127,7 @@ internal:
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: zed
-// CHECK-NEXT:     Value: 0x1008
+// CHECK-NEXT:     Value: 0x3000
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global (0x1)
 // CHECK-NEXT:     Type: None
@@ -136,7 +136,7 @@ internal:
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: protected
-// CHECK-NEXT:     Value: 0x1010
+// CHECK-NEXT:     Value: 0x3008
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global
 // CHECK-NEXT:     Type: None
@@ -145,7 +145,7 @@ internal:
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: zed3
-// CHECK-NEXT:     Value: 0x1010
+// CHECK-NEXT:     Value: 0x3008
 // CHECK-NEXT:     Size: 4
 // CHECK-NEXT:     Binding: Global
 // CHECK-NEXT:     Type: None
@@ -154,7 +154,7 @@ internal:
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: zed2
-// CHECK-NEXT:     Value: 0x100C
+// CHECK-NEXT:     Value: 0x3004
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global
 // CHECK-NEXT:     Type: None


More information about the llvm-commits mailing list