Home Original page

[PATCH v9 12/19] Merge of Object Attributes v2 during linkage (generic logic)

Jan Beulich jbeulich@suse.com
Fri Oct 31 11:44:44 GMT 2025 
On 01.09.2025 18:56, Matthieu Longo wrote:
> --- a/bfd/elf-attrs.c
> +++ b/bfd/elf-attrs.c
> @@ -18,6 +18,108 @@
>     Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
>     MA 02110-1301, USA.  */
>  
> +/* Design note regarding the merge of Object Attributes v2 during linkage
> +
> +   Entry point: _bfd_elf_link_setup_build_attributes
> +
> +   This patch adds all the generic logic to the linker to process OAv2.
> +   The linker is an "advanced" consumer of OAv2.  After parsing, it deduplicates
> +   them, merge them, detect any compatibility issues, and finally translate them
> +   to GNU properties.

Nit: "merges", "detects", "translates" (also in the commit message).

As to the translation to GNU properties: Is the same information then recorded
in two different forms in the final binary? Or are the attributes dropped, and
only the properties kept? Could this translation, at the very least, be split
off of this overly large patch?

> +   ** Overall design
> +
> +   The OAv2 processing pipeline follows a map-reduce pattern.  Obviously, the
> +   actual processing in GNU ld is not multi-threaded, and the operations are not
> +   necessarily executed directly one after another.
> +
> +   * Phase 1, map: successive per-file operations applied on the list of
> +     compatible input objects.
> +     1. Parsing of the OAv2 section's data (also used by objcopy).
> +     2. Translation of relevant GNU properties to OAv2. This is required for the
> +        backward-compatibility with input objects only marked using GNU
> +        properties.
> +     3. Sorting of the subsections and object attributes. Further operations
> +        rely on the ordering to perform some optimization in the processing of
> +        the data.
> +     4. Deduplication of subsections and object attributes, and detection of any
> +        conflict between duplicated subsections or tags.
> +     5. Translation of relevant OAv2 to GNU properties for a forward
> +        -compatibility with the GNU properties merge.
> +
> +   * Phase 2, reduce: OAv2 in input objects are merged together.
> +     1. Gathering of "frozen" values (=coming from the command-line arguments)
> +        into a virtual read-only list of subsections and attributes.
> +     2. Merging of OAv2 from an input file and the frozen input.
> +     3. Merging of the results of step 2 together. Since the OAv2 merge is
> +        commutative and associative, it can be implemented as a reduce.
> +        However, GNU ld implements it as an accumulate because it does not
> +        support multithreading.
> +     Notes: the two merge phases also perform a marking of unsupported/invalid
> +     subsections and attributes.  This marking can be used for debugging, and
> +     also more practically to drop unsupported optional subsections from the
> +     output.
> +
> +   * Phase 3, finalization of the output.
> +     1. Pruning of the unsupported/invalid subsections and attributes.
> +     2. Serialization of OAv2 data (also used by objcopy).
> +     Notes:
> +      - There is no translation of the merged OAv2 to GNU properties at this
> +        stage, as the GNU properties merge has already all the information that
> +        were translated in step 5 of stage 1.
> +      - The GNU properties are currently required as the runtime linker does
> +        not understand OAv2 yet.
> +      - Phase 3 should also include a compatibility check between the final
> +        merge result of the current link unit and input shared objects.  I opted
> +        for postponing this compatibility check, and GNU properties merge will
> +        take care of it as it already does.

The splitting into three phases also looks as if they could be boundaries at
which the patch could be split.

> @@ -424,6 +526,100 @@ bfd_elf_set_obj_attr_contents (bfd *abfd, bfd_byte *buffer, bfd_vma size)
>      abort ();
>  }
>  
> +/* Structure storing the result of a search in the list of input BFDs.
> +   - the pointer to the BFD.
> +   - the pointer to the section containing the object attributes.  */
> +typedef struct
> +{
> +  bfd *pbfd;
> +  bool has_build_attributes;
> +  asection *sec;
> +} bfd_search_result_t;
> +
> +/* Checks whether a BFD contains object attributes, and if so search for the
> +   relevant section storing them.  */
> +static bool
> +bfd_has_build_attributes (bfd *abfd, bfd_search_result_t *res)
> +{
> +  if (elf_obj_attr_subsections (abfd).size == 0)
> +    return false;
> +  res->has_build_attributes = true;
> +
> +  const char *sec_name = get_elf_backend_data (abfd)->obj_attrs_section;
> +  if ((res->sec = bfd_get_section_by_name (abfd, sec_name)) == NULL)
> +    return false;
> +  return true;
> +}

