[PATCH] D29453: [ELF] - Allow going over alignment commands in algorithm of placing non-alloc orphans.

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.

Found in linux kernel (http://lxr.free-electrons.com/source/arch/x86/boot/setup.ld#L52) that we assign
value ot _end symbol incorrectly. That was reason of "Setup too big!" assertion fail.

We placed orphans non allocatable sections before _end and so far _end value was too large.

For example without that patch list of commands after placing orphans for testcase would be:

  [0]	unique_ptr {Name="."
  [1]	unique_ptr {Name=".text"
  [2]	unique_ptr {Name=".foo"
  [3]	unique_ptr {Name=".orphan.1"
  [4]	unique_ptr {Name=".orphan.2"
  [5]	unique_ptr {Name=".comment"
  [6]	unique_ptr {Name=".symtab"
  [7]	unique_ptr {Name=".shstrtab"
  [8]	unique_ptr {Name=".strtab"
  [9]	unique_ptr {Name="."
  [10] unique_ptr {Name="_end"

but if we want to have correct _end it should be:

  [0]	unique_ptr {Name="."
  [1]	unique_ptr {Name=".text"
  [2]	unique_ptr {Name=".foo"
  [3]	unique_ptr {Name="_end"
  [4]	unique_ptr {Name=".orphan.1"
  [5]	unique_ptr {Name=".orphan.2"
  [6]	unique_ptr {Name=".comment"
  [7]	unique_ptr {Name=".symtab"
  [8]	unique_ptr {Name=".shstrtab"
  [9]	unique_ptr {Name=".strtab"

Fortunately looks there is a simple fix, patch do that.


https://reviews.llvm.org/D29453

Files:
  ELF/LinkerScript.cpp
  test/ELF/linkerscript/symbols-and-orphans.s


Index: test/ELF/linkerscript/symbols-and-orphans.s
===================================================================
--- test/ELF/linkerscript/symbols-and-orphans.s
+++ test/ELF/linkerscript/symbols-and-orphans.s
@@ -0,0 +1,40 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+
+# RUN: echo "SECTIONS { . = SIZEOF_HEADERS; \
+# RUN:  .text : { *(.text) }                \
+# RUN:  .foo : { *(.foo*) }                 \
+# RUN:  Sym = .; }" > %t.script
+# RUN: ld.lld -o %t2 --script %t.script %t
+# RUN: llvm-objdump -section-headers -t %t2 | FileCheck %s
+
+# CHECK:      Sections:
+# CHECK-NEXT: Idx Name          Size      Address          Type
+# CHECK-NEXT:   0               00000000 0000000000000000
+# CHECK-NEXT:   1 .text         00000000 00000000000000e8 TEXT DATA
+# CHECK-NEXT:   2 .foo          00000010 00000000000000e8 DATA
+# CHECK-NEXT:   3 .orphan.1     00000008 0000000000000000
+# CHECK-NEXT:   4 .orphan.2     00000008 0000000000000000
+# CHECK:      SYMBOL TABLE:
+# CHECK-NEXT: 0000000000000000 *UND* 00000000
+# CHECK-NEXT: 00000000000000f8 .foo  00000000 Sym
+
+# RUN: echo "SECTIONS { . = SIZEOF_HEADERS; \
+# RUN:  .text : { *(.text) }                \
+# RUN:  .foo : { *(.foo*) }                 \
+# RUN:  . = ALIGN(8);                       \
+# RUN:  Sym = .; }" > %t.script
+# RUN: ld.lld -o %t2 --script %t.script %t
+# RUN: llvm-objdump -section-headers -t %t2 | FileCheck %s
+
+.section .foo.1,"a"
+ .quad 0
+
+.section .foo.2,"a"
+ .quad 0
+
+.section .orphan.1,""
+ .quad 0
+
+.section .orphan.2,""
+ .quad 0
Index: ELF/LinkerScript.cpp
===================================================================
--- ELF/LinkerScript.cpp
+++ ELF/LinkerScript.cpp
@@ -681,19 +681,27 @@
 // doesn't break the intended meaning of the begin/end symbols.
 // We don't want to go over sections since Writer<ELFT>::sortSections is the
 // one in charge of deciding the order of the sections.
-// We don't want to go over alignments, since doing so in
+// We don't want to go over alignments for SHF_ALLOC sections, since doing so in
 //  rx_sec : { *(rx_sec) }
 //  . = ALIGN(0x1000);
 //  /* The RW PT_LOAD starts here*/
 //  rw_sec : { *(rw_sec) }
 // would mean that the RW PT_LOAD would become unaligned.
-static bool shouldSkip(const BaseCommand &Cmd) {
+// We allow going over alignments for non-allocatable sections, that allows
+// next script to assign _end an expected value. Non-allocatable sections placed
+// last, and can't be a reason of unaligned PT_LOAD.
+// SECTIONS {
+// .foo : { *(.foo*) }
+// . = ALIGN(16);
+// _end = .;
+// /* All non-allocatable orphans placed here */
+static bool shouldSkip(const BaseCommand &Cmd, bool IsAlloc) {
   if (isa<OutputSectionCommand>(Cmd))
     return false;
   const auto *Assign = dyn_cast<SymbolAssignment>(&Cmd);
   if (!Assign)
     return true;
-  return Assign->Name != ".";
+  return !IsAlloc || Assign->Name != ".";
 }
 
 // Orphan sections are sections present in the input files which are not
@@ -732,7 +740,7 @@
     // correct result.
     auto CmdIter = Opt.Commands.begin() + CmdIndex;
     auto E = Opt.Commands.end();
-    while (CmdIter != E && shouldSkip(**CmdIter)) {
+    while (CmdIter != E && shouldSkip(**CmdIter, Sec->Flags & SHF_ALLOC)) {
       ++CmdIter;
       ++CmdIndex;
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29453.86810.patch
Type: text/x-patch
Size: 3331 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170202/7d05740e/attachment.bin>