* [PATCH 0/2] All threads not stopped when a process exits @ 2020-02-06 8:30 Tankut Baris Aktemur 2020-02-06 8:30 ` [PATCH 1/2] gdb: define convenience function 'exists_non_stop_target' Tankut Baris Aktemur ` (4 more replies) 0 siblings, 5 replies; 8+ messages in thread From: Tankut Baris Aktemur @ 2020-02-06 8:30 UTC (permalink / raw) To: gdb-patches; +Cc: palves Hi All, This is a 2-part series that aims to fix the problem of all threads not being stopped when a process exits. The problem is, if we are in all-stop mode with multiple inferiors, and an exit event is received from an inferior, target_mourn_inferior() unpushes the process target and leaves exec_ops as the top target of the current inferior. This new top target is not non-stop. Hence, stop_all_threads() is skipped. If there are other inferiors, they remain running instead of being stopped. Thanks, Baris Tankut Baris Aktemur (2): gdb: define convenience function 'exists_non_stop_target' gdb/infrun: stop all threads if there exists a non-stop target gdb/infrun.c | 19 ++++-- gdb/target.c | 20 ++++++ gdb/target.h | 3 + gdb/testsuite/gdb.multi/stop-all-on-exit.c | 27 +++++++++ gdb/testsuite/gdb.multi/stop-all-on-exit.exp | 64 ++++++++++++++++++++ 5 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.multi/stop-all-on-exit.c create mode 100644 gdb/testsuite/gdb.multi/stop-all-on-exit.exp -- 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] gdb: define convenience function 'exists_non_stop_target' 2020-02-06 8:30 [PATCH 0/2] All threads not stopped when a process exits Tankut Baris Aktemur @ 2020-02-06 8:30 ` Tankut Baris Aktemur 2020-02-06 8:30 ` [PATCH 2/2] gdb/infrun: stop all threads if there exists a non-stop target Tankut Baris Aktemur ` (3 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Tankut Baris Aktemur @ 2020-02-06 8:30 UTC (permalink / raw) To: gdb-patches; +Cc: palves Define a predicate function that returns true if there exists an inferior with a non-stop target. gdb/ChangeLog: 2020-02-05 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> * target.h (exists_non_stop_target): New function declaration. * target.c (exists_non_stop_target): New function. --- gdb/target.c | 20 ++++++++++++++++++++ gdb/target.h | 3 +++ 2 files changed, 23 insertions(+) diff --git a/gdb/target.c b/gdb/target.c index 470ef51d69e..a332521af3b 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -3934,6 +3934,26 @@ target_is_non_stop_p (void) && target_always_non_stop_p ())); } +/* See target.h. */ + +bool +exists_non_stop_target () +{ + if (target_is_non_stop_p ()) + return true; + + scoped_restore_current_thread restore_thread; + + for (inferior *inf : all_inferiors ()) + { + switch_to_inferior_no_thread (inf); + if (target_is_non_stop_p ()) + return true; + } + + return false; +} + /* Controls if targets can report that they always run in non-stop mode. This is just for maintainers to use when debugging gdb. */ enum auto_boolean target_non_stop_enabled = AUTO_BOOLEAN_AUTO; diff --git a/gdb/target.h b/gdb/target.h index 26b71cfeb09..40ca5cac8b1 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1857,6 +1857,9 @@ extern enum auto_boolean target_non_stop_enabled; non-stop" is on. */ extern int target_is_non_stop_p (void); +/* Return true if at least one inferior has a non-stop target. */ +extern bool exists_non_stop_target (); + #define target_execution_direction() \ (current_top_target ()->execution_direction ()) -- 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] gdb/infrun: stop all threads if there exists a non-stop target 2020-02-06 8:30 [PATCH 0/2] All threads not stopped when a process exits Tankut Baris Aktemur 2020-02-06 8:30 ` [PATCH 1/2] gdb: define convenience function 'exists_non_stop_target' Tankut Baris Aktemur @ 2020-02-06 8:30 ` Tankut Baris Aktemur 2020-04-01 18:10 ` Pedro Alves 2020-03-03 8:35 ` [PING][PATCH 0/2] All threads not stopped when a process exits Tankut Baris Aktemur ` (2 subsequent siblings) 4 siblings, 1 reply; 8+ messages in thread From: Tankut Baris Aktemur @ 2020-02-06 8:30 UTC (permalink / raw) To: gdb-patches; +Cc: palves Stop all threads not only if the current target is non-stop, but also if there exists a non-stop target. The multi-target patch (5b6d1e4fa4f "Multi-target support") made the following change to gdb/inf-child.c: void inf_child_target::maybe_unpush_target () { - if (!inf_child_explicitly_opened && !have_inferiors ()) + if (!inf_child_explicitly_opened) unpush_target (this); } If we are in all-stop mode with multiple inferiors, and an exit event is received from an inferior, target_mourn_inferior() gets to this point and without the have_inferiors() check, the target is unpushed. This leads to having exec_ops as the top target. Here is a test scenario. Two executables, ./a.out returns immediately; ./sleepy just sleeps. $ gdb ./sleepy (gdb) start ... (gdb) add-inferior -exec ./a.out ... (gdb) inferior 2 [Switching to inferior 2.. (gdb) start ... (gdb) set schedule-multiple on (gdb) set debug infrun 1 (gdb) continue At this point, the exit event is received from ./a.out. Normally, this would lead to stop_all_threads() to also stop ./sleepy, but this doesn't happen, because target_is_non_stop_p() returns false. And it returns false because the top target is no longer the process target; it is the exec_ops. This patch modifies 'stop_waiting' to call 'stop_all_threads' if there exists a non-stop target, not just when the current top target is non-stop. Tested on X86_64 Linux. gdb/ChangeLog: 2020-02-05 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> * infrun.c (stop_all_threads): Update assertion, plus when stopping threads, take into account that we might be trying to stop an all-stop target. (stop_waiting): Call 'stop_all_threads' if there exists a non-stop target. gdb/testsuite/ChangeLog: 2020-02-05 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> * gdb.multi/stop-all-on-exit.c: New test. * gdb.multi/stop-all-on-exit.exp: New file. --- gdb/infrun.c | 19 ++++-- gdb/testsuite/gdb.multi/stop-all-on-exit.c | 27 +++++++++ gdb/testsuite/gdb.multi/stop-all-on-exit.exp | 64 ++++++++++++++++++++ 3 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.multi/stop-all-on-exit.c create mode 100644 gdb/testsuite/gdb.multi/stop-all-on-exit.exp diff --git a/gdb/infrun.c b/gdb/infrun.c index 3e846f8e680..fd72a744339 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4682,7 +4682,7 @@ stop_all_threads (void) int pass; int iterations = 0; - gdb_assert (target_is_non_stop_p ()); + gdb_assert (exists_non_stop_target ()); if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: stop_all_threads\n"); @@ -4713,6 +4713,18 @@ stop_all_threads (void) to tell the target to stop. */ for (thread_info *t : all_non_exited_threads ()) { + /* For a single-target setting with an all-stop target, + we would not even arrive here. For a multi-target + setting, until GDB is able to handle a mixture of + all-stop and non-stop targets, simply skip all-stop + targets' threads. This should be fine due to the + protection of commit + 2f4fcf00399bc0ad5a4fed6b530128e8be4f40da */ + + switch_to_thread_no_regs (t); + if (!target_is_non_stop_p ()) + continue; + if (t->executing) { /* If already stopping, don't request a stop again. @@ -4724,7 +4736,6 @@ stop_all_threads (void) "infrun: %s executing, " "need stop\n", target_pid_to_str (t->ptid).c_str ()); - switch_to_thread_no_regs (t); target_stop (t->ptid); t->stop_requested = 1; } @@ -7837,9 +7848,9 @@ stop_waiting (struct execution_control_state *ecs) /* Let callers know we don't want to wait for the inferior anymore. */ ecs->wait_some_more = 0; - /* If all-stop, but the target is always in non-stop mode, stop all + /* If all-stop, but there exists a non-stop target, stop all threads now that we're presenting the stop to the user. */ - if (!non_stop && target_is_non_stop_p ()) + if (!non_stop && exists_non_stop_target ()) stop_all_threads (); } diff --git a/gdb/testsuite/gdb.multi/stop-all-on-exit.c b/gdb/testsuite/gdb.multi/stop-all-on-exit.c new file mode 100644 index 00000000000..bca0c98e021 --- /dev/null +++ b/gdb/testsuite/gdb.multi/stop-all-on-exit.c @@ -0,0 +1,27 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2020 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 <unistd.h> + +static unsigned int duration = 1; + +int +main () +{ + sleep (duration); + return 0; +} diff --git a/gdb/testsuite/gdb.multi/stop-all-on-exit.exp b/gdb/testsuite/gdb.multi/stop-all-on-exit.exp new file mode 100644 index 00000000000..a0622aeab3c --- /dev/null +++ b/gdb/testsuite/gdb.multi/stop-all-on-exit.exp @@ -0,0 +1,64 @@ +# This testcase is part of GDB, the GNU debugger. + +# Copyright 2020 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/>. + +# Test that in all-stop mode with multiple inferiors, GDB stops all +# threads upon receiving an exit event from one of the inferiors. + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} { + return -1 +} + +if {![runto_main]} { + fail "starting inferior 1" + return -1 +} + +# This is a test specific for a native target, where we use the +# "-exec" argument to "add-inferior" and we explicitly don't do +# "maint set target-non-stop on". +if {![gdb_is_target_native]} { + untested "the test is aimed at a native target" + return 0 +} + +# Add a second inferior that will sleep longer. +gdb_test "add-inferior -exec $binfile" "Added inferior 2.*" \ + "add the second inferior" +gdb_test "inferior 2" ".*Switching to inferior 2.*" +if {![runto_main]} { + fail "starting inferior 2" + return -1 +} +gdb_test "print duration=10" "= 10" + +# Now continue both processes. We should get the exit event from the +# first inferior. +gdb_test_no_output "set schedule-multiple on" +gdb_continue_to_end + +# GDB is expected to have stopped the other inferior. Switch to the +# slow inferior's thread. It should not be running. +gdb_test_multiple "thread 2.1" "check thread 2.1 is not running" { + -re ".*\\(running\\)\[\r\n\]+$gdb_prompt" { + fail $gdb_test_name + } + -re ".*$gdb_prompt" { + pass $gdb_test_name + } +} -- 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] gdb/infrun: stop all threads if there exists a non-stop target 2020-02-06 8:30 ` [PATCH 2/2] gdb/infrun: stop all threads if there exists a non-stop target Tankut Baris Aktemur @ 2020-04-01 18:10 ` Pedro Alves 2020-04-01 19:35 ` Aktemur, Tankut Baris 0 siblings, 1 reply; 8+ messages in thread From: Pedro Alves @ 2020-04-01 18:10 UTC (permalink / raw) To: Tankut Baris Aktemur, gdb-patches Hi Tankut, Thanks much for this, and I'm very sorry for the delay. This looks good to me, with minor comments below. Please push. On 2/6/20 8:29 AM, Tankut Baris Aktemur wrote: > > if (debug_infrun) > fprintf_unfiltered (gdb_stdlog, "infrun: stop_all_threads\n"); > @@ -4713,6 +4713,18 @@ stop_all_threads (void) > to tell the target to stop. */ > for (thread_info *t : all_non_exited_threads ()) > { > + /* For a single-target setting with an all-stop target, > + we would not even arrive here. For a multi-target > + setting, until GDB is able to handle a mixture of > + all-stop and non-stop targets, simply skip all-stop > + targets' threads. This should be fine due to the > + protection of commit > + 2f4fcf00399bc0ad5a4fed6b530128e8be4f40da */ Missing period. But also please mention here the protection of check_multi_target_resumption instead of the commit id, so that people don't have to dig into the repo every time they need to understand this. > + > + switch_to_thread_no_regs (t); > + if (!target_is_non_stop_p ()) > + continue; > + > +# GDB is expected to have stopped the other inferior. Switch to the > +# slow inferior's thread. It should not be running. > +gdb_test_multiple "thread 2.1" "check thread 2.1 is not running" { > + -re ".*\\(running\\)\[\r\n\]+$gdb_prompt" { These leading ".*" are unnecessary -- they're implied. > + fail $gdb_test_name > + } > + -re ".*$gdb_prompt" { Here too. > + pass $gdb_test_name > + } > +} > Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] gdb/infrun: stop all threads if there exists a non-stop target 2020-04-01 18:10 ` Pedro Alves @ 2020-04-01 19:35 ` Aktemur, Tankut Baris 0 siblings, 0 replies; 8+ messages in thread From: Aktemur, Tankut Baris @ 2020-04-01 19:35 UTC (permalink / raw) To: Pedro Alves, gdb-patches On Wednesday, April 1, 2020 8:10 PM, Pedro Alves wrote: > Hi Tankut, > > Thanks much for this, and I'm very sorry for the delay. > > This looks good to me, with minor comments below. Please push. Thank you. Fixed and pushed as 53cccef118b gdb/infrun: stop all threads if there exists a non-stop target a0714d305fb gdb: define convenience function 'exists_non_stop_target' Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Gary Kershaw Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PING][PATCH 0/2] All threads not stopped when a process exits 2020-02-06 8:30 [PATCH 0/2] All threads not stopped when a process exits Tankut Baris Aktemur 2020-02-06 8:30 ` [PATCH 1/2] gdb: define convenience function 'exists_non_stop_target' Tankut Baris Aktemur 2020-02-06 8:30 ` [PATCH 2/2] gdb/infrun: stop all threads if there exists a non-stop target Tankut Baris Aktemur @ 2020-03-03 8:35 ` Tankut Baris Aktemur 2020-03-17 13:02 ` [PING^2][PATCH " Tankut Baris Aktemur 2020-04-01 13:34 ` [PING^3][PATCH " Tankut Baris Aktemur 4 siblings, 0 replies; 8+ messages in thread From: Tankut Baris Aktemur @ 2020-03-03 8:35 UTC (permalink / raw) To: gdb-patches; +Cc: palves > This is a 2-part series that aims to fix the problem of all threads > not being stopped when a process exits. The problem is, if we are in > all-stop mode with multiple inferiors, and an exit event is received > from an inferior, target_mourn_inferior() unpushes the process target > and leaves exec_ops as the top target of the current inferior. This > new top target is not non-stop. Hence, stop_all_threads() is skipped. > If there are other inferiors, they remain running instead of being > stopped. Kindly pinging for the patch at https://sourceware.org/ml/gdb-patches/2020-02/msg00122.html Thanks Baris ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PING^2][PATCH 0/2] All threads not stopped when a process exits 2020-02-06 8:30 [PATCH 0/2] All threads not stopped when a process exits Tankut Baris Aktemur ` (2 preceding siblings ...) 2020-03-03 8:35 ` [PING][PATCH 0/2] All threads not stopped when a process exits Tankut Baris Aktemur @ 2020-03-17 13:02 ` Tankut Baris Aktemur 2020-04-01 13:34 ` [PING^3][PATCH " Tankut Baris Aktemur 4 siblings, 0 replies; 8+ messages in thread From: Tankut Baris Aktemur @ 2020-03-17 13:02 UTC (permalink / raw) To: gdb-patches; +Cc: palves > This is a 2-part series that aims to fix the problem of all threads > not being stopped when a process exits. The problem is, if we are in > all-stop mode with multiple inferiors, and an exit event is received > from an inferior, target_mourn_inferior() unpushes the process target > and leaves exec_ops as the top target of the current inferior. This > new top target is not non-stop. Hence, stop_all_threads() is skipped. > If there are other inferiors, they remain running instead of being > stopped. Kindly pinging for the patch at https://sourceware.org/ml/gdb-patches/2020-02/msg00122.html Thanks Baris ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PING^3][PATCH 0/2] All threads not stopped when a process exits 2020-02-06 8:30 [PATCH 0/2] All threads not stopped when a process exits Tankut Baris Aktemur ` (3 preceding siblings ...) 2020-03-17 13:02 ` [PING^2][PATCH " Tankut Baris Aktemur @ 2020-04-01 13:34 ` Tankut Baris Aktemur 4 siblings, 0 replies; 8+ messages in thread From: Tankut Baris Aktemur @ 2020-04-01 13:34 UTC (permalink / raw) To: gdb-patches > This is a 2-part series that aims to fix the problem of all threads > not being stopped when a process exits. The problem is, if we are in > all-stop mode with multiple inferiors, and an exit event is received > from an inferior, target_mourn_inferior() unpushes the process target > and leaves exec_ops as the top target of the current inferior. This > new top target is not non-stop. Hence, stop_all_threads() is skipped. > If there are other inferiors, they remain running instead of being > stopped. Kindly pinging for the patch at https://sourceware.org/ml/gdb-patches/2020-02/msg00122.html Thanks Baris Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Gary Kershaw Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-04-01 19:36 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-06 8:30 [PATCH 0/2] All threads not stopped when a process exits Tankut Baris Aktemur 2020-02-06 8:30 ` [PATCH 1/2] gdb: define convenience function 'exists_non_stop_target' Tankut Baris Aktemur 2020-02-06 8:30 ` [PATCH 2/2] gdb/infrun: stop all threads if there exists a non-stop target Tankut Baris Aktemur 2020-04-01 18:10 ` Pedro Alves 2020-04-01 19:35 ` Aktemur, Tankut Baris 2020-03-03 8:35 ` [PING][PATCH 0/2] All threads not stopped when a process exits Tankut Baris Aktemur 2020-03-17 13:02 ` [PING^2][PATCH " Tankut Baris Aktemur 2020-04-01 13:34 ` [PING^3][PATCH " Tankut Baris Aktemur
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).