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

[Michael Snyder <msnyder@vmware.com>]

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)
>               {