* breakpoint.c (software_breakpoint_inserted_here_p): New function.
authorDaniel Jacobowitz <drow@false.org>
Sat, 31 Jan 2004 19:18:13 +0000 (19:18 +0000)
committerDaniel Jacobowitz <drow@false.org>
Sat, 31 Jan 2004 19:18:13 +0000 (19:18 +0000)
(bpstat_stop_status): Don't decrement PC.
* breakpoint.h (software_breakpoint_inserted_here_p): Add
prototype.
* infrun.c (adjust_pc_after_break): New function.
(handle_inferior_event): Call it, early.  Remove later references
to DECR_PC_AFTER_BREAK.
(normal_stop): Add commentary.

gdb/ChangeLog
gdb/breakpoint.c
gdb/breakpoint.h
gdb/infrun.c

index 3e5deadfa91cae02ee699dcb57f43110319ae749..805cac77749101a7f07f2afaa7117b11106dc9aa 100644 (file)
@@ -1,3 +1,14 @@
+2004-01-31  Daniel Jacobowitz  <drow@mvista.com>
+
+       * breakpoint.c (software_breakpoint_inserted_here_p): New function.
+       (bpstat_stop_status): Don't decrement PC.
+       * breakpoint.h (software_breakpoint_inserted_here_p): Add
+       prototype.
+       * infrun.c (adjust_pc_after_break): New function.
+       (handle_inferior_event): Call it, early.  Remove later references
+       to DECR_PC_AFTER_BREAK.
+       (normal_stop): Add commentary.
+
 2004-01-31  Daniel Jacobowitz  <drow@mvista.com>
 
        * breakpoint.c (breakpoint_re_set_one): Add missing chunk of
index 4ea0f676ff3cd801d5e03c844ab9f9719cd7f503..109a20a0a5a8dd1912763ad5ad9f82e3a04dd198 100644 (file)
@@ -1717,6 +1717,37 @@ breakpoint_inserted_here_p (CORE_ADDR pc)
   return 0;
 }
 
+/* This function returns non-zero iff there is a software breakpoint
+   inserted at PC.  */
+
+int
+software_breakpoint_inserted_here_p (CORE_ADDR pc)
+{
+  struct bp_location *bpt;
+  int any_breakpoint_here = 0;
+
+  ALL_BP_LOCATIONS (bpt)
+    {
+      if (bpt->loc_type != bp_loc_software_breakpoint)
+       continue;
+
+      if ((breakpoint_enabled (bpt->owner)
+          || bpt->owner->enable_state == bp_permanent)
+         && bpt->inserted
+         && bpt->address == pc)        /* bp is enabled and matches pc */
+       {
+         if (overlay_debugging 
+             && section_is_overlay (bpt->section) 
+             && !section_is_mapped (bpt->section))
+           continue;           /* unmapped overlay -- can't be a match */
+         else
+           return 1;
+       }
+    }
+
+  return 0;
+}
+
 /* Return nonzero if FRAME is a dummy frame.  We can't use
    DEPRECATED_PC_IN_CALL_DUMMY because figuring out the saved SP would
    take too much time, at least using frame_register() on the 68k.
@@ -2571,13 +2602,7 @@ bpstat_stop_status (CORE_ADDR *pc, int not_a_sw_breakpoint)
   /* Pointer to the last thing in the chain currently.  */
   bpstat bs = root_bs;
 