This is an odd interface: The sole caller sets res->pbfd, just to pass in the
same pointer. Why would the filling of the structure not be done solely here?

Further, is has_build_attributes actually necessary as a separate field? Can't
pbfd (being NULL or non-NULL) fulfill its purpose?

Finally (I think I had mentioned this before, but I may be misremembering and
it was in another context): Going solely be section name is, imo, problematic.
ELF has section types for a reason.

> +/* Returns True if the given BFD is an ELF object with the target backend
> +   machine code, non-dynamic (i.e. not a shared library), non-executable, and
> +   has sections.  False otherwise.
> +   Note: this function is a convenient encapsulation of the predicate used to
> +   search for objects containing object attributes in the list of BFDs.  */
> +static bool
> +elf_may_contain_obj_attrs (struct bfd_link_info *info,
> +			   bfd *bed)

"bed" stands for "backend data", like e.g. used ...

> +{
> +  const struct elf_backend_data *output_bed
> +    = get_elf_backend_data (info->output_bfd);

... here. Please let's not use misleading variable names.

> +  unsigned int elfclass = output_bed->s->elfclass;
> +  int elf_machine_code = output_bed->elf_machine_code;
> +  return (bfd_get_flavour (bed) == bfd_target_elf_flavour
> +	  && bed->section_count != 0
> +	  && (bed->flags & (DYNAMIC | EXEC_P)) == 0
> +	  && elf_machine_code == get_elf_backend_data (bed)->elf_machine_code
> +	  && elfclass == get_elf_backend_data (bed)->s->elfclass);
> +}
> +
> +/* Search for the first input object file containing object attributes.  */
> +static bfd_search_result_t
> +bfd_linear_find_first_with_obj_attrs (struct bfd_link_info *info)
> +{
> +  bfd_search_result_t res = {
> +    .pbfd = NULL,
> +    .has_build_attributes = false,
> +    .sec = NULL,
> +  };
> +
> +  for (bfd *abfd = info->input_bfds; abfd != NULL; abfd = abfd->link.next)
> +    if (elf_may_contain_obj_attrs (info, abfd))
> +      {
> +	res.pbfd = abfd;
> +	if (bfd_has_build_attributes (abfd, &res))
> +	  break;
> +      }
> +  return res;
> +}
> +
> +/* Create a build attributes section for the given bfd input.  */
> +static asection *
> +create_build_attributes_section (struct bfd_link_info *info,
> +				 bfd *ebfd)

While not as bad here, I still wonder why it's "ebfd" when the common naming is
"abfd".

