Subject: lib/29536: libpthread's sigaction implementation is broken
To: None <lib-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Martin Husemann <martin@duskware.de>
List: netbsd-bugs
Date: 02/26/2005 15:17:01
>Number:         29536
>Category:       lib
>Synopsis:       libpthread's sigaction implementation is broken
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Feb 26 15:17:01 +0000 2005
>Originator:     Martin Husemann
>Release:        NetBSD 2.99.16
>Organization:
>Environment:
System: NetBSD sunny-weather.duskware.de 2.99.16 NetBSD 2.99.16 (SUNNY) #5: Tue Feb 22 18:25:41 CET 2005 martin@sunny-weather.duskware.de:/usr/src/sys/arch/sparc64/compile/SUNNY sparc64
Architecture: sparc64
Machine: sparc64
>Description:

After I spent quite some time to fix all perl "make test" problems specific to
sparc64, I now had a look at the single remaining (non-arch dependend)
failure. It is test #5 of ext/POSIX/sigaction.

Looking at the perl code for that test is not enough, you have to take into
account the (wiered) implementation of their sigaction() wrapper (to be
found in ext/POSIX.xs in the perl source). For reasons I don't understand
they split the sigaction into two parts - one to fetch the old settings,
and another to set the new one. Some perl interpreter magic in between
and to keep the whole thing atomic a sigprocmask to block everything before
and another to unblock afterwards.

Below you find a C program that reproduces the problem - but only if linked
against libpthread! The non-threaded version works as expected. I guess
we should add a variant of this to the regression tests...

>How-To-Repeat:
Use this test program:

--8<--
#include <stdlib.h>
#include <signal.h>

static void handler(int sig, siginfo_t *info, void *ctx)
{
}

int my_sigaction(int sig, const struct sigaction *act, struct sigaction *old)
{
	int res;
	sigset_t sset, osset;

	sigfillset(&sset);
	sigprocmask(SIG_BLOCK, &sset, &osset);
	res = sigaction(sig, NULL, old);
	if (res == 0)
		res = sigaction(sig, act, NULL);
	sigprocmask(SIG_SETMASK, &osset, NULL);
	return res;
}

void main()
{
  struct sigaction act, oact;
  memset(&act, 0, sizeof act);
  memset(&oact, 0, sizeof oact);
  act.sa_sigaction = handler;
  sigemptyset(&act.sa_mask);
  sigaddset(&act.sa_mask, SIGUSR1);
  act.sa_flags = SA_SIGINFO;
  
  my_sigaction(SIGHUP, &act, &oact);
  my_sigaction(SIGHUP, &act, &oact);

  if (oact.sa_flags & SA_SIGINFO && oact.sa_sigaction == handler)
    printf("4 ok\n");
  else
    printf("4 failed\n");

  if (sigismember(&oact.sa_mask, SIGUSR1))
    printf("5 ok\n");
  else
    printf("5 failed\n");
}
-->8--

and compile it with and without -pthread:

[~/sigact] martin@sunny-weather > cc -pthread sigaction.c
sigaction.c: In function `main':
sigaction.c:25: warning: return type of `main' is not `int'
[~/sigact] martin@sunny-weather > ./a.out 
4 ok
5 failed
[~/sigact] martin@sunny-weather > cc sigaction.c
sigaction.c: In function `main':
sigaction.c:25: warning: return type of `main' is not `int'
[~/sigact] martin@sunny-weather > ./a.out 
4 ok
5 ok

>Fix:
n/a