Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus
@ 2026-09-15 14:14 Markus Metzger
  2026-09-18  7:19 ` Metzger, Markus T
  2026-09-18 12:42 ` Guinevere Larsen
  0 siblings, 2 replies; 6+ messages in thread
From: Markus Metzger @ 2026-09-15 14:14 UTC (permalink / raw)
  To: gdb-patches

Consider a scenario with two breakpoints on adjacent instructions:

    bp1 at 0xf00
    bp2 at 0xf01

as well as two threads in all-stop-on-top-of-non-stop mode.

Assume that threads A hits bp1 and we report the breakpoint hit to the
user.  When the user continues, we start a step-over for thread A at 0xf00.

Assume that thread B now hits bp1 and we report the breakpoint hit to the
user.  We stop all threads to report the event.  Meanwhile, the step-over
of thread A completes, so we save the pending waitstatus (stop_pc=0xf01,
currently_stepping=1) of thread A.

When the user continues, clear_proceed_status_thread() discards the
pending step completed waitstatus of thread A, and proceed() starts
another step-over for thread A at 0xf01.

We skip bp2 for thread A.

Remove the code in clear_proceed_status_thread() that discards a step
completed waitstatus and let it get handled normally.
---
 gdb/infrun.c                              | 25 ++-----
 gdb/testsuite/gdb.threads/adjacent-bp.c   | 48 +++++++++++++
 gdb/testsuite/gdb.threads/adjacent-bp.exp | 86 +++++++++++++++++++++++
 3 files changed, 139 insertions(+), 20 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/adjacent-bp.c
 create mode 100644 gdb/testsuite/gdb.threads/adjacent-bp.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index b9618fb6422..4ab9f4aaf64 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3100,28 +3100,13 @@ clear_proceed_status_thread (struct thread_info *tp)
   infrun_debug_printf ("%s", tp->ptid.to_string ().c_str ());
   gdb_assert (tp->internal_state () != THREAD_INT_RUNNING);
 
