[PATCH] D41635: Align SHT_NOBITS sections is they are the first on a PT_LOAD

[Rafael Ávila de Espíndola via Phabricator via llvm-commits]

rafael created this revision.
rafael added reviewers: dim, emaste, ruiu.
Herald added a subscriber: krytarowski.

We normally want to ignore SHT_NOBITS sections when computing offsets. The sh_offset of section itself seems to be irrelevant and

- If the section is in the middle of a PT_LOAD, it will make no difference on the computed offset of the followup section.
- If it is in the end of a PT_LOAD, we want to avoid its alignment changing the offset of the followup sections.

The issue is if it is at the start of the PT_LOAD. In that case we do have to align it so that the following sections have congruent address and offset module the page size. We were not handling this case.

This should fix freebsd kernel link.


https://reviews.llvm.org/D41635

Files:
  ELF/Writer.cpp
  test/ELF/linkerscript/nobits-offset.s


Index: test/ELF/linkerscript/nobits-offset.s
===================================================================
--- /dev/null
+++ test/ELF/linkerscript/nobits-offset.s
@@ -0,0 +1,18 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "SECTIONS { \
+# RUN:         .sec1 (NOLOAD) : { . += 1; } \
+# RUN:         .text          : { *(.text) } \
+# RUN:       };" > %t.script
+# RUN: ld.lld %t.o -T %t.script -o %t
+# RUN: llvm-readelf --sections %t | FileCheck %s
+
+# We used to misalign section offsets if the first section in a
+# PT_LOAD was SHT_NOBITS.
+
+# CHECK: [ 2] .text  PROGBITS  0000000000000010 001010 000010 00  AX  0   0 16
+
+.global _start
+_start:
+  nop
+.p2align 4
Index: ELF/Writer.cpp
===================================================================
--- ELF/Writer.cpp
+++ ELF/Writer.cpp
@@ -1767,30 +1767,36 @@
 // virtual address (modulo the page size) so that the loader can load
 // executables without any address adjustment.
 static uint64_t getFileAlignment(uint64_t Off, OutputSection *Cmd) {
-  // If the section is not in a PT_LOAD, we just have to align it.
-  if (!Cmd->PtLoad)
-    return alignTo(Off, Cmd->Alignment);
-
-  OutputSection *First = Cmd->PtLoad->FirstSec;
+  OutputSection *First = Cmd->PtLoad ? Cmd->PtLoad->FirstSec : nullptr;
   // The first section in a PT_LOAD has to have congruent offset and address
   // module the page size.
   if (Cmd == First)
     return alignTo(Off, std::max<uint64_t>(Cmd->Alignment, Config->MaxPageSize),
                    Cmd->Addr);
 
+  // For SHT_NOBITS we don't want the alignment of the section to impact the
+  // offset of the sections that follow. Since nothing seems to care about the
+  // sh_offset of the SHT_NOBITS section itself, just ignore it.
+  if (Cmd->Type == SHT_NOBITS)
+    return Off;
+
+  // If the section is not in a PT_LOAD, we just have to align it.
+  if (!Cmd->PtLoad)
+    return alignTo(Off, Cmd->Alignment);
+
   // If two sections share the same PT_LOAD the file offset is calculated
   // using this formula: Off2 = Off1 + (VA2 - VA1).
   return First->Offset + Cmd->Addr - First->Addr;
 }
 
 static uint64_t setOffset(OutputSection *Cmd, uint64_t Off) {
-  if (Cmd->Type == SHT_NOBITS) {
-    Cmd->Offset = Off;
-    return Off;
-  }
-
   Off = getFileAlignment(Off, Cmd);
   Cmd->Offset = Off;
+
+  // For SHT_NOBITS we should not count the size.
+  if (Cmd->Type == SHT_NOBITS)
+    return Off;
+
   return Off + Cmd->Size;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41635.128347.patch
Type: text/x-patch
Size: 2502 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171229/1dd150c0/attachment.bin>