Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers"
@ 2008-10-04  6:34 teawater
  2008-10-06  1:33 ` Michael Snyder
  0 siblings, 1 reply; 5+ messages in thread
From: teawater @ 2008-10-04  6:34 UTC (permalink / raw)
  To: gdb-patches, Michael Snyder

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

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: change_prepare_to_store_to_to_store_registers.patch --]
[-- Type: text/x-diff; name=change_prepare_to_store_to_to_store_registers.patch, Size: 5920 bytes --]

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers"
  2008-10-04  6:34 [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers" teawater
@ 2008-10-06  1:33 ` Michael Snyder
  2008-10-06  7:49   ` teawater
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Snyder @ 2008-10-06  1:33 UTC (permalink / raw)
  To: teawater; +Cc: gdb-patches

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers"
  2008-10-06  1:33 ` Michael Snyder
@ 2008-10-06  7:49   ` teawater
  2008-10-06 17:39     ` Michael Snyder
  0 siblings, 1 reply; 5+ messages in thread
From: teawater @ 2008-10-06  7:49 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches

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.

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers"
  2008-10-06  7:49   ` teawater
@ 2008-10-06 17:39     ` Michael Snyder
  2008-10-06 23:00       ` teawater
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Snyder @ 2008-10-06 17:39 UTC (permalink / raw)
  To: teawater; +Cc: gdb-patches

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers"
  2008-10-06 17:39     ` Michael Snyder
@ 2008-10-06 23:00       ` teawater
  0 siblings, 0 replies; 5+ messages in thread
From: teawater @ 2008-10-06 23:00 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches

Thanks.
And I understand the branch.

On Tue, Oct 7, 2008 at 01:37, Michael Snyder <msnyder@vmware.com> wrote:
> 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)
>>>>             {
>>>
>
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-10-06 23:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-04  6:34 [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers" teawater
2008-10-06  1:33 ` Michael Snyder
2008-10-06  7:49   ` teawater
2008-10-06 17:39     ` Michael Snyder
2008-10-06 23:00       ` teawater

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox