[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

3 files changed, 124 insertions, 3 deletions

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 00000000000..755acceeda2
--- /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 00000000000..ca2367ba434
--- /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 22ae47f1cac..5f9864d9cc0 100644
--- a/llvm/tools/llvm-objcopy/Object.cpp
+++ b/llvm/tools/llvm-objcopy/Object.cpp
@@ -685,6 +685,19 @@ template <class ELFT> void ELFObject<ELFT>::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<int64_t>(Addr % Align) - static_cast<int64_t>(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 <class ELFT> void ELFObject<ELFT>::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 <class ELFT> void ELFObject<ELFT>::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 <class ELFT> void BinaryObject<ELFT>::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;
     }