[RFA] Resubmit process record and replay, 6/10

[RFA] Resubmit process record and replay, 6/10

[teawater]

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

This patch to add some process record and replay to infrun.c.

Code for function "use_displaced_stepping" is make sure that displaced
stepping function will disable when process record and replay target
is opened.  Because process record and replay target doesn't support
displaced stepping function.

Code for function "proceed" is call function "record_not_record_set"
to set process record and replay target doesn't record the execute
log.  Because when GDB resume the inferior, process record and replay
target doesn't need to record the memory and register store operation
of GDB.

2008-11-16  Hui Zhu  <teawater@gmail.com>

	* infrun.c (use_displaced_stepping): Return false if process
	record and replay target is used.
	(proceed): Call function "record_not_record_set" if pocess
	record and replay target is used.

 infrun.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

[-- Attachment #2: 6-infrun.txt --]
[-- Type: text/plain, Size: 1052 bytes --]

--- a/infrun.c
+++ b/infrun.c
@@ -50,6 +50,8 @@
 #include "mi/mi-common.h"
 #include "event-top.h"
 
+#include "record.h"
+
 /* Prototypes for local functions */
 
 static void signals_info (char *, int);
@@ -602,7 +604,8 @@ use_displaced_stepping (struct gdbarch *
   return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
 	    && non_stop)
 	   || can_use_displaced_stepping == can_use_displaced_stepping_on)
-	  && gdbarch_displaced_step_copy_insn_p (gdbarch));
+	  && gdbarch_displaced_step_copy_insn_p (gdbarch)
+	  && !RECORD_IS_USED);
 }
 
 /* Clean out any stray displaced stepping state.  */
@@ -1270,6 +1273,12 @@ proceed (CORE_ADDR addr, enum target_sig
   if (step < 0)
     stop_after_trap = 1;
 
+   /* When GDB resume the inferior, process record target doesn't need to
+      record the memory and register store operation of GDB. So set
+      record_not_record to 1. */
+  if (RECORD_IS_USED)
+    record_not_record_set ();
+
   if (addr == (CORE_ADDR) -1)
     {
       if (pc == stop_pc && breakpoint_here_p (pc) 

Re: [RFA] Resubmit process record and replay, 6/10

[Michael Snyder]

teawater wrote:
> This patch to add some process record and replay to infrun.c.
> 
> Code for function "use_displaced_stepping" is make sure that displaced
> stepping function will disable when process record and replay target
> is opened.  Because process record and replay target doesn't support
> displaced stepping function.
> 
> Code for function "proceed" is call function "record_not_record_set"
> to set process record and replay target doesn't record the execute
> log.  Because when GDB resume the inferior, process record and replay
> target doesn't need to record the memory and register store operation
> of GDB.

Hui, a couple of suggestions:

1) Instead of "RECORD_IS_USED", how about "TARGET_IS_PROCESS_RECORD"?

2) Instead of "record_not_record_set", how about "record_skip_recording"?


Except for the names, I think this portion of the patch
is nicely self-contained.

> 
> 2008-11-16  Hui Zhu  <teawater@gmail.com>
> 
> 	* infrun.c (use_displaced_stepping): Return false if process
> 	record and replay target is used.
> 	(proceed): Call function "record_not_record_set" if pocess
> 	record and replay target is used.
> 
>  infrun.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> 
> ------------------------------------------------------------------------
> 
> --- a/infrun.c
> +++ b/infrun.c
> @@ -50,6 +50,8 @@
>  #include "mi/mi-common.h"
>  #include "event-top.h"
>  
> +#include "record.h"
> +
>  /* Prototypes for local functions */
>  
>  static void signals_info (char *, int);
> @@ -602,7 +604,8 @@ use_displaced_stepping (struct gdbarch *
>    return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
>  	    && non_stop)
>  	   || can_use_displaced_stepping == can_use_displaced_stepping_on)
> -	  && gdbarch_displaced_step_copy_insn_p (gdbarch));
> +	  && gdbarch_displaced_step_copy_insn_p (gdbarch)
> +	  && !RECORD_IS_USED);
>  }
>  
>  /* Clean out any stray displaced stepping state.  */
> @@ -1270,6 +1273,12 @@ proceed (CORE_ADDR addr, enum target_sig
>    if (step < 0)
>      stop_after_trap = 1;
>  
> +   /* When GDB resume the inferior, process record target doesn't need to
> +      record the memory and register store operation of GDB. So set
> +      record_not_record to 1. */
> +  if (RECORD_IS_USED)
> +    record_not_record_set ();
> +
>    if (addr == (CORE_ADDR) -1)
>      {
>        if (pc == stop_pc && breakpoint_here_p (pc) 

Re: [RFA] Resubmit process record and replay, 6/10

[Pedro Alves]

On Thursday 20 November 2008 01:55:39, Michael Snyder wrote:
> 1) Instead of "RECORD_IS_USED", how about "TARGET_IS_PROCESS_RECORD"?
> 
> 2) Instead of "record_not_record_set", how about "record_skip_recording"?
> 

OOC, did you guys consider investigating replacing this a
bit further?

Instead of querying if the target is XXXX, query for target has
property(ies) YYYY or is in state WWW.

This is my knee-jerk reaction to making the core side know about a
specific target.

-- 
Pedro Alves

Re: [RFA] Resubmit process record and replay, 6/10

[Michael Snyder]

Pedro Alves wrote:
> On Thursday 20 November 2008 01:55:39, Michael Snyder wrote:
>> 1) Instead of "RECORD_IS_USED", how about "TARGET_IS_PROCESS_RECORD"?
>>
>> 2) Instead of "record_not_record_set", how about "record_skip_recording"?
>>
> 
> OOC, did you guys consider investigating replacing this a
> bit further?
> 
> Instead of querying if the target is XXXX, query for target has
> property(ies) YYYY or is in state WWW.
> 
> This is my knee-jerk reaction to making the core side know about a
> specific target.
> 

