[PATCH] libgcc: fix backtrace fallback on PowerPC Big-endian. [PR103004]

[Raphael M Zinsly]

Hi Segher,

On 11/11/2021 10:43, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Nov 10, 2021 at 06:59:23PM -0300, Raphael Moreira Zinsly wrote:
>> At the end of the backtrace stream _Unwind_Find_FDE() may not be able
>> to find the frame unwind info and will later call the backtrace fallback
>> instead of finishing. This occurs when using an old libc on ppc64 due to
>> dl_iterate_phdr() not being able to set the fde in the last trace.
>> When this occurs the cfa of the trace will be behind of context's cfa.
>> Also, libgo’s probestackmaps() calls the backtrace with a null pointer
>> and can get to the backchain fallback with the same problem, in this case
>> we are only interested in find a stack map, we don't need nor can do a
>> backchain.
>> _Unwind_ForcedUnwind_Phase2() can hit the same issue as it uses
>> uw_frame_state_for(), so we need to treat _URC_NORMAL_STOP.
>>
>> libgcc/ChangeLog:
>>
>>           * config/rs6000/linux-unwind.h (ppc_backchain_fallback): turn into
>> 	 static to fix -Wmissing-prototypes. Check if it's called with a null
>> 	 argument or at the end of the backtrace and return.
>>           * unwind.inc (_Unwind_ForcedUnwind_Phase2): treat _URC_NORMAL_STOP.
> 
> Formatting is messed up.  Lines start with a capital.  Two spaces after
> full stop, while you're at it.
>

Ok.

>> -void ppc_backchain_fallback (struct _Unwind_Context *context, void *a)
>> +static void
>> +ppc_backchain_fallback (struct _Unwind_Context *context, void *a)
> 
> This was already fixed in 75ef0353a2d3.

Ops, missed that.

> 
>>   {
>>     struct frame_layout *current;
>>     struct trace_arg *arg = a;
>>     int count;
>>   
>> -  /* Get the last address computed and start with the next.  */
>> +  /* Get the last address computed.  */
>>     current = context->cfa;
> 
> Empty line after here please.  Most of the time if you have a full-line
> comment it means a new paragraph is starting.
> 

Ok.

>> +  /* If the trace CFA is not the context CFA the backtrace is done.  */
>> +  if (arg == NULL || arg->cfa != current)
>> +	return;
>> +
>> +  /* Start with next address.  */
>>     current = current->backchain;
> 
> Like you did here :-)
> 
> Do you have a testcase (that failed without this, but now doesn't)?
> 

I don't have a simple testcase for that, but many of the asan and go 
tests catch that.

> Looks okay, but please update and resend.
> 
> 
> Segher
> 

Thanks,
-- 
Raphael Moreira Zinsly