From owner-freebsd-hackers@FreeBSD.ORG Sun Jul 11 22:35:01 2010 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DBA921065675 for ; Sun, 11 Jul 2010 22:35:01 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id 9F6548FC20 for ; Sun, 11 Jul 2010 22:35:01 +0000 (UTC) Received: by iwn35 with SMTP id 35so5027050iwn.13 for ; Sun, 11 Jul 2010 15:35:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=Fz3dgQdr70qiaMtg0G+NWqDF8kV2+OqsfqIr+e6Y2u0=; b=nRZx+XHN2eD7yZiUdc3BD2jiI7QPOI2Q0e1U8t7Hxq6Uptvyfc1hpAmwfAUuJv66pR +M34rvTGKnTk7B5W/Tz34Lep42GOq8aifC28UFhNtWfoOF/Lqj97wv8HhWyuhldzhriv za1zB6ehSrYdV5KkRfQ3IFsGm7QzJyQU4nvf8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=dtVBzD6NcrAxpxFgKkQCkVf4ZK/czKubvCHhg/0K+9tI4cmX1HA84OHxgi17tQYGIZ TxH3RGZNrlP5tzceqdJavPUMNmI3yNIkzLklstXbUYrCtFWmdwJPGJlOqQn97I9n4BCY 5B75YxnqwPEIT4d0Dj7qf+C+/fb4gTmwUK+VA= MIME-Version: 1.0 Received: by 10.231.17.13 with SMTP id q13mr12581020iba.14.1278887700951; Sun, 11 Jul 2010 15:35:00 -0700 (PDT) Received: by 10.231.192.147 with HTTP; Sun, 11 Jul 2010 15:35:00 -0700 (PDT) In-Reply-To: <20100711214016.GR2408@deviant.kiev.zoral.com.ua> References: <20100711210843.GQ2408@deviant.kiev.zoral.com.ua> <20100711214016.GR2408@deviant.kiev.zoral.com.ua> Date: Sun, 11 Jul 2010 15:35:00 -0700 Message-ID: From: Garrett Cooper To: Kostik Belousov Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: hackers@freebsd.org Subject: Re: *sigpause hanging on 8.x+ X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 11 Jul 2010 22:35:01 -0000 2010/7/11 Kostik Belousov : > On Sun, Jul 11, 2010 at 02:30:01PM -0700, Garrett Cooper wrote: >> On Sun, Jul 11, 2010 at 2:08 PM, Kostik Belousov w= rote: >> > On Sun, Jul 11, 2010 at 12:39:39PM -0700, Garrett Cooper wrote: >> >> So, long story short... I've basically ported the open posix testsuit= e >> >> to FreeBSD, and one of the tests tests out sigpause. Unfortunately th= e >> >> sucker hangs on my dev box at home. >> >> >> >> I've written a short testcase that demonstrates this. It prints out: >> >> >> >> $ ~/test_sigpause >> >> 0 >> >> >> >> And proceeds to be unresponsive to signals (except SIGSTOP / SIGKILL, >> >> as expected). >> >> >> >> When I monkey around with libc's compat4.3 stuff a bit, this is what = comes up: >> >> >> >> $ env LD_LIBRARY_PATH=3D$PWD:/usr/src/lib/libc/../libthr ~/test_sigpa= use >> >> 0 >> >> before sigemptyset >> >> before _sigsuspend >> >> >> >> So it's getting stuck after calling _sigsuspend. >> >> >> >> I tried the same thing on a i386 8-STABLE VM and it hangs as well. >> >> >> >> I tried applying similar printfs in libthr but it's not hitting that >> >> code at all (it's now responding to SIGTERM though, which is >> >> interesting, but not too interesting to me). >> >> >> >> I also wrote similar code that exercised the functionality in >> >> sigsuspend, by calling sigprocmask beforehand, and it works. >> >> >> >> Thoughts? >> >> >> >> -Garrett >> >> >> >> Dev machine: >> >> FreeBSD bayonetta.local 9.0-CURRENT FreeBSD 9.0-CURRENT #1 >> >> r206173:209901M: Sun Jul 11 04:18:42 PDT 2010 >> >> root@:/usr/obj/usr/src/sys/BAYONETTA =A0amd64 >> >> VM: >> >> FreeBSD starr-bastion.localdomain 8.0-STABLE FreeBSD 8.0-STABLE #0 >> >> r207913: Tue May 11 06:21:57 UTC 2010 >> >> root@starr-bastion.localdomain:/usr/obj/usr/src/sys/GENERIC =A0i386 >> >> >> >> Index: compat-43/sigcompat.c >> >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> >> --- compat-43/sigcompat.c =A0 =A0 (revision 206173) >> >> +++ compat-43/sigcompat.c =A0 =A0 (working copy) >> >> @@ -36,6 +36,7 @@ >> >> =A0#include "namespace.h" >> >> =A0#include >> >> =A0#include >> >> +#include >> >> =A0#include >> >> =A0#include "un-namespace.h" >> >> =A0#include "libc_private.h" >> >> @@ -102,7 +103,9 @@ >> >> =A0{ >> >> =A0 =A0 =A0 sigset_t set; >> >> >> >> + =A0 =A0 printf("before sigemptyset\n"); >> >> =A0 =A0 =A0 sigemptyset(&set); >> >> + =A0 =A0 printf("before _sigsuspend\n"); >> >> =A0 =A0 =A0 set.__bits[0] =3D mask; >> >> =A0 =A0 =A0 return (_sigsuspend(&set)); >> >> =A0} >> >> @@ -111,10 +114,16 @@ >> >> =A0xsi_sigpause(int sig) >> >> =A0{ >> >> =A0 =A0 =A0 sigset_t set; >> >> + =A0 =A0 int rc; >> >> >> >> + =A0 =A0 printf("before sigemptyset\n"); >> >> =A0 =A0 =A0 sigemptyset(&set); >> >> + =A0 =A0 printf("before sigaddset\n"); >> >> =A0 =A0 =A0 sigaddset(&set, sig); >> >> - =A0 =A0 return (_sigsuspend(&set)); >> >> + =A0 =A0 printf("before _sigsuspend\n"); >> >> + =A0 =A0 rc =3D (_sigsuspend(&set)); >> >> + =A0 =A0 printf("after _sigsuspend\n"); >> >> + =A0 =A0 return rc; >> >> =A0} >> >> >> >> =A0int >> >> >> >> $ cat ~/test_sigpause.c >> >> #include >> >> #include >> >> >> >> int >> >> main (void) >> >> { >> >> =A0 =A0 =A0 =A0 printf("0\n"); >> >> =A0 =A0 =A0 =A0 fflush(stdout); >> >> =A0 =A0 =A0 =A0 (void) sigpause(1); >> >> =A0 =A0 =A0 =A0 return 0; >> >> } >> >> $ cat ~/test_sigsuspend.c >> >> #include >> >> #include >> >> >> >> int >> >> main (void) >> >> { >> >> =A0 =A0 =A0 =A0 sigset_t oset; >> >> =A0 =A0 =A0 =A0 sigset_t nset; >> >> =A0 =A0 =A0 =A0 if (sigprocmask(1, &nset, &oset) =3D=3D -1) >> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "sigprocmask(-1, &nset, &oset)= "); >> >> =A0 =A0 =A0 =A0 if (sigprocmask(-1, &nset, &oset) =3D=3D -1) >> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "sigprocmask(-1, &nset, &oset)= "); >> >> =A0 =A0 =A0 =A0 return (sigsuspend(&nset)); >> >> } >> > >> > It seems I got a sigmask for sigpause inside the xsi_sigpause() backwa= rd. >> > On the other hand, I do not understand what is your issue with sigpaus= e(). >> >> The negative testcase from the open posix testsuite was setup so that >> setting sigpause(-1) would return -1 with EINVAL, according to the >> sig* manpages (-1 is an invalid signal of course). That isn't being >> triggered with either function today. >> >> 0 seems a bit wonky too (it's an invalid signal number). >> >> My bet is that values greater than SIGRTMAX aren't interpreted properly = either. > > I will add these checks, thanks. Much obliged :)... FWIW sigprocmask fails to do the right thing in detecting the signal number: $ ~/test_sigprocmask signo =3D -1 result not sane (0 !=3D -1, errno: 0 !=3D EINVAL) signo =3D 0 result not sane (0 !=3D -1, errno: 0 !=3D EINVAL) signo =3D 1 result sane signo =3D 9 result sane signo =3D 17 result sane signo =3D 65 result sane signo =3D 64 result sane signo =3D 66 result not sane (0 !=3D -1, errno: 0 !=3D EINVAL) Would this fix that? Index: sys/kern/kern_sig.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/kern/kern_sig.c (revision 206173) +++ sys/kern/kern_sig.c (working copy) @@ -988,6 +988,9 @@ struct proc *p; int error; + if (!_SIG_VALID(how)) + return (-EINVAL); + p =3D td->td_proc; if (!(flags & SIGPROCMASK_PROC_LOCKED)) PROC_LOCK(p); I'll look for more low-hanging fruit. >> > diff --git a/lib/libc/compat-43/sigcompat.c b/lib/libc/compat-43/sigco= mpat.c >> > index c3ba30a..bab9d5c 100644 >> > --- a/lib/libc/compat-43/sigcompat.c >> > +++ b/lib/libc/compat-43/sigcompat.c >> > @@ -111,9 +111,12 @@ int >> > =A0xsi_sigpause(int sig) >> > =A0{ >> > =A0 =A0 =A0 =A0sigset_t set; >> > + =A0 =A0 =A0 int error; >> > >> > - =A0 =A0 =A0 sigemptyset(&set); >> > - =A0 =A0 =A0 sigaddset(&set, sig); >> > + =A0 =A0 =A0 error =3D _sigprocmask(SIG_BLOCK, NULL, &set); >> > + =A0 =A0 =A0 if (error !=3D 0) >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (error); >> > + =A0 =A0 =A0 sigdelset(&set, sig); >> > =A0 =A0 =A0 =A0return (_sigsuspend(&set)); >> > =A0} >> >> Doesn't this violate the restore clause noted in the manpage? >> >> =A0 =A0 =A0The xsi_sigpause() function removes sig from the signal mask = of the call- >> =A0 =A0 =A0ing process and suspend the calling process until a signal is= received. >> =A0 =A0 =A0The xsi_sigpause() function restores the signal mask of the p= rocess to >> =A0 =A0 =A0its original state before returning. >> >> So if I had a sigset defined above with sig, then redefined it, I >> would be whacking the previous handler by passing in NULL to >> _sigprocmask, correct? If so, sigpause has issues too in its >> implementation. > No, not correct. Read the description of sigsuspend. Yeah, I was wrong here: The sigsuspend() system call temporarily changes the blocked signal ma= sk to the set to which sigmask points, and then waits for a signal to arrive; on return the previous set of masked signals is restored. The signal mask set is usually empty to indicate that all signals are to b= e unblocked for the duration of the call. > Also note that the sigprocmask call does not change process mask. Not so sure about this though: The sigprocmask() system call examines and/or changes the current sign= al mask (those signals that are blocked from delivery). Signals are bloc= ked if they are members of the current signal mask set. >> There's also some interesting SIGDELSET action going on in libthr's >> copy of _sigsuspend's with SIGCANCEL (apparently that's the unofficial >> alias for SIGRTMIN as defined by libthr), but that's a sidenote for >> the actual issue seen here. Here's the test app I wrote and executed above, just for future referen= ce: Thanks! -Garrett $ cat ~/test_sigprocmask.c #include #include #include #include #define TEST_SIGPROCMASK_POS(signo) do { \ printf("signo =3D %d ", signo); \ rc =3D sigprocmask(-1, NULL, &oset); \ if (rc !=3D 0) { \ printf("result not sane (%d !=3D 0, errno: %d)\n", \ rc, errno); \ } else \ printf("result sane\n"); \ } while (0) #define TEST_SIGPROCMASK_NEG(signo) do { \ printf("signo =3D %d ", signo); \ rc =3D sigprocmask(-1, NULL, &oset); \ if (rc !=3D -1 || errno !=3D EINVAL) { \ printf("result not sane (%d !=3D -1, " \ "errno: %d !=3D EINVAL)\n", \ rc, errno); \ } else \ printf("result sane\n"); \ } while (0) int main(void) { sigset_t oset; int rc; TEST_SIGPROCMASK_NEG(-1); TEST_SIGPROCMASK_NEG(0); TEST_SIGPROCMASK_POS(SIGHUP); /* The system quietly disallows SIGKILL or SIGSTOP to be blocked. */ TEST_SIGPROCMASK_POS(SIGKILL); TEST_SIGPROCMASK_POS(SIGSTOP); TEST_SIGPROCMASK_POS(SIGRTMIN); TEST_SIGPROCMASK_POS(SIGRTMIN-1); TEST_SIGPROCMASK_NEG(SIGRTMIN+1); return (0); }