Re: [RFA] Fix hw watchpoints in process record.

[Hui Zhu <teawater@gmail.com>]

Thanks.

Hui

On Tue, Nov 24, 2009 at 09:58, Pedro Alves <pedro@codesourcery.com> wrote:
> On Sunday 22 November 2009 15:44:29, Pedro Alves wrote:
>> We were missing a target_stopped_data_address method in precord
> ...
>
>>         * record.c (record_beneath_to_stopped_by_watchpoint)
>>         (record_beneath_to_stopped_data_address, record_hw_watchpoint):
>>         New globals.
> ...
>>         (record_stopped_by_watchpoint): New.
>>         (record_stopped_data_address): New.
>>         (init_record_ops): Install them.
>>         (init_record_core_ops): Ditto.
>
> "Them", yeah right...  I actually managed to forget to
> install record_stopped_data_address in the version I committed.
>
>> @@ -1594,6 +1657,7 @@ init_record_ops (void)
>>    record_ops.to_xfer_partial = record_xfer_partial;
>>    record_ops.to_insert_breakpoint = record_insert_breakpoint;
>>    record_ops.to_remove_breakpoint = record_remove_breakpoint;
>> +  record_ops.to_stopped_by_watchpoint = record_stopped_by_watchpoint;
>>    record_ops.to_can_execute_reverse = record_can_execute_reverse;
>>    record_ops.to_stratum = record_stratum;
>>    /* Add bookmark target methods.  */
>> @@ -1801,6 +1865,7 @@ init_record_core_ops (void)
>>    record_core_ops.to_xfer_partial = record_core_xfer_partial;
>>    record_core_ops.to_insert_breakpoint = record_core_insert_breakpoint;
>>    record_core_ops.to_remove_breakpoint = record_core_remove_breakpoint;
>> +  record_core_ops.to_stopped_by_watchpoint = record_stopped_by_watchpoint;
>>    record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
>>    record_core_ops.to_has_execution = record_core_has_execution;
>>    record_core_ops.to_stratum = record_stratum;
>
> ...
>
> I've applied the patch below to fix it.
>
>
> Without this, if the target beneath supports reporting the
> stopped data address, there's a case where record can miss
> a watchpoint.  That is the case of stopping recording when
> stopped at a watchpoint, and then continue/step backwards
> until a different watchpoint triggers.  The stopp_data_address
> of the target beneath is called by mistake, and that may
> reports the wrong stopped_data_address, from the last time
> the target really ran.  E.g., on x86_64-linux :
>
>  >./gdb -q ./gdb
>  (top-gdb) start
>  Temporary breakpoint 3 at 0x454727: file ../../src/gdb/gdb.c, line 28.
>  Starting program: /home/pedro/gdb/baseline/build/gdb/gdb
>  [Thread debugging using libthread_db enabled]
>
>  Temporary breakpoint 3, main (argc=1, argv=0x7fffffffe3a8) at ../../src/gdb/gdb.c:28
>  28        memset (&args, 0, sizeof args);
>  (top-gdb) record
>  (top-gdb) watch args.argv
>  Hardware watchpoint 4: args.argv
>  (top-gdb) c
>  Continuing.
>  Hardware watchpoint 4: args.argv
>
>  Old value = (char **) 0x0
>  New value = (char **) 0x7fffffffe3a8
>  main (argc=1, argv=0x7fffffffe3a8) at ../../src/gdb/gdb.c:31
>  31        args.use_windows = 0;
>  (top-gdb) del
>  Delete all breakpoints? (y or n) y
>  (top-gdb) watch args.argc
>  Hardware watchpoint 5: args.argc
>  (top-gdb) reverse-continue
>  Continuing.
>
>  No more reverse-execution history.
>  main (argc=1, argv=0x7fffffffe3a8) at ../../src/gdb/gdb.c:28
>  28        memset (&args, 0, sizeof args);
>  (top-gdb)
>
> It should have triggered a watchpoint.  With the
> patch applied, the last reverse-continue does this instead:
>
>  (top-gdb) reverse-continue
>  Continuing.
>  Hardware watchpoint 5: args.argc
>
>  Old value = 1
>  New value = 0
>  0x000000000045473d in main (argc=1, argv=0x7fffffffe3a8) at ../../src/gdb/gdb.c:29
>  29        args.argc = argc;
>  (top-gdb)
>
> which is correct.
>
>
> This deserves a testcase, but I haven't written it yet.  Will do
> (unless someone else wants to, which I'd appreciate :-) ).
>
> --
> Pedro Alves
>
> 2009-11-24  Pedro Alves  <pedro@codesourcery.com>
>
>        * record.c (init_record_ops, init_record_core_ops): Actually
>        install record_stopped_data_address.
>
> ---
>  gdb/record.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> Index: src/gdb/record.c
> ===================================================================
> --- src.orig/gdb/record.c       2009-11-24 01:36:42.000000000 +0000
> +++ src/gdb/record.c    2009-11-24 01:37:01.000000000 +0000
> @@ -1668,6 +1668,7 @@ init_record_ops (void)
>   record_ops.to_insert_breakpoint = record_insert_breakpoint;
>   record_ops.to_remove_breakpoint = record_remove_breakpoint;
>   record_ops.to_stopped_by_watchpoint = record_stopped_by_watchpoint;
> +  record_ops.to_stopped_data_address = record_stopped_data_address;
>   record_ops.to_can_execute_reverse = record_can_execute_reverse;
>   record_ops.to_stratum = record_stratum;
>   /* Add bookmark target methods.  */
> @@ -1876,6 +1877,7 @@ init_record_core_ops (void)
>   record_core_ops.to_insert_breakpoint = record_core_insert_breakpoint;
>   record_core_ops.to_remove_breakpoint = record_core_remove_breakpoint;
>   record_core_ops.to_stopped_by_watchpoint = record_stopped_by_watchpoint;
> +  record_core_ops.to_stopped_data_address = record_stopped_data_address;
>   record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
>   record_core_ops.to_has_execution = record_core_has_execution;
>   record_core_ops.to_stratum = record_stratum;
>