[PATCH] gdb/infrun: reset thread control's step info in end_stepping_range

[PATCH] gdb/infrun: reset thread control's step info in end_stepping_range

[Tankut Baris Aktemur]

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, the 'trap_expected' and the
'step_range_end' fields of the thread's control remain set even after
the "stepi" command completes.  We fix the problem by clearing the
control fields when stepping completes.  We also add a regression test.

Regression-tested using the default, native-gdbserver, and
native-extended-gdbserver board files.
---
 gdb/infrun.c                             |  3 ++
 gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
 gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
 3 files changed, 99 insertions(+)
 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 6da46b75ac7..ef422f0e9e9 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8314,6 +8314,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] gdb/infrun: reset thread control's step info in end_stepping_range

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks
-Baris

On Wednesday, November 16, 2022 7:56 PM, Aktemur, Tankut Baris wrote:
> 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, the 'trap_expected' and the
> 'step_range_end' fields of the thread's control remain set even after
> the "stepi" command completes.  We fix the problem by clearing the
> control fields when stepping completes.  We also add a regression test.
> 
> Regression-tested using the default, native-gdbserver, and
> native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  3 ++
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  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 6da46b75ac7..ef422f0e9e9 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8314,6 +8314,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] gdb/infrun: reset thread control's step info in end_stepping_range

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks
-Baris

On Wednesday, November 16, 2022 7:56 PM, Aktemur, Tankut Baris wrote:
> 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, the 'trap_expected' and the
> 'step_range_end' fields of the thread's control remain set even after
> the "stepi" command completes.  We fix the problem by clearing the
> control fields when stepping completes.  We also add a regression test.
> 
> Regression-tested using the default, native-gdbserver, and
> native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  3 ++
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  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 6da46b75ac7..ef422f0e9e9 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8314,6 +8314,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] gdb/infrun: reset thread control's step info in end_stepping_range

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks
-Baris

On Wednesday, November 16, 2022 7:56 PM, Aktemur, Tankut Baris wrote:
> 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, the 'trap_expected' and the
> 'step_range_end' fields of the thread's control remain set even after
> the "stepi" command completes.  We fix the problem by clearing the
> control fields when stepping completes.  We also add a regression test.
> 
> Regression-tested using the default, native-gdbserver, and
> native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  3 ++
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  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 6da46b75ac7..ef422f0e9e9 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8314,6 +8314,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] gdb/infrun: reset thread control's step info in end_stepping_range

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks
-Baris

On Wednesday, November 16, 2022 7:56 PM, Aktemur, Tankut Baris wrote:
> 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, the 'trap_expected' and the
> 'step_range_end' fields of the thread's control remain set even after
> the "stepi" command completes.  We fix the problem by clearing the
> control fields when stepping completes.  We also add a regression test.
> 
> Regression-tested using the default, native-gdbserver, and
> native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  3 ++
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  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 6da46b75ac7..ef422f0e9e9 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8314,6 +8314,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] gdb/infrun: reset thread control's step info in end_stepping_range

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks
-Baris

On Wednesday, November 16, 2022 7:56 PM, Aktemur, Tankut Baris wrote:
> 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, the 'trap_expected' and the
> 'step_range_end' fields of the thread's control remain set even after
> the "stepi" command completes.  We fix the problem by clearing the
> control fields when stepping completes.  We also add a regression test.
> 
> Regression-tested using the default, native-gdbserver, and
> native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  3 ++
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  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 6da46b75ac7..ef422f0e9e9 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8314,6 +8314,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] gdb/infrun: reset thread control's step info in end_stepping_range

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks
-Baris

On Wednesday, November 16, 2022 7:56 PM, Aktemur, Tankut Baris wrote:
> 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, the 'trap_expected' and the
> 'step_range_end' fields of the thread's control remain set even after
> the "stepi" command completes.  We fix the problem by clearing the
> control fields when stepping completes.  We also add a regression test.
> 
> Regression-tested using the default, native-gdbserver, and
> native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  3 ++
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  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 6da46b75ac7..ef422f0e9e9 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8314,6 +8314,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] gdb/infrun: reset thread control's step info in end_stepping_range

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks
-Baris

On Wednesday, November 16, 2022 7:56 PM, Aktemur, Tankut Baris wrote:
> 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, the 'trap_expected' and the
> 'step_range_end' fields of the thread's control remain set even after
> the "stepi" command completes.  We fix the problem by clearing the
> control fields when stepping completes.  We also add a regression test.
> 
> Regression-tested using the default, native-gdbserver, and
> native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  3 ++
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  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 6da46b75ac7..ef422f0e9e9 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8314,6 +8314,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] gdb/infrun: reset thread control's step info in end_stepping_range

[Aktemur, Tankut Baris]

