From 13153eef561c5eb3295e1d0f3018ff32b54510af Mon Sep 17 00:00:00 2001 From: Jake Ehrlich Date: Thu, 2 Nov 2017 23:24:04 +0000 Subject: [PATCH] [llvm-objcopy] Fix bug in how segment alignment was being handled Just aligning segment offsets to segment alignment is incorrect and also wastes more space than is needed. The requirement is that p_offset == p_addr modulo p_align *not* that p_offset == 0 modulo p_align. Generally speaking we've been using p_addr == 0 modulo p_align. In fact yaml2obj can't even produce a valid situation which causes llvm-objcopy to produce incorrect results because alignment and offset were both inherited from the sections the program header covers. This change fixes this bad behavior in llvm-objcopy. Differential Revision: https://reviews.llvm.org/D39132 llvm-svn: 317284 --- .../check-addr-offset-align-binary.test | 40 +++++++++++++ .../llvm-objcopy/check-addr-offset-align.test | 67 ++++++++++++++++++++++ llvm/tools/llvm-objcopy/Object.cpp | 20 ++++++- 3 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 llvm/test/tools/llvm-objcopy/check-addr-offset-align-binary.test create mode 100644 llvm/test/tools/llvm-objcopy/check-addr-offset-align.test diff --git a/llvm/test/tools/llvm-objcopy/check-addr-offset-align-binary.test b/llvm/test/tools/llvm-objcopy/check-addr-offset-align-binary.test new file mode 100644 index 0000000..755acce --- /dev/null +++ b/llvm/test/tools/llvm-objcopy/check-addr-offset-align-binary.test @@ -0,0 +1,40 @@ +# RUN: yaml2obj %s -o %t +# RUN: llvm-objcopy -O binary %t %t2 +# RUN: od -t x1 %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: 0x0000000000001000 + Content: "c3c3c3c3" + - Name: .data + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC ] + Address: 0x1008 + AddressAlign: 0x0000000000000008 + Content: "3232" +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: 0x1008 + PAddr: 0x1008 + Align: 0x1000 + Sections: + - Section: .data + +# CHECK: 0000000 c3 c3 c3 c3 00 00 00 00 32 32 diff --git a/llvm/test/tools/llvm-objcopy/check-addr-offset-align.test b/llvm/test/tools/llvm-objcopy/check-addr-offset-align.test new file mode 100644 index 0000000..ca2367b --- /dev/null +++ b/llvm/test/tools/llvm-objcopy/check-addr-offset-align.test @@ -0,0 +1,67 @@ +# 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: 0x0000000000001000 + Content: "c3c3c3c3" + - Name: .data + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC ] + Address: 0x1008 + AddressAlign: 0x0000000000000008 + Content: "3232" +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: 0x1008 + PAddr: 0x1008 + Align: 0x1000 + Sections: + - Section: .data + +#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: 4 +#CHECK-NEXT: MemSize: 4 +#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: 0x1008 +#CHECK-NEXT: VirtualAddress: 0x1008 +#CHECK-NEXT: PhysicalAddress: 0x1008 +#CHECK-NEXT: FileSize: 2 +#CHECK-NEXT: MemSize: 2 +#CHECK-NEXT: Flags [ +#CHECK-NEXT: PF_R +#CHECK-NEXT: PF_W +#CHECK-NEXT: ] +#CHECK-NEXT: Alignment: 4096 +#CHECK-NEXT: } +#CHECK-NEXT:] diff --git a/llvm/tools/llvm-objcopy/Object.cpp b/llvm/tools/llvm-objcopy/Object.cpp index 22ae47f..5f9864d 100644 --- a/llvm/tools/llvm-objcopy/Object.cpp +++ b/llvm/tools/llvm-objcopy/Object.cpp @@ -685,6 +685,19 @@ template void ELFObject::sortSections() { CompareSections); } +static uint64_t alignToAddr(uint64_t Offset, uint64_t Addr, uint64_t Align) { + // Calculate Diff such that (Offset + Diff) & -Align == Addr & -Align. + if (Align == 0) + Align = 1; + auto Diff = + static_cast(Addr % Align) - static_cast(Offset % Align); + // We only want to add to Offset, however, so if Diff < 0 we can add Align and + // (Offset + Diff) & -Align == Addr & -Align will still hold. + if (Diff < 0) + Diff += Align; + return Offset + Diff; +} + template void ELFObject::assignOffsets() { // We need a temporary list of segments that has a special order to it // so that we know that anytime ->ParentSegment is set that segment has @@ -728,7 +741,7 @@ template void ELFObject::assignOffsets() { Segment->Offset = Parent->Offset + Segment->OriginalOffset - Parent->OriginalOffset; } else { - Offset = alignTo(Offset, Segment->Align == 0 ? 1 : Segment->Align); + Offset = alignToAddr(Offset, Segment->VAddr, Segment->Align); Segment->Offset = Offset; } Offset = std::max(Offset, Segment->Offset + Segment->FileSize); @@ -829,8 +842,9 @@ template void BinaryObject::finalize() { uint64_t Offset = 0; for (auto &Segment : this->Segments) { - if (Segment->Type == PT_LOAD && Segment->firstSection() != nullptr) { - Offset = alignTo(Offset, Segment->Align); + if (Segment->Type == llvm::ELF::PT_LOAD && + Segment->firstSection() != nullptr) { + Offset = alignToAddr(Offset, Segment->VAddr, Segment->Align); Segment->Offset = Offset; Offset += Segment->FileSize; } -- 2.7.4