[RFC] Add support of software single step to process record
Hui Zhu
teawater@gmail.com
Mon Jan 4 14:23:00 GMT 2010
Sorry guys, the prev patch is so ugly.
Thanks for teach me clear about the gdbarch_software_single_step, Pedro.
I did some extend with your idea. Because record_wait need
record_resume_step point out this resume is signal step or continue.
if (!step)
{
/* This is not hard single step. */
if (!gdbarch_software_single_step_p (gdbarch))
{
/* This is a normal continue. */
step = 1;
}
else
{
/* This arch support soft sigle step. */
if (single_step_breakpoints_inserted ())
{
/* This is a soft single step. */
record_resume_step = 1;
}
else
{
/* This is a continue.
Try to insert a soft single step breakpoint. */
if (!gdbarch_software_single_step (gdbarch,
get_current_frame ()))
{
/* This system don't want use soft single step.
Use hard sigle step. */
step = 1;
}
}
}
}
Shuchuang, please help me try this patch. Thanks.
Best regards,
Hui
2010-01-04 Hui Zhu <teawater@gmail.com>
* breakpoint.c (single_step_breakpoints_inserted): New
function.
* breakpoint.h (single_step_breakpoints_inserted): Extern.
* record.c (record_resume): Add code for software single step.
(record_wait): Ditto.
On Fri, Dec 25, 2009 at 01:38, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 23 December 2009 09:23:21, Hui Zhu wrote:
>> + struct gdbarch *gdbarch = target_thread_architecture (ptid);
>> +
>> record_message (get_current_regcache (), signal);
>
>> record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1,
>> signal);
>
> Why is this resume call still present?
>
>> +
>> + if (gdbarch_software_single_step_p (gdbarch))
>> + {
>> + if (!inserted_single_step_breakpoint_p ())
>
> Isn't this naming stale? I thought you had renamed this.
>
>> + gdbarch_software_single_step (gdbarch, get_current_frame ());
>> + record_beneath_to_resume (record_beneath_to_resume_ops,
>> + ptid, step, signal);
>> + record_resume_step = 0;
>> + }
>> + else
>> + record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1,
>> + signal);
>> }
>>
>
> You've got the predicates a bit mixed up.
>
> - gdbarch_software_single_step_p purpose is only "is there or
> not a gdbarch_software_single_step callback registered in
> this gdbarch"? It returning true does not mean that
> software single-step should be used for that single-step.
>
> - gdbarch_software_single_step can return false, meaning,
> no software single-step needs to be used.
>
> This is how stepping over atomic sequences is handled
> currently (grep for deal_with_atomic_sequence):
> gdbarch_software_single_step_p returns true, but
> gdbarch_software_single_step returns false most
> of the times. See also infrun.c:maybe_software_singlestep.
>
> I think you want this:
>
> if (!step
> && gdbarch_software_single_step_p (gdbarch)
> && !single_step_breakpoints_inserted ()
> && gdbarch_software_single_step (gdbarch, get_current_frame ()))
> record_resume_step = 0;
> else
> record_resume_step = 1;
>
> record_beneath_to_resume (record_beneath_to_resume_ops, ptid,
> record_resume_step, signal);
>
> If `step' is true when record_resume is called, and so is
> gdbarch_software_single_step_p, then it must be that infrun.c
> already determined that gdbarch_software_single_step returns
> false, otherwise, `step' would be false (maybe_software_singlestep).
>
> If `step' is false (the user is requesting a continue), and
> no single-step breakpoints are inserted yet, but,
> gdbarch_software_single_step returns false, we have ourselves
> an arch/target combo that only wants software single-stepping
> for atomic sequences, e.g., MIPS (non-linux), or PPC. If
> so, we should force hardware single-step in the target
> beneath (set record_resume_step to 1).
>
> --
> Pedro Alves
>
-------------- next part --------------
---
breakpoint.c | 10 ++++++++++
breakpoint.h | 1 +
record.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 59 insertions(+), 3 deletions(-)
--- a/breakpoint.c
+++ b/breakpoint.c
@@ -9646,6 +9646,16 @@ insert_single_step_breakpoint (struct gd
paddress (gdbarch, next_pc));
}
+/* Check if the breakpoints used for software single stepping
+ were inserted or not. */
+
+int
+single_step_breakpoints_inserted (void)
+{
+ return (single_step_breakpoints[0] != NULL
+ || single_step_breakpoints[1] != NULL);
+}
+
/* Remove and delete any breakpoints used for software single step. */
void
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -944,6 +944,7 @@ extern int remove_hw_watchpoints (void);
twice before remove is called. */
extern void insert_single_step_breakpoint (struct gdbarch *,
struct address_space *, CORE_ADDR);
+extern int single_step_breakpoints_inserted (void);
extern void remove_single_step_breakpoints (void);
/* Manage manual breakpoints, separate from the normal chain of
--- a/record.c
+++ b/record.c
@@ -1002,9 +1002,43 @@ record_resume (struct target_ops *ops, p
if (!RECORD_IS_REPLAY)
{
+ struct gdbarch *gdbarch = target_thread_architecture (ptid);
+
record_message (get_current_regcache (), signal);
- record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1,
- signal);
+
+ if (!step)
+ {
+ /* This is not hard single step. */
+ if (!gdbarch_software_single_step_p (gdbarch))
+ {
+ /* This is a normal continue. */
+ step = 1;
+ }
+ else
+ {
+ /* This arch support soft sigle step. */
+ if (single_step_breakpoints_inserted ())
+ {
+ /* This is a soft single step. */
+ record_resume_step = 1;
+ }
+ else
+ {
+ /* This is a continue.
+ Try to insert a soft single step breakpoint. */
+ if (!gdbarch_software_single_step (gdbarch,
+ get_current_frame ()))
+ {
+ /* This system don't want use soft single step.
+ Use hard sigle step. */
+ step = 1;
+ }
+ }
+ }
+ }
+
+ record_beneath_to_resume (record_beneath_to_resume_ops,
+ ptid, step, signal);
}
}
@@ -1077,6 +1111,7 @@ record_wait (struct target_ops *ops,
/* This is not a single step. */
ptid_t ret;
CORE_ADDR tmp_pc;
+ struct gdbarch *gdbarch = target_thread_architecture (inferior_ptid);
while (1)
{
@@ -1099,6 +1134,9 @@ record_wait (struct target_ops *ops,
tmp_pc = regcache_read_pc (regcache);
aspace = get_regcache_aspace (regcache);
+ if (gdbarch_software_single_step_p (gdbarch))
+ remove_single_step_breakpoints ();
+
if (target_stopped_by_watchpoint ())
{
/* Always interested in watchpoints. */
@@ -1121,6 +1159,8 @@ record_wait (struct target_ops *ops,
{
/* This must be a single-step trap. Record the
insn and issue another step. */
+ int step = 1;
+
if (!record_message_wrapper_safe (regcache,
TARGET_SIGNAL_0))
{
@@ -1129,8 +1169,13 @@ record_wait (struct target_ops *ops,
break;
}
+ if (gdbarch_software_single_step_p (gdbarch)
+ && gdbarch_software_single_step (gdbarch,
+ get_current_frame ()))
+ step = 0;
+
record_beneath_to_resume (record_beneath_to_resume_ops,
- ptid, 1,
+ ptid, step,
TARGET_SIGNAL_0);
continue;
}
More information about the Gdb-patches
mailing list