[PATCH 0/2] Some patches for multi inferior case

[PATCH 0/2] Some patches for multi inferior case

[Eduard Sargsyan via Gdb-patches]

Hi all,

this is a set of patches originally written by Alexandar Paunovic.

I just rebased them onto current master.

Regards,
Eduard

Aleksandar Paunovic (2):
  gdb: Fix deleted thread when issuing next command
  gdb: Improve the resuming of the stepped thread

 gdb/infrun.c                                  | 17 +++-
 .../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, 183 insertions(+), 1 deletion(-)
 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

-- 
2.25.1

[PATCH 1/2] gdb: Fix deleted thread when issuing next command

[Eduard Sargsyan via Gdb-patches]

From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>

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.

Signed-off-by: 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  | 84 +++++++++++++++++++
 4 files changed, 164 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 bc6521c8ec6..d794f135219 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7499,6 +7499,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 00000000000..475b2c4126e
--- /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 00000000000..475b2c4126e
--- /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 00000000000..bb0406bc659
--- /dev/null
+++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
@@ -0,0 +1,84 @@
+# 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
+}
+
+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.25.1

Re: [PATCH 1/2] gdb: Fix deleted thread when issuing next command

[Thiago Jung Bauermann via Gdb-patches]

Hello Eduard,

Eduard Sargsyan via Gdb-patches <gdb-patches@sourceware.org> writes:

> From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
>
> 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.
>
> Signed-off-by: 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  | 84 +++++++++++++++++++
>  4 files changed, 164 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

Aleksandar last posted version 4 of this patch, but this email seems to
contain either v1 or v2. Consequently, Simon's review comments aren't
addressed here. Was there a reason to return to the older version?

Also, I applied the patch to a commit from a few days ago which I had
handy (f8bfbd5b751b) and there were some failures when running the test:

                === gdb Summary ===

# of expected passes            10
# of unexpected failures        3
# of duplicate test names       1

Are the tests passing for you?

Interestingly, running the testcase with the infun.c change reverted
produced one less failure:

                === gdb Summary ===

# of expected passes            11
# of unexpected failures        2
# of duplicate test names       1

-- 
Thiago

[PATCH 2/2] gdb: Improve the resuming of the stepped thread

[Eduard Sargsyan via Gdb-patches]

From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>

Stepped thread should not be resumed (again) if the next stopping point
(next stopping PC) cannot be determined.  Do not resume the stepped thread
in this case.  Resuming would lead to an inconsistent state where the
stepped thread would be running forever and GDB would never get to
breakpoints of the other threads.  */

gdb/ChangeLog:
2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>

        * infrun.c (keep_going_stepped_thread): Do not resume an
	already executing thread.

gdb/testsuite/ChangeLog:
2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>

	* gdb.base/breakpoint-running-inferior.exp: Add a start_step
	function check.

Signed-off-by: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
---
 gdb/infrun.c                                      | 15 ++++++++++++++-
 .../gdb.base/breakpoint-running-inferior.exp      |  5 +++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index d794f135219..2d777610a37 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7472,7 +7472,7 @@ restart_after_all_stop_detach (process_stratum_target *proc_target)
 
 /* Set a previously stepped thread back to stepping.  Returns true on
    success, false if the resume is not possible (e.g., the thread
-   vanished).  */
+   vanished, or the thread breakpoint position cannot be determined).  */
 
 static bool
 keep_going_stepped_thread (struct thread_info *tp)
@@ -7509,6 +7509,19 @@ keep_going_stepped_thread (struct thread_info *tp)
       delete_thread (tp);
       return false;
     }
+  /* Step start function determines the location of the next stopping
+     point (next PC where the stepped thread should stop).  In case that
+     this position cannot be determined the step_start_function will not
+     be set.  Do not resume the stepped thread in this case.  Resuming
+     would lead to an inconsistent state where the stepped thread would be
+     running forever and GDB would never get to breakpoints of the other
+     threads.  */
+  if (tp->control.step_start_function == nullptr)
+    {
+      infrun_debug_printf ("not resuming previously stepped thread, it is "
+			   "already executing");
+      return 0;
+    }
 
   infrun_debug_printf ("resuming previously stepped thread");
 
diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
index bb0406bc659..b95f2ec0122 100644
--- a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
+++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
@@ -74,10 +74,15 @@ 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.
+# The thread should also not be in a "running" state because it is
+# stopped.
 gdb_test_multiple "thread 1.1" "Switching to thread 1.1" {
     -re "\\\[Switching to thread 1.1 \\(Thread .*\\)\\\]" {
         pass $gdb_test_name
     }
+    -re "\\\[Switching to thread 1.1.*\\(running\\)" {
+        fail $gdb_test_name
+    }
     -re ".*Thread ID 1.1 has terminated.*" {
         fail $gdb_test_name
     }
-- 
2.25.1

Re: [PATCH 2/2] gdb: Improve the resuming of the stepped thread

[Thiago Jung Bauermann via Gdb-patches]

Hello Eduard,

Eduard Sargsyan via Gdb-patches <gdb-patches@sourceware.org> writes:

> From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
>
> Stepped thread should not be resumed (again) if the next stopping point
> (next stopping PC) cannot be determined.  Do not resume the stepped thread
> in this case.  Resuming would lead to an inconsistent state where the
> stepped thread would be running forever and GDB would never get to
> breakpoints of the other threads.  */
>
> gdb/ChangeLog:
> 2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>
>
>         * infrun.c (keep_going_stepped_thread): Do not resume an
> 	already executing thread.
>
> gdb/testsuite/ChangeLog:
> 2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>
>
> 	* gdb.base/breakpoint-running-inferior.exp: Add a start_step
> 	function check.
>
> Signed-off-by: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
> ---
>  gdb/infrun.c                                      | 15 ++++++++++++++-
>  .../gdb.base/breakpoint-running-inferior.exp      |  5 +++++
>  2 files changed, 19 insertions(+), 1 deletion(-)

When Andrew Burgess reviewed this patch, Aleksandar said¹:

> Actually now when I tested this further I saw that this check (the whole patch) is
> unnecessary because GDB recovers successfully.
> Originally the regcache_read_pc function call (10 lines below my change) was throwing "PC
> register is not available" but it seems in the meantime this is fixed.
> I will put only first patch for review.

Has anything changed since then?

-- 
Thiago

¹ https://inbox.sourceware.org/gdb-patches/DM6PR11MB25889C3D6F45819B7D86EAE2F0319@DM6PR11MB2588.namprd11.prod.outlook.com/

RE: [PATCH 0/2] Some patches for multi inferior case

[Sargsyan, Eduard via Gdb-patches]

Hello.
Any ideas about the patches?

-----Original Message-----
From: Sargsyan, Eduard <eduard.sargsyan@intel.com> 
Sent: Monday, May 23, 2022 8:01 PM
To: gdb-patches@sourceware.org
Cc: Sargsyan, Eduard <eduard.sargsyan@intel.com>
Subject: [PATCH 0/2] Some patches for multi inferior case

Hi all,

this is a set of patches originally written by Alexandar Paunovic.

I just rebased them onto current master.

Regards,
Eduard

Aleksandar Paunovic (2):
  gdb: Fix deleted thread when issuing next command
  gdb: Improve the resuming of the stepped thread

 gdb/infrun.c                                  | 17 +++-
 .../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, 183 insertions(+), 1 deletion(-)  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

--
2.25.1

[PATCH 0/2] Fix next command in the running inferior

[aleksandar.paunovic via Gdb-patches]

From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>

Next command caused a thread in an inferior to be deleted and other
threads to be in inconsistent state.  This is now fixed.

Aleksandar Paunovic (2):
  gdb: Fix deleted thread when issuing next command
  gdb: Improve the resuming of the stepped thread

 gdb/infrun.c                                  | 17 +++-
 .../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, 183 insertions(+), 1 deletion(-)
 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

-- 
2.17.1

[PATCH 2/2] gdb: Improve the resuming of the stepped thread

[aleksandar.paunovic via Gdb-patches]

From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>

Stepped thread should not be resumed (again) if the next stopping point
(next stopping PC) cannot be determined.  Do not resume the stepped thread
in this case.  Resuming would lead to an inconsistent state where the
stepped thread would be running forever and GDB would never get to
breakpoints of the other threads.  */

gdb/ChangeLog:
2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>

        * infrun.c (keep_going_stepped_thread): Do not resume an
	already executing thread.

gdb/testsuite/ChangeLog:
2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>

	* gdb.base/breakpoint-running-inferior.exp: Add a start_step
	function check.

2021-06-07 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
---
 gdb/infrun.c                                      | 15 ++++++++++++++-
 .../gdb.base/breakpoint-running-inferior.exp      |  5 +++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 78da1f0e72..459a679ee6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7529,7 +7529,7 @@ restart_after_all_stop_detach (process_stratum_target *proc_target)
 
 /* Set a previously stepped thread back to stepping.  Returns true on
    success, false if the resume is not possible (e.g., the thread
-   vanished).  */
+   vanished, or the thread breakpoint position cannot be determined).  */
 
 static bool
 keep_going_stepped_thread (struct thread_info *tp)
