This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Prevent duplicate dynamic relocs for MIPS TLS


This patch follows on from (and subsumes):

    http://sources.redhat.com/ml/binutils/2006-02/msg00063.html

To recap, the problem was that two input bfds had GOT relocations for
the same forced-local TLS symbol, and only one bfd was using the correct
GOT index.  This was caused by confusion between mips_elf_local_got_index
and mips_elf_initialize_tls_slots.  Each input bfd has its own
mips_got_entry for the symbol, and mips_elf_local_got_index uses
mips_got_entry's gotidx field to track GOT indexes.  However,
mips_elf_initialize_tls_slots only initialised the gotidx
of the first mips_got_entry.

In hindsight, the patch above was only papering over the problem.
We now get the indexes right, but mips_elf_local_got_index also
uses the mips_got_entry to tell whether a TLS slot has already
been initialised.  It will therefore initialise the slot twice
and create to two sets of relocations for it.  This can be seen
as an assertion failure in the tls-hidden3 testcase below.

I think the real bug was in mips_elf_local_got_index itself.
When there's only a single GOT, it should use the hash table entry,
not the mips_got_entry, to track the index of a forced-local symbol.
(Note that mips_elf_global_got_index already makes this distinction
for global symbols.)  This patch makes it do that, and consequently
fixes the double initialisation described above.

While there, I added some (IMO) much-needed commentary above
mips_got_entry to describe the different types of entry and
how we intend to track their GOT indexes.  I also changed
mips_elf_initialize_tls_index to (a) make it correlate more
obviously with the new commentary and with the mips_*_got_index
functions and (b) avoid setting gotidx in cases where it should
not be used.

Also, I noticed a bug in the way mips_elf_initialize_tls_index and
mips_elf_multi_got communicate.  mips_elf_initialize_tls_index uses
"g->next == NULL" to check for a single GOT, but mips_elf_multi_got
calls mips_elf_initialize_tls_index for a GOT before inserting it in
the circular list.  This means that g->next will be NULL for the last
secondary GOT as well for a single GOT.  I've fixed that by inserting
the GOT into the circular list first.

I checked other uses of mips_got_entry structures, and as far as I can
tell, they seem to be making the right choice between mips_got_entry and
symbol entry fields.

Tested on mips64-linux-gnu.  OK to install?

Richard


	* elfxx-mips.c (mips_got_entry): Add more commentary.
	(mips_elf_local_got_index): Use the hash table entry to record
	the GOT index of forced-local symbols.
	(mips_elf_initialize_tls_index): Rearrange code.  Store the index
	in either the hash table entry or the mips_got_entry, not both.
	Add more commentary.
	(mips_elf_multi_got): Make sure the g->next is nonnull when calling
	mips_elf_initialize_tls_index.

	* ld-mips-elf/tls-hidden3a.s, ld-mips-elf/tls-hidden3b.s,
	* ld-mips-elf/tls-hidden3.d, ld-mips-elf/tls-hidden3.got,
	* ld-mips-elf/tls-hidden3.ld, ld-mips-elf/tls-hidden3.r,
	* ld-mips-elf/tls-hidden4a.s, ld-mips-elf/tls-hidden4b.s,
	* ld-mips-elf/tls-hidden4.got, ld-mips-elf/tls-hidden4.r: New tests.
	* ld-mips-elf/mips-elf.exp: Run them.

Index: bfd/elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.164
diff -c -p -r1.164 elfxx-mips.c
*** bfd/elfxx-mips.c	22 Mar 2006 17:15:08 -0000	1.164
--- bfd/elfxx-mips.c	26 Mar 2006 10:23:46 -0000
***************
*** 44,51 ****
  
  #include "hashtab.h"
  