-  /* Get the address where the breakpoint would have been.  The
-     "not_a_sw_breakpoint" argument is meant to distinguish between a
-     breakpoint trap event and a trace/singlestep trap event.  For a
-     trace/singlestep trap event, we would not want to subtract
-     DECR_PC_AFTER_BREAK from the PC. */
-
-  bp_addr = *pc - (not_a_sw_breakpoint ? 0 : DECR_PC_AFTER_BREAK);
+  bp_addr = *pc;
 
   ALL_BREAKPOINTS_SAFE (b, temp)
   {
@@ -2863,18 +2888,6 @@ bpstat_stop_status (CORE_ADDR *pc, int not_a_sw_breakpoint)
   bs->next = NULL;             /* Terminate the chain */
   bs = root_bs->next;          /* Re-grab the head of the chain */
 
-  if (real_breakpoint && bs)
-    {
-      if (bs->breakpoint_at->type != bp_hardware_breakpoint)
-       {
-         if (DECR_PC_AFTER_BREAK != 0)
-           {
-             *pc = bp_addr;
-             write_pc (bp_addr);
-           }
-       }
-    }
-
   /* The value of a hardware watchpoint hasn't changed, but the
      intermediate memory locations we are watching may have.  */
   if (bs && !bs->stop &&
index 85cc8b9f3bbf26b240ecf5a13c5635fa90ac21f2..2be0bb00d5481ce95a0b48fc29a8642743efd536 100644 (file)
@@ -605,6 +605,8 @@ extern enum breakpoint_here breakpoint_here_p (CORE_ADDR);
 
 extern int breakpoint_inserted_here_p (CORE_ADDR);
 
+extern int software_breakpoint_inserted_here_p (CORE_ADDR);
+
 /* FIXME: cagney/2002-11-10: The current [generic] dummy-frame code
    implements a functional superset of this function.  The only reason
    it hasn't been removed is because some architectures still don't
index 0724546ccfe7f12b40e24ae0511df460dee16aec..da67e4b06629adcdce3dfd3876f73630f9222214 100644 (file)
@@ -1313,6 +1313,74 @@ handle_step_into_function (struct execution_control_state *ecs)
   return;
 }
 
+static void
+adjust_pc_after_break (struct execution_control_state *ecs)
+{
+  CORE_ADDR stop_pc;
+
+  /* If this target does not decrement the PC after breakpoints, then
+     we have nothing to do.  */
+  if (DECR_PC_AFTER_BREAK == 0)
+    return;
+
+  /* If we've hit a breakpoint, we'll normally be stopped with SIGTRAP.  If
+     we aren't, just return.
+     
+     NOTE drow/2004-01-31: On some targets, breakpoints may generate
+     different signals (SIGILL or SIGEMT for instance), but it is less
+     clear where the PC is pointing afterwards.  It may not match
+     DECR_PC_AFTER_BREAK.  I don't know any specific target that generates
+     these signals at breakpoints (the code has been in GDB since at least
+     1992) so I can not guess how to handle them here.
+     
+     In earlier versions of GDB, a target with HAVE_NONSTEPPABLE_WATCHPOINTS
+     would have the PC after hitting a watchpoint affected by
+     DECR_PC_AFTER_BREAK.  I haven't found any target with both of these set
+     in GDB history, and it seems unlikely to be correct, so
+     HAVE_NONSTEPPABLE_WATCHPOINTS is not checked here.  */
+
+  if (ecs->ws.kind != TARGET_WAITKIND_STOPPED)
+    return;
+
+  if (ecs->ws.value.sig != TARGET_SIGNAL_TRAP)
+    return;
+
+  /* Find the location where (if we've hit a breakpoint) the breakpoint would
+     be.  */
+  stop_pc = read_pc_pid (ecs->ptid) - DECR_PC_AFTER_BREAK;
+
+  /* If we're software-single-stepping, then assume this is a breakpoint.
+     NOTE drow/2004-01-17: This doesn't check that the PC matches, or that
+     we're even in the right thread.  The software-single-step code needs
+     some modernization.
+
+     If we're not software-single-stepping, then we first check that there
+     is an enabled software breakpoint at this address.  If there is, and
+     we weren't using hardware-single-step, then we've hit the breakpoint.
+
+     If we were using hardware-single-step, we check prev_pc; if we just
+     stepped over an inserted software breakpoint, then we should decrement
+     the PC and eventually report hitting the breakpoint.  The prev_pc check
+     prevents us from decrementing the PC if we just stepped over a jump
+     instruction and landed on the instruction after a breakpoint.
+
+     The last bit checks that we didn't hit a breakpoint in a signal handler
+     without an intervening stop in sigtramp, which is detected by a new
+     stack pointer value below any usual function calling stack adjustments.
+
+     NOTE drow/2004-01-17: I'm not sure that this is necessary.  The check
+     predates checking for software single step at the same time.  Also,
+     if we've moved into a signal handler we should have seen the
+     signal.  */
+
+  if ((SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+      || (software_breakpoint_inserted_here_p (stop_pc)
+         && !(currently_stepping (ecs)
+              && prev_pc != stop_pc
+              && !(step_range_end && INNER_THAN (read_sp (), (step_sp - 16))))))
+    write_pc_pid (stop_pc, ecs->ptid);
+}
+
 /* Given an execution control state that has been freshly filled in
    by an event from the inferior, figure out what it means and take
    appropriate action.  */
@@ -1332,6 +1400,8 @@ handle_inferior_event (struct execution_control_state *ecs)
   target_last_wait_ptid = ecs->ptid;
   target_last_waitstatus = *ecs->wp;
 
+  adjust_pc_after_break (ecs);
+
   switch (ecs->infwait_state)
     {
     case infwait_thread_hop_state:
@@ -1685,19 +1755,15 @@ handle_inferior_event (struct execution_control_state *ecs)
       /* Check if a regular breakpoint has been hit before checking
          for a potential single step breakpoint. Otherwise, GDB will
          not see this breakpoint hit when stepping onto breakpoints.  */
-      if (breakpoints_inserted
-          && breakpoint_here_p (stop_pc - DECR_PC_AFTER_BREAK))
+      if (breakpoints_inserted && breakpoint_here_p (stop_pc))
        {
          ecs->random_signal = 0;
-         if (!breakpoint_thread_match (stop_pc - DECR_PC_AFTER_BREAK,
-                                       ecs->ptid))
+         if (!breakpoint_thread_match (stop_pc, ecs->ptid))
            {
              int remove_status;
 
              /* Saw a breakpoint, but it was hit by the wrong thread.
                 Just continue. */
-             if (DECR_PC_AFTER_BREAK)
-               write_pc_pid (stop_pc - DECR_PC_AFTER_BREAK, ecs->ptid);
 
              remove_status = remove_breakpoints ();
              /* Did we fail to remove breakpoints?  If so, try
@@ -1710,7 +1776,7 @@ handle_inferior_event (struct execution_control_state *ecs)
              if (remove_status != 0)
                {
                  /* FIXME!  This is obviously non-portable! */
-                 write_pc_pid (stop_pc - DECR_PC_AFTER_BREAK + 4, ecs->ptid);
+                 write_pc_pid (stop_pc + 4, ecs->ptid);
                  /* We need to restart all the threads now,
                   * unles we're running in scheduler-locked mode. 
                   * Use currently_stepping to determine whether to 
@@ -1744,17 +1810,6 @@ handle_inferior_event (struct execution_control_state *ecs)
        }
       else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
         {
-          /* Readjust the stop_pc as it is off by DECR_PC_AFTER_BREAK
-             compared to the value it would have if the system stepping
-             capability was used. This allows the rest of the code in
-             this function to use this address without having to worry
-             whether software single step is in use or not.  */
-          if (DECR_PC_AFTER_BREAK)
-            {
-              stop_pc -= DECR_PC_AFTER_BREAK;
-              write_pc_pid (stop_pc, ecs->ptid);
-            }
-
           sw_single_step_trap_p = 1;
           ecs->random_signal = 0;
         }
@@ -1886,9 +1941,6 @@ handle_inferior_event (struct execution_control_state *ecs)
          includes evaluating watchpoints, things will come to a
          stop in the correct manner.  */
 
-      if (DECR_PC_AFTER_BREAK)
-       write_pc (stop_pc - DECR_PC_AFTER_BREAK);
-
       remove_breakpoints ();
       registers_changed ();
       target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);   /* Single step */
@@ -3121,6 +3173,7 @@ normal_stop (void)
       previous_inferior_ptid = inferior_ptid;
     }
 
+  /* NOTE drow/2004-01-17: Is this still necessary?  */
   /* Make sure that the current_frame's pc is correct.  This
      is a correction for setting up the frame info before doing
      DECR_PC_AFTER_BREAK */