gdb: Don't ignore all SIGSTOP when the signal handler is set to pass

Message ID 20190910173757.3374-1-andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Sept. 10, 2019, 5:37 p.m. UTC
  It was observed that in a multi-threaded application on GNU/Linux,
that if the user has set the SIGSTOP to be pass (using GDB's handle
command) then the inferior would hang upon hitting a breakpoint.

What happens is that when a thread hits the breakpoint GDB tries to
stop all of the other threads by sending them a SIGSTOP and setting
the stop_requested flag in the target_ops structure - this can be seen
in infrun.c:stop_all_threads.

GDB then waits for all of the other threads to stop.

When the SIGSTOP event arrives we eventually end up in
linux-nat.c:linux_nat_filter_event, which has the job of deciding if
the event we're looking at (the SIGSTOP arriving in this case) is
something that should be reported back to the core of GDB.

One of the final actions of this function is to check if we stopped
due to a signal, and if we did, and the signal has been set to 'pass'
by the user then we ignore the event and resume the thread.

This code already has some conditions in place that mean the event is
reported to GDB even if the signal is in the set of signals to be
passed to the inferior.

In this commit I extend this condition to such that:

  If the signal is a SIGSTOP, and the thread's stop_requested flag is
  set (indicating we're waiting for the thread to stop with a SIGSTOP)
  then we should report this SIGSTOP to GDB and not pass it to the
  inferior.

With this change in place the test now passes.  Regression tested on
x86-64 GNU/Linux with no regressions.

gdb/ChangeLog:

	* linux-nat.c (linux_nat_filter_event): Don't ignore SIGSTOP if we
	have just sent the thread a SIGSTOP and are waiting for it to
	arrive.

gdb/testsuite/ChangeLog:

	* gdb.threads/stop-with-handle.c: New file.
	* gdb.threads/stop-with-handle.exp: New file.
---
 gdb/ChangeLog                                  |  6 +++
 gdb/linux-nat.c                                |  5 +-
 gdb/testsuite/ChangeLog                        |  5 ++
 gdb/testsuite/gdb.threads/stop-with-handle.c   | 70 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.threads/stop-with-handle.exp | 52 +++++++++++++++++++
 5 files changed, 137 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.threads/stop-with-handle.c
 create mode 100644 gdb/testsuite/gdb.threads/stop-with-handle.exp
  

Comments

Andrew Burgess Sept. 26, 2019, 11:06 p.m. UTC | #1
Ping!

Thanks,
Andrew


* Andrew Burgess <andrew.burgess@embecosm.com> [2019-09-10 18:37:57 +0100]:

> It was observed that in a multi-threaded application on GNU/Linux,
> that if the user has set the SIGSTOP to be pass (using GDB's handle
> command) then the inferior would hang upon hitting a breakpoint.
> 
> What happens is that when a thread hits the breakpoint GDB tries to
> stop all of the other threads by sending them a SIGSTOP and setting
> the stop_requested flag in the target_ops structure - this can be seen
> in infrun.c:stop_all_threads.
> 
> GDB then waits for all of the other threads to stop.
> 
> When the SIGSTOP event arrives we eventually end up in
> linux-nat.c:linux_nat_filter_event, which has the job of deciding if
> the event we're looking at (the SIGSTOP arriving in this case) is
> something that should be reported back to the core of GDB.
> 
> One of the final actions of this function is to check if we stopped
> due to a signal, and if we did, and the signal has been set to 'pass'
> by the user then we ignore the event and resume the thread.
> 
> This code already has some conditions in place that mean the event is
> reported to GDB even if the signal is in the set of signals to be
> passed to the inferior.
> 
> In this commit I extend this condition to such that:
> 
>   If the signal is a SIGSTOP, and the thread's stop_requested flag is
>   set (indicating we're waiting for the thread to stop with a SIGSTOP)
>   then we should report this SIGSTOP to GDB and not pass it to the
>   inferior.
> 
> With this change in place the test now passes.  Regression tested on
> x86-64 GNU/Linux with no regressions.
> 
> gdb/ChangeLog:
> 
> 	* linux-nat.c (linux_nat_filter_event): Don't ignore SIGSTOP if we
> 	have just sent the thread a SIGSTOP and are waiting for it to
> 	arrive.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.threads/stop-with-handle.c: New file.
> 	* gdb.threads/stop-with-handle.exp: New file.
> ---
>  gdb/ChangeLog                                  |  6 +++
>  gdb/linux-nat.c                                |  5 +-
>  gdb/testsuite/ChangeLog                        |  5 ++
>  gdb/testsuite/gdb.threads/stop-with-handle.c   | 70 ++++++++++++++++++++++++++
>  gdb/testsuite/gdb.threads/stop-with-handle.exp | 52 +++++++++++++++++++
>  5 files changed, 137 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.threads/stop-with-handle.c
>  create mode 100644 gdb/testsuite/gdb.threads/stop-with-handle.exp
> 
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 945c19f6668..5cf5c57c043 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -3150,9 +3150,12 @@ linux_nat_filter_event (int lwpid, int status)
>  
>        /* When using hardware single-step, we need to report every signal.
>  	 Otherwise, signals in pass_mask may be short-circuited
> -	 except signals that might be caused by a breakpoint.  */
> +	 except signals that might be caused by a breakpoint, or SIGSTOP
> +	 if we sent the SIGSTOP and are waiting for it to arrive.  */
>        if (!lp->step
>  	  && WSTOPSIG (status) && sigismember (&pass_mask, WSTOPSIG (status))
> +	  && (WSTOPSIG (status) != SIGSTOP
> +	      || !find_thread_ptid (lp->ptid)->stop_requested)
>  	  && !linux_wstatus_maybe_breakpoint (status))
>  	{
>  	  linux_resume_one_lwp (lp, lp->step, signo);
> diff --git a/gdb/testsuite/gdb.threads/stop-with-handle.c b/gdb/testsuite/gdb.threads/stop-with-handle.c
> new file mode 100644
> index 00000000000..da1b3d45498
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/stop-with-handle.c
> @@ -0,0 +1,70 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 Free Software Foundation, Inc.
> +
> +   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/>.  */
> +
> +#include <stdio.h>
> +#include <pthread.h>
> +#include <unistd.h>
> +
> +/* A place to record the thread.  */
> +pthread_t the_thread;
> +
> +/* The worker thread just spins forever.  */
> +void*
> +thread_worker (void *payload)
> +{
> +  while (1)
> +    {
> +      sleep (1);
> +    }
> +
> +  return NULL;
> +}
> +
> +/* Create a worker thread.  */
> +int
> +spawn_thread ()
> +{
> +  if (pthread_create (&the_thread, NULL, thread_worker, NULL))
> +    {
> +      fprintf (stderr, "Unable to create thread.\n");
> +      return 0;
> +    }
> +  return 1;
> +}
> +
> +/* A place for GDB to place a breakpoint.   */
> +void
> +breakpt ()
> +{
> +  asm ("" ::: "memory");
> +}
> +
> +/* Create a worker thread that just spins forever, then enter a loop
> +   periodically calling the BREAKPT function.  */
> +int main() {
> +  spawn_thread ();
> +
> +  while (1)
> +    {
> +      sleep (1);
> +
> +      breakpt ();
> +    }
> +
> +  return 0;
> +}
> +
> diff --git a/gdb/testsuite/gdb.threads/stop-with-handle.exp b/gdb/testsuite/gdb.threads/stop-with-handle.exp
> new file mode 100644
> index 00000000000..bab58a6129d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/stop-with-handle.exp
> @@ -0,0 +1,52 @@
> +# Copyright 2019 Free Software Foundation, Inc.
> +#
> +# 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/>.
> +
> +# This test covers a case where SIGSTOP has been configured to be
> +# passed to the target with GDB's 'handle' command, and then a
> +# multi-threaded inferior encounters an event that causes all theads
> +# to be stopped.
> +#
> +# The problem that (used) to exist was that GDB would see the SIGSTOP,
> +# but decide to ignore the signal based on the handle table.
> +
> +standard_testfile
> +
> +if {[prepare_for_testing "failed to prepare" \
> +	 "${testfile}" "${srcfile}" {debug pthreads}]} {
> +    return -1
> +}
> +
> +if ![runto_main] then {
> +    fail "can't run to main"
> +    return 0
> +}
> +
> +# Have SIGSTOP sent to the inferior.
> +gdb_test "handle SIGSTOP nostop noprint pass" \
> +    [multi_line \
> +	 "Signal\[ \t\]+Stop\[ \t\]+Print\[ \t\]+Pass to program\[ \t\]+Description" \
> +	 "SIGSTOP\[ \t\]+No\[ \t\]+No\[ \t\]+Yes\[ \t\]+Stopped \\(signal\\)"]
> +
> +# Create a breakpoint, and when we hit it automatically finish the
> +# current frame.
> +gdb_breakpoint breakpt
> +
> +# When the bug triggers this continue never completes.  GDB hits the
> +# breakpoint in thread 1, and then tries to stop the second thread by
> +# sending it SIGSTOP.  GDB sees the SIGSTOP arrive in thread 2 but
> +# incorrect decides to pass the SIGSTOP to the thread rather than
> +# bringing the thread to a stop.
> +gdb_continue_to_breakpoint breakpt
> +
> -- 
> 2.14.5
>
  
Andrew Burgess Oct. 3, 2019, 11:55 a.m. UTC | #2
I plan to merge this patch tomorrow unless anyone complains before
then.

Thanks,
Andrew


* Andrew Burgess <andrew.burgess@embecosm.com> [2019-09-27 00:06:13 +0100]:

> Ping!
> 
> Thanks,
> Andrew
> 
> 
> * Andrew Burgess <andrew.burgess@embecosm.com> [2019-09-10 18:37:57 +0100]:
> 
> > It was observed that in a multi-threaded application on GNU/Linux,
> > that if the user has set the SIGSTOP to be pass (using GDB's handle
> > command) then the inferior would hang upon hitting a breakpoint.
> > 
> > What happens is that when a thread hits the breakpoint GDB tries to
> > stop all of the other threads by sending them a SIGSTOP and setting
> > the stop_requested flag in the target_ops structure - this can be seen
> > in infrun.c:stop_all_threads.
> > 
> > GDB then waits for all of the other threads to stop.
> > 
> > When the SIGSTOP event arrives we eventually end up in
> > linux-nat.c:linux_nat_filter_event, which has the job of deciding if
> > the event we're looking at (the SIGSTOP arriving in this case) is
> > something that should be reported back to the core of GDB.
> > 
> > One of the final actions of this function is to check if we stopped
> > due to a signal, and if we did, and the signal has been set to 'pass'
> > by the user then we ignore the event and resume the thread.
> > 
> > This code already has some conditions in place that mean the event is
> > reported to GDB even if the signal is in the set of signals to be
> > passed to the inferior.
> > 
> > In this commit I extend this condition to such that:
> > 
> >   If the signal is a SIGSTOP, and the thread's stop_requested flag is
> >   set (indicating we're waiting for the thread to stop with a SIGSTOP)
> >   then we should report this SIGSTOP to GDB and not pass it to the
> >   inferior.
> > 
> > With this change in place the test now passes.  Regression tested on
> > x86-64 GNU/Linux with no regressions.
> > 
> > gdb/ChangeLog:
> > 
> > 	* linux-nat.c (linux_nat_filter_event): Don't ignore SIGSTOP if we
> > 	have just sent the thread a SIGSTOP and are waiting for it to
> > 	arrive.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.threads/stop-with-handle.c: New file.
> > 	* gdb.threads/stop-with-handle.exp: New file.
> > ---
> >  gdb/ChangeLog                                  |  6 +++
> >  gdb/linux-nat.c                                |  5 +-
> >  gdb/testsuite/ChangeLog                        |  5 ++
> >  gdb/testsuite/gdb.threads/stop-with-handle.c   | 70 ++++++++++++++++++++++++++
> >  gdb/testsuite/gdb.threads/stop-with-handle.exp | 52 +++++++++++++++++++
> >  5 files changed, 137 insertions(+), 1 deletion(-)
> >  create mode 100644 gdb/testsuite/gdb.threads/stop-with-handle.c
> >  create mode 100644 gdb/testsuite/gdb.threads/stop-with-handle.exp
> > 
> > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> > index 945c19f6668..5cf5c57c043 100644
> > --- a/gdb/linux-nat.c
> > +++ b/gdb/linux-nat.c
> > @@ -3150,9 +3150,12 @@ linux_nat_filter_event (int lwpid, int status)
> >  
> >        /* When using hardware single-step, we need to report every signal.
> >  	 Otherwise, signals in pass_mask may be short-circuited
> > -	 except signals that might be caused by a breakpoint.  */
> > +	 except signals that might be caused by a breakpoint, or SIGSTOP
> > +	 if we sent the SIGSTOP and are waiting for it to arrive.  */
> >        if (!lp->step
> >  	  && WSTOPSIG (status) && sigismember (&pass_mask, WSTOPSIG (status))
> > +	  && (WSTOPSIG (status) != SIGSTOP
> > +	      || !find_thread_ptid (lp->ptid)->stop_requested)
> >  	  && !linux_wstatus_maybe_breakpoint (status))
> >  	{
> >  	  linux_resume_one_lwp (lp, lp->step, signo);
> > diff --git a/gdb/testsuite/gdb.threads/stop-with-handle.c b/gdb/testsuite/gdb.threads/stop-with-handle.c
> > new file mode 100644
> > index 00000000000..da1b3d45498
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.threads/stop-with-handle.c
> > @@ -0,0 +1,70 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2019 Free Software Foundation, Inc.
> > +
> > +   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/>.  */
> > +
> > +#include <stdio.h>
> > +#include <pthread.h>
> > +#include <unistd.h>
> > +
> > +/* A place to record the thread.  */
> > +pthread_t the_thread;
> > +
> > +/* The worker thread just spins forever.  */
> > +void*
> > +thread_worker (void *payload)
> > +{
> > +  while (1)
> > +    {
> > +      sleep (1);
> > +    }
> > +
> > +  return NULL;
> > +}
> > +
> > +/* Create a worker thread.  */
> > +int
> > +spawn_thread ()
> > +{
> > +  if (pthread_create (&the_thread, NULL, thread_worker, NULL))
> > +    {
> > +      fprintf (stderr, "Unable to create thread.\n");
> > +      return 0;
> > +    }
> > +  return 1;
> > +}
> > +
> > +/* A place for GDB to place a breakpoint.   */
> > +void
> > +breakpt ()
> > +{
> > +  asm ("" ::: "memory");
> > +}
> > +
> > +/* Create a worker thread that just spins forever, then enter a loop
> > +   periodically calling the BREAKPT function.  */
> > +int main() {
> > +  spawn_thread ();
> > +
> > +  while (1)
> > +    {
> > +      sleep (1);
> > +
> > +      breakpt ();
> > +    }
> > +
> > +  return 0;
> > +}
> > +
> > diff --git a/gdb/testsuite/gdb.threads/stop-with-handle.exp b/gdb/testsuite/gdb.threads/stop-with-handle.exp
> > new file mode 100644
> > index 00000000000..bab58a6129d
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.threads/stop-with-handle.exp
> > @@ -0,0 +1,52 @@
> > +# Copyright 2019 Free Software Foundation, Inc.
> > +#
> > +# 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/>.
> > +
> > +# This test covers a case where SIGSTOP has been configured to be
> > +# passed to the target with GDB's 'handle' command, and then a
> > +# multi-threaded inferior encounters an event that causes all theads
> > +# to be stopped.
> > +#
> > +# The problem that (used) to exist was that GDB would see the SIGSTOP,
> > +# but decide to ignore the signal based on the handle table.
> > +
> > +standard_testfile
> > +
> > +if {[prepare_for_testing "failed to prepare" \
> > +	 "${testfile}" "${srcfile}" {debug pthreads}]} {
> > +    return -1
> > +}
> > +
> > +if ![runto_main] then {
> > +    fail "can't run to main"
> > +    return 0
> > +}
> > +
> > +# Have SIGSTOP sent to the inferior.
> > +gdb_test "handle SIGSTOP nostop noprint pass" \
> > +    [multi_line \
> > +	 "Signal\[ \t\]+Stop\[ \t\]+Print\[ \t\]+Pass to program\[ \t\]+Description" \
> > +	 "SIGSTOP\[ \t\]+No\[ \t\]+No\[ \t\]+Yes\[ \t\]+Stopped \\(signal\\)"]
> > +
> > +# Create a breakpoint, and when we hit it automatically finish the
> > +# current frame.
> > +gdb_breakpoint breakpt
> > +
> > +# When the bug triggers this continue never completes.  GDB hits the
> > +# breakpoint in thread 1, and then tries to stop the second thread by
> > +# sending it SIGSTOP.  GDB sees the SIGSTOP arrive in thread 2 but
> > +# incorrect decides to pass the SIGSTOP to the thread rather than
> > +# bringing the thread to a stop.
> > +gdb_continue_to_breakpoint breakpt
> > +
> > -- 
> > 2.14.5
> >
  
Pedro Alves Oct. 3, 2019, 12:59 p.m. UTC | #3
On 9/10/19 6:37 PM, Andrew Burgess wrote:
> It was observed that in a multi-threaded application on GNU/Linux,
> that if the user has set the SIGSTOP to be pass (using GDB's handle
> command) then the inferior would hang upon hitting a breakpoint.
> 
> What happens is that when a thread hits the breakpoint GDB tries to
> stop all of the other threads by sending them a SIGSTOP and setting
> the stop_requested flag in the target_ops structure - this can be seen
> in infrun.c:stop_all_threads.
> 
> GDB then waits for all of the other threads to stop.
> 
> When the SIGSTOP event arrives we eventually end up in
> linux-nat.c:linux_nat_filter_event, which has the job of deciding if
> the event we're looking at (the SIGSTOP arriving in this case) is
> something that should be reported back to the core of GDB.
> 
> One of the final actions of this function is to check if we stopped
> due to a signal, and if we did, and the signal has been set to 'pass'
> by the user then we ignore the event and resume the thread.
> 
> This code already has some conditions in place that mean the event is
> reported to GDB even if the signal is in the set of signals to be
> passed to the inferior.
> 
> In this commit I extend this condition to such that:
> 
>   If the signal is a SIGSTOP, and the thread's stop_requested flag is
>   set (indicating we're waiting for the thread to stop with a SIGSTOP)
>   then we should report this SIGSTOP to GDB and not pass it to the
>   inferior.
> 
> With this change in place the test now passes.  Regression tested on
> x86-64 GNU/Linux with no regressions.

Indeed, makes sense.  

I checked that gdbserver passes the new test too.  (It knows to always
report an event when gdb tries to stop a thread with 'vCont;t'.)

> +/* The worker thread just spins forever.  */
> +void*
> +thread_worker (void *payload)
> +{
> +  while (1)
> +    {
> +      sleep (1);
> +    }
> +
> +  return NULL;
> +}
> +

> +/* A place for GDB to place a breakpoint.   */
> +void
> +breakpt ()
> +{
> +  asm ("" ::: "memory");

I wonder whether this barrier is a good idea, given
that it limits portability to gcc and clang.

I checked that both clang and gcc still manage to
eliminate the function with "-flto -O2", the barrier
didn't help then.  '__attribute__((used))' instead
of the barrier works though.

> +}

> +
> +/* Create a worker thread that just spins forever, then enter a loop
> +   periodically calling the BREAKPT function.  */
> +int main() {

GNU formatting.

> +  spawn_thread ();
> +
> +  while (1)
> +    {
> +      sleep (1);
> +
> +      breakpt ();
> +    }
> +
> +  return 0;
> +}

It's better to avoid that the testcase really runs forever, in case GDB
crashes and the system doesn't kill the process.  Easily fixed with
an "alarm" call at the top of main.

LGTM otherwise.

BTW, FYI, I saw this:

$ git am /tmp/sigstop.mbox
Applying: gdb: Don't ignore all SIGSTOP when the signal handler is set to pass
.git/rebase-apply/patch:104: new blank line at EOF.
+
.git/rebase-apply/patch:162: new blank line at EOF.
+
warning: 2 lines add whitespace errors.

Thanks,
Pedro Alves
  
Andrew Burgess Oct. 3, 2019, 3:14 p.m. UTC | #4
* Pedro Alves <palves@redhat.com> [2019-10-03 13:59:14 +0100]:

> On 9/10/19 6:37 PM, Andrew Burgess wrote:
> > It was observed that in a multi-threaded application on GNU/Linux,
> > that if the user has set the SIGSTOP to be pass (using GDB's handle
> > command) then the inferior would hang upon hitting a breakpoint.
> > 
> > What happens is that when a thread hits the breakpoint GDB tries to
> > stop all of the other threads by sending them a SIGSTOP and setting
> > the stop_requested flag in the target_ops structure - this can be seen
> > in infrun.c:stop_all_threads.
> > 
> > GDB then waits for all of the other threads to stop.
> > 
> > When the SIGSTOP event arrives we eventually end up in
> > linux-nat.c:linux_nat_filter_event, which has the job of deciding if
> > the event we're looking at (the SIGSTOP arriving in this case) is
> > something that should be reported back to the core of GDB.
> > 
> > One of the final actions of this function is to check if we stopped
> > due to a signal, and if we did, and the signal has been set to 'pass'
> > by the user then we ignore the event and resume the thread.
> > 
> > This code already has some conditions in place that mean the event is
> > reported to GDB even if the signal is in the set of signals to be
> > passed to the inferior.
> > 
> > In this commit I extend this condition to such that:
> > 
> >   If the signal is a SIGSTOP, and the thread's stop_requested flag is
> >   set (indicating we're waiting for the thread to stop with a SIGSTOP)
> >   then we should report this SIGSTOP to GDB and not pass it to the
> >   inferior.
> > 
> > With this change in place the test now passes.  Regression tested on
> > x86-64 GNU/Linux with no regressions.
> 
> Indeed, makes sense.  
> 
> I checked that gdbserver passes the new test too.  (It knows to always
> report an event when gdb tries to stop a thread with 'vCont;t'.)
> 
> > +/* The worker thread just spins forever.  */
> > +void*
> > +thread_worker (void *payload)
> > +{
> > +  while (1)
> > +    {
> > +      sleep (1);
> > +    }
> > +
> > +  return NULL;
> > +}
> > +
> 
> > +/* A place for GDB to place a breakpoint.   */
> > +void
> > +breakpt ()
> > +{
> > +  asm ("" ::: "memory");
> 
> I wonder whether this barrier is a good idea, given
> that it limits portability to gcc and clang.
> 
> I checked that both clang and gcc still manage to
> eliminate the function with "-flto -O2", the barrier
> didn't help then.  '__attribute__((used))' instead
> of the barrier works though.
> 
> > +}
> 
> > +
> > +/* Create a worker thread that just spins forever, then enter a loop
> > +   periodically calling the BREAKPT function.  */
> > +int main() {
> 
> GNU formatting.
> 
> > +  spawn_thread ();
> > +
> > +  while (1)
> > +    {
> > +      sleep (1);
> > +
> > +      breakpt ();
> > +    }
> > +
> > +  return 0;
> > +}
> 
> It's better to avoid that the testcase really runs forever, in case GDB
> crashes and the system doesn't kill the process.  Easily fixed with
> an "alarm" call at the top of main.
> 
> LGTM otherwise.
> 
> BTW, FYI, I saw this:
> 
> $ git am /tmp/sigstop.mbox
> Applying: gdb: Don't ignore all SIGSTOP when the signal handler is set to pass
> .git/rebase-apply/patch:104: new blank line at EOF.
> +
> .git/rebase-apply/patch:162: new blank line at EOF.
> +
> warning: 2 lines add whitespace errors.
> 

I pushed this patch with the fixes you identified above.

Thanks,
Andrew
  

Patch

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 945c19f6668..5cf5c57c043 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3150,9 +3150,12 @@  linux_nat_filter_event (int lwpid, int status)
 
       /* When using hardware single-step, we need to report every signal.
 	 Otherwise, signals in pass_mask may be short-circuited
-	 except signals that might be caused by a breakpoint.  */
+	 except signals that might be caused by a breakpoint, or SIGSTOP
+	 if we sent the SIGSTOP and are waiting for it to arrive.  */
       if (!lp->step
 	  && WSTOPSIG (status) && sigismember (&pass_mask, WSTOPSIG (status))
+	  && (WSTOPSIG (status) != SIGSTOP
+	      || !find_thread_ptid (lp->ptid)->stop_requested)
 	  && !linux_wstatus_maybe_breakpoint (status))
 	{
 	  linux_resume_one_lwp (lp, lp->step, signo);
diff --git a/gdb/testsuite/gdb.threads/stop-with-handle.c b/gdb/testsuite/gdb.threads/stop-with-handle.c
new file mode 100644
index 00000000000..da1b3d45498
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/stop-with-handle.c
@@ -0,0 +1,70 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   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/>.  */
+
+#include <stdio.h>
+#include <pthread.h>
+#include <unistd.h>
+
+/* A place to record the thread.  */
+pthread_t the_thread;
+
+/* The worker thread just spins forever.  */
+void*
+thread_worker (void *payload)
+{
+  while (1)
+    {
+      sleep (1);
+    }
+
+  return NULL;
+}
+
+/* Create a worker thread.  */
+int
+spawn_thread ()
+{
+  if (pthread_create (&the_thread, NULL, thread_worker, NULL))
+    {
+      fprintf (stderr, "Unable to create thread.\n");
+      return 0;
+    }
+  return 1;
+}
+
+/* A place for GDB to place a breakpoint.   */
+void
+breakpt ()
+{
+  asm ("" ::: "memory");
+}
+
+/* Create a worker thread that just spins forever, then enter a loop
+   periodically calling the BREAKPT function.  */
+int main() {
+  spawn_thread ();
+
+  while (1)
+    {
+      sleep (1);
+
+      breakpt ();
+    }
+
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.threads/stop-with-handle.exp b/gdb/testsuite/gdb.threads/stop-with-handle.exp
new file mode 100644
index 00000000000..bab58a6129d
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/stop-with-handle.exp
@@ -0,0 +1,52 @@ 
+# Copyright 2019 Free Software Foundation, Inc.
+#
+# 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/>.
+
+# This test covers a case where SIGSTOP has been configured to be
+# passed to the target with GDB's 'handle' command, and then a
+# multi-threaded inferior encounters an event that causes all theads
+# to be stopped.
+#
+# The problem that (used) to exist was that GDB would see the SIGSTOP,
+# but decide to ignore the signal based on the handle table.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" \
+	 "${testfile}" "${srcfile}" {debug pthreads}]} {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
+# Have SIGSTOP sent to the inferior.
+gdb_test "handle SIGSTOP nostop noprint pass" \
+    [multi_line \
+	 "Signal\[ \t\]+Stop\[ \t\]+Print\[ \t\]+Pass to program\[ \t\]+Description" \
+	 "SIGSTOP\[ \t\]+No\[ \t\]+No\[ \t\]+Yes\[ \t\]+Stopped \\(signal\\)"]
+
+# Create a breakpoint, and when we hit it automatically finish the
+# current frame.
+gdb_breakpoint breakpt
+
+# When the bug triggers this continue never completes.  GDB hits the
+# breakpoint in thread 1, and then tries to stop the second thread by
+# sending it SIGSTOP.  GDB sees the SIGSTOP arrive in thread 2 but
+# incorrect decides to pass the SIGSTOP to the thread rather than
+# bringing the thread to a stop.
+gdb_continue_to_breakpoint breakpt
+