! /* This structure is used to hold .got entries while estimating got
!    sizes.  */
  struct mips_got_entry
  {
    /* The input bfd in which the symbol is defined.  */
--- 44,82 ----
  
  #include "hashtab.h"
  
! /* This structure is used to hold information about one GOT entry.
!    There are three types of entry:
! 
!       (1) absolute addresses
! 	    (abfd == NULL)
!       (2) SYMBOL + OFFSET addresses, where SYMBOL is local to an input bfd
! 	    (abfd != NULL, symndx >= 0)
!       (3) global and forced-local symbols
! 	    (abfd != NULL, symndx == -1)
! 
!    Type (3) entries are treated differently for different types of GOT.
!    In the "master" GOT -- i.e.  the one that describes every GOT
!    reference needed in the link -- the mips_got_entry is keyed on both
!    the symbol and the input bfd that references it.  If it turns out
!    that we need multiple GOTs, we can then use this information to
!    create separate GOTs for each input bfd.
! 
!    However, we want each of these separate GOTs to have at most one
!    entry for a given symbol, so their type (3) entries are keyed only
!    on the symbol.  The input bfd given by the "abfd" field is somewhat
!    arbitrary in this case.
! 
!    This means that when there are multiple GOTs, each GOT has a unique
!    mips_got_entry for every symbol within it.  We can therefore use the
!    mips_got_entry fields (tls_type and gotidx) to track the symbol's
!    GOT index.
! 
!    However, if it turns out that we need only a single GOT, we continue
!    to use the master GOT to describe it.  There may therefore be several
!    mips_got_entries for the same symbol, each with a different input bfd.
!    We want to make sure that each symbol gets a unique GOT entry, so when
!    there's a single GOT, we use the symbol's hash entry, not the
!    mips_got_entry fields, to track a symbol's GOT index.  */
  struct mips_got_entry
  {
    /* The input bfd in which the symbol is defined.  */
*************** mips_elf_local_got_index (bfd *abfd, bfd
*** 2371,2378 ****
      return MINUS_ONE;
  
    if (TLS_RELOC_P (r_type))
!     return mips_tls_got_index (abfd, entry->gotidx, &entry->tls_type, r_type,
! 			       info, h, value);
    else
      return entry->gotidx;
  }
--- 2402,2417 ----
      return MINUS_ONE;
  
    if (TLS_RELOC_P (r_type))
!     {
!       if (entry->symndx == -1 && g->next == NULL)
! 	/* A type (3) entry in the single-GOT case.  We use the symbol's
! 	   hash table entry to track the index.  */
! 	return mips_tls_got_index (abfd, h->tls_got_offset, &h->tls_type,
! 				   r_type, info, h, value);
!       else
! 	return mips_tls_got_index (abfd, entry->gotidx, &entry->tls_type,
! 				   r_type, info, h, value);
!     }
    else
      return entry->gotidx;
  }
*************** mips_elf_merge_gots (void **bfd2got_, vo
*** 3118,3172 ****
    return 1;
  }
  
! /* Set the TLS GOT index for the GOT entry in ENTRYP.  */
  
  static int
  mips_elf_initialize_tls_index (void **entryp, void *p)
  {
    struct mips_got_entry *entry = (struct mips_got_entry *)*entryp;
    struct mips_got_info *g = p;
  
    /* We're only interested in TLS symbols.  */
    if (entry->tls_type == 0)
      return 1;
  
!   if (entry->symndx == -1)
      {
!       /* There may be multiple mips_got_entry structs for a global variable
! 	 if there is just one GOT.  Just do this once.  */
!       if (g->next == NULL)
  	{
! 	  if (entry->d.h->tls_type & GOT_TLS_OFFSET_DONE)
  	    {
! 	      entry->gotidx = entry->d.h->tls_got_offset;
  	      return 1;
  	    }
! 	  entry->d.h->tls_type |= GOT_TLS_OFFSET_DONE;
  	}
      }
-   else if (entry->tls_type & GOT_TLS_LDM)
-     {
-       /* Similarly, there may be multiple structs for the LDM entry.  */
-       if (g->tls_ldm_offset != MINUS_TWO && g->tls_ldm_offset != MINUS_ONE)
- 	{
- 	  entry->gotidx = g->tls_ldm_offset;
- 	  return 1;
- 	}
-     }
- 
-   /* Initialize the GOT offset.  */
-   entry->gotidx = MIPS_ELF_GOT_SIZE (entry->abfd) * (long) g->tls_assigned_gotno;
-   if (g->next == NULL && entry->symndx == -1)
-     entry->d.h->tls_got_offset = entry->gotidx;
  
    if (entry->tls_type & (GOT_TLS_GD | GOT_TLS_LDM))
      g->tls_assigned_gotno += 2;
    if (entry->tls_type & GOT_TLS_IE)
      g->tls_assigned_gotno += 1;
  
-   if (entry->tls_type & GOT_TLS_LDM)
-     g->tls_ldm_offset = entry->gotidx;
- 
    return 1;
  }
  
--- 3157,3210 ----
    return 1;
  }
  
