[ELF] - Stop setting OutSecOff too early.

[grimar]

Summary

Currently LLD sets OutSecOff in addSection for input sections.
That is a fake offset (just a rude approximation to remember the order),
used for sorting SHF_LINK_ORDER sections
(see resolveShfLinkOrder, compareByFilePosition).

There are 2 problems with such approach:

1. We currently change and reuse Size field as a value assigned. Changing size is

not good because leads to bugs. Currently, SIZEOF(.bss) for empty .bss returns 2
because we add two empty synthetic sections and increase size twice by 1.
(See PR37011: https://bugs.llvm.org/show_bug.cgi?id=37011)

2. Such approach simply does not work when --symbol-ordering-file is involved,

because processing of the ordering file might break the initial section order.

Test cases provided. This fixes PR37011.

[• espindola]

LGTM

ELF/LinkerScript.cpp

void LinkerScript::assignOffsets(OutputSection *Sec) {
 
   // If neither AT nor AT> is specified for an allocatable section, the linker
   // will set the LMA such that the difference between VMA and LMA for the
   // section is the same as the preceding output section in the same region
   // https://sourceware.org/binutils/docs-2.20/ld/Output-Section-LMA.html
   if (PhdrEntry *L = Ctx->OutSec->PtLoad)
     L->LMAOffset = Ctx->LMAOffset;
 
-  // The Size previously denoted how many InputSections had been added to this
-  // section, and was used for sorting SHF_LINK_ORDER sections. Reset it to
-  // compute the actual size value.
+  // We can call this method multiple times during the creation of
+  // thunks and want to start over calculation each time.
   Sec->Size = 0;
 
   // We visited SectionsCommands from processSectionCommands to
   // layout sections. Now, we visit SectionsCommands again to fix
   // section offsets.
   for (BaseCommand *Base : Sec->SectionCommands) {
     // This handles the assignments to symbol or to the dot.
     if (auto *Cmd = dyn_cast<SymbolAssignment>(Base)) {

ELF/OutputSections.cpp

void OutputSection::addSection(InputSection *IS) {
   uint64_t AndMask =
       Config->EMachine == EM_ARM ? (uint64_t)SHF_ARM_PURECODE : 0;
   uint64_t OrMask = ~AndMask;
   uint64_t AndFlags = (Flags & IS->Flags) & AndMask;
   uint64_t OrFlags = (Flags | IS->Flags) & OrMask;
   Flags = AndFlags | OrFlags;
 
   Alignment = std::max(Alignment, IS->Alignment);
-  IS->OutSecOff = Size++;
 
   // If this section contains a table of fixed-size entries, sh_entsize
   // holds the element size. If it contains elements of different size we
   // set sh_entsize to 0.
   if (Entsize != IS->Entsize)
     Entsize = 0;
 
   if (!IS->Assigned) {

ELF/Writer.cpp

 template <class ELFT> void Writer<ELFT>::sortSections() {
   Script->adjustSectionsBeforeSorting();
 
   // Don't sort if using -r. It is not necessary and we want to preserve the
   // relative order for SHF_LINK_ORDER sections.
   if (Config->Relocatable)
     return;
 
-  for (BaseCommand *Base : Script->SectionCommands)
-    if (auto *Sec = dyn_cast<OutputSection>(Base))
-      Sec->SortRank = getSectionRank(Sec);
-
   sortInputSections();
 
+  for (BaseCommand *Base : Script->SectionCommands) {
+    auto *OS = dyn_cast<OutputSection>(Base);
+    if (!OS)
+      continue;
+    OS->SortRank = getSectionRank(OS);
+
+    // We want to assign rude approximation values to OutSecOff fields
+    // to know the relative order of the input sections. We use it for
+    // sorting SHF_LINK_ORDER sections. See resolveShfLinkOrder().
+    uint64_t I = 0;
+    for (InputSection *Sec : getInputSections(OS))
+      Sec->OutSecOff = I++;
+  }
+
   if (!Script->HasSectionsCommand) {
     // We know that all the OutputSections are contiguous in this case.
     auto E = Script->SectionCommands.end();
     auto I = Script->SectionCommands.begin();
     auto IsSection = [](BaseCommand *Base) { return isa<OutputSection>(Base); };
     I = std::find_if(I, E, IsSection);
     E = std::find_if(llvm::make_reverse_iterator(E),
                      llvm::make_reverse_iterator(I), IsSection)

test/ELF/linkerscript/empty-section-size.test

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux /dev/null -o %t.o
+# RUN: ld.lld %t.o --script %s -o %t1
+# RUN: llvm-readobj -symbols %t1 | FileCheck %s
+
+## We had a bug when LLD increased the size of the output section even
+## if it was empty. That happened because of empty synthetic sections included.
+## Here we check that size of empty output section is zero.
+
+# CHECK:       Name: foo
+# CHECK-NEXT:  Value: 0x0
+
+SECTIONS {
+  . = 0x1000;
+  .bss : { *(.bss*) *(COMMON) }
+  foo = SIZEOF(.bss);
+}

test/ELF/linkerscript/section-metadata2.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+
+# RUN: echo "_bar" > %t.ord
+# RUN: echo "_foo" >> %t.ord
+# RUN: echo "SECTIONS { .text : { *(.text.*) } }" > %t.script
+# RUN: ld.lld --symbol-ordering-file %t.ord -o %t --script %t.script %t.o
+# RUN: llvm-objdump -s %t | FileCheck %s
+
+# CHECK:      Contents of section .text:
+# CHECK-NEXT: 02000000 00000000 01000000 00000000
+# CHECK:      Contents of section .rodata:
+# CHECK-NEXT: 02000000 00000000 01000000 00000000
+
+# RUN: echo "_foo" > %t.ord
+# RUN: echo "_bar" >> %t.ord
+# RUN: echo "SECTIONS { .text : { *(.text.*) } }" > %t.script
+# RUN: ld.lld --symbol-ordering-file %t.ord -o %t --script %t.script %t.o
+# RUN: llvm-objdump -s %t | FileCheck %s --check-prefix=INV
+
+# INV:      Contents of section .text:
+# INV-NEXT: 01000000 00000000 02000000 00000000
+# INV:      Contents of section .rodata:
+# INV-NEXT: 01000000 00000000 02000000 00000000
+
+.global _start
+_start:
+
+.section .text.foo,"a",@progbits
+_foo:
+.quad 1
+
+.section .text.bar,"a",@progbits
+_bar:
+.quad 2
+
+.section .rodata.foo,"ao",@progbits,.text.foo
+.quad 1
+
+.section .rodata.bar,"ao",@progbits,.text.bar
+.quad 2