This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

Hui, can you hold off on this change?

What I'm finding is that I can reproduce the bad behavior
that Pedro demonstrated on the 20080930 branch, but ONLY
if I omit Pedro's change in adjust_pc_after_break.  If I
add Pedro's change, I can no longer reproduce any bad
behavior.

I've added Pedro's change to the branch now -- why don't
you temporarily take out all these changes, and then see
if you can make it manifest a problem.


teawater wrote: 
Sorry for understand your mean so later Pedro. I made a new patch that
Set pc if forward execute and gdbarch_decr_pc_after_break is not 0 in
replay mode. How do you think about it?


And I think 20080930 branch is need your "adjust_pc_reverse.diff". Do
you mind I check it in?

On Tue, Oct 21, 2008 at 08:21, Pedro Alves <pedro@codesourcery.com> wrote:
On Tuesday 21 October 2008 00:36:12, teawater wrote: 
I think your mean is check breakpoint in address
read_pc()+gdbarch_decr_pc_after_break (gdbarch) in record_wait, right?

Taking x86 as an example, when you're doing normal debugging and you
hit a breakpoint (SIGTRAP), the first read_pc GDB does to check where
what breakpoint was hit, will read back `breakpoint_PC + 1' --- GDB takes care
getting rid of that `+ 1' offset in infrun.c:adjust_pc_after_break.  The
idea is for you to do the same as the kernel/hardware would --- still
check for breakpoints at read_pc, but increment PC by 1 before reporting the
breakpoint to GDB's core.  E.g., see the `pc += gdbarch...' line from
the patch I posted previously, something like:

record.c:record_wait ()
{
...
+         /* Check for breakpoint hits in forward execution.  */
+         pc = read_pc ();
+         if (execution_direction == EXEC_FORWARD
+             && regular_breakpoint_inserted_here_p (pc)
+             /* && !single-stepping */)
+           {
+             status->kind = TARGET_WAITKIND_STOPPED;
+             status->value.sig = TARGET_SIGNAL_TRAP;
+             if (software_breakpoint_inserted_here_p (pc))
+               {
+                 pc += gdbarch_decr_pc_after_break (gdbarch);
+                 write_pc (pc);
+               }
+

--
Pedro Alves


------------------------------------------------------------------------

--- a/record.c
+++ b/record.c
@@ -497,6 +497,30 @@ 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)))
+ {
+ 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 +612,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 +652,24 @@ 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)
{
- 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 +700,7 @@ next:
perror_with_name (_("Process record: sigaction"));
}
+replay_out:
if (record_get_sig)
{
status->value.sig = TARGET_SIGNAL_INT;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]