[review] Add RAII class for blocking gdb signals

Message ID gerrit.1571543710000.If3f37dc57dd859c226e9e4d79458a0514746e8c6@gnutoolchain-gerrit.osci.io
State New, archived
Headers

Commit Message

Simon Marchi (Code Review) Oct. 20, 2019, 3:55 a.m. UTC
  Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/168
......................................................................

Add RAII class for blocking gdb signals

This adds configury support and an RAII class that can be used to
temporarily block signals that are used by gdb.  (This class is not
used in this patch, but it split out for easier review.)

The idea of this patch is that these signals should only be delivered
to the main thread.  So, when creating a background thread, they are
temporarily blocked; the blocked state is inherited by the new thread.

The sigprocmask man page says:

    The use of sigprocmask() is unspecified in a multithreaded
    process; see pthread_sigmask(3).

This patch changes gdb to use pthread_sigmask when appropriate, by
introducing a convenience define.

I've updated gdbserver as well, because I had to touch gdbsupport, and
because the threading patches will make it link against the thread
library.

I chose not to touch the NTO code, because I don't know anything about
that platform and because I cannot test it.

Finally, this modifies an existing spot in the Guile layer to use the
new facility.

gdb/ChangeLog
2019-10-19  Tom Tromey  <tom@tromey.com>

	* gdbsupport/signals-state-save-restore.c (original_signal_mask):
	Remove comment.
	(save_original_signals_state, restore_original_signals_state): Use
	gdb_sigmask.
	* linux-nat.c (block_child_signals, restore_child_signals_mask)
	(_initialize_linux_nat): Use gdb_sigmask.
	* guile/guile.c (_initialize_guile): Use block_signals.
	* Makefile.in (HFILES_NO_SRCDIR): Add gdb-sigmask.h.
	* gdbsupport/gdb-sigmask.h: New file.
	* event-top.c (async_sigtstp_handler): Use gdb_sigmask.
	* cp-support.c (gdb_demangle): Use gdb_sigmask.
	* gdbsupport/common.m4 (GDB_AC_COMMON): Check for
	pthread_sigmask.
	* configure, config.in: Rebuild.
	* gdbsupport/block-signals.h: New file.

gdb/gdbserver/ChangeLog
2019-10-19  Tom Tromey  <tom@tromey.com>

	* remote-utils.c (block_unblock_async_io): Use gdb_sigmask.
	* linux-low.c (linux_wait_for_event_filtered, linux_async): Use
	gdb_sigmask.
	* configure, config.in: Rebuild.

Change-Id: If3f37dc57dd859c226e9e4d79458a0514746e8c6
---
M gdb/ChangeLog
M gdb/Makefile.in
M gdb/config.in
M gdb/configure
M gdb/cp-support.c
M gdb/event-top.c
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/config.in
M gdb/gdbserver/configure
M gdb/gdbserver/linux-low.c
M gdb/gdbserver/remote-utils.c
A gdb/gdbsupport/block-signals.h
M gdb/gdbsupport/common.m4
A gdb/gdbsupport/gdb-sigmask.h
M gdb/gdbsupport/signals-state-save-restore.c
M gdb/guile/guile.c
M gdb/linux-nat.c
17 files changed, 389 insertions(+), 38 deletions(-)
  

Comments

Simon Marchi (Code Review) Nov. 22, 2019, 5:40 p.m. UTC | #1
Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/168
......................................................................


Patch Set 3: Code-Review+2

(3 comments)

OK with a few minor remarks below.

| --- /dev/null
| +++ gdb/gdbsupport/block-signals.h
| @@ -1,0 +34,26 @@ public:
| +  block_signals ()
| +  {
| +#ifdef HAVE_PTHREAD_SIGMASK
| +    sigset_t mask;
| +    sigemptyset (&mask);
| +    sigaddset (&mask, SIGINT);
| +    sigaddset (&mask, SIGCHLD);
| +    sigaddset (&mask, SIGALRM);
| +    sigaddset (&mask, SIGWINCH);
| +    pthread_sigmask (SIG_BLOCK, &mask, &m_old_mask);

PS3, Line 43:

Shouldn't this be gdb_sigmask?

| +#endif
| +  }
| +
| +  ~block_signals ()
| +  {
| +#ifdef HAVE_PTHREAD_SIGMASK
| +    pthread_sigmask (SIG_SETMASK, &m_old_mask, nullptr);

PS3, Line 50:

Ditto.

| +#endif
| +  }
| +
| +  DISABLE_COPY_AND_ASSIGN (block_signals);
| +
| +private:
| +
| +#ifdef HAVE_PTHREAD_SIGMASK
| +  sigset_t m_old_mask;
| --- gdb/gdbsupport/signals-state-save-restore.c
| +++ gdb/gdbsupport/signals-state-save-restore.c
| @@ -20,21 +20,18 @@ #include "signals-state-save-restore.h"
| +#include "gdbsupport/gdb-sigmask.h"
|  
|  #include <signal.h>
|  
|  /* The original signal actions and mask.  */
|  
|  #ifdef HAVE_SIGACTION
|  static struct sigaction original_signal_actions[NSIG];
|  
| -/* Note that we use sigprocmask without worrying about threads because
| -   the save/restore functions are called either from main, or after a
| -   fork.  In both cases, we know the calling process is single
| -   threaded.  */