@@ -7566,6 +7566,19 @@ keep_going_stepped_thread (struct thread_info *tp)
       delete_thread (tp);
       return false;
     }
+  /* Step start function determines the location of the next stopping
+     point (next PC where the stepped thread should stop).  In case that
+     this position cannot be determined the step_start_function will not
+     be set.  Do not resume the stepped thread in this case.  Resuming
+     would lead to an inconsistent state where the stepped thread would be
+     running forever and GDB would never get to breakpoints of the other
+     threads.  */
+  if (tp->control.step_start_function == nullptr)
+    {
+      infrun_debug_printf ("not resuming previously stepped thread, it is "
+			   "already executing");
+      return 0;
+    }
 
   infrun_debug_printf ("resuming previously stepped thread");
 
diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
index bb0406bc65..b95f2ec012 100644
--- a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
+++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
@@ -74,10 +74,15 @@ 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.
+# The thread should also not be in a "running" state because it is
+# stopped.
 gdb_test_multiple "thread 1.1" "Switching to thread 1.1" {
     -re "\\\[Switching to thread 1.1 \\(Thread .*\\)\\\]" {
         pass $gdb_test_name
     }
+    -re "\\\[Switching to thread 1.1.*\\(running\\)" {
+        fail $gdb_test_name
+    }
     -re ".*Thread ID 1.1 has terminated.*" {
         fail $gdb_test_name
     }
-- 
2.17.1

Re: [PATCH 2/2] gdb: Improve the resuming of the stepped thread

[Andrew Burgess]

* aleksandar.paunovic via Gdb-patches <gdb-patches@sourceware.org> [2021-06-07 10:42:03 +0200]:

> From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
> 
> Stepped thread should not be resumed (again) if the next stopping point
> (next stopping PC) cannot be determined.  Do not resume the stepped thread
> in this case.  Resuming would lead to an inconsistent state where the
> stepped thread would be running forever and GDB would never get to
> breakpoints of the other threads.  */
> 
> gdb/ChangeLog:
> 2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>
> 
>         * infrun.c (keep_going_stepped_thread): Do not resume an
> 	already executing thread.
> 
> gdb/testsuite/ChangeLog:
> 2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>
> 
> 	* gdb.base/breakpoint-running-inferior.exp: Add a start_step
> 	function check.
> 
> 2021-06-07 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
> ---
>  gdb/infrun.c                                      | 15 ++++++++++++++-
>  .../gdb.base/breakpoint-running-inferior.exp      |  5 +++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 78da1f0e72..459a679ee6 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -7529,7 +7529,7 @@ restart_after_all_stop_detach (process_stratum_target *proc_target)
>  
>  /* Set a previously stepped thread back to stepping.  Returns true on
>     success, false if the resume is not possible (e.g., the thread
> -   vanished).  */
> +   vanished, or the thread breakpoint position cannot be determined).  */
>  
>  static bool
>  keep_going_stepped_thread (struct thread_info *tp)
> @@ -7566,6 +7566,19 @@ keep_going_stepped_thread (struct thread_info *tp)
>        delete_thread (tp);
>        return false;
>      }
> +  /* Step start function determines the location of the next stopping
> +     point (next PC where the stepped thread should stop).  In case that
> +     this position cannot be determined the step_start_function will not
> +     be set.  Do not resume the stepped thread in this case.  Resuming
> +     would lead to an inconsistent state where the stepped thread would be
> +     running forever and GDB would never get to breakpoints of the other
> +     threads.  */
> +  if (tp->control.step_start_function == nullptr)
> +    {
> +      infrun_debug_printf ("not resuming previously stepped thread, it is "
> +			   "already executing");
> +      return 0;

I don't understand why the condition you're checking means that the
thread is already executing.

Also, your comment says step_start_function is the pc where we should
next stop, but it is set with find_pc_function, which just gives the
symbol for the function containing the $pc when the step/next was
started, so I don't understand why this tells GDB where to stop.

Also 'return false'.

Thanks,
Andrew


> +    }
>  
>    infrun_debug_printf ("resuming previously stepped thread");
>  
> diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
> index bb0406bc65..b95f2ec012 100644
> --- a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
> +++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
> @@ -74,10 +74,15 @@ 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.
> +# The thread should also not be in a "running" state because it is
> +# stopped.
>  gdb_test_multiple "thread 1.1" "Switching to thread 1.1" {
>      -re "\\\[Switching to thread 1.1 \\(Thread .*\\)\\\]" {
>          pass $gdb_test_name
>      }
> +    -re "\\\[Switching to thread 1.1.*\\(running\\)" {
> +        fail $gdb_test_name
> +    }
>      -re ".*Thread ID 1.1 has terminated.*" {
>          fail $gdb_test_name
>      }
> -- 
> 2.17.1
> 

RE: [PATCH 2/2] gdb: Improve the resuming of the stepped thread

[Paunovic, Aleksandar via Gdb-patches]

>> From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
>> 
>> Stepped thread should not be resumed (again) if the next stopping 
>> point (next stopping PC) cannot be determined.  Do not resume the 
>> stepped thread in this case.  Resuming would lead to an inconsistent 
>> state where the stepped thread would be running forever and GDB would 
>> never get to breakpoints of the other threads.  */
>> 
>> gdb/ChangeLog:
>> 2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>
>> 
>>         * infrun.c (keep_going_stepped_thread): Do not resume an
>> 	already executing thread.
>> 
>> gdb/testsuite/ChangeLog:
>> 2021-04-30  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>
>> 
>> 	* gdb.base/breakpoint-running-inferior.exp: Add a start_step
>> 	function check.
>> 
>> 2021-06-07 Aleksandar Paunovic <aleksandar.paunovic@intel.com>
>> ---
>>  gdb/infrun.c                                      | 15 ++++++++++++++-
>>  .../gdb.base/breakpoint-running-inferior.exp      |  5 +++++
>>  2 files changed, 19 insertions(+), 1 deletion(-)
>> 
>> diff --git a/gdb/infrun.c b/gdb/infrun.c index 78da1f0e72..459a679ee6 
>> 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -7529,7 +7529,7 @@ restart_after_all_stop_detach 
>> (process_stratum_target *proc_target)
>>  
>>  /* Set a previously stepped thread back to stepping.  Returns true on
>>     success, false if the resume is not possible (e.g., the thread
>> -   vanished).  */
>> +   vanished, or the thread breakpoint position cannot be determined).  
>> + */
>>  
>>  static bool
>>  keep_going_stepped_thread (struct thread_info *tp) @@ -7566,6 
>> +7566,19 @@ keep_going_stepped_thread (struct thread_info *tp)
>>        delete_thread (tp);
>>        return false;
>>      }
>> +  /* Step start function determines the location of the next stopping
>> +     point (next PC where the stepped thread should stop).  In case that
>> +     this position cannot be determined the step_start_function will not
>> +     be set.  Do not resume the stepped thread in this case.  Resuming
>> +     would lead to an inconsistent state where the stepped thread would be
>> +     running forever and GDB would never get to breakpoints of the other
>> +     threads.  */
>> +  if (tp->control.step_start_function == nullptr)
>> +    {
>> +      infrun_debug_printf ("not resuming previously stepped thread, it is "
>> +			   "already executing");
>> +      return 0;
>
>I don't understand why the condition you're checking means that the thread is already executing.
>
>Also, your comment says step_start_function is the pc where we should next stop, but it is set with find_pc_function, which just gives the symbol for the function containing the $pc when the step/next was started, so I don't understand why this tells GDB where to stop.
>
>Also 'return false'.
>
>Thanks,
>Andrew

Actually now when I tested this further I saw that this check (the whole patch) is unnecessary because GDB recovers successfully.
Originally the regcache_read_pc function call (10 lines below my change) was throwing "PC register is not available" but it seems in the meantime this is fixed.
I will put only first patch for review.

>> +    }
>>  
>>    infrun_debug_printf ("resuming previously stepped thread");
>>  
>> diff --git a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp 
>> b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
>> index bb0406bc65..b95f2ec012 100644
>> --- a/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
>> +++ b/gdb/testsuite/gdb.base/breakpoint-running-inferior.exp
>> @@ -74,10 +74,15 @@ 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.
>> +# The thread should also not be in a "running" state because it is # 
>> +stopped.
>>  gdb_test_multiple "thread 1.1" "Switching to thread 1.1" {
>>      -re "\\\[Switching to thread 1.1 \\(Thread .*\\)\\\]" {
>>          pass $gdb_test_name
>>      }
>> +    -re "\\\[Switching to thread 1.1.*\\(running\\)" {
>> +        fail $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