From da574d3bffbddeaf1b022c76263b0f442fe9ebfe Mon Sep 17 00:00:00 2001 From: Jake Ehrlich Date: Mon, 22 Jan 2018 19:27:30 +0000 Subject: [PATCH] [llvm-objcopy] Use physical instead of virtual address when aligning and placing sections in binary For sections with different virtual and physical addresses, alignment and placement in the output binary should be based on the physical address. Ran into this problem with a bare metal ARM project where llvm-objcopy added a lot of zero-padding before the .data section that had differing addresses. GNU objcopy did not add the padding, and after this fix, neither does llvm-objcopy. Update a test case so a section has different physical and virtual addresses. Fixes B35708 Authored By: Owen Shaw (owenpshaw) Differential Revision: https://reviews.llvm.org/D41619 llvm-svn: 323144 --- test/tools/llvm-objcopy/binary-no-paddr.test | 42 ++++++++++++++ test/tools/llvm-objcopy/binary-paddr.test | 45 +++++++++++++++ ...n-copy.test => binary-segment-layout.test} | 13 +++-- .../llvm-objcopy/two-seg-remove-end.test | 4 +- .../llvm-objcopy/two-seg-remove-first.test | 4 +- .../two-seg-remove-third-sec.test | 4 +- tools/llvm-objcopy/Object.cpp | 55 +++++++++++++------ 7 files changed, 139 insertions(+), 28 deletions(-) create mode 100644 test/tools/llvm-objcopy/binary-no-paddr.test create mode 100644 test/tools/llvm-objcopy/binary-paddr.test rename test/tools/llvm-objcopy/{basic-align-copy.test => binary-segment-layout.test} (77%) diff --git a/test/tools/llvm-objcopy/binary-no-paddr.test b/test/tools/llvm-objcopy/binary-no-paddr.test new file mode 100644 index 00000000000..4d2fba889c9 --- /dev/null +++ b/test/tools/llvm-objcopy/binary-no-paddr.test @@ -0,0 +1,42 @@ +# RUN: yaml2obj %s -o %t +# RUN: llvm-objcopy -O binary %t %t2 +# RUN: od -t x2 -v %t2 | FileCheck %s +# RUN: wc -c < %t2 | FileCheck %s --check-prefix=SIZE + +!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: 0x1004 + AddressAlign: 0x0000000000000004 + Content: "3232" +ProgramHeaders: + - Type: PT_LOAD + Flags: [ PF_X, PF_R ] + VAddr: 0x1000 + PAddr: 0x0000 + Align: 0x1000 + Sections: + - Section: .text + - Type: PT_LOAD + Flags: [ PF_R, PF_W ] + VAddr: 0x1004 + PAddr: 0x0000 + Align: 0x1000 + Sections: + - Section: .data + +# CHECK: 0000000 c3c3 c3c3 3232 +# SIZE: 6 diff --git a/test/tools/llvm-objcopy/binary-paddr.test b/test/tools/llvm-objcopy/binary-paddr.test new file mode 100644 index 00000000000..8bd7c1867a0 --- /dev/null +++ b/test/tools/llvm-objcopy/binary-paddr.test @@ -0,0 +1,45 @@ +# RUN: yaml2obj %s -o %t +# RUN: llvm-objcopy -O binary %t %t2 +# RUN: od -t x2 %t2 | FileCheck %s +# RUN: wc -c < %t2 | FileCheck %s --check-prefix=SIZE + +!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: 0x2000 + AddressAlign: 0x0000000000001000 + 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: 0x2000 + PAddr: 0x4000 + Align: 0x1000 + Sections: + - Section: .data + +# CHECK: 0000000 c3c3 c3c3 0000 0000 0000 0000 0000 0000 +# CHECK-NEXT: 0000020 0000 0000 0000 0000 0000 0000 0000 0000 +# CHECK-NEXT: * +# CHECK-NEXT: 0030000 3232 +# SIZE: 12290 diff --git a/test/tools/llvm-objcopy/basic-align-copy.test b/test/tools/llvm-objcopy/binary-segment-layout.test similarity index 77% rename from test/tools/llvm-objcopy/basic-align-copy.test rename to test/tools/llvm-objcopy/binary-segment-layout.test index 5904b043ec0..f38215f7d5e 100644 --- a/test/tools/llvm-objcopy/basic-align-copy.test +++ b/test/tools/llvm-objcopy/binary-segment-layout.test @@ -14,24 +14,25 @@ Sections: Type: SHT_PROGBITS Flags: [ SHF_ALLOC, SHF_EXECINSTR ] AddressAlign: 0x0000000000001000 + Address: 0x00 Content: "c3c3c3c3" - Name: .data Type: SHT_PROGBITS Flags: [ SHF_ALLOC, SHF_EXECINSTR ] - AddressAlign: 0x0000000000001000 + AddressAlign: 0x0000000000000008 + Address: 0x08 Content: "3232" ProgramHeaders: - Type: PT_LOAD Flags: [ PF_X, PF_R ] + VAddr: 0x00 Sections: - Section: .text - Type: PT_LOAD Flags: [ PF_R ] + VAddr: 0x08 Sections: - Section: .data -# CHECK: 0000000 c3c3 c3c3 0000 0000 0000 0000 0000 0000 -# CHECK-NEXT: 0000020 0000 0000 0000 0000 0000 0000 0000 0000 -# CHECK-NEXT: * -# CHECK-NEXT: 0010000 3232 -# SIZE: 4098 +# CHECK: 0000000 c3c3 c3c3 0000 0000 3232 +# SIZE: 10 diff --git a/test/tools/llvm-objcopy/two-seg-remove-end.test b/test/tools/llvm-objcopy/two-seg-remove-end.test index 9f625fb5f0b..f78a96410c1 100644 --- a/test/tools/llvm-objcopy/two-seg-remove-end.test +++ b/test/tools/llvm-objcopy/two-seg-remove-end.test @@ -48,8 +48,8 @@ ProgramHeaders: - Section: .text2 - Type: PT_LOAD Flags: [ PF_R ] - VAddr: 0x1000 - PAddr: 0x1000 + VAddr: 0x3000 + PAddr: 0x3000 Sections: - Section: .text3 - Section: .text4 diff --git a/test/tools/llvm-objcopy/two-seg-remove-first.test b/test/tools/llvm-objcopy/two-seg-remove-first.test index 96b39ee3f79..7d0ffefbb81 100644 --- a/test/tools/llvm-objcopy/two-seg-remove-first.test +++ b/test/tools/llvm-objcopy/two-seg-remove-first.test @@ -48,8 +48,8 @@ ProgramHeaders: - Section: .text2 - Type: PT_LOAD Flags: [ PF_R ] - VAddr: 0x1000 - PAddr: 0x1000 + VAddr: 0x3000 + PAddr: 0x3000 Sections: - Section: .text3 - Section: .text4 diff --git a/test/tools/llvm-objcopy/two-seg-remove-third-sec.test b/test/tools/llvm-objcopy/two-seg-remove-third-sec.test index ad7af7f1221..bedd4aac6ae 100644 --- a/test/tools/llvm-objcopy/two-seg-remove-third-sec.test +++ b/test/tools/llvm-objcopy/two-seg-remove-third-sec.test @@ -48,8 +48,8 @@ ProgramHeaders: - Section: .text2 - Type: PT_LOAD Flags: [ PF_R ] - VAddr: 0x1000 - PAddr: 0x1000 + VAddr: 0x3000 + PAddr: 0x3000 Sections: - Section: .text3 - Section: .text4 diff --git a/tools/llvm-objcopy/Object.cpp b/tools/llvm-objcopy/Object.cpp index 4d69e680236..944075d6197 100644 --- a/tools/llvm-objcopy/Object.cpp +++ b/tools/llvm-objcopy/Object.cpp @@ -409,7 +409,7 @@ static bool segmentOverlapsSegment(const Segment &Child, Parent.OriginalOffset + Parent.FileSize > Child.OriginalOffset; } -static bool compareSegments(const Segment *A, const Segment *B) { +static bool compareSegmentsByOffset(const Segment *A, const Segment *B) { // Any segment without a parent segment should come before a segment // that has a parent segment. if (A->OriginalOffset < B->OriginalOffset) @@ -419,6 +419,14 @@ static bool compareSegments(const Segment *A, const Segment *B) { return A->Index < B->Index; } +static bool compareSegmentsByPAddr(const Segment *A, const Segment *B) { + if (A->PAddr < B->PAddr) + return true; + if (A->PAddr > B->PAddr) + return false; + return A->Index < B->Index; +} + template void Object::readProgramHeaders(const ELFFile &ElfFile) { uint32_t Index = 0; @@ -456,9 +464,9 @@ void Object::readProgramHeaders(const ELFFile &ElfFile) { if (&Child != &Parent && segmentOverlapsSegment(*Child, *Parent)) { // We want a canonical "most parental" segment but this requires // inspecting the ParentSegment. - if (compareSegments(Parent.get(), Child.get())) + if (compareSegmentsByOffset(Parent.get(), Child.get())) if (Child->ParentSegment == nullptr || - compareSegments(Parent.get(), Child->ParentSegment)) { + compareSegmentsByOffset(Parent.get(), Child->ParentSegment)) { Child->ParentSegment = Parent.get(); } } @@ -784,7 +792,8 @@ static uint64_t alignToAddr(uint64_t Offset, uint64_t Addr, uint64_t Align) { // Orders segments such that if x = y->ParentSegment then y comes before x. static void OrderSegments(std::vector &Segments) { - std::stable_sort(std::begin(Segments), std::end(Segments), compareSegments); + std::stable_sort(std::begin(Segments), std::end(Segments), + compareSegmentsByOffset); } // This function finds a consistent layout for a list of segments starting from @@ -793,7 +802,7 @@ static void OrderSegments(std::vector &Segments) { static uint64_t LayoutSegments(std::vector &Segments, uint64_t Offset) { assert(std::is_sorted(std::begin(Segments), std::end(Segments), - compareSegments)); + compareSegmentsByOffset)); // The only way a segment should move is if a section was between two // segments and that section was removed. If that section isn't in a segment // then it's acceptable, but not ideal, to simply move it to after the @@ -947,7 +956,20 @@ template void BinaryObject::finalize() { OrderedSegments.push_back(Section->ParentSegment); } } - OrderSegments(OrderedSegments); + + // For binary output, we're going to use physical addresses instead of + // virtual addresses, since a binary output is used for cases like ROM + // loading and physical addresses are intended for ROM loading. + // However, if no segment has a physical address, we'll fallback to using + // virtual addresses for all. + if (std::all_of(std::begin(OrderedSegments), std::end(OrderedSegments), + [](const Segment *Segment) { return Segment->PAddr == 0; })) + for (const auto &Segment : OrderedSegments) + Segment->PAddr = Segment->VAddr; + + std::stable_sort(std::begin(OrderedSegments), std::end(OrderedSegments), + compareSegmentsByPAddr); + // Because we add a ParentSegment for each section we might have duplicate // segments in OrderedSegments. If there were duplicates then LayoutSegments // would do very strange things. @@ -955,6 +977,8 @@ template void BinaryObject::finalize() { std::unique(std::begin(OrderedSegments), std::end(OrderedSegments)); OrderedSegments.erase(End, std::end(OrderedSegments)); + uint64_t Offset = 0; + // Modify the first segment so that there is no gap at the start. This allows // our layout algorithm to proceed as expected while not out writing out the // gap at the start. @@ -963,19 +987,18 @@ template void BinaryObject::finalize() { auto Sec = Seg->firstSection(); auto Diff = Sec->OriginalOffset - Seg->OriginalOffset; Seg->OriginalOffset += Diff; - // The size needs to be shrunk as well + // The size needs to be shrunk as well. Seg->FileSize -= Diff; - Seg->MemSize -= Diff; - // The VAddr needs to be adjusted so that the alignment is correct as well - Seg->VAddr += Diff; - Seg->PAddr = Seg->VAddr; - // We don't want this to be shifted by alignment so we need to set the - // alignment to zero. - Seg->Align = 0; + // The PAddr needs to be increased to remove the gap before the first + // section. + Seg->PAddr += Diff; + uint64_t LowestPAddr = Seg->PAddr; + for (auto &Segment : OrderedSegments) { + Segment->Offset = Segment->PAddr - LowestPAddr; + Offset = std::max(Offset, Segment->Offset + Segment->FileSize); + } } - uint64_t Offset = LayoutSegments(OrderedSegments, 0); - // TODO: generalize LayoutSections to take a range. Pass a special range // constructed from an iterator that skips values for which a predicate does // not hold. Then pass such a range to LayoutSections instead of constructing