[RFC] unify dynamic_symbol_p implementations

[RFC] unify dynamic_symbol_p implementations

[Richard Henderson]

Anyone see anything wrong with this?


r~


	* elflink.c (_bfd_elf_dynamic_symbol_p): New.
	* elf-bfd.h (_bfd_elf_dynamic_symbol_p): Declare it.
	(SYMBOL_REFERENCES_LOCAL, SYMBOL_CALLS_LOCAL): Use it.
	* elf32-xtensa.c (xtensa_elf_dynamic_symbol_p): Likewise.
	* elf64-alpha.c (alpha_elf_dynamic_symbol_p): Likewise.
	* elf64-hppa.c (elf64_hppa_dynamic_symbol_p): Likewise.
	* elfxx-ia64.c (elfNN_ia64_dynamic_symbol_p): Likewise.
	Update all callers to provide the relocation being resolved.

Index: elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.103
diff -c -p -d -u -r1.103 elf-bfd.h
--- elf-bfd.h	8 Jul 2003 13:10:51 -0000	1.103
+++ elf-bfd.h	16 Jul 2003 23:35:31 -0000
@@ -211,23 +211,12 @@ struct elf_link_hash_entry
    function symbols not defined in an app are set to their .plt entry,
    it's necessary for shared libs to also reference the .plt even
    though the symbol is really local to the shared lib.  */
-#define SYMBOL_REFERENCES_LOCAL(INFO, H)				\
-  (((INFO)->executable							\
-    || (INFO)->symbolic							\
-    || (H)->dynindx == -1						\
-    || ELF_ST_VISIBILITY ((H)->other) == STV_INTERNAL			\
-    || ELF_ST_VISIBILITY ((H)->other) == STV_HIDDEN			\
-    || ((H)->elf_link_hash_flags & ELF_LINK_FORCED_LOCAL) != 0)		\
-   && ((H)->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) != 0)
+#define SYMBOL_REFERENCES_LOCAL(INFO, H) \
+  _bfd_elf_dynamic_symbol_p (H, INFO, 1)
 
 /* Will _calls_ to this symbol always call the version in this object?  */
-#define SYMBOL_CALLS_LOCAL(INFO, H)					\
-  (((INFO)->executable							\
-    || (INFO)->symbolic							\
-    || (H)->dynindx == -1						\
-    || ELF_ST_VISIBILITY ((H)->other) != STV_DEFAULT			\
-    || ((H)->elf_link_hash_flags & ELF_LINK_FORCED_LOCAL) != 0)		\
-   && ((H)->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) != 0)
+#define SYMBOL_CALLS_LOCAL(INFO, H) \
+  _bfd_elf_dynamic_symbol_p (H, INFO, 0)
 
 /* Records local symbols to be emitted in the dynamic symbol table.  */
 
@@ -1505,6 +1494,9 @@ extern bfd_boolean _bfd_elf_adjust_dynam
 
 extern bfd_boolean _bfd_elf_link_sec_merge_syms
   PARAMS ((struct elf_link_hash_entry *, PTR));
+
+extern bfd_boolean _bfd_elf_dynamic_symbol_p
+  PARAMS ((struct elf_link_hash_entry *, struct bfd_link_info *, bfd_boolean));
 
 extern const bfd_target *bfd_elf32_object_p
   PARAMS ((bfd *));
