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

[Jan Beulich]

On 17.11.2025 18:36, Matthieu Longo wrote:
> On 14/11/2025 07:46, Jan Beulich wrote:
>> On 13.11.2025 18:49, Matthieu Longo wrote:
>>> On 31/10/2025 11:44, Jan Beulich wrote:
>>>> On 01.09.2025 18:56, Matthieu Longo wrote:
>>>>> @@ -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?
>>>>
>>>
>>> Indeed I can move the filling of the structure inside
>>> bfd_has_build_attributes().  I moved res->pbfd inside
>>> bfd_has_build_attributes and added comments to explain better what this
>>> boolean means.
>>>
>>> /* Search for the first input object file containing object attributes.
>>>      If no such object is found, PBFD points to the last object file that
>>>      could have contained object attributes.  HAS_OBJECT_ATTRIBUTES allows
>>>      to distinguish the cases when PBFD contains or does not contain object
>>>      attributes.  If no candidate file is found, PBFD will stay NULL.  */
>>> static bfd_search_result_t
>>> bfd_linear_find_first_with_obj_attrs (const struct bfd_link_info *info)
>>> ...
>>>
>>>> Further, is has_build_attributes actually necessary as a separate field? Can't
>>>> pbfd (being NULL or non-NULL) fulfill its purpose?
>>>>
>>>
>>> When pbfd is NULL, it means that no candidate file to host an OAv2
>>> section was found.
>>> Has_object_attributes means that the file contains object attributes
>>> that are not necessarily part of an OAv2 section (sec might be NULL).
>>> For instance, they could have been added by the translation of GNU
>>> properties to OAv2.
>>>
>>>> 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.
>>>>
>>>
>>> 1. How does GNU Properties handle the case of several sections with the
>>> expected type, but with the same expected name, or different names ?
>>>
>>> Regarding the parsing of GNU properties, if several sections with
>>> NT_GNU_PROPERTY_TYPE_0 are met, all of them are deserialized and their
>>> content is appended to the list of GNU properties for this BFD input.
>>> Regarding the merge of those properties,
>>> _bfd_elf_link_setup_gnu_properties() does not seem to harmonize them
>>> before merging them. However, elf_merge_gnu_property_list() seems to
>>> handle their merge. I might be wrong, I find this code quite difficult
>>> to follow to be honest.
>>> In _bfd_elf_link_setup_gnu_properties(), the GNU properties section is
>>> searched only by name. The section type is never checked. If it does not
>>> find one with the expected name, it creates a new one.
>>
>> For anything that's pre-existing we've lost anyway. I would hope though
>> that we could learn from past mistakes. See how e.g. SFrame has gained
>> its own section type, lack of which was one of the points I made during
>> review.
>>
> 
> SHT_AARCH64_ATTRIBUTES is used for the deserialization. All sections with this type will be deserialized correctly, but if .ARM.attributes is not found, it is created as a part of the merge process. I don't see any major impact with looking up the expected name of the section.
> I am unsure whether this answers your question or not.

Looking up by name is okay-ish (that's how lookup works, after all). But
the section type then would then want checking in addition.

>>> I propose the following change to the existing implementation to clarify
>>> this situation, and document what GNU ld does.
>>> Please let me know if you think that it is clear enough.
>>
>> I'm sorry (applicable both here and below): While I understand the good
>> intention behind presenting such incremental diff-s, they're of pretty
>> limited use to me. (Part of the problem being that you mailer badly
>> line-wraps things, btw.) I will want to see the next version of the patch,
>> even if this means another reviewing round afterwards.
>>
> 
> I use thunderbird, and haven't done anything special. Only copy-pasted the diff. There should be no line wrapping happening. At least, when I read my email with thunderbird, nothing is visible.

Go look at the list archives, e.g.
https://sourceware.org/pipermail/binutils/2025-November/145646.html. TB
does require some non-default settings to keep it from wrapping lines
unduly.

Jan