This is the mail archive of the binutils@sources.redhat.com 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]

Committed, CRIS: Bogosity of merging GLOB_DATs with JUMP_SLOTs revisited, fixed in binutils.


Finally film-time for a proper fix.  This corrects the
CRIS-specific linker optimization that aims to avoid creating
two run-time relocs and different GOT entries for a symbol whose
address is both taken and called directly (instead merging GOT
entries and have a single reloc).  That can be done in a DSO,
but must not happen in an executable when the symbol is defined
in a DSO; the "visible" value (that of the PLT entry in the
executable) must be different from the value that the PLT uses.
On the other hand, in the executable we don't need a reloc for a
GOT entry for the address of the PLT.  The bug was in
overlooking the necessity of two values for the executable; the
dynamic value was used.  Fortunately, this happens only with
code compiled with -fpic/PIC.  For more, see
<URL:http://sources.redhat.com/ml/binutils/2003-10/msg00149.html>.

The glibc workaround should be reverted, but more substantial
changes and an increased level of interaction with glibc
maintainers are needed to get CVS glibc in a working state for
cris-axis-linux-gnu, so I pass on that until I can dedicate the
required time for it.  I checked this into binutils anyway.

Also committed on the 2.15 branch.

bfd:
	* elf32-cris.c (cris_elf_relocate_section) <case R_CRIS_16_GOTPLT,
	R_CRIS_16_GOTPLT>: Also error if there's no PLT for a symbol
	not defined by the executable, or defined in a DSO.
	<eliding run-time relocation of .got>: Initialize GOT entry for a
	function symbol or ELF_LINK_HASH_NEEDS_PLT statically in an
	executable.
	(cris_elf_gc_sweep_hook): Improve fallthrough marking.
	(elf_cris_try_fold_plt_to_got): Improve head comment.  Do not fold
	a PLT reloc to GOT for an executable.
	(elf_cris_adjust_dynamic_symbol): Only fold a .got.plt entry with
	.got for a DSO and explain why. 
	(elf_cris_discard_excess_program_dynamics): Also lose GOT-relocs
	and unreferenced symbols for which a PLT is defined.  Adjust
	dynamic-symbol pruning correspondingly, to make sure we don't lose
	a dynamic symbol also defined by a DSO.

ld/testsuite:
	* ld-cris/dsofnf.s, ld-cris/dsofnf2.s, ld-cris/gotplt1.d,
	ld-cris/gotplt2.d, ld-cris/gotplt3.d: New tests.

