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

[Jan Beulich]

On 15.07.2025 13:39, Matthieu Longo wrote:
> 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.
> 
> ** 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.

May a linker legitimately remove anything? (Instead I would have hoped
for some forward compatibility.)

> @@ -411,6 +513,98 @@ 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;
> +}
> +
> +/* Returns True if the given BFD is an ELF object with the current backend

What's "current"?

> +   machine code, non-dynamic (i.e. not a shared library), and has sections.

But an executable is okay? I.e. aren't after relocatable objects here?

> +   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
> +bfd_is_non_dynamic_elf_object (struct bfd_link_info *info,
> +			       bfd *abfd)
> +{
> +  const struct elf_backend_data *output_bfd
> +    = get_elf_backend_data (info->output_bfd);

"output_bed" may be a better name for this variable. It's hard to see though
why you have ...

> +  unsigned int elfclass = output_bfd->s->elfclass;
> +  int elf_machine_code = output_bfd->elf_machine_code;

... three variables for one side ...

> +  return (bfd_get_flavour (abfd) == bfd_target_elf_flavour
> +	  && abfd->section_count != 0
> +	  && (abfd->flags & DYNAMIC) == 0
> +	  && elf_machine_code == get_elf_backend_data (abfd)->elf_machine_code
> +	  && elfclass == get_elf_backend_data (abfd)->s->elfclass);

... of the comparisons and none for the other. I would suggest to have "bed"
and "output_bed", and no further ones.

As this function isn't generic, I'd recommend dropping (or replacing) its
bfd_ prefix. (This likely applies elsewhere as well.)

> +/* Search for the first input object file containing object attributes.  */
> +static bfd_search_result_t
> +bfd_linear_search_one_with_build_attributes (struct bfd_link_info *info)

s/search_one/find_first/?

