This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [Patch 1/2] infrun.c support for MIPS hardware watchpoints.
Pedro Alves wrote:
On Monday 06 April 2009 07:49:11, David Daney wrote:
+/* Try to setup for software single stepping over the specified location.
+ Return 1 if target_resume() should use hardware single step.
+
+ GDBARCH the current gdbarch.
+ PC the location to step over. */
+static int
Add an empty line between comment and function, please.
OK.
+set_for_singlestep (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+ int step = 1;
+
+ if (gdbarch_software_single_step_p (gdbarch)
+ && gdbarch_software_single_step (gdbarch, get_current_frame ()))
+ {
+ step = 0;
+ /* Do not pull these breakpoints until after a `wait' in
+ `wait_for_inferior' */
+ singlestep_breakpoints_inserted_p = 1;
+ singlestep_ptid = inferior_ptid;
+ singlestep_pc = pc;
+ }
+ return step;
+}
Because I'm dumb, while reading this, 'set_for_singlestep' and
'int step' didn't cross my bridge. How about renaming the local
variable 'hw_step'?
OK
I don't like the "set_for_singlestep" name much.
It isn't much self-describing, which is something very important in
infrun.c, since it contains horrible, huge, full-of-states, hard to
read code --- it is write-once, read-and-debug-millions-of-times
code. But, I tried to come up with a descriptive name to suggest
for it, and I didn't come up with something that made me happy,
so, ... that's OK.
We went with: maybe_software_singlestep ().
+ /* Do we need to do it the hard way, w/temp breakpoints? */
+ if (step)
+ step = set_for_singlestep (gdbarch, pc);
^ something looks strange with the indentation here.
I was incorrect, now fixed.
/* If there were any forks/vforks/execs that were caught and are
now to be followed, then do so. */
@@ -2826,11 +2837,14 @@ targets should add new threads to the th
the inferior over it. If we have non-steppable watchpoints,
we must disable the current watchpoint; it's simplest to
disable all watchpoints and breakpoints. */
-
+ int step_over_watchpoint = 1;
Similarly, rename this to hw_step. Even if this is false, we're
still doing a a high-level step-over-watchpoint.
OK.
OK with the above changes.
This is what I committed.
2009-04-13 David Daney <ddaney@caviumnetworks.com>
* infrun.c (maybe_software_singlestep): New function.
(resume): Call maybe_software_singlestep.
(handle_inferior_event): Same.
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.367
diff -u -p -r1.367 infrun.c
--- infrun.c 25 Mar 2009 21:53:10 -0000 1.367
+++ infrun.c 14 Apr 2009 00:56:16 -0000
@@ -950,6 +950,29 @@ set_schedlock_func (char *args, int from
}
}
+/* Try to setup for software single stepping over the specified location.
+ Return 1 if target_resume() should use hardware single step.
+
+ GDBARCH the current gdbarch.
+ PC the location to step over. */
+
+static int
+maybe_software_singlestep (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+ int hw_step = 1;
+
+ if (gdbarch_software_single_step_p (gdbarch)
+ && gdbarch_software_single_step (gdbarch, get_current_frame ()))
+ {
+ hw_step = 0;
+ /* Do not pull these breakpoints until after a `wait' in
+ `wait_for_inferior' */
+ singlestep_breakpoints_inserted_p = 1;
+ singlestep_ptid = inferior_ptid;
+ singlestep_pc = pc;
+ }
+ return hw_step;
+}
/* Resume the inferior, but allow a QUIT. This is useful if the user
wants to interrupt some lengthy single-stepping operation
@@ -1031,20 +1054,9 @@ a command like `return' or `jump' to con
}
}
- if (step && gdbarch_software_single_step_p (gdbarch))
- {
- /* Do it the hard way, w/temp breakpoints */
- if (gdbarch_software_single_step (gdbarch, get_current_frame ()))
- {
- /* ...and don't ask hardware to do it. */
- step = 0;
- /* and do not pull these breakpoints until after a `wait' in
- `wait_for_inferior' */
- singlestep_breakpoints_inserted_p = 1;
- singlestep_ptid = inferior_ptid;
- singlestep_pc = pc;
- }
- }
+ /* Do we need to do it the hard way, w/temp breakpoints? */
+ if (step)
+ step = maybe_software_singlestep (gdbarch, pc);
/* If there were any forks/vforks/execs that were caught and are
now to be followed, then do so. */
@@ -2826,11 +2838,14 @@ targets should add new threads to the th
the inferior over it. If we have non-steppable watchpoints,
we must disable the current watchpoint; it's simplest to
disable all watchpoints and breakpoints. */
-
+ int hw_step = 1;
+
if (!HAVE_STEPPABLE_WATCHPOINT)
remove_breakpoints ();
registers_changed ();
- target_resume (ecs->ptid, 1, TARGET_SIGNAL_0); /* Single step */
+ /* Single step */
+ hw_step = maybe_software_singlestep (current_gdbarch, read_pc ());
+ target_resume (ecs->ptid, hw_step, TARGET_SIGNAL_0);
waiton_ptid = ecs->ptid;
if (HAVE_STEPPABLE_WATCHPOINT)
infwait_state = infwait_step_watch_state;