I do share your knee jerk reaction...

Maybe I "let the cat out of the bag", because it appears
that the intended meaning of the macro was "are we recording",
but the implementation is "is this the process_record target".

Re: [RFA] Resubmit process record and replay, 6/10

[Pedro Alves]

On Thursday 20 November 2008 02:09:25, Michael Snyder wrote:

> I do share your knee jerk reaction...
> 
> Maybe I "let the cat out of the bag", because it appears
> that the intended meaning of the macro was "are we recording",
> but the implementation is "is this the process_record target".

I already felt like this before regarding the RECORD_IS_USED macro,
so it was not about the name change.

Not sure if it's "are we recording" or are we "replaying", or
a mix of both.  In any case, could each call site on the common
code be replaced by a new suitable target method/property?

-- 
Pedro Alves

Re: [RFA] Resubmit process record and replay, 6/10

[teawater]

On Thu, Nov 20, 2008 at 10:14, Pedro Alves <pedro@codesourcery.com> wrote:
> On Thursday 20 November 2008 02:09:25, Michael Snyder wrote:
>
>> I do share your knee jerk reaction...
>>
>> Maybe I "let the cat out of the bag", because it appears
>> that the intended meaning of the macro was "are we recording",
>> but the implementation is "is this the process_record target".
>
> I already felt like this before regarding the RECORD_IS_USED macro,
> so it was not about the name change.
>
> Not sure if it's "are we recording" or are we "replaying", or
> a mix of both.  In any case, could each call site on the common
> code be replaced by a new suitable target method/property?

Could you give me more message on it? I am not very clear your mean.

Thanks,
Hui

Re: [RFA] Resubmit process record and replay, 6/10

[teawater]

Hi Michael,

About "record_not_record_set", It set record_not_record to let P
record doesn't record the memory and registers control behaviors of
GDB in function record_store_registers and record_xfer_partial.

So I think the name "record_not_record_set" and
"record_skip_recording" are not very clear.
Could you please give me some advices on it?

Thanks,
Hui

