Date: Sat, 21 Oct 2017 23:39:21 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Andreas Tobler <andreast@FreeBSD.org> Cc: Tijl Coosemans <tijl@FreeBSD.org>, freebsd-current@FreeBSD.org, gerald@FreeBSD.org Subject: Re: Segfault in _Unwind_* code called from pthread_exit Message-ID: <20171021203921.GD2473@kib.kiev.ua> In-Reply-To: <b52eaeb1-f293-11ce-0ca6-a006b5fb51f5@FreeBSD.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> <20170826184034.GR1700@kib.kiev.ua> <b52eaeb1-f293-11ce-0ca6-a006b5fb51f5@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Oct 21, 2017 at 10:02:38PM +0200, Andreas Tobler 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@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?
Nothing was done AFAIK.
>
> 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?20171021203921.GD2473>