[PATCH] Issue a linker error if TLS sections are not adjacent

[H.J. Lu]

On Wed, Jan 22, 2014 at 7:55 PM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, Jan 22, 2014 at 11:30:13AM -0800, H.J. Lu wrote:
>>       PR ld/16498
>>       * elf.c (_bfd_elf_map_sections_to_segments): Issue a linker error
>>       if TLS sections are not adjacent.
>
> This part is OK.

I'd like to augment my patch like

---
index 3815e32..c0303fc 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -4117,11 +4117,31 @@ _bfd_elf_map_sections_to_segments (bfd *abfd,
struct bfd_link_info *info)
       /* Mandated PF_R.  */
       m->p_flags = PF_R;
       m->p_flags_valid = 1;
+      s = first_tls;
       for (i = 0; i < (unsigned int) tls_count; ++i)
         {
-          BFD_ASSERT (first_tls->flags & SEC_THREAD_LOCAL);
-          m->sections[i] = first_tls;
-          first_tls = first_tls->next;
+          if ((s->flags & SEC_THREAD_LOCAL) == 0)
+        {
+          _bfd_error_handler
+            (_("%B: TLS sections are not adjacent:"), abfd);
+          s = first_tls;
+          i = 0;
+          while (i < (unsigned int) tls_count)
+            {
+              if ((s->flags & SEC_THREAD_LOCAL) != 0)
+            {
+              _bfd_error_handler (_("        TLS: %A"), s);
+              i++;
+            }
+              else
+            _bfd_error_handler (_("    non-TLS: %A"), s);
+              s = s->next;
+            }
+          bfd_set_error (bfd_error_bad_value);
+          goto error_return;
+        }
+          m->sections[i] = s;
+          s = s->next;
         }

       *pm = m;
---

So that user has a clue what went wrong.  This gives me:

./ld  -shared -T tbss.t -o tls.so x.o
./ld: tls.so: TLS sections are not adjacent:
./ld:         TLS: .tdata
./ld:     non-TLS: .data
./ld:         TLS: .tbss
./ld: map sections to segments failed: Bad value

>> +SECTIONS
>> +{
>> +  .tdata  : { *(.tdata) }
>> +  .data        : { *(.data)
>> +  }
>> +}
>
> On the other hand, the testcase is really showing a fault in orphan
> section handling.  If ld was a little more clever, it would put .tbss
> after .tdata and your testcase would no longer give an error.
>

I am enclosing a simple patch which does it.  However, it
doesn't solve the second testcase in the bug report since
it has a linker script:

---
SECTIONS
{
  tls_data_init  : { *(.tdata .tdata.* .gnu.linkonce.td.*) }
  .data           :
  {
    *(.data .data.* .gnu.linkonce.d.*)
  }
  /DISCARD/ : { *(.note.GNU-stack) *(.gnu_debuglink) *(.gnu.lto_*) }
}
---

which is derived from the real application I got.  Since normally
we won't see this problem, I don't think it is worth the effect even
though the change is quite small.  I decided not to include it in
my fix since it doesn't solve my original problem.   I can include
it as well as the second testcase.

Thanks.

-- 
H.J.
-
--- /tmp/elf32.em   2014-01-22 09:49:47.335790428 -0800
+++ elf32.em    2014-01-22 10:14:47.147161310 -0800
@@ -1651,6 +1651,9 @@ gldelf_x86_64_place_orphan (asection *s,
       { ".rodata",
     SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_DATA,
     0, 0, 0, 0 },
+      { ".tdata",
+    SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_DATA | SEC_THREAD_LOCAL,
+    0, 0, 0, 0 },
       { ".data",
     SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_DATA,
     0, 0, 0, 0 },
@@ -1674,6 +1677,7 @@ gldelf_x86_64_place_orphan (asection *s,
     {
       orphan_text = 0,
       orphan_rodata,
+      orphan_tdata,
       orphan_data,
       orphan_bss,
       orphan_rel,
@@ -1801,6 +1805,8 @@ gldelf_x86_64_place_orphan (asection *s,
     place = &hold[orphan_bss];
   else if ((s->flags & SEC_SMALL_DATA) != 0)
     place = &hold[orphan_sdata];
+  else if ((s->flags & SEC_THREAD_LOCAL) != 0)
+    place = &hold[orphan_tdata];
   else if ((s->flags & SEC_READONLY) == 0)
     place = &hold[orphan_data];
   else if (((iself && (sh_type == SHT_RELA || sh_type == SHT_REL))