PS3, Line 31:

I think the comment is still relevant, even if it might need a tweak.
We don't bother to save/restore the masks of other threads, and the
original mask is thus saved in a global, because of what the comment
says.

|  static sigset_t original_signal_mask;
|  #endif
|  
|  /* See signals-state-save-restore.h.   */
|  
|  void
|  save_original_signals_state (bool quiet)
|  {
|  #ifdef HAVE_SIGACTION
  
Simon Marchi (Code Review) Nov. 22, 2019, 10:48 p.m. UTC | #2
Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/168
......................................................................


Patch Set 3:

(2 comments)

| --- /dev/null
| +++ gdb/gdbsupport/block-signals.h
| @@ -1,0 +34,19 @@ public:
| +  block_signals ()
| +  {
| +#ifdef HAVE_PTHREAD_SIGMASK
| +    sigset_t mask;
| +    sigemptyset (&mask);
| +    sigaddset (&mask, SIGINT);
| +    sigaddset (&mask, SIGCHLD);
| +    sigaddset (&mask, SIGALRM);
| +    sigaddset (&mask, SIGWINCH);
| +    pthread_sigmask (SIG_BLOCK, &mask, &m_old_mask);

PS3, Line 43:

Well, I thought perhaps not, since this class should only really do
anything
when using threads.  However, it's also harmless to change this, and
I suppose clearer -- so I have made the change.

| +#endif
| +  }
| +
| +  ~block_signals ()
| +  {
| +#ifdef HAVE_PTHREAD_SIGMASK
| +    pthread_sigmask (SIG_SETMASK, &m_old_mask, nullptr);
| +#endif
| +  }
| --- gdb/gdbsupport/signals-state-save-restore.c
| +++ gdb/gdbsupport/signals-state-save-restore.c
| @@ -20,21 +20,18 @@ #include "signals-state-save-restore.h"
| +#include "gdbsupport/gdb-sigmask.h"
|  
|  #include <signal.h>
|  
|  /* The original signal actions and mask.  */
|  
|  #ifdef HAVE_SIGACTION
|  static struct sigaction original_signal_actions[NSIG];
|  
| -/* Note that we use sigprocmask without worrying about threads because
| -   the save/restore functions are called either from main, or after a
| -   fork.  In both cases, we know the calling process is single
| -   threaded.  */

PS3, Line 31:

I rewrote the comment.

|  static sigset_t original_signal_mask;
|  #endif
|  
|  /* See signals-state-save-restore.h.   */
|  
|  void
|  save_original_signals_state (bool quiet)
|  {
|  #ifdef HAVE_SIGACTION
  
Simon Marchi (Code Review) Nov. 26, 2019, 3:41 p.m. UTC | #3
Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/168
......................................................................


Patch Set 4:

(2 comments)

| --- /dev/null
| +++ gdb/gdbsupport/block-signals.h
| @@ -1,0 +34,19 @@ public:
| +  block_signals ()
| +  {
| +#ifdef HAVE_PTHREAD_SIGMASK
| +    sigset_t mask;
| +    sigemptyset (&mask);
| +    sigaddset (&mask, SIGINT);
| +    sigaddset (&mask, SIGCHLD);
| +    sigaddset (&mask, SIGALRM);
| +    sigaddset (&mask, SIGWINCH);
| +    pthread_sigmask (SIG_BLOCK, &mask, &m_old_mask);

PS3, Line 43:

> Well, I thought perhaps not, since this class should only really do anything
> when using threads.  However, it's also harmless to change this, and
> I suppose clearer -- so I have made the change.

Yeah, both the file and class name give no hint that this is threads
related.  Also, the intro comment says that it can be used when
starting threads, but it doesn't say that it can't be used in whatever
other cases we might want to block signals.

I actually wondered whether this should block all signals instead of
just blocking a subset -- I was pondering whether the set of signals
might be target backend specific, for example.  But we can leave that
for if/when we need it.

| +#endif
| +  }
| +
| +  ~block_signals ()
| +  {
| +#ifdef HAVE_PTHREAD_SIGMASK
| +    pthread_sigmask (SIG_SETMASK, &m_old_mask, nullptr);
| +#endif
| +  }
| --- gdb/gdbsupport/signals-state-save-restore.c
| +++ gdb/gdbsupport/signals-state-save-restore.c
| @@ -20,21 +20,18 @@ #include "signals-state-save-restore.h"
| +#include "gdbsupport/gdb-sigmask.h"
|  
|  #include <signal.h>
|  
|  /* The original signal actions and mask.  */
|  
|  #ifdef HAVE_SIGACTION
|  static struct sigaction original_signal_actions[NSIG];
|  
| -/* Note that we use sigprocmask without worrying about threads because
| -   the save/restore functions are called either from main, or after a
| -   fork.  In both cases, we know the calling process is single
| -   threaded.  */

PS3, Line 31:

> I rewrote the comment.

I'm afraid I don't understand the new comment.  :-(

Maybe we should remove it afterall.

|  static sigset_t original_signal_mask;
|  #endif
|  
|  /* See signals-state-save-restore.h.   */
|  
|  void
|  save_original_signals_state (bool quiet)
|  {
|  #ifdef HAVE_SIGACTION
  
Simon Marchi (Code Review) Nov. 26, 2019, 4:53 p.m. UTC | #4
Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/168
......................................................................


Patch Set 4:

(1 comment)

I'll zap the comment again, unless you want to reword it yourself somehow.

I assume you meant to +2 this one?

| --- /dev/null
| +++ gdb/gdbsupport/block-signals.h
| @@ -1,0 +34,19 @@ public:
| +  block_signals ()
| +  {
| +#ifdef HAVE_PTHREAD_SIGMASK
| +    sigset_t mask;
| +    sigemptyset (&mask);
| +    sigaddset (&mask, SIGINT);
| +    sigaddset (&mask, SIGCHLD);
| +    sigaddset (&mask, SIGALRM);
| +    sigaddset (&mask, SIGWINCH);
| +    pthread_sigmask (SIG_BLOCK, &mask, &m_old_mask);

PS3, Line 43:

> I actually wondered whether this should block all signals instead of just blocking a subset -- I was pondering whether the set of signals might be target backend specific, for example.  But we can leave that for if/when we need it.

I think the Boehm GC needs some signal (or signals?) to be deliverable
in order to suspend threads.

| +#endif
| +  }
| +
| +  ~block_signals ()
| +  {
| +#ifdef HAVE_PTHREAD_SIGMASK
| +    pthread_sigmask (SIG_SETMASK, &m_old_mask, nullptr);
| +#endif
| +  }
  
Simon Marchi (Code Review) Nov. 26, 2019, 6:53 p.m. UTC | #5
Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/168
......................................................................


Patch Set 4: Code-Review+2

> I assume you meant to +2 this one?

Yeah.  Here is is now.
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0774312..710d7eb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,23 @@ 
 2019-10-19  Tom Tromey  <tom@tromey.com>
 
+	* gdbsupport/signals-state-save-restore.c (original_signal_mask):
+	Remove comment.
+	(save_original_signals_state, restore_original_signals_state): Use
+	gdb_sigmask.
+	* linux-nat.c (block_child_signals, restore_child_signals_mask)
+	(_initialize_linux_nat): Use gdb_sigmask.
+	* guile/guile.c (_initialize_guile): Use block_signals.
+	* Makefile.in (HFILES_NO_SRCDIR): Add gdb-sigmask.h.
+	* gdbsupport/gdb-sigmask.h: New file.
+	* event-top.c (async_sigtstp_handler): Use gdb_sigmask.
+	* cp-support.c (gdb_demangle): Use gdb_sigmask.
+	* gdbsupport/common.m4 (GDB_AC_COMMON): Check for
+	pthread_sigmask.
+	* configure, config.in: Rebuild.
+	* gdbsupport/block-signals.h: New file.
+
+2019-10-19  Tom Tromey  <tom@tromey.com>
+
 	* acinclude.m4: Include ax_pthread.m4.
 	* Makefile.in (PTHREAD_CFLAGS, PTHREAD_LIBS): New variables.
 	(INTERNAL_CFLAGS_BASE): Use PTHREAD_CFLAGS.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 5a409d2..4ace519 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1462,6 +1462,7 @@ 
 	gdbsupport/fileio.h \
 	gdbsupport/format.h \
 	gdbsupport/gdb-dlfcn.h \
+	gdbsupport/gdb-sigmask.h \
 	gdbsupport/gdb_assert.h \
 	gdbsupport/gdb_tilde_expand.h \
 	gdbsupport/gdb_locale.h \
diff --git a/gdb/config.in b/gdb/config.in
index 9b094fd..ff2fd12 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -345,6 +345,9 @@ 
 /* Have PTHREAD_PRIO_INHERIT. */
 #undef HAVE_PTHREAD_PRIO_INHERIT
 
+/* Define to 1 if you have the `pthread_sigmask' function. */
+#undef HAVE_PTHREAD_SIGMASK
+
 /* Define to 1 if you have the `ptrace64' function. */
 #undef HAVE_PTRACE64
 
diff --git a/gdb/configure b/gdb/configure
index 66cd0b8..5571cf0 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -2506,6 +2506,73 @@ 
   as_fn_set_status $ac_retval
 
 } # ac_fn_cxx_try_link
+
+# ac_fn_cxx_check_func LINENO FUNC VAR
+# ------------------------------------
+# Tests whether FUNC exists, setting the cache variable VAR accordingly
+ac_fn_cxx_check_func ()
+{
+  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
+$as_echo_n "checking for $2... " >&6; }
+if eval \${$3+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+/* Define $2 to an innocuous variant, in case <limits.h> declares $2.
+   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
+#define $2 innocuous_$2
+
+/* System header to define __stub macros and hopefully few prototypes,
+    which can conflict with char $2 (); below.
+    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+    <limits.h> exists even on freestanding compilers.  */
+
+#ifdef __STDC__
+# include <limits.h>
+#else
+# include <assert.h>
+#endif
+
+#undef $2
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char $2 ();
+/* The GNU C library defines this for functions which it implements
+    to always fail with ENOSYS.  Some functions are actually named
+    something starting with __ and the normal name is an alias.  */
+#if defined __stub_$2 || defined __stub___$2
+choke me
+#endif
+
+int
+main ()
+{
+return $2 ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_link "$LINENO"; then :
+  eval "$3=yes"
+else
+  eval "$3=no"
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+eval ac_res=\$$3
+	       { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
+
+} # ac_fn_cxx_check_func
 cat >config.log <<_ACEOF
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
@@ -14317,6 +14384,21 @@ 
 fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gdb_cv_cxx_std_thread" >&5
 $as_echo "$gdb_cv_cxx_std_thread" >&6; }
+
+    # This check must be here, while LIBS includes any necessary
+    # threading library.
+    for ac_func in pthread_sigmask
+do :
+  ac_fn_cxx_check_func "$LINENO" "pthread_sigmask" "ac_cv_func_pthread_sigmask"
+if test "x$ac_cv_func_pthread_sigmask" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_PTHREAD_SIGMASK 1
+_ACEOF
+
+fi
+done
+
+
     LIBS="$save_LIBS"
     CXXFLAGS="$save_CXXFLAGS"
   fi
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 253369b..c298d26 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -37,6 +37,7 @@ 
 #include "gdbsupport/gdb_setjmp.h"
 #include "safe-ctype.h"
 #include "gdbsupport/selftest.h"
+#include "gdbsupport/gdb-sigmask.h"
 
 #define d_left(dc) (dc)->u.s_binary.left
 #define d_right(dc) (dc)->u.s_binary.right
@@ -1573,7 +1574,7 @@ 
 	  sigset_t segv_sig_set;
 	  sigemptyset (&segv_sig_set);
 	  sigaddset (&segv_sig_set, SIGSEGV);
-	  sigprocmask (SIG_UNBLOCK, &segv_sig_set, NULL);
+	  gdb_sigmask (SIG_UNBLOCK, &segv_sig_set, NULL);
 #endif
 
 	  if (!error_reported)
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 0396dbc..6c6e0ff 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -40,6 +40,7 @@ 
 #include "gdbsupport/buffer.h"
 #include "ser-event.h"
 #include "gdb_select.h"
+#include "gdbsupport/gdb-sigmask.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -1127,7 +1128,7 @@ 
     sigset_t zero;
 
     sigemptyset (&zero);
-    sigprocmask (SIG_SETMASK, &zero, 0);
+    gdb_sigmask (SIG_SETMASK, &zero, 0);
   }
 #elif HAVE_SIGSETMASK
   sigsetmask (0);
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 768aaf7..28c3608 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,12 @@ 
 2019-10-19  Tom Tromey  <tom@tromey.com>
 
+	* remote-utils.c (block_unblock_async_io): Use gdb_sigmask.
+	* linux-low.c (linux_wait_for_event_filtered, linux_async): Use
+	gdb_sigmask.
+	* configure, config.in: Rebuild.
+
+2019-10-19  Tom Tromey  <tom@tromey.com>
+
 	* Makefile.in (PTHREAD_CFLAGS, PTHREAD_LIBS): New variables.
 	(INTERNAL_CFLAGS_BASE): Use PTHREAD_CFLAGS.
 	(GDBSERVER_LIBS): Use PTHREAD_LIBS.
diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index 4276bce..3027ffa 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -195,6 +195,9 @@ 
 /* Have PTHREAD_PRIO_INHERIT. */
 #undef HAVE_PTHREAD_PRIO_INHERIT
 
+/* Define to 1 if you have the `pthread_sigmask' function. */
+#undef HAVE_PTHREAD_SIGMASK
+
 /* Define if the target supports PTRACE_GETFPXREGS for extended register
    access. */
 #undef HAVE_PTRACE_GETFPXREGS
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 019552d..260e8c0 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -1972,6 +1972,119 @@ 
 
 } # ac_fn_c_check_decl
 
+# ac_fn_cxx_try_link LINENO
+# -------------------------
+# Try to link conftest.$ac_ext, and return whether this succeeded.
+ac_fn_cxx_try_link ()
+{
+  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
+  rm -f conftest.$ac_objext conftest$ac_exeext
+  if { { ac_try="$ac_link"
+case "(($ac_try" in
+  *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+  *) ac_try_echo=$ac_try;;
+esac
+eval ac_try_echo="\"\$as_me:${as_lineno-$LINENO}: $ac_try_echo\""
+$as_echo "$ac_try_echo"; } >&5
+  (eval "$ac_link") 2>conftest.err
+  ac_status=$?
+  if test -s conftest.err; then
+    grep -v '^ *+' conftest.err >conftest.er1
+    cat conftest.er1 >&5
+    mv -f conftest.er1 conftest.err
+  fi
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; } && {
+	 test -z "$ac_cxx_werror_flag" ||
+	 test ! -s conftest.err
+       } && test -s conftest$ac_exeext && {
+	 test "$cross_compiling" = yes ||
+	 test -x conftest$ac_exeext
+       }; then :
+  ac_retval=0
+else
+  $as_echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+	ac_retval=1
+fi
+  # Delete the IPA/IPO (Inter Procedural Analysis/Optimization) information
+  # created by the PGI compiler (conftest_ipa8_conftest.oo), as it would
+  # interfere with the next link command; also delete a directory that is
+  # left behind by Apple's compiler.  We do this before executing the actions.
+  rm -rf conftest.dSYM conftest_ipa8_conftest.oo
+  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
+  as_fn_set_status $ac_retval
+
+} # ac_fn_cxx_try_link
+
+# ac_fn_cxx_check_func LINENO FUNC VAR
+# ------------------------------------
+# Tests whether FUNC exists, setting the cache variable VAR accordingly
+ac_fn_cxx_check_func ()
+{
+  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
+$as_echo_n "checking for $2... " >&6; }
+if eval \${$3+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+/* Define $2 to an innocuous variant, in case <limits.h> declares $2.
+   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
+#define $2 innocuous_$2
+
+/* System header to define __stub macros and hopefully few prototypes,
+    which can conflict with char $2 (); below.
+    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+    <limits.h> exists even on freestanding compilers.  */
+
+#ifdef __STDC__
+# include <limits.h>
+#else
+# include <assert.h>
+#endif
+
+#undef $2
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char $2 ();
+/* The GNU C library defines this for functions which it implements
+    to always fail with ENOSYS.  Some functions are actually named
+    something starting with __ and the normal name is an alias.  */
+#if defined __stub_$2 || defined __stub___$2
+choke me
+#endif
+
+int
+main ()
+{
+return $2 ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_link "$LINENO"; then :
+  eval "$3=yes"
+else
+  eval "$3=no"
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+eval ac_res=\$$3
+	       { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
+
+} # ac_fn_cxx_check_func
+
 # ac_fn_c_compute_int LINENO EXPR VAR INCLUDES
 # --------------------------------------------
 # Tries to find the compile-time value of EXPR in a program that includes
@@ -7609,6 +7722,21 @@ 
 fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gdb_cv_cxx_std_thread" >&5
 $as_echo "$gdb_cv_cxx_std_thread" >&6; }
+
+    # This check must be here, while LIBS includes any necessary
+    # threading library.
+    for ac_func in pthread_sigmask
+do :
+  ac_fn_cxx_check_func "$LINENO" "pthread_sigmask" "ac_cv_func_pthread_sigmask"
+if test "x$ac_cv_func_pthread_sigmask" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_PTHREAD_SIGMASK 1
+_ACEOF
+
+fi
+done
+
+
     LIBS="$save_LIBS"
     CXXFLAGS="$save_CXXFLAGS"
   fi
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 0e4b14e..9a4c267 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -50,6 +50,7 @@ 
 #include "gdbsupport/common-inferior.h"
 #include "nat/fork-inferior.h"
 #include "gdbsupport/environ.h"
+#include "gdbsupport/gdb-sigmask.h"
 #include "gdbsupport/scoped_restore.h"
 #ifndef ELFMAG0
 /* Don't include <linux/elf.h> here.  If it got included by gdb_proc_service.h
@@ -2689,7 +2690,7 @@ 
   /* Make sure SIGCHLD is blocked until the sigsuspend below.  Block
      all signals while here.  */
   sigfillset (&block_mask);
-  sigprocmask (SIG_BLOCK, &block_mask, &prev_mask);
+  gdb_sigmask (SIG_BLOCK, &block_mask, &prev_mask);
 
   /* Always pull all events out of the kernel.  We'll randomly select
      an event LWP out of all that have events, to prevent
@@ -2775,7 +2776,7 @@ 
 	{
 	  if (debug_threads)
 	    debug_printf ("LLW: exit (no unwaited-for LWP)\n");
-	  sigprocmask (SIG_SETMASK, &prev_mask, NULL);
+	  gdb_sigmask (SIG_SETMASK, &prev_mask, NULL);
 	  return -1;
 	}
 
@@ -2785,7 +2786,7 @@ 
 	  if (debug_threads)
 	    debug_printf ("WNOHANG set, no event found\n");
 
-	  sigprocmask (SIG_SETMASK, &prev_mask, NULL);
+	  gdb_sigmask (SIG_SETMASK, &prev_mask, NULL);
 	  return 0;
 	}
 
@@ -2794,11 +2795,11 @@ 
 	debug_printf ("sigsuspend'ing\n");
 
       sigsuspend (&prev_mask);
-      sigprocmask (SIG_SETMASK, &prev_mask, NULL);
+      gdb_sigmask (SIG_SETMASK, &prev_mask, NULL);
       goto retry;
     }
 
-  sigprocmask (SIG_SETMASK, &prev_mask, NULL);
+  gdb_sigmask (SIG_SETMASK, &prev_mask, NULL);
 
   current_thread = event_thread;
 
@@ -6215,7 +6216,7 @@ 
       sigemptyset (&mask);
       sigaddset (&mask, SIGCHLD);
 
-      sigprocmask (SIG_BLOCK, &mask, NULL);
+      gdb_sigmask (SIG_BLOCK, &mask, NULL);
 
       if (enable)
 	{
@@ -6223,7 +6224,7 @@ 
 	    {
 	      linux_event_pipe[0] = -1;
 	      linux_event_pipe[1] = -1;
-	      sigprocmask (SIG_UNBLOCK, &mask, NULL);
+	      gdb_sigmask (SIG_UNBLOCK, &mask, NULL);
 
 	      warning ("creating event pipe failed.");
 	      return previous;
@@ -6249,7 +6250,7 @@ 
 	  linux_event_pipe[1] = -1;
 	}
 
-      sigprocmask (SIG_UNBLOCK, &mask, NULL);
+      gdb_sigmask (SIG_UNBLOCK, &mask, NULL);
     }
 
   return previous;
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index d7da4b7..c7f97f3 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -28,6 +28,7 @@ 
 #include "gdbsupport/rsp-low.h"
 #include "gdbsupport/netstuff.h"
 #include "gdbsupport/filestuff.h"
+#include "gdbsupport/gdb-sigmask.h"
 #include <ctype.h>
 #if HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
@@ -807,7 +808,7 @@ 
 
   sigemptyset (&sigio_set);
   sigaddset (&sigio_set, SIGIO);
-  sigprocmask (block ? SIG_BLOCK : SIG_UNBLOCK, &sigio_set, NULL);
+  gdb_sigmask (block ? SIG_BLOCK : SIG_UNBLOCK, &sigio_set, NULL);
 #endif
 }
 
diff --git a/gdb/gdbsupport/block-signals.h b/gdb/gdbsupport/block-signals.h
new file mode 100644
index 0000000..04a53a2
--- /dev/null
+++ b/gdb/gdbsupport/block-signals.h
@@ -0,0 +1,65 @@ 
+/* Block signals used by gdb
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_BLOCK_SIGNALS_H
+#define GDBSUPPORT_BLOCK_SIGNALS_H
+
+#include <signal.h>
+
+namespace gdb
+{
+
+/* This is an RAII class that temporarily blocks the signals needed by
+   gdb.  This can be used before starting a new thread to ensure that
+   this thread starts with the appropriate signals blocked.  */
+class block_signals
+{
+public:
+  block_signals ()
+  {
+#ifdef HAVE_PTHREAD_SIGMASK
+    sigset_t mask;
+    sigemptyset (&mask);
+    sigaddset (&mask, SIGINT);
+    sigaddset (&mask, SIGCHLD);
+    sigaddset (&mask, SIGALRM);
+    sigaddset (&mask, SIGWINCH);
+    pthread_sigmask (SIG_BLOCK, &mask, &m_old_mask);
+#endif
+  }
+
+  ~block_signals ()
+  {
+#ifdef HAVE_PTHREAD_SIGMASK
+    pthread_sigmask (SIG_SETMASK, &m_old_mask, nullptr);
+#endif
+  }
+
+  DISABLE_COPY_AND_ASSIGN (block_signals);
+
+private:
+
+#ifdef HAVE_PTHREAD_SIGMASK
+  sigset_t m_old_mask;
+#endif
+};
+
+}
+
+#endif /* GDBSUPPORT_BLOCK_SIGNALS_H */
diff --git a/gdb/gdbsupport/common.m4 b/gdb/gdbsupport/common.m4
index bc35c0e..7da765b 100644
--- a/gdb/gdbsupport/common.m4
+++ b/gdb/gdbsupport/common.m4
@@ -54,6 +54,11 @@ 
     [[std::thread t(callback);]])],
 				  gdb_cv_cxx_std_thread=yes,
 				  gdb_cv_cxx_std_thread=no)])
