Skip to content

Commit

Permalink
[llvm-objcopy] Don't specialize the all zero p_paddr case
Browse files Browse the repository at this point in the history
Spotted by https://reviews.llvm.org/D74755#1998673

> it looks like OrderedSegments in the function is only used to set the physical address to the virtual address when there are no physical addresses set amongst these sections.

I believe this behavior was copied from https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=6ffd79000b45e77b3625143932ffbf781b6aecab (2008-05)
The commit was made for some corner cases of very old linkers.
This special rule does not seem useful and remove it can allow us to
delete a large chunk of code.

Reviewed By: jhenderson, jakehehrlich

Differential Revision: https://reviews.llvm.org/D78786
  • Loading branch information
MaskRay committed Apr 27, 2020
1 parent dab1326 commit fd624e6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 46 deletions.
27 changes: 17 additions & 10 deletions llvm/test/tools/llvm-objcopy/ELF/binary-no-paddr.test
@@ -1,7 +1,16 @@
# RUN: yaml2obj %s -o %t
# RUN: llvm-objcopy -O binary %t %t2
# RUN: od -t x2 -v %t2 | FileCheck %s --ignore-case
# RUN: wc -c < %t2 | FileCheck %s --check-prefix=SIZE
# RUN: yaml2obj -D PADDR=1 %s -o %t1
# RUN: llvm-objcopy -O binary %t1 %t1.out
# RUN: od -t x2 -v %t1.out | FileCheck %s --ignore-case
# RUN: wc -c < %t1.out | FileCheck %s --check-prefix=SIZE

## When all p_paddr fields are 0, GNU objcopy resets LMA to VMA
## and gives a different output.
## https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=6ffd79000b45e77b3625143932ffbf781b6aecab
## We don't implement this special rule. The p_paddr=0 output is the same as
## the p_paddr=1 case.
# RUN: yaml2obj -D PADDR=0 %s -o %t0
# RUN: llvm-objcopy -O binary %t0 %t0.out
# RUN: cmp %t1.out %t0.out

!ELF
FileHeader:
Expand All @@ -26,17 +35,15 @@ ProgramHeaders:
- Type: PT_LOAD
Flags: [ PF_X, PF_R ]
VAddr: 0x1000
PAddr: 0x0000
Align: 0x1000
PAddr: [[PADDR]]
Sections:
- Section: .text
- Type: PT_LOAD
Flags: [ PF_R, PF_W ]
VAddr: 0x1004
PAddr: 0x0000
Align: 0x1000
PAddr: [[PADDR]]
Sections:
- Section: .data

# CHECK: 0000000 c3c3 c3c3 3232
# SIZE: 6
# CHECK: 0000000 3232 c3c3
# SIZE: 4
36 changes: 0 additions & 36 deletions llvm/tools/llvm-objcopy/ELF/Object.cpp
Expand Up @@ -1101,14 +1101,6 @@ static bool compareSegmentsByOffset(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;
}

void BasicELFBuilder::initFileHeader() {
Obj->Flags = 0x0;
Obj->Type = ET_REL;
Expand Down Expand Up @@ -2224,34 +2216,6 @@ Error BinaryWriter::write() {
}

Error BinaryWriter::finalize() {
// 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
// already had it's offset properly set. We only want to consider the segments
// that will affect layout of allocated sections so we only add those.
std::vector<Segment *> OrderedSegments;
for (const SectionBase &Sec : Obj.allocSections())
if (Sec.ParentSegment != nullptr)
OrderedSegments.push_back(Sec.ParentSegment);

// 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 (all_of(OrderedSegments,
[](const Segment *Seg) { return Seg->PAddr == 0; }))
for (Segment *Seg : OrderedSegments)
Seg->PAddr = Seg->VAddr;

llvm::stable_sort(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.
auto End =
std::unique(std::begin(OrderedSegments), std::end(OrderedSegments));
OrderedSegments.erase(End, std::end(OrderedSegments));

// Compute the section LMA based on its sh_offset and the containing segment's
// p_offset and p_paddr. Also compute the minimum LMA of all sections as
// MinAddr. In the output, the contents between address 0 and MinAddr will be
Expand Down

0 comments on commit fd624e6

Please sign in to comment.