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

[Jan Beulich]

On 14.11.2025 11:56, Matthieu Longo wrote:
> On 14/11/2025 07:46, Jan Beulich wrote:
>>>> Could this translation, at the very least, be split off of this
>>>> overly large patch?
>>> Done, but in my opinion, the benefit of it for the review is really
>>> minimal, and it makes the commit message even worse, because now,
>>> instead of having a whole description in one place, it is split in two.
>> Both to this and ...
>>
>>>>> +   ** 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.
>>> I am really not convinced about this split.
>>> Unless you consider it mandatory to move forward, I prefer to abstain
>>> from it.
>> ... this - splitting isn't mandatory, but as you have seen, I gave up reviewing
>> changes to one of the files here at some point. Solely reading through all of
>> your replies here has already taken excessively long, and I'm yet to write
>> replies where ones are needed. The bigger a change, the more likely that it'll
>> sit for a long time, since to review it one needs to find a big enough chunk of
>> time. Reviewing piecemeal is possible in theory, but doing a lot of reviews I
>> find that quite undesirable. IOW it is in your own interest to try and present
>> your work in manageable chunks.
> 
> What do you think about the following split ?
> 
> 1. The new types, the plumbing, i.e the whole body of 
> _bfd_elf_link_setup_object_attributes() as it is now, along with the 
> skeletons of the functions that it calls. The text description at the 
> top of the files includes everything, even if it is not implemented yet.
> 2. The implementation of phase 1: filling in the skeletons for the 
> finding of the first BFD with object attributes, creation of the section 
> if missing, sorting of the subsections, deduplication of subsections and 
> attributes. Any static functions called by those skeletons is also added.
> 3. The implementation of phase 2: filling all the skeletons related to 
> the merge, also adding the backend handlers and any static functions 
> called by those.
> 4. The implementation of phase 3: pruning of non-ok attributes and 
> subsections.
> 5. Add the translations from phase 1 and 3, i.e. change the body of 
> _bfd_elf_link_setup_object_attributes() and add new functions called 
> from _bfd_elf_link_setup_object_attributes() and new backend handlers.

Sounds plausible, but it is you who knows whether this will result in
each piece to be somewhat similar size (as splitting 4 tiny changes and
then still having a 5th giant patch won't help all that much).

> If you disagree with the split, please be explicit in the answer to 
> guide me. I am really struggling with this split. All those patches 
> would compile individually, but none would be really testable until 
> patch 4 at least, and I would consider the feature broken until patch 5. 
> Changing the tests between 4 and 5 is quite a lot of work, with zero 
> added value, so I would like to keep the tests as they are now.
> 
> Beyond the purpose of the code review, right before merging the changes 
> once you approved the patch series, should those patches be squashed 
> together into one as it is now ? Would it make more sense from a git 
> history perspective ?

Personally I prefer things to be committed as they were reviewed / approved.

Jan