+
+    # This check must be here, while LIBS includes any necessary
+    # threading library.
+    AC_CHECK_FUNCS([pthread_sigmask])
+
     LIBS="$save_LIBS"
     CXXFLAGS="$save_CXXFLAGS"
   fi
diff --git a/gdb/gdbsupport/gdb-sigmask.h b/gdb/gdbsupport/gdb-sigmask.h
new file mode 100644
index 0000000..08ad973
--- /dev/null
+++ b/gdb/gdbsupport/gdb-sigmask.h
@@ -0,0 +1,45 @@ 
+/* sigprocmask wrapper for gdb
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_GDB_SIGMASK_H
+#define GDBSUPPORT_GDB_SIGMASK_H
+
+#include <signal.h>
+
+#ifdef HAVE_SIGPROCMASK
+
+#ifdef HAVE_PTHREAD_SIGMASK
+#define gdb_sigmask pthread_sigmask
+#else
+#define gdb_sigmask sigprocmask
+#endif
+
+#else /* HAVE_SIGPROCMASK */
+
+/* Other code checks HAVE_SIGPROCMASK, but if there happened to be a
+   system that only had pthread_sigmask, we could still use it with
+   some extra changes.  */
+#ifdef HAVE_PTHREAD_SIGMASK
+#error pthead_sigmask available without sigprocmask - please report
+#endif
+
+#endif /* HAVE_SIGPROCMASK */
+
+
+#endif /* GDBSUPPORT_GDB_SIGMASK_H */
diff --git a/gdb/gdbsupport/signals-state-save-restore.c b/gdb/gdbsupport/signals-state-save-restore.c
index c66d2dd..25a8220 100644
--- a/gdb/gdbsupport/signals-state-save-restore.c
+++ b/gdb/gdbsupport/signals-state-save-restore.c
@@ -17,6 +17,7 @@ 
 
 #include "common-defs.h"
 #include "signals-state-save-restore.h"
