Message ID | 20211110215923.52841-1-rzinsly@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | libgcc: fix backtrace fallback on PowerPC Big-endian. [PR103004] | expand |
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. > -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. > { > 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. > + /* 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)? Looks okay, but please update and resend. Segher
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,
diff --git a/libgcc/config/rs6000/linux-unwind.h b/libgcc/config/rs6000/linux-unwind.h index 8deccc1d650..131d2d9cb7e 100644 --- a/libgcc/config/rs6000/linux-unwind.h +++ b/libgcc/config/rs6000/linux-unwind.h @@ -395,14 +395,20 @@ struct frame_layout }; -void ppc_backchain_fallback (struct _Unwind_Context *context, void *a) +static void +ppc_backchain_fallback (struct _Unwind_Context *context, void *a) { 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; + /* 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; for (count = arg->count; current != NULL; current = current->backchain) diff --git a/libgcc/unwind.inc b/libgcc/unwind.inc index 456a5ee682f..dc2f9c13e97 100644 --- a/libgcc/unwind.inc +++ b/libgcc/unwind.inc @@ -160,12 +160,13 @@ _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc, /* Set up fs to describe the FDE for the caller of cur_context. */ code = uw_frame_state_for (context, &fs); - if (code != _URC_NO_REASON && code != _URC_END_OF_STACK) + if (code != _URC_NO_REASON && code != _URC_END_OF_STACK + && code != _URC_NORMAL_STOP) return _URC_FATAL_PHASE2_ERROR; /* Unwind successful. */ action = _UA_FORCE_UNWIND | _UA_CLEANUP_PHASE; - if (code == _URC_END_OF_STACK) + if (code == _URC_END_OF_STACK || code == _URC_NORMAL_STOP) action |= _UA_END_OF_STACK; stop_code = (*stop) (1, action, exc->exception_class, exc, context, stop_argument);