diff mbox series

[RFC,8/8] gdb, infrun: only step over breakpoints that were hit

Message ID 20261001102847.576880-9-markus.t.metzger@intel.com
State New
Headers
Series breakpoint handling |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 warning Skipped because it is an RFC
linaro-tcwg-bot/tcwg_gdb_build--master-arm warning Skipped because it is an RFC

Commit Message

Metzger, Markus T Oct. 1, 2026, 10:28 a.m. UTC 
  When a thread hits a breakpoint, we note that this thread will need to
step over the breakpoint when it is next resumed, 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;

in process_event_stop_test().

In addition, there is a special check whether there is a breakpoint at the
current location 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;

This does not consider whether we actually hit the breakpoint.  So, when
setting $pc to a location with a breakpoint and continuing, we will skip
it, but when jumping to that same location, we will hit the breakpoint.

Prior to the parent patch, when stepping onto the breakpoint location in
some other thread that was resumed while stepping the current thread, and
then switching to that thread, we had skipped that breakpoint.

There even is some special code in other locations to avoid the special
handling in proceed():

    /* Start the target running.  Do not use -1 continuation as it would skip
       breakpoint right at the entry point.  */
    proceed (regcache_read_pc (get_thread_regcache (inferior_thread ())),
             GDB_SIGNAL_0);

Removing this special case in proceed() makes GDB behave more consistently.
It does introduce potentially undesired behavior when using the break
command without arguments, though.  E.g.

    (gdb) break
    (gdb) continue

will now hit the newly inserted breakpoint that used to be skipped.

On the other hand, any other thread at the same location would have hit
the breakpoint, even though the current thread skipped it.  E.g.

    (gdb) break
    (gdb) thread <some other thread>
    (gdb) continue

would have hit the breakpoint (assuming no scheduler-locking).
---
 gdb/infcmd.c | 10 ++++++----
 gdb/infrun.c | 20 +-------------------
 2 files changed, 7 insertions(+), 23 deletions(-)
diff mbox series

Patch

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index d7544168362..043f5e890bf 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -654,10 +654,8 @@  run_command_1 (const char *args, int from_tty, enum run_how run_how)
      prior to a proceed call.  */
   clear_proceed_status (0);
 
-  /* Start the target running.  Do not use -1 continuation as it would skip
-     breakpoint right at the entry point.  */
-  proceed (regcache_read_pc (get_thread_regcache (inferior_thread ())),
-	   GDB_SIGNAL_0);
+  /* Start the target running.  */
+  proceed ((CORE_ADDR) -1, GDB_SIGNAL_0);
 
   /* Since there was no error, there's no need to finish the thread
      states here.  */
@@ -1312,6 +1310,10 @@  jump_command (const char *arg, int from_tty)
       gdb_printf (".\n");
     }
 
+  /* Any pending step-over is cancelled by the jump.  */
+  thread_info *tp = inferior_thread ();
+  tp->stepping_over_breakpoint = 0;
+
   clear_proceed_status (0);
   proceed (addr, GDB_SIGNAL_0);
 }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 44c41be1baa..2450394e3d1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3634,7 +3634,6 @@  proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   INFRUN_SCOPED_DEBUG_ENTER_EXIT;
 
   struct gdbarch *gdbarch;
-  CORE_ADDR pc;
 
   /* If we're stopped at a fork/vfork, switch to either the parent or child
      thread as defined by the "set follow-fork-mode" command, or, if both
@@ -3658,11 +3657,9 @@  proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 
   regcache *regcache = get_thread_regcache (cur_thr);
   gdbarch = regcache->arch ();
-  pc = regcache_read_pc_protected (regcache);
 
   /* Fill in with reasonable starting values.  */
   cur_thr->stepped_breakpoint = 0;
-  cur_thr->stepping_over_breakpoint = 0;
   cur_thr->stepping_over_watchpoint = 0;
   cur_thr->step_after_step_resume_breakpoint = 0;
 
@@ -3677,22 +3674,7 @@  proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 
   if (addr == (CORE_ADDR) -1)
     {
-      const address_space *aspace = cur_thr->inf->aspace.get ();
-
-      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;
-      else if (gdbarch_single_step_through_delay_p (gdbarch)
+      if (gdbarch_single_step_through_delay_p (gdbarch)
 	       && gdbarch_single_step_through_delay (gdbarch,
 						     get_current_frame ()))
 	/* We stepped onto an instruction that needs to be stepped