RE: [Valgrind-users] New experimental skin: signal handler checker

[Nicholas N. <nj...@ca...>]

On Thu, 3 Jul 2003, Troels Walsted Hansen wrote:

> I tried your skin on RH8 + all current updates.
>
> It seems to work great and identified a potential deadlock (calling
> pthread_mutex_unlock() in a SIGINT/SIGTERM handler) in one of the
> applications I tried it with.
>
> Didn't encounter any problems.
>
> Definitely a nice addition to Valgrind's skin arsenal.

Great, thanks for the feedback, I'm glad it worked well for you.

----

Warning: here be Technical mumbo-jumbo...

In practice, I don't know how likely deadlock is if you call
pthread_mutex_unlock() from a signal handler, but the LinuxThreads
pthread_mutex_lock() man page says this:

  ASYNC-SIGNAL SAFETY
     The mutex functions are not async-signal safe. What this means is  that
     they should not be called from a signal handler. In particular, calling
     pthread_mutex_lock or pthread_mutex_unlock from a  signal  handler  may
     deadlock the calling thread.

Similar advice is given at www.burningvoid.com/iaq/posix-signals.html:

     One important safety tip is that pthreads constructs like mutexes,
     condition variables, etc. do not have defined behavior inside signal
     handlers. So you cannot use pthreads calls or synchronize with other
     threads while inside a signal handler.

Currently the only pthread functions in the blacklist are
pthread_mutex_{lock,unlock,trylock}, maybe others (eg.  condition variable
ones) should be too.  The blacklist currently looks like this:

   Char* bad_funcs[] = {
      "_IO_printf", "_IO_puts",
      "syslog", "strdup",
      "__libc_malloc", "__libc_calloc", "__libc_realloc", "__libc_free",
      "inet_ntoa", "gethostbyname", "gethostbyaddr", "getservbynam",
      "readdir", "__readdir", "__readdir64", "strtok",
      "ttyname", "getttyname", "getlogin", "cuserid",
      "getpwent", "getpwnam", "getpwuid",
      "getgrent", "getgrnam", "getgrgid",
      "asctime", "ctime", "gmtime", "localtime",
      "fclose", "fflush", "fflush_unlocked", "fopen", "freopen",
      "fdopen", "fopencookie", "fmemopen", "open_memstream",
      "fprintf", "vfprintf", "fscanf", "vfscanf", "fgetc", "getc",
      "getc_unlocked", "fgetc_unlocked", "fputc", "putc", "fputc_unlocked",
      "putc_unlocked", "getw", "putw", "fgets", "fputs", "ungetc", "fseek",
      "rewind",
      "__pthread_mutex_lock", "__pthread_mutex_unlock",
      "__pthread_mutex_trylock",
      NULL
   };

I'd welcome suggestions for new ones, or any corrections.

Crocus also has a --strict option, which uses a whitelist instead of a
blacklist;  any function called from a signal handler that's not on the
whitelist causes a warning.  Unfortunately, user-defined functions can be
reentrant so that's not such a good approach.  For completeness, and in
case you're having trouble sleeping, here's the current whitelist:

   /* From "Advanced Programming in the UNIX Environment", Stevens, p278.
      Those marked with a "+" are not specified by POSIX.1 as reentrant,
      but are listed in the SVR4 SVID. */
   Char* good_funcs[] = {
      "_exit", "abort"/*+*/, "access", "alarm",
      "cfgetispeed", "cfgetospeed", "cfsetispeed", "cfsetospeed",
      "chdir", "chmod", "chown", "close", "creat", "dup", "dup2", "execle",
      "execve", "exit"/*+*/, "fcntl", "fork", "fstat",
      "getegid", "geteuid", "getgid", "getgroups", "getpgrp", "getpid",
      "getppid", "getuid",
      "kill", "link", "longjmp"/*+*/, "lseek", "mkdir", "mkfifo", "open",
      "pathconf", "pause", "pipe", "read", "rename", "rmdir",
      "setgid", "setpgid", "setsid", "setuid",
      "__sigaction", "__libc_sigaction",
      "sigaddset", "sigdelset", "sigemptyset", "sigfillset",
      "sigismember", "signal"/*+*/, "sigpending",
      "__sigprocmask",
      "sigsuspend",
      "sleep", "stat", "sysconf",
      "tcdrain", "tcflow", "tcflush", "tcgetattr", "tcgetpgrp", "tcsendbreak",
      "tcsetattr", "tcsetpgrp", "time", "times", "umask", "uname", "unlink",
      "utime", "wait", "waitpid", "write",
      NULL
   };

And maybe I should use the term "signal safe" when talking about functions
that can be called from signal handlers, rather than "reentrant"?  Or are
they the same thing?  I haven't quite worked out the exact differences
between "signal safe", "thread safe", and "reentrant".  AFAICT, they're
all pretty similar, but there might be some functions that satisfy one but
not all the definitions?

N