public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: Compact EH Support
@ 2016-08-26  5:58 Alan Modra
  2016-09-02 14:19 ` Moore, Catherine
  0 siblings, 1 reply; 3+ messages in thread
From: Alan Modra @ 2016-08-26  5:58 UTC (permalink / raw)
  To: Catherine_Moore, bschmidt, paul; +Cc: binutils

Re https://sourceware.org/ml/binutils/2015-05/msg00237.html

Would the person responsible for _bfd_elf_section_for_symbol please
add a comment describing what the function does, in particular why it
is looking for symbols defined in discarded sections.  It doesn't make
much sense to me..

Please also move the function to elf-eh-frame.c and make it static.

-- 
Alan Modra
Australia Development Lab, IBM

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

* RE: Compact EH Support
  2016-08-26  5:58 Compact EH Support Alan Modra
@ 2016-09-02 14:19 ` Moore, Catherine
  2016-09-03  3:43   ` Alan Modra
  0 siblings, 1 reply; 3+ messages in thread
From: Moore, Catherine @ 2016-09-02 14:19 UTC (permalink / raw)
  To: Alan Modra, bschmidt, paul; +Cc: binutils



> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Friday, August 26, 2016 1:58 AM
> To: Moore, Catherine <Catherine_Moore@mentor.com>;
> bschmidt@redhat.com; paul@nowt.org
> Cc: binutils@sourceware.org
> Subject: Re: Compact EH Support
> 
> Re https://sourceware.org/ml/binutils/2015-05/msg00237.html
> 
> Would the person responsible for _bfd_elf_section_for_symbol please
> add a comment describing what the function does, in particular why it
> is looking for symbols defined in discarded sections.  It doesn't make
> much sense to me..
> 
> Please also move the function to elf-eh-frame.c and make it static.
> 
> --
> Alan Modra
> Australia Development Lab, IBM

The intent with the addition of _bfd_elf_section_for_symbol was to pull common code from bfd_reloc_symbol_deleted_p and bfd_elf_parse_eh_frame_entry.
This patch adds the comment and updates bfd_reloc_symbol_deleted_p to call _bfd_section_for_symbol.
OK to push?

2016-09-02  Catherine Moore  <clm@codesourcery.com>

        * elflink.c ( _bfd_elf_section_for_symbol): Add comment.
        Merge discard criteria from bfd_elf_reloc_symbol_deleted_p.
        (bfd_elf_reloc_symbol_deleted_p): Delete common code. Call
        _bfd_section_for_symbol.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 9e9a33c..00603be 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -58,6 +58,11 @@ struct elf_find_verdep_info
 static bfd_boolean _bfd_elf_fix_symbol_flags
   (struct elf_link_hash_entry *, struct elf_info_failed *);

+/*  Given a relocation cookie and its symndx, return the section
+    that defines the symbol or NULL if the symbol is not defined.
+    When DISCARD is true, a local symbol that has been defined in
+    a discarded section will be treated as an undefined symbol.  */
+
 asection *
 _bfd_elf_section_for_symbol (struct elf_reloc_cookie *cookie,
                             unsigned long r_symndx,
@@ -92,8 +97,10 @@ _bfd_elf_section_for_symbol (struct elf_reloc_cookie *cookie,
       /* Need to: get the symbol; get the section.  */
       isym = &cookie->locsyms[r_symndx];
       isec = bfd_section_from_elf_index (cookie->abfd, isym->st_shndx);
-      if (isec != NULL
-         && discard ? discarded_section (isec) : 1)
+
+      if (!discard
+         || (isec != NULL
+             && (isec->kept_section != NULL || discarded_section (isec))))
        return isec;
      }
   return NULL;
@@ -13509,40 +13516,8 @@ bfd_elf_reloc_symbol_deleted_p (bfd_vma offset, void *cookie)
       if (r_symndx == STN_UNDEF)
        return TRUE;

-      if (r_symndx >= rcookie->locsymcount
-         || ELF_ST_BIND (rcookie->locsyms[r_symndx].st_info) != STB_LOCAL)
-       {
-         struct elf_link_hash_entry *h;
-
-         h = rcookie->sym_hashes[r_symndx - rcookie->extsymoff];
-
-         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->root.type == bfd_link_hash_defined
-              || h->root.type == bfd_link_hash_defweak)
-             && (h->root.u.def.section->owner != rcookie->abfd
-                 || h->root.u.def.section->kept_section != NULL
-                 || discarded_section (h->root.u.def.section)))
-           return TRUE;
-       }
-      else
-       {
-         /* It's not a relocation against a global symbol,
-            but it could be a relocation against a local
-            symbol for a discarded section.  */
-         asection *isec;
-         Elf_Internal_Sym *isym;
-
-         /* Need to: get the symbol; get the section.  */
-         isym = &rcookie->locsyms[r_symndx];
-         isec = bfd_section_from_elf_index (rcookie->abfd, isym->st_shndx);
-         if (isec != NULL
-             && (isec->kept_section != NULL
-                 || discarded_section (isec)))
-           return TRUE;
-       }
+      if (_bfd_elf_section_for_symbol (rcookie, r_symndx, TRUE) != NULL)
+       return TRUE;
       return FALSE;
     }
   return FALSE;

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

* Re: Compact EH Support
  2016-09-02 14:19 ` Moore, Catherine