> +/* Create a build attributes section for the given bfd input.  */
> +static asection *
> +create_build_attributes_section (struct bfd_link_info *info,
> +				 bfd *ebfd)
> +{
> +  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->einfo (
> +      _("%F%P: failed to create %s section\n"), sec_name);
> +
> +  unsigned align = (bfd_get_mach (ebfd) & bfd_mach_aarch64_ilp32) ? 2 : 3;

Wait - we're in generic code here? How can there be an Arm64 specific check?
Did you perhaps mean to use struct bfd_arch_info's bits_per_address?

> +  if (!bfd_set_section_alignment (sec, align))
> +    info->callbacks->einfo (_("%F%pA: failed to align section\n"), sec);

What is %F? I understand there are very few other uses, but I can't help the
impression that they're all stale (at best). If I try to use it, all I get is
"%F" in the output. Ah, here we go - commit 8d97c1a53f3d. You mean to use
->fatal() instead (also a few lines up, and perhaps elsewhere).

> @@ -454,8 +648,8 @@ static known_subsection_v2 obj_attr_v2_known_gnu_subsections[] =
>    /* Note for the future: GNU subsections can be added here below.  */
>  };
>  
> -/* Return True if the given subsection name is part of the reserved "gnu-testing"
> -   namespace.  */
> +/* Return True if the given subsection name is part of the reserved testing
> +   namespace, i.e. SUBSEC_NAME begins with "gnu-testing".  */
>  static bool
>  gnu_testing_namespace (const char *subsec_name)
>  {

Was this adjustment meant to be done in patch 04?

> @@ -555,6 +749,1285 @@ 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,
> +				  obj_attr_v2 *attr)
> +{
> +  const struct elf_backend_data *be = get_elf_backend_data (info->output_bfd);

The commonly used name for this is "bed", I think.

> +  const obj_attr_info_t *tag_info
> +    = known_obj_attr_v2_find_by_tag (be, subsec->name, attr->tag);
> +  if (tag_info == NULL)
> +    {
> +      attr->status = obj_attr_v2_unknown;
> +      if (subsec->encoding == OA_ENC_ULEB128)
> +	attr->vals.uint_val = 0;
> +      else
> +	attr->vals.string_val = NULL;
> +      return;
> +    }
> +
> +  if (be->obj_attr_v2_default_value != NULL
> +      && be->obj_attr_v2_default_value (info, tag_info, subsec, attr))
> +    {}
> +  else if (subsec->encoding == OA_ENC_NTBS)
> +    {
> +      if (tag_info->default_value.val.string != NULL)
> +	{
> +	  if (attr->vals.string_val != NULL)
> +	    free ((void *) attr->vals.string_val);
> +	  attr->vals.string_val = strdup (tag_info->default_value.val.string);

xstrdup() ?

> +	}
> +      else
> +	attr->vals.string_val = NULL;

Leaking the earlier string in this case?

> +/* Merge policy Integer-AND: apply bitwise AND between REF and RHS.  */
> +obj_attr_v2_merge_result
> +obj_attr_v2_tag_merge_AND (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,
> +    .vals.uint_val = 0,
> +    .reason = MERGE_OK,
> +  };
> +
> +  uint32_t original_value = ref->vals.uint_val;
> +  res.vals.uint_val = (ref->vals.uint_val & rhs->vals.uint_val);
> +  res.merge = (res.vals.uint_val != original_value);
> +  if (res.vals.uint_val == original_value)

Doing twice in a row the effectively same comparison?

> +/* 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_tag_merge_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);
> +
> +  size_t frozen_s_size = 0;
> +  if (frozen && frozen->vals.string_val)
> +    frozen_s_size = strlen (frozen->vals.string_val);
> +
> +  obj_attr_v2_merge_result res = {
> +    .merge = false,
> +    .vals.uint_val = 0,
> +    .reason = MERGE_OK,
> +  };
> +
> +  if (ref->vals.string_val && rhs->vals.string_val)
> +    {
> +      res.merge = true;
> +      size_t ref_s_size = strlen (ref->vals.string_val);
> +      size_t rhs_s_size = strlen (rhs->vals.string_val);
> +      char *buffer = malloc (ref_s_size + 1 + rhs_s_size + 1);

xmalloc() (Hopefully you've already gone through and changed all the
allocations throughout the series.)

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

This is the first use of the variable - why is it declared and set at the
top of the function?

> +/* 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,
> +    .vals.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);
> +    }
> +
> +  /* Note for the future: the merge of generic object attributes should be
> +     added here, between the architecture-specific merge, and the reserved GNU
> +     testing namespace.  */
> +
> +  /* GNU testing merge policies are looked up last.  If MERGE_OK is detected,
> +     the subsection is considered unmergeable.  */
> +  if (! res.merge && (res.reason == UNSUPPORTED || res.reason == MERGE_OK))

I can't bring comment and code together.

> +    {
> +      if ((policy = gnu_testing_merge_subsection (subsec->name))
> +	   != SUBSECTION_TESTING_MERGE_UNSUPPORTED)
> +	{
> +	  /* Only the first two attributes can be merged, others won't and will
> +	     be discarded.  */
> +	  if (lhs->tag <= 1)

By inverting this and handling ...

> +	    {
> +	      if (policy == SUBSECTION_TESTING_MERGE_AND_POLICY)
> +		res = obj_attr_v2_tag_merge_AND (info, abfd, subsec, lhs, rhs,
> +		  frozen);
> +	      else if (policy == SUBSECTION_TESTING_MERGE_OR_POLICY)
> +		res = obj_attr_v2_tag_merge_OR (info, abfd, subsec, lhs, rhs,
> +		  frozen);
> +	      else if (policy == SUBSECTION_TESTING_MERGE_ADD_POLICY)
> +		res = obj_attr_v2_tag_merge_ADD (info, abfd, subsec, lhs, rhs,
> +		  frozen);
> +	    }
> +	  else
> +	    res.reason = UNSUPPORTED;

... this first, you can save a level of indentation, improving readability.

> +	}
> +    }
> +
> +  return res;
> +}
> +
> +/* Append a new default-initialized attribute with the same key as AREF to the
> +   given subsection.  */
> +static void
> +oav2_subsection_append_attr_default (struct bfd_link_info *info,
> +				     obj_attr_subsection_v2 *s_abfd_missing,
> +				     obj_attr_v2 *a_ref)
> +{
> +  obj_attr_v2 *new_attr = oav2_attr_default (info, s_abfd_missing, a_ref);
> +  LINKED_LIST_APPEND(obj_attr_v2) (s_abfd_missing, new_attr);
> +}
> +
> +/* Return a new default-initialized subsection with the same parameters as
> +   SUBSEC.  */
> +static obj_attr_subsection_v2 *
> +oav2_subsection_default_new (struct bfd_link_info *info,
> +			     obj_attr_subsection_v2 *subsec)
> +{
> +  obj_attr_subsection_v2 *new_subsec
> +    = _bfd_elf_obj_attr_subsection_v2_init (subsec->name, subsec->scope,
> +      subsec->optional, subsec->encoding);

Nit: For the pending open parenthesis this line wants indenting yet another
level.

> +/* 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 *be = get_elf_backend_data (abfd);
> +  const char* tag_s = obj_attr_v2_tag_to_string (be, s_ref->name, a_ref->tag);
> +  if (s_ref->encoding == OA_ENC_ULEB128)
> +    {
> +      if (tag_s)
> +	info->callbacks->einfo (
> +	  _("%X%pB, %pB: error: mismatching values 0x%x and 0x%x for "

In case I didn't say so elsewhere already: Please prefer %#x over 0x%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)
> +{
> +  if (! s_abfd->optional)
> +    {
> +      info->callbacks->einfo
> +	(_("%X%pB: error: missing required object attributes subsection %s\n"),
> +	 ref_bfd, s_abfd->name);
> +      return false;
> +    }
> +
> +  /* Compute default values of the missing attributes in REF, but present in
> +     ABFD, and merge REF's generated subsection with the one of ABFD.  */
> +  obj_attr_subsection_v2 *s_ref = oav2_subsection_default_new (info, s_abfd);
> +  obj_attr_subsection_v2 *s_merged
> +    = handle_subsection_merge (info, ref_bfd, abfd, s_ref, s_abfd, s_frozen);
> +  BFD_ASSERT (s_merged == s_ref); // FIXME: I am not sure whether that it is true or false. If true, eliminate next free.

Such wants sorting before this can go in.

> +/* Check for mismatch between the parameters of subsections S1 and S2.
> +   Note: F1 can be null when comparing FROZEN and the first object file used to
> +   store the merge result.  If an error is reported, it means that one of the
> +   definition of S1 or S2 is corrupted.  Most likely S2 because it is a user
> +   input, or S1 if it is a programmation error of FROZEN.  In the second case,
> +   please raise a bug to binutils bug tracker.  */
> +static bool
> +oav2_subsection_mismatching_params (struct bfd_link_info *info,
> +				    bfd *f1, bfd *f2,
> +				    obj_attr_subsection_v2 *s1,
> +				    obj_attr_subsection_v2 *s2)
> +{
> +  if (! gnu_testing_namespace (s1->name))
> +    {
> +      /* Check whether the subsection is known, and if so, match against the
> +	 expected properties.
> +	 Note: this piece of code must be guarded against gnu-testing
> +	 subsections, as the backend method looks up at the known subsections.
> +	 Since the "fictive" entry for gnu-testing known subsection has random
> +	 values for its encoding and optionality, it won't be able to detect
> +	 mismatching parameters correctly.  */
> +      bool match_known = true;
> +      if (get_elf_backend_data (f2)->obj_attr_subsection_v2_match_known != NULL)
> +	match_known = get_elf_backend_data (f2)
> +	  ->obj_attr_subsection_v2_match_known (info, f2, s2);
> +      if (! match_known)
> +	return true;
> +    }
> +
> +  bool mismatch = (s1->encoding != s2->encoding
> +		   || s1->optional != s2->optional);
> +
> +  if (mismatch)
> +    {
> +      if (f1 != NULL)
> +	info->callbacks->einfo (_("%X%pB, %pB: error: parameters of subsection"
> +	  " '%s' are mismatching. (%s, %s) VS (%s, %s)\n"), f1, f2, s1->name,

I'm not a native speaker, but I think it wants to be "mismatched" here, much
like you have ...

> +	  oav2_comprehension_to_string (s1->optional),
> +	  oav2_encoding_to_string (s1->encoding),
> +	  oav2_comprehension_to_string (s2->optional),
> +	  oav2_encoding_to_string (s2->encoding));
> +      else
> +	info->callbacks->einfo (_("%X%pB: error: parameters of subsection"
> +	  " '%s' are corrupted. (%s, %s) VS (%s, %s)\n"), f2, s1->name,

... "corrupted" here.

> +/* Merge object attributes from FROZEN into the object file REF_BFD.
> +   Note: this function is called only once before starting the merge process
> +   between the object files.  REF_BFD is used to store the result of the merge,
> +   but REF_BFD is also an input file, so any mismatch against FROZEN should be
> +   raised before the values of REF_BFD be modified.  */
> +static bool
> +oav2_subsections_merge_frozen (struct bfd_link_info *info,
> +			       bfd *abfd,
> +			       obj_attr_subsection_v2 *s_frozen)
> +{
> +  if (s_frozen == NULL)
> +    return true;
> +
> +  bool success = true;
> +
> +  obj_attr_subsection_v2 *s_abfd = elf_obj_attr_subsections (abfd).first;
> +  while (s_frozen != NULL && s_abfd != NULL)
> +    {
> +      int cmp = strcmp (s_abfd->name, s_frozen->name);
> +      if (cmp < 0) /* ABFD has a subsection that FROZEN doesn't have.  */
> +	{
> +	  /* No need to try to merge anything here.  */
> +	  s_abfd = s_abfd->next;
> +	}
> +      else if (cmp > 0) /* FROZEN has a subsection that ABFD doesn't have.  */
> +	{
> +	  success &= handle_subsection_additional (info, abfd, abfd,
> +	    s_abfd, s_frozen, s_frozen);
> +	  s_frozen = s_frozen->next;
> +	}
> +      else /* Both ABFD and frozen have the subsection.  */
> +	{
> +	  bool mismatch = oav2_subsection_mismatching_params (info, NULL, abfd,
> +	    s_frozen, s_abfd);
> +	  success &= ! mismatch;
> +	  if (mismatch)
> +	    /* FROZEN cannot be corrupted as it is generated from the command
> +	       line arguments.  If it is corrupted, it is a bug.  */
> +	    s_abfd->status = obj_attr_subsection_v2_corrupted;
> +	  else
> +	    success &= (handle_subsection_merge (info, abfd, abfd,
> +	      s_abfd, s_frozen, s_frozen) != NULL);
> +
> +	  s_abfd = s_abfd->next;
> +	  s_frozen = s_frozen->next;
> +	}
> +    }
> +
> +  /* No need to go through the remaining sections of ABFD, only mismatch against
> +     FROZEN are interesting.  */

Nit (grammmar): Either "mismatches" or "is".

> +  for (; s_frozen != NULL; s_frozen = s_frozen->next)
> +    success &= handle_subsection_additional (info, abfd, abfd,
> +      elf_obj_attr_subsections (abfd).last, s_frozen, s_frozen);
> +
> +  return success;
> +}
> +
> +/* Merge object attributes from object file ABFD into REF_BFD.  */
> +static bool
> +oav2_subsections_merge (struct bfd_link_info *info, bfd *ref_bfd, bfd *abfd)
> +{
> +  bool success = true;
> +  obj_attr_subsection_list *out_frozen_subsecs
> +    = &elf_obj_attr_subsections (info->output_bfd);
> +  obj_attr_subsection_list *abfd_subsecs = &elf_obj_attr_subsections (abfd);
> +  obj_attr_subsection_list *ref_subsecs = &elf_obj_attr_subsections (ref_bfd);
> +
> +  obj_attr_subsection_v2 *s_frozen_first = out_frozen_subsecs->first;
> +  obj_attr_subsection_v2 *s_abfd = abfd_subsecs->first;
> +  obj_attr_subsection_v2 *s_ref = ref_subsecs->first;
> +
> +  /* Translate object attributes from abfd to GNU properties if they have an
> +     equivalence.  */
> +  _bfd_elf_translate_relevant_obj_attrs_to_gnu_props (abfd);

How does this call fit here? Below you're only dealing with attributes,
afaict.

> +  while (s_abfd != NULL && s_ref != NULL)
> +    {
> +      int cmp = strcmp (s_ref->name, s_abfd->name);
> +
> +      if (cmp < 0) /* REF has a subsection that ABFD doesn't have.  */
> +	{
> +	  if (s_ref->status != obj_attr_subsection_v2_ok)
> +	    {
> +	      s_ref = s_ref->next;
> +	      continue;
> +	    }
> +
> +	  obj_attr_subsection_v2 *s_frozen
> +	    = obj_attr_subsection_v2_find_by_name (s_frozen_first, s_ref->name,
> +						   true);
> +
> +	  /* Mismatching between REF and FROZEN already done in
> +	     oav2_subsections_merge_frozen.  */
> +	  success &= handle_subsection_missing (info, ref_bfd, abfd, s_ref,
> +	    s_frozen);
> +
> +	  if (s_frozen != NULL)
> +	    s_frozen_first = s_frozen->next;
> +	  s_ref = s_ref->next;
> +	}
> +      else if (cmp > 0) /* ABFD has a subsection that REF doesn't have.  */
> +	{
> +	  if (s_abfd->status != obj_attr_subsection_v2_ok)
> +	    {
> +	      s_abfd = s_abfd->next;
> +	      continue;
> +	    }
> +
> +	  obj_attr_subsection_v2 *s_frozen
> +	    = obj_attr_subsection_v2_find_by_name (s_frozen_first, s_abfd->name,
> +						   true);
> +	  if (s_frozen != NULL)
> +	    {
> +	      /* Check any mismatch against ABFD and FROZEN.  */
> +	      bool mismatch = oav2_subsection_mismatching_params (info, NULL,
> +		abfd, s_frozen, s_abfd);
> +	      success &= ! mismatch;
> +	      if (mismatch)
> +		s_abfd->status = obj_attr_subsection_v2_corrupted;
> +	      else
> +		success &= handle_subsection_additional (info, ref_bfd, abfd,
> +		  s_ref, s_abfd, s_frozen);

This and ...

> +	    }
> +	  else
> +	    success &= handle_subsection_additional (info, ref_bfd, abfd, s_ref,
> +	      s_abfd, NULL);

... this look to be easy to fold into just a single call. (Same pattern again
at least once further down.)

> +/* Compact duplicated tag declarations in a same subsection.
> +   Return True on success, False if any issue is found during the compaction,
> +   i.e. conflicting values for the same tag.  */
> +static bool
> +oav2_compact_tags (bfd *abfd, obj_attr_subsection_v2 *subsec)
> +{
> +  bool success = true;
> +
> +
> +  for (obj_attr_v2 *a = subsec->first;
> +       a != NULL && a->next != NULL;)
> +    {
> +      if (a->tag != a->next->tag)
> +	{
> +	  a = a->next;
> +	  continue;
> +	}
> +
> +      if (subsec->encoding == OA_ENC_ULEB128)
> +	{
> +	  if (a->vals.uint_val != a->next->vals.uint_val)
> +	    {
> +	      success = false;
> +	      _bfd_error_handler (_("%pB: error: found duplicated attributes "
> +		"'Tag_unknown_%u' with conflicting values (0x%x vs 0x%x) in "

What does it follow from that the tag is unknown?

> +		"subsection %s"), abfd, a->tag, a->vals.uint_val,
> +		a->next->vals.uint_val, subsec->name);
> +	    }
> +	  else
> +	    LINKED_LIST_REMOVE(obj_attr_v2) (subsec, a->next);
> +	}
> +      else /* (subsec->encoding == NTBS)  */
> +	{
> +	  if (strcmp (a->vals.string_val, a->next->vals.string_val) != 0)

Code in obj_attr_v2_tag_merge_ADD() suggests that vals.string_val can be NULL,
in which case it would be invalid to pass to strcmp().

> +/* Merge two subsections together (object attributes v2 only).
> +   The result is stored into subsec1.  subsec2 is destroyed.
> +   Return true if the merge was successful, false otherwise.
> +   Note: subsec1 and subsec2 are expected to be sorted before the call to this
> +   function.  */
> +static bool
> +oav2_subsection_destructive_merge (bfd *abfd,
> +				   obj_attr_subsection_v2 *subsec1,
> +				   obj_attr_subsection_v2 *subsec2)
> +{
> +  BFD_ASSERT (subsec1->encoding == subsec2->encoding
> +	   && subsec1->optional == subsec2->optional);
> +
> +  bool success = true;
> +
> +  success &= oav2_compact_tags (abfd, subsec1);
> +  success &= oav2_compact_tags (abfd, subsec2);
> +
> +  obj_attr_v2 *a1 = subsec1->first;
> +  obj_attr_v2 *a2 = subsec2->first;
> +  while (a1 != NULL && a2 != NULL)
> +    {
> +      if (a1->tag < a2->tag)
> +	{} /* Nothing to do, a1 is already in subsec1.  */
> +      else if (a1->tag > a2->tag)
> +	{
> +	  /* a2 is missing in subsec1, add it.  */
> +	  obj_attr_v2 *previous = LINKED_LIST_REMOVE(obj_attr_v2) (subsec2, a2);
> +	  LINKED_LIST_INSERT_BEFORE(obj_attr_v2) (subsec1, a2, a1);
> +	  a2 = previous;
> +	}
> +      else
> +	{
> +	  if (subsec1->encoding == OA_ENC_ULEB128
> +	   && a1->vals.uint_val != a2->vals.uint_val)

Nit: Indentation.

> +	    {
> +	      success = false;
> +	      _bfd_error_handler
> +		(_("%pB: error: found 2 subsections with the same name '%s' "
> +		   "and found conflicting values (%#x vs %#x) for object "
> +		   "attribute 'Tag_unknown_%u'"),
> +		   abfd, subsec1->name, a1->vals.uint_val, a2->vals.uint_val,
> +		   a1->tag);
> +	    }
> +	  else if (subsec1->encoding == OA_ENC_NTBS
> +	    && strcmp (a1->vals.string_val, a2->vals.string_val) != 0)

Again.

> @@ -1146,9 +2619,9 @@ oav2_parse_subsection (bfd *abfd,
>      ? OA_SUBSEC_PUBLIC
>      : OA_SUBSEC_PRIVATE;
>  
> -  obj_attr_subsection_v2 *subsec =
> -    _bfd_elf_obj_attr_subsection_v2_init (subsection_name, scope, optional_raw,
> -					  attr_type_raw);
> +  obj_attr_subsection_v2 *subsec
> +    = _bfd_elf_obj_attr_subsection_v2_init (subsection_name, scope,
> +					    optional_raw, attr_type_raw);
>    while (cursor < end)
>      {
>        BufferReadOp_t op_ = oav2_parse_attr (abfd, cursor, end, attr_type_raw);

Misplaced format adjustment?

Once again an overly large patch, close to impossible to sensibly review.

Jan