elf_link_hash_entry vs generic_link_hash_entry

elf_link_hash_entry vs generic_link_hash_entry

[Nick Clifton]

Hi Guys,

  Ian Taylor has pointed out that my recent patch to elfxx-target.h
  has actually broken several elf based ports.  (Specifically: pj,
  m88k, m68hc11, m68hc12, i960, d30v, arc, gen).  The problem is that
  these ports uses the generic linker code to perform section
  relocation rather than having their own specific code.  This breaks
  if the elf hash table structure (elf_link_hash_entry) is used
  instead of the generic_link_hash_entry structure, since the two
  structures are not compatable.  The reason that my patch changed
  elfxx-target.h so that all elf backends would use elf_link_hash_entry
  is that several other parts of the elf linker rely upon using other
  fields which are only found in that structure.

  As I see there are three ways that we can fix this:

   1. Require that all ELF backends define their own section
      relocation function and final link function.  Make it a #error
      if they do not.  Fix all the ports that currently do not do
      this.  This is Ian's recommended solution.

   2. Add the fields from generic_link_hash_entry to
      elf_link_hash_entry so that the generic functions can be used
      with the elf structure.  This is my preferred solution, but as
      Ian points out, this will increase the size of the hash table
      and that is already a significant memory hog.

   3. Go back to the old situation where elf ports that do not provide
      their own relocation backends use the generic linker hash table
      entry structure, but add code to the other elf routines to
      detect this and fail before they try to access fields in the
      hash structure that are not there.

  Any comments ?

Cheers
        Nick

Re: elf_link_hash_entry vs generic_link_hash_entry

[Thiemo Seufer]

Nick Clifton wrote:
> Hi Guys,
> 
>   Ian Taylor has pointed out that my recent patch to elfxx-target.h
>   has actually broken several elf based ports.  (Specifically: pj,
>   m88k, m68hc11, m68hc12, i960, d30v, arc, gen).  The problem is that
>   these ports uses the generic linker code to perform section
>   relocation rather than having their own specific code.  This breaks
>   if the elf hash table structure (elf_link_hash_entry) is used
>   instead of the generic_link_hash_entry structure, since the two
>   structures are not compatable.  The reason that my patch changed
>   elfxx-target.h so that all elf backends would use elf_link_hash_entry
>   is that several other parts of the elf linker rely upon using other
>   fields which are only found in that structure.
> 
>   As I see there are three ways that we can fix this:
> 
>    1. Require that all ELF backends define their own section
>       relocation function and final link function.  Make it a #error
>       if they do not.  Fix all the ports that currently do not do
>       this.  This is Ian's recommended solution.

What about introducing something like generic_elf_final_link_relocate()
and generic_elf_final_link() instead of code duplication?


Thiemo

Re: elf_link_hash_entry vs generic_link_hash_entry

[Ian Lance Taylor]

Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de> writes:

> Nick Clifton wrote:
> > Hi Guys,
> > 
> >   Ian Taylor has pointed out that my recent patch to elfxx-target.h
> >   has actually broken several elf based ports.  (Specifically: pj,
> >   m88k, m68hc11, m68hc12, i960, d30v, arc, gen).  The problem is that
> >   these ports uses the generic linker code to perform section
> >   relocation rather than having their own specific code.  This breaks
> >   if the elf hash table structure (elf_link_hash_entry) is used
> >   instead of the generic_link_hash_entry structure, since the two
> >   structures are not compatable.  The reason that my patch changed
> >   elfxx-target.h so that all elf backends would use elf_link_hash_entry
> >   is that several other parts of the elf linker rely upon using other
> >   fields which are only found in that structure.
> > 
> >   As I see there are three ways that we can fix this:
> > 
> >    1. Require that all ELF backends define their own section
> >       relocation function and final link function.  Make it a #error
> >       if they do not.  Fix all the ports that currently do not do
> >       this.  This is Ian's recommended solution.
> 
> What about introducing something like generic_elf_final_link_relocate()
> and generic_elf_final_link() instead of code duplication?

Each ELF backend has different relocation handling, so a simple
implementation of this would not work.

It should, however, be possible to design a relocation structure which
captured most of the essential elements of ELF relocation types--GOT
creation, etc.--and then use more generic code to handle it.  I think
that this project should not be confused with what Nick is talking
about.

BTW, although I think that Nick's choice 1 is reasonable, I would
actually recommend his choice 3: fix the broken code which assumes
that it knows what type of hash table it is dealing with.  That code
will also break linking directly to S-records.  Admittedly linking
directly to S-records probably doesn't work, but as far as I know
making it work has not been abandoned as a goal.  If it has, I believe
that considerable simplifications are possible.

Ian

Re: elf_link_hash_entry vs generic_link_hash_entry

[H . J . Lu]

On Wed, Aug 22, 2001 at 08:41:11AM +0100, Nick Clifton wrote:
> 
>    3. Go back to the old situation where elf ports that do not provide
>       their own relocation backends use the generic linker hash table
>       entry structure, but add code to the other elf routines to
>       detect this and fail before they try to access fields in the
>       hash structure that are not there.
> 

