[llvm-objcopy] Fix major layout bugs in llvm-objcopy

Somehow a few massive errors slipped though the cracks of testing.

1. The code in Segment::finalize was left over from the old layout
algorithm. In certain situations this would cause very strange issues
with segment layout. For instance in the shift-segments.test case it
would cause the second segment to have the same offset as the first.

2. In debugging this I discovered another issue. Namely section alignment
was not being computed based on Section->Align but instead
Section->Offset which is bizarre and makes no sense. I have no clue how
it worked in the first place. This issue is also fixed

3. Fixing #2 exposed a bug where things were not being written past the end
of the file that technically should have been. This was because in
certain cases (like overlapping-segments) the end of the file wouldn't
always be bumped if the offset could be chosen relative to an existing
segment that already had it's offset chosen. For fully nested segments
this is fine but for overlapping segments this leaves the end of the
file short. So I changed how the offset is bumped when looping though
segments.

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@314918 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/test/tools/llvm-objcopy/segment-shift.test b/test/tools/llvm-objcopy/segment-shift.test
new file mode 100644 (file)
index 0000000..635fdcc
--- /dev/null
+++ b/test/tools/llvm-objcopy/segment-shift.test
@@ -0,0 +1,70 @@
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy %t %t2
+# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x1000
+    AddressAlign:    0x1000
+    Size:            0x1000
+  - Name:            .text2
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x2000
+    AddressAlign:    0x1000
+    Size:            0x1000
+  - Name:            .text3
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x3000
+    AddressAlign:    0x1000
+    Size:            0x1000
+ProgramHeaders:
+  - Type: PT_LOAD
+    Flags: [ PF_R ]
+    VAddr: 0x1000
+    PAddr: 0x1000
+    Sections:
+      - Section: .text
+  - Type: PT_LOAD
+    Flags: [ PF_X, PF_R ]
+    VAddr: 0x3000
+    PAddr: 0x3000
+    Sections:
+      - Section: .text3
+
+# CHECK:     ProgramHeaders [
+# CHECK-NEXT:  ProgramHeader {
+# CHECK-NEXT:    Type: PT_LOAD (0x1)
+# CHECK-NEXT:    Offset: 0x1000
+# CHECK-NEXT:    VirtualAddress: 0x1000
+# CHECK-NEXT:    PhysicalAddress: 0x1000
+# CHECK-NEXT:    FileSize: 4096
+# CHECK-NEXT:    MemSize: 4096
+# CHECK-NEXT:    Flags [ (0x4)
+# CHECK-NEXT:      PF_R (0x4)
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Alignment: 4096
+# CHECK-NEXT:  }
+# CHECK-NEXT:  ProgramHeader {
+# CHECK-NEXT:    Type: PT_LOAD (0x1)
+# CHECK-NEXT:    Offset: 0x2000
+# CHECK-NEXT:    VirtualAddress: 0x3000
+# CHECK-NEXT:    PhysicalAddress: 0x3000
+# CHECK-NEXT:    FileSize: 4096
+# CHECK-NEXT:    MemSize: 4096
+# CHECK-NEXT:    Flags [ (0x5)
+# CHECK-NEXT:      PF_R (0x4)
+# CHECK-NEXT:      PF_X (0x1)
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Alignment: 4096
+# CHECK-NEXT:  }
+# CHECK-NEXT:]

diff --git a/tools/llvm-objcopy/Object.cpp b/tools/llvm-objcopy/Object.cpp
index 103937eaac94c9b79a9b4f7b828439df705b27be..3e6606f70950c256ac863152ddeaa0d30161a48d 100644 (file)
--- a/tools/llvm-objcopy/Object.cpp
+++ b/tools/llvm-objcopy/Object.cpp
@@ -30,18 +30,6 @@ template <class ELFT> void Segment::writeHeader(FileOutputBuffer &Out) const {
   Phdr.p_align = Align;
 }
 
-void Segment::finalize() {
-  auto FirstSec = firstSection();
-  if (FirstSec) {
-    // It is possible for a gap to be at the begining of a segment. Because of
-    // this we need to compute the new offset based on how large this gap was
-    // in the source file. Section layout should have already ensured that this
-    // space is not used for something else.
-    uint64_t OriginalOffset = Offset;
-    Offset = FirstSec->Offset - (FirstSec->OriginalOffset - OriginalOffset);
-  }
-}
-
 void Segment::writeSegment(FileOutputBuffer &Out) const {
   uint8_t *Buf = Out.getBufferStart() + Offset;
   // We want to maintain segments' interstitial data and contents exactly.
@@ -656,8 +644,8 @@ template <class ELFT> void ELFObject<ELFT>::assignOffsets() {
     } else {
       Offset = alignTo(Offset, Segment->Align == 0 ? 1 : Segment->Align);
       Segment->Offset = Offset;
-      Offset += Segment->FileSize;
     }
+    Offset = std::max(Offset, Segment->Offset + Segment->FileSize);
   }
   // Now the offset of every segment has been set we can assign the offsets
   // of each section. For sections that are covered by a segment we should use
@@ -673,7 +661,7 @@ template <class ELFT> void ELFObject<ELFT>::assignOffsets() {
       Section->Offset =
           Segment->Offset + (Section->OriginalOffset - Segment->OriginalOffset);
     } else {
-      Offset = alignTo(Offset, Section->Offset);
+      Offset = alignTo(Offset, Section->Align == 0 ? 1 : Section->Align);
       Section->Offset = Offset;
       if (Section->Type != SHT_NOBITS)
         Offset += Section->Size;
@@ -720,9 +708,6 @@ template <class ELFT> void ELFObject<ELFT>::finalize() {
     Section->NameIndex = this->SectionNames->findIndex(Section->Name);
     Section->finalize();
   }
-
-  for (auto &Segment : this->Segments)
-    Segment->finalize();
 }
 
 template <class ELFT> size_t BinaryObject<ELFT>::totalSize() const {
@@ -742,8 +727,6 @@ void BinaryObject<ELFT>::write(FileOutputBuffer &Out) const {
 }
 
 template <class ELFT> void BinaryObject<ELFT>::finalize() {
-  for (auto &Segment : this->Segments)
-    Segment->finalize();
 
   // Put all segments in offset order.
   auto CompareSegments = [](const SegPtr &A, const SegPtr &B) {

diff --git a/tools/llvm-objcopy/Object.h b/tools/llvm-objcopy/Object.h
index 02aeeca7706c856df44fd33b466709854a50758d..398eed7e16c05c911d609b1b084ee9b284b12bcb 100644 (file)
--- a/tools/llvm-objcopy/Object.h
+++ b/tools/llvm-objcopy/Object.h
@@ -93,7 +93,6 @@ public:
   Segment *ParentSegment = nullptr;
 
   Segment(llvm::ArrayRef<uint8_t> Data) : Contents(Data) {}
-  void finalize();
   const SectionBase *firstSection() const {
     if (!Sections.empty())
       return *Sections.begin();