@ 2016-09-03  3:43   ` Alan Modra
  0 siblings, 0 replies; 3+ messages in thread
From: Alan Modra @ 2016-09-03  3:43 UTC (permalink / raw)
  To: Moore, Catherine; +Cc: bschmidt, paul, binutils

On Fri, Sep 02, 2016 at 02:18:57PM +0000, Moore, Catherine wrote:
> The intent with the addition of _bfd_elf_section_for_symbol was to pull common code from bfd_reloc_symbol_deleted_p and bfd_elf_parse_eh_frame_entry.
> This patch adds the comment and updates bfd_reloc_symbol_deleted_p to call _bfd_section_for_symbol.
> OK to push?

No.

> 2016-09-02  Catherine Moore  <clm@codesourcery.com>
> 
>         * elflink.c ( _bfd_elf_section_for_symbol): Add comment.
>         Merge discard criteria from bfd_elf_reloc_symbol_deleted_p.
>         (bfd_elf_reloc_symbol_deleted_p): Delete common code. Call
>         _bfd_section_for_symbol.
> 
> diff --git a/bfd/elflink.c b/bfd/elflink.c
> index 9e9a33c..00603be 100644
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -58,6 +58,11 @@ struct elf_find_verdep_info
>  static bfd_boolean _bfd_elf_fix_symbol_flags
>    (struct elf_link_hash_entry *, struct elf_info_failed *);
> 
> +/*  Given a relocation cookie and its symndx, return the section
> +    that defines the symbol or NULL if the symbol is not defined.
> +    When DISCARD is true, a local symbol that has been defined in
> +    a discarded section will be treated as an undefined symbol.  */
> +

I'd be happy if the function were true to name and performed as per
the first two lines of your new comment.  But this is *not* what
_bfd_elf_section_for_symbol does.  Instead, for global symbols, it
only returns the section if the symbol is defined in a discarded
section, and it does so regardless of the parameter DISCARD.  That is
inconsistent with the treatment of local symbols, and probably a bug
in the use of the function by _bfd_elf_parse_eh_frame_entry.

>  asection *
>  _bfd_elf_section_for_symbol (struct elf_reloc_cookie *cookie,
>                              unsigned long r_symndx,
> @@ -92,8 +97,10 @@ _bfd_elf_section_for_symbol (struct elf_reloc_cookie *cookie,
>        /* Need to: get the symbol; get the section.  */
>        isym = &cookie->locsyms[r_symndx];
>        isec = bfd_section_from_elf_index (cookie->abfd, isym->st_shndx);
> -      if (isec != NULL
> -         && discard ? discarded_section (isec) : 1)
> +
> +      if (!discard
> +         || (isec != NULL
> +             && (isec->kept_section != NULL || discarded_section (isec))))
>         return isec;
>       }
>    return NULL;
> @@ -13509,40 +13516,8 @@ bfd_elf_reloc_symbol_deleted_p (bfd_vma offset, void *cookie)
>        if (r_symndx == STN_UNDEF)
>         return TRUE;
> 
> -      if (r_symndx >= rcookie->locsymcount
> -         || ELF_ST_BIND (rcookie->locsyms[r_symndx].st_info) != STB_LOCAL)
> -       {
> -         struct elf_link_hash_entry *h;
> -
> -         h = rcookie->sym_hashes[r_symndx - rcookie->extsymoff];
> -
> -         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->root.type == bfd_link_hash_defined
> -              || h->root.type == bfd_link_hash_defweak)
> -             && (h->root.u.def.section->owner != rcookie->abfd
> -                 || h->root.u.def.section->kept_section != NULL
> -                 || discarded_section (h->root.u.def.section)))
> -           return TRUE;

The code you're deleting here does not match the code in
_bfd_elf_section_for_symbol!

> -       }
> -      else
> -       {
> -         /* It's not a relocation against a global symbol,
> -            but it could be a relocation against a local
> -            symbol for a discarded section.  */
> -         asection *isec;
> -         Elf_Internal_Sym *isym;
> -
> -         /* Need to: get the symbol; get the section.  */
> -         isym = &rcookie->locsyms[r_symndx];
> -         isec = bfd_section_from_elf_index (rcookie->abfd, isym->st_shndx);
> -         if (isec != NULL
> -             && (isec->kept_section != NULL
> -                 || discarded_section (isec)))
> -           return TRUE;
> -       }
> +      if (_bfd_elf_section_for_symbol (rcookie, r_symndx, TRUE) != NULL)
> +       return TRUE;
>        return FALSE;
>      }
>    return FALSE;

It seems likely to me that you should delete "discard" from
_bfd_elf_section_for_symbol and any qualification of the section being
discarded, in the function.  Here you can then use

    isec = _bfd_elf_section_for_symbol (rcookie, r_symndx);
    return (isec != NULL
	    && (isec->owner != rcookie->abfd
		|| isec->kept_section != NULL
		|| discarded_section (isec)));

-- 
Alan Modra
Australia Development Lab, IBM

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

end of thread, other threads:[~2016-09-03  3:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-26  5:58 Compact EH Support Alan Modra
2016-09-02 14:19 ` Moore, Catherine
2016-09-03  3:43   ` 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).