[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

1641–1644 ↗

(On Diff #54667)

// 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.

1646 ↗

(On Diff #54667)

Rename this getFileAlignment.

1674–1676 ↗

(On Diff #54667)

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

1646 ↗

(On Diff #54667)

Done.

1658 ↗

(On Diff #54667)

Added Mask variable to simplify.

1674–1676 ↗

(On Diff #54667)

Done.

[ruiu]

ELF/Writer.cpp

1672–1679 ↗

(On Diff #54824)

Can this be

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

[ruiu]

ELF/Writer.cpp

1667–1670 ↗

(On Diff #54824)

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

[grimar]

• Addressed review comments.

[grimar]

ELF/Writer.cpp

1667–1670 ↗

(On Diff #54824)

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 ↗

(On Diff #54824)

I think yes.

[ruiu]

LGTM

ELF/Writer.cpp

1687–1694 ↗

(On Diff #54992)

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

[grimar]

ELF/Writer.cpp

1687–1694 ↗

(On Diff #54992)

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

lld/trunk/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;
+  return alignTo(Off, Target->PageSize, Sec->getVA());
+}
+
 // 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

lld/trunk/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