RFC: ld: Support ELF GNU program properties

[Nick Clifton]

Hi H.J.

  Sorry for not responding to this earlier.

> Any comments?

Are the contents of the .note.gnu.property section documented somewhere ?
(Other than an email that is).

I think that you ought to add a ld/NEWS entry for this feature.

Did you check this patch with any non-x86 targets ?  (Just to make sure...)


> +typedef struct elf_property
> +{
> +  unsigned int type;
> +  unsigned int datasz;
> +  union
> +    {
> +      bfd_vma value;
> +    } u;
> +  enum elf_property_kind kind;
> +} elf_property;

I found it very confusing that you have the same 'type' and 'datasz' fields
as an Elf_Internal_Note structure, but with different sizes and different
meanings.  Would it be possible to use different field names in this structure ?

Also why do you have a union with only one element ?  I am guessing
that it is for future extensions, but it would be nice to know.


> +/* Get a property, allocate a new one if needed.  */
> +
> +elf_property *
> +_bfd_elf_get_property (bfd *abfd, unsigned int type, unsigned int datasz)
> +{
> +  elf_property_list *p, **lastp;
> +
> +  /* Keep the property list in order of type.  */
> +  lastp = &elf_properties (abfd);

Paranoia - I would suggest checking that ABFD is an elf format bfd before
attempting to access the elf_properties list.  (I know that you have already
checked the bfd type in _bfd_elf_link_setup_gnu_properties, but this function
can be called from elsewhere and the caller might not have been so vigilant).


> +  for (p = *lastp; p; p = p->next)
> +    {
> +      /* Reuse the existing entry.  */
> +      if (type == p->property.type)
> +        return &p->property;

Should you check that datasz == p->property.datasz as well ?


> +  p = (elf_property_list *) bfd_alloc (abfd, sizeof (*p));

Paranoia again - bfd_alloc() can return NULL...


> +	  if (type < GNU_PROPERTY_LOUSER && bed->parse_gnu_properties)
> +	    {
> +	      enum elf_property_kind kind
> +		= bed->parse_gnu_properties (abfd, type, ptr, datasz);
> +	      if (kind == property_corrupt)
> +		{
> +		  /* Clear all properties.  */
> +		  elf_properties (abfd) = NULL;
> +		  return FALSE;

Presumably no error message is printed here because it is assumed that this
has been done in bed->parse_gnu_properties.  The (very) brief comment in elf-bfd.h
however makes no mention of this requirement.


> +static bfd_boolean
> +elf_merge_gnu_properties (bfd *abfd, elf_property *prop, elf_property *p)

Slightly confusing parameter names.  I would have gone with prop_1 and prop_2 or
propA and propB.  Something like that.


> +	    /* Discard .note.gnu.property section in the rest inputs.  */
> +	    sec = bfd_get_section_by_name (abfd, ".note.gnu.property");

Why not use NOTE_GNU_PROPERTY_SECTION_NAME here ?  It is already defined in include/elf/common.h

 
That's all that I can see.

Cheers
  Nick