Date: Sat, 26 Aug 2017 21:40:34 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: 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: <20170826184034.GR1700@kib.kiev.ua> In-Reply-To: <20170826202813.1240a1ef@kalimero.tijl.coosemans.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170826184034.GR1700>