Problem with LD-to-LE relaxation in gnu2 tls-dialect (IA32 and X86-64)

[Alexandre Oliva <aoliva at redhat dot com>]

On Jun 19, 2008, "Cary Coutant" <ccoutant@google.com> wrote:

> Here's a test case for the problem I was talking to you about...

Thanks for your report.

So, basically the problem was that, when relaxing Local Dynamic to
Local Exec, when linking an executable, we set up the base address as
the lowest address of the executable's TLS Block (_TLS_MODULE_BASE_),
and then added to it the NTPOFF of the variables (relaxed from
DTPOFF), which by itself would have been enough to get to the variable
addresses starting from the Thread Pointer.  Oops.

Thanks for pointing this out.  I'd missed this even when writing the
linker testcases.  So much for writing testcases to the spec rather
than for expected behavior :-(

> In gold, I set _TLS_MODULE_BASE_ properly, and this example works.

> I tried this simple patch in bfd/elf32-i386.c:

>  	  if (!(_bfd_generic_link_add_one_symbol
>  		(info, output_bfd, "_TLS_MODULE_BASE_", BSF_LOCAL,
> -		 tls_sec, 0, NULL, FALSE,
> +		 tls_sec, tls_sec->size, NULL, FALSE,
bed-> collect, &bh)))

> But it looks like tls_sec->size is still zero at this point, so it
> didn't help. I can look into this further and submit a patch if you
> like. I'll also submit a bug report if you agree with my analysis of
> the problem.

AFAICT, gold will actually work in main executables, but break in
dynamic libraries, because there _TLS_MODULE_BASE_ *is* supposed to be
the lowest address.  I haven't got as far as as building gold to
verify what I grasped from the code (or to fix it, for that matter),
but I did verify that LD worked in dynamic libraries before the patch
with the old ld, it was broken only for main executables.  So, unless
I misunderstood something, gold will need some fixing as well.  Try to
compile the attached source file (slightly modified from the one you
sent me) as below (assuming your gcc will use gold as the linker) and
you'll see.

gcc -O2 -mtls-dialect=gnu2 tls.c -DTEST -fpic -shared -o libtls.so
gcc -O2 -mtls-dialect=gnu2 tls.c -DMAIN libtls.so -o tls-lib -Wl,-R.
gcc -O2 -mtls-dialect=gnu2 tls.c -o tls-dyn
gcc -O2 -mtls-dialect=gnu2 tls.c -static -o tls-stt

./tls-lib
./tls-dyn
./tls-stt

With the patch below (after the test file), using the old linker, both
-m32 and -m64, all 3 tests ran successfully for me, and the linker
i386 and x86-64 fixed testsuites passed, on an x86_64-linux-gnu native
build.  Ok to install?

FWIW, I've also amended the ABI extension RFC so as to note this
situation and specify the expected outcome.  The updated version is
available at the following URL:
http://people.redhat.com/aoliva/writeups/TLS/RFC-TLSDESC-x86.txt

#if !defined (MAIN) || defined (TEST)
#include <stdio.h>

static __thread int v2;
static __thread int v4 = 4;

void
t2()
{
  v2 = 20;
}

void
t4()
{
  v4 = 40;
}

int
test()
{
  int tv2, tv4;
  t2();
  t4();
  tv2 = v2;
  tv4 = v4;
  if (tv2 != 20)
    printf("v2 != 20\n");;
  if (tv4 != 40)
    printf("v4 != 40\n");;
  return 0;
}
#else
extern int test(void);
#endif

#if ! defined (TEST) || defined (MAIN)

int
main()
{
  test();
}

#endif

for  bfd/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* elf32-i386.c (struct elf_i386_link_hash_table): Added field
	tls_module_base.
	(elf_i386_link_hash_table_create): Initialize it.
	(elf_i386_always_size_sections): Set it.
	(set_tls_module_base): New.
	(elf_i386_relocate_sections): Call it.
	* elf64-x86-64.c (struct elf64_x86_64_link_hash_table): Added
	field tls_module_base.
	(elf64_x86_64_link_hash_table_create): Initialize it.
	(elf64_x86_64_always_size_sections): Set it.
	(set_tls_module_base): New.
	(elf64_x86_64_relocate_sections): Call it.
	Reported by Cary Coutant <ccoutant@google.com>

for ld/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* ld-i386/tlsbindesc.dd: Adjust incorrect expectations for LD to
	LE relaxation.
	* ld-x86-64/tlsbindesc.dd: Likewise.
	* ld-i386/tlsbindesc.rd: Adjust address of _TLS_MODULE_BASE_.
	* ld-x86-64/tlsbindesc.rd: Likewise.
	Reported by Cary Coutant <ccoutant@google.com>

Index: bfd/elf32-i386.c
===================================================================
--- bfd/elf32-i386.c.orig	2008-07-04 02:45:50.000000000 -0300
+++ bfd/elf32-i386.c	2008-07-04 02:47:28.000000000 -0300
@@ -692,6 +692,9 @@ struct elf_i386_link_hash_table
 
   /* Small local sym to section mapping cache.  */
   struct sym_sec_cache sym_sec;
+
+  /* _TLS_MODULE_BASE_ symbol.  */
+  struct bfd_link_hash_entry *tls_module_base;
 };
 
 /* Get the i386 ELF linker hash table from a link_info structure.  */
