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?

teawater wrote: 
The old patch make my_waitpid_record set pc even if this is not a breakpoint.
So I make a new patch that my_waitpid_record just set pc when this is
a breakpoint.


Well, before I can evaluate the patch, I need a test case
to see what behavior it is fixing.  Doesn't have to be a
formal DEJAGNU script, just something like the printf example
that you posted for the other bug.

Right now, I am unable to get the reverse-20080930-branch
to exhibit any bad behavior that I could attribute to this
issue.  It seems to work just fine...


2008-10-24 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.

        * linux-nat.c (my_waitpid_record): Add
        gdbarch_decr_pc_after_break to pc if need.



On Fri, Oct 24, 2008 at 17:57, teawater <teawater@gmail.com> wrote:
Hi buddies,

This is the new patch that fix the break bug.

But I think I still need to add some code to deal with signal.

2008-10-24 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.

       * linux-nat.c (my_waitpid_record): Add
       gdbarch_decr_pc_after_break to pc if need.

Thanks,
Hui

On Fri, Oct 24, 2008 at 16:10, teawater <teawater@gmail.com> wrote:
Thanks Pedro and Michael,

I think the reason is P record let inferior step recycle in the
linux-nat target.
So when it break by breakpint, it will not let
(pc+gdbarch_decr_pc_after_break (gdbarch)). Then after
adjust_pc_after_break, The PC is error.

I will try to deal with it.

Hui

On Fri, Oct 24, 2008 at 09:50, Pedro Alves <pedro@codesourcery.com> wrote:
On Friday 24 October 2008 01:37:31, Michael Snyder wrote: 
In sum, it appears that decr_pc_after_break doesn't matter when you have
continguous breakpoints, as long as you get from from B1's address to B2's
address by single-stepping.  All is good then, it appears!
I agree, at least that is the conclusion I am leaning toward.

Not so fast!  I knew I had to spend a little extra thinking about
it, 'cause I knew something was broken, just couldn't find what.  :-)
*as long as you get from from B1's address to B2's address
by single-stepping* was a restriction that doesn't always apply.

Here's a test that will fail in forward record/replay mode, but not
in normal "play" mode.

volatile int global_foo = 0;

int
main (int argc, char **argv)
{
 asm ("nop"); /* 1st insn */
 asm ("nop"); /* 2nd insn */
 asm ("nop"); /* 3rd insn */
 asm ("nop"); /* 4th insn */
 if (!global_foo)
   goto ahead;
 asm ("nop"); /* 5th insn */
 asm ("nop"); /* 6th insn */
 asm ("nop"); /* 7th insn */
 asm ("nop"); /* 8th insn */  <<< break 1 here
 ahead:
 asm ("nop"); /* 9th insn */  <<< break 2 here
 end:
 return 0;
}

If you let the program reply until break 2 is hit, and assuming insn
8th and 9th are assembled as contiguous (they do on x86 -O0 for me), you'll
see that adjust_pc_after_break will indeed make it appear that breakpoint
1 was hit.  Now, nops are nops, but real code could have something
else there...

/me goes back to bed.

--
Pedro Alves


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

--- a/linux-nat.c
+++ b/linux-nat.c
@@ -514,7 +514,9 @@ my_waitpid_record (int pid, int *status,
struct bp_location *bl;
struct breakpoint *b;
CORE_ADDR pc;
+ CORE_ADDR decr_pc_after_break;
struct lwp_info *lp;
+ int is_breakpoint = 1;
wait_begin:
ret = my_waitpid (pid, status, flags);
@@ -530,7 +532,7 @@ wait_begin:
if (WIFSTOPPED (*status) && WSTOPSIG (*status) == SIGTRAP)
{
- /* Check if there is a breakpoint */
+ /* Check if there is a breakpoint. */
pc = 0;
registers_changed ();
for (bl = bp_location_chain; bl; bl = bl->global_next)
@@ -602,7 +604,26 @@ wait_begin:
goto wait_begin;
}
+ is_breakpoint = 0;
+
out:
+ /* Add gdbarch_decr_pc_after_break to pc because pc will be break at address
+ add gdbarch_decr_pc_after_break when inferior non-step execute. */
+ if (is_breakpoint)
+ {
+ decr_pc_after_break = gdbarch_decr_pc_after_break
+ (get_regcache_arch (get_thread_regcache (pid_to_ptid (ret))));
+ if (decr_pc_after_break)
+ {
+ if (!pc)
+ {
+ pc = regcache_read_pc (get_thread_regcache (pid_to_ptid (ret)));
+ }
+ regcache_write_pc (get_thread_regcache (pid_to_ptid (ret)),
+ pc + decr_pc_after_break);
+ }
+ }
+
return ret;
}
--- a/record.c
+++ b/record.c
@@ -497,6 +497,33 @@ 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;
+
+ status->kind = TARGET_WAITKIND_STOPPED;
+
+ /* 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;
@@ -521,7 +548,6 @@ record_wait (ptid_t ptid, struct target_
/* Loop over the record_list, looking for the next place to
stop. */
- status->kind = TARGET_WAITKIND_STOPPED;
do
{
/* Check for beginning and end of log. */
@@ -588,10 +614,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 +654,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 +703,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]