Index: elf32-xtensa.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-xtensa.c,v
retrieving revision 1.5
diff -c -p -d -u -r1.5 elf32-xtensa.c
--- elf32-xtensa.c	10 Jul 2003 19:01:47 -0000	1.5
+++ elf32-xtensa.c	16 Jul 2003 23:35:31 -0000
@@ -1838,38 +1838,9 @@ xtensa_elf_dynamic_symbol_p (info, h)
      struct bfd_link_info *info;
      struct elf_link_hash_entry *h;
 {
-  if (h == NULL)
-    return FALSE;
-
-  while (h->root.type == bfd_link_hash_indirect
-	 || h->root.type == bfd_link_hash_warning)
-    h = (struct elf_link_hash_entry *) h->root.u.i.link;
-
-  if (h->dynindx == -1)
-    return FALSE;
-
-  if (h->root.type == bfd_link_hash_undefweak
-      || h->root.type == bfd_link_hash_defweak)
-    return TRUE;
-
-  switch (ELF_ST_VISIBILITY (h->other))
-    {
-    case STV_DEFAULT:
-      break;
-    case STV_HIDDEN:
-    case STV_INTERNAL:
-      return FALSE;
-    case STV_PROTECTED:
-      if (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR)
-        return FALSE;
-      break;
-    }
-
-  if ((info->shared && !info->symbolic)
-      || (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
-    return TRUE;
-
-  return FALSE;
+  /* ??? What, if anything, needs to happen wrt STV_PROTECTED and PLT
+     entries?  For now assume the worst.  */
+  return _bfd_elf_dynamic_symbol_p (h, info, 1);
 }
 
 
Index: elf64-alpha.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-alpha.c,v
retrieving revision 1.101
diff -c -p -d -u -r1.101 elf64-alpha.c
--- elf64-alpha.c	4 Jul 2003 13:53:37 -0000	1.101
+++ elf64-alpha.c	16 Jul 2003 23:35:31 -0000
@@ -47,7 +47,7 @@
 #define ECOFF_64
 #include "ecoffswap.h"
 
-static int alpha_elf_dynamic_symbol_p
+static bfd_boolean alpha_elf_dynamic_symbol_p
   PARAMS ((struct elf_link_hash_entry *, struct bfd_link_info *));
 static struct bfd_hash_entry * elf64_alpha_link_hash_newfunc
   PARAMS ((struct bfd_hash_entry *, struct bfd_hash_table *, const char *));
@@ -269,49 +269,17 @@ struct alpha_elf_link_hash_table
 #define alpha_elf_sym_hashes(abfd) \
   ((struct alpha_elf_link_hash_entry **)elf_sym_hashes(abfd))
 
-/* Should we do dynamic things to this symbol?  */
+/* Should we do dynamic things to this symbol?  This differs from the 
+   generic version in that we never need to consider function pointer
+   equality wrt PLT entries -- we don't create a PLT entry if a symbol's
+   address is ever taken.  */
 
-static int
+static inline bfd_boolean
 alpha_elf_dynamic_symbol_p (h, info)
      struct elf_link_hash_entry *h;
      struct bfd_link_info *info;
 {
-  if (h == NULL)
-    return FALSE;
-
-  while (h->root.type == bfd_link_hash_indirect
-	 || h->root.type == bfd_link_hash_warning)
-    h = (struct elf_link_hash_entry *) h->root.u.i.link;
-
-  if (h->dynindx == -1)
-    return FALSE;
-
-  if (h->root.type == bfd_link_hash_undefweak
-      || h->root.type == bfd_link_hash_defweak)
-    return TRUE;
-
-  switch (ELF_ST_VISIBILITY (h->other))
-    {
-    case STV_DEFAULT:
-      break;
-    case STV_HIDDEN:
-    case STV_INTERNAL:
-      return FALSE;
-    case STV_PROTECTED:
-      if (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR)
-        return FALSE;
-      break;
-    }
-
-  if ((info->shared && !info->symbolic)
-      || ((h->elf_link_hash_flags
-	   & (ELF_LINK_HASH_DEF_DYNAMIC
-	      | ELF_LINK_HASH_DEF_REGULAR
-	      | ELF_LINK_HASH_REF_REGULAR))
-	  == (ELF_LINK_HASH_DEF_DYNAMIC | ELF_LINK_HASH_REF_REGULAR)))
-    return TRUE;
-
-  return FALSE;
+  return _bfd_elf_dynamic_symbol_p (h, info, 0);
 }
 
 /* Create an entry in a Alpha ELF linker hash table.  */
Index: elf64-hppa.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-hppa.c,v
retrieving revision 1.33
diff -c -p -d -u -r1.33 elf64-hppa.c
--- elf64-hppa.c	25 Jun 2003 06:40:23 -0000	1.33
+++ elf64-hppa.c	16 Jul 2003 23:35:31 -0000
@@ -956,30 +956,19 @@ elf64_hppa_dynamic_symbol_p (h, info)
      struct elf_link_hash_entry *h;
      struct bfd_link_info *info;
 {
-  if (h == NULL)
-    return FALSE;
-
-  while (h->root.type == bfd_link_hash_indirect
-	 || h->root.type == bfd_link_hash_warning)
-    h = (struct elf_link_hash_entry *) h->root.u.i.link;
-
-  if (h->dynindx == -1)
-    return FALSE;
-
-  if (h->root.type == bfd_link_hash_undefweak
-      || h->root.type == bfd_link_hash_defweak)
-    return TRUE;
+  /* ??? What, if anything, needs to happen wrt STV_PROTECTED symbols
+     and relocations that retrieve a function descriptor?  Assume the
+     worst for now.  */
+  if (_bfd_elf_dynamic_symbol_p (h, info, 1))
+    {
+      /* ??? Why is this here and not elsewhere is_local_label_name.  */
+      if (h->root.root.string[0] == '$' && h->root.root.string[1] == '$')
+	return FALSE;
 
-  if (h->root.root.string[0] == '$' && h->root.root.string[1] == '$')
+      return TRUE;
+    }
+  else
     return FALSE;
-
-  if ((info->shared && (!info->symbolic || info->allow_shlib_undefined))
-      || ((h->elf_link_hash_flags
-	   & (ELF_LINK_HASH_DEF_DYNAMIC | ELF_LINK_HASH_REF_REGULAR))
-	  == (ELF_LINK_HASH_DEF_DYNAMIC | ELF_LINK_HASH_REF_REGULAR)))
-    return TRUE;
-
-  return FALSE;
 }
 
 /* Mark all funtions exported by this file so that we can later allocate
Index: elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.36
diff -c -p -d -u -r1.36 elflink.c
--- elflink.c	4 Jul 2003 01:50:11 -0000	1.36
+++ elflink.c	16 Jul 2003 23:35:32 -0000
@@ -2474,3 +2474,63 @@ _bfd_elf_link_sec_merge_syms (h, data)
 
   return TRUE;
 }
+
+/* Returns false if the symbol referred to by H should be considered
+   to resolve local to the current module, and true if it should be
+   considered to bind dynamically.  */
+
+bfd_boolean
+_bfd_elf_dynamic_symbol_p (h, info, ignore_protected)
+     struct elf_link_hash_entry *h;
+     struct bfd_link_info *info;
+     bfd_boolean ignore_protected;
+{
+  bfd_boolean binding_stays_local_p;
+
+  if (h == NULL)
+    return FALSE;
+
+  while (h->root.type == bfd_link_hash_indirect
+	 || h->root.type == bfd_link_hash_warning)
+    h = (struct elf_link_hash_entry *) h->root.u.i.link;
+
+  /* If it was forced local, then clearly it's not dynamic.  */
+  if (h->dynindx == -1)
+    return FALSE;
+  if (h->elf_link_hash_flags & ELF_LINK_FORCED_LOCAL)
+    return FALSE;
+
+  /* Identify the cases where name binding rules say that a
+     visible symbol resolves locally.  */
+  binding_stays_local_p = info->executable || info->symbolic;
+
+  switch (ELF_ST_VISIBILITY (h->other))
+    {
+    case STV_INTERNAL:
+    case STV_HIDDEN:
+      return FALSE;
+
+    case STV_PROTECTED:
+      /* Proper resolution for function pointer equality may require
+	 that these symbols perhaps be resolved dynamically, even though
+	 we should be resolving them to the current module.  */
+      if (!ignore_protected)
+	binding_stays_local_p = TRUE;
+      break;
+
+    default:
+      /* With STV_DEFAULT, weak symbols do not bind locally.  */
+      if (h->root.type == bfd_link_hash_undefweak
+          || h->root.type == bfd_link_hash_defweak)
+	return TRUE;
+      break;
+    }
+
+  /* If it isn't defined locally, then clearly it's dynamic.  */
+  if ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
+    return TRUE;
+
+  /* Otherwise, the symbol is dynamic if binding rules don't tell
+     us that it remains local.  */
+  return !binding_stays_local_p;
+}
Index: elfxx-ia64.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-ia64.c,v
retrieving revision 1.90
diff -c -p -d -u -r1.90 elfxx-ia64.c
--- elfxx-ia64.c	16 Jul 2003 22:48:58 -0000	1.90
+++ elfxx-ia64.c	16 Jul 2003 23:35:32 -0000
@@ -200,7 +200,7 @@ static bfd_boolean elfNN_ia64_modify_seg
 static bfd_boolean elfNN_ia64_is_local_label_name
   PARAMS ((bfd *abfd, const char *name));
 static bfd_boolean elfNN_ia64_dynamic_symbol_p
-  PARAMS ((struct elf_link_hash_entry *h, struct bfd_link_info *info));
+  PARAMS ((struct elf_link_hash_entry *h, struct bfd_link_info *info, int));
 static bfd_boolean elfNN_ia64_local_hash_table_init
   PARAMS ((struct elfNN_ia64_local_hash_table *ht, bfd *abfd,
 	   new_hash_entry_func new));
@@ -831,7 +831,7 @@ elfNN_ia64_relax_section (abfd, sec, lin
 	    }
 
 	  /* Can't do anything else with dynamic symbols.  */
-	  else if (elfNN_ia64_dynamic_symbol_p (h, link_info))
+	  else if (elfNN_ia64_dynamic_symbol_p (h, link_info, r_type))
 	    continue;
 
 	  else
@@ -1508,38 +1508,16 @@ elfNN_ia64_is_local_label_name (abfd, na
 /* Should we do dynamic things to this symbol?  */
 
 static bfd_boolean
-elfNN_ia64_dynamic_symbol_p (h, info)
+elfNN_ia64_dynamic_symbol_p (h, info, r_type)
      struct elf_link_hash_entry *h;
      struct bfd_link_info *info;
+     int r_type;
 {
-  if (h == NULL)
-    return FALSE;
-
-  while (h->root.type == bfd_link_hash_indirect
-	 || h->root.type == bfd_link_hash_warning)
-    h = (struct elf_link_hash_entry *) h->root.u.i.link;
-
-  if (h->dynindx == -1)
-    return FALSE;
-  switch (ELF_ST_VISIBILITY (h->other))
-    {
-    case STV_INTERNAL:
-    case STV_HIDDEN:
-      return FALSE;
-    default:
-      break;
-    }
-
-  if (h->root.type == bfd_link_hash_undefweak
-      || h->root.type == bfd_link_hash_defweak)
-    return TRUE;
-
-  /* If it isn't defined locally, then clearly it's dynamic.  */
-  if ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
-    return TRUE;
+  bfd_boolean ignore_protected
+    = ((r_type & 0xf8) == 0x40		/* FPTR relocs */
+       || (r_type & 0xf8) == 0x50);	/* LTOFF_FPTR relocs */
 
-  /* Identify the cases where name binding rules say it resolves local.  */
-  return !(info->executable || info->symbolic);
+  return _bfd_elf_dynamic_symbol_p (h, info, ignore_protected);
 }
 \f
 static bfd_boolean
@@ -2415,7 +2393,7 @@ allocate_global_data_got (dyn_i, data)
 
   if ((dyn_i->want_got || dyn_i->want_gotx)
       && ! dyn_i->want_fptr
-      && elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info))
+      && elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info, 0))
      {
        dyn_i->got_offset = x->ofs;
        x->ofs += 8;
@@ -2427,7 +2405,7 @@ allocate_global_data_got (dyn_i, data)
     }
   if (dyn_i->want_dtpmod)
     {
-      if (elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info))
+      if (elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info, 0))
 	{
 	  dyn_i->dtpmod_offset = x->ofs;
 	  x->ofs += 8;
@@ -2464,7 +2442,7 @@ allocate_global_fptr_got (dyn_i, data)
 
   if (dyn_i->want_got
       && dyn_i->want_fptr
-      && elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info))
+      && elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info, R_IA64_FPTR64LSB))
     {
       dyn_i->got_offset = x->ofs;
       x->ofs += 8;
@@ -2482,7 +2460,7 @@ allocate_local_got (dyn_i, data)
   struct elfNN_ia64_allocate_data *x = (struct elfNN_ia64_allocate_data *)data;
 
   if ((dyn_i->want_got || dyn_i->want_gotx)
-      && !elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info))
+      && !elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info, 0))
     {
       dyn_i->got_offset = x->ofs;
       x->ofs += 8;
@@ -2576,7 +2554,7 @@ allocate_plt_entries (dyn_i, data)
 	  h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
       /* ??? Versioned symbols seem to lose ELF_LINK_HASH_NEEDS_PLT.  */
-      if (elfNN_ia64_dynamic_symbol_p (h, x->info))
+      if (elfNN_ia64_dynamic_symbol_p (h, x->info, 0))
 	{
 	  bfd_size_type offset = x->ofs;
 	  if (offset == 0)
@@ -2654,7 +2632,10 @@ allocate_dynrel_entries (dyn_i, data)
   bfd_boolean dynamic_symbol, shared, resolved_zero;
 
   ia64_info = elfNN_ia64_hash_table (x->info);
-  dynamic_symbol = elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info);
+
+  /* Note that this can't be used in relation to FPTR relocs below.  */
+  dynamic_symbol = elfNN_ia64_dynamic_symbol_p (dyn_i->h, x->info, 0);
+
   shared = x->info->shared;
   resolved_zero = (dyn_i->h
 		   && ELF_ST_VISIBILITY (dyn_i->h->other)
@@ -3342,7 +3323,7 @@ set_got_entry (abfd, info, dyn_i, dynind
 		|| ELF_ST_VISIBILITY (dyn_i->h->other) == STV_DEFAULT
 		|| dyn_i->h->root.type != bfd_link_hash_undefweak)
 	    && dyn_r_type != R_IA64_DTPREL64LSB)
-           || elfNN_ia64_dynamic_symbol_p (dyn_i->h, info)
+           || elfNN_ia64_dynamic_symbol_p (dyn_i->h, info, dyn_r_type)
 	   || (dynindx != -1 && dyn_r_type == R_IA64_FPTR64LSB))
 	  && (!dyn_i->want_ltoff_fptr
 	      || !info->pie
@@ -3807,7 +3788,6 @@ elfNN_ia64_relocate_section (output_bfd,
       asection *sym_sec;
       bfd_byte *hit_addr;
       bfd_boolean dynamic_symbol_p;
-      bfd_boolean local_symbol_p;
       bfd_boolean undef_weak_ref;
 
       r_type = ELFNN_R_TYPE (rel->r_info);
@@ -3918,12 +3898,7 @@ elfNN_ia64_relocate_section (output_bfd,
 
       hit_addr = contents + rel->r_offset;
       value += rel->r_addend;
-      dynamic_symbol_p = elfNN_ia64_dynamic_symbol_p (h, info);
-      /* Is this symbol locally defined? A protected symbol is locallly
-	 defined. But its function descriptor may not. Use it with
-	 caution.  */
-      local_symbol_p = (! dynamic_symbol_p
-			|| ELF_ST_VISIBILITY (h->other) != STV_DEFAULT);
+      dynamic_symbol_p = elfNN_ia64_dynamic_symbol_p (h, info, r_type);
 
       switch (r_type)
 	{
@@ -3952,7 +3927,7 @@ elfNN_ia64_relocate_section (output_bfd,
 	      /* If we don't need dynamic symbol lookup, find a
 		 matching RELATIVE relocation.  */
 	      dyn_r_type = r_type;
-	      if (! local_symbol_p)
+	      if (! dynamic_symbol_p)
 		{
 		  dynindx = h->dynindx;
 		  addend = rel->r_addend;
@@ -4300,7 +4275,7 @@ elfNN_ia64_relocate_section (output_bfd,
 
 	      /* If we don't need dynamic symbol lookup, install two
 		 RELATIVE relocations.  */
-	      if (local_symbol_p)
+	      if (dynamic_symbol_p)
 		{
 		  unsigned int dyn_r_type;
 

Re: [RFC] unify dynamic_symbol_p implementations

[Alan Modra]

On Wed, Jul 16, 2003 at 04:47:50PM -0700, Richard Henderson wrote:
> 	* elflink.c (_bfd_elf_dynamic_symbol_p): New.
> 	* elf-bfd.h (_bfd_elf_dynamic_symbol_p): Declare it.
> 	(SYMBOL_REFERENCES_LOCAL, SYMBOL_CALLS_LOCAL): Use it.

Looks reasonable to me, except these hunks.

> @@ -3952,7 +3927,7 @@ elfNN_ia64_relocate_section (output_bfd,
>  	      /* If we don't need dynamic symbol lookup, find a
>  		 matching RELATIVE relocation.  */
>  	      dyn_r_type = r_type;
> -	      if (! local_symbol_p)
> +	      if (! dynamic_symbol_p)
>  		{
>  		  dynindx = h->dynindx;
>  		  addend = rel->r_addend;
> @@ -4300,7 +4275,7 @@ elfNN_ia64_relocate_section (output_bfd,
>  
>  	      /* If we don't need dynamic symbol lookup, install two
>  		 RELATIVE relocations.  */
> -	      if (local_symbol_p)
> +	      if (dynamic_symbol_p)
>  		{
>  		  unsigned int dyn_r_type;
>  

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] unify dynamic_symbol_p implementations

[Richard Henderson]

On Thu, Jul 17, 2003 at 10:52:42PM +0930, Alan Modra wrote:
> Looks reasonable to me, except these hunks.

Um, those hunks are exactly the unification of the 
STV_PROTECTED handling vs function descriptors.


r~

Re: [RFC] unify dynamic_symbol_p implementations

[H. J. Lu]

On Thu, Jul 17, 2003 at 09:28:15AM -0700, Richard Henderson wrote:
> On Thu, Jul 17, 2003 at 10:52:42PM +0930, Alan Modra wrote:
> > Looks reasonable to me, except these hunks.
> 
> Um, those hunks are exactly the unification of the 
> STV_PROTECTED handling vs function descriptors.
> 

You changed

  dynamic_symbol_p = elfNN_ia64_dynamic_symbol_p (h, info);
  local_symbol_p = (! dynamic_symbol_p
		    || ELF_ST_VISIBILITY (h->other) != STV_DEFAULT);
  ...
  if (! local_symbol_p)
  ...
  if (local_symbol_p)
  ...

to

  dynamic_symbol_p = elfNN_ia64_dynamic_symbol_p (h, info, r_type);
  ...
  if (! dynamic_symbol_p)
  ...
  if (dynamic_symbol_p)
  ...

For those symbols on which the new elfNN_ia64_dynamic_symbol_p returns
the same as the old one, the code logic is reversed. Is that
intentional?


H.J.

Re: [RFC] unify dynamic_symbol_p implementations

[Hans-Peter Nilsson]

On Thu, 17 Jul 2003, H. J. Lu wrote:
> On Thu, Jul 17, 2003 at 09:28:15AM -0700, Richard Henderson wrote:
> > On Thu, Jul 17, 2003 at 10:52:42PM +0930, Alan Modra wrote:
> > > Looks reasonable to me, except these hunks.
> > Um, those hunks are exactly the unification of the
> > STV_PROTECTED handling vs function descriptors.
> You changed
 [seemingly obvious functional change for ia64]
> For those symbols on which the new elfNN_ia64_dynamic_symbol_p returns
> the same as the old one, the code logic is reversed. Is that
> intentional?

Hey, if it had been wrong, it surely would have been caught in
some ia64 ld test-case! 8-)

brgds, H-P

Re: [RFC] unify dynamic_symbol_p implementations

[Richard Henderson]

On Thu, Jul 17, 2003 at 05:18:07PM -0400, Hans-Peter Nilsson wrote:
> Hey, if it had been wrong, it surely would have been caught in
> some ia64 ld test-case! 8-)

Feh.  Actually, it didn't.  :-(


r~

Re: [RFC] unify dynamic_symbol_p implementations

[Bob Wilson]

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

On Friday 18 July 2003 03:21 pm, Richard Henderson wrote:
> On Fri, Jul 18, 2003 at 03:16:12PM -0700, Bob Wilson wrote:
> > Right.  We always set -fpic for Xtensa/Linux.
>
> Ah, ok.  In which case you shouldn't have any problems.
> I've already committed the patch (with ia64 fixed), so
> you should just do the s/0/1/ and update the commentary.

OK.  While I was at it, I made xtensa_elf_dynamic_symbol_p an inline function 
and swapped the order of its arguments to match _bfd_elf_dynamic_symbol_p.  I 
committed the following patch on the mainline.

2003-07-23  Bob Wilson  <bob.wilson@acm.org>

        * elf32-xtensa.c (xtensa_elf_dynamic_symbol_p): Make inline and move
        near the beginning of the file.  Swap order of arguments.  Call
        _bfd_elf_dynamic_symbol_p with "ignore_protected" set to 0.
        (elf_xtensa_fix_refcounts): Adjust xtensa_elf_dynamic_symbol_p call.
        (elf_xtensa_relocate_section): Likewise.
        (shrink_dynamic_reloc_sections): Likewise.


[-- Attachment #2: xt-dynsym.diff --]
[-- Type: text/x-diff, Size: 3419 bytes --]

Index: elf32-xtensa.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-xtensa.c,v
retrieving revision 1.7
diff -u -p -r1.7 elf32-xtensa.c
--- elf32-xtensa.c	24 Jul 2003 00:13:42 -0000	1.7
+++ elf32-xtensa.c	24 Jul 2003 00:14:08 -0000
@@ -102,6 +102,8 @@ static bfd_boolean elf_xtensa_new_sectio
 
 /* Local helper functions.  */
 
+static bfd_boolean xtensa_elf_dynamic_symbol_p
+  PARAMS ((struct elf_link_hash_entry *, struct bfd_link_info *));
 static int property_table_compare
   PARAMS ((const PTR, const PTR));
 static bfd_boolean elf_xtensa_in_literal_pool
@@ -131,8 +133,6 @@ static void do_fix_for_relocatable_link
   PARAMS ((Elf_Internal_Rela *, bfd *, asection *));
 static void do_fix_for_final_link
   PARAMS ((Elf_Internal_Rela *, asection *, bfd_vma *));
-static bfd_boolean xtensa_elf_dynamic_symbol_p
-  PARAMS ((struct bfd_link_info *, struct elf_link_hash_entry *));
 static bfd_vma elf_xtensa_create_plt_entry
   PARAMS ((bfd *, bfd *, unsigned));
 static int elf_xtensa_combine_prop_entries
@@ -449,6 +449,21 @@ static const bfd_byte elf_xtensa_le_plt_
   0			/* unused */
 };
 
+
+static inline bfd_boolean
+xtensa_elf_dynamic_symbol_p (h, info)
+     struct elf_link_hash_entry *h;
+     struct bfd_link_info *info;
+{
+  /* Check if we should do dynamic things to this symbol.  The
+     "ignore_protected" argument need not be set, because Xtensa code
+     does not require special handling of STV_PROTECTED to make function
+     pointer comparisons work properly.  The PLT addresses are never
+     used for function pointers.  */
+
+  return _bfd_elf_dynamic_symbol_p (h, info, 0);
+}
+
 \f
 static int
 property_table_compare (ap, bp)
@@ -1063,7 +1078,7 @@ elf_xtensa_fix_refcounts (h, arg)
   if (h->root.type == bfd_link_hash_warning)
     h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
-  if (! xtensa_elf_dynamic_symbol_p (info, h))
+  if (! xtensa_elf_dynamic_symbol_p (h, info))
     elf_xtensa_make_sym_local (info, h);
 
   /* If the symbol has a relocation outside the GOT, set the
@@ -1830,17 +1845,6 @@ elf_xtensa_create_plt_entry (dynobj, out
 }
 
 
-static bfd_boolean
-xtensa_elf_dynamic_symbol_p (info, h)
-     struct bfd_link_info *info;
-     struct elf_link_hash_entry *h;
-{
-  /* ??? What, if anything, needs to happen wrt STV_PROTECTED and PLT
-     entries?  For now assume the worst.  */
-  return _bfd_elf_dynamic_symbol_p (h, info, 1);
-}
-
-
 /* Relocate an Xtensa ELF section.  This is invoked by the linker for
    both relocatable and final links.  */
 
@@ -2067,7 +2071,7 @@ elf_xtensa_relocate_section (output_bfd,
       /* Generate dynamic relocations.  */
       if (elf_hash_table (info)->dynamic_sections_created)
 	{
-	  bfd_boolean dynamic_symbol = xtensa_elf_dynamic_symbol_p (info, h);
+	  bfd_boolean dynamic_symbol = xtensa_elf_dynamic_symbol_p (h, info);
 
 	  if (dynamic_symbol && (r_type == R_XTENSA_OP0
 				 || r_type == R_XTENSA_OP1
@@ -4963,7 +4967,7 @@ shrink_dynamic_reloc_sections (info, abf
   else
     h = sym_hashes[r_symndx - symtab_hdr->sh_info];
 
-  dynamic_symbol = xtensa_elf_dynamic_symbol_p (info, h);
+  dynamic_symbol = xtensa_elf_dynamic_symbol_p (h, info);
 
   if ((r_type == R_XTENSA_32 || r_type == R_XTENSA_PLT)
       && (input_section->flags & SEC_ALLOC) != 0

Re: [RFC] unify dynamic_symbol_p implementations

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Alan Modra wrote:

> Has someone written up a draft psABI that defines weak symbol behaviour?
> Or better, a draft gABI?  gABI on www.caldera.com is still 24 April 2001.

No, and I doubt it'll ever happen.  There is too much diversion.  Weak
symbol are, and always have been, for the implementation only.  Only the
system libraries may use them and they are tightly coupled to the
runtime linker.


> Also, if ld follows the new semantics, which versions of glibc will no
> longer build?  Not that this should deter us from changing ld, but we'd
> need to mention in release notes that older versions of glibc require
> older versions of binutils.

I don't think there is any problem with not building.  Using new lds
with old glibcs might be a problem but I have no concrete examples since
I don't know the ld changes.

- -- 
- --------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE/HLEZ2ijCOnn/RHQRAvlVAKCgzqgVAQRVjTrOh5fUD/+p/9JSdACgv/FP
DU9jBd0IAPltipOIMdYrpcs=
=iGT7
-----END PGP SIGNATURE-----

Re: [RFC] unify dynamic_symbol_p implementations

[Alan Modra]

On Mon, Jul 21, 2003 at 04:25:34PM -0700, Ulrich Drepper wrote:
> H. J. Lu wrote:
> > There were different interpretations of how weak symbols should work
> > and we discussed it at the ia64 psABI meeting. The conclusion was
> > glibc was wrong.
> 
> glibc was not wrong (please is impossible given the behavior is
> unspecified).  But coming to a common, reasonable semantics made more
> sense with glibc's behavior changing (Irix would be effected, too, if
> they would make such a change).

Has someone written up a draft psABI that defines weak symbol behaviour?
Or better, a draft gABI?  gABI on www.caldera.com is still 24 April 2001.

Also, if ld follows the new semantics, which versions of glibc will no
longer build?  Not that this should deter us from changing ld, but we'd
need to mention in release notes that older versions of glibc require
older versions of binutils.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] unify dynamic_symbol_p implementations

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

H. J. Lu wrote:

> There were different interpretations of how weak symbols should work
> and we discussed it at the ia64 psABI meeting. The conclusion was
> glibc was wrong.

glibc was not wrong (please is impossible given the behavior is
unspecified).  But coming to a common, reasonable semantics made more
sense with glibc's behavior changing (Irix would be effected, too, if
they would make such a change).

- -- 
- --------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE/HHZx2ijCOnn/RHQRAgIMAJ9Cb81/pKQJvtBxOMTWVxQLcFMZoACfcIJ7
r1ZmWUR+o/51v4/gDMkEZGE=
=ah0f
-----END PGP SIGNATURE-----

Re: [RFC] unify dynamic_symbol_p implementations

[H. J. Lu]

On Mon, Jul 21, 2003 at 01:48:35PM -0700, Richard Henderson wrote:
> On Mon, Jul 21, 2003 at 12:28:08PM -0700, H. J. Lu wrote:
> > A weak symbol in executable shouldn't be overridden unless there is a
> > definition in a shared library at link time.
> 
> Who says?  IIRC this has varied between glibc versions.
> 

There were different interpretations of how weak symbols should work
and we discussed it at the ia64 psABI meeting. The conclusion was
glibc was wrong. I added some weak symbol tests into binutils and
marked them XFAIL. This glibc bug has been fixed and I removed those
XFAILs from binutils tests.


H.J.

Re: [RFC] unify dynamic_symbol_p implementations

[Richard Henderson]

On Mon, Jul 21, 2003 at 12:28:08PM -0700, H. J. Lu wrote:
> A weak symbol in executable shouldn't be overridden unless there is a
> definition in a shared library at link time.

Who says?  IIRC this has varied between glibc versions.


r~

Re: [RFC] unify dynamic_symbol_p implementations

[H. J. Lu]

On Mon, Jul 21, 2003 at 12:43:51AM +0930, Alan Modra wrote:
> 
> One interesting place that this differs from (the inverse of) Richard's
> function is in the treatment of weak and undefweak dynamic syms.  I
> suspect I should allow a weak dynamic sym in an executable to be
> overridden, even if we don't find a strong sym in a shared lib supplied
> at link time.  Comments?
> 

A weak symbol in executable shouldn't be overridden unless there is a
definition in a shared library at link time. I am testing this patch
on ia64. It has passed "make check". I will build gcc and glibc with
the new linker.


H.J.
---
2003-07-21  H.J. Lu <hongjiu.lu@intel.com>

	* elflink.c (_bfd_elf_dynamic_symbol_p): Don't return TRUE for
	weak symbols.

--- bfd/elflink.c.weak	2003-07-21 07:34:13.000000000 -0700
+++ bfd/elflink.c	2003-07-21 12:17:03.000000000 -0700
@@ -2519,10 +2519,6 @@ _bfd_elf_dynamic_symbol_p (h, info, igno
       break;
 
     default:
-      /* With STV_DEFAULT, weak symbols do not bind locally.  */
-      if (h->root.type == bfd_link_hash_undefweak
-          || h->root.type == bfd_link_hash_defweak)
-	return TRUE;
       break;
     }
 

Re: [RFC] unify dynamic_symbol_p implementations

[H. J. Lu]

On Mon, Jul 21, 2003 at 09:43:48AM +0930, Alan Modra wrote:
> On Sun, Jul 20, 2003 at 08:26:43AM -0700, H. J. Lu wrote:
> > Weak defined symbols are purely for link time and only meaningful for
> > relocatable inputs.
> 
> Hmm, OK.  Seems a little odd that we make weak undef dynamic then.

Weak undef can be dynamic if there is a definition from a shared library. It
is just weak defined should be treated as strong when a competing definition
comes from a shared library.

> 
> I'm going to install my patch so that x86 and ppc work again.  It's
> likely that ia64 could use _bfd_elf_symbol_refs_local_p instead of
> _bfd_elf_dynamic_symbol_p, but I don't really have the time right
> now to figure out what needs doing.
> 

I will check out ia64 tomorrow.


H.J.

Re: [RFC] unify dynamic_symbol_p implementations

[Alan Modra]

On Sun, Jul 20, 2003 at 08:26:43AM -0700, H. J. Lu wrote:
> Weak defined symbols are purely for link time and only meaningful for
> relocatable inputs.

Hmm, OK.  Seems a little odd that we make weak undef dynamic then.

I'm going to install my patch so that x86 and ppc work again.  It's
likely that ia64 could use _bfd_elf_symbol_refs_local_p instead of
_bfd_elf_dynamic_symbol_p, but I don't really have the time right
now to figure out what needs doing.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] unify dynamic_symbol_p implementations

[H. J. Lu]

On Mon, Jul 21, 2003 at 12:43:51AM +0930, Alan Modra wrote:
> On Fri, Jul 18, 2003 at 11:21:06PM -0700, H. J. Lu wrote:
> > That is not the problem. _bfd_elf_dynamic_symbol_p came from
> > elfxx-ia64.c, which isn't symmetric with SYMBOL_REFERENCES_LOCAL.
> > I don't know how we can get it both ways.
> 
> Yes, I see I agreed with Richard's patch too quickly..  The problem is
> that SYMBOL_REFERENCES_LOCAL and SYMBOL_CALLS_LOCAL as used by ppc,
> ppc64, x86, and x86_64 should only return true if we have a local
> definition.  Causes failures in ld tests of visibility (hidden_undef).
> I do like the idea of a predicate function though.
> 
> 	* elf-bfd.h (SYMBOL_REFERENCES_LOCAL, SYMBOL_CALLS_LOCAL): Use..
> 	(_bfd_elf_symbol_refs_local_p): ..this.  Declare.
> 	* elflink.c (_bfd_elf_symbol_refs_local_p): New function.
> 	* elf32-i386.c (elf_i386_relocate_section): Remove h NULL test
> 	now done in _bfd_elf_symbol_refs_local_p.
> 	* elf32-ppc.c (ppc_elf_relocate_section): Likewise.
> 	* elf64-ppc.c (ppc64_elf_relocate_section): Likewise.
> 	* elf64-x86-64.c (elf64_x86_64_relocate_section): Likewise.
> 
> One interesting place that this differs from (the inverse of) Richard's
> function is in the treatment of weak and undefweak dynamic syms.  I
> suspect I should allow a weak dynamic sym in an executable to be
> overridden, even if we don't find a strong sym in a shared lib supplied
> at link time.  Comments?

Weak defined symbols are purely for link time and only meaningful for
relocatable inputs. Any defintions from shared libraries are ignored
if there is a defintion in relocatable inputs, weak or strong. As runtime,
they are as good as strong. I added some testcases for them. Please do
"make check" on x86 with glibc 2.3 from CVS. If you do not get any
weak symbol check failures, you should be ok.


H.J.

Re: [RFC] unify dynamic_symbol_p implementations

[Alan Modra]

On Fri, Jul 18, 2003 at 11:21:06PM -0700, H. J. Lu wrote:
> That is not the problem. _bfd_elf_dynamic_symbol_p came from
> elfxx-ia64.c, which isn't symmetric with SYMBOL_REFERENCES_LOCAL.
> I don't know how we can get it both ways.

Yes, I see I agreed with Richard's patch too quickly..  The problem is
that SYMBOL_REFERENCES_LOCAL and SYMBOL_CALLS_LOCAL as used by ppc,
ppc64, x86, and x86_64 should only return true if we have a local
definition.  Causes failures in ld tests of visibility (hidden_undef).
I do like the idea of a predicate function though.

	* elf-bfd.h (SYMBOL_REFERENCES_LOCAL, SYMBOL_CALLS_LOCAL): Use..
	(_bfd_elf_symbol_refs_local_p): ..this.  Declare.
	* elflink.c (_bfd_elf_symbol_refs_local_p): New function.
	* elf32-i386.c (elf_i386_relocate_section): Remove h NULL test
	now done in _bfd_elf_symbol_refs_local_p.
	* elf32-ppc.c (ppc_elf_relocate_section): Likewise.
	* elf64-ppc.c (ppc64_elf_relocate_section): Likewise.
	* elf64-x86-64.c (elf64_x86_64_relocate_section): Likewise.

One interesting place that this differs from (the inverse of) Richard's
function is in the treatment of weak and undefweak dynamic syms.  I
suspect I should allow a weak dynamic sym in an executable to be
overridden, even if we don't find a strong sym in a shared lib supplied
at link time.  Comments?

Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.105
diff -u -p -r1.105 elf-bfd.h
--- bfd/elf-bfd.h	19 Jul 2003 04:12:22 -0000	1.105
+++ bfd/elf-bfd.h	20 Jul 2003 13:45:36 -0000
@@ -212,11 +212,11 @@ struct elf_link_hash_entry
    it's necessary for shared libs to also reference the .plt even
    though the symbol is really local to the shared lib.  */
 #define SYMBOL_REFERENCES_LOCAL(INFO, H) \
-  !_bfd_elf_dynamic_symbol_p (H, INFO, 1)
+  _bfd_elf_symbol_refs_local_p (H, INFO, 0)
 
 /* Will _calls_ to this symbol always call the version in this object?  */
 #define SYMBOL_CALLS_LOCAL(INFO, H) \
-  !_bfd_elf_dynamic_symbol_p (H, INFO, 0)
+  _bfd_elf_symbol_refs_local_p (H, INFO, 1)
 
 /* Records local symbols to be emitted in the dynamic symbol table.  */
 
@@ -1496,6 +1496,9 @@ extern bfd_boolean _bfd_elf_link_sec_mer
   PARAMS ((struct elf_link_hash_entry *, PTR));
 
 extern bfd_boolean _bfd_elf_dynamic_symbol_p
+  PARAMS ((struct elf_link_hash_entry *, struct bfd_link_info *, bfd_boolean));
+
+extern bfd_boolean _bfd_elf_symbol_refs_local_p
   PARAMS ((struct elf_link_hash_entry *, struct bfd_link_info *, bfd_boolean));
 
 extern const bfd_target *bfd_elf32_object_p
Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.39
diff -u -p -r1.39 elflink.c
--- bfd/elflink.c	19 Jul 2003 06:41:42 -0000	1.39
+++ bfd/elflink.c	20 Jul 2003 13:45:46 -0000
@@ -2534,3 +2534,52 @@ _bfd_elf_dynamic_symbol_p (h, info, igno
      us that it remains local.  */
   return !binding_stays_local_p;
 }
+
+/* Return true if the symbol referred to by H should be considered
+   to resolve local to the current module, and false otherwise.  Differs
+   from (the inverse of) _bfd_elf_dynamic_symbol_p in the treatment of
+   undefined symbols and weak symbols.  */
+
+bfd_boolean
+_bfd_elf_symbol_refs_local_p (h, info, local_protected)
+     struct elf_link_hash_entry *h;
+     struct bfd_link_info *info;
+     bfd_boolean local_protected;
+{
+  /* If it's a local sym, of course we resolve locally.  */
+  if (h == NULL)
+    return TRUE;
+
+  /* If we don't have a definition in a regular file, then we can't
+     resolve locally.  The sym is either undefined or dynamic.  */
+  if ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
+    return FALSE;
+
+  /* Forced local symbols resolve locally.  */
+  if ((h->elf_link_hash_flags & ELF_LINK_FORCED_LOCAL) != 0)
+    return TRUE;
+
+  /* As do non-dynamic symbols.  */
+  if (h->dynindx == -1)
+    return TRUE;
+
+  /* At this point, we know the symbol is defined and dynamic.  In an
+     executable it must resolve locally, likewise when building symbolic
+     shared libraries.  */
+  if (info->executable || info->symbolic)
+    return TRUE;
+
+  /* Now deal with defined dynamic symbols in shared libraries.  Ones
+     with default visibility might not resolve locally.  */
+  if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
+    return FALSE;
+
+  /* However, STV_HIDDEN or STV_INTERNAL ones must be local.  */
+  if (ELF_ST_VISIBILITY (h->other) != STV_PROTECTED)
+    return TRUE;
+
+  /* Function pointer equality tests may require that STV_PROTECTED
+     symbols be treated as dynamic symbols, even when we know that the
+     dynamic linker will resolve them locally.  */
+  return local_protected;
+}
Index: bfd/elf32-i386.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-i386.c,v
retrieving revision 1.107
diff -u -p -r1.107 elf32-i386.c
--- bfd/elf32-i386.c	4 Jul 2003 13:53:37 -0000	1.107
+++ bfd/elf32-i386.c	20 Jul 2003 13:45:37 -0000
@@ -2338,8 +2338,7 @@ elf_i386_relocate_section (bfd *output_b
 		   || ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
 		   || h->root.type != bfd_link_hash_undefweak)
 	       && (r_type != R_386_PC32
-		   || (h != NULL
-		       && !SYMBOL_CALLS_LOCAL (info, h))))
+		   || !SYMBOL_CALLS_LOCAL (info, h)))
 	      || (ELIMINATE_COPY_RELOCS
 		  && !info->shared
 		  && h != NULL
Index: bfd/elf32-ppc.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-ppc.c,v
retrieving revision 1.93
diff -u -p -r1.93 elf32-ppc.c
--- bfd/elf32-ppc.c	12 Jul 2003 11:16:46 -0000	1.93
+++ bfd/elf32-ppc.c	20 Jul 2003 13:45:39 -0000
@@ -5175,9 +5175,8 @@ ppc_elf_relocate_section (bfd *output_bf
 	case R_PPC_REL14_BRNTAKEN:
 	  /* If these relocations are not to a named symbol, they can be
 	     handled right here, no need to bother the dynamic linker.  */
-	  if (h == NULL
-	      || strcmp (h->root.root.string, "_GLOBAL_OFFSET_TABLE_") == 0
-	      || SYMBOL_REFERENCES_LOCAL (info, h))
+	  if (SYMBOL_REFERENCES_LOCAL (info, h)
+	      || strcmp (h->root.root.string, "_GLOBAL_OFFSET_TABLE_") == 0)
 	    break;
 	  /* fall through */
 
@@ -5207,8 +5206,7 @@ ppc_elf_relocate_section (bfd *output_bf
 		   || ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
 		   || h->root.type != bfd_link_hash_undefweak)
 	       && (MUST_BE_DYN_RELOC (r_type)
-		   || (h != NULL
-		       && !SYMBOL_CALLS_LOCAL (info, h))))
+		   || !SYMBOL_CALLS_LOCAL (info, h)))
 	      || (ELIMINATE_COPY_RELOCS
 		  && !info->shared
 		  && (input_section->flags & SEC_ALLOC) != 0
@@ -5263,8 +5261,7 @@ ppc_elf_relocate_section (bfd *output_bf
 
 	      if (skip)
 		memset (&outrel, 0, sizeof outrel);
-	      else if (h != NULL
-		       && !SYMBOL_REFERENCES_LOCAL (info, h))
+	      else if (!SYMBOL_REFERENCES_LOCAL (info, h))
 		{
 		  unresolved_reloc = FALSE;
 		  outrel.r_info = ELF32_R_INFO (h->dynindx, r_type);
Index: bfd/elf64-ppc.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-ppc.c,v
retrieving revision 1.118
diff -u -p -r1.118 elf64-ppc.c
--- bfd/elf64-ppc.c	10 Jul 2003 00:37:26 -0000	1.118
+++ bfd/elf64-ppc.c	20 Jul 2003 13:45:43 -0000
@@ -8159,8 +8159,7 @@ ppc64_elf_relocate_section (bfd *output_
 		   || ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
 		   || h->root.type != bfd_link_hash_undefweak)
 	       && (MUST_BE_DYN_RELOC (r_type)
-		   || (h != NULL
-		       && !SYMBOL_CALLS_LOCAL (info, h))))
+		   || !SYMBOL_CALLS_LOCAL (info, h)))
 	      || (ELIMINATE_COPY_RELOCS
 		  && !info->shared
 		  && h != NULL
@@ -8206,8 +8205,7 @@ ppc64_elf_relocate_section (bfd *output_
 
 	      if (skip)
 		memset (&outrel, 0, sizeof outrel);
-	      else if (h != NULL
-		       && !SYMBOL_REFERENCES_LOCAL (info, h)
+	      else if (!SYMBOL_REFERENCES_LOCAL (info, h)
 		       && !is_opd
 		       && r_type != R_PPC64_TOC)
 		outrel.r_info = ELF64_R_INFO (h->dynindx, r_type);
Index: bfd/elf64-x86-64.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-x86-64.c,v
retrieving revision 1.63
diff -u -p -r1.63 elf64-x86-64.c
--- bfd/elf64-x86-64.c	4 Jul 2003 13:53:37 -0000	1.63
+++ bfd/elf64-x86-64.c	20 Jul 2003 13:45:45 -0000
@@ -2015,8 +2015,7 @@ elf64_x86_64_relocate_section (bfd *outp
 	       && ((r_type != R_X86_64_PC8
 		    && r_type != R_X86_64_PC16
 		    && r_type != R_X86_64_PC32)
-		   || (h != NULL
-		       && !SYMBOL_CALLS_LOCAL (info, h))))
+		   || !SYMBOL_CALLS_LOCAL (info, h)))
 	      || (ELIMINATE_COPY_RELOCS
 		  && !info->shared
 		  && h != NULL

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] unify dynamic_symbol_p implementations

[H. J. Lu]

On Sat, Jul 19, 2003 at 03:27:25PM +0930, Alan Modra wrote:
> 
> > 	* elflink.c (_bfd_elf_dynamic_symbol_p): Return TRUE
> > 	immediately if symbol isn't defined locally.
> 
> This isn't so obvious, and I think wrong.  You do happen to be restoring
> more or less the original SYMBOL_REFERENCES_LOCAL behaviour, but I think
> the idea is to move the weak undef handling into
> _bfd_elf_dynamic_symbol_p.
> 

That is not the problem. _bfd_elf_dynamic_symbol_p came from
elfxx-ia64.c, which isn't symmetric with SYMBOL_REFERENCES_LOCAL.
I don't know how we can get it both ways.


H.J.

Re: [RFC] unify dynamic_symbol_p implementations

[Alan Modra]

On Fri, Jul 18, 2003 at 09:13:30PM -0700, H. J. Lu wrote:
> I checked in the following patch to fix it.
> 
> 	* elf-bfd.h (SYMBOL_REFERENCES_LOCAL): Fix a typo.
> 	(SYMBOL_CALLS_LOCAL): Likewise.

Obvious change.

> 	* elflink.c (_bfd_elf_dynamic_symbol_p): Return TRUE
> 	immediately if symbol isn't defined locally.

This isn't so obvious, and I think wrong.  You do happen to be restoring
more or less the original SYMBOL_REFERENCES_LOCAL behaviour, but I think
the idea is to move the weak undef handling into
_bfd_elf_dynamic_symbol_p.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [RFC] unify dynamic_symbol_p implementations

[H. J. Lu]

On Fri, Jul 18, 2003 at 09:13:30PM -0700, H. J. Lu wrote:
> On Fri, Jul 18, 2003 at 07:46:42PM -0700, H. J. Lu wrote:
> > On Fri, Jul 18, 2003 at 03:21:50PM -0700, Richard Henderson wrote:
> > > On Fri, Jul 18, 2003 at 03:16:12PM -0700, Bob Wilson wrote:
> > > > Right.  We always set -fpic for Xtensa/Linux.
> > > 
> > > Ah, ok.  In which case you shouldn't have any problems.
> > > I've already committed the patch (with ia64 fixed), so
> > > you should just do the s/0/1/ and update the commentary.
> > > 
> > 
> > I got so many "make check" failures like
> > 
> > /export/build/linux/binutils-debug/build-i686-linux/ld/ld-new:
> > /usr/lib/crt1.o(.text+0x1d): unresolvable relocation against symbol
> > `__libc_start_main@@GLIBC_2.0'
> > FAIL: bootstrap
> > 
> > on RedHat 9. Did "make check" work for you on RedHat 9?
> > 
> > 
> 
> I checked in the following patch to fix it.
> 
> 

Linux/ia64 is still broken. I got

+FAIL: bootstrap with --static
+FAIL: visibility (hidden_weak)
+FAIL: visibility (hidden_weak) (PIC main)
+FAIL: visibility (protected_weak)
+FAIL: visibility (protected_weak) (PIC main)
+FAIL: common hidden symbol
+FAIL: weak hidden symbol DSO first
+FAIL: undefined
+FAIL: undefined function
+FAIL: undefined line
+FAIL: weak undefined symbols

from "make check" in ld on RedHat AS 2.1.


H.J.

Re: [RFC] unify dynamic_symbol_p implementations

[H. J. Lu]

On Fri, Jul 18, 2003 at 07:46:42PM -0700, H. J. Lu wrote:
> On Fri, Jul 18, 2003 at 03:21:50PM -0700, Richard Henderson wrote:
> > On Fri, Jul 18, 2003 at 03:16:12PM -0700, Bob Wilson wrote:
> > > Right.  We always set -fpic for Xtensa/Linux.
> > 
> > Ah, ok.  In which case you shouldn't have any problems.
> > I've already committed the patch (with ia64 fixed), so
> > you should just do the s/0/1/ and update the commentary.
> > 
> 
> I got so many "make check" failures like
> 
> /export/build/linux/binutils-debug/build-i686-linux/ld/ld-new:
> /usr/lib/crt1.o(.text+0x1d): unresolvable relocation against symbol
> `__libc_start_main@@GLIBC_2.0'
> FAIL: bootstrap
> 
> on RedHat 9. Did "make check" work for you on RedHat 9?
> 
> 

I checked in the following patch to fix it.


H.J.
--
2003-07-18  H.J. Lu <hongjiu.lu@intel.com>

	* elf-bfd.h (SYMBOL_REFERENCES_LOCAL): Fix a typo.
	(SYMBOL_CALLS_LOCAL): Likewise.

	* elflink.c (_bfd_elf_dynamic_symbol_p): Return TRUE
	immediately if symbol isn't defined locally.

--- bfd/elf-bfd.h.local	2003-07-18 18:55:11.000000000 -0700
+++ bfd/elf-bfd.h	2003-07-18 21:06:30.000000000 -0700
@@ -212,11 +212,11 @@ struct elf_link_hash_entry
    it's necessary for shared libs to also reference the .plt even
    though the symbol is really local to the shared lib.  */
 #define SYMBOL_REFERENCES_LOCAL(INFO, H) \
-  _bfd_elf_dynamic_symbol_p (H, INFO, 1)
+  !_bfd_elf_dynamic_symbol_p (H, INFO, 1)
 
 /* Will _calls_ to this symbol always call the version in this object?  */
 #define SYMBOL_CALLS_LOCAL(INFO, H) \
-  _bfd_elf_dynamic_symbol_p (H, INFO, 0)
+  !_bfd_elf_dynamic_symbol_p (H, INFO, 0)
 
 /* Records local symbols to be emitted in the dynamic symbol table.  */
 
--- bfd/elflink.c.local	2003-07-18 18:52:24.000000000 -0700
+++ bfd/elflink.c	2003-07-18 21:05:40.000000000 -0700
@@ -2494,6 +2494,10 @@ _bfd_elf_dynamic_symbol_p (h, info, igno
 	 || h->root.type == bfd_link_hash_warning)
     h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
+  /* If it isn't defined locally, then clearly it's dynamic.  */
+  if ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
+    return TRUE;
+
   /* If it was forced local, then clearly it's not dynamic.  */
   if (h->dynindx == -1)
     return FALSE;
@@ -2526,10 +2530,6 @@ _bfd_elf_dynamic_symbol_p (h, info, igno
       break;
     }
 
-  /* If it isn't defined locally, then clearly it's dynamic.  */
-  if ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
-    return TRUE;
-
   /* Otherwise, the symbol is dynamic if binding rules don't tell
      us that it remains local.  */
   return !binding_stays_local_p;

Re: [RFC] unify dynamic_symbol_p implementations

[H. J. Lu]

On Fri, Jul 18, 2003 at 03:21:50PM -0700, Richard Henderson wrote:
> On Fri, Jul 18, 2003 at 03:16:12PM -0700, Bob Wilson wrote:
> > Right.  We always set -fpic for Xtensa/Linux.
> 
> Ah, ok.  In which case you shouldn't have any problems.
> I've already committed the patch (with ia64 fixed), so
> you should just do the s/0/1/ and update the commentary.
> 

I got so many "make check" failures like

/export/build/linux/binutils-debug/build-i686-linux/ld/ld-new:
/usr/lib/crt1.o(.text+0x1d): unresolvable relocation against symbol
`__libc_start_main@@GLIBC_2.0'
FAIL: bootstrap

on RedHat 9. Did "make check" work for you on RedHat 9?


H.J.

Re: [RFC] unify dynamic_symbol_p implementations

[Richard Henderson]

On Fri, Jul 18, 2003 at 03:16:12PM -0700, Bob Wilson wrote:
> Right.  We always set -fpic for Xtensa/Linux.

Ah, ok.  In which case you shouldn't have any problems.
I've already committed the patch (with ia64 fixed), so
you should just do the s/0/1/ and update the commentary.


r~

Re: [RFC] unify dynamic_symbol_p implementations

[Bob Wilson]

On Friday 18 July 2003 15:04, Daniel Jacobowitz wrote:
> On Fri, Jul 18, 2003 at 03:02:39PM -0700, Richard Henderson wrote:
> > Um, unless I'm missing something, this means you compiled the
> > test case with -fpic or such.  I want the main executable.
>
> I'm pretty sure Xtensa, like MIPS/Linux, defaults to PIC
> applications...

Right.  We always set -fpic for Xtensa/Linux.  There are only minor code 
differences for PIC vs. non-PIC on Xtensa targets, so we just set it all the 
time.  I don't understand this well enough to be confident that it works in 
all cases.  If you still think there might be issues, I'd welcome any more 
suggestions of issues to investigate.

Re: [RFC] unify dynamic_symbol_p implementations

[Daniel Jacobowitz]

On Fri, Jul 18, 2003 at 03:02:39PM -0700, Richard Henderson wrote:
> On Fri, Jul 18, 2003 at 02:18:59PM -0700, Bob Wilson wrote:
> > Maybe it is still OK, though.  It doesn't end up in .rodata.  GCC's 
> > default_elf_select_rtx_section() function in varasm.c puts it in 
> > .data.rel.ro.
> 
> Um, unless I'm missing something, this means you compiled the
> test case with -fpic or such.  I want the main executable.

I'm pretty sure Xtensa, like MIPS/Linux, defaults to PIC
applications...

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC] unify dynamic_symbol_p implementations

[Richard Henderson]

On Fri, Jul 18, 2003 at 02:18:59PM -0700, Bob Wilson wrote:
> Maybe it is still OK, though.  It doesn't end up in .rodata.  GCC's 
> default_elf_select_rtx_section() function in varasm.c puts it in 
> .data.rel.ro.

Um, unless I'm missing something, this means you compiled the
test case with -fpic or such.  I want the main executable.


r~

Re: [RFC] unify dynamic_symbol_p implementations

[Bob Wilson]

Yup, I was missing something :-)

Maybe it is still OK, though.  It doesn't end up in .rodata.  GCC's 
default_elf_select_rtx_section() function in varasm.c puts it in 
.data.rel.ro.  I had never even heard of this section before, but it seems to 
exist for something like this case.

Help?

On Friday 18 July 2003 13:56, Richard Henderson wrote:
> On Fri, Jul 18, 2003 at 01:09:14PM -0700, Bob Wilson wrote:
> > The R_XTENSA_32 reloc used for the function address turns into an
> > R_XTENSA_GLOB_DAT reloc in the executable.  The runtime linker
> > resolves this reloc at startup.  Am I missing something?
>
> Really?  Even with
>
> 	extern void foo(void);
> 	static void (* const bar) (void) = foo;
>
> Doesn't bar wind up in .rodata?  And if so, it really shouldn't
> have a dynamic relocation against it.

Re: [RFC] unify dynamic_symbol_p implementations

[Richard Henderson]

On Fri, Jul 18, 2003 at 01:09:14PM -0700, Bob Wilson wrote:
> The R_XTENSA_32 reloc used for the function address turns into an 
> R_XTENSA_GLOB_DAT reloc in the executable.  The runtime linker
> resolves this reloc at startup.  Am I missing something?

Really?  Even with

	extern void foo(void);
	static void (* const bar) (void) = foo;

Doesn't bar wind up in .rodata?  And if so, it really shouldn't
have a dynamic relocation against it.


r~

Re: [RFC] unify dynamic_symbol_p implementations

[Bob Wilson]

On Friday 18 July 2003 12:57, Richard Henderson wrote:
> On Fri, Jul 18, 2003 at 12:02:40PM -0700, Bob Wilson wrote:
> > I think the "ignore_protected" argument should be 0 for the Xtensa port. 
> > A separate relocation (R_XTENSA_32) is used when taking the address of a
> > function than when calling it (R_XTENSA_PLT), so there shouldn't be an
> > issue with incorrectly comparing the PLT addresses instead of the
> > function addresses.  Unless I'm missing something here, you shouldn't
> > have to "assume the worst".
>
> What happens when an executable takes the address of a
> function defined in a shared library?

The R_XTENSA_32 reloc used for the function address turns into an 
R_XTENSA_GLOB_DAT reloc in the executable.  The runtime linker resolves this 
reloc at startup.  Am I missing something?

Re: [RFC] unify dynamic_symbol_p implementations

[Richard Henderson]

On Fri, Jul 18, 2003 at 12:02:40PM -0700, Bob Wilson wrote:
> I think the "ignore_protected" argument should be 0 for the Xtensa port.  A 
> separate relocation (R_XTENSA_32) is used when taking the address of a 
> function than when calling it (R_XTENSA_PLT), so there shouldn't be an issue 
> with incorrectly comparing the PLT addresses instead of the function 
> addresses.  Unless I'm missing something here, you shouldn't have to "assume 
> the worst".

What happens when an executable takes the address of a 
function defined in a shared library?


r~

Re: [RFC] unify dynamic_symbol_p implementations

[Bob Wilson]

I think the "ignore_protected" argument should be 0 for the Xtensa port.  A 
separate relocation (R_XTENSA_32) is used when taking the address of a 
function than when calling it (R_XTENSA_PLT), so there shouldn't be an issue 
with incorrectly comparing the PLT addresses instead of the function 
addresses.  Unless I'm missing something here, you shouldn't have to "assume 
the worst".

> --- elf32-xtensa.c      10 Jul 2003 19:01:47 -0000      1.5
> +++ elf32-xtensa.c      16 Jul 2003 23:35:31 -0000
> @@ -1838,38 +1838,9 @@ xtensa_elf_dynamic_symbol_p (info, h)
> 
[ stuff deleted from here ]
> +  /* ??? What, if anything, needs to happen wrt STV_PROTECTED and PLT
> +     entries?  For now assume the worst.  */
> +  return _bfd_elf_dynamic_symbol_p (h, info, 1);
>  }


Another very minor point is that I find your new comment below confusing.  It 
describes a difference from the generic version but then goes on to use the 
generic version.  I think you're referring to passing 0 for the 
"ignore_protected" argument, right?

--- elf64-alpha.c       4 Jul 2003 13:53:37 -0000       1.101
+++ elf64-alpha.c       16 Jul 2003 23:35:31 -0000
[ chunk deleted ]
@@ -269,49 +269,17 @@ struct alpha_elf_link_hash_table
 #define alpha_elf_sym_hashes(abfd) \
   ((struct alpha_elf_link_hash_entry **)elf_sym_hashes(abfd))
 
-/* Should we do dynamic things to this symbol?  */
+/* Should we do dynamic things to this symbol?  This differs from the 
+   generic version in that we never need to consider function pointer
+   equality wrt PLT entries -- we don't create a PLT entry if a symbol's
+   address is ever taken.  */