Re: [RFA] Submit process record and replay third time, 3/9

Pedro Alves
2009-03-09T19:31:16Z

On Wednesday 04 March 2009 03:41:59, teawater wrote:

> >> +# For the process record and replay target.
> >> +M:int:process_record:CORE_ADDR addr:addr
> >
> > You'll need to extend this comment a little further.  What is this
> > callback really for?  E.g., what is it supposed to do?  These things
> > should be documented here.
>
> What about the following:
> # Record a execution log of instruction at address addr.

How about something more descriptive like:

/* Parse the instruction at ADDR storing in the record execution log
the registers and memory ranges that will be affected when the
instruction executes, along with their current values. Return -1
if something goes wrong, 0 otherwise. */

> > About the interface itself, would it be possible to adjust the
> > interface to make this callback's implementations not call record.c
> > functions, but instead have record.c work only with the results of
> > this callback?
>
> Are you mean i386_process_record doesn't call the function in record.c?

Yes.

> That is so hard. A lot of this record is same for each arch. So I
> encapsulation them to be some function.
> For example, record_arch_list_add_reg and record_arch_list_add_mem.
> Another arch will need it in the future.
> So, do you think it's ok?

Let's keep it as is for now. Not much use in iterating over
this detail.

> >> +M:void:process_record_dasm:void
> >> +
> >
> > I'm puzzled by this one.  What's this for?  I can't see it being
> > used anywhere, did I miss something?  What's "dasm"?  If its not
> > used for anything yet, let's remove it for now.
> >
> In replay mode, gdb will call gdbarch_process_record_dasm to let arch
> special code analyzes the current instruction and do some replay job.
> It will make record speed up and decrease the memory use.
>
> It just support by mips arch, but mips precord code is removed now.
> So I will removed it and add it back when some arch support it.

Thanks.

> By the way, process_record_dasm is so ugly name. Do you have some idea with it?

No, sorry, because I don't know what "dasm" means.

--
Pedro Alves