[RFA record/replay] Eliminate global variables

[RFA record/replay] Eliminate global variables

[Michael Snyder]

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

The change to 'use_displaced_stepping' makes these
changes unnecessary.  GDB will not use displaced stepping
while recording or replaying.


[-- Attachment #2: displaced-step.txt --]
[-- Type: text/plain, Size: 3901 bytes --]

2008-10-06  Michael Snyder  <msnyder@vmware.com>

	* record.c (displaced_step_fixup): Remove.
	(record_message_cleanups): Remove displaced step handling.
	(record_message): Remove displaced step handling.
	* infrun.c (displaced_step_ptid): Back to being static.
	(displaced_step_original, displaced_step_copy): Ditto.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.300.2.9
diff -u -p -r1.300.2.9 infrun.c
--- infrun.c	6 Oct 2008 17:23:31 -0000	1.300.2.9
+++ infrun.c	6 Oct 2008 22:56:32 -0000
@@ -594,9 +594,7 @@ static ptid_t deferred_step_ptid;
 /* If this is not null_ptid, this is the thread carrying out a
    displaced single-step.  This thread's state will require fixing up
    once it has completed its step.  */
-/* Record.c(record_message) use it to confirm if the next step is displaced
-   step. */
-ptid_t displaced_step_ptid;
+static ptid_t displaced_step_ptid;
 
 struct displaced_step_request
 {
@@ -615,9 +613,7 @@ static struct gdbarch *displaced_step_gd
 static struct displaced_step_closure *displaced_step_closure;
 
 /* The address of the original instruction, and the copy we made.  */
-/* Record.c(record_message) use them to get the original PC and set it back.
-   Because record_message need to record the original PC. */
-CORE_ADDR displaced_step_original, displaced_step_copy;
+static CORE_ADDR displaced_step_original, displaced_step_copy;
 
 /* Saved contents of copy area.  */
 static gdb_byte *displaced_step_saved_copy;
Index: record.c
===================================================================
RCS file: /cvs/src/src/gdb/Attic/record.c,v
retrieving revision 1.1.2.17
diff -u -p -r1.1.2.17 record.c
--- record.c	6 Oct 2008 17:52:17 -0000	1.1.2.17
+++ record.c	6 Oct 2008 22:56:32 -0000
@@ -37,9 +37,6 @@ record_t *record_arch_list_head = NULL;
 record_t *record_arch_list_tail = NULL;
 struct regcache *record_regcache = NULL;
 
-extern void displaced_step_fixup (ptid_t event_ptid,
-				  enum target_signal signal);
-
 /* 0 ask user. 1 auto delete the last record_t.  */
 static int record_insn_max_mode = 0;
 static int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
@@ -54,8 +51,6 @@ static int record_not_record = 0;
 int record_will_store_registers = 0;
 
 extern struct bp_location *bp_location_chain;
-extern ptid_t displaced_step_ptid;
-extern CORE_ADDR displaced_step_original, displaced_step_copy;
 
 /* The real beneath function pointers.  */
 void (*record_beneath_to_resume) (ptid_t, int, enum target_signal);
@@ -326,11 +321,6 @@ static void
 record_message_cleanups (void *ignore)
 {
   record_list_release (record_arch_list_tail);
-
-  /* Clean for displaced stepping */
-  if (!ptid_equal (displaced_step_ptid, null_ptid))
-    displaced_step_fixup (displaced_step_ptid, TARGET_SIGNAL_TRAP);
-
   set_executing (inferior_ptid, 0);
   normal_stop ();
 }
@@ -349,24 +339,7 @@ record_message (struct gdbarch *gdbarch)
 
   record_regcache = get_current_regcache ();
 
-  if (!ptid_equal (displaced_step_ptid, null_ptid))
-    {
-      /* Deal with displaced stepping.  */
-      if (record_debug)
-	{
-	  fprintf_unfiltered (gdb_stdlog,
-			      "Record: this stepping is displaced stepping.  Change PC register to original address 0x%s before call gdbarch_record.  After that, change it back to 0x%s.\n",
-			      paddr_nz (displaced_step_original),
-			      paddr_nz (displaced_step_copy));
-	}
-      regcache_write_pc (record_regcache, displaced_step_original);
-      ret = gdbarch_record (gdbarch, displaced_step_copy);
-      regcache_write_pc (record_regcache, displaced_step_copy);
-    }
-  else
-    {
-      ret = gdbarch_record (gdbarch, regcache_read_pc (record_regcache));
-    }
+  ret = gdbarch_record (gdbarch, regcache_read_pc (record_regcache));
 
   if (ret > 0)
     error (_("Record: record pause the program."));

Re: [RFA record/replay] Eliminate global variables

[teawater]

If you have checked in the code that disable the displaced_stepping
when precord is enable, this patch is OK. Please check it in.

Thanks,
Hui

On Tue, Oct 7, 2008 at 09:04, Michael Snyder <msnyder@vmware.com> wrote:
> The change to 'use_displaced_stepping' makes these
> changes unnecessary.  GDB will not use displaced stepping
> while recording or replaying.
>
>
> 2008-10-06  Michael Snyder  <msnyder@vmware.com>
>
>        * record.c (displaced_step_fixup): Remove.
>        (record_message_cleanups): Remove displaced step handling.
>        (record_message): Remove displaced step handling.
>        * infrun.c (displaced_step_ptid): Back to being static.
>        (displaced_step_original, displaced_step_copy): Ditto.
>
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.300.2.9
> diff -u -p -r1.300.2.9 infrun.c
> --- infrun.c    6 Oct 2008 17:23:31 -0000       1.300.2.9
> +++ infrun.c    6 Oct 2008 22:56:32 -0000
> @@ -594,9 +594,7 @@ static ptid_t deferred_step_ptid;
>  /* If this is not null_ptid, this is the thread carrying out a
>    displaced single-step.  This thread's state will require fixing up
>    once it has completed its step.  */
> -/* Record.c(record_message) use it to confirm if the next step is displaced
> -   step. */
> -ptid_t displaced_step_ptid;
> +static ptid_t displaced_step_ptid;
>
>  struct displaced_step_request
>  {
> @@ -615,9 +613,7 @@ static struct gdbarch *displaced_step_gd
>  static struct displaced_step_closure *displaced_step_closure;
>
>  /* The address of the original instruction, and the copy we made.  */
> -/* Record.c(record_message) use them to get the original PC and set it
> back.
> -   Because record_message need to record the original PC. */
> -CORE_ADDR displaced_step_original, displaced_step_copy;
> +static CORE_ADDR displaced_step_original, displaced_step_copy;
>
>  /* Saved contents of copy area.  */
>  static gdb_byte *displaced_step_saved_copy;
> Index: record.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/Attic/record.c,v
> retrieving revision 1.1.2.17
> diff -u -p -r1.1.2.17 record.c
> --- record.c    6 Oct 2008 17:52:17 -0000       1.1.2.17
> +++ record.c    6 Oct 2008 22:56:32 -0000
> @@ -37,9 +37,6 @@ record_t *record_arch_list_head = NULL;
>  record_t *record_arch_list_tail = NULL;
>  struct regcache *record_regcache = NULL;
>
> -extern void displaced_step_fixup (ptid_t event_ptid,
> -                                 enum target_signal signal);
> -
>  /* 0 ask user. 1 auto delete the last record_t.  */
>  static int record_insn_max_mode = 0;
>  static int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
> @@ -54,8 +51,6 @@ static int record_not_record = 0;
>  int record_will_store_registers = 0;
>
>  extern struct bp_location *bp_location_chain;
> -extern ptid_t displaced_step_ptid;
> -extern CORE_ADDR displaced_step_original, displaced_step_copy;
>
>  /* The real beneath function pointers.  */
>  void (*record_beneath_to_resume) (ptid_t, int, enum target_signal);
> @@ -326,11 +321,6 @@ static void
>  record_message_cleanups (void *ignore)
>  {
>   record_list_release (record_arch_list_tail);
> -
> -  /* Clean for displaced stepping */
> -  if (!ptid_equal (displaced_step_ptid, null_ptid))
> -    displaced_step_fixup (displaced_step_ptid, TARGET_SIGNAL_TRAP);
> -
>   set_executing (inferior_ptid, 0);
>   normal_stop ();
>  }
> @@ -349,24 +339,7 @@ record_message (struct gdbarch *gdbarch)
>
>   record_regcache = get_current_regcache ();
>
> -  if (!ptid_equal (displaced_step_ptid, null_ptid))
> -    {
> -      /* Deal with displaced stepping.  */
> -      if (record_debug)
> -       {
> -         fprintf_unfiltered (gdb_stdlog,
> -                             "Record: this stepping is displaced stepping.
>  Change PC register to original address 0x%s before call gdbarch_record.
>  After that, change it back to 0x%s.\n",
> -                             paddr_nz (displaced_step_original),
> -                             paddr_nz (displaced_step_copy));
> -       }
> -      regcache_write_pc (record_regcache, displaced_step_original);
> -      ret = gdbarch_record (gdbarch, displaced_step_copy);
> -      regcache_write_pc (record_regcache, displaced_step_copy);
> -    }
> -  else
> -    {
> -      ret = gdbarch_record (gdbarch, regcache_read_pc (record_regcache));
> -    }
> +  ret = gdbarch_record (gdbarch, regcache_read_pc (record_regcache));
>
>   if (ret > 0)
>     error (_("Record: record pause the program."));
>
>

Re: [RFA record/replay] Eliminate global variables

[Michael Snyder]

Done, and thanks for the quick response!

teawater wrote:
> If you have checked in the code that disable the displaced_stepping
> when precord is enable, this patch is OK. Please check it in.
> 
> Thanks,
> Hui
> 
> On Tue, Oct 7, 2008 at 09:04, Michael Snyder <msnyder@vmware.com> wrote:
>> The change to 'use_displaced_stepping' makes these
>> changes unnecessary.  GDB will not use displaced stepping
>> while recording or replaying.
>>
>>
>> 2008-10-06  Michael Snyder  <msnyder@vmware.com>
>>
>>        * record.c (displaced_step_fixup): Remove.
>>        (record_message_cleanups): Remove displaced step handling.
>>        (record_message): Remove displaced step handling.
>>        * infrun.c (displaced_step_ptid): Back to being static.
>>        (displaced_step_original, displaced_step_copy): Ditto.
>>
>> Index: infrun.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/infrun.c,v
>> retrieving revision 1.300.2.9
>> diff -u -p -r1.300.2.9 infrun.c
>> --- infrun.c    6 Oct 2008 17:23:31 -0000       1.300.2.9
>> +++ infrun.c    6 Oct 2008 22:56:32 -0000
>> @@ -594,9 +594,7 @@ static ptid_t deferred_step_ptid;
>>  /* If this is not null_ptid, this is the thread carrying out a
>>    displaced single-step.  This thread's state will require fixing up
>>    once it has completed its step.  */
>> -/* Record.c(record_message) use it to confirm if the next step is displaced
>> -   step. */
>> -ptid_t displaced_step_ptid;
>> +static ptid_t displaced_step_ptid;
>>
>>  struct displaced_step_request
>>  {
>> @@ -615,9 +613,7 @@ static struct gdbarch *displaced_step_gd
>>  static struct displaced_step_closure *displaced_step_closure;
>>
>>  /* The address of the original instruction, and the copy we made.  */
>> -/* Record.c(record_message) use them to get the original PC and set it
>> back.
>> -   Because record_message need to record the original PC. */
>> -CORE_ADDR displaced_step_original, displaced_step_copy;
>> +static CORE_ADDR displaced_step_original, displaced_step_copy;
>>
>>  /* Saved contents of copy area.  */
>>  static gdb_byte *displaced_step_saved_copy;
>> Index: record.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/Attic/record.c,v
>> retrieving revision 1.1.2.17
>> diff -u -p -r1.1.2.17 record.c
>> --- record.c    6 Oct 2008 17:52:17 -0000       1.1.2.17
>> +++ record.c    6 Oct 2008 22:56:32 -0000
>> @@ -37,9 +37,6 @@ record_t *record_arch_list_head = NULL;
>>  record_t *record_arch_list_tail = NULL;
>>  struct regcache *record_regcache = NULL;
>>
>> -extern void displaced_step_fixup (ptid_t event_ptid,
>> -                                 enum target_signal signal);
>> -
>>  /* 0 ask user. 1 auto delete the last record_t.  */
>>  static int record_insn_max_mode = 0;
>>  static int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
>> @@ -54,8 +51,6 @@ static int record_not_record = 0;
>>  int record_will_store_registers = 0;
>>
>>  extern struct bp_location *bp_location_chain;
>> -extern ptid_t displaced_step_ptid;
>> -extern CORE_ADDR displaced_step_original, displaced_step_copy;
>>
>>  /* The real beneath function pointers.  */
>>  void (*record_beneath_to_resume) (ptid_t, int, enum target_signal);
>> @@ -326,11 +321,6 @@ static void
>>  record_message_cleanups (void *ignore)
>>  {
>>   record_list_release (record_arch_list_tail);
>> -
>> -  /* Clean for displaced stepping */
>> -  if (!ptid_equal (displaced_step_ptid, null_ptid))
>> -    displaced_step_fixup (displaced_step_ptid, TARGET_SIGNAL_TRAP);
>> -
>>   set_executing (inferior_ptid, 0);
>>   normal_stop ();
>>  }
>> @@ -349,24 +339,7 @@ record_message (struct gdbarch *gdbarch)
>>
>>   record_regcache = get_current_regcache ();
>>
>> -  if (!ptid_equal (displaced_step_ptid, null_ptid))
>> -    {
>> -      /* Deal with displaced stepping.  */
>> -      if (record_debug)
>> -       {
>> -         fprintf_unfiltered (gdb_stdlog,
>> -                             "Record: this stepping is displaced stepping.
>>  Change PC register to original address 0x%s before call gdbarch_record.
>>  After that, change it back to 0x%s.\n",
>> -                             paddr_nz (displaced_step_original),
>> -                             paddr_nz (displaced_step_copy));
>> -       }
>> -      regcache_write_pc (record_regcache, displaced_step_original);
>> -      ret = gdbarch_record (gdbarch, displaced_step_copy);
>> -      regcache_write_pc (record_regcache, displaced_step_copy);
>> -    }
>> -  else
>> -    {
>> -      ret = gdbarch_record (gdbarch, regcache_read_pc (record_regcache));
>> -    }
>> +  ret = gdbarch_record (gdbarch, regcache_read_pc (record_regcache));
>>
>>   if (ret > 0)
>>     error (_("Record: record pause the program."));
>>
>>

Re: [RFA record/replay] Eliminate global variables

[teawater]

You are welcome.

On Tue, Oct 7, 2008 at 09:16, Michael Snyder <msnyder@vmware.com> wrote:
> Done, and thanks for the quick response!
>
> teawater wrote:
>>
>> If you have checked in the code that disable the displaced_stepping
>> when precord is enable, this patch is OK. Please check it in.
>>
>> Thanks,
>> Hui
>>
>> On Tue, Oct 7, 2008 at 09:04, Michael Snyder <msnyder@vmware.com> wrote:
>>>
>>> The change to 'use_displaced_stepping' makes these
>>> changes unnecessary.  GDB will not use displaced stepping
>>> while recording or replaying.
>>>
>>>
>>> 2008-10-06  Michael Snyder  <msnyder@vmware.com>
>>>
>>>       * record.c (displaced_step_fixup): Remove.
>>>       (record_message_cleanups): Remove displaced step handling.
>>>       (record_message): Remove displaced step handling.
>>>       * infrun.c (displaced_step_ptid): Back to being static.
>>>       (displaced_step_original, displaced_step_copy): Ditto.
>>>
>>> Index: infrun.c
>>> ===================================================================
>>> RCS file: /cvs/src/src/gdb/infrun.c,v
>>> retrieving revision 1.300.2.9
>>> diff -u -p -r1.300.2.9 infrun.c
>>> --- infrun.c    6 Oct 2008 17:23:31 -0000       1.300.2.9
>>> +++ infrun.c    6 Oct 2008 22:56:32 -0000
>>> @@ -594,9 +594,7 @@ static ptid_t deferred_step_ptid;
>>>  /* If this is not null_ptid, this is the thread carrying out a
>>>   displaced single-step.  This thread's state will require fixing up
>>>   once it has completed its step.  */
>>> -/* Record.c(record_message) use it to confirm if the next step is
>>> displaced
>>> -   step. */
>>> -ptid_t displaced_step_ptid;
>>> +static ptid_t displaced_step_ptid;
>>>
>>>  struct displaced_step_request
>>>  {
>>> @@ -615,9 +613,7 @@ static struct gdbarch *displaced_step_gd
>>>  static struct displaced_step_closure *displaced_step_closure;
>>>
>>>  /* The address of the original instruction, and the copy we made.  */
>>> -/* Record.c(record_message) use them to get the original PC and set it
>>> back.
>>> -   Because record_message need to record the original PC. */
>>> -CORE_ADDR displaced_step_original, displaced_step_copy;
>>> +static CORE_ADDR displaced_step_original, displaced_step_copy;
>>>
>>>  /* Saved contents of copy area.  */
>>>  static gdb_byte *displaced_step_saved_copy;
>>> Index: record.c
>>> ===================================================================
>>> RCS file: /cvs/src/src/gdb/Attic/record.c,v
>>> retrieving revision 1.1.2.17
>>> diff -u -p -r1.1.2.17 record.c
>>> --- record.c    6 Oct 2008 17:52:17 -0000       1.1.2.17
>>> +++ record.c    6 Oct 2008 22:56:32 -0000
>>> @@ -37,9 +37,6 @@ record_t *record_arch_list_head = NULL;
>>>  record_t *record_arch_list_tail = NULL;
>>>  struct regcache *record_regcache = NULL;
>>>
>>> -extern void displaced_step_fixup (ptid_t event_ptid,
>>> -                                 enum target_signal signal);
>>> -
>>>  /* 0 ask user. 1 auto delete the last record_t.  */
>>>  static int record_insn_max_mode = 0;
>>>  static int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
>>> @@ -54,8 +51,6 @@ static int record_not_record = 0;
>>>  int record_will_store_registers = 0;
>>>
>>>  extern struct bp_location *bp_location_chain;
>>> -extern ptid_t displaced_step_ptid;
>>> -extern CORE_ADDR displaced_step_original, displaced_step_copy;
>>>
>>>  /* The real beneath function pointers.  */
>>>  void (*record_beneath_to_resume) (ptid_t, int, enum target_signal);
>>> @@ -326,11 +321,6 @@ static void
>>>  record_message_cleanups (void *ignore)
>>>  {
>>>  record_list_release (record_arch_list_tail);
>>> -
>>> -  /* Clean for displaced stepping */
>>> -  if (!ptid_equal (displaced_step_ptid, null_ptid))
>>> -    displaced_step_fixup (displaced_step_ptid, TARGET_SIGNAL_TRAP);
>>> -
>>>  set_executing (inferior_ptid, 0);
>>>  normal_stop ();
>>>  }
>>> @@ -349,24 +339,7 @@ record_message (struct gdbarch *gdbarch)
>>>
>>>  record_regcache = get_current_regcache ();
>>>
>>> -  if (!ptid_equal (displaced_step_ptid, null_ptid))
>>> -    {
>>> -      /* Deal with displaced stepping.  */
>>> -      if (record_debug)
>>> -       {
>>> -         fprintf_unfiltered (gdb_stdlog,
>>> -                             "Record: this stepping is displaced
>>> stepping.
>>>  Change PC register to original address 0x%s before call gdbarch_record.
>>>  After that, change it back to 0x%s.\n",
>>> -                             paddr_nz (displaced_step_original),
>>> -                             paddr_nz (displaced_step_copy));
>>> -       }
>>> -      regcache_write_pc (record_regcache, displaced_step_original);
>>> -      ret = gdbarch_record (gdbarch, displaced_step_copy);
>>> -      regcache_write_pc (record_regcache, displaced_step_copy);
>>> -    }
>>> -  else
>>> -    {
>>> -      ret = gdbarch_record (gdbarch, regcache_read_pc
>>> (record_regcache));
>>> -    }
>>> +  ret = gdbarch_record (gdbarch, regcache_read_pc (record_regcache));
>>>
>>>  if (ret > 0)
>>>    error (_("Record: record pause the program."));
>>>
>>>
>
>