[PATCH v3] gdb: Fix deleted thread when issuing next command

[PATCH v3] gdb: Fix deleted thread when issuing next command

[Aleksandar Paunovic]

V2 can be found here https://sourceware.org/pipermail/gdb-patches/2021-June/179888.html
V3 addresses all comments from Simon.
Changes compared to V2:
* Commit message updated to indicate target_thread_alive was called on the wrong target stack.
* Tests are moved to gdb.multi
* Tests are renamed from breakpoint-running-inferior to break-while-other-inf-steps
* Duplicated tests are removed with with_test_prefix
* Skip the test with native-extended-gdbserver board with [target_info gdb_protocl] check.

Best,
Aleksandar

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 and therefor target_thread_alive was called on the wrong
target stack.

The fixed scenario:
~~~
$ gdb -q break-while-other-inf-steps-1-exe
(gdb) set schedule-multiple off
(gdb) break break-while-other-inf-steps-1.c:26
(gdb) run
(gdb) add-inferior -no-connection
(gdb) inferior 2
(gdb) spawn gdbserver :2346 break-while-other-inf-steps-2-exe
(gdb) target remote :2346
(gdb) break break-while-other-inf-steps-2.c:26
(gdb) continue
(gdb) thread 1.1
(gdb) clear break-while-other-inf-steps-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.multi/break-while-other-inf-steps-1.c: New file.
	* gdb.multi/break-while-other-inf-steps-2.c: New file.
	* gdb.multi/break-while-other-inf-steps.exp: New file.

2021-06-23 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
---
 gdb/infrun.c                                  |  2 +
 .../gdb.multi/break-while-other-inf-steps-1.c | 39 ++++++++
 .../gdb.multi/break-while-other-inf-steps-2.c | 39 ++++++++
 .../gdb.multi/break-while-other-inf-steps.exp | 99 +++++++++++++++++++
 4 files changed, 179 insertions(+)
 create mode 100644 gdb/testsuite/gdb.multi/break-while-other-inf-steps-1.c
 create mode 100644 gdb/testsuite/gdb.multi/break-while-other-inf-steps-2.c
 create mode 100644 gdb/testsuite/gdb.multi/break-while-other-inf-steps.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9469b74af3..1f50771e75 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7559,6 +7559,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.multi/break-while-other-inf-steps-1.c b/gdb/testsuite/gdb.multi/break-while-other-inf-steps-1.c
new file mode 100644
index 0000000000..475b2c4126
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/break-while-other-inf-steps-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.multi/break-while-other-inf-steps-2.c b/gdb/testsuite/gdb.multi/break-while-other-inf-steps-2.c
new file mode 100644
index 0000000000..475b2c4126
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/break-while-other-inf-steps-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.multi/break-while-other-inf-steps.exp b/gdb/testsuite/gdb.multi/break-while-other-inf-steps.exp
new file mode 100644
index 0000000000..6936d3db3d
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/break-while-other-inf-steps.exp
@@ -0,0 +1,99 @@
+# 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
+
+# This test explicitly sets up native and remote target.  Do not run
+# it with a board file which defaults to a remote target.
+if {[target_info gdb_protocol] != ""} {
+	return 0
+}
+
+standard_testfile break-while-other-inf-steps-1.c break-while-other-inf-steps-2.c
+set binfile2 ${binfile}2
+
+with_test_prefix "testfile 1" {
+    if  { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+        untested "failed to compile first testcase"
+        return -1
+    }
+}
+
+with_test_prefix "testfile 2" {
+    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

Re: [PATCH v3] gdb: Fix deleted thread when issuing next command

[Simon Marchi]

On 2021-06-23 2:16 p.m., Aleksandar Paunovic wrote:
> V2 can be found here https://sourceware.org/pipermail/gdb-patches/2021-June/179888.html
> V3 addresses all comments from Simon.
> Changes compared to V2:
> * Commit message updated to indicate target_thread_alive was called on the wrong target stack.
> * Tests are moved to gdb.multi
> * Tests are renamed from breakpoint-running-inferior to break-while-other-inf-steps
> * Duplicated tests are removed with with_test_prefix
> * Skip the test with native-extended-gdbserver board with [target_info gdb_protocl] check.
> 

Thanks.  There's just one issue to fix: I noticed that even with the
default board, the test doesn't finish cleanly, it finishes with:

  WARNING: Timed out waiting for EOF in server after monitor exit

The useful portion of the test finishes in like 1 second, but then takes
time to exit because of that.  If you `tail -F testsuite/gdb.log` which
running the test, you'll see what I mean.

> diff --git a/gdb/testsuite/gdb.multi/break-while-other-inf-steps.exp b/gdb/testsuite/gdb.multi/break-while-other-inf-steps.exp
> new file mode 100644
> index 0000000000..6936d3db3d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/break-while-other-inf-steps.exp
> @@ -0,0 +1,99 @@
> +# 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
> +
> +# This test explicitly sets up native and remote target.  Do not run
> +# it with a board file which defaults to a remote target.
> +if {[target_info gdb_protocol] != ""} {
> +	return 0
> +}
> +
> +standard_testfile break-while-other-inf-steps-1.c break-while-other-inf-steps-2.c

You can shorten this to:

  standard_testfile -1.c -2.c

Simon