@@ -767,6 +770,7 @@ elf_i386_link_hash_table_create (bfd *ab
   ret->is_vxworks = 0;
   ret->srelplt2 = NULL;
   ret->plt0_pad_byte = 0;
+  ret->tls_module_base = NULL;
 
   return &ret->elf.root;
 }
@@ -2431,6 +2435,9 @@ elf_i386_always_size_sections (bfd *outp
 		 tls_sec, 0, NULL, FALSE,
 		 bed->collect, &bh)))
 	    return FALSE;
+
+	  elf_i386_hash_table (info)->tls_module_base = bh;
+
 	  tlsbase = (struct elf_link_hash_entry *)bh;
 	  tlsbase->def_regular = 1;
 	  tlsbase->other = STV_HIDDEN;
@@ -2475,6 +2482,27 @@ elf_i386_fake_sections (bfd *abfd ATTRIB
   return TRUE;
 }
 
+/* _TLS_MODULE_BASE_ needs to be treated especially when linking
+   executables.  Rather than setting it to the beginning of the TLS
+   section, we have to set it to the end.    This function may be called
+   multiple times, it is idempotent.  */
+
+static void
+set_tls_module_base (struct bfd_link_info *info)
+{
+  struct bfd_link_hash_entry *base;
+
+  if (!info->executable)
+    return;
+
+  base = elf_i386_hash_table (info)->tls_module_base;
+
+  if (!base)
+    return;
+
+  base->u.def.value = elf_hash_table (info)->tls_size;
+}
+
 /* Return the base VMA address which should be subtracted from real addresses
    when resolving @dtpoff relocation.
    This is PT_TLS segment p_vaddr.  */
@@ -2536,6 +2564,8 @@ elf_i386_relocate_section (bfd *output_b
 		    && !strcmp (input_section->output_section->name,
 				".tls_vars"));
 
+  set_tls_module_base (info);
+
   rel = relocs;
   relend = relocs + input_section->reloc_count;
   for (; rel < relend; rel++)
Index: bfd/elf64-x86-64.c
===================================================================
--- bfd/elf64-x86-64.c.orig	2008-07-04 02:45:51.000000000 -0300
+++ bfd/elf64-x86-64.c	2008-07-04 02:47:28.000000000 -0300
@@ -503,6 +503,9 @@ struct elf64_x86_64_link_hash_table
 
   /* Small local sym to section mapping cache.  */
   struct sym_sec_cache sym_sec;
