[llvm] c973123 - [llvm-objcopy] -O binary: do not align physical addresses
Krzysztof Parzyszek via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 20 15:45:57 PDT 2023
Author: Alexey Karyakin
Date: 2023-06-20T15:45:33-07:00
New Revision: c973123fdca7c32d2f7cbbf19883677c81e1edb0
URL: https://github.com/llvm/llvm-project/commit/c973123fdca7c32d2f7cbbf19883677c81e1edb0
DIFF: https://github.com/llvm/llvm-project/commit/c973123fdca7c32d2f7cbbf19883677c81e1edb0.diff
LOG: [llvm-objcopy] -O binary: do not align physical addresses
llvm-objcopy should not insert padding before a section if its
physical addresses is not aligned to section's alignment. This
behavior will match GNU objcopy and is important for embedded images
where the physical address is used to store the initial data image.
The loader typically will copy this image using a start symbol
created by the linker. If llvm-objcopy inserts padding before such a
section, the symbol address will not match the location in the image.
This commit refines the change in https://reviews.llvm.org/D128961
which intended to align sections which type changed from NOBITS and
their offset may not be aligned. However, it affected all sections.
Fix https://github.com/llvm/llvm-project/issues/62636
Reviewed By: jhenderson, MaskRay
Differential Revision: https://reviews.llvm.org/D150276
Added:
Modified:
llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
llvm/lib/ObjCopy/ELF/ELFObject.cpp
llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test
Removed:
################################################################################
diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index 689c9152c7ddc..70bf97b1b8482 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -97,6 +97,14 @@ static uint64_t getSectionFlagsPreserveMask(uint64_t OldFlags,
return (OldFlags & PreserveMask) | (NewFlags & ~PreserveMask);
}
+static void setSectionType(SectionBase &Sec, uint64_t Type) {
+ // If Sec's type is changed from SHT_NOBITS due to --set-section-flags,
+ // Offset may not be aligned. Align it to max(Align, 1).
+ if (Sec.Type == ELF::SHT_NOBITS && Type != ELF::SHT_NOBITS)
+ Sec.Offset = alignTo(Sec.Offset, std::max(Sec.Align, uint64_t(1)));
+ Sec.Type = Type;
+}
+
static void setSectionFlagsAndType(SectionBase &Sec, SectionFlag Flags) {
Sec.Flags = getSectionFlagsPreserveMask(Sec.Flags, getNewShfFlags(Flags));
@@ -106,7 +114,7 @@ static void setSectionFlagsAndType(SectionBase &Sec, SectionFlag Flags) {
if (Sec.Type == SHT_NOBITS &&
(!(Sec.Flags & ELF::SHF_ALLOC) ||
Flags & (SectionFlag::SecContents | SectionFlag::SecLoad)))
- Sec.Type = SHT_PROGBITS;
+ setSectionType(Sec, ELF::SHT_PROGBITS);
}
static ElfType getOutputElfType(const Binary &Bin) {
@@ -684,7 +692,7 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig,
}
auto It2 = Config.SetSectionType.find(Sec.Name);
if (It2 != Config.SetSectionType.end())
- Sec.Type = It2->second;
+ setSectionType(Sec, It2->second);
}
}
diff --git a/llvm/lib/ObjCopy/ELF/ELFObject.cpp b/llvm/lib/ObjCopy/ELF/ELFObject.cpp
index 54a4ca5a76ed0..697afab2a617b 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObject.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObject.cpp
@@ -2652,12 +2652,9 @@ Error BinaryWriter::finalize() {
// MinAddr will be skipped.
uint64_t MinAddr = UINT64_MAX;
for (SectionBase &Sec : Obj.allocSections()) {
- // If Sec's type is changed from SHT_NOBITS due to --set-section-flags,
- // Offset may not be aligned. Align it to max(Align, 1).
if (Sec.ParentSegment != nullptr)
- Sec.Addr = alignTo(Sec.Offset - Sec.ParentSegment->Offset +
- Sec.ParentSegment->PAddr,
- std::max(Sec.Align, uint64_t(1)));
+ Sec.Addr =
+ Sec.Offset - Sec.ParentSegment->Offset + Sec.ParentSegment->PAddr;
if (Sec.Type != SHT_NOBITS && Sec.Size > 0)
MinAddr = std::min(MinAddr, Sec.Addr);
}
diff --git a/llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test b/llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test
index 311da40ae0991..6eb535747f1c6 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test
@@ -254,3 +254,55 @@ ProgramHeaders:
FileSize: 0x2
MemSize: 0x13
Align: 0x1000
+
+## Test that sections do not move relative to their physical addresses if
+## the physical address is not aligned. This behavior matches GNU objcopy
+## and is important for embedded images where the physical address is
+## used to store the initial data image. Without converting the type of a
+## NOBITS section, don't align the offset. This matches GNU objcopy when
+## the physical address of a section is not aligned.
+
+# RUN: yaml2obj --docnum=8 %s -o %t8
+# RUN: llvm-objcopy -O binary --only-section=.text --only-section=.data %t8 %t8.out
+# RUN: od -A x -t x1 %t8.out | FileCheck %s --check-prefix=PHYSUNALIGNED --ignore-case
+
+# PHYSUNALIGNED: 000000 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 00
+# PHYSUNALIGNED-NEXT: 000010 11 22 33 44 bd ac cd ab
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_X86_64
+ProgramHeaders:
+ - Type: PT_LOAD
+ Flags: [ PF_X, PF_R ]
+ FirstSec: .text
+ LastSec: .text
+ VAddr: 0x200000
+ PAddr: 0x200000
+ Align: 0x1000
+ - Type: PT_LOAD
+ Flags: [ PF_W, PF_R ]
+ FirstSec: .data
+ LastSec: .data
+ VAddr: 0x400000
+ PAddr: 0x200014
+## PAddr is not aligned by sh_addralign(.data)
+ Align: 0x1000
+Sections:
+ - Name: .text
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
+ Address: 0x200000
+ AddressAlign: 0x1
+ Offset: 0x1000
+ Content: 112233445566778899aabbccddeeff0011223344
+ - Name: .data
+ Type: SHT_PROGBITS
+ Flags: [ SHF_WRITE, SHF_ALLOC ]
+ Address: 0x400000
+ AddressAlign: 0x8
+ Offset: 0x2000
+ Content: BDACCDAB
More information about the llvm-commits
mailing list