I'd like this one. How about we turn elf_hash_table into something
specific to each backend? For those targets which don't use
_bfd_elf_link_hash_table_create, it will return NULL or a fatal
bfd error.


H.J.

Re: elf_link_hash_entry vs generic_link_hash_entry

[H . J . Lu]

On Thu, Aug 23, 2001 at 09:21:40AM -0700, H . J . Lu wrote:
> On Wed, Aug 22, 2001 at 08:41:11AM +0100, Nick Clifton wrote:
> > 
> >    3. Go back to the old situation where elf ports that do not provide
> >       their own relocation backends use the generic linker hash table
> >       entry structure, but add code to the other elf routines to
> >       detect this and fail before they try to access fields in the
> >       hash structure that are not there.
> > 
> 
> I'd like this one. How about we turn elf_hash_table into something
> specific to each backend? For those targets which don't use
> _bfd_elf_link_hash_table_create, it will return NULL or a fatal
> bfd error.
> 
> 

Here is a patch. Ian, as you mentioned, linking directly to S-records
no longer works. I got

/export/build/gnu/binutils/build-full-i686-linux/ld/ld-new  -o tmpdir/sr2.sr -T text 0x1000 --oformat srec tmpdir/sr1.o tmpdir/sr2.o
child killed: segmentation violation
FAIL: S-records
/export/build/gnu/binutils/build-full-i686-linux/ld/ld-new  -o tmpdir/sr2.sr -T text 0x1000 --oformat srec tmpdir/sr3.o
child killed: segmentation violation
FAIL: S-records with constructors

How should we fix it?

Thanks.


H.J.
----
2001-08-23  H.J. Lu  <hjl@gnu.org>

	* bfdlink.h (bfd_link_hash_table_type): New. The linker hash
	table type, bfd_link_generic_hash_table and
	bfd_link_elf_hash_table.
	(bfd_link_hash_table): Add a new field, type, for the linker
	hash table type.

2001-08-23  H.J. Lu  <hjl@gnu.org>

	* elf-bfd.h (elf_hash_table): Return NULL if the linker hash
	table is not an ELF linker hash table.

	* elf.c (_bfd_elf_link_hash_table_init): Set the linker hash
	table type to bfd_link_elf_hash_table.

	* elfxx-target.h (bfd_elfNN_bfd_link_hash_table_create): Revert
	the last change.

	* linker.c (_bfd_link_hash_table_init): Set the linker hash
	table type to bfd_link_generic_hash_table.

Index: bfd/elf-bfd.h
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf-bfd.h,v
retrieving revision 1.17
diff -u -p -r1.17 elf-bfd.h
--- bfd/elf-bfd.h	2001/08/23 17:02:19	1.17
+++ bfd/elf-bfd.h	2001/08/23 18:25:39
@@ -280,7 +280,9 @@ struct elf_link_hash_table
 
 /* Get the ELF linker hash table from a link_info structure.  */
 
-#define elf_hash_table(p) ((struct elf_link_hash_table *) ((p)->hash))
+#define elf_hash_table(p)						\
+  ((struct elf_link_hash_table *)					\
+     (((p)->hash->type == bfd_link_elf_hash_table) ? (p)->hash : NULL))
 \f
 /* Constant information held for an ELF backend.  */
 
