Re: [PATCH] s390: Remove backchain-based fallback from backtrace

[Florian Weimer <fweimer at redhat dot com>]

* Stefan Liebler:

> Hi Florian,
>
> is there a background story behind this patch or do you just want to
> consolidate the s390-32/s390-64 implementation?

I'm working to consolidate the dynamic linking of the unwinder for the
backtrace function and for pthread_cancel/pthread_exit.  That's how I
encountered the issue.

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

Ah, I somehow misread the code.  __backchain_backtrace does not actually
call backtrace_helper, so it does not depend on _Unwind_GetIP.

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

That sounds like a problem, but it's not the one I originally expected.

I still think the code change in my patch is valid because pretty much
all systems will have libgcc_s installed (it's required for C++, after
all), and thus the backchain processing is essentially dead.  It's only
used of dlopen fails (e.g., due to lack of file descriptors).  But
that's not useful because the fallback implements something completely
different.

> Do you know how other architectures behave if there are no
> unwind-tables?

There is considerable disagreement whether unwind tables (asynchronous
or not) are part of the ABI.  I think they are, but people trying to
build the smallest binaries disagree quite vehemently.

Some targets say that if a frame does not have unwinding information,
then the presence of a frame pointer must be assumed.  But that's far
from uniform.  On other targets, lack of unwinding information means
that the current function is a leaf function which does not change the
stack pointer.

> 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?

I think this has to be part of the libgcc_s unwinder because in many
environments, the system libraries have DWARF unwinding data, but other
objects may not have them and could use the backchain.  Then the
question becomes how to mark frames as using the backchain-based
unwinder.

Thanks,
Florian