+
+  /* _TLS_MODULE_BASE_ symbol.  */
+  struct bfd_link_hash_entry *tls_module_base;
 };
 
 /* Get the x86-64 ELF linker hash table from a link_info structure.  */
@@ -575,6 +578,7 @@ elf64_x86_64_link_hash_table_create (bfd
   ret->tlsdesc_got = 0;
   ret->tls_ld_got.refcount = 0;
   ret->sgotplt_jump_table_size = 0;
+  ret->tls_module_base = NULL;
 
   return &ret->elf.root;
 }
@@ -2239,6 +2243,9 @@ elf64_x86_64_always_size_sections (bfd *
 		 tls_sec, 0, NULL, FALSE,
 		 bed->collect, &bh)))
 	    return FALSE;
+
+	  elf64_x86_64_hash_table (info)->tls_module_base = bh;
+
 	  tlsbase = (struct elf_link_hash_entry *)bh;
 	  tlsbase->def_regular = 1;
 	  tlsbase->other = STV_HIDDEN;
@@ -2249,6 +2256,27 @@ elf64_x86_64_always_size_sections (bfd *
   return TRUE;
 }
 
+/* _TLS_MODULE_BASE_ needs to be treated especially when linking
+   executables.  Rather than setting it to the beginning of the TLS
+   section, we have to set it to the end.  This function may be called
+   multiple times, it is idempotent.  */
+
+static void
+set_tls_module_base (struct bfd_link_info *info)
+{
+  struct bfd_link_hash_entry *base;
+
+  if (!info->executable)
+    return;
+
+  base = elf64_x86_64_hash_table (info)->tls_module_base;
+
+  if (!base)
+    return;
+
+  base->u.def.value = elf_hash_table (info)->tls_size;
+}
+
 /* Return the base VMA address which should be subtracted from real addresses
    when resolving @dtpoff relocation.
    This is PT_TLS segment p_vaddr.  */
