Re: [PATCH 1/2] LD: Export relative-from-absolute symbol marking to BFD

["Maciej W. Rozycki" <macro at mips dot com>]

Hi Alan,

> > --- binutils.orig/bfd/linker.c	2018-07-06 01:38:47.000000000 +0100
> > +++ binutils/bfd/linker.c	2018-07-13 02:30:30.552331879 +0100
> > @@ -484,7 +484,19 @@ _bfd_link_hash_table_init
> >  
> >  /* Look up a symbol in a link hash table.  If follow is TRUE, we
> >     follow bfd_link_hash_indirect and bfd_link_hash_warning links to
> > -   the real symbol.  */
> > +   the real symbol.
> > +
> > +.{* Return TRUE if the symbol described by a linker hash entry H
> > +.   is going to be absolute.  Linker-script defined symbols can be
> > +.   converted from absolute to section-relative ones late in the
> > +.   link.  Use this macro to correctly determine whether the symbol
> > +.   will actually end up absolute in output.  *}
> > +.#define bfd_is_abs_symbol(H) \
> > +.  ((H)->type == bfd_link_hash_defined \
> 
> Either test both bfd_link_hash_defined and bfd_link_hash_defweak here
> or test neither, relying on the tests to be done before using this
> macro.  I'm inclined to do the latter, given that the macro is a
> replacement for a test of h->u.def.section.

 I don't think you can define a weak symbol in a linker script, but for 
the sake of a possible future expansion I agree (though having the check 
localised in `bfd_is_abs_symbol' would mean a single place to update with 
such a future update).

 Removing the check for the symbol type from here will require new code 
(such as one I'll be adding with the fix for PR ld/21375) to check it 
explicitly beforehand as referring `->u.def.section' on a symbol of a 
different type is invalid.  I guess I'm fine with that though.

> > +.   && bfd_is_abs_section ((H)->u.def.section) \
> > +.   && !((H)->ldscript_def && (H)->rel_from_abs))
> 
> Testing ldscript_def is superfluous.

 OK, thanks for weighing in -- I actually changed my mind a few times 
about this when working with this code.

> Other than that, the patch looks OK.

 OK, I'll update and repost as I yet need to complete change descriptions 
for PR ld/21375 patches anyway.

 Thanks for the lightning-fast review!

  Maciej