ld/testsuite/ld-cris/dsofnf.s::
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ ld/testsuite/ld-cris/dsofnf.s	Mon Mar 22 01:33:11 2004
@@ -0,0 +1,7 @@
+ .text
+ .global f
+ .type f,@function
+f:
+ move.d [$r0+dsofn:GOT],$r1
+0:
+ .size f,0b-f
ld/testsuite/ld-cris/dsofnf2.s::
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ ld/testsuite/ld-cris/dsofnf2.s	Mon Mar 22 01:33:11 2004
@@ -0,0 +1,8 @@
+ .text
+ .global f
+ .type f,@function
+f:
+ move.d [$r0+dsofn:GOT],$r1
+ move.d dsofn,$r2
+0:
+ .size f,0b-f
ld/testsuite/ld-cris/gotplt1.d::
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ ld/testsuite/ld-cris/gotplt1.d	Mon Mar 22 03:17:18 2004
@@ -0,0 +1,48 @@
+#source: dso-2.s
+#source: dsofnf2.s
+#source: gotrel1.s
+#as: --pic --no-underscore
+#ld: -m crislinux tmpdir/libdso-1.so
+#objdump: -sR
+
+# Make sure we don't merge a PLT-specific entry
+# (R_CRIS_JUMP_SLOT) with a non-PLT-GOT-specific entry
+# (R_CRIS_GLOB_DAT) in an executable, since they may have
+# different contents there.  (If we merge them in a DSO it's ok:
+# we make a round-trip to the PLT in the executable if it's
+# referenced there, but that's still perceived as better than
+# having an unnecessary PLT, dynamic reloc and lookup in the
+# DSO.)  In the executable, the GOT contents for the non-PLT
+# reloc should be constant.
+
+.*:     file format elf32-cris
+
+DYNAMIC RELOCATION RECORDS
+OFFSET   TYPE              VALUE 
+000822d4 R_CRIS_JUMP_SLOT  dsofn
+
+Contents of section .*
+#...
+Contents of section \.rela\.plt:
+ 801d8 d4220800 0b060000 00000000           .*
+Contents of section \.plt:
+ 801e4 fce17e7e 7f0dcc22 0800307a 7f0dd022  .*
+ 801f4 08003009 7f0dd422 08003009 3f7e0000  .*
+ 80204 00002ffe d8ffffff                    .*
+Contents of section \.text:
+ 8020c 5f1d0c00 30096f1d 0c000000 30090000  .*
+ 8021c 6f0d1000 0000611a 6f2ef801 08000000  .*
+ 8022c 6f3e64df ffff0000                    .*
+Contents of section \.dynamic:
+ 82240 01000000 01000000 04000000 e4000800  .*
+ 82250 05000000 84010800 06000000 14010800  .*
+ 82260 0a000000 51000000 0b000000 10000000  .*
+ 82270 15000000 00000000 03000000 c8220800  .*
+ 82280 02000000 0c000000 14000000 07000000  .*
+ 82290 17000000 d8010800 00000000 00000000  .*
+ 822a0 00000000 00000000 00000000 00000000  .*
+ 822b0 00000000 00000000 00000000 00000000  .*
+ 822c0 00000000 00000000                    .*
+Contents of section \.got:
+ 822c8 40220800 00000000 00000000 00020800  .*
+ 822d8 f8010800                             .*
ld/testsuite/ld-cris/gotplt2.d::
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ ld/testsuite/ld-cris/gotplt2.d	Mon Mar 22 03:20:49 2004
@@ -0,0 +1,37 @@
+#source: dso-2.s
+#source: dsofnf.s
+#source: gotrel1.s
+#as: --pic --no-underscore
+#ld: -shared -m crislinux -z nocombreloc
+#objdump: -sR
+
+# Make sure we merge a PLT-specific entry (usually
+# R_CRIS_JUMP_SLOT) with a GOT-specific entry (R_CRIS_GLOB_DAT)
+# in a DSO.  It's ok: we make a round-trip to the PLT in the
+# executable if it's referenced there, but that's still
+# perceived as better than having an unnecessary PLT, dynamic
+# reloc and lookup in the DSO.)
+
+.*:     file format elf32-cris
+
+DYNAMIC RELOCATION RECORDS
+OFFSET   TYPE              VALUE 
+0000235c R_CRIS_GLOB_DAT   dsofn
+
+Contents of section .*
+#...
+Contents of section \.rela\.got:
+ 02a8 5c230000 0a120000 00000000           .*
+Contents of section \.text:
+ 02b4 5f1d0c00 30096f1d 0c000000 30090000  .*
+ 02c4 6f0d0c00 0000611a 6f3e7cdf ffff0000  .*
+Contents of section \.dynamic:
+ 22e0 04000000 94000000 05000000 5c020000  .*
+ 22f0 06000000 2c010000 0a000000 49000000  .*
+ 2300 0b000000 10000000 07000000 a8020000  .*
+ 2310 08000000 0c000000 09000000 0c000000  .*
+ 2320 00000000 00000000 00000000 00000000  .*
+ 2330 00000000 00000000 00000000 00000000  .*
+ 2340 00000000 00000000 00000000 00000000  .*
+Contents of section \.got:
+ 2350 e0220000 00000000 00000000 00000000  .*
ld/testsuite/ld-cris/gotplt3.d::
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ ld/testsuite/ld-cris/gotplt3.d	Mon Mar 22 03:21:28 2004
@@ -0,0 +1,35 @@
+#source: dso-2.s
+#source: dsofnf.s
+#source: gotrel1.s
+#source: dso-1.s
+#as: --pic --no-underscore
+#ld: -shared -m crislinux -z nocombreloc
+#objdump: -sR
+
+# Like gotplt2, but make sure we merge right when we have a
+# definition of the function too.
+
+.*:     file format elf32-cris
+
+DYNAMIC RELOCATION RECORDS
+OFFSET   TYPE              VALUE 
+0000235c R_CRIS_GLOB_DAT   dsofn
+
+Contents of section .*
+#...
+Contents of section \.rela\.got:
+ 02a8 5c230000 0a120000 00000000           .*
+Contents of section \.text:
+ 02b4 5f1d0c00 30096f1d 0c000000 30090000  .*
+ 02c4 6f0d0c00 0000611a 6f3e7cdf ffff0000  .*
+ 02d4 0f050000                             .*
+Contents of section \.dynamic:
+ 22e0 04000000 94000000 05000000 5c020000  .*
+ 22f0 06000000 2c010000 0a000000 49000000  .*
+ 2300 0b000000 10000000 07000000 a8020000  .*
+ 2310 08000000 0c000000 09000000 0c000000  .*
+ 2320 00000000 00000000 00000000 00000000  .*
+ 2330 00000000 00000000 00000000 00000000  .*
+ 2340 00000000 00000000 00000000 00000000  .*
+Contents of section \.got:
+ 2350 e0220000 00000000 00000000 00000000  .*
Index: bfd/elf32-cris.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-cris.c,v
retrieving revision 1.48
diff -c -p -r1.48 elf32-cris.c
*** bfd/elf32-cris.c	22 Mar 2004 02:28:15 -0000	1.48
--- bfd/elf32-cris.c	22 Mar 2004 02:36:11 -0000
*************** cris_elf_relocate_section (output_bfd, i
*** 960,970 ****
  	     statically linking PIC code, or when using -Bsymbolic.  Check
  	     that we instead have a GOT entry as done for us by
  	     elf_cris_adjust_dynamic_symbol, and drop through into the
! 	     ordinary GOT cases.  */
! 	  if (h != NULL && h->got.offset == (bfd_vma) -1)
  	    {
  	      (*_bfd_error_handler)
! 		(_("%s: No PLT nor GOT for relocation %s against symbol `%s' from %s section"),
  		 bfd_archive_filename (input_bfd),
  		 cris_elf_howto_table[r_type].name,
  		 symname[0] != '\0' ? symname : _("[whose name is lost]"),
--- 960,984 ----
  	     statically linking PIC code, or when using -Bsymbolic.  Check
  	     that we instead have a GOT entry as done for us by
  	     elf_cris_adjust_dynamic_symbol, and drop through into the
! 	     ordinary GOT cases.  This must not happen for the
! 	     executable, because any reference it does to a function
! 	     that is satisfied by a DSO must generate a PLT.  We assume
! 	     these call-specific relocs don't address non-functions.  */
! 	  if (h != NULL
! 	      && (h->got.offset == (bfd_vma) -1
! 		  || (!info->shared
! 		      && !((h->elf_link_hash_flags
! 			    & ELF_LINK_HASH_DEF_REGULAR) != 0
! 			   || ((h->elf_link_hash_flags
! 				& ELF_LINK_HASH_DEF_DYNAMIC) == 0
! 			      && h->root.type == bfd_link_hash_undefweak)))))
  	    {
  	      (*_bfd_error_handler)
! 		((h->got.offset == (bfd_vma) -1)
! 		 ? _("%s: No PLT nor GOT for relocation %s against\
!  symbol `%s' from %s section")
! 		 : _("%s: No PLT for relocation %s against\
!  symbol `%s' from %s section"),
  		 bfd_archive_filename (input_bfd),
  		 cris_elf_howto_table[r_type].name,
  		 symname[0] != '\0' ? symname : _("[whose name is lost]"),
*************** cris_elf_relocate_section (output_bfd, i
*** 996,1013 ****
  
  		if (!elf_hash_table (info)->dynamic_sections_created
  		    || (! info->shared
! 			&& (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR))
  		    || (info->shared
  			&& (info->symbolic || h->dynindx == -1)
  			&& (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR)))
  		  {
  		    /* This wasn't checked above for ! info->shared, but
! 		       must hold there if we get here; the symbol must be
! 		       defined in the regular program, or be undefweak.  */
  		    BFD_ASSERT (!elf_hash_table (info)->dynamic_sections_created
  				|| info->shared
  				|| (h->elf_link_hash_flags
  				    & ELF_LINK_HASH_DEF_REGULAR) != 0
  				|| h->root.type == bfd_link_hash_undefweak);
  
  		    /* This is actually a static link, or it is a
--- 1010,1034 ----
  
  		if (!elf_hash_table (info)->dynamic_sections_created
  		    || (! info->shared
! 			&& ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR)
! 			    || h->type == STT_FUNC
! 			    || (h->elf_link_hash_flags
! 				& ELF_LINK_HASH_NEEDS_PLT)))
  		    || (info->shared
  			&& (info->symbolic || h->dynindx == -1)
  			&& (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR)))
  		  {
  		    /* This wasn't checked above for ! info->shared, but
! 		       must hold there if we get here; the symbol must
! 		       be defined in the regular program or be undefweak
! 		       or be a function or otherwise need a PLT.  */
  		    BFD_ASSERT (!elf_hash_table (info)->dynamic_sections_created
  				|| info->shared
  				|| (h->elf_link_hash_flags
  				    & ELF_LINK_HASH_DEF_REGULAR) != 0
+ 				|| h->type == STT_FUNC
+ 				|| (h->elf_link_hash_flags
+ 				    & ELF_LINK_HASH_NEEDS_PLT)
  				|| h->root.type == bfd_link_hash_undefweak);
  
  		    /* This is actually a static link, or it is a
*************** elf_cris_finish_dynamic_symbol (output_b
*** 1511,1522 ****
  	}
      }
  
!   /* We don't emit .got relocs for symbols that aren't in the
!      dynamic-symbols table for an ordinary program and are either defined
!      by the program or are undefined weak symbols.  */
    if (h->got.offset != (bfd_vma) -1
        && (info->shared
  	  || (h->dynindx != -1
  	      && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0
  	      && h->root.type != bfd_link_hash_undefweak)))
      {
--- 1532,1546 ----
  	}
      }
  
!   /* For an ordinary program, we emit .got relocs only for symbols that
!      are in the dynamic-symbols table and are either defined by the
!      program or are undefined weak symbols, or are function symbols
!      where we do not output a PLT: the PLT reloc was output above and all
!      references to the function symbol are redirected to the PLT.  */
    if (h->got.offset != (bfd_vma) -1
        && (info->shared
  	  || (h->dynindx != -1
+ 	      && h->plt.offset == (bfd_vma) -1
  	      && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0
  	      && h->root.type != bfd_link_hash_undefweak)))
      {
*************** cris_elf_gc_sweep_hook (abfd, info, sec,
*** 1838,1850 ****
  	  r_symndx = ELF32_R_SYM (rel->r_info);
  	  if (r_symndx < symtab_hdr->sh_info)
  	    goto local_got_reloc;
! 
  	case R_CRIS_32_PLT_GOTREL:
  	  /* FIXME: We don't garbage-collect away the .got section.  */
  	  if (local_got_refcounts != NULL)
  	    local_got_refcounts[-1]--;
  	  /* Fall through.  */
- 
  	case R_CRIS_8_PCREL:
  	case R_CRIS_16_PCREL:
  	case R_CRIS_32_PCREL:
--- 1862,1873 ----
  	  r_symndx = ELF32_R_SYM (rel->r_info);
  	  if (r_symndx < symtab_hdr->sh_info)
  	    goto local_got_reloc;
! 	  /* Fall through.  */
  	case R_CRIS_32_PLT_GOTREL:
  	  /* FIXME: We don't garbage-collect away the .got section.  */
  	  if (local_got_refcounts != NULL)
  	    local_got_refcounts[-1]--;
  	  /* Fall through.  */
  	case R_CRIS_8_PCREL:
  	case R_CRIS_16_PCREL:
  	case R_CRIS_32_PCREL:
*************** elf_cris_adjust_gotplt_to_got (h, p)
*** 1929,1944 ****
     want to do this:
  
     - When all PLT references are GOTPLT references, and there are GOT
!      references.  We don't have to generate a PLT at all.
  
!    - When there are both (ordinary) PLT references and GOT references.
       We want to make the PLT reference use the ordinary GOT entry rather
!      than a run-time dynamically resolved GOTPLT entry (since the GOT
!      entry will have to be resolved at startup anyway).
  
     Though the latter case is handled when room for the PLT is allocated,
     not here.
  
     Note that this function is called before symbols are forced local by
     version scripts.  The differing cases are handled by
     elf_cris_hide_symbol.  */
--- 1952,1973 ----
     want to do this:
  
     - When all PLT references are GOTPLT references, and there are GOT
!      references, and this is not the executable.  We don't have to
!      generate a PLT at all.
  
!    - When there are both (ordinary) PLT references and GOT references,
!      and this isn't the executable.
       We want to make the PLT reference use the ordinary GOT entry rather
!      than R_CRIS_JUMP_SLOT, a run-time dynamically resolved GOTPLT entry,
!      since the GOT entry will have to be resolved at startup anyway.
  
     Though the latter case is handled when room for the PLT is allocated,
     not here.
  
+    By folding into the GOT, we may need a round-trip to a PLT in the
+    executable for calls, a loss in performance.  Still, losing a
+    reloc is a win in size and at least in start-up time.
+ 
     Note that this function is called before symbols are forced local by
     version scripts.  The differing cases are handled by
     elf_cris_hide_symbol.  */
*************** elf_cris_adjust_dynamic_symbol (info, h)
*** 2045,2054 ****
  					   info);
  	}
  
!       /* If there are only GOT references and GOTPLT references to this
! 	 PLT entry, get rid of the PLT.  */
!       if (! elf_cris_try_fold_plt_to_got ((struct elf_cris_link_hash_entry *)
! 					  h, info))
  	return FALSE;
  
        /* GC or folding may have rendered this entry unused.  */
--- 2074,2087 ----
  					   info);
  	}
  
!       /* If we had a R_CRIS_GLOB_DAT that didn't have to point to a PLT;
! 	 where a pointer-equivalent symbol was unimportant (i.e. more
! 	 like R_CRIS_JUMP_SLOT after symbol evaluation) we could get rid
! 	 of the PLT.  We can't for the executable, because the GOT
! 	 entries will point to the PLT there (and be constant).  */
!       if (info->shared
! 	  && !elf_cris_try_fold_plt_to_got ((struct elf_cris_link_hash_entry*)
! 					    h, info))
  	return FALSE;
  
        /* GC or folding may have rendered this entry unused.  */
*************** elf_cris_adjust_dynamic_symbol (info, h)
*** 2086,2093 ****
  
        /* If there's already a GOT entry, use that, not a .got.plt.  A
  	 GOT field still has a reference count when we get here; it's
! 	 not yet changed to an offset.  */
!       if (h->got.refcount > 0)
  	{
  	  h->got.refcount += h->plt.refcount;
  
--- 2119,2129 ----
  
        /* If there's already a GOT entry, use that, not a .got.plt.  A
  	 GOT field still has a reference count when we get here; it's
! 	 not yet changed to an offset.  We can't do this for an
! 	 executable, because then the reloc associated with the PLT
! 	 would get a non-PLT reloc pointing to the PLT.  FIXME: Move
! 	 this to elf_cris_try_fold_plt_to_got.  */
!       if (info->shared && h->got.refcount > 0)
  	{
  	  h->got.refcount += h->plt.refcount;
  
*************** elf_cris_discard_excess_program_dynamics
*** 2860,2868 ****
  
    /* If we're not creating a shared library and have a symbol which is
       referred to by .got references, but the symbol is defined locally,
!      (or rather, not not defined by a DSO) then lose the reloc for the
!      .got (don't allocate room for it).  */
!   if ((h->root.elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) == 0)
      {
        if (h->root.got.refcount > 0
  	  /* The size of this section is only valid and in sync with the
--- 2896,2906 ----
  
    /* If we're not creating a shared library and have a symbol which is
       referred to by .got references, but the symbol is defined locally,
!      (or rather, not defined by a DSO) then lose the reloc for the .got
!      (don't allocate room for it).  Likewise for relocs for something
!      for which we create a PLT.  */
!   if ((h->root.elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) == 0
!       || h->root.plt.refcount > 0)
      {
        if (h->root.got.refcount > 0
  	  /* The size of this section is only valid and in sync with the
*************** elf_cris_discard_excess_program_dynamics
*** 2888,2894 ****
  	 introduce new problems.  Of course we don't do this if we're
  	 exporting all dynamic symbols.  */
        if (! info->export_dynamic
! 	  && (h->root.elf_link_hash_flags & ELF_LINK_HASH_REF_DYNAMIC) == 0)
  	{
  	  h->root.dynindx = -1;
  	  _bfd_elf_strtab_delref (elf_hash_table (info)->dynstr,
--- 2926,2933 ----
  	 introduce new problems.  Of course we don't do this if we're
  	 exporting all dynamic symbols.  */
        if (! info->export_dynamic
! 	  && (h->root.elf_link_hash_flags
! 	      & (ELF_LINK_HASH_DEF_DYNAMIC|ELF_LINK_HASH_REF_DYNAMIC)) == 0)
  	{
  	  h->root.dynindx = -1;
  	  _bfd_elf_strtab_delref (elf_hash_table (info)->dynstr,

brgds, H-P


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