[PATCH v4 1/2] ld: bfd: sframe: KEEP .sframe sections and support gc-sections
Jan Beulich
jbeulich@suse.com
Tue Jan 27 15:10:54 GMT 2026
On 27.01.2026 15:48, Claudiu Zissulescu wrote:
>>> @@ -15302,15 +15320,13 @@ bfd_elf_discard_info (bfd *output_bfd, struct bfd_link_info *info)
>>> if (!init_reloc_cookie_for_section (&cookie, info, i, false))
>>> return -1;
>>>
>>> - if (_bfd_elf_parse_sframe (abfd, info, i, &cookie))
>>> + _bfd_elf_parse_sframe (abfd, info, i, &cookie);
>>
>> If the return value of the function becomes irrelevant, the function should
>> imo be changed to return void. First of course it wants to become clear why
>> the return value suddenly doesn't matter anymore. The description indeed
>> mentions this, but in a way that suggests that now the return value should
>> be checked at the other, new call site. Furthermore "already parsed" isn't
>> anything the function appears to be handling. Instead it looks as if upon
>> re-invocation it would do all the work again, leaking what was allocated on
>> the first run through (and potentially emitting the same error message
>> again). What am I overlooking?
>>
>> Imo, if "already parsed" needs handling, it should result in the function
>> returning true. After all, for the call here
>> - if it may also not have been called before, its return value would still
>> be relevant here,
>> - if it was guaranteed to have been called before, why would you call it a
>> 2nd time at all?
>
> The issue here is that the _bfd_elf_parse_sframe return false for either
> an error during parsing or if the sframe section is already parsed.
> Thus, TRUE is only returned when the input sframe is freshly parsed in
> the function call. I think this is an error.
This would likely be a mistake, yes, but: Going through the function's
return statements I can't spot one which would be for "already parsed".
Or wait, there is one with a comment "This file does not contain .sframe
information." Yet the sec->sec_info_type there isn't about that at all,
which I think is pretty misleading.
The one case where returning "false" for "already parsed" might make
sense is when there are two SFrame sections in an object, which - iirc -
we can't deal with right now. But since here this is for a single
section, I think the function wants to return true when
sec->sec_info_type == SEC_INFO_TYPE_SFRAME. And of course the comment
wants correcting.
Jan
More information about the Binutils
mailing list