Index: bfd/elf.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf.c,v
retrieving revision 1.62
diff -u -p -r1.62 elf.c
--- bfd/elf.c	2001/08/23 17:02:20	1.62
+++ bfd/elf.c	2001/08/23 18:25:40
@@ -1047,6 +1047,7 @@ _bfd_elf_link_hash_table_init (table, ab
 						struct bfd_hash_table *,
 						const char *));
 {
+  boolean ret;
   table->dynamic_sections_created = false;
   table->dynobj = NULL;
   /* The first dynamic symbol is a dummy.  */
@@ -1060,7 +1061,9 @@ _bfd_elf_link_hash_table_init (table, ab
   table->stab_info = NULL;
   table->merge_info = NULL;
   table->dynlocal = NULL;
-  return _bfd_link_hash_table_init (&table->root, abfd, newfunc);
+  ret = _bfd_link_hash_table_init (&table->root, abfd, newfunc);
+  table->root.type = bfd_link_elf_hash_table;
+  return ret;
 }
 
 /* Create an ELF linker hash table.  */
Index: bfd/elfxx-target.h
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elfxx-target.h,v
retrieving revision 1.1.1.21
diff -u -p -r1.1.1.21 elfxx-target.h
--- bfd/elfxx-target.h	2001/08/23 16:39:09	1.1.1.21
+++ bfd/elfxx-target.h	2001/08/23 18:25:40
@@ -162,11 +162,16 @@ Foundation, Inc., 59 Temple Place - Suit
   _bfd_elf_canonicalize_dynamic_reloc
 #endif
 
+#ifdef elf_backend_relocate_section
 #ifndef bfd_elfNN_bfd_link_hash_table_create
 #define bfd_elfNN_bfd_link_hash_table_create _bfd_elf_link_hash_table_create
 #endif
-#ifndef elf_backend_relocate_section
+#else /* ! defined (elf_backend_relocate_section) */
 /* If no backend relocate_section routine, use the generic linker.  */
+#ifndef bfd_elfNN_bfd_link_hash_table_create
+#define bfd_elfNN_bfd_link_hash_table_create \
+  _bfd_generic_link_hash_table_create
+#endif
 #ifndef bfd_elfNN_bfd_link_add_symbols
 #define bfd_elfNN_bfd_link_add_symbols	_bfd_generic_link_add_symbols
 #endif
Index: bfd/linker.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/linker.c,v
retrieving revision 1.10
diff -u -p -r1.10 linker.c
--- bfd/linker.c	2001/08/17 17:56:20	1.10
+++ bfd/linker.c	2001/08/23 18:25:40
@@ -483,6 +483,7 @@ _bfd_link_hash_table_init (table, abfd, 
   table->creator = abfd->xvec;
   table->undefs = NULL;
   table->undefs_tail = NULL;
+  table->type = bfd_link_generic_hash_table;
   return bfd_hash_table_init (&table->table, newfunc);
 }
 
Index: include/bfdlink.h
===================================================================
RCS file: /work/cvs/gnu/binutils/include/bfdlink.h,v
retrieving revision 1.15
diff -u -p -r1.15 bfdlink.h
--- include/bfdlink.h	2001/08/23 17:02:23	1.15
+++ include/bfdlink.h	2001/08/23 18:25:42
@@ -42,6 +42,12 @@ enum bfd_link_discard
   discard_all		/* Discard all locals.  */
 };
 \f
+enum bfd_link_hash_table_type
+{
+  bfd_link_generic_hash_table,
+  bfd_link_elf_hash_table
+};
+
 /* These are the possible types of an entry in the BFD link hash
    table.  */
 
@@ -146,6 +152,8 @@ struct bfd_link_hash_table
   struct bfd_link_hash_entry *undefs;
   /* Entries are added to the tail of the undefs list.  */
   struct bfd_link_hash_entry *undefs_tail;
+  /* The type of the ink hash table.  */
+  enum bfd_link_hash_table_type type;
 };
 
 /* Look up an entry in a link hash table.  If FOLLOW is true, this

Re: elf_link_hash_entry vs generic_link_hash_entry

[H . J . Lu]

On Thu, Aug 23, 2001 at 11:36:56AM -0700, H . J . Lu wrote:
> On Thu, Aug 23, 2001 at 09:21:40AM -0700, H . J . Lu wrote:
> > On Wed, Aug 22, 2001 at 08:41:11AM +0100, Nick Clifton wrote:
> > > 
> > >    3. Go back to the old situation where elf ports that do not provide
> > >       their own relocation backends use the generic linker hash table
> > >       entry structure, but add code to the other elf routines to
> > >       detect this and fail before they try to access fields in the
> > >       hash structure that are not there.
> > > 
> > 
> > I'd like this one. How about we turn elf_hash_table into something
> > specific to each backend? For those targets which don't use
> > _bfd_elf_link_hash_table_create, it will return NULL or a fatal
> > bfd error.
> > 
> > 
> 
> Here is a patch. Ian, as you mentioned, linking directly to S-records
> no longer works. I got
> 
> /export/build/gnu/binutils/build-full-i686-linux/ld/ld-new  -o tmpdir/sr2.sr -T text 0x1000 --oformat srec tmpdir/sr1.o tmpdir/sr2.o
> child killed: segmentation violation
> FAIL: S-records
> /export/build/gnu/binutils/build-full-i686-linux/ld/ld-new  -o tmpdir/sr2.sr -T text 0x1000 --oformat srec tmpdir/sr3.o
> child killed: segmentation violation
> FAIL: S-records with constructors
> 
> How should we fix it?
> 

How about this patch?

H.J.
----
2001-08-23  H.J. Lu  <hjl@gnu.org>

        * elflink.h (elf_link_add_object_symbols): Don't merge section
        nor add the file to the loaded list if the linker hash table
        is not an an ELF linker hash table.

Index: elflink.h
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elflink.h,v
retrieving revision 1.70
diff -u -p -r1.70 elflink.h
--- elflink.h	2001/08/23 17:02:20	1.70
+++ elflink.h	2001/08/23 19:07:05
@@ -1068,7 +1068,10 @@ elf_link_add_object_symbols (abfd, info)
   Elf_External_Sym *esymend;
   struct elf_backend_data *bed;
   boolean dt_needed;
+  struct elf_link_hash_table *hash_table;
 
+  hash_table = elf_hash_table (info);
+
   bed = get_elf_backend_data (abfd);
   add_symbol_hook = bed->elf_add_symbol_hook;
   collect = bed->collect;
@@ -2330,33 +2333,32 @@ elf_link_add_object_symbols (abfd, info)
 	}
     }
 
-  if (! info->relocateable && ! dynamic)
+  if (hash_table && ! info->relocateable && ! dynamic)
     {
       asection *s;
 
       for (s = abfd->sections; s != NULL; s = s->next)
 	if ((s->flags & SEC_MERGE)
-	    && ! _bfd_merge_section (abfd,
-				     &elf_hash_table (info)->merge_info,
-				     s, &elf_section_data (s)->merge_info))
+	    && ! _bfd_merge_section (abfd, &hash_table->merge_info, s,
+				     &elf_section_data (s)->merge_info))
 	  goto error_return;
     }
+
+  if (hash_table)
+    {
+      /* We add this to the loaded list.  */
+      struct elf_link_loaded_list *n, **pn;
 
