[lld][elf2] Sort allocated TLS sections at the start of the R/W PT_LOAD.

[ruiu]

ELF/Writer.cpp

300–302 ↗

(On Diff #37442)

It sounds like a compiler bug if it emits .tbss or .tdata without the writable bit. Do we actually care about this case?

303 ↗

(On Diff #37442)

(AFlags & SHF_TLS) && (AFlags & SHF_ALLOC)

[Bigcheese]

ELF/Writer.cpp

300–302 ↗

(On Diff #37442)

There's assembly. It also makes layout a bit simpler, as you can assume all TLS sections are together, and there's no good reason to reject it.

[ruiu]

ELF/Writer.cpp

300–302 ↗

(On Diff #37442)

That's still a user code bug, no? I'm not convinced unless it is very prevalent issue that we had to take care of this specially.

[• rafael]

Also please remove the "don't care" sections from the test:

.section v,"T",@nobits
.section u,"T"
section h,"aT",@nobits
.section f,"aT"

We only really care how we sort tls sections that are aw

ELF/Writer.cpp

327 ↗

(On Diff #37442)

Move this past the comment

// If we got here we know that both A and B are in the same PT_LOAD.

[• rafael]

In summary, this is what I think we should do.

• t.diff1 KBDownload

lld/trunk/ELF/Writer.cpp

static bool compareOutputSections(OutputSectionBase<ELFT> *A,
   // For a corresponding reason, put non exec sections first (the program
   // header PT_LOAD is not executable).
   bool AIsExec = AFlags & SHF_EXECINSTR;
   bool BIsExec = BFlags & SHF_EXECINSTR;
   if (AIsExec != BIsExec)
     return BIsExec;
 
   // If we got here we know that both A and B are in the same PT_LOAD.
+
+  // The TLS initialization block needs to be a single contiguous block in a R/W
+  // PT_LOAD, so stick TLS sections directly before R/W sections. The TLS NOBITS
+  // sections are placed here as they don't take up virtual address space in the
+  // PT_LOAD.
+  bool AIsTLS = AFlags & SHF_TLS;
+  bool BIsTLS = BFlags & SHF_TLS;
+  if (AIsTLS != BIsTLS)
+    return AIsTLS;
+
   // The next requirement we have is to put nobits sections last. The
   // reason is that the only thing the dynamic linker will see about
   // them is a p_memsz that is larger than p_filesz. Seeing that it
   // zeros the end of the PT_LOAD, so that has to correspond to the
   // nobits sections.
   bool AIsNoBits = A->getType() == SHT_NOBITS;
   bool BIsNoBits = B->getType() == SHT_NOBITS;
   if (AIsNoBits != BIsNoBits)

lld/trunk/test/elf2/section-layout.s

 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 # RUN: ld.lld2 %t -o %tout
 # RUN: llvm-readobj -sections %tout | FileCheck %s
 # REQUIRES: x86
 
 # Check that sections are laid out in the correct order.
 
 .global _start
 .text
 _start:
 
-.section h,""
-.section g,"",@nobits
-.section f,"aw",@nobits
-.section e,"aw"
+.section t,"x",@nobits
+.section s,"x"
+.section r,"w",@nobits
+.section q,"w"
+.section p,"wx",@nobits
+.section o,"wx"
+.section n,"",@nobits
+.section m,""
+
+.section l,"awx",@nobits
+.section k,"awx"
+.section j,"aw",@nobits
+.section i,"aw"
+.section g,"awT",@nobits
+.section e,"awT"
 .section d,"ax",@nobits
 .section c,"ax"
 .section b,"a",@nobits
 .section a,"a"
 
 // CHECK: Name: a
 // CHECK: Name: b
 // CHECK: Name: c
 // CHECK: Name: d
+
+// TLS sections are only sorted on NOBITS.
 // CHECK: Name: e
-// CHECK: Name: f
-// CHECK: Name: h
 // CHECK: Name: g
+
+// CHECK: Name: i
+// CHECK: Name: j
+// CHECK: Name: k
+// CHECK: Name: l
+
+// Non allocated sections are in input order.
+// CHECK: Name: t
+// CHECK: Name: s
+// CHECK: Name: r
+// CHECK: Name: q
+// CHECK: Name: p
+// CHECK: Name: o
+// CHECK: Name: n
+// CHECK: Name: m