[ELF] - Align sections file offsets correctly.

[grimar]

Summary

System V ABI 4.1 specifies that program header's p_vaddr should equal p_offset, modulo p_align.
(https://docs.oracle.com/cd/E19683-01/816-1386/chapter6-83432/index.html).
I was able to violate this using the linkerscript.
Patch fixes the issue.

[ruiu]

ELF/Writer.cpp

1656–1659

// Adjusts the file alignment for a given output section and returns
// its new file offset. The file offset must be the same with its
// virtual address (modulo the page size) so that the loader can load
// executables without any address adjustment.

1661

Rename this getFileAlignment.

1691–1692

You want to move these three lines of code to the new function because it handles alignment as well. Then you can remove Align parameter from the function.

[grimar]

ELF/Writer.cpp

1661

Done.

1673

Added Mask variable to simplify.

1691–1692

Done.

[ruiu]

ELF/Writer.cpp

1672–1679

Can this be

return alignTo(Off, PageSize, Sec->getVA())?

[ruiu]

ELF/Writer.cpp

1667–1670

For relocatable output, do we even have to adjust file offset?

[grimar]

• Addressed review comments.

[grimar]

ELF/Writer.cpp

1667–1670

Did not find anywhere that we should not do that for -r. But gold and bfd both do that, so I guess yes,
probably there is no special rules for relocation outputs.

1672–1679

I think yes.

[ruiu]

LGTM

ELF/Writer.cpp

1672–1679

Nice. I knew alignTo accepts the third argument, but this is the first time I see that is useful.

[grimar]

ELF/Writer.cpp

1672–1679

Yeah, I also saw it, but it was completely not obvious for me that it can be replacement for what was wrote before.

ELF/Writer.cpp

if (needsPtLoad<ELFT>(Sec)) {
       uintX_t TVA = VA + ThreadBssOffset;
       TVA = alignTo(TVA, Align);
       Sec->setVA(TVA);
       ThreadBssOffset = TVA - VA + Sec->getSize();
     }
   }
 }
 
+// Adjusts the file alignment for a given output section and returns
+// its new file offset. The file offset must be the same with its
+// virtual address (modulo the page size) so that the loader can load
+// executables without any address adjustment.
+template <class ELFT, class uintX_t>
+static uintX_t getFileAlignment(uintX_t Off, OutputSectionBase<ELFT> *Sec) {
+  uintX_t Align = Sec->getAlign();
+  if (Sec->PageAlign)
+    Align = std::max<uintX_t>(Align, Target->PageSize);
+  Off = alignTo(Off, Align);
+
+  // Relocatable output does not have program headers
+  // and does not need any other offset adjusting.
+  if (Config->Relocatable || !(Sec->getFlags() & SHF_ALLOC))
+    return Off;
+
+  uintX_t PageSize = Target->PageSize;
+  uintX_t VA = Sec->getVA();
+  uintX_t Mask = PageSize - 1;
+  if ((VA & Mask) != (Off & Mask)) {
+    uintX_t Val = ((Off & ~Mask) | (VA & Mask));
+    Off = Val < Off ? (Val + PageSize) : Val;
+  }
+  return Off;
+}
+
 // Assign file offsets to output sections.
 template <class ELFT> void Writer<ELFT>::assignFileOffsets() {
   uintX_t Off =
       Out<ELFT>::ElfHeader->getSize() + Out<ELFT>::ProgramHeaders->getSize();
 
   for (OutputSectionBase<ELFT> *Sec : OutputSections) {
     if (Sec->getType() == SHT_NOBITS) {
       Sec->setFileOffset(Off);
       continue;
     }
-    uintX_t Align = Sec->getAlign();
-    if (Sec->PageAlign)
-      Align = std::max<uintX_t>(Align, Target->PageSize);
-    Off = alignTo(Off, Align);
+
+    Off = getFileAlignment<ELFT>(Off, Sec);
     Sec->setFileOffset(Off);
     Off += Sec->getSize();
   }
   SectionHeaderOff = alignTo(Off, sizeof(uintX_t));
   FileSize = SectionHeaderOff + (OutputSections.size() + 1) * sizeof(Elf_Shdr);
 }
 
 // Finalize the program headers. We call this function after we assign

test/ELF/phdr-align.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+
+# RUN: echo "SECTIONS { \
+# RUN:  .bss : { *(.bss) } \
+# RUN:  .data : { *(.data) } \
+# RUN:  .text : { *(.text) } }" > %t.script
+# RUN: ld.lld %t.o --script %t.script -o %t
+# RUN: llvm-readobj -sections -symbols %t | FileCheck %s
+
+# CHECK:      Sections [
+# CHECK-NEXT:  Section {
+# CHECK-NEXT:    Index: 0
+# CHECK-NEXT:    Name:  (0)
+# CHECK-NEXT:    Type: SHT_NULL
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x0
+# CHECK-NEXT:    Offset: 0x0
+# CHECK-NEXT:    Size: 0
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 0
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:  Section {
+# CHECK-NEXT:    Index: 1
+# CHECK-NEXT:    Name: .bss
+# CHECK-NEXT:    Type: SHT_NOBITS
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      SHF_ALLOC
+# CHECK-NEXT:      SHF_WRITE
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x158
+# CHECK-NEXT:    Offset: 0x158
+# CHECK-NEXT:    Size: 6
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 1
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:  Section {
+# CHECK-NEXT:    Index: 2
+# CHECK-NEXT:    Name: .data
+# CHECK-NEXT:    Type: SHT_PROGBITS
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      SHF_ALLOC
+# CHECK-NEXT:      SHF_WRITE
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x15E
+# CHECK-NEXT:    Offset: 0x15E
+# CHECK-NEXT:    Size: 2
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 1
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:  Section {
+# CHECK-NEXT:    Index: 3
+# CHECK-NEXT:    Name: .text
+# CHECK-NEXT:    Type: SHT_PROGBITS
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      SHF_ALLOC
+# CHECK-NEXT:      SHF_EXECINSTR
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Address: 0x160
+# CHECK-NEXT:    Offset: 0x160
+# CHECK-NEXT:    Size: 1
+# CHECK-NEXT:    Link: 0
+# CHECK-NEXT:    Info: 0
+# CHECK-NEXT:    AddressAlignment: 4
+# CHECK-NEXT:    EntrySize: 0
+# CHECK-NEXT:  }
+
+.global _start
+.text
+_start:
+ nop
+.data
+ .word 1
+.bss
+ .space 6