Re: [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers"

[Michael Snyder <msnyder@vmware.com>]

teawater wrote:
> This patch is for reverse-20080930-branch.
> Do you think I can check-in code for reverse-20080717-branch directly?
> 
> In actually, 2 branches make me puzzled.

reverse-20080930 branch is not ready for public consumption yet.
I'm using it for syncing changes and marshalling them for submission.
Let's just consider that to be my private branch until I say it's ready.

Your patch applies cleanly to reverse-20080717-branch, and
I've built and tested it there (with the corresponding change
to regcache).  This eliminates the use of a global variable
to pass information between regcache and record.c.

Sorry about the branches confusion -- allow me to check this
in for you, to get us back in sync.


> 
> On Mon, Oct 6, 2008 at 09:30, Michael Snyder <msnyder@vmware.com> wrote:
>> Good, please commit
>> (would you add the corresponding change to regcache.c too?)
>>
>> teawater wrote:
>>> Hi,
>>>
>>> To_prepare_to_store can make process record control the register
>>> change operation before set the value of regcache in function
>>> "regcache_raw_write". But it can't get the regnum.
>>> So I change it to "to_store_registers". It can get the regnum. And if
>>> record want cancel the operation the operation. It can invalidate the
>>> value with itself.
>>>
>>> 2008-10-04  Hui Zhu  <teawater@gmail.com>
>>>
>>>        Change from "to_prepare_to_store" to "to_store_registers".
>>>
>>>        * record.c (record_beneath_to_prepare_to_store): Removed.
>>>        (record_beneath_to_store_registers): New function pointer.
>>>        Instead "record_beneath_to_prepare_to_store". Will point
>>>        to the low strata target "to_store_registers" function.
>>>        (record_prepare_to_store): Removed.
>>>        (record_store_registers): New function.
>>>        Instead "record_prepare_to_store". Record the change of
>>>        registers from GDB.
>>>        (init_record_ops): Change record_prepare_to_store to
>>>        record_store_registers.
>>>        * record.h (record_beneath_to_prepare_to_store): Removed.
>>>        (record_beneath_to_store_registers): New extern.
>>>        * target.c (update_current_target): Change
>>>        record_beneath_to_prepare_to_store to
>>>        record_beneath_to_store_registers.
>>>
>>> Thanks,
>>> Hui
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> --- a/ChangeLog
>>> +++ b/ChangeLog
>>> @@ -1,3 +1,23 @@
>>> +2008-10-04  Hui Zhu  <teawater@gmail.com>
>>> +
>>> +       Change from "to_prepare_to_store" to "to_store_registers".
>>> +
>>> +       * record.c (record_beneath_to_prepare_to_store): Removed.
>>> +       (record_beneath_to_store_registers): New function pointer.
>>> +       Instead "record_beneath_to_prepare_to_store". Will point
>>> +       to the low strata target "to_store_registers" function.
>>> +       (record_prepare_to_store): Removed.
>>> +       (record_store_registers): New function.
>>> +       Instead "record_prepare_to_store". Record the change of
>>> +       registers from GDB.
>>> +       (init_record_ops): Change record_prepare_to_store to
>>> +       record_store_registers.
>>> +       * record.h (record_beneath_to_prepare_to_store): Removed.
>>> +       (record_beneath_to_store_registers): New extern.
>>> +       * target.c (update_current_target): Change
>>> +       record_beneath_to_prepare_to_store to
>>> +       record_beneath_to_store_registers.
>>> +
>>>  2008-10-02  Michael Snyder  <msnyder@vmware.com>
>>>          * reverse.c (reverse-continue): Remove a comma from docs string,
>>> --- a/record.c
>>> +++ b/record.c
>>> @@ -56,7 +56,7 @@ extern struct bp_location *bp_location_c
>>>  /* The real beneath function pointers.  */
>>>  void (*record_beneath_to_resume) (ptid_t, int, enum target_signal);
>>>  ptid_t (*record_beneath_to_wait) (ptid_t, struct target_waitstatus *);
>>> -void (*record_beneath_to_prepare_to_store) (struct regcache *);
>>> +void (*record_beneath_to_store_registers) (struct regcache *, int regno);
>>>  LONGEST (*record_beneath_to_xfer_partial) (struct target_ops * ops,
>>>                                           enum target_object object,
>>>                                           const char *annex,
>>> @@ -805,23 +805,51 @@ record_registers_change (struct regcache
>>>     }
>>>  }
>>>  -/* XXX: I don't know how to do if GDB call function
>>> target_store_registers
>>> -   without call function target_prepare_to_store.  */
>>> -
>>>  static void
>>> -record_prepare_to_store (struct regcache *regcache)
>>> +record_store_registers (struct regcache *regcache, int regno)
>>>  {
>>>   if (!record_not_record)
>>>     {
>>>       if (RECORD_IS_REPLAY)
>>>        {
>>> +         int n;
>>>          struct cleanup *old_cleanups;
>>> +
>>>          /* Let user choice if he want to write register or not.  */
>>> -         if (!nquery (_("Becuse GDB is in replay mode, changing the value
>>> of a register will destroy the record from this point forward.  Change
>>> register %s?"),
>>> -                      gdbarch_register_name (get_regcache_arch
>>> -                                             (regcache),
>>> -
>>> record_regcache_raw_write_regnum)))
>>> +         if (regno < 0)
>>> +           {
>>> +             n =
>>> +               nquery (_
>>> +                       ("Becuse GDB is in replay mode, changing the value
>>> of a register will destroy the record from this point forward. Change all
>>> register?"));
>>> +           }
>>> +         else
>>> +           {
>>> +             n =
>>> +               nquery (_
>>> +                       ("Becuse GDB is in replay mode, changing the value
>>> of a register will destroy the record from this point forward. Change
>>> register %s?"),
>>> +                       gdbarch_register_name (get_regcache_arch
>>> (regcache),
>>> +                                              regno));
>>> +           }
>>> +
>>> +         if (!n)
>>>            {
>>> +             /* Invalidate the value of regcache that set in function
>>> +                "regcache_raw_write". */
>>> +             if (regno < 0)
>>> +               {
>>> +                 int i;
>>> +                 for (i = 0;
>>> +                      i < gdbarch_num_regs (get_regcache_arch
>>> (regcache));
>>> +                      i++)
>>> +                   {
>>> +                     regcache_invalidate (regcache, i);
>>> +                   }
>>> +               }
>>> +             else
>>> +               {
>>> +                 regcache_invalidate (regcache, regno);
>>> +               }
>>> +
>>>              error (_("Record: record cancel the operation."));
>>>            }
>>>  @@ -829,9 +857,9 @@ record_prepare_to_store (struct regcache
>>>          record_list_release_next ();
>>>        }
>>>  -      record_registers_change (regcache,
>>> record_regcache_raw_write_regnum);
>>> +      record_registers_change (regcache, regno);
>>>     }
>>> -  record_beneath_to_prepare_to_store (regcache);
>>> +  record_beneath_to_store_registers (regcache, regno);
>>>  }
>>>   /* record_xfer_partial -- behavior is conditional on RECORD_IS_REPLAY.
>>> @@ -964,7 +992,7 @@ init_record_ops (void)
>>>   record_ops.to_mourn_inferior = record_mourn_inferior;
>>>   record_ops.to_kill = record_kill;
>>>   record_ops.to_create_inferior = find_default_create_inferior;        /*
>>> Make record suppport command "run".  */
>>> -  record_ops.to_prepare_to_store = record_prepare_to_store;
>>> +  record_ops.to_store_registers = record_store_registers;
>>>   record_ops.to_xfer_partial = record_xfer_partial;
>>>   record_ops.to_insert_breakpoint = record_insert_breakpoint;
>>>   record_ops.to_remove_breakpoint = record_remove_breakpoint;
>>> --- a/record.h
>>> +++ b/record.h
>>> @@ -75,7 +75,6 @@ extern struct regcache *record_regcache;
>>>   extern struct target_ops record_ops;
>>>  extern int record_resume_step;
>>> -extern int record_regcache_raw_write_regnum;
>>>  extern enum exec_direction_kind record_execdir;
>>>   extern int record_arch_list_add_reg (int num);
>>> @@ -86,7 +85,7 @@ extern void 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
>>> *);
>>> -extern void (*record_beneath_to_prepare_to_store) (struct regcache *);
>>> +extern void (*record_beneath_to_store_registers) (struct regcache *, int
>>> regno);
>>>  extern LONGEST (*record_beneath_to_xfer_partial) (struct target_ops *
>>> ops,
>>>                                                  enum target_object
>>> object,
>>>                                                  const char *annex,
>>> --- a/target.c
>>> +++ b/target.c
>>> @@ -380,7 +380,7 @@ update_current_target (void)
>>>        current_target.FIELD = (TARGET)->FIELD
>>>     record_beneath_to_resume = NULL;
>>> -  record_beneath_to_prepare_to_store = NULL;
>>> +  record_beneath_to_store_registers = NULL;
>>>   record_beneath_to_xfer_partial = NULL;
>>>   record_beneath_to_insert_breakpoint = NULL;
>>>   record_beneath_to_remove_breakpoint = NULL;
>>> @@ -485,9 +485,9 @@ update_current_target (void)
>>>              {
>>>                record_beneath_to_wait = t->to_wait;
>>>              }
>>> -           if (!record_beneath_to_prepare_to_store)
>>> +           if (!record_beneath_to_store_registers)
>>>              {
>>> -               record_beneath_to_prepare_to_store =
>>> t->to_prepare_to_store;
>>> +               record_beneath_to_store_registers = t->to_store_registers;
>>>              }
>>>            if (!record_beneath_to_xfer_partial)
>>>              {
>>