[PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Tankut Baris Aktemur via Gdb-patches]

======
Changes in V2:

* Changed the thread state check to `(tp->state != THREAD_RUNNING)`.
* Rebased on the current master.

The latest discussion was
https://sourceware.org/pipermail/gdb-patches/2023-May/199613.html

======

Suppose we have two inferiors on an all-stop target with schedule-multi
set on:

  $ gdb -q
  (gdb) target extended-remote | gdbserver --multi -
  Remote debugging using | gdbserver --multi -
  Remote debugging using stdio
  (gdb) file /temp/test
  Reading symbols from /temp/test...
  (gdb) set remote exec-file /temp/test
  (gdb) start
  Temporary breakpoint 1 at 0x115c: file test.c, line 8.
  Starting program: /temp/test
  stdin/stdout redirected
  Process /temp/test created; pid = 864027
  ...

  Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd218) at test.c:8
  8         foo();
  (gdb) add-inferior
  [New inferior 2]
  Added inferior 2 on connection 1 (extended-remote | gdbserver --multi -)
  (gdb) inferior 2
  [Switching to inferior 2 [<null>] (<noexec>)]
  (gdb) file /temp/test
  Reading symbols from /temp/test...
  (gdb) set remote exec-file /temp/test
  (gdb) tbreak 2
  Temporary breakpoint 2 at 0x555555555131: /temp/test.c:2. (2 locations)
  (gdb) run
  Starting program: /temp/test
  stdin/stdout redirected
  Process /temp/test created; pid = 864430
  ...

  Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
  2         int a = 42;
  (gdb) set schedule-multi on
  (gdb)

At this point, detaching the first inferior works fine:

  (gdb) detach inferiors 1
  Detaching from program: /temp/test, process 858904
  Detaching from process 858904
  [Inferior 1 (process 858904) detached]
  (gdb) info inferiors
    Num  Description       Connection                                Executable
    1    <null>            1 (extended-remote | gdbserver --multi -) /temp/test
  * 2    process 858925    1 (extended-remote | gdbserver --multi -) /temp/test
  (gdb)

Let us now repeat exactly the same scenario, but before detaching, we
make the current thread single-step an instruction:

  ...
  Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
  2         int a = 42;
  (gdb) stepi
  3         int b = 43;
  (gdb) detach inferiors 1
  Detaching from program: /temp/test, process 876580
  Detaching from process 876580
  gdbserver: Couldn't reap LWP 876580 while detaching: No child processes
  [Inferior 1 (process 876580) detached]
  (gdb) 3   int b = 43;

