Date: Sat, 21 Oct 2017 22:02:38 +0200 From: Andreas Tobler <andreast@FreeBSD.org> To: Konstantin Belousov <kostikbel@gmail.com>, Tijl Coosemans <tijl@FreeBSD.org> Cc: freebsd-current@FreeBSD.org, gerald@FreeBSD.org Subject: Re: Segfault in _Unwind_* code called from pthread_exit Message-ID: <b52eaeb1-f293-11ce-0ca6-a006b5fb51f5@FreeBSD.org> In-Reply-To: <20170826184034.GR1700@kib.kiev.ua> References: <20170823163707.096f93ab@kalimero.tijl.coosemans.org> <20170824154235.GD1700@kib.kiev.ua> <20170824180830.199885b0@kalimero.tijl.coosemans.org> <20170825173851.09116ddc@kalimero.tijl.coosemans.org> <20170825234442.GO1700@kib.kiev.ua> <20170826202813.1240a1ef@kalimero.tijl.coosemans.org> <20170826184034.GR1700@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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)
>> @@ -251,6 +251,7 @@ 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
Tia,
Andreas
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?b52eaeb1-f293-11ce-0ca6-a006b5fb51f5>