On Thu, Nov 20, 2008 at 09:55, Michael Snyder <msnyder@vmware.com> wrote:
> teawater wrote:
>>
>> This patch to add some process record and replay to infrun.c.
>>
>> Code for function "use_displaced_stepping" is make sure that displaced
>> stepping function will disable when process record and replay target
>> is opened.  Because process record and replay target doesn't support
>> displaced stepping function.
>>
>> Code for function "proceed" is call function "record_not_record_set"
>> to set process record and replay target doesn't record the execute
>> log.  Because when GDB resume the inferior, process record and replay
>> target doesn't need to record the memory and register store operation
>> of GDB.
>
> Hui, a couple of suggestions:
>
> 1) Instead of "RECORD_IS_USED", how about "TARGET_IS_PROCESS_RECORD"?
>
> 2) Instead of "record_not_record_set", how about "record_skip_recording"?
>
>
> Except for the names, I think this portion of the patch
> is nicely self-contained.
>
>>
>> 2008-11-16  Hui Zhu  <teawater@gmail.com>
>>
>>        * infrun.c (use_displaced_stepping): Return false if process
>>        record and replay target is used.
>>        (proceed): Call function "record_not_record_set" if pocess
>>        record and replay target is used.
>>
>>  infrun.c |   11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>>
>> ------------------------------------------------------------------------
>>
>> --- a/infrun.c
>> +++ b/infrun.c
>> @@ -50,6 +50,8 @@
>>  #include "mi/mi-common.h"
>>  #include "event-top.h"
>>  +#include "record.h"
>> +
>>  /* Prototypes for local functions */
>>   static void signals_info (char *, int);
>> @@ -602,7 +604,8 @@ use_displaced_stepping (struct gdbarch *
>>   return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
>>            && non_stop)
>>           || can_use_displaced_stepping == can_use_displaced_stepping_on)
>> -         && gdbarch_displaced_step_copy_insn_p (gdbarch));
>> +         && gdbarch_displaced_step_copy_insn_p (gdbarch)
>> +         && !RECORD_IS_USED);
>>  }
>>   /* Clean out any stray displaced stepping state.  */
>> @@ -1270,6 +1273,12 @@ proceed (CORE_ADDR addr, enum target_sig
>>   if (step < 0)
>>     stop_after_trap = 1;
>>  +   /* When GDB resume the inferior, process record target doesn't need
>> to
>> +      record the memory and register store operation of GDB. So set
>> +      record_not_record to 1. */
>> +  if (RECORD_IS_USED)
>> +    record_not_record_set ();
>> +
>>   if (addr == (CORE_ADDR) -1)
>>     {
>>       if (pc == stop_pc && breakpoint_here_p (pc)
>
>

Re: [RFA] Resubmit process record and replay, 6/10

[Michael Snyder]

teawater wrote:
> Hi Michael,
> 
> About "record_not_record_set", It set record_not_record to let P
> record doesn't record the memory and registers control behaviors of
> GDB in function record_store_registers and record_xfer_partial.
> 
> So I think the name "record_not_record_set" and
> "record_skip_recording" are not very clear.
> Could you please give me some advices on it?

Yeah, that's pretty much the way I understood it.

It sets a one-time flag that says "omit (skip) recording
registers and memory that would otherwise be recorded".

And if I understand correctly, this is to avoid adding
changes to the record log that are made by gdb when it
resumes the target.  It's only called from "proceed()".

I'm not completely clear on what those changes are.
Is gdb modifying the PC?  Or are you perhaps trying to
avoid recording breakpoints?

Is there another way to detect and avoid recording these changes?

Re: [RFA] Resubmit process record and replay, 6/10

[teawater]

On Tue, Nov 25, 2008 at 03:16, Michael Snyder <msnyder@vmware.com> wrote:
> teawater wrote:
>>
>> Hi Michael,
>>
>> About "record_not_record_set", It set record_not_record to let P
>> record doesn't record the memory and registers control behaviors of
>> GDB in function record_store_registers and record_xfer_partial.
>>
>> So I think the name "record_not_record_set" and
>> "record_skip_recording" are not very clear.
>> Could you please give me some advices on it?
>
> Yeah, that's pretty much the way I understood it.
>
> It sets a one-time flag that says "omit (skip) recording
> registers and memory that would otherwise be recorded".
>
> And if I understand correctly, this is to avoid adding
> changes to the record log that are made by gdb when it
> resumes the target.  It's only called from "proceed()".
>
> I'm not completely clear on what those changes are.
> Is gdb modifying the PC?  Or are you perhaps trying to
> avoid recording breakpoints?

I think avoid recording breakpoints is the main affect.
Another function is help deal with displaced step. Of course, P record
and displaced step will not work together now.

I think I add "record_not_record" function is because I want
record_store_registers and record_xfer_partial just record the user
level change, not for others.
What do you think about it?

>
> Is there another way to detect and avoid recording these changes?
>
>

I have no idea on it.

What about change the name to "record_skip_recording_gdb_behavior"?


Thanks,
Hui

Re: [RFA] Resubmit process record and replay, 6/10

[Michael Snyder]

teawater wrote:
> On Tue, Nov 25, 2008 at 03:16, Michael Snyder <msnyder@vmware.com> wrote:
>> teawater wrote:
>>> Hi Michael,
>>>
>>> About "record_not_record_set", It set record_not_record to let P
>>> record doesn't record the memory and registers control behaviors of
>>> GDB in function record_store_registers and record_xfer_partial.
>>>
>>> So I think the name "record_not_record_set" and
>>> "record_skip_recording" are not very clear.
>>> Could you please give me some advices on it?
>> Yeah, that's pretty much the way I understood it.
>>
>> It sets a one-time flag that says "omit (skip) recording
>> registers and memory that would otherwise be recorded".
>>
>> And if I understand correctly, this is to avoid adding
>> changes to the record log that are made by gdb when it
>> resumes the target.  It's only called from "proceed()".
>>
>> I'm not completely clear on what those changes are.
>> Is gdb modifying the PC?  Or are you perhaps trying to
>> avoid recording breakpoints?
> 
> I think avoid recording breakpoints is the main affect.
> Another function is help deal with displaced step. Of course, P record
> and displaced step will not work together now.
> 
> I think I add "record_not_record" function is because I want
> record_store_registers and record_xfer_partial just record the user
> level change, not for others.
> What do you think about it?

OK, so if we ignore displaced stepping for now, then can we
limit the issue to breakpoints?

Breakpoint writes will all pass through functions called
memory_insert_breakpoint and memory_remove_breakpoint (mem-break.c).

So what we want to do is get the information from there into
record.c.  I guess you could do pretty much what you are doing
now, only call the access function from mem-break.c instead of
from infrun.  It would help to localize it and make its meaning
clear.

Maybe call it "dont_record_memory_breakpoint" or something like that.

Re: [RFA] Resubmit process record and replay, 6/10

[Pedro Alves]

On Monday 24 November 2008 03:23:06, teawater wrote:
> > Not sure if it's "are we recording" or are we "replaying", or
> > a mix of both.  In any case, could each call site on the common
> > code be replaced by a new suitable target method/property?
> 
> Could you give me more message on it? I am not very clear your mean.

What is the property or state that you want to check
here?  You should export that through the target_ops interface,
instead of making infrun.c tied to a record.c and
the record target.

Currently, GDB only distinguises reverse and forward
execution.  Does it also need to know that replaying is a
special case of forward execution?

Perhaps you want to check if the current
target is replaying?

 target_is_replaying()

?

Note that this would be a proper target_ops method, not
a reference record_ops, like in your current macro.

But, why do you need to protect `proceed' with the record
target, while reverse/replay debugging against sid or WMware
or Virtutech didn't need it?  If they also need it, or will
need it, what's the check that GDB should do to prevent
the bad writes from happening in those targets too?

And, why only in `proceed'?

Figuring this out, and knowing *exactly* what is it that this
check is protecting against will let us know if there's some
other better way.  Plain ignoring writes may or may not be
the right thing to do here.  Can you show an example of what
you're protecting against?

E.g., should you instead prohibit the 'jump ADDR' command at a
higher layer when replaying or executing backwards?

-- 
Pedro Alves

Re: [RFA] Resubmit process record and replay, 6/10

[teawater]

Sorry I forget a big part that need it.
When GDB work in replay mode, P record will set regs and memory in
record_wait. All of them can't be record.

So what about set not_record flag to record_wait in replay mode,
record_insert_breakpoint and record_remove_breakpoint.

And about the name of this flag, do you have some idea on it?


Thanks,
Hui


On Wed, Nov 26, 2008 at 02:22, Michael Snyder <msnyder@vmware.com> wrote:
> teawater wrote:
>>
>> On Tue, Nov 25, 2008 at 03:16, Michael Snyder <msnyder@vmware.com> wrote:
>>>
>>> teawater wrote:
>>>>
>>>> Hi Michael,
>>>>
>>>> About "record_not_record_set", It set record_not_record to let P
>>>> record doesn't record the memory and registers control behaviors of
>>>> GDB in function record_store_registers and record_xfer_partial.
>>>>
>>>> So I think the name "record_not_record_set" and
>>>> "record_skip_recording" are not very clear.
>>>> Could you please give me some advices on it?
>>>
>>> Yeah, that's pretty much the way I understood it.
>>>
>>> It sets a one-time flag that says "omit (skip) recording
>>> registers and memory that would otherwise be recorded".
>>>
>>> And if I understand correctly, this is to avoid adding
>>> changes to the record log that are made by gdb when it
>>> resumes the target.  It's only called from "proceed()".
>>>
>>> I'm not completely clear on what those changes are.
>>> Is gdb modifying the PC?  Or are you perhaps trying to
>>> avoid recording breakpoints?
>>
>> I think avoid recording breakpoints is the main affect.
>> Another function is help deal with displaced step. Of course, P record
>> and displaced step will not work together now.
>>
>> I think I add "record_not_record" function is because I want
>> record_store_registers and record_xfer_partial just record the user
>> level change, not for others.
>> What do you think about it?
>
> OK, so if we ignore displaced stepping for now, then can we
> limit the issue to breakpoints?
>
> Breakpoint writes will all pass through functions called
> memory_insert_breakpoint and memory_remove_breakpoint (mem-break.c).
>
> So what we want to do is get the information from there into
> record.c.  I guess you could do pretty much what you are doing
> now, only call the access function from mem-break.c instead of
> from infrun.  It would help to localize it and make its meaning
> clear.
>
> Maybe call it "dont_record_memory_breakpoint" or something like that.
>
>

Re: [RFA] Resubmit process record and replay, 6/10

[teawater]

About RECORD_IS_USED, it's mean that record target is used by  GDB.
#define RECORD_IS_USED   \
     (current_target.beneath == &record_ops)


About "record_not_record_set", I told it with Muhael in another
thread. Maybe it will be moved to record_wait in replay mode,
record_insert_breakpoint and record_remove_breakpoint.


Thanks,
Hui


On Wed, Nov 26, 2008 at 07:09, Pedro Alves <pedro@codesourcery.com> wrote:
> On Monday 24 November 2008 03:23:06, teawater wrote:
>> > Not sure if it's "are we recording" or are we "replaying", or
>> > a mix of both.  In any case, could each call site on the common
>> > code be replaced by a new suitable target method/property?
>>
>> Could you give me more message on it? I am not very clear your mean.
>
> What is the property or state that you want to check
> here?  You should export that through the target_ops interface,
> instead of making infrun.c tied to a record.c and
> the record target.
>
> Currently, GDB only distinguises reverse and forward
> execution.  Does it also need to know that replaying is a
> special case of forward execution?
>
> Perhaps you want to check if the current
> target is replaying?
>
>  target_is_replaying()
>
> ?
>
> Note that this would be a proper target_ops method, not
> a reference record_ops, like in your current macro.
>
> But, why do you need to protect `proceed' with the record
> target, while reverse/replay debugging against sid or WMware
> or Virtutech didn't need it?  If they also need it, or will
> need it, what's the check that GDB should do to prevent
> the bad writes from happening in those targets too?
>
> And, why only in `proceed'?
>
> Figuring this out, and knowing *exactly* what is it that this
> check is protecting against will let us know if there's some
> other better way.  Plain ignoring writes may or may not be
> the right thing to do here.  Can you show an example of what
> you're protecting against?
>
> E.g., should you instead prohibit the 'jump ADDR' command at a
> higher layer when replaying or executing backwards?
>
> --
> Pedro Alves
>

Re: [RFA] Resubmit process record and replay, 6/10

[teawater]

Hi Michael,

Could you help me in
http://sourceware.org/ml/gdb-patches/2008-11/msg00695.html

Thanks,
Hui

On Wed, Nov 26, 2008 at 10:33, teawater <teawater@gmail.com> wrote:
> Sorry I forget a big part that need it.
> When GDB work in replay mode, P record will set regs and memory in
> record_wait. All of them can't be record.
>
> So what about set not_record flag to record_wait in replay mode,
> record_insert_breakpoint and record_remove_breakpoint.
>
> And about the name of this flag, do you have some idea on it?
>
>
> Thanks,
> Hui
>
>
> On Wed, Nov 26, 2008 at 02:22, Michael Snyder <msnyder@vmware.com> wrote:
>> teawater wrote:
>>>
>>> On Tue, Nov 25, 2008 at 03:16, Michael Snyder <msnyder@vmware.com> wrote:
>>>>
>>>> teawater wrote:
>>>>>
>>>>> Hi Michael,
>>>>>
>>>>> About "record_not_record_set", It set record_not_record to let P
>>>>> record doesn't record the memory and registers control behaviors of
>>>>> GDB in function record_store_registers and record_xfer_partial.
>>>>>
>>>>> So I think the name "record_not_record_set" and
>>>>> "record_skip_recording" are not very clear.
>>>>> Could you please give me some advices on it?
>>>>
>>>> Yeah, that's pretty much the way I understood it.
>>>>
>>>> It sets a one-time flag that says "omit (skip) recording
>>>> registers and memory that would otherwise be recorded".
>>>>
>>>> And if I understand correctly, this is to avoid adding
>>>> changes to the record log that are made by gdb when it
>>>> resumes the target.  It's only called from "proceed()".
>>>>
>>>> I'm not completely clear on what those changes are.
>>>> Is gdb modifying the PC?  Or are you perhaps trying to
>>>> avoid recording breakpoints?
>>>
>>> I think avoid recording breakpoints is the main affect.
>>> Another function is help deal with displaced step. Of course, P record
>>> and displaced step will not work together now.
>>>
>>> I think I add "record_not_record" function is because I want
>>> record_store_registers and record_xfer_partial just record the user
>>> level change, not for others.
>>> What do you think about it?
>>
>> OK, so if we ignore displaced stepping for now, then can we
>> limit the issue to breakpoints?
>>
>> Breakpoint writes will all pass through functions called
>> memory_insert_breakpoint and memory_remove_breakpoint (mem-break.c).
>>
>> So what we want to do is get the information from there into
>> record.c.  I guess you could do pretty much what you are doing
>> now, only call the access function from mem-break.c instead of
>> from infrun.  It would help to localize it and make its meaning
>> clear.
>>
>> Maybe call it "dont_record_memory_breakpoint" or something like that.
>>
>>
>

Re: [RFA] Resubmit process record and replay, 6/10

[teawater]

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

Hi,

I make a tmp patch to make this idea clear.  It test with testsuite is OK.

And I still didn't change "record_not_record_set" cause I didn't have
idea on it.

Thanks,
Hui




On Wed, Nov 26, 2008 at 10:33, teawater <teawater@gmail.com> wrote:
> Sorry I forget a big part that need it.
> When GDB work in replay mode, P record will set regs and memory in
> record_wait. All of them can't be record.
>
> So what about set not_record flag to record_wait in replay mode,
> record_insert_breakpoint and record_remove_breakpoint.
>
> And about the name of this flag, do you have some idea on it?
>
>
> Thanks,
> Hui
>
>
> On Wed, Nov 26, 2008 at 02:22, Michael Snyder <msnyder@vmware.com> wrote:
>> teawater wrote:
>>>
>>> On Tue, Nov 25, 2008 at 03:16, Michael Snyder <msnyder@vmware.com> wrote:
>>>>
>>>> teawater wrote:
>>>>>
>>>>> Hi Michael,
>>>>>
>>>>> About "record_not_record_set", It set record_not_record to let P
>>>>> record doesn't record the memory and registers control behaviors of
>>>>> GDB in function record_store_registers and record_xfer_partial.
>>>>>
>>>>> So I think the name "record_not_record_set" and
>>>>> "record_skip_recording" are not very clear.
>>>>> Could you please give me some advices on it?
>>>>
>>>> Yeah, that's pretty much the way I understood it.
>>>>
>>>> It sets a one-time flag that says "omit (skip) recording
>>>> registers and memory that would otherwise be recorded".
>>>>
>>>> And if I understand correctly, this is to avoid adding
>>>> changes to the record log that are made by gdb when it
>>>> resumes the target.  It's only called from "proceed()".
>>>>
>>>> I'm not completely clear on what those changes are.
>>>> Is gdb modifying the PC?  Or are you perhaps trying to
>>>> avoid recording breakpoints?
>>>
>>> I think avoid recording breakpoints is the main affect.
>>> Another function is help deal with displaced step. Of course, P record
>>> and displaced step will not work together now.
>>>
>>> I think I add "record_not_record" function is because I want
>>> record_store_registers and record_xfer_partial just record the user
>>> level change, not for others.
>>> What do you think about it?
>>
>> OK, so if we ignore displaced stepping for now, then can we
>> limit the issue to breakpoints?
>>
>> Breakpoint writes will all pass through functions called
>> memory_insert_breakpoint and memory_remove_breakpoint (mem-break.c).
>>
>> So what we want to do is get the information from there into
>> record.c.  I guess you could do pretty much what you are doing
>> now, only call the access function from mem-break.c instead of
>> from infrun.  It would help to localize it and make its meaning
>> clear.
>>
>> Maybe call it "dont_record_memory_breakpoint" or something like that.
>>
>>
>

[-- Attachment #2: tmp-skip.txt --]
[-- Type: text/plain, Size: 4976 bytes --]

Index: gdb/infrun.c
===================================================================
--- gdb.orig/infrun.c	2008-12-11 10:57:22.000000000 +0800
+++ gdb/infrun.c	2008-12-11 11:03:14.000000000 +0800
@@ -607,7 +607,7 @@
 	    && non_stop)
 	   || can_use_displaced_stepping == can_use_displaced_stepping_on)
 	  && gdbarch_displaced_step_copy_insn_p (gdbarch)
-	  && !RECORD_IS_USED);
+	  && !TARGET_IS_PROCESS_RECORD);
 }
 
 /* Clean out any stray displaced stepping state.  */
