[llvm-objcopy] Fix handling of zero-size segments in llvm-objcopy

Some ELF files produced by lld may have zero-size segment placeholders as shown
below. Since GNU_STACK Offset is 0, the current code makes it the lowest used
offset, and relocates all the segments over the ELF header. The resulting
binary is total garbage.

This change fixes how llvm-objcopy handles PT_PHDR properlly by treating ELF
headers and the program header table as segments to allow the layout algorithm
decide where those should go.

Author: vit9696

Differential Revision: https://reviews.llvm.org/D42872

llvm-svn: 325189

diff --git a/llvm/test/tools/llvm-objcopy/marker-segment.test b/llvm/test/tools/llvm-objcopy/marker-segment.test
new file mode 100644 (file)
index 0000000..01b55eb
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/marker-segment.test
@@ -0,0 +1,111 @@
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy %t %t2
+# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_ARM
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x1000
+    AddressAlign:    0x0000000000001000
+    Content:         "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x2000
+    AddressAlign:    0x0000000000001000
+    Content:         "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"
+  - Name:            .xdata
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+  - Name:            .after
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_ALLOC ]
+    Size:            64
+ProgramHeaders:
+  - Type: PT_LOAD
+    Flags: [ PF_X, PF_R ]
+    VAddr: 0x1000
+    PAddr: 0x1000
+    Align: 0x1000
+    Sections:
+      - Section: .text
+  - Type: PT_LOAD
+    Flags: [ PF_R, PF_W ]
+    VAddr: 0x2000
+    PAddr: 0x2000
+    Align: 0x1000
+    Sections:
+      - Section: .data
+  - Type: 0x6474e551 # GNU_STACK
+    Flags: [ PF_R, PF_W ]
+    VAddr: 0x0000
+    PAddr: 0x0000
+    Align: 0x0000
+  - Type: PT_LOAD
+    Flags: [ PF_R ]
+    VAddr: 0x2010
+    PAddr: 0x2010
+    Sections:
+      - Section: .xdata
+      - Section: .after
+
+#CHECK:     ProgramHeaders [
+#CHECK-NEXT:   ProgramHeader {
+#CHECK-NEXT:    Type: PT_LOAD
+#CHECK-NEXT:    Offset: 0x1000
+#CHECK-NEXT:    VirtualAddress: 0x1000
+#CHECK-NEXT:    PhysicalAddress: 0x1000
+#CHECK-NEXT:    FileSize: 16
+#CHECK-NEXT:    MemSize: 16
+#CHECK-NEXT:    Flags [
+#CHECK-NEXT:      PF_R
+#CHECK-NEXT:      PF_X
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Alignment: 4096
+#CHECK-NEXT:  }
+#CHECK-NEXT:  ProgramHeader {
+#CHECK-NEXT:    Type: PT_LOAD
+#CHECK-NEXT:    Offset: 0x2000
+#CHECK-NEXT:    VirtualAddress: 0x2000
+#CHECK-NEXT:    PhysicalAddress: 0x2000
+#CHECK-NEXT:    FileSize: 16
+#CHECK-NEXT:    MemSize: 16
+#CHECK-NEXT:    Flags [
+#CHECK-NEXT:      PF_R
+#CHECK-NEXT:      PF_W
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Alignment: 4096
+#CHECK-NEXT:  }
+#CHECK-NEXT:  ProgramHeader {
+#CHECK-NEXT:    Type: PT_GNU_STACK
+#CHECK-NEXT:    Offset: 0x0
+#CHECK-NEXT:    VirtualAddress: 0x0
+#CHECK-NEXT:    PhysicalAddress: 0x0
+#CHECK-NEXT:    FileSize: 0
+#CHECK-NEXT:    MemSize: 0
+#CHECK-NEXT:    Flags [
+#CHECK-NEXT:      PF_R
+#CHECK-NEXT:      PF_W
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Alignment: 0
+#CHECK-NEXT:  }
+#CHECK-NEXT:  ProgramHeader {
+#CHECK-NEXT:    Type: PT_LOAD
+#CHECK-NEXT:    Offset: 0x2010
+#CHECK-NEXT:    VirtualAddress: 0x2010
+#CHECK-NEXT:    PhysicalAddress: 0x2010
+#CHECK-NEXT:    FileSize: 0
+#CHECK-NEXT:    MemSize: 64
+#CHECK-NEXT:    Flags [
+#CHECK-NEXT:      PF_R
+#CHECK-NEXT:    ]
+#CHECK-NEXT:    Alignment: 1
+#CHECK-NEXT:  }
+#CHECK-NEXT:]