+#include "gdbsupport/gdb-sigmask.h"
 
 #include <signal.h>
 
@@ -25,10 +26,6 @@ 
 #ifdef HAVE_SIGACTION
 static struct sigaction original_signal_actions[NSIG];
 
-/* Note that we use sigprocmask without worrying about threads because
-   the save/restore functions are called either from main, or after a
-   fork.  In both cases, we know the calling process is single
-   threaded.  */
 static sigset_t original_signal_mask;
 #endif
 
@@ -41,7 +38,7 @@ 
   int i;
   int res;
 
-  res = sigprocmask (0,  NULL, &original_signal_mask);
+  res = gdb_sigmask (0,  NULL, &original_signal_mask);
   if (res == -1)
     perror_with_name (("sigprocmask"));
 
@@ -110,7 +107,7 @@ 
 	perror_with_name (("sigaction"));
     }
 
-  res = sigprocmask (SIG_SETMASK,  &original_signal_mask, NULL);
+  res = gdb_sigmask (SIG_SETMASK,  &original_signal_mask, NULL);
   if (res == -1)
     perror_with_name (("sigprocmask"));
 #endif
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index 55929f4..d745c56 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -36,6 +36,7 @@ 
 #include "guile-internal.h"
 #endif
 #include <signal.h>
+#include "gdbsupport/block-signals.h"
 
 /* The Guile version we're using.
    We *could* use the macros in libguile/version.h but that would preclude
@@ -798,10 +799,6 @@ 
 
 #if HAVE_GUILE
   {
-#ifdef HAVE_SIGPROCMASK
-    sigset_t sigchld_mask, prev_mask;
-#endif
-
     /* The Python support puts the C side in module "_gdb", leaving the Python
        side to define module "gdb" which imports "_gdb".  There is evidently no
        similar convention in Guile so we skip this.  */