There is a mysterious line info output.  Running the scenario with
infrun debug logs reveals more information.

  ...
  Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
  2         int a = 42;
  (gdb) stepi
  3         int b = 43;
  (gdb) set debug infrun on
  (gdb) detach inferiors 1
  [infrun] scoped_disable_commit_resumed: reason=detaching
  Detaching from program: /temp/test, process 872445
  Detaching from process 872445
  gdbserver: Couldn't reap LWP 872445 while detaching: No child processes
  [Inferior 1 (process 872445) detached]
  [infrun] start_step_over: enter
    [infrun] start_step_over: stealing global queue of threads to step, length = 0
    [infrun] operator(): step-over queue now empty
  [infrun] start_step_over: exit
  [infrun] restart_stepped_thread: switching back to stepped thread (stepping)
  [infrun] keep_going_stepped_thread: resuming previously stepped thread
  [infrun] keep_going_stepped_thread: expected thread advanced also (0x555555555131 -> 0x555555555138)
  [infrun] clear_step_over_info: clearing step over info
  [infrun] do_target_resume: resume_ptid=-1.0.0, step=0, sig=GDB_SIGNAL_0
  [infrun] infrun_async: enable=1
  [infrun] reset: reason=detaching
  [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target extended-remote
  [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target extended-remote
  (gdb) [infrun] fetch_inferior_event: enter
    [infrun] scoped_disable_commit_resumed: reason=handling event
    [infrun] do_target_wait: Found 2 inferiors, starting at #0
    [infrun] random_pending_event_thread: None found.
    [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
    [infrun] print_target_wait_results:   872464.872464.0 [Thread 872464.872464],
    [infrun] print_target_wait_results:   status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
    [infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
    [infrun] context_switch: Switching context from 0.0.0 to 872464.872464.0
    [infrun] handle_signal_stop: stop_pc=0x555555555138
    [infrun] handle_signal_stop: [872464.872464.0] hit its single-step breakpoint
    [infrun] handle_signal_stop: delayed software breakpoint trap, ignoring
    [infrun] process_event_stop_test: stepi/nexti
    [infrun] stop_waiting: stop_waiting
  3         int b = 43;
    [infrun] infrun_async: enable=0
    [infrun] reset: reason=handling event
    [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for target extended-remote, no resumed threads
  [infrun] fetch_inferior_event: exit

GDB attempted to do a step-over for the current thread.  This takes us
to the commit that introduced restarting step-overs:

  commit 408f66864a1a823591b26420410c982174c239a2
  Author: Pedro Alves <pedro@palves.net>
  Date:   Mon Jan 11 20:01:58 2021 +0000

    detach in all-stop with threads running

    A following patch will add a testcase that has a number of threads
    constantly stepping over a breakpoint, and then has GDB detach the
    process, while threads are running.  If we have more than one inferior
    running, and we detach from just one of the inferiors, we expect that
    the remaining inferior continues running.  However, in all-stop, if
    GDB needs to pause the target for the detach, nothing is re-resuming
    the other inferiors after the detach.  "info threads" shows the
    threads as running, but they really aren't.  This fixes it.

However, the thread that was resumed for step-over in our scenario did
not have an interrupted step-over; it had completed its stepi already.
More debugging reveals that the thread is resumed because of the
following two conditions in `restart_stepped_thread`:

      if (tp->control.trap_expected)
        {
          infrun_debug_printf ("switching back to stepped thread (step-over)");

          if (keep_going_stepped_thread (tp))
            return true;
        }

and

      if (tp->control.step_range_end)
        {
          infrun_debug_printf ("switching back to stepped thread (stepping)");

          if (keep_going_stepped_thread (tp))
            return true;
        }

The root cause of the problem is, `restart_stepped_thread` checks for
the thread state as

    if (tp->state == THREAD_EXITED)
      continue;

but the thread's state is THREAD_STOPPED.  To fix, we change the state
check to

    if (tp->state != THREAD_RUNNING)

Additionally, the 'trap_expected' and the 'step_range_end' fields of
the thread's control remain set even after the "stepi" command
completes, creating a half-baked internal state that can be misleading
when debugging.  We address this problem by clearing the control
fields when stepping completes.  We also add a regression test.

Regression-tested on X86_64 Linux using the default, native-gdbserver,
and native-extended-gdbserver board files.
---
 gdb/infrun.c                             |  7 ++-
 gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
 gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.c
 create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 58da1cef29e..f40d924dc7a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7776,7 +7776,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
 
   for (thread_info *tp : all_threads_safe ())
     {
-      if (tp->state == THREAD_EXITED)
+      if (tp->state != THREAD_RUNNING)
 	continue;
 
       if (tp->has_pending_waitstatus ())
@@ -7800,7 +7800,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
 
   for (thread_info *tp : all_threads_safe ())
     {
-      if (tp->state == THREAD_EXITED)
+      if (tp->state != THREAD_RUNNING)
 	continue;
 
       if (tp->has_pending_waitstatus ())
@@ -8523,6 +8523,9 @@ static void
 end_stepping_range (struct execution_control_state *ecs)
 {
   ecs->event_thread->control.stop_step = 1;
+  ecs->event_thread->control.trap_expected = 0;
+  ecs->event_thread->control.step_range_start = 0;
+  ecs->event_thread->control.step_range_end = 0;
   stop_waiting (ecs);
 }
 
diff --git a/gdb/testsuite/gdb.multi/detach-stepi.c b/gdb/testsuite/gdb.multi/detach-stepi.c
new file mode 100644
index 00000000000..d365645fb3f
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/detach-stepi.c
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 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/>.  */
+
+void
+a_function ()
+{
+  int a = 42;
+}
+
+int
+main ()
+{
+  int b = 43;
+  a_function ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp b/gdb/testsuite/gdb.multi/detach-stepi.exp
new file mode 100644
index 00000000000..28ef8c4f9f7
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
@@ -0,0 +1,66 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2022 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/>.
+
+# Test detaching from an inferior after a thread in another inferior
+# completes a stepi.  This is a regression test for a bug that was
+# causing an inadvertent resume of the just-stepped thread.
+
+standard_testfile
+
+if {[use_gdb_stub]} {
+    untested "using gdb stub"
+    return 0
+}
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+delete_breakpoints
+
+# Setup inferior 2.
+gdb_test "add-inferior" "Added inferior .*" \
+    "add empty inferior"
+gdb_test "inferior 2" "Switching to inferior .*" \
+    "switch to inferior"
+
+gdb_load $binfile
+runto "a_function"
+gdb_test "info inferiors"
+
+# The bug for which this regression test is written appears in
+# schedule-multi mode.
+gdb_test_no_output "set schedule-multiple on"
+
+# Single-step the thread in Inferior 2, then detach Inferior 1.
+gdb_test "info threads" ".*" "threads before stepi"
+gdb_test "stepi"
+gdb_test "info threads" ".*" "threads after stepi"
+
+gdb_test "set debug infrun on"
+gdb_test_multiple "detach inferior 1" "" {
+    -re "resuming previously stepped thread.*$gdb_prompt" {
+	fail $gdb_test_name
+    }
+    -re "$gdb_prompt $" {
+	pass $gdb_test_name
+    }
+}
-- 
2.25.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

Re: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Bruno Larsen via Gdb-patches]

On 20/06/2023 17:24, Tankut Baris Aktemur via Gdb-patches wrote:
> ======
> Changes in V2:
>
> * Changed the thread state check to `(tp->state != THREAD_RUNNING)`.
> * Rebased on the current master.
>
> The latest discussion was
> https://sourceware.org/pipermail/gdb-patches/2023-May/199613.html
>
> ======
>
> Suppose we have two inferiors on an all-stop target with schedule-multi
> set on:
>
>    $ gdb -q
>    (gdb) target extended-remote | gdbserver --multi -
>    Remote debugging using | gdbserver --multi -
>    Remote debugging using stdio
>    (gdb) file /temp/test
>    Reading symbols from /temp/test...
>    (gdb) set remote exec-file /temp/test
>    (gdb) start
>    Temporary breakpoint 1 at 0x115c: file test.c, line 8.
>    Starting program: /temp/test
>    stdin/stdout redirected
>    Process /temp/test created; pid = 864027
>    ...
>
>    Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd218) at test.c:8
>    8         foo();
>    (gdb) add-inferior
>    [New inferior 2]
>    Added inferior 2 on connection 1 (extended-remote | gdbserver --multi -)
>    (gdb) inferior 2
>    [Switching to inferior 2 [<null>] (<noexec>)]
>    (gdb) file /temp/test
>    Reading symbols from /temp/test...
>    (gdb) set remote exec-file /temp/test
>    (gdb) tbreak 2
>    Temporary breakpoint 2 at 0x555555555131: /temp/test.c:2. (2 locations)
>    (gdb) run
>    Starting program: /temp/test
>    stdin/stdout redirected
>    Process /temp/test created; pid = 864430
>    ...
>
>    Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
>    2         int a = 42;
>    (gdb) set schedule-multi on
>    (gdb)
>
> At this point, detaching the first inferior works fine:
>
>    (gdb) detach inferiors 1
>    Detaching from program: /temp/test, process 858904
>    Detaching from process 858904
>    [Inferior 1 (process 858904) detached]
>    (gdb) info inferiors
>      Num  Description       Connection                                Executable
>      1    <null>            1 (extended-remote | gdbserver --multi -) /temp/test
>    * 2    process 858925    1 (extended-remote | gdbserver --multi -) /temp/test
>    (gdb)
>
> Let us now repeat exactly the same scenario, but before detaching, we
> make the current thread single-step an instruction:
>
>    ...
>    Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
>    2         int a = 42;
>    (gdb) stepi
>    3         int b = 43;
>    (gdb) detach inferiors 1
>    Detaching from program: /temp/test, process 876580
>    Detaching from process 876580
>    gdbserver: Couldn't reap LWP 876580 while detaching: No child processes
>    [Inferior 1 (process 876580) detached]
>    (gdb) 3   int b = 43;
>
> There is a mysterious line info output.  Running the scenario with
> infrun debug logs reveals more information.
>
>    ...
>    Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
>    2         int a = 42;
>    (gdb) stepi
>    3         int b = 43;
>    (gdb) set debug infrun on
>    (gdb) detach inferiors 1
>    [infrun] scoped_disable_commit_resumed: reason=detaching
>    Detaching from program: /temp/test, process 872445
>    Detaching from process 872445
>    gdbserver: Couldn't reap LWP 872445 while detaching: No child processes
>    [Inferior 1 (process 872445) detached]
>    [infrun] start_step_over: enter
>      [infrun] start_step_over: stealing global queue of threads to step, length = 0
>      [infrun] operator(): step-over queue now empty
>    [infrun] start_step_over: exit
>    [infrun] restart_stepped_thread: switching back to stepped thread (stepping)
>    [infrun] keep_going_stepped_thread: resuming previously stepped thread
>    [infrun] keep_going_stepped_thread: expected thread advanced also (0x555555555131 -> 0x555555555138)
>    [infrun] clear_step_over_info: clearing step over info
>    [infrun] do_target_resume: resume_ptid=-1.0.0, step=0, sig=GDB_SIGNAL_0
>    [infrun] infrun_async: enable=1
>    [infrun] reset: reason=detaching
>    [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target extended-remote
>    [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target extended-remote
>    (gdb) [infrun] fetch_inferior_event: enter
>      [infrun] scoped_disable_commit_resumed: reason=handling event
>      [infrun] do_target_wait: Found 2 inferiors, starting at #0
>      [infrun] random_pending_event_thread: None found.
>      [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
>      [infrun] print_target_wait_results:   872464.872464.0 [Thread 872464.872464],
>      [infrun] print_target_wait_results:   status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
>      [infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
>      [infrun] context_switch: Switching context from 0.0.0 to 872464.872464.0
>      [infrun] handle_signal_stop: stop_pc=0x555555555138
>      [infrun] handle_signal_stop: [872464.872464.0] hit its single-step breakpoint
>      [infrun] handle_signal_stop: delayed software breakpoint trap, ignoring
>      [infrun] process_event_stop_test: stepi/nexti
>      [infrun] stop_waiting: stop_waiting
>    3         int b = 43;
>      [infrun] infrun_async: enable=0
>      [infrun] reset: reason=handling event
>      [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for target extended-remote, no resumed threads
>    [infrun] fetch_inferior_event: exit
>
> GDB attempted to do a step-over for the current thread.  This takes us
> to the commit that introduced restarting step-overs:
>
>    commit 408f66864a1a823591b26420410c982174c239a2
>    Author: Pedro Alves <pedro@palves.net>
>    Date:   Mon Jan 11 20:01:58 2021 +0000
>
>      detach in all-stop with threads running
>
>      A following patch will add a testcase that has a number of threads
>      constantly stepping over a breakpoint, and then has GDB detach the
>      process, while threads are running.  If we have more than one inferior
>      running, and we detach from just one of the inferiors, we expect that
>      the remaining inferior continues running.  However, in all-stop, if
>      GDB needs to pause the target for the detach, nothing is re-resuming
>      the other inferiors after the detach.  "info threads" shows the
>      threads as running, but they really aren't.  This fixes it.
>
> However, the thread that was resumed for step-over in our scenario did
> not have an interrupted step-over; it had completed its stepi already.
> More debugging reveals that the thread is resumed because of the
> following two conditions in `restart_stepped_thread`:
>
>        if (tp->control.trap_expected)
>          {
>            infrun_debug_printf ("switching back to stepped thread (step-over)");
>
>            if (keep_going_stepped_thread (tp))
>              return true;
>          }
>
> and
>
>        if (tp->control.step_range_end)
>          {
>            infrun_debug_printf ("switching back to stepped thread (stepping)");
>
>            if (keep_going_stepped_thread (tp))
>              return true;
>          }
>
> The root cause of the problem is, `restart_stepped_thread` checks for
> the thread state as
>
>      if (tp->state == THREAD_EXITED)
>        continue;
>
> but the thread's state is THREAD_STOPPED.  To fix, we change the state
> check to
>
>      if (tp->state != THREAD_RUNNING)
>
> Additionally, the 'trap_expected' and the 'step_range_end' fields of
> the thread's control remain set even after the "stepi" command
> completes, creating a half-baked internal state that can be misleading
> when debugging.  We address this problem by clearing the control
> fields when stepping completes.  We also add a regression test.
>
> Regression-tested on X86_64 Linux using the default, native-gdbserver,
> and native-extended-gdbserver board files.

Hi Tankut!

Thanks for working on this. I don't see any regressions, but the test 
you added doesn't seem to fail before your change. Does it not work with 
the default options for the testsuite?

-- 
Cheers,
Bruno

> ---
>   gdb/infrun.c                             |  7 ++-
>   gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>   gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>   3 files changed, 101 insertions(+), 2 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.c
>   create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.exp
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 58da1cef29e..f40d924dc7a 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -7776,7 +7776,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
>   
>     for (thread_info *tp : all_threads_safe ())
>       {
> -      if (tp->state == THREAD_EXITED)
> +      if (tp->state != THREAD_RUNNING)
>   	continue;
>   
>         if (tp->has_pending_waitstatus ())
> @@ -7800,7 +7800,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
>   
>     for (thread_info *tp : all_threads_safe ())
>       {
> -      if (tp->state == THREAD_EXITED)
> +      if (tp->state != THREAD_RUNNING)
>   	continue;
>   
>         if (tp->has_pending_waitstatus ())
> @@ -8523,6 +8523,9 @@ static void
>   end_stepping_range (struct execution_control_state *ecs)
>   {
>     ecs->event_thread->control.stop_step = 1;
> +  ecs->event_thread->control.trap_expected = 0;
> +  ecs->event_thread->control.step_range_start = 0;
> +  ecs->event_thread->control.step_range_end = 0;
>     stop_waiting (ecs);
>   }
>   
> diff --git a/gdb/testsuite/gdb.multi/detach-stepi.c b/gdb/testsuite/gdb.multi/detach-stepi.c
> new file mode 100644
> index 00000000000..d365645fb3f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/detach-stepi.c
> @@ -0,0 +1,30 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 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/>.  */
> +
> +void
> +a_function ()
> +{
> +  int a = 42;
> +}
> +
> +int
> +main ()
> +{
> +  int b = 43;
> +  a_function ();
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp b/gdb/testsuite/gdb.multi/detach-stepi.exp
> new file mode 100644
> index 00000000000..28ef8c4f9f7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
> @@ -0,0 +1,66 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2022 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/>.
> +
> +# Test detaching from an inferior after a thread in another inferior
> +# completes a stepi.  This is a regression test for a bug that was
> +# causing an inadvertent resume of the just-stepped thread.
> +
> +standard_testfile
> +
> +if {[use_gdb_stub]} {
> +    untested "using gdb stub"
> +    return 0
> +}
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> +    return -1
> +}
> +
> +if {![runto_main]} {
> +    return -1
> +}
> +
> +delete_breakpoints
> +
> +# Setup inferior 2.
> +gdb_test "add-inferior" "Added inferior .*" \
> +    "add empty inferior"
> +gdb_test "inferior 2" "Switching to inferior .*" \
> +    "switch to inferior"
> +
> +gdb_load $binfile
> +runto "a_function"
> +gdb_test "info inferiors"
> +
> +# The bug for which this regression test is written appears in
> +# schedule-multi mode.
> +gdb_test_no_output "set schedule-multiple on"
> +
> +# Single-step the thread in Inferior 2, then detach Inferior 1.
> +gdb_test "info threads" ".*" "threads before stepi"
> +gdb_test "stepi"
> +gdb_test "info threads" ".*" "threads after stepi"
> +
> +gdb_test "set debug infrun on"
> +gdb_test_multiple "detach inferior 1" "" {
> +    -re "resuming previously stepped thread.*$gdb_prompt" {
> +	fail $gdb_test_name
> +    }
> +    -re "$gdb_prompt $" {
> +	pass $gdb_test_name
> +    }
> +}

RE: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Aktemur, Tankut Baris via Gdb-patches]

On Wednesday, June 21, 2023 2:35 PM, Bruno Larsen wrote:
> > Regression-tested on X86_64 Linux using the default, native-gdbserver,
> > and native-extended-gdbserver board files.
> 
> Hi Tankut!
> 
> Thanks for working on this. I don't see any regressions, but the test
> you added doesn't seem to fail before your change. Does it not work with
> the default options for the testsuite?

Hi Bruno,

Thanks for checking.  The bug condition is triggered when the target is all-stop.
Could you please try with --target_board=native-extended-gdbserver ?
Then you should see

  FAIL: gdb.multi/detach-stepi.exp: detach inferior 1

  === gdb Summary ===

  # of expected passes            8
  # of unexpected failures        1

-Baris

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

Re: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Bruno Larsen via Gdb-patches]

On 22/06/2023 09:02, Aktemur, Tankut Baris wrote:
> On Wednesday, June 21, 2023 2:35 PM, Bruno Larsen wrote:
>>> Regression-tested on X86_64 Linux using the default, native-gdbserver,
>>> and native-extended-gdbserver board files.
>> Hi Tankut!
>>
>> Thanks for working on this. I don't see any regressions, but the test
>> you added doesn't seem to fail before your change. Does it not work with
>> the default options for the testsuite?
> Hi Bruno,
>
> Thanks for checking.  The bug condition is triggered when the target is all-stop.
> Could you please try with --target_board=native-extended-gdbserver ?
> Then you should see
>
>    FAIL: gdb.multi/detach-stepi.exp: detach inferior 1
>
>    === gdb Summary ===
>
>    # of expected passes            8
>    # of unexpected failures        1
>
Ah got it! I have tested and it fixes the problem :)

Tested-By: Bruno Larsen <blarsen@redhat.com>

-- 
Cheers,
Bruno

RE: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Aktemur, Tankut Baris via Gdb-patches]

On Thursday, June 22, 2023 9:40 AM, Bruno Larsen wrote:
> On 22/06/2023 09:02, Aktemur, Tankut Baris wrote:
> > On Wednesday, June 21, 2023 2:35 PM, Bruno Larsen wrote:
> >>> Regression-tested on X86_64 Linux using the default, native-gdbserver,
> >>> and native-extended-gdbserver board files.
> >> Hi Tankut!
> >>
> >> Thanks for working on this. I don't see any regressions, but the test
> >> you added doesn't seem to fail before your change. Does it not work with
> >> the default options for the testsuite?
> > Hi Bruno,
> >
> > Thanks for checking.  The bug condition is triggered when the target is all-stop.
> > Could you please try with --target_board=native-extended-gdbserver ?
> > Then you should see
> >
> >    FAIL: gdb.multi/detach-stepi.exp: detach inferior 1
> >
> >    === gdb Summary ===
> >
> >    # of expected passes            8
> >    # of unexpected failures        1
> >
> Ah got it! I have tested and it fixes the problem :)
> 
> Tested-By: Bruno Larsen <blarsen@redhat.com>

Thank you very much for testing!  I added the label to the commit message.

-Baris


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

RE: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Aktemur, Tankut Baris]

Kindly pinging.

(The patch was Tested-By: Guinevere Larsen <blarsen@redhat.com> )

Regards
-Baris

On Tuesday, June 20, 2023 5:25 PM, Aktemur, Tankut Baris wrote:
> ======
> Changes in V2:
> 
> * Changed the thread state check to `(tp->state != THREAD_RUNNING)`.
> * Rebased on the current master.
> 
> The latest discussion was
> https://sourceware.org/pipermail/gdb-patches/2023-May/199613.html
> 
> ======
> 
> Suppose we have two inferiors on an all-stop target with schedule-multi
> set on:
> 
>   $ gdb -q
>   (gdb) target extended-remote | gdbserver --multi -
>   Remote debugging using | gdbserver --multi -
>   Remote debugging using stdio
>   (gdb) file /temp/test
>   Reading symbols from /temp/test...
>   (gdb) set remote exec-file /temp/test
>   (gdb) start
>   Temporary breakpoint 1 at 0x115c: file test.c, line 8.
>   Starting program: /temp/test
>   stdin/stdout redirected
>   Process /temp/test created; pid = 864027
>   ...
> 
>   Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd218) at test.c:8
>   8         foo();
>   (gdb) add-inferior
>   [New inferior 2]
>   Added inferior 2 on connection 1 (extended-remote | gdbserver --multi -)
>   (gdb) inferior 2
>   [Switching to inferior 2 [<null>] (<noexec>)]
>   (gdb) file /temp/test
>   Reading symbols from /temp/test...
>   (gdb) set remote exec-file /temp/test
>   (gdb) tbreak 2
>   Temporary breakpoint 2 at 0x555555555131: /temp/test.c:2. (2 locations)
>   (gdb) run
>   Starting program: /temp/test
>   stdin/stdout redirected
>   Process /temp/test created; pid = 864430
>   ...
> 
>   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
>   2         int a = 42;
>   (gdb) set schedule-multi on
>   (gdb)
> 
> At this point, detaching the first inferior works fine:
> 
>   (gdb) detach inferiors 1
>   Detaching from program: /temp/test, process 858904
>   Detaching from process 858904
>   [Inferior 1 (process 858904) detached]
>   (gdb) info inferiors
>     Num  Description       Connection                                Executable
>     1    <null>            1 (extended-remote | gdbserver --multi -) /temp/test
>   * 2    process 858925    1 (extended-remote | gdbserver --multi -) /temp/test
>   (gdb)
> 
> Let us now repeat exactly the same scenario, but before detaching, we
> make the current thread single-step an instruction:
> 
>   ...
>   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
>   2         int a = 42;
>   (gdb) stepi
>   3         int b = 43;
>   (gdb) detach inferiors 1
>   Detaching from program: /temp/test, process 876580
>   Detaching from process 876580
>   gdbserver: Couldn't reap LWP 876580 while detaching: No child processes
>   [Inferior 1 (process 876580) detached]
>   (gdb) 3   int b = 43;
> 
> There is a mysterious line info output.  Running the scenario with
> infrun debug logs reveals more information.
> 
>   ...
>   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
>   2         int a = 42;
>   (gdb) stepi
>   3         int b = 43;
>   (gdb) set debug infrun on
>   (gdb) detach inferiors 1
>   [infrun] scoped_disable_commit_resumed: reason=detaching
>   Detaching from program: /temp/test, process 872445
>   Detaching from process 872445
>   gdbserver: Couldn't reap LWP 872445 while detaching: No child processes
>   [Inferior 1 (process 872445) detached]
>   [infrun] start_step_over: enter
>     [infrun] start_step_over: stealing global queue of threads to step, length = 0
>     [infrun] operator(): step-over queue now empty
>   [infrun] start_step_over: exit
>   [infrun] restart_stepped_thread: switching back to stepped thread (stepping)
>   [infrun] keep_going_stepped_thread: resuming previously stepped thread
>   [infrun] keep_going_stepped_thread: expected thread advanced also (0x555555555131 -
> > 0x555555555138)
>   [infrun] clear_step_over_info: clearing step over info
>   [infrun] do_target_resume: resume_ptid=-1.0.0, step=0, sig=GDB_SIGNAL_0
>   [infrun] infrun_async: enable=1
>   [infrun] reset: reason=detaching
>   [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target
> extended-remote
>   [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target
> extended-remote
>   (gdb) [infrun] fetch_inferior_event: enter
>     [infrun] scoped_disable_commit_resumed: reason=handling event
>     [infrun] do_target_wait: Found 2 inferiors, starting at #0
>     [infrun] random_pending_event_thread: None found.
>     [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
>     [infrun] print_target_wait_results:   872464.872464.0 [Thread 872464.872464],
>     [infrun] print_target_wait_results:   status->kind = STOPPED, sig =
> GDB_SIGNAL_TRAP
>     [infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
>     [infrun] context_switch: Switching context from 0.0.0 to 872464.872464.0
>     [infrun] handle_signal_stop: stop_pc=0x555555555138
>     [infrun] handle_signal_stop: [872464.872464.0] hit its single-step breakpoint
>     [infrun] handle_signal_stop: delayed software breakpoint trap, ignoring
>     [infrun] process_event_stop_test: stepi/nexti
>     [infrun] stop_waiting: stop_waiting
>   3         int b = 43;
>     [infrun] infrun_async: enable=0
>     [infrun] reset: reason=handling event
>     [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for
> target extended-remote, no resumed threads
>   [infrun] fetch_inferior_event: exit
> 
> GDB attempted to do a step-over for the current thread.  This takes us
> to the commit that introduced restarting step-overs:
> 
>   commit 408f66864a1a823591b26420410c982174c239a2
>   Author: Pedro Alves <pedro@palves.net>
>   Date:   Mon Jan 11 20:01:58 2021 +0000
> 
>     detach in all-stop with threads running
> 
>     A following patch will add a testcase that has a number of threads
>     constantly stepping over a breakpoint, and then has GDB detach the
>     process, while threads are running.  If we have more than one inferior
>     running, and we detach from just one of the inferiors, we expect that
>     the remaining inferior continues running.  However, in all-stop, if
>     GDB needs to pause the target for the detach, nothing is re-resuming
>     the other inferiors after the detach.  "info threads" shows the
>     threads as running, but they really aren't.  This fixes it.
> 
> However, the thread that was resumed for step-over in our scenario did
> not have an interrupted step-over; it had completed its stepi already.
> More debugging reveals that the thread is resumed because of the
> following two conditions in `restart_stepped_thread`:
> 
>       if (tp->control.trap_expected)
>         {
>           infrun_debug_printf ("switching back to stepped thread (step-over)");
> 
>           if (keep_going_stepped_thread (tp))
>             return true;
>         }
> 
> and
> 
>       if (tp->control.step_range_end)
>         {
>           infrun_debug_printf ("switching back to stepped thread (stepping)");
> 
>           if (keep_going_stepped_thread (tp))
>             return true;
>         }
> 
> The root cause of the problem is, `restart_stepped_thread` checks for
> the thread state as
> 
>     if (tp->state == THREAD_EXITED)
>       continue;
> 
> but the thread's state is THREAD_STOPPED.  To fix, we change the state
> check to
> 
>     if (tp->state != THREAD_RUNNING)
> 
> Additionally, the 'trap_expected' and the 'step_range_end' fields of
> the thread's control remain set even after the "stepi" command
> completes, creating a half-baked internal state that can be misleading
> when debugging.  We address this problem by clearing the control
> fields when stepping completes.  We also add a regression test.
> 
> Regression-tested on X86_64 Linux using the default, native-gdbserver,
> and native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  7 ++-
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 101 insertions(+), 2 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.c
>  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.exp
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 58da1cef29e..f40d924dc7a 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -7776,7 +7776,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> 
>    for (thread_info *tp : all_threads_safe ())
>      {
> -      if (tp->state == THREAD_EXITED)
> +      if (tp->state != THREAD_RUNNING)
>  	continue;
> 
>        if (tp->has_pending_waitstatus ())
> @@ -7800,7 +7800,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> 
>    for (thread_info *tp : all_threads_safe ())
>      {
> -      if (tp->state == THREAD_EXITED)
> +      if (tp->state != THREAD_RUNNING)
>  	continue;
> 
>        if (tp->has_pending_waitstatus ())
> @@ -8523,6 +8523,9 @@ static void
>  end_stepping_range (struct execution_control_state *ecs)
>  {
>    ecs->event_thread->control.stop_step = 1;
> +  ecs->event_thread->control.trap_expected = 0;
> +  ecs->event_thread->control.step_range_start = 0;
> +  ecs->event_thread->control.step_range_end = 0;
>    stop_waiting (ecs);
>  }
> 
> diff --git a/gdb/testsuite/gdb.multi/detach-stepi.c b/gdb/testsuite/gdb.multi/detach-
> stepi.c
> new file mode 100644
> index 00000000000..d365645fb3f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/detach-stepi.c
> @@ -0,0 +1,30 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 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/>.  */
> +
> +void
> +a_function ()
> +{
> +  int a = 42;
> +}
> +
> +int
> +main ()
> +{
> +  int b = 43;
> +  a_function ();
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp
> b/gdb/testsuite/gdb.multi/detach-stepi.exp
> new file mode 100644
> index 00000000000..28ef8c4f9f7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
> @@ -0,0 +1,66 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2022 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/>.
> +
> +# Test detaching from an inferior after a thread in another inferior
> +# completes a stepi.  This is a regression test for a bug that was
> +# causing an inadvertent resume of the just-stepped thread.
> +
> +standard_testfile
> +
> +if {[use_gdb_stub]} {
> +    untested "using gdb stub"
> +    return 0
> +}
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> +    return -1
> +}
> +
> +if {![runto_main]} {
> +    return -1
> +}
> +
> +delete_breakpoints
> +
> +# Setup inferior 2.
> +gdb_test "add-inferior" "Added inferior .*" \
> +    "add empty inferior"
> +gdb_test "inferior 2" "Switching to inferior .*" \
> +    "switch to inferior"
> +
> +gdb_load $binfile
> +runto "a_function"
> +gdb_test "info inferiors"
> +
> +# The bug for which this regression test is written appears in
> +# schedule-multi mode.
> +gdb_test_no_output "set schedule-multiple on"
> +
> +# Single-step the thread in Inferior 2, then detach Inferior 1.
> +gdb_test "info threads" ".*" "threads before stepi"
> +gdb_test "stepi"
> +gdb_test "info threads" ".*" "threads after stepi"
> +
> +gdb_test "set debug infrun on"
> +gdb_test_multiple "detach inferior 1" "" {
> +    -re "resuming previously stepped thread.*$gdb_prompt" {
> +	fail $gdb_test_name
> +    }
> +    -re "$gdb_prompt $" {
> +	pass $gdb_test_name
> +    }
> +}
> --
> 2.25.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

RE: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks,
-Baris


On Tuesday, November 21, 2023 9:01 PM, Aktemur, Tankut Baris wrote:
> Kindly pinging.
> 
> (The patch was Tested-By: Guinevere Larsen <blarsen@redhat.com> )
> 
> Regards
> -Baris
> 
> On Tuesday, June 20, 2023 5:25 PM, Aktemur, Tankut Baris wrote:
> > ======
> > Changes in V2:
> >
> > * Changed the thread state check to `(tp->state != THREAD_RUNNING)`.
> > * Rebased on the current master.
> >
> > The latest discussion was
> > https://sourceware.org/pipermail/gdb-patches/2023-May/199613.html
> >
> > ======
> >
> > Suppose we have two inferiors on an all-stop target with schedule-multi
> > set on:
> >
> >   $ gdb -q
> >   (gdb) target extended-remote | gdbserver --multi -
> >   Remote debugging using | gdbserver --multi -
> >   Remote debugging using stdio
> >   (gdb) file /temp/test
> >   Reading symbols from /temp/test...
> >   (gdb) set remote exec-file /temp/test
> >   (gdb) start
> >   Temporary breakpoint 1 at 0x115c: file test.c, line 8.
> >   Starting program: /temp/test
> >   stdin/stdout redirected
> >   Process /temp/test created; pid = 864027
> >   ...
> >
> >   Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd218) at test.c:8
> >   8         foo();
> >   (gdb) add-inferior
> >   [New inferior 2]
> >   Added inferior 2 on connection 1 (extended-remote | gdbserver --multi -)
> >   (gdb) inferior 2
> >   [Switching to inferior 2 [<null>] (<noexec>)]
> >   (gdb) file /temp/test
> >   Reading symbols from /temp/test...
> >   (gdb) set remote exec-file /temp/test
> >   (gdb) tbreak 2
> >   Temporary breakpoint 2 at 0x555555555131: /temp/test.c:2. (2 locations)
> >   (gdb) run
> >   Starting program: /temp/test
> >   stdin/stdout redirected
> >   Process /temp/test created; pid = 864430
> >   ...
> >
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) set schedule-multi on
> >   (gdb)
> >
> > At this point, detaching the first inferior works fine:
> >
> >   (gdb) detach inferiors 1
> >   Detaching from program: /temp/test, process 858904
> >   Detaching from process 858904
> >   [Inferior 1 (process 858904) detached]
> >   (gdb) info inferiors
> >     Num  Description       Connection                                Executable
> >     1    <null>            1 (extended-remote | gdbserver --multi -) /temp/test
> >   * 2    process 858925    1 (extended-remote | gdbserver --multi -) /temp/test
> >   (gdb)
> >
> > Let us now repeat exactly the same scenario, but before detaching, we
> > make the current thread single-step an instruction:
> >
> >   ...
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) stepi
> >   3         int b = 43;
> >   (gdb) detach inferiors 1
> >   Detaching from program: /temp/test, process 876580
> >   Detaching from process 876580
> >   gdbserver: Couldn't reap LWP 876580 while detaching: No child processes
> >   [Inferior 1 (process 876580) detached]
> >   (gdb) 3   int b = 43;
> >
> > There is a mysterious line info output.  Running the scenario with
> > infrun debug logs reveals more information.
> >
> >   ...
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) stepi
> >   3         int b = 43;
> >   (gdb) set debug infrun on
> >   (gdb) detach inferiors 1
> >   [infrun] scoped_disable_commit_resumed: reason=detaching
> >   Detaching from program: /temp/test, process 872445
> >   Detaching from process 872445
> >   gdbserver: Couldn't reap LWP 872445 while detaching: No child processes
> >   [Inferior 1 (process 872445) detached]
> >   [infrun] start_step_over: enter
> >     [infrun] start_step_over: stealing global queue of threads to step, length = 0
> >     [infrun] operator(): step-over queue now empty
> >   [infrun] start_step_over: exit
> >   [infrun] restart_stepped_thread: switching back to stepped thread (stepping)
> >   [infrun] keep_going_stepped_thread: resuming previously stepped thread
> >   [infrun] keep_going_stepped_thread: expected thread advanced also (0x555555555131 -
> > > 0x555555555138)
> >   [infrun] clear_step_over_info: clearing step over info
> >   [infrun] do_target_resume: resume_ptid=-1.0.0, step=0, sig=GDB_SIGNAL_0
> >   [infrun] infrun_async: enable=1
> >   [infrun] reset: reason=detaching
> >   [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target
> > extended-remote
> >   [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target
> > extended-remote
> >   (gdb) [infrun] fetch_inferior_event: enter
> >     [infrun] scoped_disable_commit_resumed: reason=handling event
> >     [infrun] do_target_wait: Found 2 inferiors, starting at #0
> >     [infrun] random_pending_event_thread: None found.
> >     [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
> >     [infrun] print_target_wait_results:   872464.872464.0 [Thread 872464.872464],
> >     [infrun] print_target_wait_results:   status->kind = STOPPED, sig =
> > GDB_SIGNAL_TRAP
> >     [infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
> >     [infrun] context_switch: Switching context from 0.0.0 to 872464.872464.0
> >     [infrun] handle_signal_stop: stop_pc=0x555555555138
> >     [infrun] handle_signal_stop: [872464.872464.0] hit its single-step breakpoint
> >     [infrun] handle_signal_stop: delayed software breakpoint trap, ignoring
> >     [infrun] process_event_stop_test: stepi/nexti
> >     [infrun] stop_waiting: stop_waiting
> >   3         int b = 43;
> >     [infrun] infrun_async: enable=0
> >     [infrun] reset: reason=handling event
> >     [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for
> > target extended-remote, no resumed threads
> >   [infrun] fetch_inferior_event: exit
> >
> > GDB attempted to do a step-over for the current thread.  This takes us
> > to the commit that introduced restarting step-overs:
> >
> >   commit 408f66864a1a823591b26420410c982174c239a2
> >   Author: Pedro Alves <pedro@palves.net>
> >   Date:   Mon Jan 11 20:01:58 2021 +0000
> >
> >     detach in all-stop with threads running
> >
> >     A following patch will add a testcase that has a number of threads
> >     constantly stepping over a breakpoint, and then has GDB detach the
> >     process, while threads are running.  If we have more than one inferior
> >     running, and we detach from just one of the inferiors, we expect that
> >     the remaining inferior continues running.  However, in all-stop, if
> >     GDB needs to pause the target for the detach, nothing is re-resuming
> >     the other inferiors after the detach.  "info threads" shows the
> >     threads as running, but they really aren't.  This fixes it.
> >
> > However, the thread that was resumed for step-over in our scenario did
> > not have an interrupted step-over; it had completed its stepi already.
> > More debugging reveals that the thread is resumed because of the
> > following two conditions in `restart_stepped_thread`:
> >
> >       if (tp->control.trap_expected)
> >         {
> >           infrun_debug_printf ("switching back to stepped thread (step-over)");
> >
> >           if (keep_going_stepped_thread (tp))
> >             return true;
> >         }
> >
> > and
> >
> >       if (tp->control.step_range_end)
> >         {
> >           infrun_debug_printf ("switching back to stepped thread (stepping)");
> >
> >           if (keep_going_stepped_thread (tp))
> >             return true;
> >         }
> >
> > The root cause of the problem is, `restart_stepped_thread` checks for
> > the thread state as
> >
> >     if (tp->state == THREAD_EXITED)
> >       continue;
> >
> > but the thread's state is THREAD_STOPPED.  To fix, we change the state
> > check to
> >
> >     if (tp->state != THREAD_RUNNING)
> >
> > Additionally, the 'trap_expected' and the 'step_range_end' fields of
> > the thread's control remain set even after the "stepi" command
> > completes, creating a half-baked internal state that can be misleading
> > when debugging.  We address this problem by clearing the control
> > fields when stepping completes.  We also add a regression test.
> >
> > Regression-tested on X86_64 Linux using the default, native-gdbserver,
> > and native-extended-gdbserver board files.
> > ---
> >  gdb/infrun.c                             |  7 ++-
> >  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
> >  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
> >  3 files changed, 101 insertions(+), 2 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.c
> >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.exp
> >
> > diff --git a/gdb/infrun.c b/gdb/infrun.c
> > index 58da1cef29e..f40d924dc7a 100644
> > --- a/gdb/infrun.c
> > +++ b/gdb/infrun.c
> > @@ -7776,7 +7776,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> >
> >    for (thread_info *tp : all_threads_safe ())
> >      {
> > -      if (tp->state == THREAD_EXITED)
> > +      if (tp->state != THREAD_RUNNING)
> >  	continue;
> >
> >        if (tp->has_pending_waitstatus ())
> > @@ -7800,7 +7800,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> >
> >    for (thread_info *tp : all_threads_safe ())
> >      {
> > -      if (tp->state == THREAD_EXITED)
> > +      if (tp->state != THREAD_RUNNING)
> >  	continue;
> >
> >        if (tp->has_pending_waitstatus ())
> > @@ -8523,6 +8523,9 @@ static void
> >  end_stepping_range (struct execution_control_state *ecs)
> >  {
> >    ecs->event_thread->control.stop_step = 1;
> > +  ecs->event_thread->control.trap_expected = 0;
> > +  ecs->event_thread->control.step_range_start = 0;
> > +  ecs->event_thread->control.step_range_end = 0;
> >    stop_waiting (ecs);
> >  }
> >
> > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.c b/gdb/testsuite/gdb.multi/detach-
> > stepi.c
> > new file mode 100644
> > index 00000000000..d365645fb3f
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/detach-stepi.c
> > @@ -0,0 +1,30 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2022 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/>.  */
> > +
> > +void
> > +a_function ()
> > +{
> > +  int a = 42;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +  int b = 43;
> > +  a_function ();
> > +  return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp
> > b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > new file mode 100644
> > index 00000000000..28ef8c4f9f7
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > @@ -0,0 +1,66 @@
> > +# This testcase is part of GDB, the GNU debugger.
> > +
> > +# Copyright 2022 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/>.
> > +
> > +# Test detaching from an inferior after a thread in another inferior
> > +# completes a stepi.  This is a regression test for a bug that was
> > +# causing an inadvertent resume of the just-stepped thread.
> > +
> > +standard_testfile
> > +
> > +if {[use_gdb_stub]} {
> > +    untested "using gdb stub"
> > +    return 0
> > +}
> > +
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> > +    return -1
> > +}
> > +
> > +if {![runto_main]} {
> > +    return -1
> > +}
> > +
> > +delete_breakpoints
> > +
> > +# Setup inferior 2.
> > +gdb_test "add-inferior" "Added inferior .*" \
> > +    "add empty inferior"
> > +gdb_test "inferior 2" "Switching to inferior .*" \
> > +    "switch to inferior"
> > +
> > +gdb_load $binfile
> > +runto "a_function"
> > +gdb_test "info inferiors"
> > +
> > +# The bug for which this regression test is written appears in
> > +# schedule-multi mode.
> > +gdb_test_no_output "set schedule-multiple on"
> > +
> > +# Single-step the thread in Inferior 2, then detach Inferior 1.
> > +gdb_test "info threads" ".*" "threads before stepi"
> > +gdb_test "stepi"
> > +gdb_test "info threads" ".*" "threads after stepi"
> > +
> > +gdb_test "set debug infrun on"
> > +gdb_test_multiple "detach inferior 1" "" {
> > +    -re "resuming previously stepped thread.*$gdb_prompt" {
> > +	fail $gdb_test_name
> > +    }
> > +    -re "$gdb_prompt $" {
> > +	pass $gdb_test_name
> > +    }
> > +}
> > --
> > 2.25.1


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks,
-Baris


On Tuesday, November 21, 2023 9:01 PM, Aktemur, Tankut Baris wrote:
> Kindly pinging.
> 
> (The patch was Tested-By: Guinevere Larsen <blarsen@redhat.com> )
> 
> Regards
> -Baris
> 
> On Tuesday, June 20, 2023 5:25 PM, Aktemur, Tankut Baris wrote:
> > ======
> > Changes in V2:
> >
> > * Changed the thread state check to `(tp->state != THREAD_RUNNING)`.
> > * Rebased on the current master.
> >
> > The latest discussion was
> > https://sourceware.org/pipermail/gdb-patches/2023-May/199613.html
> >
> > ======
> >
> > Suppose we have two inferiors on an all-stop target with schedule-multi
> > set on:
> >
> >   $ gdb -q
> >   (gdb) target extended-remote | gdbserver --multi -
> >   Remote debugging using | gdbserver --multi -
> >   Remote debugging using stdio
> >   (gdb) file /temp/test
> >   Reading symbols from /temp/test...
> >   (gdb) set remote exec-file /temp/test
> >   (gdb) start
> >   Temporary breakpoint 1 at 0x115c: file test.c, line 8.
> >   Starting program: /temp/test
> >   stdin/stdout redirected
> >   Process /temp/test created; pid = 864027
> >   ...
> >
> >   Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd218) at test.c:8
> >   8         foo();
> >   (gdb) add-inferior
> >   [New inferior 2]
> >   Added inferior 2 on connection 1 (extended-remote | gdbserver --multi -)
> >   (gdb) inferior 2
> >   [Switching to inferior 2 [<null>] (<noexec>)]
> >   (gdb) file /temp/test
> >   Reading symbols from /temp/test...
> >   (gdb) set remote exec-file /temp/test
> >   (gdb) tbreak 2
> >   Temporary breakpoint 2 at 0x555555555131: /temp/test.c:2. (2 locations)
> >   (gdb) run
> >   Starting program: /temp/test
> >   stdin/stdout redirected
> >   Process /temp/test created; pid = 864430
> >   ...
> >
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) set schedule-multi on
> >   (gdb)
> >
> > At this point, detaching the first inferior works fine:
> >
> >   (gdb) detach inferiors 1
> >   Detaching from program: /temp/test, process 858904
> >   Detaching from process 858904
> >   [Inferior 1 (process 858904) detached]
> >   (gdb) info inferiors
> >     Num  Description       Connection                                Executable
> >     1    <null>            1 (extended-remote | gdbserver --multi -) /temp/test
> >   * 2    process 858925    1 (extended-remote | gdbserver --multi -) /temp/test
> >   (gdb)
> >
> > Let us now repeat exactly the same scenario, but before detaching, we
> > make the current thread single-step an instruction:
> >
> >   ...
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) stepi
> >   3         int b = 43;
> >   (gdb) detach inferiors 1
> >   Detaching from program: /temp/test, process 876580
> >   Detaching from process 876580
> >   gdbserver: Couldn't reap LWP 876580 while detaching: No child processes
> >   [Inferior 1 (process 876580) detached]
> >   (gdb) 3   int b = 43;
> >
> > There is a mysterious line info output.  Running the scenario with
> > infrun debug logs reveals more information.
> >
> >   ...
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) stepi
> >   3         int b = 43;
> >   (gdb) set debug infrun on
> >   (gdb) detach inferiors 1
> >   [infrun] scoped_disable_commit_resumed: reason=detaching
> >   Detaching from program: /temp/test, process 872445
> >   Detaching from process 872445
> >   gdbserver: Couldn't reap LWP 872445 while detaching: No child processes
> >   [Inferior 1 (process 872445) detached]
> >   [infrun] start_step_over: enter
> >     [infrun] start_step_over: stealing global queue of threads to step, length = 0
> >     [infrun] operator(): step-over queue now empty
> >   [infrun] start_step_over: exit
> >   [infrun] restart_stepped_thread: switching back to stepped thread (stepping)
> >   [infrun] keep_going_stepped_thread: resuming previously stepped thread
> >   [infrun] keep_going_stepped_thread: expected thread advanced also (0x555555555131 -
> > > 0x555555555138)
> >   [infrun] clear_step_over_info: clearing step over info
> >   [infrun] do_target_resume: resume_ptid=-1.0.0, step=0, sig=GDB_SIGNAL_0
> >   [infrun] infrun_async: enable=1
> >   [infrun] reset: reason=detaching
> >   [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target
> > extended-remote
> >   [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target
> > extended-remote
> >   (gdb) [infrun] fetch_inferior_event: enter
> >     [infrun] scoped_disable_commit_resumed: reason=handling event
> >     [infrun] do_target_wait: Found 2 inferiors, starting at #0
> >     [infrun] random_pending_event_thread: None found.
> >     [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
> >     [infrun] print_target_wait_results:   872464.872464.0 [Thread 872464.872464],
> >     [infrun] print_target_wait_results:   status->kind = STOPPED, sig =
> > GDB_SIGNAL_TRAP
> >     [infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
> >     [infrun] context_switch: Switching context from 0.0.0 to 872464.872464.0
> >     [infrun] handle_signal_stop: stop_pc=0x555555555138
> >     [infrun] handle_signal_stop: [872464.872464.0] hit its single-step breakpoint
> >     [infrun] handle_signal_stop: delayed software breakpoint trap, ignoring
> >     [infrun] process_event_stop_test: stepi/nexti
> >     [infrun] stop_waiting: stop_waiting
> >   3         int b = 43;
> >     [infrun] infrun_async: enable=0
> >     [infrun] reset: reason=handling event
> >     [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for
> > target extended-remote, no resumed threads
> >   [infrun] fetch_inferior_event: exit
> >
> > GDB attempted to do a step-over for the current thread.  This takes us
> > to the commit that introduced restarting step-overs:
> >
> >   commit 408f66864a1a823591b26420410c982174c239a2
> >   Author: Pedro Alves <pedro@palves.net>
> >   Date:   Mon Jan 11 20:01:58 2021 +0000
> >
> >     detach in all-stop with threads running
> >
> >     A following patch will add a testcase that has a number of threads
> >     constantly stepping over a breakpoint, and then has GDB detach the
> >     process, while threads are running.  If we have more than one inferior
> >     running, and we detach from just one of the inferiors, we expect that
> >     the remaining inferior continues running.  However, in all-stop, if
> >     GDB needs to pause the target for the detach, nothing is re-resuming
> >     the other inferiors after the detach.  "info threads" shows the
> >     threads as running, but they really aren't.  This fixes it.
> >
> > However, the thread that was resumed for step-over in our scenario did
> > not have an interrupted step-over; it had completed its stepi already.
> > More debugging reveals that the thread is resumed because of the
> > following two conditions in `restart_stepped_thread`:
> >
> >       if (tp->control.trap_expected)
> >         {
> >           infrun_debug_printf ("switching back to stepped thread (step-over)");
> >
> >           if (keep_going_stepped_thread (tp))
> >             return true;
> >         }
> >
> > and
> >
> >       if (tp->control.step_range_end)
> >         {
> >           infrun_debug_printf ("switching back to stepped thread (stepping)");
> >
> >           if (keep_going_stepped_thread (tp))
> >             return true;
> >         }
> >
> > The root cause of the problem is, `restart_stepped_thread` checks for
> > the thread state as
> >
> >     if (tp->state == THREAD_EXITED)
> >       continue;
> >
> > but the thread's state is THREAD_STOPPED.  To fix, we change the state
> > check to
> >
> >     if (tp->state != THREAD_RUNNING)
> >
> > Additionally, the 'trap_expected' and the 'step_range_end' fields of
> > the thread's control remain set even after the "stepi" command
> > completes, creating a half-baked internal state that can be misleading
> > when debugging.  We address this problem by clearing the control
> > fields when stepping completes.  We also add a regression test.
> >
> > Regression-tested on X86_64 Linux using the default, native-gdbserver,
> > and native-extended-gdbserver board files.
> > ---
> >  gdb/infrun.c                             |  7 ++-
> >  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
> >  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
> >  3 files changed, 101 insertions(+), 2 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.c
> >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.exp
> >
> > diff --git a/gdb/infrun.c b/gdb/infrun.c
> > index 58da1cef29e..f40d924dc7a 100644
> > --- a/gdb/infrun.c
> > +++ b/gdb/infrun.c
> > @@ -7776,7 +7776,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> >
> >    for (thread_info *tp : all_threads_safe ())
> >      {
> > -      if (tp->state == THREAD_EXITED)
> > +      if (tp->state != THREAD_RUNNING)
> >  	continue;
> >
> >        if (tp->has_pending_waitstatus ())
> > @@ -7800,7 +7800,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> >
> >    for (thread_info *tp : all_threads_safe ())
> >      {
> > -      if (tp->state == THREAD_EXITED)
> > +      if (tp->state != THREAD_RUNNING)
> >  	continue;
> >
> >        if (tp->has_pending_waitstatus ())
> > @@ -8523,6 +8523,9 @@ static void
> >  end_stepping_range (struct execution_control_state *ecs)
> >  {
> >    ecs->event_thread->control.stop_step = 1;
> > +  ecs->event_thread->control.trap_expected = 0;
> > +  ecs->event_thread->control.step_range_start = 0;
> > +  ecs->event_thread->control.step_range_end = 0;
> >    stop_waiting (ecs);
> >  }
> >
> > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.c b/gdb/testsuite/gdb.multi/detach-
> > stepi.c
> > new file mode 100644
> > index 00000000000..d365645fb3f
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/detach-stepi.c
> > @@ -0,0 +1,30 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2022 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/>.  */
> > +
> > +void
> > +a_function ()
> > +{
> > +  int a = 42;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +  int b = 43;
> > +  a_function ();
> > +  return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp
> > b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > new file mode 100644
> > index 00000000000..28ef8c4f9f7
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > @@ -0,0 +1,66 @@
> > +# This testcase is part of GDB, the GNU debugger.
> > +
> > +# Copyright 2022 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/>.
> > +
> > +# Test detaching from an inferior after a thread in another inferior
> > +# completes a stepi.  This is a regression test for a bug that was
> > +# causing an inadvertent resume of the just-stepped thread.
> > +
> > +standard_testfile
> > +
> > +if {[use_gdb_stub]} {
> > +    untested "using gdb stub"
> > +    return 0
> > +}
> > +
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> > +    return -1
> > +}
> > +
> > +if {![runto_main]} {
> > +    return -1
> > +}
> > +
> > +delete_breakpoints
> > +
> > +# Setup inferior 2.
> > +gdb_test "add-inferior" "Added inferior .*" \
> > +    "add empty inferior"
> > +gdb_test "inferior 2" "Switching to inferior .*" \
> > +    "switch to inferior"
> > +
> > +gdb_load $binfile
> > +runto "a_function"
> > +gdb_test "info inferiors"
> > +
> > +# The bug for which this regression test is written appears in
> > +# schedule-multi mode.
> > +gdb_test_no_output "set schedule-multiple on"
> > +
> > +# Single-step the thread in Inferior 2, then detach Inferior 1.
> > +gdb_test "info threads" ".*" "threads before stepi"
> > +gdb_test "stepi"
> > +gdb_test "info threads" ".*" "threads after stepi"
> > +
> > +gdb_test "set debug infrun on"
> > +gdb_test_multiple "detach inferior 1" "" {
> > +    -re "resuming previously stepped thread.*$gdb_prompt" {
> > +	fail $gdb_test_name
> > +    }
> > +    -re "$gdb_prompt $" {
> > +	pass $gdb_test_name
> > +    }
> > +}
> > --
> > 2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks,
-Baris


On Tuesday, November 21, 2023 9:01 PM, Aktemur, Tankut Baris wrote:
> Kindly pinging.
> 
> (The patch was Tested-By: Guinevere Larsen <blarsen@redhat.com> )
> 
> Regards
> -Baris
> 
> On Tuesday, June 20, 2023 5:25 PM, Aktemur, Tankut Baris wrote:
> > ======
> > Changes in V2:
> >
> > * Changed the thread state check to `(tp->state != THREAD_RUNNING)`.
> > * Rebased on the current master.
> >
> > The latest discussion was
> > https://sourceware.org/pipermail/gdb-patches/2023-May/199613.html
> >
> > ======
> >
> > Suppose we have two inferiors on an all-stop target with schedule-multi
> > set on:
> >
> >   $ gdb -q
> >   (gdb) target extended-remote | gdbserver --multi -
> >   Remote debugging using | gdbserver --multi -
> >   Remote debugging using stdio
> >   (gdb) file /temp/test
> >   Reading symbols from /temp/test...
> >   (gdb) set remote exec-file /temp/test
> >   (gdb) start
> >   Temporary breakpoint 1 at 0x115c: file test.c, line 8.
> >   Starting program: /temp/test
> >   stdin/stdout redirected
> >   Process /temp/test created; pid = 864027
> >   ...
> >
> >   Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd218) at test.c:8
> >   8         foo();
> >   (gdb) add-inferior
> >   [New inferior 2]
> >   Added inferior 2 on connection 1 (extended-remote | gdbserver --multi -)
> >   (gdb) inferior 2
> >   [Switching to inferior 2 [<null>] (<noexec>)]
> >   (gdb) file /temp/test
> >   Reading symbols from /temp/test...
> >   (gdb) set remote exec-file /temp/test
> >   (gdb) tbreak 2
> >   Temporary breakpoint 2 at 0x555555555131: /temp/test.c:2. (2 locations)
> >   (gdb) run
> >   Starting program: /temp/test
> >   stdin/stdout redirected
> >   Process /temp/test created; pid = 864430
> >   ...
> >
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) set schedule-multi on
> >   (gdb)
> >
> > At this point, detaching the first inferior works fine:
> >
> >   (gdb) detach inferiors 1
> >   Detaching from program: /temp/test, process 858904
> >   Detaching from process 858904
> >   [Inferior 1 (process 858904) detached]
> >   (gdb) info inferiors
> >     Num  Description       Connection                                Executable
> >     1    <null>            1 (extended-remote | gdbserver --multi -) /temp/test
> >   * 2    process 858925    1 (extended-remote | gdbserver --multi -) /temp/test
> >   (gdb)
> >
> > Let us now repeat exactly the same scenario, but before detaching, we
> > make the current thread single-step an instruction:
> >
> >   ...
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) stepi
> >   3         int b = 43;
> >   (gdb) detach inferiors 1
> >   Detaching from program: /temp/test, process 876580
> >   Detaching from process 876580
> >   gdbserver: Couldn't reap LWP 876580 while detaching: No child processes
> >   [Inferior 1 (process 876580) detached]
> >   (gdb) 3   int b = 43;
> >
> > There is a mysterious line info output.  Running the scenario with
> > infrun debug logs reveals more information.
> >
> >   ...
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) stepi
> >   3         int b = 43;
> >   (gdb) set debug infrun on
> >   (gdb) detach inferiors 1
> >   [infrun] scoped_disable_commit_resumed: reason=detaching
> >   Detaching from program: /temp/test, process 872445
> >   Detaching from process 872445
> >   gdbserver: Couldn't reap LWP 872445 while detaching: No child processes
> >   [Inferior 1 (process 872445) detached]
> >   [infrun] start_step_over: enter
> >     [infrun] start_step_over: stealing global queue of threads to step, length = 0
> >     [infrun] operator(): step-over queue now empty
> >   [infrun] start_step_over: exit
> >   [infrun] restart_stepped_thread: switching back to stepped thread (stepping)
> >   [infrun] keep_going_stepped_thread: resuming previously stepped thread
> >   [infrun] keep_going_stepped_thread: expected thread advanced also (0x555555555131 -
> > > 0x555555555138)
> >   [infrun] clear_step_over_info: clearing step over info
> >   [infrun] do_target_resume: resume_ptid=-1.0.0, step=0, sig=GDB_SIGNAL_0
> >   [infrun] infrun_async: enable=1
> >   [infrun] reset: reason=detaching
> >   [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target
> > extended-remote
> >   [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target
> > extended-remote
> >   (gdb) [infrun] fetch_inferior_event: enter
> >     [infrun] scoped_disable_commit_resumed: reason=handling event
> >     [infrun] do_target_wait: Found 2 inferiors, starting at #0
> >     [infrun] random_pending_event_thread: None found.
> >     [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
> >     [infrun] print_target_wait_results:   872464.872464.0 [Thread 872464.872464],
> >     [infrun] print_target_wait_results:   status->kind = STOPPED, sig =
> > GDB_SIGNAL_TRAP
> >     [infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
> >     [infrun] context_switch: Switching context from 0.0.0 to 872464.872464.0
> >     [infrun] handle_signal_stop: stop_pc=0x555555555138
> >     [infrun] handle_signal_stop: [872464.872464.0] hit its single-step breakpoint
> >     [infrun] handle_signal_stop: delayed software breakpoint trap, ignoring
> >     [infrun] process_event_stop_test: stepi/nexti
> >     [infrun] stop_waiting: stop_waiting
> >   3         int b = 43;
> >     [infrun] infrun_async: enable=0
> >     [infrun] reset: reason=handling event
> >     [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for
> > target extended-remote, no resumed threads
> >   [infrun] fetch_inferior_event: exit
> >
> > GDB attempted to do a step-over for the current thread.  This takes us
> > to the commit that introduced restarting step-overs:
> >
> >   commit 408f66864a1a823591b26420410c982174c239a2
> >   Author: Pedro Alves <pedro@palves.net>
> >   Date:   Mon Jan 11 20:01:58 2021 +0000
> >
> >     detach in all-stop with threads running
> >
> >     A following patch will add a testcase that has a number of threads
> >     constantly stepping over a breakpoint, and then has GDB detach the
> >     process, while threads are running.  If we have more than one inferior
> >     running, and we detach from just one of the inferiors, we expect that
> >     the remaining inferior continues running.  However, in all-stop, if
> >     GDB needs to pause the target for the detach, nothing is re-resuming
> >     the other inferiors after the detach.  "info threads" shows the
> >     threads as running, but they really aren't.  This fixes it.
> >
> > However, the thread that was resumed for step-over in our scenario did
> > not have an interrupted step-over; it had completed its stepi already.
> > More debugging reveals that the thread is resumed because of the
> > following two conditions in `restart_stepped_thread`:
> >
> >       if (tp->control.trap_expected)
> >         {
> >           infrun_debug_printf ("switching back to stepped thread (step-over)");
> >
> >           if (keep_going_stepped_thread (tp))
> >             return true;
> >         }
> >
> > and
> >
> >       if (tp->control.step_range_end)
> >         {
> >           infrun_debug_printf ("switching back to stepped thread (stepping)");
> >
> >           if (keep_going_stepped_thread (tp))
> >             return true;
> >         }
> >
> > The root cause of the problem is, `restart_stepped_thread` checks for
> > the thread state as
> >
> >     if (tp->state == THREAD_EXITED)
> >       continue;
> >
> > but the thread's state is THREAD_STOPPED.  To fix, we change the state
> > check to
> >
> >     if (tp->state != THREAD_RUNNING)
> >
> > Additionally, the 'trap_expected' and the 'step_range_end' fields of
> > the thread's control remain set even after the "stepi" command
> > completes, creating a half-baked internal state that can be misleading
> > when debugging.  We address this problem by clearing the control
> > fields when stepping completes.  We also add a regression test.
> >
> > Regression-tested on X86_64 Linux using the default, native-gdbserver,
> > and native-extended-gdbserver board files.
> > ---
> >  gdb/infrun.c                             |  7 ++-
> >  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
> >  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
> >  3 files changed, 101 insertions(+), 2 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.c
> >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.exp
> >
> > diff --git a/gdb/infrun.c b/gdb/infrun.c
> > index 58da1cef29e..f40d924dc7a 100644
> > --- a/gdb/infrun.c
> > +++ b/gdb/infrun.c
> > @@ -7776,7 +7776,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> >
> >    for (thread_info *tp : all_threads_safe ())
> >      {
> > -      if (tp->state == THREAD_EXITED)
> > +      if (tp->state != THREAD_RUNNING)
> >  	continue;
> >
> >        if (tp->has_pending_waitstatus ())
> > @@ -7800,7 +7800,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> >
> >    for (thread_info *tp : all_threads_safe ())
> >      {
> > -      if (tp->state == THREAD_EXITED)
> > +      if (tp->state != THREAD_RUNNING)
> >  	continue;
> >
> >        if (tp->has_pending_waitstatus ())
> > @@ -8523,6 +8523,9 @@ static void
> >  end_stepping_range (struct execution_control_state *ecs)
> >  {
> >    ecs->event_thread->control.stop_step = 1;
> > +  ecs->event_thread->control.trap_expected = 0;
> > +  ecs->event_thread->control.step_range_start = 0;
> > +  ecs->event_thread->control.step_range_end = 0;
> >    stop_waiting (ecs);
> >  }
> >
> > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.c b/gdb/testsuite/gdb.multi/detach-
> > stepi.c
> > new file mode 100644
> > index 00000000000..d365645fb3f
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/detach-stepi.c
> > @@ -0,0 +1,30 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2022 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/>.  */
> > +
> > +void
> > +a_function ()
> > +{
> > +  int a = 42;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +  int b = 43;
> > +  a_function ();
> > +  return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp
> > b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > new file mode 100644
> > index 00000000000..28ef8c4f9f7
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > @@ -0,0 +1,66 @@
> > +# This testcase is part of GDB, the GNU debugger.
> > +
> > +# Copyright 2022 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/>.
> > +
> > +# Test detaching from an inferior after a thread in another inferior
> > +# completes a stepi.  This is a regression test for a bug that was
> > +# causing an inadvertent resume of the just-stepped thread.
> > +
> > +standard_testfile
> > +
> > +if {[use_gdb_stub]} {
> > +    untested "using gdb stub"
> > +    return 0
> > +}
> > +
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> > +    return -1
> > +}
> > +
> > +if {![runto_main]} {
> > +    return -1
> > +}
> > +
> > +delete_breakpoints
> > +
> > +# Setup inferior 2.
> > +gdb_test "add-inferior" "Added inferior .*" \
> > +    "add empty inferior"
> > +gdb_test "inferior 2" "Switching to inferior .*" \
> > +    "switch to inferior"
> > +
> > +gdb_load $binfile
> > +runto "a_function"
> > +gdb_test "info inferiors"
> > +
> > +# The bug for which this regression test is written appears in
> > +# schedule-multi mode.
> > +gdb_test_no_output "set schedule-multiple on"
> > +
> > +# Single-step the thread in Inferior 2, then detach Inferior 1.
> > +gdb_test "info threads" ".*" "threads before stepi"
> > +gdb_test "stepi"
> > +gdb_test "info threads" ".*" "threads after stepi"
> > +
> > +gdb_test "set debug infrun on"
> > +gdb_test_multiple "detach inferior 1" "" {
> > +    -re "resuming previously stepped thread.*$gdb_prompt" {
> > +	fail $gdb_test_name
> > +    }
> > +    -re "$gdb_prompt $" {
> > +	pass $gdb_test_name
> > +    }
> > +}
> > --
> > 2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks,
-Baris


On Tuesday, November 21, 2023 9:01 PM, Aktemur, Tankut Baris wrote:
> Kindly pinging.
> 
> (The patch was Tested-By: Guinevere Larsen <blarsen@redhat.com> )
> 
> Regards
> -Baris
> 
> On Tuesday, June 20, 2023 5:25 PM, Aktemur, Tankut Baris wrote:
> > ======
> > Changes in V2:
> >
> > * Changed the thread state check to `(tp->state != THREAD_RUNNING)`.
> > * Rebased on the current master.
> >
> > The latest discussion was
> > https://sourceware.org/pipermail/gdb-patches/2023-May/199613.html
> >
> > ======
> >
> > Suppose we have two inferiors on an all-stop target with schedule-multi
> > set on:
> >
> >   $ gdb -q
> >   (gdb) target extended-remote | gdbserver --multi -
> >   Remote debugging using | gdbserver --multi -
> >   Remote debugging using stdio
> >   (gdb) file /temp/test
> >   Reading symbols from /temp/test...
> >   (gdb) set remote exec-file /temp/test
> >   (gdb) start
> >   Temporary breakpoint 1 at 0x115c: file test.c, line 8.
> >   Starting program: /temp/test
> >   stdin/stdout redirected
> >   Process /temp/test created; pid = 864027
> >   ...
> >
> >   Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd218) at test.c:8
> >   8         foo();
> >   (gdb) add-inferior
> >   [New inferior 2]
> >   Added inferior 2 on connection 1 (extended-remote | gdbserver --multi -)
> >   (gdb) inferior 2
> >   [Switching to inferior 2 [<null>] (<noexec>)]
> >   (gdb) file /temp/test
> >   Reading symbols from /temp/test...
> >   (gdb) set remote exec-file /temp/test
> >   (gdb) tbreak 2
> >   Temporary breakpoint 2 at 0x555555555131: /temp/test.c:2. (2 locations)
> >   (gdb) run
> >   Starting program: /temp/test
> >   stdin/stdout redirected
> >   Process /temp/test created; pid = 864430
> >   ...
> >
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) set schedule-multi on
> >   (gdb)
> >
> > At this point, detaching the first inferior works fine:
> >
> >   (gdb) detach inferiors 1
> >   Detaching from program: /temp/test, process 858904
> >   Detaching from process 858904
> >   [Inferior 1 (process 858904) detached]
> >   (gdb) info inferiors
> >     Num  Description       Connection                                Executable
> >     1    <null>            1 (extended-remote | gdbserver --multi -) /temp/test
> >   * 2    process 858925    1 (extended-remote | gdbserver --multi -) /temp/test
> >   (gdb)
> >
> > Let us now repeat exactly the same scenario, but before detaching, we
> > make the current thread single-step an instruction:
> >
> >   ...
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) stepi
> >   3         int b = 43;
> >   (gdb) detach inferiors 1
> >   Detaching from program: /temp/test, process 876580
> >   Detaching from process 876580
> >   gdbserver: Couldn't reap LWP 876580 while detaching: No child processes
> >   [Inferior 1 (process 876580) detached]
> >   (gdb) 3   int b = 43;
> >
> > There is a mysterious line info output.  Running the scenario with
> > infrun debug logs reveals more information.
> >
> >   ...
> >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> >   2         int a = 42;
> >   (gdb) stepi
> >   3         int b = 43;
> >   (gdb) set debug infrun on
> >   (gdb) detach inferiors 1
> >   [infrun] scoped_disable_commit_resumed: reason=detaching
> >   Detaching from program: /temp/test, process 872445
> >   Detaching from process 872445
> >   gdbserver: Couldn't reap LWP 872445 while detaching: No child processes
> >   [Inferior 1 (process 872445) detached]
> >   [infrun] start_step_over: enter
> >     [infrun] start_step_over: stealing global queue of threads to step, length = 0
> >     [infrun] operator(): step-over queue now empty
> >   [infrun] start_step_over: exit
> >   [infrun] restart_stepped_thread: switching back to stepped thread (stepping)
> >   [infrun] keep_going_stepped_thread: resuming previously stepped thread
> >   [infrun] keep_going_stepped_thread: expected thread advanced also (0x555555555131 -
> > > 0x555555555138)
> >   [infrun] clear_step_over_info: clearing step over info
> >   [infrun] do_target_resume: resume_ptid=-1.0.0, step=0, sig=GDB_SIGNAL_0
> >   [infrun] infrun_async: enable=1
> >   [infrun] reset: reason=detaching
> >   [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target
> > extended-remote
> >   [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target
> > extended-remote
> >   (gdb) [infrun] fetch_inferior_event: enter
> >     [infrun] scoped_disable_commit_resumed: reason=handling event
> >     [infrun] do_target_wait: Found 2 inferiors, starting at #0
> >     [infrun] random_pending_event_thread: None found.
> >     [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
> >     [infrun] print_target_wait_results:   872464.872464.0 [Thread 872464.872464],
> >     [infrun] print_target_wait_results:   status->kind = STOPPED, sig =
> > GDB_SIGNAL_TRAP
> >     [infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
> >     [infrun] context_switch: Switching context from 0.0.0 to 872464.872464.0
> >     [infrun] handle_signal_stop: stop_pc=0x555555555138
> >     [infrun] handle_signal_stop: [872464.872464.0] hit its single-step breakpoint
> >     [infrun] handle_signal_stop: delayed software breakpoint trap, ignoring
> >     [infrun] process_event_stop_test: stepi/nexti
> >     [infrun] stop_waiting: stop_waiting
> >   3         int b = 43;
> >     [infrun] infrun_async: enable=0
> >     [infrun] reset: reason=handling event
> >     [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for
> > target extended-remote, no resumed threads
> >   [infrun] fetch_inferior_event: exit
> >
> > GDB attempted to do a step-over for the current thread.  This takes us
> > to the commit that introduced restarting step-overs:
> >
> >   commit 408f66864a1a823591b26420410c982174c239a2
> >   Author: Pedro Alves <pedro@palves.net>
> >   Date:   Mon Jan 11 20:01:58 2021 +0000
> >
> >     detach in all-stop with threads running
> >
> >     A following patch will add a testcase that has a number of threads
> >     constantly stepping over a breakpoint, and then has GDB detach the
> >     process, while threads are running.  If we have more than one inferior
> >     running, and we detach from just one of the inferiors, we expect that
> >     the remaining inferior continues running.  However, in all-stop, if
> >     GDB needs to pause the target for the detach, nothing is re-resuming
> >     the other inferiors after the detach.  "info threads" shows the
> >     threads as running, but they really aren't.  This fixes it.
> >
> > However, the thread that was resumed for step-over in our scenario did
> > not have an interrupted step-over; it had completed its stepi already.
> > More debugging reveals that the thread is resumed because of the
> > following two conditions in `restart_stepped_thread`:
> >
> >       if (tp->control.trap_expected)
> >         {
> >           infrun_debug_printf ("switching back to stepped thread (step-over)");
> >
> >           if (keep_going_stepped_thread (tp))
> >             return true;
> >         }
> >
> > and
> >
> >       if (tp->control.step_range_end)
> >         {
> >           infrun_debug_printf ("switching back to stepped thread (stepping)");
> >
> >           if (keep_going_stepped_thread (tp))
> >             return true;
> >         }
> >
> > The root cause of the problem is, `restart_stepped_thread` checks for
> > the thread state as
> >
> >     if (tp->state == THREAD_EXITED)
> >       continue;
> >
> > but the thread's state is THREAD_STOPPED.  To fix, we change the state
> > check to
> >
> >     if (tp->state != THREAD_RUNNING)
> >
> > Additionally, the 'trap_expected' and the 'step_range_end' fields of
> > the thread's control remain set even after the "stepi" command
> > completes, creating a half-baked internal state that can be misleading
> > when debugging.  We address this problem by clearing the control
> > fields when stepping completes.  We also add a regression test.
> >
> > Regression-tested on X86_64 Linux using the default, native-gdbserver,
> > and native-extended-gdbserver board files.
> > ---
> >  gdb/infrun.c                             |  7 ++-
> >  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
> >  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
> >  3 files changed, 101 insertions(+), 2 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.c
> >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.exp
> >
> > diff --git a/gdb/infrun.c b/gdb/infrun.c
> > index 58da1cef29e..f40d924dc7a 100644
> > --- a/gdb/infrun.c
> > +++ b/gdb/infrun.c
> > @@ -7776,7 +7776,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> >
> >    for (thread_info *tp : all_threads_safe ())
> >      {
> > -      if (tp->state == THREAD_EXITED)
> > +      if (tp->state != THREAD_RUNNING)
> >  	continue;
> >
> >        if (tp->has_pending_waitstatus ())
> > @@ -7800,7 +7800,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
> >
> >    for (thread_info *tp : all_threads_safe ())
> >      {
> > -      if (tp->state == THREAD_EXITED)
> > +      if (tp->state != THREAD_RUNNING)
> >  	continue;
> >
> >        if (tp->has_pending_waitstatus ())
> > @@ -8523,6 +8523,9 @@ static void
> >  end_stepping_range (struct execution_control_state *ecs)
> >  {
> >    ecs->event_thread->control.stop_step = 1;
> > +  ecs->event_thread->control.trap_expected = 0;
> > +  ecs->event_thread->control.step_range_start = 0;
> > +  ecs->event_thread->control.step_range_end = 0;
> >    stop_waiting (ecs);
> >  }
> >
> > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.c b/gdb/testsuite/gdb.multi/detach-
> > stepi.c
> > new file mode 100644
> > index 00000000000..d365645fb3f
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/detach-stepi.c
> > @@ -0,0 +1,30 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2022 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/>.  */
> > +
> > +void
> > +a_function ()
> > +{
> > +  int a = 42;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +  int b = 43;
> > +  a_function ();
> > +  return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp
> > b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > new file mode 100644
> > index 00000000000..28ef8c4f9f7
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > @@ -0,0 +1,66 @@
> > +# This testcase is part of GDB, the GNU debugger.
> > +
> > +# Copyright 2022 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/>.
> > +
> > +# Test detaching from an inferior after a thread in another inferior
> > +# completes a stepi.  This is a regression test for a bug that was
> > +# causing an inadvertent resume of the just-stepped thread.
> > +
> > +standard_testfile
> > +
> > +if {[use_gdb_stub]} {
> > +    untested "using gdb stub"
> > +    return 0
> > +}
> > +
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> > +    return -1
> > +}
> > +
> > +if {![runto_main]} {
> > +    return -1
> > +}
> > +
> > +delete_breakpoints
> > +
> > +# Setup inferior 2.
> > +gdb_test "add-inferior" "Added inferior .*" \
> > +    "add empty inferior"
> > +gdb_test "inferior 2" "Switching to inferior .*" \
> > +    "switch to inferior"
> > +
> > +gdb_load $binfile
> > +runto "a_function"
> > +gdb_test "info inferiors"
> > +
> > +# The bug for which this regression test is written appears in
> > +# schedule-multi mode.
> > +gdb_test_no_output "set schedule-multiple on"
> > +
> > +# Single-step the thread in Inferior 2, then detach Inferior 1.
> > +gdb_test "info threads" ".*" "threads before stepi"
> > +gdb_test "stepi"
> > +gdb_test "info threads" ".*" "threads after stepi"
> > +
> > +gdb_test "set debug infrun on"
> > +gdb_test_multiple "detach inferior 1" "" {
> > +    -re "resuming previously stepped thread.*$gdb_prompt" {
> > +	fail $gdb_test_name
> > +    }
> > +    -re "$gdb_prompt $" {
> > +	pass $gdb_test_name
> > +    }
> > +}
> > --
> > 2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Aktemur, Tankut Baris]

Kindly pinging.
The patch still applies smoothly.

Regards,
-Baris

On Wednesday, April 23, 2025 9:59 AM, Aktemur, Tankut Baris wrote:
> Kindly pinging.
> 
> Thanks,
> -Baris
> 
> 
> On Tuesday, November 21, 2023 9:01 PM, Aktemur, Tankut Baris wrote:
> > Kindly pinging.
> >
> > (The patch was Tested-By: Guinevere Larsen <blarsen@redhat.com> )
> >
> > Regards
> > -Baris
> >
> > On Tuesday, June 20, 2023 5:25 PM, Aktemur, Tankut Baris wrote:
> > > ======
> > > Changes in V2:
> > >
> > > * Changed the thread state check to `(tp->state != THREAD_RUNNING)`.
> > > * Rebased on the current master.
> > >
> > > The latest discussion was
> > > https://sourceware.org/pipermail/gdb-patches/2023-May/199613.html
> > >
> > > ======
> > >
> > > Suppose we have two inferiors on an all-stop target with schedule-multi
> > > set on:
> > >
> > >   $ gdb -q
> > >   (gdb) target extended-remote | gdbserver --multi -
> > >   Remote debugging using | gdbserver --multi -
> > >   Remote debugging using stdio
> > >   (gdb) file /temp/test
> > >   Reading symbols from /temp/test...
> > >   (gdb) set remote exec-file /temp/test
> > >   (gdb) start
> > >   Temporary breakpoint 1 at 0x115c: file test.c, line 8.
> > >   Starting program: /temp/test
> > >   stdin/stdout redirected
> > >   Process /temp/test created; pid = 864027
> > >   ...
> > >
> > >   Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd218) at test.c:8
> > >   8         foo();
> > >   (gdb) add-inferior
> > >   [New inferior 2]
> > >   Added inferior 2 on connection 1 (extended-remote | gdbserver --multi -)
> > >   (gdb) inferior 2
> > >   [Switching to inferior 2 [<null>] (<noexec>)]
> > >   (gdb) file /temp/test
> > >   Reading symbols from /temp/test...
> > >   (gdb) set remote exec-file /temp/test
> > >   (gdb) tbreak 2
> > >   Temporary breakpoint 2 at 0x555555555131: /temp/test.c:2. (2 locations)
> > >   (gdb) run
> > >   Starting program: /temp/test
> > >   stdin/stdout redirected
> > >   Process /temp/test created; pid = 864430
> > >   ...
> > >
> > >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> > >   2         int a = 42;
> > >   (gdb) set schedule-multi on
> > >   (gdb)
> > >
> > > At this point, detaching the first inferior works fine:
> > >
> > >   (gdb) detach inferiors 1
> > >   Detaching from program: /temp/test, process 858904
> > >   Detaching from process 858904
> > >   [Inferior 1 (process 858904) detached]
> > >   (gdb) info inferiors
> > >     Num  Description       Connection
> Executable
> > >     1    <null>            1 (extended-remote | gdbserver --multi -)
> /temp/test
> > >   * 2    process 858925    1 (extended-remote | gdbserver --multi -)
> /temp/test
> > >   (gdb)
> > >
> > > Let us now repeat exactly the same scenario, but before detaching, we
> > > make the current thread single-step an instruction:
> > >
> > >   ...
> > >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> > >   2         int a = 42;
> > >   (gdb) stepi
> > >   3         int b = 43;
> > >   (gdb) detach inferiors 1
> > >   Detaching from program: /temp/test, process 876580
> > >   Detaching from process 876580
> > >   gdbserver: Couldn't reap LWP 876580 while detaching: No child processes
> > >   [Inferior 1 (process 876580) detached]
> > >   (gdb) 3   int b = 43;
> > >
> > > There is a mysterious line info output.  Running the scenario with
> > > infrun debug logs reveals more information.
> > >
> > >   ...
> > >   Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
> > >   2         int a = 42;
> > >   (gdb) stepi
> > >   3         int b = 43;
> > >   (gdb) set debug infrun on
> > >   (gdb) detach inferiors 1
> > >   [infrun] scoped_disable_commit_resumed: reason=detaching
> > >   Detaching from program: /temp/test, process 872445
> > >   Detaching from process 872445
> > >   gdbserver: Couldn't reap LWP 872445 while detaching: No child processes
> > >   [Inferior 1 (process 872445) detached]
> > >   [infrun] start_step_over: enter
> > >     [infrun] start_step_over: stealing global queue of threads to step,
> length = 0
> > >     [infrun] operator(): step-over queue now empty
> > >   [infrun] start_step_over: exit
> > >   [infrun] restart_stepped_thread: switching back to stepped thread
> (stepping)
> > >   [infrun] keep_going_stepped_thread: resuming previously stepped thread
> > >   [infrun] keep_going_stepped_thread: expected thread advanced also
> (0x555555555131 -
> > > > 0x555555555138)
> > >   [infrun] clear_step_over_info: clearing step over info
> > >   [infrun] do_target_resume: resume_ptid=-1.0.0, step=0, sig=GDB_SIGNAL_0
> > >   [infrun] infrun_async: enable=1
> > >   [infrun] reset: reason=detaching
> > >   [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for
> target
> > > extended-remote
> > >   [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for
> target
> > > extended-remote
> > >   (gdb) [infrun] fetch_inferior_event: enter
> > >     [infrun] scoped_disable_commit_resumed: reason=handling event
> > >     [infrun] do_target_wait: Found 2 inferiors, starting at #0
> > >     [infrun] random_pending_event_thread: None found.
> > >     [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1],
> status) =
> > >     [infrun] print_target_wait_results:   872464.872464.0 [Thread
> 872464.872464],
> > >     [infrun] print_target_wait_results:   status->kind = STOPPED, sig =
> > > GDB_SIGNAL_TRAP
> > >     [infrun] handle_inferior_event: status->kind = STOPPED, sig =
> GDB_SIGNAL_TRAP
> > >     [infrun] context_switch: Switching context from 0.0.0 to 872464.872464.0
> > >     [infrun] handle_signal_stop: stop_pc=0x555555555138
> > >     [infrun] handle_signal_stop: [872464.872464.0] hit its single-step
> breakpoint
> > >     [infrun] handle_signal_stop: delayed software breakpoint trap, ignoring
> > >     [infrun] process_event_stop_test: stepi/nexti
> > >     [infrun] stop_waiting: stop_waiting
> > >   3         int b = 43;
> > >     [infrun] infrun_async: enable=0
> > >     [infrun] reset: reason=handling event
> > >     [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-
> resumed for
> > > target extended-remote, no resumed threads
> > >   [infrun] fetch_inferior_event: exit
> > >
> > > GDB attempted to do a step-over for the current thread.  This takes us
> > > to the commit that introduced restarting step-overs:
> > >
> > >   commit 408f66864a1a823591b26420410c982174c239a2
> > >   Author: Pedro Alves <pedro@palves.net>
> > >   Date:   Mon Jan 11 20:01:58 2021 +0000
> > >
> > >     detach in all-stop with threads running
> > >
> > >     A following patch will add a testcase that has a number of threads
> > >     constantly stepping over a breakpoint, and then has GDB detach the
> > >     process, while threads are running.  If we have more than one inferior
> > >     running, and we detach from just one of the inferiors, we expect that
> > >     the remaining inferior continues running.  However, in all-stop, if
> > >     GDB needs to pause the target for the detach, nothing is re-resuming
> > >     the other inferiors after the detach.  "info threads" shows the
> > >     threads as running, but they really aren't.  This fixes it.
> > >
> > > However, the thread that was resumed for step-over in our scenario did
> > > not have an interrupted step-over; it had completed its stepi already.
> > > More debugging reveals that the thread is resumed because of the
> > > following two conditions in `restart_stepped_thread`:
> > >
> > >       if (tp->control.trap_expected)
> > >         {
> > >           infrun_debug_printf ("switching back to stepped thread (step-
> over)");
> > >
> > >           if (keep_going_stepped_thread (tp))
> > >             return true;
> > >         }
> > >
> > > and
> > >
> > >       if (tp->control.step_range_end)
> > >         {
> > >           infrun_debug_printf ("switching back to stepped thread
> (stepping)");
> > >
> > >           if (keep_going_stepped_thread (tp))
> > >             return true;
> > >         }
> > >
> > > The root cause of the problem is, `restart_stepped_thread` checks for
> > > the thread state as
> > >
> > >     if (tp->state == THREAD_EXITED)
> > >       continue;
> > >
> > > but the thread's state is THREAD_STOPPED.  To fix, we change the state
> > > check to
> > >
> > >     if (tp->state != THREAD_RUNNING)
> > >
> > > Additionally, the 'trap_expected' and the 'step_range_end' fields of
> > > the thread's control remain set even after the "stepi" command
> > > completes, creating a half-baked internal state that can be misleading
> > > when debugging.  We address this problem by clearing the control
> > > fields when stepping completes.  We also add a regression test.
> > >
> > > Regression-tested on X86_64 Linux using the default, native-gdbserver,
> > > and native-extended-gdbserver board files.
> > > ---
> > >  gdb/infrun.c                             |  7 ++-
> > >  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
> > >  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
> > >  3 files changed, 101 insertions(+), 2 deletions(-)
> > >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.c
> > >  create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.exp
> > >
> > > diff --git a/gdb/infrun.c b/gdb/infrun.c
> > > index 58da1cef29e..f40d924dc7a 100644
> > > --- a/gdb/infrun.c
> > > +++ b/gdb/infrun.c
> > > @@ -7776,7 +7776,7 @@ restart_stepped_thread (process_stratum_target
> *resume_target,
> > >
> > >    for (thread_info *tp : all_threads_safe ())
> > >      {
> > > -      if (tp->state == THREAD_EXITED)
> > > +      if (tp->state != THREAD_RUNNING)
> > >  	continue;
> > >
> > >        if (tp->has_pending_waitstatus ())
> > > @@ -7800,7 +7800,7 @@ restart_stepped_thread (process_stratum_target
> *resume_target,
> > >
> > >    for (thread_info *tp : all_threads_safe ())
> > >      {
> > > -      if (tp->state == THREAD_EXITED)
> > > +      if (tp->state != THREAD_RUNNING)
> > >  	continue;
> > >
> > >        if (tp->has_pending_waitstatus ())
> > > @@ -8523,6 +8523,9 @@ static void
> > >  end_stepping_range (struct execution_control_state *ecs)
> > >  {
> > >    ecs->event_thread->control.stop_step = 1;
> > > +  ecs->event_thread->control.trap_expected = 0;
> > > +  ecs->event_thread->control.step_range_start = 0;
> > > +  ecs->event_thread->control.step_range_end = 0;
> > >    stop_waiting (ecs);
> > >  }
> > >
> > > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.c
> b/gdb/testsuite/gdb.multi/detach-
> > > stepi.c
> > > new file mode 100644
> > > index 00000000000..d365645fb3f
> > > --- /dev/null
> > > +++ b/gdb/testsuite/gdb.multi/detach-stepi.c
> > > @@ -0,0 +1,30 @@
> > > +/* This testcase is part of GDB, the GNU debugger.
> > > +
> > > +   Copyright 2022 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/>.  */
> > > +
> > > +void
> > > +a_function ()
> > > +{
> > > +  int a = 42;
> > > +}
> > > +
> > > +int
> > > +main ()
> > > +{
> > > +  int b = 43;
> > > +  a_function ();
> > > +  return 0;
> > > +}
> > > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp
> > > b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > > new file mode 100644
> > > index 00000000000..28ef8c4f9f7
> > > --- /dev/null
> > > +++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > > @@ -0,0 +1,66 @@
> > > +# This testcase is part of GDB, the GNU debugger.
> > > +
> > > +# Copyright 2022 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/>.
> > > +
> > > +# Test detaching from an inferior after a thread in another inferior
> > > +# completes a stepi.  This is a regression test for a bug that was
> > > +# causing an inadvertent resume of the just-stepped thread.
> > > +
> > > +standard_testfile
> > > +
> > > +if {[use_gdb_stub]} {
> > > +    untested "using gdb stub"
> > > +    return 0
> > > +}
> > > +
> > > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> > > +    return -1
> > > +}
> > > +
> > > +if {![runto_main]} {
> > > +    return -1
> > > +}
> > > +
> > > +delete_breakpoints
> > > +
> > > +# Setup inferior 2.
> > > +gdb_test "add-inferior" "Added inferior .*" \
> > > +    "add empty inferior"
> > > +gdb_test "inferior 2" "Switching to inferior .*" \
> > > +    "switch to inferior"
> > > +
> > > +gdb_load $binfile
> > > +runto "a_function"
> > > +gdb_test "info inferiors"
> > > +
> > > +# The bug for which this regression test is written appears in
> > > +# schedule-multi mode.
> > > +gdb_test_no_output "set schedule-multiple on"
> > > +
> > > +# Single-step the thread in Inferior 2, then detach Inferior 1.
> > > +gdb_test "info threads" ".*" "threads before stepi"
> > > +gdb_test "stepi"
> > > +gdb_test "info threads" ".*" "threads after stepi"
> > > +
> > > +gdb_test "set debug infrun on"
> > > +gdb_test_multiple "detach inferior 1" "" {
> > > +    -re "resuming previously stepped thread.*$gdb_prompt" {
> > > +	fail $gdb_test_name
> > > +    }
> > > +    -re "$gdb_prompt $" {
> > > +	pass $gdb_test_name
> > > +    }
> > > +}
> > > --
> > > 2.25.1

Intel Deutschland GmbH
Registered Address: Dornacher Straße 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht München HRB 186928

Re: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Kevin Buettner]

Hi Baris,

I've taken a look at your patch and have found a few nits that'll
merit a v3, though some, I think, are due to changes made to the
testing framework since you originally posted your patch.

The C code looks okay to me, but posting a v3 with the testsuite nits
fixed might get a few more pairs of eyes on it before it goes in.

On Tue, 20 Jun 2023 17:24:55 +0200
Tankut Baris Aktemur via Gdb-patches <gdb-patches@sourceware.org> wrote:

[...]
> diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp
> b/gdb/testsuite/gdb.multi/detach-stepi.exp new file mode 100644
> index 00000000000..28ef8c4f9f7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
> @@ -0,0 +1,66 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2022 Free Software Foundation, Inc.

I think that the copyright year should be updated.

[...]
> +standard_testfile
> +
> +if {[use_gdb_stub]} {
> +    untested "using gdb stub"
> +    return 0
> +}

The above should be replaced with:

  require !use_gdb_stub

Other tests in gdb.multi also use:

  require allow_multi_inferior_tests

...so I recommend adding this too.

> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> +    return -1
> +}
> +
> +if {![runto_main]} {
> +    return -1
> +}
> +
> +delete_breakpoints
> +
> +# Setup inferior 2.
> +gdb_test "add-inferior" "Added inferior .*" \
> +    "add empty inferior"
> +gdb_test "inferior 2" "Switching to inferior .*" \
> +    "switch to inferior"
> +
> +gdb_load $binfile
> +runto "a_function"
> +gdb_test "info inferiors"

Is there something that you want to look for with "info inferiors"?
Or is it for debugging purposes?

> +
> +# The bug for which this regression test is written appears in
> +# schedule-multi mode.
> +gdb_test_no_output "set schedule-multiple on"
> +
> +# Single-step the thread in Inferior 2, then detach Inferior 1.
> +gdb_test "info threads" ".*" "threads before stepi"
> +gdb_test "stepi"

Is there some pattern which might be usefully matched for the stepi?

> +gdb_test "info threads" ".*" "threads after stepi"
> +
> +gdb_test "set debug infrun on"

Use gdb_test_no_output for the above command.

> +gdb_test_multiple "detach inferior 1" "" {
> +    -re "resuming previously stepped thread.*$gdb_prompt" {
> +	fail $gdb_test_name
> +    }
> +    -re "$gdb_prompt $" {
> +	pass $gdb_test_name
> +    }
> +}

RE: [PATCH v2] gdb/infrun: do not restart a stepped thread if not running (was: gdb/infrun: reset thread control's step info in end_stepping_range)

[Aktemur, Tankut Baris]

Hi Kevin,

On Saturday, February 21, 2026 9:45 PM, Kevin Buettner wrote:
> Hi Baris,
> 
> I've taken a look at your patch and have found a few nits that'll
> merit a v3, though some, I think, are due to changes made to the
> testing framework since you originally posted your patch.
> 
> The C code looks okay to me, but posting a v3 with the testsuite nits
> fixed might get a few more pairs of eyes on it before it goes in.

Thank you.

> On Tue, 20 Jun 2023 17:24:55 +0200
> Tankut Baris Aktemur via Gdb-patches <gdb-patches@sourceware.org> wrote:
> 
> [...]
> > diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp
> > b/gdb/testsuite/gdb.multi/detach-stepi.exp new file mode 100644
> > index 00000000000..28ef8c4f9f7
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
> > @@ -0,0 +1,66 @@
> > +# This testcase is part of GDB, the GNU debugger.
> > +
> > +# Copyright 2022 Free Software Foundation, Inc.
> 
> I think that the copyright year should be updated.
> 
> [...]
> > +standard_testfile
> > +
> > +if {[use_gdb_stub]} {
> > +    untested "using gdb stub"
> > +    return 0
> > +}
> 
> The above should be replaced with:
> 
>   require !use_gdb_stub
> 
> Other tests in gdb.multi also use:
> 
>   require allow_multi_inferior_tests
> 
> ...so I recommend adding this too.

I removed use_gdb_stub and added `require allow_multi_inferior_tests` only
because it checks for use_gdb_stub, too.

> > +
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
> > +    return -1
> > +}
> > +
> > +if {![runto_main]} {
> > +    return -1
> > +}
> > +
> > +delete_breakpoints
> > +
> > +# Setup inferior 2.
> > +gdb_test "add-inferior" "Added inferior .*" \
> > +    "add empty inferior"
> > +gdb_test "inferior 2" "Switching to inferior .*" \
> > +    "switch to inferior"
> > +
> > +gdb_load $binfile
> > +runto "a_function"
> > +gdb_test "info inferiors"
> 
> Is there something that you want to look for with "info inferiors"?
> Or is it for debugging purposes?

It was only for logging purposes.  I changed it to

  gdb_test "info inferiors" ".*" "info inferiors for logging"

> > +
> > +# The bug for which this regression test is written appears in
> > +# schedule-multi mode.
> > +gdb_test_no_output "set schedule-multiple on"
> > +
> > +# Single-step the thread in Inferior 2, then detach Inferior 1.
> > +gdb_test "info threads" ".*" "threads before stepi"
> > +gdb_test "stepi"
> 
> Is there some pattern which might be usefully matched for the stepi?

Nothing interesting, really.  I added ".*" to make this clear.

> > +gdb_test "info threads" ".*" "threads after stepi"
> > +
> > +gdb_test "set debug infrun on"
> 
> Use gdb_test_no_output for the above command.

I removed the command completely.

> > +gdb_test_multiple "detach inferior 1" "" {
> > +    -re "resuming previously stepped thread.*$gdb_prompt" {
> > +	fail $gdb_test_name
> > +    }
> > +    -re "$gdb_prompt $" {
> > +	pass $gdb_test_name
> > +    }
> > +}

I'll send v3 soon.  Thanks for the review!

-Baris



Intel Deutschland GmbH
Registered Address: Dornacher Straße 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht München HRB 186928

[PATCH v3] gdb/infrun: do not restart a stepped thread if not running

[Tankut Baris Aktemur]

[-- Attachment #1: Type: text/plain, Size: 12561 bytes --]

======
Changes in V3:

* Addressed Kevin's comments from
  https://sourceware.org/pipermail/gdb-patches/2026-February/225235.html

* Rebased on the current master; ran regression tests again.

Changes in V2:

* Changed the thread state check to `(tp->state != THREAD_RUNNING)`.
* Rebased on the current master.

The latest discussion was
https://sourceware.org/pipermail/gdb-patches/2023-May/199613.html

======

Suppose we have two inferiors on an all-stop target with schedule-multi
set on:

  $ gdb -q
  (gdb) target extended-remote | gdbserver --multi -
  Remote debugging using | gdbserver --multi -
  Remote debugging using stdio
  (gdb) file /temp/test
  Reading symbols from /temp/test...
  (gdb) set remote exec-file /temp/test
  (gdb) start
  Temporary breakpoint 1 at 0x115c: file test.c, line 8.
  Starting program: /temp/test
  stdin/stdout redirected
  Process /temp/test created; pid = 864027
  ...

  Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd218) at test.c:8
  8         foo();
  (gdb) add-inferior
  [New inferior 2]
  Added inferior 2 on connection 1 (extended-remote | gdbserver --multi -)
  (gdb) inferior 2
  [Switching to inferior 2 [<null>] (<noexec>)]
  (gdb) file /temp/test
  Reading symbols from /temp/test...
  (gdb) set remote exec-file /temp/test
  (gdb) tbreak 2
  Temporary breakpoint 2 at 0x555555555131: /temp/test.c:2. (2 locations)
  (gdb) run
  Starting program: /temp/test
  stdin/stdout redirected
  Process /temp/test created; pid = 864430
  ...

  Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
  2         int a = 42;
  (gdb) set schedule-multi on
  (gdb)

At this point, detaching the first inferior works fine:

  (gdb) detach inferiors 1
  Detaching from program: /temp/test, process 858904
  Detaching from process 858904
  [Inferior 1 (process 858904) detached]
  (gdb) info inferiors
    Num  Description       Connection                                Executable
    1    <null>            1 (extended-remote | gdbserver --multi -) /temp/test
  * 2    process 858925    1 (extended-remote | gdbserver --multi -) /temp/test
  (gdb)

Let us now repeat exactly the same scenario, but before detaching, we
make the current thread single-step an instruction:

  ...
  Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
  2         int a = 42;
  (gdb) stepi
  3         int b = 43;
  (gdb) detach inferiors 1
  Detaching from program: /temp/test, process 876580
  Detaching from process 876580
  gdbserver: Couldn't reap LWP 876580 while detaching: No child processes
  [Inferior 1 (process 876580) detached]
  (gdb) [Switching to Thread 877351.877351]
  3   int b = 43;

There is a mysterious line info output.  Running the scenario with
infrun debug logs reveals more information.

  ...
  Thread 2.1 "test" hit Temporary breakpoint 2, foo () at test.c:2
  2         int a = 42;
  (gdb) stepi
  3         int b = 43;
  (gdb) set debug infrun on
  (gdb) detach inferiors 1
  [infrun] scoped_disable_commit_resumed: reason=detaching
  [infrun] scoped_disable_commit_resumed: reason=detaching
  Detaching from program: /temp/test, process 3537498
  Detaching from process 3537498
  gdbserver: Couldn't reap LWP 3537498 while detaching: No child processes
  [Inferior 1 (process 3537498) detached]
  [infrun] reset: reason=detaching
  [infrun] start_step_over: enter
    [infrun] start_step_over: stealing global queue of threads to step, length = 0
    [infrun] operator(): step-over queue now empty
  [infrun] start_step_over: exit
  [infrun] restart_stepped_thread: switching back to stepped thread (stepping)
  [infrun] keep_going_stepped_thread: resuming previously stepped thread
  [infrun] keep_going_stepped_thread: expected thread advanced also (0x555555555131 -> 0x555555555138)
  [infrun] clear_step_over_info: clearing step over info
  [infrun] do_target_resume: resume_ptid=-1.0.0, step=0, sig=GDB_SIGNAL_0
  [infrun] infrun_async: enable=1
  [infrun] reset: reason=detaching
  [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target extended-remote
  [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target extended-remote
  (gdb) [infrun] fetch_inferior_event: enter
    [infrun] scoped_disable_commit_resumed: reason=handling event
    [infrun] do_target_wait: Found 2 inferiors, starting at #0
    [infrun] random_pending_event_thread: None found.
    [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
    [infrun] print_target_wait_results:   3537875.3537875.0 [Thread 3537875.3537875],
    [infrun] print_target_wait_results:   status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
    [infrun] handle_inferior_event: status->kind = STOPPED, sig = GDB_SIGNAL_TRAP
    [infrun] context_switch: Switching context from 0.0.0 to 3537875.3537875.0
    [infrun] handle_signal_stop: stop_pc=0x555555555138
    [infrun] handle_signal_stop: [3537875.3537875.0] hit its single-step breakpoint
    [infrun] handle_signal_stop: delayed software breakpoint trap, ignoring
    [infrun] process_event_stop_test: stepi/nexti
    [infrun] stop_waiting: stop_waiting
  [Switching to Thread 3537875.3537875]
  3       }
    [infrun] infrun_async: enable=0
    [infrun] reset: reason=handling event
    [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for target extended-remote, no resumed threads
  [infrun] fetch_inferior_event: exit

GDB attempted to do a step-over for the current thread.  This takes us
to the commit that introduced restarting step-overs:

  commit 408f66864a1a823591b26420410c982174c239a2
  Author: Pedro Alves <pedro@palves.net>
  Date:   Mon Jan 11 20:01:58 2021 +0000

    detach in all-stop with threads running

    A following patch will add a testcase that has a number of threads
    constantly stepping over a breakpoint, and then has GDB detach the
    process, while threads are running.  If we have more than one inferior
    running, and we detach from just one of the inferiors, we expect that
    the remaining inferior continues running.  However, in all-stop, if
    GDB needs to pause the target for the detach, nothing is re-resuming
    the other inferiors after the detach.  "info threads" shows the
    threads as running, but they really aren't.  This fixes it.

However, the thread that was resumed for step-over in our scenario did
not have an interrupted step-over; it had completed its stepi already.
More debugging reveals that the thread is resumed because of the
following two conditions in `restart_stepped_thread`:

      if (tp->control.trap_expected)
        {
          infrun_debug_printf ("switching back to stepped thread (step-over)");

          if (keep_going_stepped_thread (tp))
            return true;
        }

and

      if (tp->control.step_range_end)
        {
          infrun_debug_printf ("switching back to stepped thread (stepping)");

          if (keep_going_stepped_thread (tp))
            return true;
        }

The root cause of the problem is, `restart_stepped_thread` checks for
the thread state as

    if (tp->state == THREAD_EXITED)
      continue;

but the thread's state is THREAD_STOPPED.  To fix, we change the state
check to

    if (tp->state != THREAD_RUNNING)

Additionally, the 'trap_expected' and the 'step_range_end' fields of
the thread's control remain set even after the "stepi" command
completes, creating a half-baked internal state that can be misleading
when debugging.  We address this problem by clearing the control
fields when stepping completes.  We also add a regression test.

Regression-tested on X86_64 Linux using the default, native-gdbserver,
and native-extended-gdbserver board files.

Tested-By: Guinevere Larsen <guinevere@redhat.com>
---
 gdb/infrun.c                             |  7 ++-
 gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
 gdb/testsuite/gdb.multi/detach-stepi.exp | 69 ++++++++++++++++++++++++
 3 files changed, 104 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.c
 create mode 100644 gdb/testsuite/gdb.multi/detach-stepi.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6bcd8ec5cc0..ee5551cb483 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8459,7 +8459,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
 
   for (thread_info &tp : all_threads_safe ())
     {
-      if (tp.state == THREAD_EXITED)
+      if (tp.state != THREAD_RUNNING)
 	continue;
 
       if (tp.has_pending_waitstatus ())
@@ -8483,7 +8483,7 @@ restart_stepped_thread (process_stratum_target *resume_target,
 
   for (thread_info &tp : all_threads_safe ())
     {
-      if (tp.state == THREAD_EXITED)
+      if (tp.state != THREAD_RUNNING)
 	continue;
 
       if (tp.has_pending_waitstatus ())
@@ -9204,6 +9204,9 @@ static void
 end_stepping_range (struct execution_control_state *ecs)
 {
   ecs->event_thread->control.stop_step = 1;
+  ecs->event_thread->control.trap_expected = 0;
+  ecs->event_thread->control.step_range_start = 0;
+  ecs->event_thread->control.step_range_end = 0;
   stop_waiting (ecs);
 }
 
diff --git a/gdb/testsuite/gdb.multi/detach-stepi.c b/gdb/testsuite/gdb.multi/detach-stepi.c
new file mode 100644
index 00000000000..45d8d0c14b4
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/detach-stepi.c
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022-2026 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/>.  */
+
+void
+a_function (void)
+{
+  int a = 42;
+}
+
+int
+main (void)
+{
+  int b = 43;
+  a_function ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.multi/detach-stepi.exp b/gdb/testsuite/gdb.multi/detach-stepi.exp
new file mode 100644
index 00000000000..74beb022bfd
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/detach-stepi.exp
@@ -0,0 +1,69 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2022-2026 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/>.
+
+# Test detaching from an inferior after a thread in another inferior
+# completes a stepi.  This is a regression test for a bug that was
+# causing an inadvertent resume of the just-stepped thread.
+
+standard_testfile
+
+require allow_multi_inferior_tests
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+delete_breakpoints
+
+# Set up inferior 2.
+gdb_test "add-inferior" "Added inferior .*" \
+    "add another inferior"
+gdb_test "inferior 2" "Switching to inferior .*" \
+    "switch inferior"
+
+gdb_load $binfile
+if {![runto "a_function"]} {
+    return -1
+}
+
+gdb_test "info inferiors" ".*" "info inferiors for logging"
+
+# The bug for which this regression test is written appears in
+# schedule-multi mode.
+gdb_test_no_output "set schedule-multiple on"
+
+# Single-step the thread in Inferior 2, then detach Inferior 1.
+gdb_test "info threads" ".*" "threads before stepi"
+gdb_test "stepi" ".*"
+gdb_test "info threads" ".*" "threads after stepi"
+
+gdb_test "detach inferior 1" "Inferior 1 \[^\r\n\]+ detached\\]"
+
+# Bad GDB did an unexpected switch to a thread after the detach
+# command above.
+gdb_test_multiple "print 123" "no unexpected switch" {
+    -re "Switching to .*" {
+	fail $gdb_test_name
+    }
+    -re -wrap "" {
+	pass $gdb_test_name
+    }
+}
-- 
2.34.1


[-- Attachment #2.1: Type: text/plain, Size: 329 bytes --]

Intel Deutschland GmbH
Registered Address: Dornacher Straße 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht München HRB 186928

[-- Attachment #2.2: Type: text/html, Size: 387 bytes --]

Re: [PATCH v3] gdb/infrun: do not restart a stepped thread if not running

[Kevin Buettner]

On Mon, 23 Feb 2026 11:36:55 +0100
Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> wrote:

> ======
> Changes in V3:
> 
> * Addressed Kevin's comments from
>   https://sourceware.org/pipermail/gdb-patches/2026-February/225235.html
> 
> * Rebased on the current master; ran regression tests again.

I've looked it over again and have done some testing.  LGTM; I think
it's ready to go in.

Approved-by: Kevin Buettner <kevinb@redhat.com>