@@ -2319,6 +2347,8 @@ elf64_x86_64_relocate_section (bfd *outp
   local_got_offsets = elf_local_got_offsets (input_bfd);
   local_tlsdesc_gotents = elf64_x86_64_local_tlsdesc_gotent (input_bfd);
 
+  set_tls_module_base (info);
+
   rel = relocs;
   relend = relocs + input_section->reloc_count;
   for (; rel < relend; rel++)
Index: ld/testsuite/ld-i386/tlsbindesc.dd
===================================================================
--- ld/testsuite/ld-i386/tlsbindesc.dd.orig	2007-05-07 02:37:00.000000000 -0300
+++ ld/testsuite/ld-i386/tlsbindesc.dd	2008-07-04 02:53:45.000000000 -0300
@@ -90,7 +90,7 @@ Disassembly of section .text:
  [0-9a-f]+:	90[ 	]+nop *
  [0-9a-f]+:	90[ 	]+nop *
 #  LD -> LE
- [0-9a-f]+:	8d 05 00 f0 ff ff[ 	]+lea    0xfffff000,%eax
+ [0-9a-f]+:	8d 05 00 00 00 00[ 	]+lea    0x0,%eax
  [0-9a-f]+:	66 90[ 	]+xchg   %ax,%ax
  [0-9a-f]+:	90[ 	]+nop *
  [0-9a-f]+:	90[ 	]+nop *
@@ -105,7 +105,7 @@ Disassembly of section .text:
  [0-9a-f]+:	90[ 	]+nop *
  [0-9a-f]+:	90[ 	]+nop *
 #  LD -> LE against hidden variables
- [0-9a-f]+:	8d 05 00 f0 ff ff[ 	]+lea    0xfffff000,%eax
+ [0-9a-f]+:	8d 05 00 00 00 00[ 	]+lea    0x0,%eax
  [0-9a-f]+:	66 90[ 	]+xchg   %ax,%ax
  [0-9a-f]+:	90[ 	]+nop *
  [0-9a-f]+:	90[ 	]+nop *
Index: ld/testsuite/ld-x86-64/tlsbindesc.dd
===================================================================
--- ld/testsuite/ld-x86-64/tlsbindesc.dd.orig	2007-05-07 02:37:00.000000000 -0300
+++ ld/testsuite/ld-x86-64/tlsbindesc.dd	2008-07-04 03:05:29.000000000 -0300
@@ -63,7 +63,7 @@ Disassembly of section .text:
   [0-9a-f]+:	90[ 	]+nop *
   [0-9a-f]+:	90[ 	]+nop *
 #  LD -> LE
-  [0-9a-f]+:	48 c7 c0 60 ff ff ff[ 	]+mov    \$0xf+60,%rax
+  [0-9a-f]+:	48 c7 c0 00 00 00 00[ 	]+mov    \$0x0,%rax
   [0-9a-f]+:	66 90[ 	]+xchg   %ax,%ax
   [0-9a-f]+:	90[ 	]+nop *
   [0-9a-f]+:	90[ 	]+nop *
Index: ld/testsuite/ld-i386/tlsbindesc.rd
===================================================================
--- ld/testsuite/ld-i386/tlsbindesc.rd.orig	2007-10-05 14:50:27.000000000 -0300
+++ ld/testsuite/ld-i386/tlsbindesc.rd	2008-07-04 03:04:45.000000000 -0300
@@ -109,7 +109,7 @@ Symbol table '\.symtab' contains [0-9]+ 
  +[0-9]+: 00000094 +0 TLS +LOCAL  DEFAULT +8 bl6
  +[0-9]+: 00000098 +0 TLS +LOCAL  DEFAULT +8 bl7
  +[0-9]+: 0000009c +0 TLS +LOCAL  DEFAULT +8 bl8
- +[0-9]+: 00000000 +0 TLS +LOCAL  HIDDEN +7 _TLS_MODULE_BASE_
+ +[0-9]+: 00001000 +0 TLS +LOCAL  HIDDEN +7 _TLS_MODULE_BASE_
  +[0-9]+: 0+804a060 +0 OBJECT  LOCAL  HIDDEN    9 _DYNAMIC
  +[0-9]+: [0-9a-f]+ +0 OBJECT  LOCAL  HIDDEN   11 _GLOBAL_OFFSET_TABLE_
  +[0-9]+: 0+ +0 TLS +GLOBAL DEFAULT  UND sG3
Index: ld/testsuite/ld-x86-64/tlsbindesc.rd
===================================================================
--- ld/testsuite/ld-x86-64/tlsbindesc.rd.orig	2007-10-05 14:50:27.000000000 -0300
+++ ld/testsuite/ld-x86-64/tlsbindesc.rd	2008-07-04 03:06:21.000000000 -0300
@@ -100,7 +100,7 @@ Symbol table '\.symtab' contains [0-9]+ 
  +[0-9]+: 0+94 +0 TLS +LOCAL  DEFAULT +8 bl6
  +[0-9]+: 0+98 +0 TLS +LOCAL  DEFAULT +8 bl7
  +[0-9]+: 0+9c +0 TLS +LOCAL  DEFAULT +8 bl8
- +[0-9]+: 0+0 +0 TLS +LOCAL  HIDDEN +7 _TLS_MODULE_BASE_
+ +[0-9]+: 0+a0 +0 TLS +LOCAL  HIDDEN +7 _TLS_MODULE_BASE_
  +[0-9]+: 0+601258 +0 OBJECT  LOCAL  HIDDEN    9 _DYNAMIC
  +[0-9]+: 0+601378 +0 OBJECT  LOCAL  HIDDEN   11 _GLOBAL_OFFSET_TABLE_
  +[0-9]+: 0+1c +0 TLS +GLOBAL DEFAULT +7 sg8

-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}
FSFLA Board Member       ÂSÃ Libre! => http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}