Re: Segfault in _Unwind_* code called from pthread_exit

On 22.10.17 02:18, Tijl Coosemans wrote:
> On Sat, 21 Oct 2017 22:02:38 +0200 Andreas Tobler <andreast_at_FreeBSD.org> wrote:
>> On 26.08.17 20:40, Konstantin Belousov wrote:
>>> On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote:
>>>> On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov <kostikbel_at_gmail.com> wrote:
>>>>> How does llvm unwinder detects that the return address is a garbage ?
>>>>
>>>> It just stops unwinding when it can't find frame information (stored in
>>>> .eh_frame sections).  GCC unwinder doesn't give up yet and checks if the
>>>> return address points to the signal trampoline (which means the current
>>>> frame is that of a signal handler).  It has built-in knowledge of how to
>>>> unwind to the signal trampoline frame.
>>> So llvm just gives up on signal frames ?
>>>    
>>>> A noreturn attribute isn't enough.  You can still unwind such functions.
>>>> They are allowed to throw exceptions for example.
>>> Ok.
>>>    
>>>> I did consider using
>>>> a CFI directive (see patch below) and it works, but it's architecture
>>>> specific and it's inserted after the function prologue so there's still
>>>> a window of a few instructions where a stack unwinder will try to use
>>>> the return address.
>>>>
>>>> Index: lib/libthr/thread/thr_create.c
>>>> ===================================================================
>>>> --- lib/libthr/thread/thr_create.c      (revision 322802)
>>>> +++ lib/libthr/thread/thr_create.c      (working copy)
>>>> _at__at_ -251,6 +251,7 _at__at_ create_stack(struct pthread_attr *pattr)
>>>>    static void
>>>>    thread_start(struct pthread *curthread)
>>>>    {
>>>> +       __asm(".cfi_undefined %rip");
>>>>           sigset_t set;
>>>>    
>>>>           if (curthread->attr.suspend == THR_CREATE_SUSPENDED)
>>>
>>> I like this approach much more than the previous patch.  What can be
>>> done is to provide asm trampoline which calls thread_start().  There you
>>> can add the .cfi_undefined right at the entry.
>>>
>>> It is somewhat more work than just setting the return address on the
>>> kernel-constructed pseudo stack frame, but I believe this is ultimately
>>> correct way.  You still can do it only on some arches, if you do not
>>> have incentive to code asm for all of them.
>>>
>>> Also crt1 probably should get the same treatment, despite we already set
>>> %rbp to zero AFAIR.
>>
>> Did some commit result out of this discussion or is this subject still
>> under investigation?
>>
>> Curious because I got this gcc PR:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635

If I add the above to lib/libthr/thread/thr_create.c the mentioned PR works.

> Sorry, but I didn't and won't have time to work on this.

Np.

> Ideally I think there should be a function attribute to mark functions
> as entry points.  The compiler would add ".cfi_undefined %rip" to such
> functions (and maybe optimise the function prologue because there are
> no caller registers that need to be preserved).  If you have connections
> in the GCC community maybe you could discuss that with them.

Well, from my understanding I'd have to teach every compiler to do so, 
right? (Beside that I do not know how to.)

I think we need another solution to find out if an unwind context is 
garbage.

I'll take a look at how llvm does this w/o segfaulting.

Thx,
Andreas