[PATCH 3/3] ld: Cleanup sframe_decoder_init_func_bfdinfo use of cookie->rel

[Jan Beulich]

On 25.02.2025 13:40, Hans-Peter Nilsson wrote:
> On Tue, 25 Feb 2025, Jan Beulich wrote:
> 
>> On 24.02.2025 18:44, Hans-Peter Nilsson wrote:
>>> On Fri, 21 Feb 2025, Jens Remus wrote:
>>>> diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
>>>> index 67ed874e0252..db166d485f9e 100644
>>>> --- a/bfd/elf-sframe.c
>>>> +++ b/bfd/elf-sframe.c
>>>> @@ -118,9 +118,9 @@ sframe_decoder_init_func_bfdinfo (bfd *abfd,
>>>>    if ((sec->flags & SEC_LINKER_CREATED) && cookie->rels == NULL)
>>>>      return true;
>>>>  
>>>> +  cookie->rel = cookie->rels;
>>>>    for (i = 0; i < fde_count; i++)
>>>>      {
>>>> -      cookie->rel = cookie->rels + i;
>>>>        BFD_ASSERT (cookie->rel < cookie->relend);
>>>
>>> That BFD_ASSERT looks like it should be updated or moved after 
>>> that change, or else it won't work as a kind of bounds-check 
>>> within the loop.
>>
>> Why would it not work as such anymore? Aiui it has the exact same effect
>> as before.
> 
> Key phrase being "within the loop" and a differing view of what 
> "work" means.  While it has the exact same *effect*, the 
> *intent* was apparently to bounds-check cookie->rel before being 
> used after the assignment with the loop variable.  But, 
> cookie->rel is no longer assigned within the loop.

It still is (and hence why the change here is correct), at the bottom
of the loop body.

>  Thus it 
> should be moved together with the cookie->rel assignment.

That would be wrong; it would check only the initial value then. I'd
view such a moved assertion as largely pointless.

Jan

> Is that perhaps something to which you can agree?
> 
> brgds, H-P