[llvm-objcopy] -O binary: do not align physical addresses

[quic-akaryaki]

Summary

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.

This is an updated version of https://reviews.llvm.org/D150276, with
a fix to a test endianness issue.

Fix https://github.com/llvm/llvm-project/issues/62636

[quic-akaryaki]

This change fixes an endianness issue with the test in https://reviews.llvm.org/D150276.

[quic-akaryaki]

I'll update the original one.

llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp

static uint64_t getSectionFlagsPreserveMask(uint64_t OldFlags,
   const uint64_t PreserveMask =
       (ELF::SHF_COMPRESSED | ELF::SHF_GROUP | ELF::SHF_LINK_ORDER |
        ELF::SHF_MASKOS | ELF::SHF_MASKPROC | ELF::SHF_TLS |
        ELF::SHF_INFO_LINK) &
       ~ELF::SHF_EXCLUDE;
   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));
 
   // In GNU objcopy, certain flags promote SHT_NOBITS to SHT_PROGBITS. This rule
   // may promote more non-ALLOC sections than GNU objcopy, but it is fine as
   // non-ALLOC SHT_NOBITS sections do not make much sense.
   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) {
   // Infer output ELF type from the input ELF object
   if (isa<ELFObjectFile<ELF32LE>>(Bin))
     return ELFT_ELF32LE;
   if (isa<ELFObjectFile<ELF64LE>>(Bin))
     return ELFT_ELF64LE;
if (!Config.SetSectionFlags.empty() || !Config.SetSectionType.empty()) {
     for (auto &Sec : Obj.sections()) {
       const auto Iter = Config.SetSectionFlags.find(Sec.Name);
       if (Iter != Config.SetSectionFlags.end()) {
         const SectionFlagsUpdate &SFU = Iter->second;
         setSectionFlagsAndType(Sec, SFU.NewFlags);
       }
       auto It2 = Config.SetSectionType.find(Sec.Name);
       if (It2 != Config.SetSectionType.end())
-        Sec.Type = It2->second;
+        setSectionType(Sec, It2->second);
     }
   }
 
   if (!Config.SectionsToRename.empty()) {
     std::vector<RelocationSectionBase *> RelocSections;
     DenseSet<SectionBase *> RenamedSections;
     for (SectionBase &Sec : Obj.sections()) {
       auto *RelocSec = dyn_cast<RelocationSectionBase>(&Sec);

llvm/lib/ObjCopy/ELF/ELFObject.cpp

 
 Error BinaryWriter::finalize() {
   // 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 non-empty
   // sections as MinAddr. In the output, the contents between address 0 and
   // 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);
   }
 
   // Now that every section has been laid out we just need to compute the total
   // file size. This might not be the same as the offset returned by
   // layoutSections, because we want to truncate the last segment to the end of
   // its last non-empty section, to match GNU objcopy's behaviour.

llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test

ProgramHeaders:
   - Type:     PT_LOAD
     Flags:    [ PF_R, PF_W, PF_X ]
     Offset:   0x1000
     VAddr:    0x1000
     PAddr:    0x1000
     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