@@ -813,25 +810,20 @@ 
     scm_set_automatic_finalization_enabled (0);
 #endif
 
-#ifdef HAVE_SIGPROCMASK
-    /* Before we initialize Guile, block SIGCHLD.
+    /* Before we initialize Guile, block signals needed by gdb
+       (especially SIGCHLD).
        This is done so that all threads created during Guile initialization
        have SIGCHLD blocked.  PR 17247.
        Really libgc and Guile should do this, but we need to work with
        libgc 7.4.x.  */
-    sigemptyset (&sigchld_mask);
-    sigaddset (&sigchld_mask, SIGCHLD);
-    sigprocmask (SIG_BLOCK, &sigchld_mask, &prev_mask);
-#endif
+    {
+      gdb::block_signals blocker;
 
-    /* scm_with_guile is the most portable way to initialize Guile.
-       Plus we need to initialize the Guile support while in Guile mode
-       (e.g., called from within a call to scm_with_guile).  */
-    scm_with_guile (call_initialize_gdb_module, NULL);
-
-#ifdef HAVE_SIGPROCMASK
-    sigprocmask (SIG_SETMASK, &prev_mask, NULL);
-#endif
+      /* scm_with_guile is the most portable way to initialize Guile.
+	 Plus we need to initialize the Guile support while in Guile mode
+	 (e.g., called from within a call to scm_with_guile).  */
+      scm_with_guile (call_initialize_gdb_module, NULL);
+    }
 
     /* Set Guile's backtrace to match the "set guile print-stack" default.
        [N.B. The two settings are still separate.]
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 22e8303..d3a7e4e 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -67,6 +67,7 @@ 
 #include "nat/linux-namespaces.h"
 #include "gdbsupport/fileio.h"
 #include "gdbsupport/scope-exit.h"
+#include "gdbsupport/gdb-sigmask.h"
 
 /* This comment documents high-level logic of this file.
 
@@ -764,7 +765,7 @@ 
   if (!sigismember (&blocked_mask, SIGCHLD))
     sigaddset (&blocked_mask, SIGCHLD);
 
-  sigprocmask (SIG_BLOCK, &blocked_mask, prev_mask);
+  gdb_sigmask (SIG_BLOCK, &blocked_mask, prev_mask);
 }
 
 /* Restore child signals mask, previously returned by
@@ -773,7 +774,7 @@ 
 static void
 restore_child_signals_mask (sigset_t *prev_mask)
 {
-  sigprocmask (SIG_SETMASK, prev_mask, NULL);
+  gdb_sigmask (SIG_SETMASK, prev_mask, NULL);
 }
 
 /* Mask of signals to pass directly to the inferior.  */
@@ -4564,7 +4565,7 @@ 
   sigaction (SIGCHLD, &sigchld_action, NULL);
 
   /* Make sure we don't block SIGCHLD during a sigsuspend.  */
-  sigprocmask (SIG_SETMASK, NULL, &suspend_mask);
+  gdb_sigmask (SIG_SETMASK, NULL, &suspend_mask);
   sigdelset (&suspend_mask, SIGCHLD);
 
   sigemptyset (&blocked_mask);