[PATCH] s390: Remove backchain-based fallback from backtrace
Stefan Liebler
stli@linux.ibm.com
Tue Feb 11 15:31:00 GMT 2020
Hi Florian,
is there a background story behind this patch or do you just want to
consolidate the s390-32/s390-64 implementation?
On 2/10/20 7:04 PM, Florian Weimer wrote:
> Stefan,
>
> do you agree this change is correct? It's not really clear to me how
> the fallback unwinder can work given its dependency on _Unwind_GetIP.
The current situation:
In init, the libgcc_s.so.1 functions _Unwind_Backtrace and _Unwind_GetIP
are dlsym'ed. If _Unwind_GetIP is not available, then _Unwind_Backtrace
is also marked as not available.
In __backtrace, there is a check if _Unwind_Backtrace and implicitly
_Unwind_GetIP is available.
If not available, then the __backchain_backtrace is used.
Otherwise the _Unwind_Backtrace from libgcc is used.
__backchain_backtrace does not rely on libgcc_s.so.1.
Instead it tries to walk along the backchain (according to the ABI). But
I assume the usual binaries do not have those backchain pointers as you
have to use the gcc -mbackchain flag: "Maintain backchain pointer.". If
not activated, the corresponding backchain-slots on the stack are not
set and contain random values.
As far as I know, there are customers which build with this backchain
feature. Thus we don't want to remove it.
_Unwind_Backtrace from libgcc can't handle the backchain. It relies on
gcc -fasynchronous-unwind-tables.
That means, if somebody has no libgcc and has not build with
-mbackchain, then __backchain_backtrace will try to walk anywhere and we
could get segfaults as it seems the sanity checks (backchain-slot ==
NULL or backchain-slot > __libc_stack_end) are not sufficient!?
This also means, if somebody has build with backchain and has libgcc
installed, then always _Unwind_Backtrace is called.
If the binary was build with gcc -fno-asynchronous-unwind-tables, then
there is no retry with __backchain_backtrace and there won't be a backtrace.
If the binary contains the unwind-tables, then those are used and not
the backchain.
Do you know how other architectures behave if there are no unwind-tables?
Would it be possible to check if the current binaries have unwind-tables
and then use _Unwind_Backtrace if libgcc is available? If not,
__backchain_backtrace could be used as fallback.
Should __backchain_backtrace have more checks regarding addresses in
backchain-slot and/or the assumed function-address?
Other thoughts?
>
> I tested this on s390-linux-gnu and s390x-linux-gnu (but couldn't run
> the C++ tests on s390 due to test environment limitations).
>
> Thanks,
> Florian
> 8<------------------------------------------------------------------8<
> This backtrace variant is used when _Unwind_Backtrace is not found in
> libgcc_s. It still depends on _Unwind_GetIP from libgcc_s, so it does
> not work if loading libgcc_s fails. Therefore, as long as libgcc_s
> provides a definition of _Unwind_Backtrace, the fallback is not useful.
>
> After this change, the implementation is the same on s390 and s390x
> and can be shared.
>
> -----
> sysdeps/s390/{s390-32 => }/backtrace.c | 58 +------------
> sysdeps/s390/s390-64/backtrace.c | 147 ---------------------------------
> 2 files changed, 1 insertion(+), 204 deletions(-)
>
> diff --git a/sysdeps/s390/s390-32/backtrace.c b/sysdeps/s390/backtrace.c
> similarity index 60%
> rename from sysdeps/s390/s390-32/backtrace.c
> rename to sysdeps/s390/backtrace.c
> index 497e4f8875..5801e5e211 100644
> --- a/sysdeps/s390/s390-32/backtrace.c
> +++ b/sysdeps/s390/backtrace.c
> @@ -24,36 +24,6 @@
> #include <stdlib.h>
> #include <unwind.h>
>
> -/* This is a global variable set at program start time. It marks the
> - highest used stack address. */
> -extern void *__libc_stack_end;
> -
> -
> -/* This is the stack layout we see for every non-leaf function.
> - size offset
> - %r15 -> +------------------+
> - 4 | back chain | 0
> - 4 | end of stack | 4
> - 8 | glue | 8
> - 8 | scratch | 16
> - 40 | save area r6-r15 | 24
> - 16 | save area f4,f6 | 64
> - 16 | empty | 80
> - +------------------+
> - r14 in the save area holds the return address.
> -*/
> -
> -struct layout
> -{
> - int back_chain;
> - int end_of_stack;
> - int glue[2];
> - int scratch[2];
> - int save_grps[10];
> - int save_fp[4];
> - int empty[2];
> -};
> -
> struct trace_arg
> {
> void **array;
> @@ -77,32 +47,6 @@ init (void)
> if (unwind_getip == NULL)
> unwind_backtrace = NULL;
> }
> -
> -static int
> -__backchain_backtrace (void **array, int size)
> -{
> - /* We assume that all the code is generated with frame pointers set. */
> - struct layout *stack;
> - int cnt = 0;
> -
> - __asm__ ("LR %0,%%r15" : "=d" (stack) );
> - /* We skip the call to this function, it makes no sense to record it. */
> - stack = (struct layout *) stack->back_chain;
> - while (cnt < size)
> - {
> - if (stack == NULL || (void *) stack > __libc_stack_end)
> - /* This means the address is out of range. Note that for the
> - toplevel we see a frame pointer with value NULL which clearly is
> - out of range. */
> - break;
> -
> - array[cnt++] = (void *) (stack->save_grps[8] & 0x7fffffff);
> -
> - stack = (struct layout *) stack->back_chain;
> - }
> -
> - return cnt;
> -}
> #else
> # define unwind_backtrace _Unwind_Backtrace
> # define unwind_getip _Unwind_GetIP
> @@ -136,7 +80,7 @@ __backtrace (void **array, int size)
> __libc_once (once, init);
>
> if (unwind_backtrace == NULL)
> - return __backchain_backtrace (array, size);
> + return 0;
> #endif
>
> unwind_backtrace (backtrace_helper, &arg);
> diff --git a/sysdeps/s390/s390-64/backtrace.c b/sysdeps/s390/s390-64/backtrace.c
> deleted file mode 100644
> index 5d14e01280..0000000000
> --- a/sysdeps/s390/s390-64/backtrace.c
> +++ /dev/null
> @@ -1,147 +0,0 @@
> -/* Return backtrace of current program state. 64 bit S/390 version.
> - Copyright (C) 2001-2020 Free Software Foundation, Inc.
> - Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>.
> - This file is part of the GNU C Library.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <https://www.gnu.org/licenses/>. */
> -
> -#include <libc-lock.h>
> -#include <dlfcn.h>
> -#include <execinfo.h>
> -#include <stddef.h>
> -#include <stdlib.h>
> -#include <unwind.h>
> -
> -
> -/* This is a global variable set at program start time. It marks the
> - highest used stack address. */
> -extern void *__libc_stack_end;
> -
> -
> -/* This is the stack layout we see for every non-leaf function.
> - size offset
> - %r15 -> +------------------+
> - 8 | back chain | 0
> - 8 | end of stack | 8
> - 32 | scratch | 16
> - 80 | save area r6-r15 | 48
> - 16 | save area f4,f6 | 128
> - 16 | empty | 144
> - +------------------+
> - r14 in the save area holds the return address.
> -*/
> -
> -struct layout
> -{
> - long back_chain;
> - long end_of_stack;
> - long scratch[4];
> - long save_grps[10];
> - long save_fp[2];
> - long empty[2];
> -};
> -
> -struct trace_arg
> -{
> - void **array;
> - int cnt, size;
> -};
> -
> -#ifdef SHARED
> -static _Unwind_Reason_Code (*unwind_backtrace) (_Unwind_Trace_Fn, void *);
> -static _Unwind_Ptr (*unwind_getip) (struct _Unwind_Context *);
> -
> -static void
> -init (void)
> -{
> - void *handle = __libc_dlopen ("libgcc_s.so.1");
> -
> - if (handle == NULL)
> - return;
> -
> - unwind_backtrace = __libc_dlsym (handle, "_Unwind_Backtrace");
> - unwind_getip = __libc_dlsym (handle, "_Unwind_GetIP");
> - if (unwind_getip == NULL)
> - unwind_backtrace = NULL;
> -}
> -
> -static int
> -__backchain_backtrace (void **array, int size)
> -{
> - /* We assume that all the code is generated with frame pointers set. */
> - struct layout *stack;
> - int cnt = 0;
> -
> - __asm__ ("LGR %0,%%r15" : "=d" (stack) );
> - /* We skip the call to this function, it makes no sense to record it. */
> - stack = (struct layout *) stack->back_chain;
> - while (cnt < size)
> - {
> - if (stack == NULL || (void *) stack > __libc_stack_end)
> - /* This means the address is out of range. Note that for the
> - toplevel we see a frame pointer with value NULL which clearly is
> - out of range. */
> - break;
> -
> - array[cnt++] = (void *) stack->save_grps[8];
> -
> - stack = (struct layout *) stack->back_chain;
> - }
> -
> - return cnt;
> -}
> -#else
> -# define unwind_backtrace _Unwind_Backtrace
> -# define unwind_getip _Unwind_GetIP
> -#endif
> -
> -static _Unwind_Reason_Code
> -backtrace_helper (struct _Unwind_Context *ctx, void *a)
> -{
> - struct trace_arg *arg = a;
> -
> - /* We are first called with address in the __backtrace function.
> - Skip it. */
> - if (arg->cnt != -1)
> - arg->array[arg->cnt] = (void *) unwind_getip (ctx);
> - if (++arg->cnt == arg->size)
> - return _URC_END_OF_STACK;
> - return _URC_NO_REASON;
> -}
> -
> -int
> -__backtrace (void **array, int size)
> -{
> - struct trace_arg arg = { .array = array, .size = size, .cnt = -1 };
> -
> - if (size <= 0)
> - return 0;
> -
> -#ifdef SHARED
> - __libc_once_define (static, once);
> -
> - __libc_once (once, init);
> -
> - if (unwind_backtrace == NULL)
> - return __backchain_backtrace (array, size);
> -#endif
> -
> - unwind_backtrace (backtrace_helper, &arg);
> -
> - return arg.cnt != -1 ? arg.cnt : 0;
> -}
> -
> -weak_alias (__backtrace, backtrace)
> -libc_hidden_def (__backtrace)
>
More information about the Libc-alpha
mailing list