-  {
-    /* We add this to the loaded list.  */
-    struct elf_link_loaded_list *n, **pn;
-
-    n = ((struct elf_link_loaded_list *)
-	 bfd_alloc (abfd, sizeof (struct elf_link_loaded_list)));
-    if (n == NULL)
-      goto error_return;
-    n->next = NULL;
-    n->abfd = abfd;
-    for (pn = &elf_hash_table (info)->loaded; *pn != NULL;
-	 pn = &(*pn)->next)
-      ;
-    *pn = n;
-  }
+      n = ((struct elf_link_loaded_list *)
+	   bfd_alloc (abfd, sizeof (struct elf_link_loaded_list)));
+      if (n == NULL)
+	goto error_return;
+      n->next = NULL;
+      n->abfd = abfd;
+      for (pn = &hash_table->loaded; *pn != NULL; pn = &(*pn)->next)
+	;
+      *pn = n;
+    }
 
   return true;
 

Re: elf_link_hash_entry vs generic_link_hash_entry

[Nick Clifton]

Hi H.J.

> How about this patch?
> 
> H.J.
> ----
> 2001-08-23  H.J. Lu  <hjl@gnu.org>
> 
>         * elflink.h (elf_link_add_object_symbols): Don't merge section
>         nor add the file to the loaded list if the linker hash table
>         is not an an ELF linker hash table.

This patch does not appear to be based against the current binutils
sources.  (I guess it was made against your own source tree ?)
Instead I created a modified version of your patch.

I had to make one change though - I reverted the definition of
elf_hash_table() back to its original version, since it cannot be
allowed to return NULL.  (It is used in too many places as the first
argument to elf_link_hash_lookup).  Instead I created a new macro
called 'is_elf_hash_table' and I added code to check it in
elf_link_add_object_symbols(), like this.

Cheers
        Nick

2001-08-24  Nick Clifton  <nickc@cambridge.redhat.com>

	* elf-bfd.h (elf_hash_table): Revert definition.
	(is_elf_hash_table): New macro.
	* elflink.h (elf_link_add_object_symbols): Test
	is_elf_hash_table before accessing ELF only fields in hash
	structure.
	(elf_link_create_dynamic_sections): Fail if not using an ELF
	hash structure.
	(elf_add_dynamic_entry): Fail if not using an ELF hash
	structure.
	(elf_link_record_local_dynamic_symbol): Fail if not using an
	ELF hash structure.
	(size_dynamic_sections): Fail if not using an ELF hash
	structure.
	(elf_adjust_dynamic_symbol): Fail if not using an ELF
	hash structure.
	(elf_bfd_final_link): Fail if not using an ELF hash
	structure.

Index: elflink.h
===================================================================
RCS file: /cvs/src/src/bfd/elflink.h,v
retrieving revision 1.102
diff -p -r1.102 elflink.h
*** elflink.h	2001/08/24 11:17:28	1.102
--- elflink.h	2001/08/24 16:28:30
*************** elf_link_add_object_symbols (abfd, info)
*** 916,922 ****
--- 916,925 ----
    Elf_External_Sym *esymend;
    struct elf_backend_data *bed;
    boolean dt_needed;
+   struct elf_link_hash_table * hash_table;
  
+   hash_table = elf_hash_table (info);
+ 
    bed = get_elf_backend_data (abfd);
    add_symbol_hook = bed->elf_add_symbol_hook;
    collect = bed->collect;