! /* Set the TLS GOT index for the GOT entry in ENTRYP.  ENTRYP's NEXT field
!    is null iff there is just a single GOT.  */
  
  static int
  mips_elf_initialize_tls_index (void **entryp, void *p)
  {
    struct mips_got_entry *entry = (struct mips_got_entry *)*entryp;
    struct mips_got_info *g = p;
+   bfd_vma next_index;
  
    /* We're only interested in TLS symbols.  */
    if (entry->tls_type == 0)
      return 1;
  
!   next_index = MIPS_ELF_GOT_SIZE (entry->abfd) * (long) g->tls_assigned_gotno;
! 
!   if (entry->symndx == -1 && g->next == NULL)
      {
!       /* A type (3) got entry in the single-GOT case.  We use the symbol's
! 	 hash table entry to track its index.  */
!       if (entry->d.h->tls_type & GOT_TLS_OFFSET_DONE)
! 	return 1;
!       entry->d.h->tls_type |= GOT_TLS_OFFSET_DONE;
!       entry->d.h->tls_got_offset = next_index;
!     }
!   else
!     {
!       if (entry->tls_type & GOT_TLS_LDM)
  	{
! 	  /* There are separate mips_got_entry objects for each input bfd
! 	     that requires an LDM entry.  Make sure that all LDM entries in
! 	     a GOT resolve to the same index.  */
! 	  if (g->tls_ldm_offset != MINUS_TWO && g->tls_ldm_offset != MINUS_ONE)
  	    {
! 	      entry->gotidx = g->tls_ldm_offset;
  	      return 1;
  	    }
! 	  g->tls_ldm_offset = next_index;
  	}
+       entry->gotidx = next_index;
      }
  
+   /* Account for the entries we've just allocated.  */
    if (entry->tls_type & (GOT_TLS_GD | GOT_TLS_LDM))
      g->tls_assigned_gotno += 2;
    if (entry->tls_type & GOT_TLS_IE)
      g->tls_assigned_gotno += 1;
  
    return 1;
  }
  
