[PATCH v4 1/2] ld: bfd: sframe: KEEP .sframe sections and support gc-sections

[Claudiu Zissulescu]

>> @@ -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.

So, either I can keep the function to _bfd_elf_parse_sframe return but I 
need to change it to reflect the status if a frame is parsed or not, or 
to remove the returning and let _bfd_elf_discard_section_sframe to deal 
with it.

Anyhow, I'll rework this part of the patch to clarify it.

Thank you for finding this,
Claudiu