> +{
> +  asection *sec;
> +  const char *sec_name = get_elf_backend_data (ebfd)->obj_attrs_section;
> +  sec = bfd_make_section_with_flags (ebfd,
> +				     sec_name,
> +				     (SEC_READONLY
> +				      | SEC_HAS_CONTENTS
> +				      | SEC_DATA));
> +  if (sec == NULL)
> +    info->callbacks->fatal (_("%P: failed to create %s section\n"), sec_name);
> +
> +  unsigned align
> +    = (get_elf_backend_data (info->output_bfd)->s->elfclass == ELFCLASS64
> +       ? 3
> +       : 2);

What in the format requires different alignment for 32- vs 64-bit ELF?

> +  if (!bfd_set_section_alignment (sec, align))
> +    info->callbacks->fatal (_("%pA: failed to align section\n"), sec);
> +
> +  elf_section_type (sec) = get_elf_backend_data (ebfd)->obj_attrs_section_type;
> +
> +  bfd_set_section_size (sec, bfd_elf_obj_attr_size (ebfd));
> +
> +  return sec;
> +}
> +
>  /* The first two tags in gnu-testing namespace are known, and so have a name and
>     can be initialized to the default value ('0' or NULL) depending on the
>     encoding specified on the subsection.  Any tags above 1 will be considered
> @@ -563,6 +759,1309 @@ oav2_encoding_to_string (obj_attr_encoding_v2 encoding)
>    return (encoding == OA_ENC_ULEB128) ? "ULEB128" : "NTBS";
>  }
>  
> +/* Initialize the given ATTR with its default value coming from the known tag
> +   registry.  */
> +static void
> +oav2_attr_overwrite_with_default (struct bfd_link_info *info,
> +				  obj_attr_subsection_v2 *subsec,

subsec is only and input, isn't it? (info looks to be, too, but uses down the
call tree may require it to be pointer-to-non-const, as sadly libbfd still is
pretty far from being const-correct.)

Again, constification of pointer targets wants to be done throughout the series,
wherever possible.