Kindly pinging.

Thanks
-Baris

On Wednesday, November 16, 2022 7:56 PM, Aktemur, Tankut Baris wrote:
> 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, the 'trap_expected' and the
> 'step_range_end' fields of the thread's control remain set even after
> the "stepi" command completes.  We fix the problem by clearing the
> control fields when stepping completes.  We also add a regression test.
> 
> Regression-tested using the default, native-gdbserver, and
> native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  3 ++
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  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 6da46b75ac7..ef422f0e9e9 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8314,6 +8314,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] gdb/infrun: reset thread control's step info in end_stepping_range

[Simon Marchi]

On 11/16/22 13:56, Tankut Baris Aktemur via Gdb-patches wrote:
> 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, the 'trap_expected' and the
> 'step_range_end' fields of the thread's control remain set even after
> the "stepi" command completes.  We fix the problem by clearing the
> control fields when stepping completes.  We also add a regression test.

Isn't the bug mainly that restart_stepped_thread tries to resume a
thread that isn't meant to be resumed?  It checks for:

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

but the thread's state is THREAD_STOPPED.  It seems to me like this
condition should be changed to:

  if (tp->state != THREAD_RUNNING)
    continue;

such that restart_stepped_thread only considers threads that are meant
to be resumed.

But I think I am missing some information.  Let's say you have two
inferiors running on an all-stop target, and you detach one of them (as
what is described by Pedro's commit message, if I understand correctly)
I'm not sure what stops the threads of the non-detached inferior.  I
don't see a call to target_stop or similar on the code path.  Is it the
detach target call that implicitly leaves them stopped?  If so, it
probably means that the m_resumed and m_executing become stale.  Perhaps
not relevant for this problem, but just something that crossed my mind.

With that said, I'm not against the principle of your patch.  There is
some information in the thread_control_state structure that is valid
from the point a step is started, until the thread stops, and becomes
meaningless after that.  By clearing this information as soon as it
becomes irrelevant, it reduces the chances of something using it by
mistake later.

While playing with GDB, trying to reproduce your bug, I think I found
two more:

1. Trying to start a second process with schedule-multiple on seems
broken:


    $ ./gdb -nx -q --data-directory=data-directory \
	-iex "set debuginfod enabled off" \
	-ex "set schedule-multiple on" \
	-ex "file /home/smarchi/build/binutils-gdb/gdb/a.out" -ex start \
	-ex "add-inferior" -ex "inferior 2" \
	-ex "file /home/smarchi/build/binutils-gdb/gdb/a.out" -ex start
    Reading symbols from /home/smarchi/build/binutils-gdb/gdb/a.out...
    Temporary breakpoint 1 at 0x401689: file test.c, line 3.
    Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out

    Temporary breakpoint 1, main () at test.c:3
    3         int a = 0;
    [New inferior 2]
    Added inferior 2 on connection 1 (native)
    [Switching to inferior 2 [<null>] (<noexec>)]
    Reading symbols from /home/smarchi/build/binutils-gdb/gdb/a.out...
    Temporary breakpoint 2 at 0x401689: -qualified main. (2 locations)
    Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out
    Cannot remove breakpoints because program is no longer writable.
    Further execution is probably impossible.

    Thread 2.1 "a.out" hit Temporary breakpoint 2.2, main () at test.c:3
    3         int a = 0;
    warning: Error removing breakpoint 2
    (gdb) info inferiors
      Num  Description       Connection           Executable
      1    process 152722    1 (native)           /home/smarchi/build/binutils-gdb/gdb/a.out
    * 2    process 152726    1 (native)           /home/smarchi/build/binutils-gdb/gdb/a.out
    (gdb) i th
      Id   Target Id              Frame
      1.1  process 152722         Couldn't get registers: No such process.

Oh, seems like I had already found that last year: https://sourceware.org/bugzilla/show_bug.cgi?id=28777

2. In a specific case, detaching one inferior makes threads from the
   other inferior disappear:

    $ ./gdb -nx -q --data-directory=data-directory -iex "set debuginfod enabled off" -ex "maint set target-non-stop off" -ex "file /home/smarchi/build/binutils-gdb/gdb/a.out" -ex start -ex "add-inferior" -ex "inferior 2" -ex "file /home/smarchi/build/binutils-gdb/gdb/a.out" -ex start -ex "set schedule-multiple on" -ex stepi
    Reading symbols from /home/smarchi/build/binutils-gdb/gdb/a.out...
    Temporary breakpoint 1 at 0x401689: file test.c, line 3.
    Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out

    Temporary breakpoint 1, main () at test.c:3
    3         int a = 0;
    [New inferior 2]
    Added inferior 2 on connection 1 (native)
    [Switching to inferior 2 [<null>] (<noexec>)]
    Reading symbols from /home/smarchi/build/binutils-gdb/gdb/a.out...
    Temporary breakpoint 2 at 0x401689: -qualified main. (2 locations)
    Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out

    Thread 2.1 "a.out" hit Temporary breakpoint 2.2, main () at test.c:3
    3         int a = 0;
    5           ++a;
    (gdb) i th
      Id   Target Id              Frame
      1.1  process 155486 "a.out" 0x0000000000401694 in main () at test.c:5
    * 2.1  process 155489 "a.out" main () at test.c:5
    (gdb) detach inferiors 1
    Detaching from program: /home/smarchi/build/binutils-gdb/gdb/a.out, process 155486
    [Inferior 1 (process 155486) detached]
    (gdb) i th
    No threads.

I tracked this last one to having the wrong current inferior when
calling target_thread_alive in keep_going_stepped_thread.  Your patch
would fix that bug, because it would make it so we just wouldn't enter
keep_going_stepped_thread.  But I'd like to get a patch in to fix that
problem, because it is still wrong.

> Regression-tested using the default, native-gdbserver, and
> native-extended-gdbserver board files.
> ---
>  gdb/infrun.c                             |  3 ++
>  gdb/testsuite/gdb.multi/detach-stepi.c   | 30 +++++++++++
>  gdb/testsuite/gdb.multi/detach-stepi.exp | 66 ++++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
>  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 6da46b75ac7..ef422f0e9e9 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8314,6 +8314,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
> +    }
> +}