diff --git a/llvm/tools/llvm-objcopy/Object.cpp b/llvm/tools/llvm-objcopy/Object.cpp
index 4a0ff51..53291a3 100644 (file)
--- a/llvm/tools/llvm-objcopy/Object.cpp
+++ b/llvm/tools/llvm-objcopy/Object.cpp
@@ -31,11 +31,10 @@ using namespace object;
 using namespace ELF;
 
 template <class ELFT> void ELFWriter<ELFT>::writePhdr(const Segment &Seg) {
-  using Elf_Ehdr = typename ELFT::Ehdr;
   using Elf_Phdr = typename ELFT::Phdr;
 
   uint8_t *Buf = BufPtr->getBufferStart();
-  Buf += sizeof(Elf_Ehdr) + Seg.Index * sizeof(Elf_Phdr);
+  Buf += Obj.ProgramHdrSegment.Offset + Seg.Index * sizeof(Elf_Phdr);
   Elf_Phdr &Phdr = *reinterpret_cast<Elf_Phdr *>(Buf);
   Phdr.p_type = Seg.Type;
   Phdr.p_flags = Seg.Flags;
@@ -456,6 +455,23 @@ static bool compareSegmentsByPAddr(const Segment *A, const Segment *B) {
   return A->Index < B->Index;
 }
 
+template <class ELFT>
+void ELFBuilder<ELFT>::setParentSegment(Segment &Child) {
+  for (auto &Parent : Obj.segments()) {
+    // Every segment will overlap with itself but we don't want a segment to
+    // be it's own parent so we avoid that situation.
+    if (&Child != &Parent && segmentOverlapsSegment(Child, Parent)) {
+      // We want a canonical "most parental" segment but this requires
+      // inspecting the ParentSegment.
+      if (compareSegmentsByOffset(&Parent, &Child))
+        if (Child.ParentSegment == nullptr ||
+            compareSegmentsByOffset(&Parent, Child.ParentSegment)) {
+          Child.ParentSegment = &Parent;
+        }
+    }
+  }
+}
+
 template <class ELFT> void ELFBuilder<ELFT>::readProgramHeaders() {
   uint32_t Index = 0;
   for (const auto &Phdr : unwrapOrError(ElfFile.program_headers())) {
@@ -482,23 +498,40 @@ template <class ELFT> void ELFBuilder<ELFT>::readProgramHeaders() {
       }
     }
   }
+
+  auto &ElfHdr = Obj.ElfHdrSegment;
+  // Creating multiple PT_PHDR segments technically is not valid, but PT_LOAD
+  // segments must not overlap, and other types fit even less.
+  ElfHdr.Type = PT_PHDR;
+  ElfHdr.Flags = 0;
+  ElfHdr.OriginalOffset = ElfHdr.Offset = 0;
+  ElfHdr.VAddr = 0;
+  ElfHdr.PAddr = 0;
+  ElfHdr.FileSize = ElfHdr.MemSize = sizeof(Elf_Ehdr);
+  ElfHdr.Align = 0;
+  ElfHdr.Index = Index++;
+
+  const auto &Ehdr = *ElfFile.getHeader();
+  auto &PrHdr = Obj.ProgramHdrSegment;
+  PrHdr.Type = PT_PHDR;
+  PrHdr.Flags = 0;
+  // The spec requires us to have p_vaddr % p_align == p_offset % p_align.
+  // Whereas this works automatically for ElfHdr, here OriginalOffset is
+  // always non-zero and to ensure the equation we assign the same value to
+  // VAddr as well.
+  PrHdr.OriginalOffset = PrHdr.Offset = PrHdr.VAddr = Ehdr.e_phoff;
+  PrHdr.PAddr = 0;
+  PrHdr.FileSize = PrHdr.MemSize = Ehdr.e_phentsize * Ehdr.e_phnum;
+  // The spec requires us to naturally align all the fields. 
+  PrHdr.Align = sizeof(Elf_Addr);
+  PrHdr.Index = Index++;
+
   // Now we do an O(n^2) loop through the segments in order to match up
   // segments.
-  for (auto &Child : Obj.segments()) {
-    for (auto &Parent : Obj.segments()) {
-      // Every segment will overlap with itself but we don't want a segment to
-      // be it's own parent so we avoid that situation.
-      if (&Child != &Parent && segmentOverlapsSegment(Child, Parent)) {
-        // We want a canonical "most parental" segment but this requires
-        // inspecting the ParentSegment.
-        if (compareSegmentsByOffset(&Parent, &Child))
-          if (Child.ParentSegment == nullptr ||
-              compareSegmentsByOffset(&Parent, Child.ParentSegment)) {
-            Child.ParentSegment = &Parent;
-          }
-      }
-    }
-  }
+  for (auto &Child : Obj.segments())
+    setParentSegment(Child);
+  setParentSegment(ElfHdr);
+  setParentSegment(PrHdr);
 }
 
 template <class ELFT>
