* [PING] [PATCH v2] gdb: Fix deleted thread when issuing next command
@ 2021-06-22 10:23 Paunovic, Aleksandar
2021-06-22 21:55 ` Simon Marchi
0 siblings, 1 reply; 2+ messages in thread
From: Paunovic, Aleksandar @ 2021-06-22 10:23 UTC (permalink / raw)
To: gdb-patches
When issuing "next" command the thread got deleted even though it was still alive and running. This happened because the thread was examined under a wrong inferior.
The fixed scenario:
~~~
$ gdb -q breakpoint-running-inferior-1-exe
(gdb) set schedule-multiple off
(gdb) break breakpoint-running-inferior-1.c:26
(gdb) run
(gdb) add-inferior -no-connection
(gdb) inferior 2
(gdb) spawn gdbserver :2346 breakpoint-running-inferior-2-exe
(gdb) target remote :2346
(gdb) break breakpoint-running-inferior-2.c:26
(gdb) continue
(gdb) thread 1.1
(gdb) clear breakpoint-running-inferior-2.c:26
(gdb) set schedule-multiple on
(gdb) next
(gdb) thread 1.1
~~~
Before:
~~~
Thread ID 1.1 has terminated.
~~~
Now:
~~~
Switching to thread 1.1
~~~
gdb/ChangeLog:
2021-04-30 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
* infrun.c (keep_going_stepped_thread): Switch to correct
inferior and check if thread is executing.
gdb/testsuite/ChangeLog:
2021-04-30 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
* gdb.base/breakpoint-running-inferior-1.c: New file.
* gdb.base/breakpoint-running-inferior-2.c: New file.
* gdb.base/breakpoint-running-inferior.exp: New file.
2021-06-14 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
---
gdb/infrun.c | 2 +
.../gdb.base/breakpoint-running-inferior-1.c | 39 ++++++++ .../gdb.base/breakpoint-running-inferior-2.c | 39 ++++++++ .../gdb.base/breakpoint-running-inferior.exp | 89 +++++++++++++++++++
4 files changed, 169 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/breakpoint-running-inferior-1.c
create mode 100644 gdb/testsuite/gdb.base/breakpoint-running-inferior-2.c
create mode 100644 gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
diff --git a/gdb/infrun.c b/gdb/infrun.c index 4bd21fde59..abe4db31c5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7561,6 +7561,8 @@ keep_going_stepped_thread (struct thread_info *tp)
stepping thread is still alive. For that reason, we need to
synchronously query the target now. */
+ /* Make sure that we are within a correct inferior. */
+ switch_to_inferior_no_thread (tp->inf);
if (tp->state == THREAD_EXITED || !target_thread_alive (tp->ptid))
{
infrun_debug_printf ("not resuming previously stepped thread, it has "
diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior-1.c b/gdb/testsuite/gdb.base/breakpoint-running-inferior-1.c
new file mode 100644
index 0000000000..475b2c4126
--- /dev/null
+++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior-1.c
@@ -0,0 +1,39 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2021 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>
+#include <pthread.h>
+
+void *
+forever ()
+{
+ /* Wait for alarm. */
+ while (1)
+ sleep (1); /* break here */
+}
+
+int
+main ()
+{
+ alarm (30);
+
+ pthread_t forever_thread;
+ pthread_create (&forever_thread, NULL, *forever, NULL); pthread_join
+ (forever_thread, NULL);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior-2.c b/gdb/testsuite/gdb.base/breakpoint-running-inferior-2.c
new file mode 100644
index 0000000000..475b2c4126
--- /dev/null
+++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior-2.c
@@ -0,0 +1,39 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2021 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>
+#include <pthread.h>
+
+void *
+forever ()
+{
+ /* Wait for alarm. */
+ while (1)
+ sleep (1); /* break here */
+}
+
+int
+main ()
+{
+ alarm (30);
+
+ pthread_t forever_thread;
+ pthread_create (&forever_thread, NULL, *forever, NULL); pthread_join
+ (forever_thread, NULL);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
new file mode 100644
index 0000000000..932a885e49
--- /dev/null
+++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
@@ -0,0 +1,89 @@
+# Copyright 2021 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/>.
+
+# Create two inferiors with different targets. The first runs on top
+of # a native target while the second on a remote target. Both
+inferiors # use the copy of the same source code. The copy is done in
+order to make # sure that a breakpoint is only in inferior 2. While in
+inferior 1, do # a "next" which should break in a thread in inferior 2.
+# Both executables will run total of 4 threads (2 per executable) and #
+we will put a breakpoint only in the second executable to achieve this.
+
+load_lib gdbserver-support.exp
+
+standard_testfile breakpoint-running-inferior-1.c
+breakpoint-running-inferior-2.c set binfile2 ${binfile}2
+
+if { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested "failed to compile first testcase"
+ return -1
+}
+
+if { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug}] != "" } {
+ untested "failed to compile second testcase"
+ return -1
+}
+
+# We need ability to stop all threads.
+# Hence, we go with the all-stop-on-top-of-non-stop mode.
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"maint set target-non-stop on\""
+ clean_restart ${binfile}
+}
+
+gdb_test_no_output "set schedule-multiple off"
+
+if {![runto_main]} {
+ untested "failed to run to main"
+ return -1
+}
+set break_pos [gdb_get_line_number "break here" ${srcfile}]
+gdb_breakpoint "$srcfile:$break_pos"
+gdb_continue_to_breakpoint "continue to breakpoint in inf 1"
+
+gdb_test "add-inferior -no-connection" "Added inferior 2.*" "add empty inferior 2"
+gdb_test "inferior 2" "Switching to inferior 2.*" "switch to inferior 2"
+
+# Start gdbserver and connect.
+set res [gdbserver_start "" $binfile2]
+set gdbserver_protocol [lindex $res 0]
+set gdbserver_gdbport [lindex $res 1]
+set res [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport] if
+![gdb_assert {$res == 0} "connect"] {
+ return -1
+}
+
+set break_pos [gdb_get_line_number "break here" ${srcfile2}]
+gdb_breakpoint "$srcfile2:$break_pos"
+
+gdb_continue_to_breakpoint "continue to breakpoint in inf 2"
+
+gdb_test "thread 1.1" "Switching to thread 1.1.*" "first switch to thread 1.1"
+
+# Remove the breakpoint from inf 1. Now only a breakpoint in inf 2 remains.
+gdb_test "clear $srcfile:$break_pos" "Deleted breakpoint 2 " "remove inf 1 breakpoint"
+gdb_test_no_output "set schedule-multiple on"
+gdb_test "next" ".*Thread 2.*hit Breakpoint.*" "next while in inferior 1"
+
+# We should be able to normally switch to thread 1.1.
+# In case of a bad GDB flow the GDB was losing the thread.
+gdb_test_multiple "thread 1.1" "Switching to thread 1.1" {
+ -re "\\\[Switching to thread 1.1 \\(Thread .*\\)\\\]" {
+ pass $gdb_test_name
+ }
+ -re ".*Thread ID 1.1 has terminated.*" {
+ fail $gdb_test_name
+ }
+}
--
2.17.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PING] [PATCH v2] gdb: Fix deleted thread when issuing next command
2021-06-22 10:23 [PING] [PATCH v2] gdb: Fix deleted thread when issuing next command Paunovic, Aleksandar
@ 2021-06-22 21:55 ` Simon Marchi
0 siblings, 0 replies; 2+ messages in thread
From: Simon Marchi @ 2021-06-22 21:55 UTC (permalink / raw)
To: Paunovic, Aleksandar, gdb-patches
On 2021-06-22 6:23 a.m., Paunovic, Aleksandar via Gdb-patches wrote:
> When issuing "next" command the thread got deleted even though it was still alive and running. This happened because the thread was examined under a wrong inferior.
>
> The fixed scenario:
> ~~~
> $ gdb -q breakpoint-running-inferior-1-exe
> (gdb) set schedule-multiple off
> (gdb) break breakpoint-running-inferior-1.c:26
> (gdb) run
> (gdb) add-inferior -no-connection
> (gdb) inferior 2
> (gdb) spawn gdbserver :2346 breakpoint-running-inferior-2-exe
> (gdb) target remote :2346
> (gdb) break breakpoint-running-inferior-2.c:26
> (gdb) continue
> (gdb) thread 1.1
> (gdb) clear breakpoint-running-inferior-2.c:26
> (gdb) set schedule-multiple on
> (gdb) next
> (gdb) thread 1.1
> ~~~
>
> Before:
> ~~~
> Thread ID 1.1 has terminated.
> ~~~
>
> Now:
> ~~~
> Switching to thread 1.1
> ~~~
>
> gdb/ChangeLog:
> 2021-04-30 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
>
> * infrun.c (keep_going_stepped_thread): Switch to correct
> inferior and check if thread is executing.
>
> gdb/testsuite/ChangeLog:
> 2021-04-30 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
>
> * gdb.base/breakpoint-running-inferior-1.c: New file.
> * gdb.base/breakpoint-running-inferior-2.c: New file.
> * gdb.base/breakpoint-running-inferior.exp: New file.
>
> 2021-06-14 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
> ---
> gdb/infrun.c | 2 +
> .../gdb.base/breakpoint-running-inferior-1.c | 39 ++++++++ .../gdb.base/breakpoint-running-inferior-2.c | 39 ++++++++ .../gdb.base/breakpoint-running-inferior.exp | 89 +++++++++++++++++++
> 4 files changed, 169 insertions(+)
> create mode 100644 gdb/testsuite/gdb.base/breakpoint-running-inferior-1.c
> create mode 100644 gdb/testsuite/gdb.base/breakpoint-running-inferior-2.c
> create mode 100644 gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c index 4bd21fde59..abe4db31c5 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -7561,6 +7561,8 @@ keep_going_stepped_thread (struct thread_info *tp)
> stepping thread is still alive. For that reason, we need to
> synchronously query the target now. */
>
> + /* Make sure that we are within a correct inferior. */
> + switch_to_inferior_no_thread (tp->inf);
> if (tp->state == THREAD_EXITED || !target_thread_alive (tp->ptid))
So, if I understand correctly, the important point is that
target_thread_alive is called on the wrong target stack, so it
erroneously returns that the thread is not alive? If so, the commit
message should mention that, as a step that leads to the problem.
Otherwise, the fix makes sense to me. keep_going_stepped_thread is
called with loops that iterate over all threads, so it will necessarily
be called with threads of different inferiors / target, it makes sense
that we have to switch to the correct inferior.
> {
> infrun_debug_printf ("not resuming previously stepped thread, it has "
> diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior-1.c b/gdb/testsuite/gdb.base/breakpoint-running-inferior-1.c
> new file mode 100644
> index 0000000000..475b2c4126
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior-1.c
> @@ -0,0 +1,39 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2021 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>
> +#include <pthread.h>
> +
> +void *
> +forever ()
> +{
> + /* Wait for alarm. */
> + while (1)
> + sleep (1); /* break here */
> +}
> +
> +int
> +main ()
> +{
> + alarm (30);
> +
> + pthread_t forever_thread;
> + pthread_create (&forever_thread, NULL, *forever, NULL); pthread_join
> + (forever_thread, NULL);
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior-2.c b/gdb/testsuite/gdb.base/breakpoint-running-inferior-2.c
> new file mode 100644
> index 0000000000..475b2c4126
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior-2.c
> @@ -0,0 +1,39 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2021 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>
> +#include <pthread.h>
> +
> +void *
> +forever ()
> +{
> + /* Wait for alarm. */
> + while (1)
> + sleep (1); /* break here */
> +}
> +
> +int
> +main ()
> +{
> + alarm (30);
> +
> + pthread_t forever_thread;
> + pthread_create (&forever_thread, NULL, *forever, NULL); pthread_join
> + (forever_thread, NULL);
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
> new file mode 100644
> index 0000000000..932a885e49
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
> @@ -0,0 +1,89 @@
> +# Copyright 2021 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/>.
> +
> +# Create two inferiors with different targets. The first runs on top
> +of # a native target while the second on a remote target. Both
> +inferiors # use the copy of the same source code. The copy is done in
> +order to make # sure that a breakpoint is only in inferior 2. While in
> +inferior 1, do # a "next" which should break in a thread in inferior 2.
> +# Both executables will run total of 4 threads (2 per executable) and #
> +we will put a breakpoint only in the second executable to achieve this.
The test would probably belong to gdb.multi, this is where this kind of
interaction between multiple targets are tested.
I have the feeling that "breakpoint-running-inferior" is not a very
accurate name for this test. The test steps a thread in an inferior
while a thread in another inferior is expected to hit a breakpoint, so I
guess it should be something along those lines.
When running the test, I get:
DUPLICATE: gdb.base/breakpoint-running-inferior.exp: successfully compiled posix threads test case
Please get rid of that. If it's not possible to pass an explicit test
name to gdb_compile_pthreads, you can probably use with_test_prefix.
When running your test with the native-extended-gdbserver board:
make check TESTS="gdb.base/breakpoint-running-inferior.exp" RUNTESTFLAGS="--target_board=native-extended-gdbserver"
It passes, but also says:
WARNING: Timed out waiting for EOF in server after monitor exit
This is not ideal, as the test hangs for $timeout at the end, which
makes the testsuite run for longer unnecessarily. I checked other
multi-target tests, and they seem to be skipped with that board:
make check TESTS="gdb.multi/multi-target-no-resumed.exp" RUNTESTFLAGS="--target_board=native-extended-gdbserver"
This is the code in gdb.multi/multi-target.exp.tcl that likely causes it
to be skipped:
# The plain remote target can't do multiple inferiors.
if {[target_info gdb_protocol] != ""} {
return 0
}
The comment looks wrong to me, because the code causes the test to be
skipped even with gdb_protocol is extended-remote. But the
resulting behavior makes sense to me: in this test we explicitly set up
our targets, native and remote, so it's a bit pointless to run with a
board that defaults to a remote target.
Some other tests use the approach of doing a "disconnect" in case we are
in this situation, for example:
https://gitlab.com/gnutools/gdb/-/blob/224506e95d2d44aa6583cbcda9f4b7305f834ab3/gdb/testsuite/gdb.server/server-exec-info.exp#L30-32
But I find it a bit pointless, because we end up testing exactly the
same thing as the default unix board then. So we might as well just
skip it.
Simon
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-06-22 21:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-22 10:23 [PING] [PATCH v2] gdb: Fix deleted thread when issuing next command Paunovic, Aleksandar
2021-06-22 21:55 ` Simon Marchi
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).