[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

llvm-svn: 314918

1 files changed, 2 insertions, 19 deletions

diff --git a/llvm/tools/llvm-objcopy/Object.cpp b/llvm/tools/llvm-objcopy/Object.cpp
index 103937eaac9..3e6606f7095 100644
--- a/llvm/tools/llvm-objcopy/Object.cpp
+++ b/llvm/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) {