[PATCH,V3 6/9] bfd: gas: ld: libsframe: adopt new encoding for FDE func start addr field

[Indu Bhagat]

On 6/16/25 11:24 PM, Jan Beulich wrote:
> On 17.06.2025 07:59, Indu Bhagat wrote:
>> On 6/16/25 10:28 PM, Jan Beulich wrote:
>>> On 16.06.2025 23:02, Indu Bhagat wrote:
>>>> On 6/16/25 12:23 AM, Jan Beulich wrote:
>>>>> On 16.06.2025 08:43, Indu Bhagat wrote:
>>>>>> On 6/13/25 5:39 AM, Jan Beulich wrote:
>>>>>>> On 13.06.2025 09:32, Indu Bhagat wrote:
>>>>>>>> --- a/bfd/elf-sframe.c
>>>>>>>> +++ b/bfd/elf-sframe.c
>>>>>>>> @@ -328,6 +328,8 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
>>>>>>>>        int8_t sfd_ctx_fixed_ra_offset;
>>>>>>>>        uint8_t dctx_version;
>>>>>>>>        uint8_t ectx_version;
>>>>>>>> +  uint8_t dctx_flags;
>>>>>>>> +  uint8_t ectx_flags;
>>>>>>>>        int encerr = 0;
>>>>>>>>           struct elf_link_hash_table *htab;
>>>>>>>> @@ -351,6 +353,8 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
>>>>>>>>        if (sfd_ctx == NULL || sfe_info == NULL)
>>>>>>>>          return false;
>>>>>>>>      +  dctx_flags = sframe_decoder_get_flags (sfd_ctx);
>>>>>>>> +
>>>>>>>>        if (htab->sfe_info.sfe_ctx == NULL)
>>>>>>>>          {
>>>>>>>>            sfd_ctx_abi_arch = sframe_decoder_get_abi_arch (sfd_ctx);
>>>>>>>> @@ -361,8 +365,12 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
>>>>>>>>            if (!sfd_ctx_abi_arch)
>>>>>>>>          return false;
>>>>>>>>      +      /* Reset SFRAME_F_FDE_SORTED for the encoder context.  This will be set
>>>>>>>> +     later when FDEs are finally sorted before emission in the output
>>>>>>>> +     section.  */
>>>>>>>> +      uint8_t tflags = dctx_flags & ~SFRAME_F_FDE_SORTED;
>>>>>>>>            htab->sfe_info.sfe_ctx = sframe_encode (SFRAME_VERSION_2,
>>>>>>>> -                          0, /* SFrame flags.  */
>>>>>>>> +                          tflags, /* SFrame flags.  */
>>>>>>>>                                sfd_ctx_abi_arch,
>>>>>>>>                                sfd_ctx_fixed_fp_offset,
>>>>>>>>                                sfd_ctx_fixed_ra_offset,
>>>>>>> I'm likely confused here, too: Why would the one flag need special casing?
>>>>> You didn't address this question.
>>>>>
>>>> (Aplogies I mixed it up in the other paragraph, but here is why we special case SFRAME_F_FDE_SORTED...)
>>>>
>>>> Currently the three flags defined are such that_only_ SFRAME_F_FDE_SORTED needs reset at the time of linking, because this property in broken while the sections are being linked in memory (as the SFrame FDEs are being placed in the output section) until the eventual sort at write time.
>>> And why does the intermediate state of the flag matter?
>>
>> Other than reflecting the transient state of the in-memory contents more aptly (i.e., the in-memory SFrame FDEs in the SFrame encoder object, while linking is in progress, are not sorted in the order of their start PCs), there is no functional purpose of doing so.
> 
> Perhaps make the comment express this somehow, then? Doing so may avoid
> confusion later on.
> 

OK.  I have changed the comment to:

/* In-memory FDEs in the encoder object are unsorted during linking and
    will be sorted before emission.  Reset SFRAME_F_FDE_SORTED to aptly
    reflect that (doing so has no other functional value at this time
    though).  */

Thanks