From patchwork Wed Feb 5 11:54:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 37692 Received: (qmail 81036 invoked by alias); 5 Feb 2020 11:54:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 81016 invoked by uid 89); 5 Feb 2020 11:54:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy=tankutbarisaktemurintelcom, sk:tankut, nonstop, U*tankut.baris.aktemur X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Feb 2020 11:54:40 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id AB3782046F; Wed, 5 Feb 2020 06:54:38 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id 3C6662039A for ; Wed, 5 Feb 2020 06:54:30 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 1D9CC28170 for ; Wed, 5 Feb 2020 06:54:30 -0500 (EST) X-Gerrit-PatchSet: 1 Date: Wed, 5 Feb 2020 06:54:30 -0500 From: "Tankut Baris Aktemur (Code Review)" To: gdb-patches@sourceware.org Message-ID: Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange Subject: [review] gdb/infrun: stop all threads if there exists a non-stop target X-Gerrit-Change-Id: I6de26f990dc3b32ed526e60507e7bfe37fa1c6f1 X-Gerrit-Change-Number: 766 X-Gerrit-ChangeURL: X-Gerrit-Commit: 26d4c43cdb1c54dbe1d03f645abb75aab0413cd2 References: Reply-To: tankut.baris.aktemur@intel.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/766 ...................................................................... gdb/infrun: stop all threads if there exists a non-stop target 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 * 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 * gdb.multi/stop-all-on-exit.c: New test. * gdb.multi/stop-all-on-exit.exp: New file. Change-Id: I6de26f990dc3b32ed526e60507e7bfe37fa1c6f1 --- M gdb/infrun.c A gdb/testsuite/gdb.multi/stop-all-on-exit.c A gdb/testsuite/gdb.multi/stop-all-on-exit.exp 3 files changed, 106 insertions(+), 4 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 3e846f8..fd72a74 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4682,7 +4682,7 @@ 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 @@ 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 @@ "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 @@ /* 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 0000000..bca0c98 --- /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 . */ + +#include + +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 0000000..a0622ae --- /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 . + +# 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 + } +}