Date: Sat, 26 Aug 2017 02:44:42 +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: <20170825234442.GO1700@kib.kiev.ua> In-Reply-To: <20170825173851.09116ddc@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>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Aug 25, 2017 at 05:38:51PM +0200, Tijl Coosemans wrote: > So both GCC and LLVM unwinding look up the return address in the CFI > table and fail when the return address is garbage, but LLVM treats this > as an end-of-stack condition while GCC further tries to see if the > return address points to a signal trampoline by testing the instruction > bytes at that address. On amd64 the garbage address is unreadable so it > segfaults. On i386 it is readable, the test fails and GCC returns > end-of-stack. How does llvm unwinder detects that the return address is a garbage ? > > To fix the crash and get predictable behaviour in the other cases I > propose always setting the return address to 0. The attached patch does > this for i386 and amd64. I don't know if other architectures need a > similar patch. > Index: sys/amd64/amd64/vm_machdep.c > =================================================================== > --- sys/amd64/amd64/vm_machdep.c (revision 322802) > +++ sys/amd64/amd64/vm_machdep.c (working copy) > @@ -507,6 +507,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void * > (((uintptr_t)stack->ss_sp + stack->ss_size - 4) & ~0x0f) - 4; > td->td_frame->tf_rip = (uintptr_t)entry; > > + /* Sentinel return address to stop stack unwinding. */ > + suword32((void *)td->td_frame->tf_rsp, 0); > + > /* Pass the argument to the entry point. */ > suword32((void *)(td->td_frame->tf_rsp + sizeof(int32_t)), > (uint32_t)(uintptr_t)arg); > @@ -529,6 +532,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void * > td->td_frame->tf_fs = _ufssel; > td->td_frame->tf_gs = _ugssel; > td->td_frame->tf_flags = TF_HASSEGS; > + > + /* Sentinel return address to stop stack unwinding. */ > + suword((void *)td->td_frame->tf_rsp, 0); > > /* Pass the argument to the entry point. */ > td->td_frame->tf_rdi = (register_t)arg; > Index: sys/i386/i386/vm_machdep.c > =================================================================== > --- sys/i386/i386/vm_machdep.c (revision 322802) > +++ sys/i386/i386/vm_machdep.c (working copy) > @@ -524,6 +524,9 @@ cpu_set_upcall(struct thread *td, void (*entry)(void * > (((int)stack->ss_sp + stack->ss_size - 4) & ~0x0f) - 4; > td->td_frame->tf_eip = (int)entry; > > + /* Sentinel return address to stop stack unwinding. */ > + suword((void *)td->td_frame->tf_esp, 0); > + > /* Pass the argument to the entry point. */ > suword((void *)(td->td_frame->tf_esp + sizeof(void *)), > (int)arg); I do not object against this, but I believe that a better solution exists for the system side (putting my change for gcc unwinder to detect the signal frame aside). The thread_start() sentinel in libthr should get proper dwarf annotation of not having the return address. May be normal function attributes of no return are enough to force compilers to generate required unwind data. Might be some more magic with inline asm and .cfi_return_column set to undefined.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170825234442.GO1700>