[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