public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [questions] elflink.c, back-end-data
@ 2007-07-26  1:23 msnyder
  2007-07-26 13:47 ` Alan Modra
  0 siblings, 1 reply; 2+ messages in thread
From: msnyder @ 2007-07-26  1:23 UTC (permalink / raw)
  To: binutils

Don't feel like I understand the issues well enough to submit a patch...
can somebody help me out here?

In _bfd_elf_fix_symbol_flags, we start with bed == NULL.
We only conditionally initialize it...
  /* Backend specific symbol fixup.  */
  if (elf_hash_table (eif->info)->dynobj)
    {
      bed = get_elf_backend_data (elf_hash_table (eif->info)->dynobj);

But before long we use it as if confident it's been initialized:
      (*bed->elf_backend_hide_symbol) (eif->info, h, force_local);

Then later, we eclipse it with a block-local version, which seems
to get initialized exactly the same as before (unles eif->info
or eif->info->dynobj can change...):

    if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
        && h->root.type == bfd_link_hash_undefweak)
      {
        const struct elf_backend_data *bed;
        bed = get_elf_backend_data (elf_hash_table (eif->info)->dynobj);
        (*bed->elf_backend_hide_symbol) (eif->info, h, TRUE);
      }

And then a short time later, we use it again as if it had certainly
been initialized (seems to me this last use would have benefited
from the preceeding initialization, had there not been an eclipsing
block local):

          (*bed->elf_backend_copy_indirect_symbol) (eif->info, weakdef, h);

Am I missing something?

Michael


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [questions] elflink.c, back-end-data
  2007-07-26  1:23 [questions] elflink.c, back-end-data msnyder
@ 2007-07-26 13:47 ` Alan Modra
  0 siblings, 0 replies; 2+ messages in thread
From: Alan Modra @ 2007-07-26 13:47 UTC (permalink / raw)
  To: msnyder; +Cc: binutils

On Wed, Jul 25, 2007 at 05:54:34PM -0700, msnyder@sonic.net wrote:
> In _bfd_elf_fix_symbol_flags, we start with bed == NULL.
> We only conditionally initialize it...
>   /* Backend specific symbol fixup.  */
>   if (elf_hash_table (eif->info)->dynobj)
>     {
>       bed = get_elf_backend_data (elf_hash_table (eif->info)->dynobj);

elf_hash_table (eif->info)->dynobj will always be non-NULL, so the
test is redundant.  elf_fix_symbol_flags is call from two places,
_bfd_elf_link_assign_sym_version, and _bfd_elf_adjust_dynamic_symbol.
Neither of these functions will be called if dynobj is NULL.

> Then later, we eclipse it with a block-local version, which seems

silly.

> Am I missing something?

Unnecessary code is confusing.

	* elflink.c (_bfd_elf_fix_symbol_flags): Remove unnecessary
	check on dynobj.  Remove bed shadow.

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.272
diff -u -p -r1.272 elflink.c
--- bfd/elflink.c	24 Jul 2007 19:54:01 -0000	1.272
+++ bfd/elflink.c	26 Jul 2007 13:44:39 -0000
@@ -2368,7 +2368,7 @@ bfd_boolean
 _bfd_elf_fix_symbol_flags (struct elf_link_hash_entry *h,
 			   struct elf_info_failed *eif)
 {
-  const struct elf_backend_data *bed = NULL;
+  const struct elf_backend_data *bed;
 
   /* If this symbol was mentioned in a non-ELF file, try to set
      DEF_REGULAR and REF_REGULAR correctly.  This is the only way to
@@ -2429,13 +2429,10 @@ _bfd_elf_fix_symbol_flags (struct elf_li
     }
 
   /* Backend specific symbol fixup.  */
-  if (elf_hash_table (eif->info)->dynobj)
-    {
-      bed = get_elf_backend_data (elf_hash_table (eif->info)->dynobj);
-      if (bed->elf_backend_fixup_symbol
-	  && !(*bed->elf_backend_fixup_symbol) (eif->info, h))
-	return FALSE;
-    }
+  bed = get_elf_backend_data (elf_hash_table (eif->info)->dynobj);
+  if (bed->elf_backend_fixup_symbol
+      && !(*bed->elf_backend_fixup_symbol) (eif->info, h))
+    return FALSE;
 
   /* If this is a final link, and the symbol was defined as a common
      symbol in a regular object file, and there was no definition in
@@ -2473,11 +2470,7 @@ _bfd_elf_fix_symbol_flags (struct elf_li
      hide it from the dynamic linker.  */
   if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
       && h->root.type == bfd_link_hash_undefweak)
-    {
-      const struct elf_backend_data *bed;
-      bed = get_elf_backend_data (elf_hash_table (eif->info)->dynobj);
-      (*bed->elf_backend_hide_symbol) (eif->info, h, TRUE);
-    }
+    (*bed->elf_backend_hide_symbol) (eif->info, h, TRUE);
 
   /* If this is a weak defined symbol in a dynamic object, and we know
      the real definition in the dynamic object, copy interesting flags

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-07-26 13:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-07-26  1:23 [questions] elflink.c, back-end-data msnyder
2007-07-26 13:47 ` Alan Modra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).