When trying to fix bug #2 above, I wrote a very similar test that tests
using a variety of configurations.  I would take a bit of your test and
a bit of mine to make one that covers everything.

Simon

RE: [PATCH] gdb/infrun: reset thread control's step info in end_stepping_range

[Aktemur, Tankut Baris]

On Thursday, April 27, 2023 7:05 PM, Simon Marchi wrote:
> On 11/16/22 13:56, Tankut Baris Aktemur via Gdb-patches wrote:
> > 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, the 'trap_expected' and the
> > 'step_range_end' fields of the thread's control remain set even after
> > the "stepi" command completes.  We fix the problem by clearing the
> > control fields when stepping completes.  We also add a regression test.
> 
> Isn't the bug mainly that restart_stepped_thread tries to resume a
> thread that isn't meant to be resumed?  It checks for:
> 
>       if (tp->state == THREAD_EXITED)
> 	continue;
> 
> but the thread's state is THREAD_STOPPED.  It seems to me like this
> condition should be changed to:
> 
>   if (tp->state != THREAD_RUNNING)
>     continue;
> 
> such that restart_stepped_thread only considers threads that are meant
> to be resumed.

This makes sense and would fix the bug.  The code would also be aligned
with the condition in `restart_after_all_stop_detach`.  I add this change
to v2.

> But I think I am missing some information.  Let's say you have two
> inferiors running on an all-stop target, and you detach one of them (as
> what is described by Pedro's commit message, if I understand correctly)
> I'm not sure what stops the threads of the non-detached inferior.  I
> don't see a call to target_stop or similar on the code path.  Is it the
> detach target call that implicitly leaves them stopped?  If so, it
> probably means that the m_resumed and m_executing become stale.  Perhaps
> not relevant for this problem, but just something that crossed my mind.

The all-stop target (e.g. a gdbserver in default case) does not respond
to a detach command while something is running. 

  (gdb) continue &
  Continuing.
  (gdb) Reading /lib/x86_64-linux-gnu/libstdc++.so.6 from remote target...
  ...
  detach
  Cannot execute this command while the target is running.
  Use the "interrupt" command to stop the target
  and then try again.
  (gdb)

So, the target must be explicitly stopped by the user.  Pedro also did
so in the gdb.threads/detach-step-over.exp test:

                -re "Cannot execute this command while the target is running.*$::gdb_prompt $" {
                    # Testing against a remote server that doesn't do
                    # non-stop mode.  Explicitly interrupt.  This
                    # doesn't test the same code paths in GDB, but
                    # it's still something.

I'm not sure, though, if this sufficiently answers your question.

> With that said, I'm not against the principle of your patch.  There is
> some information in the thread_control_state structure that is valid
> from the point a step is started, until the thread stops, and becomes
> meaningless after that.  By clearing this information as soon as it
> becomes irrelevant, it reduces the chances of something using it by
> mistake later.

Ack.  I agree and will keep those changes in v2.

> While playing with GDB, trying to reproduce your bug, I think I found
> two more:

Thinking about this from a wider angle, does it make sense that commands
like 'start', 'run', and 'detach' are impacted by schedule-multi on?
Would it not make more sense that those commands apply to a single inferior
only, e.g.  internally setting schedule-multi to off?

Thanks
-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