From: Jake Ehrlich Date: Wed, 4 Oct 2017 17:44:42 +0000 (+0000) Subject: [llvm-objcopy] Fix major layout bugs in llvm-objcopy X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=85da10aa69a47d5d1fc7c108c474e1f392b8ae75;p=llvm [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 index 00000000000..635fdccf75e --- /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 103937eaac9..3e6606f7095 100644 --- a/tools/llvm-objcopy/Object.cpp +++ b/tools/llvm-objcopy/Object.cpp @@ -30,18 +30,6 @@ template 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 void ELFObject::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 void ELFObject::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 void ELFObject::finalize() { Section->NameIndex = this->SectionNames->findIndex(Section->Name); Section->finalize(); } - - for (auto &Segment : this->Segments) - Segment->finalize(); } template size_t BinaryObject::totalSize() const { @@ -742,8 +727,6 @@ void BinaryObject::write(FileOutputBuffer &Out) const { } template void BinaryObject::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 02aeeca7706..398eed7e16c 100644 --- a/tools/llvm-objcopy/Object.h +++ b/tools/llvm-objcopy/Object.h @@ -93,7 +93,6 @@ public: Segment *ParentSegment = nullptr; Segment(llvm::ArrayRef Data) : Contents(Data) {} - void finalize(); const SectionBase *firstSection() const { if (!Sections.empty()) return *Sections.begin();