> +				  obj_attr_v2 *attr)
> +{
> +  const struct elf_backend_data *bed = get_elf_backend_data (info->output_bfd);
> +
> +  const obj_attr_info_t *attr_info
> +    = obj_attr_v2_find_known_by_tag (bed, subsec->name, attr->tag);
> +  if (attr_info == NULL)
> +    {
> +      attr->status = obj_attr_v2_unknown;
> +      if (subsec->encoding == OA_ENC_ULEB128)
> +	attr->val.uint_val = 0;
> +      else
> +	attr->val.string_val = NULL;
> +      return;
> +    }
> +
> +  if (bed->obj_attr_v2_default_value != NULL
> +      && bed->obj_attr_v2_default_value (info, attr_info, subsec, attr))
> +    {}
> +  else if (subsec->encoding == OA_ENC_NTBS)
> +    {
> +      if (attr->val.string_val != NULL)
> +	{
> +	  free ((void *) attr->val.string_val);
> +	  attr->val.string_val = NULL;
> +	}
> +      if (attr_info->default_value.string_val != NULL)
> +	attr->val.string_val = xstrdup (attr_info->default_value.string_val);
> +    }
> +  else
> +    attr->val.uint_val = attr_info->default_value.uint_val;
> +}
> +
> +/* Create a new attribute with the same key (=tag) as ATTR, and initialized with
> +   its default value from the known tag registry.  */
> +static obj_attr_v2 *
> +oav2_attr_default (struct bfd_link_info *info,
> +		   obj_attr_subsection_v2 *subsec,
> +		   obj_attr_v2 *attr)
> +{
> +  obj_attr_v2 *new_attr = _bfd_elf_obj_attr_v2_copy (attr, subsec->encoding);
> +  oav2_attr_overwrite_with_default (info, subsec, new_attr);
> +  return new_attr;
> +}
> +
> +/* The currently supported merge policy in the testing GNU namespace.
> +   - bitwise AND: apply bitwise AND.
> +   - bitwise OR: apply bitwise OR.
> +   - String-ADD: concatenates strings together with a '+' in-between.
> +   Note: Such policies should only be used for testing.  */
> +typedef enum {
> +  SUBSECTION_TESTING_MERGE_UNSUPPORTED = 0,
> +  SUBSECTION_TESTING_MERGE_AND_POLICY = 1,
> +  SUBSECTION_TESTING_MERGE_OR_POLICY = 2,
> +  SUBSECTION_TESTING_MERGE_ADD_POLICY = 3,
> +} gnu_testing_merge_policy;
> +
> +/* Determine which merge policy will be applied to SUBSEC.  The GNU policy are
> +   detected from the name of the subsection.  It should follow the following
> +   pattern: "gnu-testing-XXXXXX-MERGE-<POLICY>".
> +   Return one of the known merge policy if recognised, UNSUPPORTED otherwise.  */
> +static gnu_testing_merge_policy
> +gnu_testing_merge_subsection (const char *subsec_name)
> +{
> +  if (! gnu_testing_namespace (subsec_name))
> +    return SUBSECTION_TESTING_MERGE_UNSUPPORTED;
> +
> +  size_t subsec_name_len = strlen (subsec_name);
> +  if (strcmp ("-MERGE-AND", subsec_name + subsec_name_len - 10) == 0)
> +    return SUBSECTION_TESTING_MERGE_AND_POLICY;
> +  else if (strcmp ("-MERGE-OR", subsec_name + subsec_name_len - 9) == 0)
> +    return SUBSECTION_TESTING_MERGE_OR_POLICY;
> +  else if (strcmp ("-MERGE-ADD", subsec_name + subsec_name_len - 10) == 0)
> +    return SUBSECTION_TESTING_MERGE_ADD_POLICY;
> +  else
> +    return SUBSECTION_TESTING_MERGE_UNSUPPORTED;
> +}
> +
> +/* Merge policy Integer-AND: apply bitwise AND between REF and RHS.  */
> +obj_attr_v2_merge_result
> +obj_attr_v2_merge_policy_AND (struct bfd_link_info *info ATTRIBUTE_UNUSED,

I agree with the use of the term "policy" further up, but the functions are
actors, not policies, so I don't think their name should include the word.

As a non-static function this may also again want to gain _bfd_ or bfd_ as
a prefix? (Why is it non-static anyway? The sole caller looks to liver further
down in this same file.)

> +			      bfd *abfd ATTRIBUTE_UNUSED,
> +			      obj_attr_subsection_v2 *subsec,
> +			      obj_attr_v2 *ref, obj_attr_v2 *rhs,
> +			      obj_attr_v2 *frozen ATTRIBUTE_UNUSED)
> +{
> +  BFD_ASSERT (subsec->encoding == OA_ENC_ULEB128);
> +
> +  obj_attr_v2_merge_result res = {
> +    .merge = true,
> +    .val.uint_val = 0,

Pointless initializer - it's overwritten unconditionally below, and even if
it wasn't the use of designated initializers for the other fields would have
it obtain value 0.

> +    .reason = MERGE_OK,
> +  };
> +
> +  uint32_t original_value = ref->val.uint_val;
> +  res.val.uint_val = (ref->val.uint_val & rhs->val.uint_val);
> +  res.merge = (res.val.uint_val != original_value);
> +  if (!res.merge)
> +    res.reason = SAME_VALUE_AS_REF;
> +
> +  return res;
> +}
> +
> +/* Merge policy Integer-OR: apply bitwise OR between REF and RHS.  */
> +static obj_attr_v2_merge_result
> +obj_attr_v2_merge_policy_OR (struct bfd_link_info *info ATTRIBUTE_UNUSED,
> +			     bfd *abfd ATTRIBUTE_UNUSED,
> +			     obj_attr_subsection_v2 *subsec,
> +			     obj_attr_v2 *ref, obj_attr_v2 *rhs,
> +			     obj_attr_v2 *frozen ATTRIBUTE_UNUSED)
> +{
> +  BFD_ASSERT (subsec->encoding == OA_ENC_ULEB128);
> +
> +  obj_attr_v2_merge_result res = {
> +    .merge = true,
> +    .val.uint_val = 0,
> +    .reason = MERGE_OK,
> +  };
> +
> +  uint32_t original_value = ref->val.uint_val;
> +  res.val.uint_val = (ref->val.uint_val | rhs->val.uint_val);
> +  res.merge = (res.val.uint_val != original_value);
> +  if (res.val.uint_val == original_value)
> +    res.reason = SAME_VALUE_AS_REF;
> +
> +  return res;
> +}
> +
> +/* Merge policy String-ADD: concatenates strings from REF and RHS together
> +   adding a '+' character in-between.  */
> +static obj_attr_v2_merge_result
> +obj_attr_v2_merge_policy_ADD (struct bfd_link_info *info ATTRIBUTE_UNUSED,
> +			      bfd *abfd ATTRIBUTE_UNUSED,
> +			      obj_attr_subsection_v2 *subsec,
> +			      obj_attr_v2 *ref, obj_attr_v2 *rhs,
> +			      obj_attr_v2 *frozen)
> +{
> +  BFD_ASSERT (subsec->encoding == OA_ENC_NTBS);
> +
> +  obj_attr_v2_merge_result res = {
> +    .merge = false,
> +    .val.uint_val = 0,

That's the wrong field here, isn't it?

> +    .reason = MERGE_OK,
> +  };
> +
> +  if (ref->val.string_val && rhs->val.string_val)
> +    {
> +      res.merge = true;
> +      size_t ref_s_size = strlen (ref->val.string_val);
> +      size_t rhs_s_size = strlen (rhs->val.string_val);
> +      char *buffer = xmalloc (ref_s_size + 1 + rhs_s_size + 1);
> +      res.val.string_val = buffer;
> +      memcpy (buffer, ref->val.string_val, ref_s_size);
> +      buffer += ref_s_size;
> +      *buffer = '+';
> +      ++buffer;
> +      memcpy (buffer, rhs->val.string_val, rhs_s_size + 1);
> +    }
> +  else if (ref->val.string_val)
> +    {
> +      /* Nothing to do, frozen (if not NULL) should already be merged with
> +	 it.  */
> +      res.reason = SAME_VALUE_AS_REF;

No was to assert what the comment says?

> +    }
> +  else if (rhs->val.string_val)
> +    {
> +      res.merge = true;
> +
> +      size_t frozen_s_size
> +	= (frozen && frozen->val.string_val
> +	   ? strlen (frozen->val.string_val)
> +	   : 0);

The purpose of "frozen" could do with commenting on anyway.

> +      if (frozen_s_size == 0)
> +	{
> +	  res.val.string_val = rhs->val.string_val;
> +	  rhs->val.string_val = NULL;

That is, the caller's input variable is altered, and the caller needs to be
aware that it cannot be used again subsequently?

> +	}
> +      else
> +	{
> +	  size_t rhs_s_size = strlen (rhs->val.string_val);
> +	  char *buffer = xmalloc (frozen_s_size + 1 + rhs_s_size + 1);
> +	  res.val.string_val = buffer;
> +	  memcpy (buffer, frozen->val.string_val, frozen_s_size);
> +	  buffer += frozen_s_size;
> +	  *buffer = '+';
> +	  ++buffer;
> +	  memcpy (buffer, rhs->val.string_val, rhs_s_size + 1);
> +	}
> +    }
> +  return res;
> +}
> +
> +/* Return the merge result between attributes LHS, RHS and FROZEN.  */
> +static obj_attr_v2_merge_result
> +oav2_attr_merge (struct bfd_link_info *info,
> +		 bfd *abfd,
> +		 obj_attr_subsection_v2 *subsec,
> +		 obj_attr_v2 *lhs, obj_attr_v2 *rhs,
> +		 obj_attr_v2 *frozen, bool frozen_as_abfd)
> +{
> +  obj_attr_v2_merge_result res = {
> +    .merge = false,
> +    .val.uint_val = 0,
> +    .reason = MERGE_OK,
> +  };
> +
> +  gnu_testing_merge_policy policy;
> +
> +  if (get_elf_backend_data (abfd)->obj_attr_v2_tag_merge != NULL)
> +    {
> +      if (frozen_as_abfd)
> +	{
> +	  obj_attr_v2 *tmp = lhs;
> +	  lhs = rhs;
> +	  rhs = tmp;
> +	}
> +      res = get_elf_backend_data (abfd)->obj_attr_v2_tag_merge (info, abfd,
> +	subsec, lhs, rhs, frozen);

Again it's entirely unclear to me what "frozen" and "frozen_as_abfd" are about.

> +/* Report missing required attribute with key TAG in subsection SREF.  */
> +static void
> +report_missing_required_obj_attr (struct bfd_link_info *info,
> +				  bfd *abfd,
> +				  obj_attr_subsection_v2 *s_ref,
> +				  obj_attr_tag_t tag)
> +{
> +  const struct elf_backend_data *bed = get_elf_backend_data (abfd);
> +  const char *tag_s = obj_attr_v2_tag_to_string (bed, s_ref->name, tag);
> +  info->callbacks->einfo (
> +    _("%X%pB: error: missing required object attribute '%s' in subsection "
> +      "'%s'\n"), abfd, tag_s, s_ref->name);

Imo better:

  info->callbacks->einfo (
    _("%X%pB: error: missing required object attribute '%s' in subsection '%s'\n"),
    abfd, tag_s, s_ref->name);

Splitting format strings should be avoided, unless the lines get really long (but
then the diagnostic text likely is too verbose anyway).

> +}
> +
> +/* Report required attribute A_ABFD mismatching with A_REF.  */
> +static void
> +report_mismatching_required_obj_attr (struct bfd_link_info *info,
> +				      bfd *ref_bfd,
> +				      bfd *abfd,
> +				      obj_attr_subsection_v2 *s_ref,
> +				      obj_attr_v2 *a_ref,
> +				      obj_attr_v2 *a_abfd)
> +{
> +  const struct elf_backend_data *bed = get_elf_backend_data (abfd);
> +  const char* tag_s = obj_attr_v2_tag_to_string (bed, s_ref->name, a_ref->tag);
> +  if (s_ref->encoding == OA_ENC_ULEB128)
> +    {
> +      info->callbacks->einfo (
> +	_("%X%pB, %pB: error: mismatching values 0x%x and 0x%x for "

%#x

> +/* Merge case 3: S_ABFD does not have a S_REF equivalent.
> +   1. Create a new default-initialized S_REF subsection.
> +   2. Merge S_ABFD into S_REF.
> +   3. Insert S_REF into REF.  */
> +static bool
> +handle_subsection_additional (struct bfd_link_info *info,
> +			      bfd *ref_bfd, bfd *abfd,
> +			      obj_attr_subsection_v2 *s_ref_next,
> +			      obj_attr_subsection_v2 *s_abfd,
> +			      obj_attr_subsection_v2 *s_frozen)

There's nothing here that REF in the comment matches.

(Giving up here.)

> --- a/bfd/elf-attrs.h
> +++ b/bfd/elf-attrs.h
> @@ -55,6 +55,16 @@ typedef union obj_attr_value_v2 {
>    const char* string_val;
>  } obj_attr_value_v2;
>  
> +typedef enum obj_attr_v2_status
> +{
> +  /* An attribute that is unknown to the linker, and so cannot be merged.  */
> +  obj_attr_v2_unknown = 0,
> +  /* An attribute that was reported as corrupted.  */
> +  obj_attr_v2_corrupted,
> +  /* A valid attribute.  */
> +  obj_attr_v2_ok,
> +} obj_attr_v2_status;

Iirc already on an earlier version I asked that typedef-s please either be
omitted or be identified as such (by a _t suffix).

> @@ -153,3 +179,36 @@ obj_attr_v2_find_known_by_tag (const struct elf_backend_data *,
>  extern const char *
>  obj_attr_v2_tag_to_string (const struct elf_backend_data *, const char*,
>  			   obj_attr_tag_t);
> +
> +enum obj_attr_v2_merge_result_reason
> +{
> +  /* Default: everything is ok.  */
> +  MERGE_OK = 0,
> +  /* The result value of the merge is the same as REF.  */
> +  SAME_VALUE_AS_REF,
> +  /* No implementation of a merge for this attribute exists.  */
> +  UNSUPPORTED,
> +  /* The merge failed, an error message should be logged.  */
> +  ERROR,
> +};

These identifiers would be fine if they lived in a .c file. For them to live in
a header, I think they need some disambiguating prefix. I also assume there's
no dependency anywhere that would require MERGE_OK to explictly have value 0
assigned (which it would get anyway if the "= 0" was dropped)?

> +typedef struct {
> +  /* Should the merge be performed ?  */
> +  bool merge;
> +  /* The merged value.  */
> +  union obj_attr_value_v2 val;
> +  /* If the merge should not be performed, give the reason to differentiate
> +     error cases from normal cases.  Typically, if REF already is set to the
> +     same value as the merged result, no merge is needed, and this is not an
> +     error.  */
> +  enum obj_attr_v2_merge_result_reason reason;
> +} obj_attr_v2_merge_result;
> +
> +/* Re-usable merge policies.  */
> +/* For now, only AND-merge is used by AArch64 backend.  Additional policies
> +   (Integer-OR, String-ADD) are part of the GNU testing namespace.  If they
> +   appear to be usefull for a backend at some point, they should be exposed
> +   to the backend here below.  */
> +extern obj_attr_v2_merge_result
> +obj_attr_v2_merge_policy_AND (struct bfd_link_info *, bfd *,
> +			      obj_attr_subsection_v2 *, obj_attr_v2 *,
> +			      obj_attr_v2 *, obj_attr_v2 *);

May want a _bfd_ or bfd_ (depending on where it's used) prefix again?

> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -1662,6 +1662,26 @@ struct elf_backend_data
>    /* The size of the array of known subsections.  */
>    const size_t obj_attr_v2_known_subsections_size;
>  
> +  /* Translate the relevant GNU properties to object attributes v2.  */
> +  void (*translate_relevant_gnu_props_to_obj_attrs) (bfd *,
> +    elf_property_list *);
> +
> +  /* Translate the relevant object attributes v2 to GNU properties.  */
> +  void (*translate_relevant_obj_attrs_to_gnu_props) (bfd *,
> +    obj_attr_subsection_v2 *);

What is "relevant" intended to convey in the names?

> +  /* Check build attributes subsection v2 against expected properties.  */
> +  bool (*obj_attr_subsection_v2_match_known) (struct bfd_link_info *, bfd *,
> +    obj_attr_subsection_v2 *);

Comment says "expected" while the function name says "known". I think it's
meant to be the latter?

> +  /* Get default value for an attribute.  */
> +  bool (*obj_attr_v2_default_value) (struct bfd_link_info *,
> +    const obj_attr_info_t *, obj_attr_subsection_v2 *, obj_attr_v2 *);
> +
> +  /* Merge a build attribute v2.  */
> +  obj_attr_v2_merge_result (*obj_attr_v2_tag_merge) (struct bfd_link_info *,
> +    bfd *, obj_attr_subsection_v2 *, obj_attr_v2 *, obj_attr_v2 *, obj_attr_v2 *);

"build attribute" in the comment?

> @@ -1683,6 +1703,9 @@ struct elf_backend_data
>    bool (*merge_gnu_properties) (struct bfd_link_info *, bfd *, bfd *,
>  				       elf_property *, elf_property *);
>  
> +  /* Set up build attributes.  */
> +  bfd *(*setup_build_attributes) (struct bfd_link_info *);

Hmm, more "build" here. Am I missing something?

> --- a/bfd/elf-properties.c
> +++ b/bfd/elf-properties.c
> @@ -812,9 +812,9 @@ _bfd_elf_link_setup_gnu_properties (struct bfd_link_info *info)
>      return NULL;
>  
>    /* Merge .note.gnu.property sections.  */
> -  info->callbacks->minfo (_("\n"));
> +  info->callbacks->minfo ("\n");
>    info->callbacks->minfo (_("Merging program properties\n"));
> -  info->callbacks->minfo (_("\n"));
> +  info->callbacks->minfo ("\n");

Entirely unrelated change in an already overly large patch?

Jan
More information about the Binutils mailing list