@@ -1312,12 +1312,6 @@
   if (step < 0)
     stop_after_trap = 1;
 
-   /* When GDB resume the inferior, process record target doesn't need to
-      record the memory and register store operation of GDB. So set
-      record_not_record to 1. */
-  if (RECORD_IS_USED)
-    record_not_record_set ();
-
   if (addr == (CORE_ADDR) -1)
     {
       if (pc == stop_pc && breakpoint_here_p (pc) 
@@ -2050,6 +2044,10 @@
   if (software_breakpoint_inserted_here_p (breakpoint_pc)
       || (non_stop && moribund_breakpoint_here_p (breakpoint_pc)))
     {
+      struct cleanup *old_cleanups;
+      if (TARGET_IS_PROCESS_RECORD)
+	old_cleanups = record_not_record_set ();
+
       /* When using hardware single-step, a SIGTRAP is reported for both
 	 a completed single-step and a software breakpoint.  Need to
 	 differentiate between the two, as the latter needs adjusting
@@ -2073,6 +2071,9 @@
 	  || !currently_stepping (ecs->event_thread)
 	  || ecs->event_thread->prev_pc == breakpoint_pc)
 	regcache_write_pc (regcache, breakpoint_pc);
+
+      if (TARGET_IS_PROCESS_RECORD)
+	do_cleanups (old_cleanups);
     }
 }
 
Index: gdb/record.c
===================================================================
--- gdb.orig/record.c	2008-12-11 10:57:22.000000000 +0800
+++ gdb/record.c	2008-12-11 11:03:56.000000000 +0800
@@ -173,6 +173,13 @@
 static void
 record_arch_list_add (record_t * rec)
 {
+  if (record_debug > 1)
+    {
+      fprintf_unfiltered (gdb_stdlog,
+			  "Process record: record_arch_list_add 0x%s.\n",
+			  paddr_nz ((CORE_ADDR)rec));
+    }
+
   if (record_arch_list_tail)
     {
       record_arch_list_tail->next = rec;
@@ -372,11 +379,13 @@
   record_not_record = 0;
 }
 
-void
+struct cleanup *
 record_not_record_set (void)
 {
   struct cleanup *old_cleanups = make_cleanup (record_not_record_cleanups, 0);
   record_not_record = 1;
+
+  return old_cleanups;
 }
 
 static void
@@ -407,7 +416,7 @@
     }
 
   /* Check if record target is already running */
-  if (RECORD_IS_USED)
+  if (TARGET_IS_PROCESS_RECORD)
     {
       if (!nquery
 	  (_("Process record target already running, do you want to delete the old record log?")))
@@ -483,6 +492,8 @@
 static ptid_t
 record_wait (ptid_t ptid, struct target_waitstatus *status)
 {
+  struct cleanup *set_cleanups = record_not_record_set ();
+
   if (record_debug)
     {
       fprintf_unfiltered (gdb_stdlog,
@@ -835,6 +846,7 @@
       discard_cleanups (old_cleanups);
     }
 
+  do_cleanups (set_cleanups);
   return inferior_ptid;
 }
 
@@ -1073,7 +1085,12 @@
 {
   if (!RECORD_IS_REPLAY)
     {
-      return record_beneath_to_insert_breakpoint (bp_tgt);
+      struct cleanup *old_cleanups = record_not_record_set ();
+      int ret = record_beneath_to_insert_breakpoint (bp_tgt);
+
+      do_cleanups (old_cleanups);
+
+      return ret;
     }
 
   return 0;
@@ -1084,7 +1101,12 @@
 {
   if (!RECORD_IS_REPLAY)
     {
-      return record_beneath_to_remove_breakpoint (bp_tgt);
+      struct cleanup *old_cleanups = record_not_record_set ();
+      int ret = record_beneath_to_remove_breakpoint (bp_tgt);
+
+      do_cleanups (old_cleanups);
+
+      return ret;
     }
 
   return 0;
@@ -1142,7 +1164,7 @@
 static void
 cmd_record_delete (char *args, int from_tty)
 {
-  if (RECORD_IS_USED)
+  if (TARGET_IS_PROCESS_RECORD)
     {
       if (RECORD_IS_REPLAY)
 	{
@@ -1168,7 +1190,7 @@
 static void
 cmd_record_stop (char *args, int from_tty)
 {
-  if (RECORD_IS_USED)
+  if (TARGET_IS_PROCESS_RECORD)
     {
       if (!record_list || !from_tty || query (_("Delete recorded log and stop recording?")))
 	{
Index: gdb/record.h
===================================================================
--- gdb.orig/record.h	2008-12-11 10:57:22.000000000 +0800
+++ gdb/record.h	2008-12-11 11:03:33.000000000 +0800
@@ -20,7 +20,7 @@
 #ifndef _RECORD_H_
 #define _RECORD_H_
 
-#define RECORD_IS_USED   \
+#define TARGET_IS_PROCESS_RECORD   \
      (current_target.beneath == &record_ops)
 #define RECORD_IS_REPLAY \
      (record_list->next || execution_direction == EXEC_REVERSE)
@@ -80,7 +80,7 @@
 extern int record_arch_list_add_mem (CORE_ADDR addr, int len);
 extern int record_arch_list_add_end (int need_dasm);
 extern void record_message (struct gdbarch *gdbarch);
-extern void record_not_record_set (void);
+extern struct cleanup * record_not_record_set (void);
 
 extern void (*record_beneath_to_resume) (ptid_t, int, enum target_signal);
 extern ptid_t (*record_beneath_to_wait) (ptid_t, struct target_waitstatus *);

Re: [RFA] Resubmit process record and replay, 6/10

[teawater]

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

This is the new version patch.

On Sun, Nov 16, 2008 at 16:22, teawater <teawater@gmail.com> wrote:
> This patch to add some process record and replay to infrun.c.
>
> Code for function "use_displaced_stepping" is make sure that displaced
> stepping function will disable when process record and replay target
> is opened.  Because process record and replay target doesn't support
> displaced stepping function.
>
> Code for function "proceed" is call function "record_not_record_set"
> to set process record and replay target doesn't record the execute
> log.  Because when GDB resume the inferior, process record and replay
> target doesn't need to record the memory and register store operation
> of GDB.
>
> 2008-11-16  Hui Zhu  <teawater@gmail.com>
>
>        * infrun.c (use_displaced_stepping): Return false if process
>        record and replay target is used.
>        (proceed): Call function "record_not_record_set" if pocess
>        record and replay target is used.
>
>  infrun.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>

[-- Attachment #2: 6-infrun.txt --]
[-- Type: text/plain, Size: 970 bytes --]

--- a/infrun.c
+++ b/infrun.c
@@ -50,6 +50,8 @@
 #include "mi/mi-common.h"
 #include "event-top.h"
 
+#include "record.h"
+
 /* Prototypes for local functions */
 
 static void signals_info (char *, int);
@@ -604,7 +606,8 @@
   return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
 	    && non_stop)
 	   || can_use_displaced_stepping == can_use_displaced_stepping_on)
-	  && gdbarch_displaced_step_copy_insn_p (gdbarch));
+	  && gdbarch_displaced_step_copy_insn_p (gdbarch)
+	  && !RECORD_IS_USED);
 }
 
 /* Clean out any stray displaced stepping state.  */
@@ -1309,6 +1312,12 @@
   if (step < 0)
     stop_after_trap = 1;
 
+   /* When GDB resume the inferior, process record target doesn't need to
+      record the memory and register store operation of GDB. So set
+      record_not_record to 1. */
+  if (RECORD_IS_USED)
+    record_not_record_set ();
+
   if (addr == (CORE_ADDR) -1)
     {
       if (pc == stop_pc && breakpoint_here_p (pc)