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

[Markus Metzger <markus.t.metzger@intel.com>]

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