-  /* If we're starting a new sequence, then the previous finished
-     single-step is no longer relevant.  */
   if (tp->has_pending_waitstatus ())
     {
-      if (tp->stop_reason () == TARGET_STOPPED_BY_SINGLE_STEP)
-	{
-	  infrun_debug_printf ("pending event of %s was a finished step. "
-			       "Discarding.",
-			       tp->ptid.to_string ().c_str ());
-
-	  tp->set_internal_state (THREAD_INT_STOPPED);
-	  tp->clear_pending_waitstatus ();
-	  tp->set_stop_reason (TARGET_STOPPED_BY_NO_REASON);
-	}
-      else
-	{
-	  infrun_debug_printf
-	    ("thread %s has pending wait status %s (currently_stepping=%d).",
-	     tp->ptid.to_string ().c_str (),
-	     tp->pending_waitstatus ().to_string ().c_str (),
-	     tp->control.currently_stepping);
-	}
+      infrun_debug_printf
+	("thread %s has pending wait status %s (currently_stepping=%d).",
+	 tp->ptid.to_string ().c_str (),
+	 tp->pending_waitstatus ().to_string ().c_str (),
+	 tp->control.currently_stepping);
     }
 
   /* If this signal should not be seen by program, give it zero.
diff --git a/gdb/testsuite/gdb.threads/adjacent-bp.c b/gdb/testsuite/gdb.threads/adjacent-bp.c
new file mode 100644
index 00000000000..e72e674f52a
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/adjacent-bp.c
@@ -0,0 +1,48 @@
+/* Copyright 2026 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <unistd.h>
+
+static  pthread_barrier_t barrier;
+
+static void *
+test (void *arg)
+{
+  pthread_barrier_wait (&barrier);
+  int a = 0;  /* break here.  */
+  int b = 0;
+  int c = 0;
+  return arg;
+}
+
+int
+main ()
+{
+  pthread_t th;
+
+  alarm (500);
+
+  pthread_barrier_init (&barrier, NULL, 2);
+  pthread_create (&th, NULL, test, NULL);
+  test (NULL);
+
+  pthread_join (th, NULL);
+  pthread_barrier_destroy (&barrier);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/adjacent-bp.exp b/gdb/testsuite/gdb.threads/adjacent-bp.exp
new file mode 100644
index 00000000000..9e7782c01c2
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/adjacent-bp.exp
@@ -0,0 +1,86 @@
+# Copyright 2026 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that GDB does not skip a breakpoint when a step-over completes
+# while another event leads to a stop.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+	  {debug pthreads}]} {
+    return
+}
+
+if {![runto_main]} {
+    return
+}
+
+# Find a sequence of adjacent instructions.
+set bp_line [gdb_get_line_number "break here"]
+set pcs {}
+gdb_test_multiple "info line $bp_line" "" {
+    -re -wrap "starts at address ($hex).*" {
+	pass $gdb_test_name
+
+	set line "\\s+($hex) \[^\r\n\]+"
+	gdb_test_multiple "x/3i $expect_out(1,string)" "disassemble" {
+	    -re -wrap "$line\r\n$line\r\n$line.*" {
+		pass $gdb_test_name
+
+		lappend pcs $expect_out(1,string)
+		lappend pcs $expect_out(2,string)
+		lappend pcs $expect_out(3,string)
+	    }
+	    -re -wrap "" {
+		fail $gdb_test_name
+	    }
+	}
+    }
+    -re -wrap "" {
+	fail $gdb_test_name
+    }
+}
+
+# Set breakpoints on adjacent instructions.
+foreach pc $pcs {
+    gdb_breakpoint "\*$pc"
+}
+
+# Continue from breakpoint to breakpoint.
+set hits [dict create]
+set iter 0
+gdb_test_multiple "continue" "" {
+    -re -wrap "hit Breakpoint.*" {
+	dict incr hits [get_hexadecimal_valueof "\$pc" invalid "stop $iter"]
+	incr iter
+	send_gdb "continue\n"
+	exp_continue
+    }
+    -re -wrap "$inferior_exited_re normally.*" {
+	pass "$gdb_test_name"
+    }
+}
+
+# We expect all breakpoints to be hit by both threads.
+foreach pc $pcs {
+    if {[dict exists $hits $pc]} {
+	gdb_assert {[dict get $hits $pc] eq 2} "breakpoint at $pc"
+	dict unset hits $pc
+    } else {
+	fail "breakpoint at $pc"
+    }
+}
+# And no unrelated stops.
+gdb_assert {[dict size $hits] eq 0} "no extra stops"
-- 
2.53.0

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus
  2026-09-15 14:14 [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus Markus Metzger
@ 2026-09-18  7:19 ` Metzger, Markus T
  2026-09-18 12:42 ` Guinevere Larsen
  1 sibling, 0 replies; 6+ messages in thread
From: Metzger, Markus T @ 2026-09-18  7:19 UTC (permalink / raw)
  To: gdb-patches

This fixes gdb/34380.  I added

    Fixes gdb/34380.

to the commit message.

Markus.

>-----Original Message-----
>From: Metzger, Markus T <markus.t.metzger@intel.com>
>Sent: Tuesday, September 15, 2026 4:15 PM
>To: gdb-patches@sourceware.org
>Subject: [PATCH] gdb, infrun: do not discard a step-completed pending
>waitstatus
>
>Consider a scenario with two breakpoints on adjacent instructions:
>
>    bp1 at 0xf00
>    bp2 at 0xf01
>
>as well as two threads in all-stop-on-top-of-non-stop mode.
>
>Assume that threads A hits bp1 and we report the breakpoint hit to the
>user.  When the user continues, we start a step-over for thread A at 0xf00.
>
>Assume that thread B now hits bp1 and we report the breakpoint hit to the
>user.  We stop all threads to report the event.  Meanwhile, the step-over
>of thread A completes, so we save the pending waitstatus (stop_pc=0xf01,
>currently_stepping=1) of thread A.
>
>When the user continues, clear_proceed_status_thread() discards the
>pending step completed waitstatus of thread A, and proceed() starts
>another step-over for thread A at 0xf01.
>
>We skip bp2 for thread A.
>
>Remove the code in clear_proceed_status_thread() that discards a step
>completed waitstatus and let it get handled normally.
>---
> gdb/infrun.c                              | 25 ++-----
> gdb/testsuite/gdb.threads/adjacent-bp.c   | 48 +++++++++++++
> gdb/testsuite/gdb.threads/adjacent-bp.exp | 86 +++++++++++++++++++++++
> 3 files changed, 139 insertions(+), 20 deletions(-)
> create mode 100644 gdb/testsuite/gdb.threads/adjacent-bp.c
> create mode 100644 gdb/testsuite/gdb.threads/adjacent-bp.exp
>
>diff --git a/gdb/infrun.c b/gdb/infrun.c
>index b9618fb6422..4ab9f4aaf64 100644
>--- a/gdb/infrun.c
>+++ b/gdb/infrun.c
>@@ -3100,28 +3100,13 @@ clear_proceed_status_thread (struct thread_info
>*tp)
>   infrun_debug_printf ("%s", tp->ptid.to_string ().c_str ());
>   gdb_assert (tp->internal_state () != THREAD_INT_RUNNING);
>
>-  /* If we're starting a new sequence, then the previous finished
>-     single-step is no longer relevant.  */
>   if (tp->has_pending_waitstatus ())
>     {
>-      if (tp->stop_reason () == TARGET_STOPPED_BY_SINGLE_STEP)
>-	{
>-	  infrun_debug_printf ("pending event of %s was a finished step. "
>-			       "Discarding.",
>-			       tp->ptid.to_string ().c_str ());
>-
>-	  tp->set_internal_state (THREAD_INT_STOPPED);
>-	  tp->clear_pending_waitstatus ();
>-	  tp->set_stop_reason (TARGET_STOPPED_BY_NO_REASON);
>-	}
>-      else
>-	{
>-	  infrun_debug_printf
>-	    ("thread %s has pending wait status %s (currently_stepping=%d).",
>-	     tp->ptid.to_string ().c_str (),
>-	     tp->pending_waitstatus ().to_string ().c_str (),
>-	     tp->control.currently_stepping);
>-	}
>+      infrun_debug_printf
>+	("thread %s has pending wait status %s (currently_stepping=%d).",
>+	 tp->ptid.to_string ().c_str (),
>+	 tp->pending_waitstatus ().to_string ().c_str (),
>+	 tp->control.currently_stepping);
>     }
>
>   /* If this signal should not be seen by program, give it zero.
>diff --git a/gdb/testsuite/gdb.threads/adjacent-bp.c
>b/gdb/testsuite/gdb.threads/adjacent-bp.c
>new file mode 100644
>index 00000000000..e72e674f52a
>--- /dev/null
>+++ b/gdb/testsuite/gdb.threads/adjacent-bp.c
>@@ -0,0 +1,48 @@
>+/* Copyright 2026 Free Software Foundation, Inc.
>+
>+   This file is part of GDB.
>+
>+   This program is free software; you can redistribute it and/or modify
>+   it under the terms of the GNU General Public License as published by
>+   the Free Software Foundation; either version 3 of the License, or
>+   (at your option) any later version.
>+
>+   This program is distributed in the hope that it will be useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+   GNU General Public License for more details.
>+
>+   You should have received a copy of the GNU General Public License
>+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>+
>+#include <pthread.h>
>+#include <unistd.h>
>+
>+static  pthread_barrier_t barrier;
>+
>+static void *
>+test (void *arg)
>+{
>+  pthread_barrier_wait (&barrier);
>+  int a = 0;  /* break here.  */
>+  int b = 0;
>+  int c = 0;
>+  return arg;
>+}
>+
>+int
>+main ()
>+{
>+  pthread_t th;
>+
>+  alarm (500);
>+
>+  pthread_barrier_init (&barrier, NULL, 2);
>+  pthread_create (&th, NULL, test, NULL);
>+  test (NULL);
>+
>+  pthread_join (th, NULL);
>+  pthread_barrier_destroy (&barrier);
>+
>+  return 0;
>+}
>diff --git a/gdb/testsuite/gdb.threads/adjacent-bp.exp
>b/gdb/testsuite/gdb.threads/adjacent-bp.exp
>new file mode 100644
>index 00000000000..9e7782c01c2
>--- /dev/null
>+++ b/gdb/testsuite/gdb.threads/adjacent-bp.exp
>@@ -0,0 +1,86 @@
>+# Copyright 2026 Free Software Foundation, Inc.
>+
>+# This program is free software; you can redistribute it and/or modify
>+# it under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# This program is distributed in the hope that it will be useful,
>+# but WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>+
>+# Test that GDB does not skip a breakpoint when a step-over completes
>+# while another event leads to a stop.
>+
>+standard_testfile
>+
>+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
>+	  {debug pthreads}]} {
>+    return
>+}
>+
>+if {![runto_main]} {
>+    return
>+}
>+
>+# Find a sequence of adjacent instructions.
>+set bp_line [gdb_get_line_number "break here"]
>+set pcs {}
>+gdb_test_multiple "info line $bp_line" "" {
>+    -re -wrap "starts at address ($hex).*" {
>+	pass $gdb_test_name
>+
>+	set line "\\s+($hex) \[^\r\n\]+"
>+	gdb_test_multiple "x/3i $expect_out(1,string)" "disassemble" {
>+	    -re -wrap "$line\r\n$line\r\n$line.*" {
>+		pass $gdb_test_name
>+
>+		lappend pcs $expect_out(1,string)
>+		lappend pcs $expect_out(2,string)
>+		lappend pcs $expect_out(3,string)
>+	    }
>+	    -re -wrap "" {
>+		fail $gdb_test_name
>+	    }
>+	}
>+    }
>+    -re -wrap "" {
>+	fail $gdb_test_name
>+    }
>+}
>+
>+# Set breakpoints on adjacent instructions.
>+foreach pc $pcs {
>+    gdb_breakpoint "\*$pc"
>+}
>+
>+# Continue from breakpoint to breakpoint.
>+set hits [dict create]
>+set iter 0
>+gdb_test_multiple "continue" "" {
>+    -re -wrap "hit Breakpoint.*" {
>+	dict incr hits [get_hexadecimal_valueof "\$pc" invalid "stop $iter"]
>+	incr iter
>+	send_gdb "continue\n"
>+	exp_continue
>+    }
>+    -re -wrap "$inferior_exited_re normally.*" {
>+	pass "$gdb_test_name"
>+    }
>+}
>+
>+# We expect all breakpoints to be hit by both threads.
>+foreach pc $pcs {
>+    if {[dict exists $hits $pc]} {
>+	gdb_assert {[dict get $hits $pc] eq 2} "breakpoint at $pc"
>+	dict unset hits $pc
>+    } else {
>+	fail "breakpoint at $pc"
>+    }
>+}
>+# And no unrelated stops.
>+gdb_assert {[dict size $hits] eq 0} "no extra stops"
>--
>2.53.0

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus
  2026-09-15 14:14 [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus Markus Metzger
  2026-09-18  7:19 ` Metzger, Markus T
@ 2026-09-18 12:42 ` Guinevere Larsen
  2026-09-21 11:42   ` Metzger, Markus T
  1 sibling, 1 reply; 6+ messages in thread
From: Guinevere Larsen @ 2026-09-18 12:42 UTC (permalink / raw)
  To: Markus Metzger, gdb-patches


On 9/15/26 11:14 AM, Markus Metzger wrote:
> Consider a scenario with two breakpoints on adjacent instructions:
>
>      bp1 at 0xf00
>      bp2 at 0xf01
>
> as well as two threads in all-stop-on-top-of-non-stop mode.
>
> Assume that threads A hits bp1 and we report the breakpoint hit to the
> user.  When the user continues, we start a step-over for thread A at 0xf00.
>
> Assume that thread B now hits bp1 and we report the breakpoint hit to the
> user.  We stop all threads to report the event.  Meanwhile, the step-over
> of thread A completes, so we save the pending waitstatus (stop_pc=0xf01,
> currently_stepping=1) of thread A.
>
> When the user continues, clear_proceed_status_thread() discards the
> pending step completed waitstatus of thread A, and proceed() starts
> another step-over for thread A at 0xf01.
>
> We skip bp2 for thread A.
>
> Remove the code in clear_proceed_status_thread() that discards a step
> completed waitstatus and let it get handled normally.

Hi! Thanks for working on this.

I have a question about the situation in general. From what I 
understand, the original intent of the code is that, if a user has asked 
for a step, but we hit something more important (like watchpoint or 
breakpoint), then we shouldn't mention the step. With this change, a 
user will now see the step once they resume from the breakpoint, if I 
understand the code correctly.

Should we restrict this change to only announcing breakpoints, or is 
this behavior change for stepping ok?

I don't have an opinion on what is better, but I think if we think that 
the new behavior is better, it should be called out in the commit message.

-- 
Cheers,
Guinevere Larsen
it/its
she/her (deprecated)

> ---
>   gdb/infrun.c                              | 25 ++-----
>   gdb/testsuite/gdb.threads/adjacent-bp.c   | 48 +++++++++++++
>   gdb/testsuite/gdb.threads/adjacent-bp.exp | 86 +++++++++++++++++++++++
>   3 files changed, 139 insertions(+), 20 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.threads/adjacent-bp.c
>   create mode 100644 gdb/testsuite/gdb.threads/adjacent-bp.exp
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index b9618fb6422..4ab9f4aaf64 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -3100,28 +3100,13 @@ clear_proceed_status_thread (struct thread_info *tp)
>     infrun_debug_printf ("%s", tp->ptid.to_string ().c_str ());
>     gdb_assert (tp->internal_state () != THREAD_INT_RUNNING);
>   
> -  /* If we're starting a new sequence, then the previous finished
> -     single-step is no longer relevant.  */
>     if (tp->has_pending_waitstatus ())
>       {
> -      if (tp->stop_reason () == TARGET_STOPPED_BY_SINGLE_STEP)
> -	{
> -	  infrun_debug_printf ("pending event of %s was a finished step. "
> -			       "Discarding.",
> -			       tp->ptid.to_string ().c_str ());
> -
> -	  tp->set_internal_state (THREAD_INT_STOPPED);
> -	  tp->clear_pending_waitstatus ();
> -	  tp->set_stop_reason (TARGET_STOPPED_BY_NO_REASON);
> -	}
> -      else
> -	{
> -	  infrun_debug_printf
> -	    ("thread %s has pending wait status %s (currently_stepping=%d).",
> -	     tp->ptid.to_string ().c_str (),
> -	     tp->pending_waitstatus ().to_string ().c_str (),
> -	     tp->control.currently_stepping);
> -	}
> +      infrun_debug_printf
> +	("thread %s has pending wait status %s (currently_stepping=%d).",
> +	 tp->ptid.to_string ().c_str (),
> +	 tp->pending_waitstatus ().to_string ().c_str (),
> +	 tp->control.currently_stepping);
>       }
>   
>     /* If this signal should not be seen by program, give it zero.
> diff --git a/gdb/testsuite/gdb.threads/adjacent-bp.c b/gdb/testsuite/gdb.threads/adjacent-bp.c
> new file mode 100644
> index 00000000000..e72e674f52a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/adjacent-bp.c
> @@ -0,0 +1,48 @@
> +/* Copyright 2026 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <pthread.h>
> +#include <unistd.h>
> +
> +static  pthread_barrier_t barrier;
> +
> +static void *
> +test (void *arg)
> +{
> +  pthread_barrier_wait (&barrier);
> +  int a = 0;  /* break here.  */
> +  int b = 0;
> +  int c = 0;
> +  return arg;
> +}
> +
> +int
> +main ()
> +{
> +  pthread_t th;
> +
> +  alarm (500);
> +
> +  pthread_barrier_init (&barrier, NULL, 2);
> +  pthread_create (&th, NULL, test, NULL);
> +  test (NULL);
> +
> +  pthread_join (th, NULL);
> +  pthread_barrier_destroy (&barrier);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.threads/adjacent-bp.exp b/gdb/testsuite/gdb.threads/adjacent-bp.exp
> new file mode 100644
> index 00000000000..9e7782c01c2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/adjacent-bp.exp
> @@ -0,0 +1,86 @@
> +# Copyright 2026 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test that GDB does not skip a breakpoint when a step-over completes
> +# while another event leads to a stop.
> +
> +standard_testfile
> +
> +if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> +	  {debug pthreads}]} {
> +    return
> +}
> +
> +if {![runto_main]} {
> +    return
> +}
> +
> +# Find a sequence of adjacent instructions.
> +set bp_line [gdb_get_line_number "break here"]
> +set pcs {}
> +gdb_test_multiple "info line $bp_line" "" {
> +    -re -wrap "starts at address ($hex).*" {
> +	pass $gdb_test_name
> +
> +	set line "\\s+($hex) \[^\r\n\]+"
> +	gdb_test_multiple "x/3i $expect_out(1,string)" "disassemble" {
> +	    -re -wrap "$line\r\n$line\r\n$line.*" {
> +		pass $gdb_test_name
> +
> +		lappend pcs $expect_out(1,string)
> +		lappend pcs $expect_out(2,string)
> +		lappend pcs $expect_out(3,string)
> +	    }
> +	    -re -wrap "" {
> +		fail $gdb_test_name
> +	    }
> +	}
> +    }
> +    -re -wrap "" {
> +	fail $gdb_test_name
> +    }
> +}
> +
> +# Set breakpoints on adjacent instructions.
> +foreach pc $pcs {
> +    gdb_breakpoint "\*$pc"
> +}
> +
> +# Continue from breakpoint to breakpoint.
> +set hits [dict create]
> +set iter 0
> +gdb_test_multiple "continue" "" {
> +    -re -wrap "hit Breakpoint.*" {
> +	dict incr hits [get_hexadecimal_valueof "\$pc" invalid "stop $iter"]
> +	incr iter
> +	send_gdb "continue\n"
> +	exp_continue
> +    }
> +    -re -wrap "$inferior_exited_re normally.*" {
> +	pass "$gdb_test_name"
> +    }
> +}
> +
> +# We expect all breakpoints to be hit by both threads.
> +foreach pc $pcs {
> +    if {[dict exists $hits $pc]} {
> +	gdb_assert {[dict get $hits $pc] eq 2} "breakpoint at $pc"
> +	dict unset hits $pc
> +    } else {
> +	fail "breakpoint at $pc"
> +    }
> +}
> +# And no unrelated stops.
> +gdb_assert {[dict size $hits] eq 0} "no extra stops"


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus
  2026-09-18 12:42 ` Guinevere Larsen
@ 2026-09-21 11:42   ` Metzger, Markus T
  2026-09-21 20:30     ` Guinevere Larsen
  0 siblings, 1 reply; 6+ messages in thread
From: Metzger, Markus T @ 2026-09-21 11:42 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: gdb-patches

>On 9/15/26 11:14 AM, Markus Metzger wrote:
>> Consider a scenario with two breakpoints on adjacent instructions:
>>
>>      bp1 at 0xf00
>>      bp2 at 0xf01
>>
>> as well as two threads in all-stop-on-top-of-non-stop mode.
>>
>> Assume that threads A hits bp1 and we report the breakpoint hit to the
>> user.  When the user continues, we start a step-over for thread A at 0xf00.
>>
>> Assume that thread B now hits bp1 and we report the breakpoint hit to the
>> user.  We stop all threads to report the event.  Meanwhile, the step-over
>> of thread A completes, so we save the pending waitstatus (stop_pc=0xf01,
>> currently_stepping=1) of thread A.
>>
>> When the user continues, clear_proceed_status_thread() discards the
>> pending step completed waitstatus of thread A, and proceed() starts
>> another step-over for thread A at 0xf01.
>>
>> We skip bp2 for thread A.
>>
>> Remove the code in clear_proceed_status_thread() that discards a step
>> completed waitstatus and let it get handled normally.
>
>Hi! Thanks for working on this.
>
>I have a question about the situation in general. From what I
>understand, the original intent of the code is that, if a user has asked
>for a step, but we hit something more important (like watchpoint or
>breakpoint), then we shouldn't mention the step. With this change, a
>user will now see the step once they resume from the breakpoint, if I
>understand the code correctly.

If I understand correctly, the scenario is that the user steps thread A,
the step completes, but GDB picks another event from thread B, say,
a breakpoint hit.  The user then continues, and GDB processes the saved
step completed event from thread A.

In clear_proceed_status_thread(), we set control.step_range_end = 0
for thread A when we continue thread B from the breakpoint.
When processing the saved step completed event, process_event_stop_test()
continues the thread:

  if (ecs->event_thread->control.step_range_end == 0)
    {
      infrun_debug_printf ("no stepping, continue");
      /* Likewise if we aren't even stepping.  */
      keep_going (ecs);
      return;
    }


If we instead switch to thread A and then step thread A, we get an extra
stop event.  When source stepping, we'd simply keep stepping and the
user wouldn't notice the difference.  When stepping a single instruction,
however, we'd stop without making progress.

And if we switch to thread A, insert a breakpoint at the current location,
and continue, we start a step-over for that breakpoint, but then interpret
the pending event as result of this step-over and report the breakpoint.

This is different from current GDB behavior.

If that breakpoint had already been there and thread A hit it, but GDB picked
the event of thread B, we'd not discard the pending event of thread A, so
when we switch to thread A and continue, we report the breakpoint.

The behavior differs whether we stopped at a breakpoint or stepped onto it.
In the latter case, we might skip the breakpoint.  We also wouldn't call
finish_step_over() and we wouldn't clean up displaced stepping state.
Breakpoint actions wouldn't trigger, either.

I'm not sure what would happen if a thread was interrupted and stopped
right at a breakpoint location.  I'd expect the breakpoint to fire when that
thread is resumed, or, for the current thread, the interrupt being re-interpreted
as a breakpoint hit.  Just like a step-completed onto a breakpoint location
is reported as breakpoint hit when done by the current thread.

We need to handle the step completed event and finish the step-over.
It is also good that we now report a breakpoint we stepped onto reliably,
whether this is done by the current thread or some other thread.

We may need some special case to not report a newly inserted breakpoint
at the current location after switching to a thread that completed a step
in the background when resuming that thread.  And for the stepi case.

Both are more niche than the bug this patch is fixing.

Markus.
________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus
  2026-09-21 11:42   ` Metzger, Markus T
@ 2026-09-21 20:30     ` Guinevere Larsen
  2026-09-22  5:48       ` Metzger, Markus T
  0 siblings, 1 reply; 6+ messages in thread
From: Guinevere Larsen @ 2026-09-21 20:30 UTC (permalink / raw)
  To: Metzger, Markus T; +Cc: gdb-patches

On 9/21/26 8:42 AM, Metzger, Markus T wrote:
>> On 9/15/26 11:14 AM, Markus Metzger wrote:
>>> Consider a scenario with two breakpoints on adjacent instructions:
>>>
>>>       bp1 at 0xf00
>>>       bp2 at 0xf01
>>>
>>> as well as two threads in all-stop-on-top-of-non-stop mode.
>>>
>>> Assume that threads A hits bp1 and we report the breakpoint hit to the
>>> user.  When the user continues, we start a step-over for thread A at 0xf00.
>>>
>>> Assume that thread B now hits bp1 and we report the breakpoint hit to the
>>> user.  We stop all threads to report the event.  Meanwhile, the step-over
>>> of thread A completes, so we save the pending waitstatus (stop_pc=0xf01,
>>> currently_stepping=1) of thread A.
>>>
>>> When the user continues, clear_proceed_status_thread() discards the
>>> pending step completed waitstatus of thread A, and proceed() starts
>>> another step-over for thread A at 0xf01.
>>>
>>> We skip bp2 for thread A.
>>>
>>> Remove the code in clear_proceed_status_thread() that discards a step
>>> completed waitstatus and let it get handled normally.
>> Hi! Thanks for working on this.
>>
>> I have a question about the situation in general. From what I
>> understand, the original intent of the code is that, if a user has asked
>> for a step, but we hit something more important (like watchpoint or
>> breakpoint), then we shouldn't mention the step. With this change, a
>> user will now see the step once they resume from the breakpoint, if I
>> understand the code correctly.
> If I understand correctly, the scenario is that the user steps thread A,
> the step completes, but GDB picks another event from thread B, say,
> a breakpoint hit.  The user then continues, and GDB processes the saved
> step completed event from thread A.
>
> In clear_proceed_status_thread(), we set control.step_range_end = 0
> for thread A when we continue thread B from the breakpoint.
> When processing the saved step completed event, process_event_stop_test()
> continues the thread:
>
>    if (ecs->event_thread->control.step_range_end == 0)
>      {
>        infrun_debug_printf ("no stepping, continue");
>        /* Likewise if we aren't even stepping.  */
>        keep_going (ecs);
>        return;
>      }
>
>
> If we instead switch to thread A and then step thread A, we get an extra
> stop event.  When source stepping, we'd simply keep stepping and the
> user wouldn't notice the difference.  When stepping a single instruction,
> however, we'd stop without making progress.
>
> And if we switch to thread A, insert a breakpoint at the current location,
> and continue, we start a step-over for that breakpoint, but then interpret
> the pending event as result of this step-over and report the breakpoint.
>
> This is different from current GDB behavior.
>
> If that breakpoint had already been there and thread A hit it, but GDB picked
> the event of thread B, we'd not discard the pending event of thread A, so
> when we switch to thread A and continue, we report the breakpoint.
>
> The behavior differs whether we stopped at a breakpoint or stepped onto it.
> In the latter case, we might skip the breakpoint.  We also wouldn't call
> finish_step_over() and we wouldn't clean up displaced stepping state.
> Breakpoint actions wouldn't trigger, either.
>
> I'm not sure what would happen if a thread was interrupted and stopped
> right at a breakpoint location.  I'd expect the breakpoint to fire when that
> thread is resumed, or, for the current thread, the interrupt being re-interpreted
> as a breakpoint hit.  Just like a step-completed onto a breakpoint location
> is reported as breakpoint hit when done by the current thread.
>
> We need to handle the step completed event and finish the step-over.
> It is also good that we now report a breakpoint we stepped onto reliably,
> whether this is done by the current thread or some other thread.
>
> We may need some special case to not report a newly inserted breakpoint
> at the current location after switching to a thread that completed a step
> in the background when resuming that thread.  And for the stepi case.
>
> Both are more niche than the bug this patch is fixing.

Right, this makes sense. Yeah, I agree that this is more niche than the 
bug you're dealing with, but I think it would be nice to document that 
this behavior change is known so that if we get a bug about this in the 
future, we know what's going on.

I don't know enough to review the code but I ran the test and I see it 
fixes the issue, so feel free to add my test tag

Tested-By: Guinevere Larsen <guinevere@redhat.com>

-- 
Cheers,
Guinevere Larsen
it/its
she/her (deprecated)

>
> Markus.
> ________________________________________
> Intel Deutschland GmbH
>
> Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
>
> Tel: +49 (89) 99143-0
>
> www.intel.de
>
> Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman
>
> Chairperson of the Supervisory Board: Sonja Pierer
>
> Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus
  2026-09-21 20:30     ` Guinevere Larsen
@ 2026-09-22  5:48       ` Metzger, Markus T
  0 siblings, 0 replies; 6+ messages in thread
From: Metzger, Markus T @ 2026-09-22  5:48 UTC (permalink / raw)
  To: Guinevere Larsen; +Cc: gdb-patches

>On 9/21/26 8:42 AM, Metzger, Markus T wrote:
>>> On 9/15/26 11:14 AM, Markus Metzger wrote:
>>>> Consider a scenario with two breakpoints on adjacent instructions:
>>>>
>>>>       bp1 at 0xf00
>>>>       bp2 at 0xf01
>>>>
>>>> as well as two threads in all-stop-on-top-of-non-stop mode.
>>>>
>>>> Assume that threads A hits bp1 and we report the breakpoint hit to the
>>>> user.  When the user continues, we start a step-over for thread A at 0xf00.
>>>>
>>>> Assume that thread B now hits bp1 and we report the breakpoint hit to
>the
>>>> user.  We stop all threads to report the event.  Meanwhile, the step-over
>>>> of thread A completes, so we save the pending waitstatus (stop_pc=0xf01,
>>>> currently_stepping=1) of thread A.
>>>>
>>>> When the user continues, clear_proceed_status_thread() discards the
>>>> pending step completed waitstatus of thread A, and proceed() starts
>>>> another step-over for thread A at 0xf01.
>>>>
>>>> We skip bp2 for thread A.
>>>>
>>>> Remove the code in clear_proceed_status_thread() that discards a step
>>>> completed waitstatus and let it get handled normally.
>>> Hi! Thanks for working on this.
>>>
>>> I have a question about the situation in general. From what I
>>> understand, the original intent of the code is that, if a user has asked
>>> for a step, but we hit something more important (like watchpoint or
>>> breakpoint), then we shouldn't mention the step. With this change, a
>>> user will now see the step once they resume from the breakpoint, if I
>>> understand the code correctly.
>> If I understand correctly, the scenario is that the user steps thread A,
>> the step completes, but GDB picks another event from thread B, say,
>> a breakpoint hit.  The user then continues, and GDB processes the saved
>> step completed event from thread A.
>>
>> In clear_proceed_status_thread(), we set control.step_range_end = 0
>> for thread A when we continue thread B from the breakpoint.
>> When processing the saved step completed event,
>process_event_stop_test()
>> continues the thread:
>>
>>    if (ecs->event_thread->control.step_range_end == 0)
>>      {
>>        infrun_debug_printf ("no stepping, continue");
>>        /* Likewise if we aren't even stepping.  */
>>        keep_going (ecs);
>>        return;
>>      }
>>
>>
>> If we instead switch to thread A and then step thread A, we get an extra
>> stop event.  When source stepping, we'd simply keep stepping and the
>> user wouldn't notice the difference.  When stepping a single instruction,
>> however, we'd stop without making progress.
>>
>> And if we switch to thread A, insert a breakpoint at the current location,
>> and continue, we start a step-over for that breakpoint, but then interpret
>> the pending event as result of this step-over and report the breakpoint.
>>
>> This is different from current GDB behavior.
>>
>> If that breakpoint had already been there and thread A hit it, but GDB picked
>> the event of thread B, we'd not discard the pending event of thread A, so
>> when we switch to thread A and continue, we report the breakpoint.
>>
>> The behavior differs whether we stopped at a breakpoint or stepped onto it.
>> In the latter case, we might skip the breakpoint.  We also wouldn't call
>> finish_step_over() and we wouldn't clean up displaced stepping state.
>> Breakpoint actions wouldn't trigger, either.
>>
>> I'm not sure what would happen if a thread was interrupted and stopped
>> right at a breakpoint location.  I'd expect the breakpoint to fire when that
>> thread is resumed, or, for the current thread, the interrupt being re-
>interpreted
>> as a breakpoint hit.  Just like a step-completed onto a breakpoint location
>> is reported as breakpoint hit when done by the current thread.
>>
>> We need to handle the step completed event and finish the step-over.
>> It is also good that we now report a breakpoint we stepped onto reliably,
>> whether this is done by the current thread or some other thread.
>>
>> We may need some special case to not report a newly inserted breakpoint
>> at the current location after switching to a thread that completed a step
>> in the background when resuming that thread.  And for the stepi case.
>>
>> Both are more niche than the bug this patch is fixing.
>
>Right, this makes sense. Yeah, I agree that this is more niche than the
>bug you're dealing with, but I think it would be nice to document that
>this behavior change is known so that if we get a bug about this in the
>future, we know what's going on.
>
>I don't know enough to review the code but I ran the test and I see it
>fixes the issue, so feel free to add my test tag
>
>Tested-By: Guinevere Larsen <guinevere@redhat.com>

Thanks.  I added this to the end of the commit message: "

    Remove the code in clear_proceed_status_thread() that discards a step
    completed waitstatus and let it get handled normally.  This involves
    calling finish_step_over() to clean up displaced stepping state.
    
    This introduces an additional step completed stop event for thread A that
    results in two user-visible changes in behavior when switching to thread A
    and resuming that thread:
    
      - stepi/nexti completes without making progress
    
      - adding a breakpoint at the current location before resuming hits that
        breakpoint
    
    Fixes gdb/34380.
    
    Tested-By: Guinevere Larsen <guinevere@redhat.com>
".

Maybe I find a way to address them before some maintainer approves the
patch.  The second issue appears to be a special case in proceed():

      if (cur_thr->stop_pc_p ()
	  && pc == cur_thr->stop_pc ()
	  && breakpoint_here_p (aspace, pc) == ordinary_breakpoint_here
	  && execution_direction != EXEC_REVERSE)
	/* There is a breakpoint at the address we will resume at,
	   step one instruction before inserting breakpoints so that
	   we do not stop right away (and report a second hit at this
	   breakpoint).

	   Note, we don't do this in reverse, because we won't
	   actually be executing the breakpoint insn anyway.
	   We'll be (un-)executing the previous instruction.  */
	cur_thr->stepping_over_breakpoint = 1;

We're normally setting stepping_over_breakpoint = 1 when we hit a breakpoint
in process_event_stop_test(), e.g.

    case BPSTAT_WHAT_STOP_NOISY:
      infrun_debug_printf ("BPSTAT_WHAT_STOP_NOISY");
      stop_print_frame = true;

      /* Assume the thread stopped for a breakpoint.  We'll still check
	 whether a/the breakpoint is there when the thread is next
	 resumed.  */
      ecs->event_thread->stepping_over_breakpoint = 1;

      stop_waiting (ecs);
      return;

This is probably done to support the 'break' command without arguments.
The part 'and report a second hit at this breakpoint' doesn't sound right (anymore)
because we should have set it when we hit the breakpoint the first time.

Regards,
Markus.
________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-22  5:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15 14:14 [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus Markus Metzger
2026-09-18  7:19 ` Metzger, Markus T
2026-09-18 12:42 ` Guinevere Larsen
2026-09-21 11:42   ` Metzger, Markus T
2026-09-21 20:30     ` Guinevere Larsen
2026-09-22  5:48       ` Metzger, Markus T

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox