[PING] [PATCH v2] gdb: Fix deleted thread when issuing next command
Simon Marchi
simon.marchi@polymtl.ca
Tue Jun 22 21:55:18 GMT 2021
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
More information about the Gdb-patches
mailing list