@@ -739,7 +772,7 @@ template <class ELFT> void ELFWriter<ELFT>::writeEhdr() {
   Ehdr.e_machine = Obj.Machine;
   Ehdr.e_version = Obj.Version;
   Ehdr.e_entry = Obj.Entry;
-  Ehdr.e_phoff = sizeof(Elf_Ehdr);
+  Ehdr.e_phoff = Obj.ProgramHdrSegment.Offset;
   Ehdr.e_flags = Obj.Flags;
   Ehdr.e_ehsize = sizeof(Elf_Ehdr);
   Ehdr.e_phentsize = sizeof(Elf_Phdr);
@@ -913,17 +946,13 @@ template <class ELFT> void ELFWriter<ELFT>::assignOffsets() {
   std::vector<Segment *> OrderedSegments;
   for (auto &Segment : Obj.segments())
     OrderedSegments.push_back(&Segment);
+  OrderedSegments.push_back(&Obj.ElfHdrSegment);
+  OrderedSegments.push_back(&Obj.ProgramHdrSegment);
   OrderSegments(OrderedSegments);
-  // The size of ELF + program headers will not change so it is ok to assume
-  // that the first offset of the first segment is a good place to start
-  // outputting sections. This covers both the standard case and the PT_PHDR
-  // case.
-  uint64_t Offset;
-  if (!OrderedSegments.empty()) {
-    Offset = OrderedSegments[0]->Offset;
-  } else {
-    Offset = sizeof(Elf_Ehdr);
-  }
+  // Offset is used as the start offset of the first segment to be laid out.
+  // Since the ELF Header (ElfHdrSegment) must be at the start of the file,
+  // we start at offset 0.
+  uint64_t Offset = 0;
   Offset = LayoutSegments(OrderedSegments, Offset);
   Offset = LayoutSections(Obj.sections(), Offset);
   // If we need to write the section header table out then we need to align the

diff --git a/llvm/tools/llvm-objcopy/Object.h b/llvm/tools/llvm-objcopy/Object.h
index 05f7480..88357ad 100644 (file)
--- a/llvm/tools/llvm-objcopy/Object.h
+++ b/llvm/tools/llvm-objcopy/Object.h
@@ -238,6 +238,7 @@ public:
   Segment *ParentSegment = nullptr;
 
   Segment(ArrayRef<uint8_t> Data) : Contents(Data) {}
+  Segment() {}
 
   const SectionBase *firstSection() const {
     if (!Sections.empty())
@@ -511,11 +512,14 @@ using object::ELFObjectFile;
 
 template <class ELFT> class ELFBuilder {
 private:
+  using Elf_Addr = typename ELFT::Addr;
   using Elf_Shdr = typename ELFT::Shdr;
+  using Elf_Ehdr = typename ELFT::Ehdr;
 
   const ELFFile<ELFT> &ElfFile;
   Object &Obj;
 
+  void setParentSegment(Segment &Child);
   void readProgramHeaders();
   void initSymbolTable(SymbolTableSection *SymTab);
   void readSectionHeaders();
@@ -557,6 +561,15 @@ public:
   using ConstRange = iterator_range<pointee_iterator<
       typename std::vector<std::unique_ptr<T>>::const_iterator>>;
 
+  // It is often the case that the ELF header and the program header table are
+  // not present in any segment. This could be a problem during file layout,
+  // because other segments may get assigned an offset where either of the
+  // two should reside, which will effectively corrupt the resulting binary.
+  // Other than that we use these segments to track program header offsets
+  // when they may not follow the ELF header.
+  Segment ElfHdrSegment;
+  Segment ProgramHdrSegment;
+
   uint8_t Ident[16];
   uint64_t Entry;
   uint64_t SHOffset;