Re: [reverse/record] adjust_pc_after_break in reverse execution mode?

teawater
2008-10-22T00:39:25Z

[-- Attachment #1: Type: text/plain, Size: 1820 bytes --]

I see.
I make a patch for it.

2008-10-21 Hui Zhu <teawater@gmail.com>

* record.c (record_wait): Check breakpint before forward
execute in replay mode.
Check breakpoint use function "breakpoint_inserted_here_p"
in replay mode.
Set pc if forward execute, gdbarch_decr_pc_after_break is not
0 and this is not single step in replay mode.

Thanks,
Hui

On Wed, Oct 22, 2008 at 02:30, Michael Snyder <msnyder@vmware.com> wrote:
> teawater wrote:
>>
>> Sorry I send too much Email. I found that:
>>
>> if (singlestep_breakpoints_inserted_p
>> || !ptid_equal (ecs->ptid, inferior_ptid)
>> || !currently_stepping (ecs->event_thread)
>> || ecs->event_thread->prev_pc == breakpoint_pc)
>> regcache_write_pc (regcache, breakpoint_pc);
>>
>> Before write_pc, there are a lot of thing to check. Do we need to
>> check it in record_wait?
>> If so, it actually useless cause it will be set back in
>> adjust_pc_after_break?
>> Maybe we can let adjust_pc_after_break disable in replay mode.
>>
>> How do you think?
>
> I think we should leave adjust_pc_after_break alone,
> and change record_wait so that it adjusts the pc by
> adding decr_pc_after_break(gdbarch) when appropriate.
>
> Whenever possible, gdb should not need to know the difference
> between replay and live debugging. This keeps things simple,
> and preserves modularity.
>
> Of course, you don't have access to the "ecs" object, which
> is local to infrun. But you do know whether or not gdb is
> stepping. And (for now) you know that there is only one
> thread, so you can (for now) ignore the thread id (ptid).
>
> The value of "step" that was passed to record_resume
> came from "currently_stepping", so you should be able
> to use that.
>
> Something close to the patch that Pedro posted should work...
>
>
>
>

[-- Attachment #2: record_wait_breakpoint.txt --]
[-- Type: text/plain, Size: 2878 bytes --]

--- a/record.c
+++ b/record.c
@@ -497,6 +497,31 @@ record_wait (ptid_t ptid, struct target_
int continue_flag = 1;
int first_record_end = 1;
struct cleanup *old_cleanups = make_cleanup (record_wait_cleanups, 0);
+ CORE_ADDR tmp_pc;
+
+ /* Check breakpoint when forward execute. */
+ if (execution_direction == EXEC_FORWARD)
+ {
+ tmp_pc = regcache_read_pc (regcache);
+ if (breakpoint_inserted_here_p (tmp_pc))
+ {
+ if (record_debug)
+ {
+ fprintf_unfiltered (gdb_stdlog,
+ "Process record: break at 0x%s.\n",
+ paddr_nz (tmp_pc));
+ }
+ if (gdbarch_decr_pc_after_break (get_regcache_arch (regcache))
+ && !record_resume_step)
+ {
+ regcache_write_pc (regcache,
+ tmp_pc +
+ gdbarch_decr_pc_after_break
+ (get_regcache_arch (regcache)));
+ }
+ goto replay_out;
+ }
+ }

record_get_sig = 0;
act.sa_handler = record_sig_handler;
@@ -588,10 +613,6 @@ record_wait (ptid_t ptid, struct target_
}
else
{
- CORE_ADDR tmp_pc;
- struct bp_location *bl;
- struct breakpoint *b;
-
if (record_debug > 1)
{
fprintf_unfiltered (gdb_stdlog,
@@ -632,35 +653,25 @@ record_wait (ptid_t ptid, struct target_
}

/* check breakpoint */
- tmp_pc = read_pc ();
- for (bl = bp_location_chain; bl; bl = bl->global_next)
+ tmp_pc = regcache_read_pc (regcache);
+ if (breakpoint_inserted_here_p (tmp_pc))
{
- b = bl->owner;
- gdb_assert (b);
- if (b->enable_state != bp_enabled
- && b->enable_state != bp_permanent)
- continue;
-
- if (b->type == bp_watchpoint || b->type == bp_catch_fork
- || b->type == bp_catch_vfork
- || b->type == bp_catch_exec
- || b->type == bp_hardware_watchpoint
- || b->type == bp_read_watchpoint
- || b->type == bp_access_watchpoint)
+ if (record_debug)
{
- continue;
+ fprintf_unfiltered (gdb_stdlog,
+ "Process record: break at 0x%s.\n",
+ paddr_nz (tmp_pc));
}
- if (bl->address == tmp_pc)
+ if (gdbarch_decr_pc_after_break (get_regcache_arch (regcache))
+ && execution_direction == EXEC_FORWARD
+ && !record_resume_step)
{
- if (record_debug)
- {
- fprintf_unfiltered (gdb_stdlog,
- "Process record: break at 0x%s.\n",
- paddr_nz (tmp_pc));
- }
- continue_flag = 0;
- break;
+ regcache_write_pc (regcache,
+ tmp_pc +
+ gdbarch_decr_pc_after_break
+ (get_regcache_arch (regcache)));
}
+ continue_flag = 0;
}
}
if (execution_direction == EXEC_REVERSE)
@@ -691,6 +702,7 @@ next:
perror_with_name (_("Process record: sigaction"));
}

+replay_out:
if (record_get_sig)
{
status->value.sig = TARGET_SIGNAL_INT;