Date: Thu, 02 Sep 2010 17:03:13 +0000 From: David Xu <davidxu@freebsd.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r212076 - head/lib/libthr/thread Message-ID: <4C7FD8D1.1000702@freebsd.org> In-Reply-To: <20100902084116.GJ2396@deviant.kiev.zoral.com.ua> References: <201009010218.o812IX5G048257@svn.freebsd.org> <20100902081136.GG2396@deviant.kiev.zoral.com.ua> <4C7FD232.9080100@freebsd.org> <20100902084116.GJ2396@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Kostik Belousov wrote: > On Thu, Sep 02, 2010 at 04:34:58PM +0000, David Xu wrote: >> Kostik Belousov wrote: >>> On Wed, Sep 01, 2010 at 02:18:33AM +0000, David Xu wrote: >>>> Author: davidxu >>>> Date: Wed Sep 1 02:18:33 2010 >>>> New Revision: 212076 >>>> URL: http://svn.freebsd.org/changeset/base/212076 >>>> >>>> Log: >>>> Add signal handler wrapper, the reason to add it becauses there are >>>> some cases we want to improve: >>>> 1) if a thread signal got a signal while in cancellation point, >>>> it is possible the TDP_WAKEUP may be eaten by signal handler >>>> if the handler called some interruptibly system calls. >>>> 2) In signal handler, we want to disable cancellation. >>>> 3) When thread holding some low level locks, it is better to >>>> disable signal, those code need not to worry reentrancy, >>>> sigprocmask system call is avoided because it is a bit expensive. >>>> The signal handler wrapper works in this way: >>>> 1) libthr installs its signal handler if user code invokes sigaction >>>> to install its handler, the user handler is recorded in internal >>>> array. >>>> 2) when a signal is delivered, libthr's signal handler is invoke, >>>> libthr checks if thread holds some low level lock or is in critical >>>> region, if it is true, the signal is buffered, and all signals are >>>> masked, once the thread leaves critical region, correct signal >>>> mask is restored and buffered signal is processed. >>>> 3) before user signal handler is invoked, cancellation is temporarily >>>> disabled, after user signal handler is returned, cancellation state >>>> is restored, and pending cancellation is rescheduled. >>>> +static void >>>> +thr_sighandler(int sig, siginfo_t *info, void *_ucp) >>>> +{ >>>> + if ((actp->sa_flags & SA_SIGINFO) != 0) >>>> + (*(sigfunc))(sig, info, ucp); >>>> + else { >>>> + ((ohandler)(*sigfunc))( >>>> + sig, info->si_code, (struct sigcontext *)ucp, >>>> + info->si_addr, (__sighandler_t *)sigfunc); >>>> + } >>> I do not think this is very important, but freebsd old-style signal >>> handler fourth argument is usually the faulted %eip value. This is >>> most likely irrelevant for any source that is linked with libthr.so >>> new enough to contain this change. >> Isn't the si_addr in siginfo a fault address ? I remembered I saved >> the fault address in ksiginfo_t which is converted to userland >> siginfo, and fault address should be there. what's wrong here ? >> > Oops, sorry, I miscalculated the position of the arguments :(. Sorry, I think I also misunderstood you too. :( I think kernel still has some compatible problems. I just skimmed it again, and I found a problem. In RELENG_4, I found sys/i386/i386/machdep.c has following code in sendsig(): sf.sf_signum = sig; sf.sf_ucontext = (register_t)&sfp->sf_uc; if (SIGISMEMBER(p->p_sigacts->ps_siginfo, sig)) { /* Signal handler installed with SA_SIGINFO. */ sf.sf_siginfo = (register_t)&sfp->sf_si; sf.sf_ahu.sf_action = (__siginfohandler_t *)catcher; /* fill siginfo structure */ sf.sf_si.si_signo = sig; sf.sf_si.si_code = code; sf.sf_si.si_addr = (void*)regs->tf_err; } else { /* Old FreeBSD-style arguments. */ sf.sf_siginfo = code; sf.sf_addr = regs->tf_err; <------- sf.sf_ahu.sf_handler = catcher; } the sf.sf_addr is assigned by tf_err here. In later branch ( I don't know which ), it seems it uses ksi.ksi_addr, /* Old FreeBSD-style arguments. */ sf.sf_arg2 = ksi->ksi_code; sf.sf_addr = (register_t)ksi->ksi_addr; sf.sf_ahu.sf_handler = catcher; the tf_err may not be equal to ksi_addr! This may need to be fixed.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C7FD8D1.1000702>