*************** mips_elf_multi_got (bfd *abfd, struct bf
*** 3492,3507 ****
        g->local_gotno += assign + pages;
        assign = g->local_gotno + g->global_gotno + g->tls_gotno;
  
        /* Set up any TLS entries.  We always place the TLS entries after
  	 all non-TLS entries.  */
        g->tls_assigned_gotno = g->local_gotno + g->global_gotno;
        htab_traverse (g->got_entries, mips_elf_initialize_tls_index, g);
  
!       /* Take g out of the direct list, and push it onto the reversed
! 	 list that gg points to.  */
!       gn = g->next;
!       g->next = gg->next;
!       gg->next = g;
        g = gn;
  
        /* Mark global symbols in every non-primary GOT as ineligible for
--- 3530,3548 ----
        g->local_gotno += assign + pages;
        assign = g->local_gotno + g->global_gotno + g->tls_gotno;
  
+       /* Take g out of the direct list, and push it onto the reversed
+ 	 list that gg points to.  g->next is guaranteed to be nonnull after
+ 	 this operation, as required by mips_elf_initialize_tls_index. */
+       gn = g->next;
+       g->next = gg->next;
+       gg->next = g;
+ 
        /* Set up any TLS entries.  We always place the TLS entries after
  	 all non-TLS entries.  */
        g->tls_assigned_gotno = g->local_gotno + g->global_gotno;
        htab_traverse (g->got_entries, mips_elf_initialize_tls_index, g);
  
!       /* Move onto the next GOT.  It will be a secondary GOT if nonull.  */
        g = gn;
  
        /* Mark global symbols in every non-primary GOT as ineligible for
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden3a.s	2006-03-26 11:10:46.000000000 +0100
***************
*** 0 ****
--- 1,10 ----
+ 	.macro	load
+ 	lw	$4,%gottprel(foo\@)($gp)
+ 	.endm
+ 
+ 	.rept	4
+ 	load
+ 	.endr
+ 
+ 	.section .tdata,"awT",@progbits
+ 	.fill	0xabc0
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden3b.s	2006-03-26 11:11:39.000000000 +0100
***************
*** 0 ****
--- 1,18 ----
+ 	.macro	load
+ 	.text
+ 	lw	$4,%gottprel(foo\@)($gp)
+ 
+ 	.global foo\@
+ 	.type   foo\@,@object
+ 	.size   foo\@,4
+ 	.section .tdata,"awT",@progbits
+ foo\@:
+ 	.word   \@
+ 	.endm
+ 
+ 	.rept	4
+ 	load
+ 	.endr
+ 
+ 	.data
+ 	.word	undef
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden3.d	2006-03-27 09:03:58.000000000 +0100
***************
*** 0 ****
--- 1,24 ----
+ 
+ .*:     file format .*
+ 
+ Disassembly of section \.text:
+ 
+ #
+ # The TLS entries are ordered as follows:
+ #
+ #	foo0	(-0x7ff0 + 0x20)
+ #	foo2	(-0x7ff0 + 0x24)
+ #	foo3	(-0x7ff0 + 0x28)
+ #	foo1	(-0x7ff0 + 0x2c)
+ #
+ # Any order would be acceptable, but it must match the .got dump.
+ #
+ 00080c00 <\.text>:
+    80c00:	8f848030 	lw	a0,-32720\(gp\)
+    80c04:	8f84803c 	lw	a0,-32708\(gp\)
+    80c08:	8f848034 	lw	a0,-32716\(gp\)
+    80c0c:	8f848038 	lw	a0,-32712\(gp\)
+    80c10:	8f848030 	lw	a0,-32720\(gp\)
+    80c14:	8f84803c 	lw	a0,-32708\(gp\)
+    80c18:	8f848034 	lw	a0,-32716\(gp\)
+    80c1c:	8f848038 	lw	a0,-32712\(gp\)
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden3.got	2006-03-27 09:05:07.000000000 +0100
***************
*** 0 ****
--- 1,24 ----
+ 
+ .*:     file format .*
+ 
+ #
+ # The GOT layout is:
+ #
+ #	- 2 reserved entries
+ #	- 5 local page entries
+ #	- 1 global entry for "undef"
+ #	- 4 TLS entries
+ #
+ # The order of the TLS entries is:
+ #
+ #	foo0	(offset 0x20)
+ #	foo2	(offset 0x24)
+ #	foo3	(offset 0x28)
+ #	foo1	(offset 0x2c)
+ #
+ # Any order would be acceptable, but it must match the .d dump.
+ #
+ Contents of section \.got:
+  90000 00000000 80000000 00000000 00000000  .*
+  90010 00000000 00000000 00000000 00000000  .*
+  90020 0000abc0 0000abc8 0000abcc 0000abc4  .*
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden3.ld	2006-03-26 10:16:35.000000000 +0100
***************
*** 0 ****
--- 1,31 ----
+ SECTIONS
+ {
+   . = 0x80000;
+   .interp : { *(.interp) }
+   .hash : { *(.hash) }
+   .dynsym : { *(.dynsym) }
+   .dynstr : { *(.dynstr) }
+ 
+   . = ALIGN (0x400);
+   .rel.dyn : { *(.rel.dyn) }
+ 
+   . = ALIGN (0x400);
+   .MIPS.stubs : { *(.MIPS.stubs) }
+ 
+   . = ALIGN (0x400);
+   .text : { *(.text) }
+ 
+   . = ALIGN (0x10000);
+   _gp = . + 0x7ff0;
+   .got : { *(.got) }
+ 
+   . = ALIGN (0x400);
+   .tdata : { *(.tdata) }
+ 
+   /DISCARD/ : { *(.reginfo) }
+ }
+ 
+ VERSION
+ {
+   { local: *; };
+ }
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden3.r	2006-03-26 11:14:53.000000000 +0100
***************
*** 0 ****
--- 1,13 ----
+ 
+ Relocation section '\.rel\.dyn' at offset .* contains 6 entries:
+  Offset     Info    Type            Sym\.Value  Sym\. Name
+ 00000000  00000000 R_MIPS_NONE      
+ #
+ # The order of the next four entries doesn't matter.  The important thing
+ # is that there is exactly one entry per GOT TLS slot.
+ #
+ 00090020  0000002f R_MIPS_TLS_TPREL3
+ 0009002c  0000002f R_MIPS_TLS_TPREL3
+ 00090024  0000002f R_MIPS_TLS_TPREL3
+ 00090028  0000002f R_MIPS_TLS_TPREL3
+ 00090030  .*03 R_MIPS_REL32      00000000   undef
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden4a.s	2006-03-26 11:11:21.000000000 +0100
***************
*** 0 ****
--- 1,18 ----
+ 	.macro	load
+ 	lw	$4,%gottprel(foo\@)($gp)
+ 	.endm
+ 
+ 	.rept	4
+ 	load
+ 	.endr
+ 
+ 	.macro	load2
+ 	lw	$4,%got(undefa\@)($gp)
+ 	.endm
+ 
+ 	.rept	0x3000
+ 	load2
+ 	.endr
+ 
+ 	.section .tdata,"awT",@progbits
+ 	.fill	0xabc0
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden4b.s	2006-03-26 11:11:47.000000000 +0100
***************
*** 0 ****
--- 1,27 ----
+ 	.macro	load
+ 	.text
+ 	lw	$4,%gottprel(foo\@)($gp)
+ 
+ 	.global foo\@
+ 	.type   foo\@,@object
+ 	.size   foo\@,4
+ 	.section .tdata,"awT",@progbits
+ foo\@:
+ 	.word   \@
+ 	.endm
+ 
+ 	.rept	4
+ 	load
+ 	.endr
+ 
+ 	.text
+ 	.macro	load2
+ 	lw	$4,%got(undefb\@)($gp)
+ 	.endm
+ 
+ 	.rept	0x3000
+ 	load2
+ 	.endr
+ 
+ 	.data
+ 	.word	undef
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden4.got	2006-03-26 11:17:34.000000000 +0100
***************
*** 0 ****
--- 1,28 ----
+ 
+ .*:     file format .*
+ 
+ Contents of section \.got:
+ #
+ # The order of the TLS entries in this GOT is:
+ #
+ #     foo2
+ #     foo3
+ #     foo0
+ #     foo1
+ #
+ # The order and address don't matter; the important thing is that the
+ # addresses match the relocs in the .r dump and that there is a separate
+ # entry for each symbol.
+ #
+ #...
+  1c4080 0000abc8 0000abcc 0000abc0 0000abc4  .*
+ #
+ # Likewise, but the order of the entries in this GOT is:
+ #
+ #     foo3
+ #     foo2
+ #     foo0
+ #     foo1
+ #...
+  1d00c0 00000000 00000000 00000000 0000abcc  .*
+  1d00d0 0000abc8 0000abc0 0000abc4           .*
*** /dev/null	2005-06-16 22:49:09.000000000 +0100
--- ld/testsuite/ld-mips-elf/tls-hidden4.r	2006-03-26 11:17:00.000000000 +0100
***************
*** 0 ****
--- 1,19 ----
+ 
+ Relocation section '\.rel\.dyn' at offset .* contains .* entries:
+  Offset     Info    Type            Sym\.Value  Sym\. Name
+ 00000000  00000000 R_MIPS_NONE      
+ #
+ # The order and addresses of the next eight entries don't matter.  The
+ # important thing is that there is exactly one entry per GOT TLS slot
+ # and that the addresses match those in the .got dump.
+ #
+ 001d00d4  0000002f R_MIPS_TLS_TPREL3
+ 001d00d8  0000002f R_MIPS_TLS_TPREL3
+ 001d00d0  0000002f R_MIPS_TLS_TPREL3
+ 001d00cc  0000002f R_MIPS_TLS_TPREL3
+ 001c4088  0000002f R_MIPS_TLS_TPREL3
+ 001c408c  0000002f R_MIPS_TLS_TPREL3
+ 001c4080  0000002f R_MIPS_TLS_TPREL3
+ 001c4084  0000002f R_MIPS_TLS_TPREL3
+ .* R_MIPS_REL32 .*
+ #pass
Index: ld/testsuite/ld-mips-elf/mips-elf.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/mips-elf.exp,v
retrieving revision 1.34
diff -c -p -r1.34 mips-elf.exp
*** ld/testsuite/ld-mips-elf/mips-elf.exp	22 Mar 2006 09:28:14 -0000	1.34
--- ld/testsuite/ld-mips-elf/mips-elf.exp	27 Mar 2006 08:19:18 -0000
*************** set mips_tls_tests {
*** 199,204 ****
--- 199,215 ----
       "-EB -march=mips1 -32 -KPIC" {tls-hidden2a.s tls-hidden2b.s}
       {{objdump -drj.text tls-hidden2.d} {objdump -sj.got tls-hidden2-got.d}}
       "tls-hidden2.so"}
+     {"Shared library with TLS and hidden symbols (3)"
+      "-shared -melf32btsmip -T tls-hidden3.ld"
+      "-EB -march=mips2 -32 -KPIC" {tls-hidden3a.s tls-hidden3b.s}
+      {{objdump -dj.text tls-hidden3.d} {objdump -sj.got tls-hidden3.got}
+       {readelf --relocs tls-hidden3.r}}
+      "tls-hidden3.so"}
+     {"Shared library with TLS and hidden symbols (4)"
+      "-shared -melf32btsmip -T tls-hidden3.ld"
+      "-EB -march=mips2 -32 -KPIC" {tls-hidden4a.s tls-hidden4b.s}
+      {{objdump -sj.got tls-hidden4.got} {readelf --relocs tls-hidden4.r}}
+      "tls-hidden4.so"}
  }
  
  if {[istarget mips*-*-linux*]} {


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]