*************** elf_link_add_object_symbols (abfd, info)
*** 970,976 ****
  		{
  		  struct elf_link_hash_entry *h;
  
! 		  h = elf_link_hash_lookup (elf_hash_table (info), name,
  					    false, false, true);
  
  		  /* FIXME: What about bfd_link_hash_common?  */
--- 973,979 ----
  		{
  		  struct elf_link_hash_entry *h;
  
! 		  h = elf_link_hash_lookup (hash_table, name,
  					    false, false, true);
  
  		  /* FIXME: What about bfd_link_hash_common?  */
*************** elf_link_add_object_symbols (abfd, info)
*** 1085,1097 ****
           format.  FIXME: If there are no input BFD's of the same
           format as the output, we can't make a shared library.  */
        if (info->shared
! 	  && ! elf_hash_table (info)->dynamic_sections_created
  	  && abfd->xvec == info->hash->creator)
  	{
  	  if (! elf_link_create_dynamic_sections (abfd, info))
  	    goto error_return;
  	}
      }
    else
      {
        asection *s;
--- 1088,1103 ----
           format.  FIXME: If there are no input BFD's of the same
           format as the output, we can't make a shared library.  */
        if (info->shared
! 	  && is_elf_hash_table (info)
! 	  && ! hash_table->dynamic_sections_created
  	  && abfd->xvec == info->hash->creator)
  	{
  	  if (! elf_link_create_dynamic_sections (abfd, info))
  	    goto error_return;
  	}
      }
+   else if (! is_elf_hash_table (info))
+     goto error_return;
    else
      {
        asection *s;
*************** elf_link_add_object_symbols (abfd, info)
*** 1194,1200 ****
  		  n->name = anm;
  		  n->by = abfd;
  		  n->next = NULL;
! 		  for (pn = &elf_hash_table (info)->needed;
  		       *pn != NULL;
  		       pn = &(*pn)->next)
  		    ;
--- 1200,1206 ----
  		  n->name = anm;
  		  n->by = abfd;
  		  n->next = NULL;
! 		  for (pn = & hash_table->needed;
  		       *pn != NULL;
  		       pn = &(*pn)->next)
  		    ;
*************** elf_link_add_object_symbols (abfd, info)
*** 1209,1216 ****
  		     to clear runpath.  Do _NOT_ bfd_release, as that
  		     frees all more recently bfd_alloc'd blocks as
  		     well.  */
! 		  if (rpath && elf_hash_table (info)->runpath)
! 		    elf_hash_table (info)->runpath = NULL;
  
  		  n = ((struct bfd_link_needed_list *)
  		       bfd_alloc (abfd, sizeof (struct bfd_link_needed_list)));
--- 1215,1222 ----
  		     to clear runpath.  Do _NOT_ bfd_release, as that
  		     frees all more recently bfd_alloc'd blocks as
  		     well.  */
! 		  if (rpath && hash_table->runpath)
! 		    hash_table->runpath = NULL;
  
  		  n = ((struct bfd_link_needed_list *)
  		       bfd_alloc (abfd, sizeof (struct bfd_link_needed_list)));
*************** elf_link_add_object_symbols (abfd, info)
*** 1225,1231 ****
  		  n->name = anm;
  		  n->by = abfd;
  		  n->next = NULL;
! 		  for (pn = &elf_hash_table (info)->runpath;
  		       *pn != NULL;
  		       pn = &(*pn)->next)
  		    ;
--- 1231,1237 ----
  		  n->name = anm;
  		  n->by = abfd;
  		  n->next = NULL;
! 		  for (pn = & hash_table->runpath;
  		       *pn != NULL;
  		       pn = &(*pn)->next)
  		    ;
*************** elf_link_add_object_symbols (abfd, info)
*** 1252,1258 ****
  		  n->name = anm;
  		  n->by = abfd;
  		  n->next = NULL;
! 		  for (pn = &elf_hash_table (info)->runpath;
  		       *pn != NULL;
  		       pn = &(*pn)->next)
  		    ;
--- 1258,1264 ----
  		  n->name = anm;
  		  n->by = abfd;
  		  n->next = NULL;
! 		  for (pn = & hash_table->runpath;
  		       *pn != NULL;
  		       pn = &(*pn)->next)
  		    ;
*************** elf_link_add_object_symbols (abfd, info)
*** 1277,1298 ****
  
        /* If this is the first dynamic object found in the link, create
  	 the special sections required for dynamic linking.  */
!       if (! elf_hash_table (info)->dynamic_sections_created)
! 	{
! 	  if (! elf_link_create_dynamic_sections (abfd, info))
! 	    goto error_return;
! 	}
  
        if (add_needed)
  	{
  	  /* Add a DT_NEEDED entry for this dynamic object.  */
! 	  oldsize = _bfd_stringtab_size (elf_hash_table (info)->dynstr);
! 	  strindex = _bfd_stringtab_add (elf_hash_table (info)->dynstr, name,
  					 true, false);
  	  if (strindex == (bfd_size_type) -1)
  	    goto error_return;
  
! 	  if (oldsize == _bfd_stringtab_size (elf_hash_table (info)->dynstr))
  	    {
  	      asection *sdyn;
  	      Elf_External_Dyn *dyncon, *dynconend;
--- 1283,1302 ----
  
        /* If this is the first dynamic object found in the link, create
  	 the special sections required for dynamic linking.  */
!       if (! hash_table->dynamic_sections_created)
! 	if (! elf_link_create_dynamic_sections (abfd, info))
! 	  goto error_return;
  
        if (add_needed)
  	{
  	  /* Add a DT_NEEDED entry for this dynamic object.  */
! 	  oldsize = _bfd_stringtab_size (hash_table->dynstr);
! 	  strindex = _bfd_stringtab_add (hash_table->dynstr, name,
  					 true, false);
  	  if (strindex == (bfd_size_type) -1)
  	    goto error_return;
  
! 	  if (oldsize == _bfd_stringtab_size (hash_table->dynstr))
  	    {
  	      asection *sdyn;
  	      Elf_External_Dyn *dyncon, *dynconend;
*************** elf_link_add_object_symbols (abfd, info)
*** 1302,1309 ****
  		 have already included this dynamic object in the
  		 link, just ignore it.  There is no reason to include
  		 a particular dynamic object more than once.  */
! 	      sdyn = bfd_get_section_by_name (elf_hash_table (info)->dynobj,
! 					      ".dynamic");
  	      BFD_ASSERT (sdyn != NULL);
  
  	      dyncon = (Elf_External_Dyn *) sdyn->contents;
--- 1306,1312 ----
  		 have already included this dynamic object in the
  		 link, just ignore it.  There is no reason to include
  		 a particular dynamic object more than once.  */
! 	      sdyn = bfd_get_section_by_name (hash_table->dynobj, ".dynamic");
  	      BFD_ASSERT (sdyn != NULL);
  
  	      dyncon = (Elf_External_Dyn *) sdyn->contents;
*************** elf_link_add_object_symbols (abfd, info)
*** 1313,1320 ****
  		{
  		  Elf_Internal_Dyn dyn;
  
! 		  elf_swap_dyn_in (elf_hash_table (info)->dynobj, dyncon,
! 				   &dyn);
  		  if (dyn.d_tag == DT_NEEDED
  		      && dyn.d_un.d_val == strindex)
  		    {
--- 1316,1322 ----
  		{
  		  Elf_Internal_Dyn dyn;
  
! 		  elf_swap_dyn_in (hash_table->dynobj, dyncon, & dyn);
  		  if (dyn.d_tag == DT_NEEDED
  		      && dyn.d_un.d_val == strindex)
  		    {
*************** elf_link_add_object_symbols (abfd, info)
*** 1969,1993 ****
  	      bfd_size_type oldsize;
  	      bfd_size_type strindex;
  
  	      /* The symbol from a DT_NEEDED object is referenced from
  	         the regular object to create a dynamic executable. We
  		 have to make sure there is a DT_NEEDED entry for it.  */
  
  	      dt_needed = false;
! 	      oldsize = _bfd_stringtab_size (elf_hash_table (info)->dynstr);
! 	      strindex = _bfd_stringtab_add (elf_hash_table (info)->dynstr,
  	      				     elf_dt_soname (abfd),
  					     true, false);
  	      if (strindex == (bfd_size_type) -1)
  		goto error_return;
  
  	      if (oldsize
! 		  == _bfd_stringtab_size (elf_hash_table (info)->dynstr))
  		{
  		  asection *sdyn;
  		  Elf_External_Dyn *dyncon, *dynconend;
  
! 		  sdyn = bfd_get_section_by_name (elf_hash_table (info)->dynobj,
  						  ".dynamic");
  		  BFD_ASSERT (sdyn != NULL);
  
--- 1971,1998 ----
  	      bfd_size_type oldsize;
  	      bfd_size_type strindex;
  
+ 	      if (! is_elf_hash_table (info))
+ 		goto error_return;
+ 
  	      /* The symbol from a DT_NEEDED object is referenced from
  	         the regular object to create a dynamic executable. We
  		 have to make sure there is a DT_NEEDED entry for it.  */
  
  	      dt_needed = false;
! 	      oldsize = _bfd_stringtab_size (hash_table->dynstr);
! 	      strindex = _bfd_stringtab_add (hash_table->dynstr,
  	      				     elf_dt_soname (abfd),
  					     true, false);
  	      if (strindex == (bfd_size_type) -1)
  		goto error_return;
  
  	      if (oldsize
! 		  == _bfd_stringtab_size (hash_table->dynstr))
  		{
  		  asection *sdyn;
  		  Elf_External_Dyn *dyncon, *dynconend;
  
! 		  sdyn = bfd_get_section_by_name (hash_table->dynobj,
  						  ".dynamic");
  		  BFD_ASSERT (sdyn != NULL);
  
*************** elf_link_add_object_symbols (abfd, info)
*** 1998,2004 ****
  		    {
  		      Elf_Internal_Dyn dyn;
  
! 		      elf_swap_dyn_in (elf_hash_table (info)->dynobj,
  				       dyncon, &dyn);
  		      BFD_ASSERT (dyn.d_tag != DT_NEEDED ||
  				  dyn.d_un.d_val != strindex);
--- 2003,2009 ----
  		    {
  		      Elf_Internal_Dyn dyn;
  
! 		      elf_swap_dyn_in (hash_table->dynobj,
  				       dyncon, &dyn);
  		      BFD_ASSERT (dyn.d_tag != DT_NEEDED ||
  				  dyn.d_un.d_val != strindex);
*************** elf_link_add_object_symbols (abfd, info)
*** 2155,2160 ****
--- 2160,2166 ----
        && ! info->relocateable
        && ! info->traditional_format
        && info->hash->creator->flavour == bfd_target_elf_flavour
+       && is_elf_hash_table (info)
        && (info->strip != strip_all && info->strip != strip_debugger))
      {
        asection *stab, *stabstr;
*************** elf_link_add_object_symbols (abfd, info)
*** 2170,2176 ****
  
  	      secdata = elf_section_data (stab);
  	      if (! _bfd_link_section_stabs (abfd,
! 					     &elf_hash_table (info)->stab_info,
  					     stab, stabstr,
  					     &secdata->stab_info))
  		goto error_return;
--- 2176,2182 ----
  
  	      secdata = elf_section_data (stab);
  	      if (! _bfd_link_section_stabs (abfd,
! 					     & hash_table->stab_info,
  					     stab, stabstr,
  					     &secdata->stab_info))
  		goto error_return;
*************** elf_link_add_object_symbols (abfd, info)
*** 2178,2192 ****
  	}
      }
  
!   if (! info->relocateable && ! dynamic)
      {
        asection *s;
  
        for (s = abfd->sections; s != NULL; s = s->next)
  	if ((s->flags & SEC_MERGE)
! 	    && ! _bfd_merge_section (abfd,
! 				     &elf_hash_table (info)->merge_info,
! 				     s, &elf_section_data (s)->merge_info))
  	  goto error_return;
      }
  
--- 2184,2198 ----
  	}
      }
  
!   if (! info->relocateable && ! dynamic
!       && is_elf_hash_table (info))
      {
        asection *s;
  
        for (s = abfd->sections; s != NULL; s = s->next)
  	if ((s->flags & SEC_MERGE)
! 	    && ! _bfd_merge_section (abfd, & hash_table->merge_info, s,
! 				     & elf_section_data (s)->merge_info))
  	  goto error_return;
      }
  
*************** elf_link_create_dynamic_sections (abfd, 
*** 2219,2224 ****
--- 2225,2233 ----
    struct elf_link_hash_entry *h;
    struct elf_backend_data *bed;
  
+   if (! is_elf_hash_table (info))
+     return false;
+ 
    if (elf_hash_table (info)->dynamic_sections_created)
      return true;
  
*************** elf_add_dynamic_entry (info, tag, val)
*** 2343,2348 ****
--- 2352,2360 ----
    size_t newsize;
    bfd_byte *newcontents;
  
+   if (! is_elf_hash_table (info))
+     return false;
+ 
    dynobj = elf_hash_table (info)->dynobj;
  
    s = bfd_get_section_by_name (dynobj, ".dynamic");
*************** elf_link_record_local_dynamic_symbol (in
*** 2379,2384 ****
--- 2391,2399 ----
    unsigned long dynstr_index;
    char *name;
  
+   if (! is_elf_hash_table (info))
+     return false;
+ 
    /* See if the entry exists already.  */
    for (entry = elf_hash_table (info)->dynlocal; entry ; entry = entry->next)
      if (entry->input_bfd == input_bfd && entry->input_indx == input_indx)
*************** NAME(bfd_elf,size_dynamic_sections) (out
*** 2895,2900 ****
--- 2910,2918 ----
    if (info->hash->creator->flavour != bfd_target_elf_flavour)
      return true;
  
+   if (! is_elf_hash_table (info))
+     return false;
+ 
    /* The backend may have to create some sections regardless of whether
       we're dynamic or not.  */
    bed = get_elf_backend_data (output_bfd);
*************** elf_fix_symbol_flags (h, eif)
*** 3542,3553 ****
--- 3560,3573 ----
       backend specifically; we can't just clear PLT-related data here.  */
    if ((h->elf_link_hash_flags & ELF_LINK_HASH_NEEDS_PLT) != 0
        && eif->info->shared
+       && is_elf_hash_table (eif->info)
        && (eif->info->symbolic
  	  || ELF_ST_VISIBILITY (h->other) == STV_INTERNAL
  	  || ELF_ST_VISIBILITY (h->other) == STV_HIDDEN)
        && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) != 0)
      {
        struct elf_backend_data *bed;
+ 
        bed = get_elf_backend_data (elf_hash_table (eif->info)->dynobj);
        if (ELF_ST_VISIBILITY (h->other) == STV_INTERNAL
  	  || ELF_ST_VISIBILITY (h->other) == STV_HIDDEN)
*************** elf_adjust_dynamic_symbol (h, data)
*** 3602,3607 ****
--- 3622,3630 ----
    if (h->root.type == bfd_link_hash_indirect)
      return true;
  
+   if (! is_elf_hash_table (eif->info))
+     return false;
+ 
    /* Fix the symbol flags.  */
    if (! elf_fix_symbol_flags (h, eif))
      return false;
*************** elf_bfd_final_link (abfd, info)
*** 4511,4516 ****
--- 4534,4542 ----
    boolean merged;
    size_t relativecount = 0;
    asection *reldyn = 0;
+ 
+   if (! is_elf_hash_table (info))
+     return false;
  
    if (info->shared)
      abfd->flags |= DYNAMIC;

Re: elf_link_hash_entry vs generic_link_hash_entry

[H . J . Lu]

On Fri, Aug 24, 2001 at 05:32:19PM +0100, Nick Clifton wrote:
> 
> This patch does not appear to be based against the current binutils
> sources.  (I guess it was made against your own source tree ?)
> Instead I created a modified version of your patch.

Probably.

> 
> I had to make one change though - I reverted the definition of
> elf_hash_table() back to its original version, since it cannot be
> allowed to return NULL.  (It is used in too many places as the first

I think it should return NULL when you try to access the generic hash
table as an ELF hash table. Otherwise, you don't even know you are
overriding memory which doesn't belong to you. It is not too hard to
fix the broken code, which is wrong to begin with.

H.J.

Re: elf_link_hash_entry vs generic_link_hash_entry

[H . J . Lu]

On Fri, Aug 24, 2001 at 05:32:19PM +0100, Nick Clifton wrote:
> 
> I had to make one change though - I reverted the definition of
> elf_hash_table() back to its original version, since it cannot be
> allowed to return NULL.  (It is used in too many places as the first
> argument to elf_link_hash_lookup).  Instead I created a new macro
> called 'is_elf_hash_table' and I added code to check it in
> elf_link_add_object_symbols(), like this.
> 

I don't like it. I prefer elf_hash_table

1. Returns NULL on generic hash table

#define elf_hash_table(p)	\
  ((struct elf_link_hash_table *) \
     (((p)->hash->type == bfd_link_elf_hash_table) ? (p)->hash : NULL))

so that ld gets a core dump. Or

2. Aborts on generic hash table

#define elf_hash_table(p)	\
  ((struct elf_link_hash_table *) \
     (((p)->hash->type == bfd_link_elf_hash_table)
      ? (p)->hash : (abort (), NULL)))

so that we can catch all the bogus code.


H.J.

Re: elf_link_hash_entry vs generic_link_hash_entry

[Nick Clifton]

Hi H.J.

> 2001-08-23  H.J. Lu  <hjl@gnu.org>
> 
> 	* bfdlink.h (bfd_link_hash_table_type): New. The linker hash
> 	table type, bfd_link_generic_hash_table and
> 	bfd_link_elf_hash_table.
> 	(bfd_link_hash_table): Add a new field, type, for the linker
> 	hash table type.
> 
> 2001-08-23  H.J. Lu  <hjl@gnu.org>
> 
> 	* elf-bfd.h (elf_hash_table): Return NULL if the linker hash
> 	table is not an ELF linker hash table.
> 
> 	* elf.c (_bfd_elf_link_hash_table_init): Set the linker hash
> 	table type to bfd_link_elf_hash_table.
> 
> 	* elfxx-target.h (bfd_elfNN_bfd_link_hash_table_create): Revert
> 	the last change.
> 
> 	* linker.c (_bfd_link_hash_table_init): Set the linker hash
> 	table type to bfd_link_generic_hash_table.

Approved and applied (since I was checking it at the time).

Cheers
        Nick

Re: elf_link_hash_entry vs generic_link_hash_entry

[H . J . Lu]

On Fri, Aug 24, 2001 at 05:15:10PM +0100, Nick Clifton wrote:
> Hi H.J.
> 
> > 2001-08-23  H.J. Lu  <hjl@gnu.org>
> > 
> > 	* bfdlink.h (bfd_link_hash_table_type): New. The linker hash
> > 	table type, bfd_link_generic_hash_table and
> > 	bfd_link_elf_hash_table.
> > 	(bfd_link_hash_table): Add a new field, type, for the linker
> > 	hash table type.
> > 
> > 2001-08-23  H.J. Lu  <hjl@gnu.org>
> > 
> > 	* elf-bfd.h (elf_hash_table): Return NULL if the linker hash
> > 	table is not an ELF linker hash table.
> > 
> > 	* elf.c (_bfd_elf_link_hash_table_init): Set the linker hash
> > 	table type to bfd_link_elf_hash_table.
> > 
> > 	* elfxx-target.h (bfd_elfNN_bfd_link_hash_table_create): Revert
> > 	the last change.
> > 
> > 	* linker.c (_bfd_link_hash_table_init): Set the linker hash
> > 	table type to bfd_link_generic_hash_table.
> 
> Approved and applied (since I was checking it at the time).

Thanks. But I didn't see them in CVS :-).



H.J.

Re: elf_link_hash_entry vs generic_link_hash_entry

[Richard Henderson]

On Thu, Aug 23, 2001 at 11:36:56AM -0700, H . J . Lu wrote:
> Here is a patch. Ian, as you mentioned, linking directly to S-records
> no longer works.
[...]
> How should we fix it?

In general I think it's unfixable.  Yes, you were able to come
up with something that worked for i386, but not for targets that
require non-trivial bookkeeping during final link -- ia64 and 
alpha come to mind here.

Besides, I think such is a waste of time and effort.  If you 
want S-records, we have this nice objcopy utiltity.


r~