[RFA] Displaced stepping just enable in non-stop mode

[RFA] Displaced stepping just enable in non-stop mode

[teawater]

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

Hi,

After communicated in http://sourceware.org/ml/gdb/2008-10/msg00019.html.
I make a patch make displaced stepping just enable in non-stop mode.

2008-10-07  Hui Zhu  <teawater@gmail.com>

	* infrun.c (use_displaced_stepping): Displaced stepping just
	enable in non-stop mode.

Thanks,
Hui

[-- Attachment #2: displaced_step_non_step.txt --]
[-- Type: text/plain, Size: 697 bytes --]

--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2008-10-07  Hui Zhu  <teawater@gmail.com>
+
+	* infrun.c (use_displaced_stepping): Displaced stepping just
+	enable in non-stop mode.
+
 2008-10-06  Doug Evans  <dje@google.com>
 
 	* dwarf2read.c (dwarf2_die_debug): New static global.
--- a/infrun.c
+++ b/infrun.c
@@ -568,11 +568,12 @@ Debugger's willingness to use displaced 
 }
 
 /* Return non-zero if displaced stepping is enabled, and can be used
-   with GDBARCH.  */
+   with GDBARCH, and in non-stop mode.  */
 static int
 use_displaced_stepping (struct gdbarch *gdbarch)
 {
   return (can_use_displaced_stepping
+	  && non_stop
 	  && gdbarch_displaced_step_copy_insn_p (gdbarch));
 }
 

Re: [RFA] Displaced stepping just enable in non-stop mode

[Joel Brobecker]

> 2008-10-07  Hui Zhu  <teawater@gmail.com>
> 
> 	* infrun.c (use_displaced_stepping): Displaced stepping just
> 	enable in non-stop mode.

There is still a piece missing. Namely, allow the user to force
displaced-stepping in a mode that is not non-stop.  Pedro suggested that
we either change the maint command, or add a new one. How about changing
the maint command into a mainstream command that's a tri-state:

  set use-displaced-stepping (auto|on|off)

By default, it's "auto" which means that the debugger tries to determine
from the situation whether it's necessary or not (basically if non-stop).
But the user can then override the decision either way.

-- 
Joel

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

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

Hi Joel,

According to your mail, I make a new patch.

2008-10-08  Hui Zhu  <teawater@gmail.com>

	Displaced stepping can be set to
	auto (enable in non-stop mode), on, off.

	* infrun.c (can_use_displaced_stepping): Change type to const
	char pointer.
	(can_use_displaced_stepping_auto): New string.
	(can_use_displaced_stepping_on): New string.
	(can_use_displaced_stepping_off): New string.
	(can_use_displaced_stepping_enum): New array.
	(use_displaced_stepping): Return non-zero if displaced stepping
 	is auto, and can be used with GDBARCH, and in non-stop mode.
	Return non-zero if displaced stepping is on, and can be used
	with GDBARCH.
	(_initialize_infrun): "can-use-displaced-stepping" function
	change to add_setshow_enum_cmd.

Thanks,
Hui

On Tue, Oct 7, 2008 at 20:17, Joel Brobecker <brobecker@adacore.com> wrote:
>> 2008-10-07  Hui Zhu  <teawater@gmail.com>
>>
>>       * infrun.c (use_displaced_stepping): Displaced stepping just
>>       enable in non-stop mode.
>
> There is still a piece missing. Namely, allow the user to force
> displaced-stepping in a mode that is not non-stop.  Pedro suggested that
> we either change the maint command, or add a new one. How about changing
> the maint command into a mainstream command that's a tri-state:
>
>  set use-displaced-stepping (auto|on|off)
>
> By default, it's "auto" which means that the debugger tries to determine
> from the situation whether it's necessary or not (basically if non-stop).
> But the user can then override the decision either way.
>
> --
> Joel
>

[-- Attachment #2: displaced_step_non_step.txt --]
[-- Type: text/plain, Size: 3596 bytes --]

--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2008-10-08  Hui Zhu  <teawater@gmail.com>
+
+	Displaced stepping can be set to
+	auto (enable in non-stop mode), on, off.
+
+	* infrun.c (can_use_displaced_stepping): Change type to const
+	char pointer.
+	(can_use_displaced_stepping_auto): New string.
+	(can_use_displaced_stepping_on): New string.
+	(can_use_displaced_stepping_off): New string.
+	(can_use_displaced_stepping_enum): New array.
+	(use_displaced_stepping): Return non-zero if displaced stepping
+ 	is auto, and can be used with GDBARCH, and in non-stop mode.
+	Return non-zero if displaced stepping is on, and can be used
+	with GDBARCH.
+	(_initialize_infrun): "can-use-displaced-stepping" function
+	change to add_setshow_enum_cmd.
+
 2008-10-06  Doug Evans  <dje@google.com>
 
 	* dwarf2read.c (dwarf2_die_debug): New static global.
--- a/infrun.c
+++ b/infrun.c
@@ -553,10 +553,22 @@ static CORE_ADDR displaced_step_original
 /* Saved contents of copy area.  */
 static gdb_byte *displaced_step_saved_copy;
 
-/* When this is non-zero, we are allowed to use displaced stepping, if
-   the architecture supports it.  When this is zero, we use
+/* When this is auto, we are allowed to use displaced stepping,
+   If the architecture supports it and in non-stop mode.
+   When this is on, we are allowed to use displaced stepping, if
+   the architecture supports it.  When this is off, we use
    traditional the hold-and-step approach.  */
-int can_use_displaced_stepping = 1;
+const char can_use_displaced_stepping_auto[] = "auto";
+const char can_use_displaced_stepping_on[] = "on";
+const char can_use_displaced_stepping_off[] = "off";
+const char *can_use_displaced_stepping_enum[] =
+{
+  can_use_displaced_stepping_auto,
+  can_use_displaced_stepping_on,
+  can_use_displaced_stepping_off,
+  NULL,
+};
+const char *can_use_displaced_stepping = can_use_displaced_stepping_auto;
 static void
 show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
 				 struct cmd_list_element *c,
@@ -567,12 +579,16 @@ Debugger's willingness to use displaced 
 "breakpoints is %s.\n"), value);
 }
 
-/* Return non-zero if displaced stepping is enabled, and can be used
+/* Return non-zero if displaced stepping is auto, and can be used
+   with GDBARCH, and in non-stop mode.
+   Return non-zero if displaced stepping is on, and can be used
    with GDBARCH.  */
 static int
 use_displaced_stepping (struct gdbarch *gdbarch)
 {
-  return (can_use_displaced_stepping
+  return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
+	    && non_stop)
+	   || can_use_displaced_stepping == can_use_displaced_stepping_on)
 	  && gdbarch_displaced_step_copy_insn_p (gdbarch));
 }
 
@@ -4857,11 +4873,14 @@ function is skipped and the step command
 			   show_step_stop_if_no_debug,
 			   &setlist, &showlist);
 
-  add_setshow_boolean_cmd ("can-use-displaced-stepping", class_maintenance,
+  add_setshow_enum_cmd ("can-use-displaced-stepping", class_maintenance,
+			   can_use_displaced_stepping_enum,
 			   &can_use_displaced_stepping, _("\
 Set debugger's willingness to use displaced stepping."), _("\
 Show debugger's willingness to use displaced stepping."), _("\
-If zero, gdb will not use displaced stepping to step over\n\
+If auto, gdb will auto use displaced stepping if it need (in non-stop mode).\n\
+If on, gdb will use displaced stepping if such is supported by the target.\n\
+If off, gdb will not use displaced stepping to step over\n\
 breakpoints, even if such is supported by the target."),
 			   NULL,
 			   show_can_use_displaced_stepping,

Re: [RFA] Displaced stepping just enable in non-stop mode

[Pedro Alves]

Hi,

Thanks for doing this.

On Wednesday 08 October 2008 07:10:43, teawater wrote:

> -int can_use_displaced_stepping = 1;
> +const char can_use_displaced_stepping_auto[] = "auto";
> +const char can_use_displaced_stepping_on[] = "on";
> +const char can_use_displaced_stepping_off[] = "off";
> +const char *can_use_displaced_stepping_enum[] =
> +{
> +  can_use_displaced_stepping_auto,
> +  can_use_displaced_stepping_on,
> +  can_use_displaced_stepping_off,
> +  NULL,
> +};
> +const char *can_use_displaced_stepping = can_use_displaced_stepping_auto;

Could you make these static while you're at it, please?  It was just missed
in the old boolean setting.

>  static void
>  show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
>                                  struct cmd_list_element *c,
> @@ -567,12 +579,16 @@ Debugger's willingness to use displaced 
>  "breakpoints is %s.\n"), value);

Could you update this function to show what effect the auto setting
is currently having?  See breakpoint.c:show_always_inserted_mode for
an example.

>  }
>  
> -/* Return non-zero if displaced stepping is enabled, and can be used
> +/* Return non-zero if displaced stepping is auto, and can be used
> +   with GDBARCH, and in non-stop mode.
> +   Return non-zero if displaced stepping is on, and can be used
>     with GDBARCH.  */
>  static int
>  use_displaced_stepping (struct gdbarch *gdbarch)
>  {
> -  return (can_use_displaced_stepping
> +  return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
> +           && non_stop)
> +          || can_use_displaced_stepping == can_use_displaced_stepping_on)
>           && gdbarch_displaced_step_copy_insn_p (gdbarch));
>  }
>  
> @@ -4857,11 +4873,14 @@ function is skipped and the step command
>                            show_step_stop_if_no_debug,
>                            &setlist, &showlist);
>  
> -  add_setshow_boolean_cmd ("can-use-displaced-stepping", class_maintenance,
> +  add_setshow_enum_cmd ("can-use-displaced-stepping", class_maintenance,
> +                          can_use_displaced_stepping_enum,
>                            &can_use_displaced_stepping, _("\
>  Set debugger's willingness to use displaced stepping."), _("\
>  Show debugger's willingness to use displaced stepping."), _("\
> -If zero, gdb will not use displaced stepping to step over\n\
> +If auto, gdb will auto use displaced stepping if it need (in non-stop mode).\n\
> +If on, gdb will use displaced stepping if such is supported by the target.\n\
> +If off, gdb will not use displaced stepping to step over\n\
>  breakpoints, even if such is supported by the target."),

This also needs to be updated in the manual.  I'd suggest borrowing the text
from breakpoint.c's "always-inserted" command description to make it a
bit more descriptive, and mention which mode is the default.  Should mostly
be a matter of copy-paste, I think.

-- 
Pedro Alves

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

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

Hi Pedro,

Thanks for your example, I make a new patch for infrun.c and other for manual.

2008-10-10  Hui Zhu  <teawater@gmail.com>

	Displaced stepping can be set to
	auto (enable in non-stop mode), on, off.

	* infrun.c (can_use_displaced_stepping): Change type to const
	char pointer.
	(can_use_displaced_stepping_auto): New string.
	(can_use_displaced_stepping_on): New string.
	(can_use_displaced_stepping_off): New string.
	(can_use_displaced_stepping_enum): New array.
	(show_can_use_displaced_stepping): In auto mode, also show the
	current effect of the option.
	(use_displaced_stepping): Return non-zero if displaced stepping
 	is auto, and can be used with GDBARCH, and in non-stop mode.
	Return non-zero if displaced stepping is on, and can be used
	with GDBARCH.
	(_initialize_infrun): "can-use-displaced-stepping" function
	change to add_setshow_enum_cmd.

2008-10-10  Hui Zhu  <teawater@gmail.com>

	* gdb.texinfo (can-use-displaced-stepping) Describe the auto mode
	setting, and make it the default.


Thanks,
Hui

On Thu, Oct 9, 2008 at 22:49, Pedro Alves <pedro@codesourcery.com> wrote:
> Hi,
>
> Thanks for doing this.
>
> On Wednesday 08 October 2008 07:10:43, teawater wrote:
>
>> -int can_use_displaced_stepping = 1;
>> +const char can_use_displaced_stepping_auto[] = "auto";
>> +const char can_use_displaced_stepping_on[] = "on";
>> +const char can_use_displaced_stepping_off[] = "off";
>> +const char *can_use_displaced_stepping_enum[] =
>> +{
>> +  can_use_displaced_stepping_auto,
>> +  can_use_displaced_stepping_on,
>> +  can_use_displaced_stepping_off,
>> +  NULL,
>> +};
>> +const char *can_use_displaced_stepping = can_use_displaced_stepping_auto;
>
> Could you make these static while you're at it, please?  It was just missed
> in the old boolean setting.
>
>>  static void
>>  show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
>>                                  struct cmd_list_element *c,
>> @@ -567,12 +579,16 @@ Debugger's willingness to use displaced
>>  "breakpoints is %s.\n"), value);
>
> Could you update this function to show what effect the auto setting
> is currently having?  See breakpoint.c:show_always_inserted_mode for
> an example.
>
>>  }
>>
>> -/* Return non-zero if displaced stepping is enabled, and can be used
>> +/* Return non-zero if displaced stepping is auto, and can be used
>> +   with GDBARCH, and in non-stop mode.
>> +   Return non-zero if displaced stepping is on, and can be used
>>     with GDBARCH.  */
>>  static int
>>  use_displaced_stepping (struct gdbarch *gdbarch)
>>  {
>> -  return (can_use_displaced_stepping
>> +  return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
>> +           && non_stop)
>> +          || can_use_displaced_stepping == can_use_displaced_stepping_on)
>>           && gdbarch_displaced_step_copy_insn_p (gdbarch));
>>  }
>>
>> @@ -4857,11 +4873,14 @@ function is skipped and the step command
>>                            show_step_stop_if_no_debug,
>>                            &setlist, &showlist);
>>
>> -  add_setshow_boolean_cmd ("can-use-displaced-stepping", class_maintenance,
>> +  add_setshow_enum_cmd ("can-use-displaced-stepping", class_maintenance,
>> +                          can_use_displaced_stepping_enum,
>>                            &can_use_displaced_stepping, _("\
>>  Set debugger's willingness to use displaced stepping."), _("\
>>  Show debugger's willingness to use displaced stepping."), _("\
>> -If zero, gdb will not use displaced stepping to step over\n\
>> +If auto, gdb will auto use displaced stepping if it need (in non-stop mode).\n\
>> +If on, gdb will use displaced stepping if such is supported by the target.\n\
>> +If off, gdb will not use displaced stepping to step over\n\
>>  breakpoints, even if such is supported by the target."),
>
> This also needs to be updated in the manual.  I'd suggest borrowing the text
> from breakpoint.c's "always-inserted" command description to make it a
> bit more descriptive, and mention which mode is the default.  Should mostly
> be a matter of copy-paste, I think.
>
> --
> Pedro Alves
>

[-- Attachment #2: displaced_step_non_step.txt --]
[-- Type: text/plain, Size: 4201 bytes --]

--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,23 @@
+2008-10-10  Hui Zhu  <teawater@gmail.com>
+
+	Displaced stepping can be set to
+	auto (enable in non-stop mode), on, off.
+
+	* infrun.c (can_use_displaced_stepping): Change type to const
+	char pointer.
+	(can_use_displaced_stepping_auto): New string.
+	(can_use_displaced_stepping_on): New string.
+	(can_use_displaced_stepping_off): New string.
+	(can_use_displaced_stepping_enum): New array.
+	(show_can_use_displaced_stepping): In auto mode, also show the
+	current effect of the option.
+	(use_displaced_stepping): Return non-zero if displaced stepping
+ 	is auto, and can be used with GDBARCH, and in non-stop mode.
+	Return non-zero if displaced stepping is on, and can be used
+	with GDBARCH.
+	(_initialize_infrun): "can-use-displaced-stepping" function
+	change to add_setshow_enum_cmd.
+
 2008-10-09  Pedro Alves  <pedro@codesourcery.com>
 
 	* remote.c (remote_wait): Rename to...
--- a/infrun.c
+++ b/infrun.c
@@ -553,26 +553,50 @@ static CORE_ADDR displaced_step_original
 /* Saved contents of copy area.  */
 static gdb_byte *displaced_step_saved_copy;
 
-/* When this is non-zero, we are allowed to use displaced stepping, if
-   the architecture supports it.  When this is zero, we use
+/* When this is auto, we are allowed to use displaced stepping,
+   If the architecture supports it and in non-stop mode.
+   When this is on, we are allowed to use displaced stepping, if
+   the architecture supports it.  When this is off, we use
    traditional the hold-and-step approach.  */
-int can_use_displaced_stepping = 1;
+static const char can_use_displaced_stepping_auto[] = "auto";
+static const char can_use_displaced_stepping_on[] = "on";
+static const char can_use_displaced_stepping_off[] = "off";
+static const char *can_use_displaced_stepping_enum[] =
+{
+  can_use_displaced_stepping_auto,
+  can_use_displaced_stepping_on,
+  can_use_displaced_stepping_off,
+  NULL,
+};
+static const char *can_use_displaced_stepping =
+  can_use_displaced_stepping_auto;
 static void
 show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
 				 struct cmd_list_element *c,
 				 const char *value)
 {
-  fprintf_filtered (file, _("\
-Debugger's willingness to use displaced stepping to step over "
-"breakpoints is %s.\n"), value);
+  if (can_use_displaced_stepping == can_use_displaced_stepping_auto)
+    {
+      fprintf_filtered (file, _("\
+Debugger's willingness to use displaced stepping to step over " "breakpoints is %s (currently %s).\n"), value, non_stop ? "on" : "off");
+    }
+  else
+    {
+      fprintf_filtered (file, _("\
+Debugger's willingness to use displaced stepping to step over " "breakpoints is %s.\n"), value);
+    }
 }
 
-/* Return non-zero if displaced stepping is enabled, and can be used
+/* Return non-zero if displaced stepping is auto, and can be used
+   with GDBARCH, and in non-stop mode.
+   Return non-zero if displaced stepping is on, and can be used
    with GDBARCH.  */
 static int
 use_displaced_stepping (struct gdbarch *gdbarch)
 {
-  return (can_use_displaced_stepping
+  return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
+	    && non_stop)
+	   || can_use_displaced_stepping == can_use_displaced_stepping_on)
 	  && gdbarch_displaced_step_copy_insn_p (gdbarch));
 }
 
@@ -4857,11 +4881,14 @@ function is skipped and the step command
 			   show_step_stop_if_no_debug,
 			   &setlist, &showlist);
 
-  add_setshow_boolean_cmd ("can-use-displaced-stepping", class_maintenance,
+  add_setshow_enum_cmd ("can-use-displaced-stepping", class_maintenance,
+			   can_use_displaced_stepping_enum,
 			   &can_use_displaced_stepping, _("\
 Set debugger's willingness to use displaced stepping."), _("\
 Show debugger's willingness to use displaced stepping."), _("\
-If zero, gdb will not use displaced stepping to step over\n\
+If auto, gdb will auto use displaced stepping if it need (in non-stop mode).\n\
+If on, gdb will use displaced stepping if such is supported by the target.\n\
+If off, gdb will not use displaced stepping to step over\n\
 breakpoints, even if such is supported by the target."),
 			   NULL,
 			   show_can_use_displaced_stepping,

[-- Attachment #3: displaced_step_non_step_doc.txt --]
[-- Type: text/plain, Size: 1979 bytes --]

--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2008-10-10  Hui Zhu  <teawater@gmail.com>
+
+	* gdb.texinfo (can-use-displaced-stepping) Describe the auto mode
+	setting, and make it the default.
+
 2008-10-06  Doug Evans  <dje@google.com>
 
 	* gdb.texinfo (set debug dwarf2-die): Document it.
--- a/gdb.texinfo
+++ b/gdb.texinfo
@@ -23935,16 +23935,25 @@ Shared library events.
 
 @kindex maint set can-use-displaced-stepping
 @kindex maint show can-use-displaced-stepping
-@cindex displaced stepping support
-@cindex out-of-line single-stepping
-@item maint set can-use-displaced-stepping
-@itemx maint show can-use-displaced-stepping
-Control whether or not @value{GDBN} will do @dfn{displaced stepping}
-if the target supports it.  The default is on.  Displaced stepping is
-a way to single-step over breakpoints without removing them from the
-inferior, by executing an out-of-line copy of the instruction that was
-originally at the breakpoint location.  It is also known as
-out-of-line single-stepping.
+@table @code
+@item maint set can-use-displaced-stepping on
+If the target supports it.  Gdb behaves are single-step over
+breakpoints without removing them from the inferior, by executing
+an out-of-line copy of the instruction that was originally at the
+breakpoint location.  It is also known as out-of-line single-stepping.
+
+@item maint set can-use-displaced-stepping off
+Gdb behaves are single-step over breakpoints with removing them from
+the inferior.
+
+@cindex non-stop mode, and @code{breakpoint always-inserted}
+@item maint set can-use-displaced-stepping auto
+This is the default mode.  If @value{GDBN} is controlling the inferior
+in non-stop mode (@pxref{Non-Stop Mode}), gdb behaves as if
+@code{can-use-displaced-stepping} mode is on.  If @value{GDBN} is
+controlling the inferior in all-stop mode, @value{GDBN} behaves as if
+@code{can-use-displaced-stepping} mode is off.
+@end table
 
 @kindex maint check-symtabs
 @item maint check-symtabs

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

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

Fix a little bug in "displaced_step_non_step_doc.txt".

On Fri, Oct 10, 2008 at 11:37, teawater <teawater@gmail.com> wrote:
> Hi Pedro,
>
> Thanks for your example, I make a new patch for infrun.c and other for manual.
>
> 2008-10-10  Hui Zhu  <teawater@gmail.com>
>
>        Displaced stepping can be set to
>        auto (enable in non-stop mode), on, off.
>
>        * infrun.c (can_use_displaced_stepping): Change type to const
>        char pointer.
>        (can_use_displaced_stepping_auto): New string.
>        (can_use_displaced_stepping_on): New string.
>        (can_use_displaced_stepping_off): New string.
>        (can_use_displaced_stepping_enum): New array.
>        (show_can_use_displaced_stepping): In auto mode, also show the
>        current effect of the option.
>        (use_displaced_stepping): Return non-zero if displaced stepping
>        is auto, and can be used with GDBARCH, and in non-stop mode.
>        Return non-zero if displaced stepping is on, and can be used
>        with GDBARCH.
>        (_initialize_infrun): "can-use-displaced-stepping" function
>        change to add_setshow_enum_cmd.
>
> 2008-10-10  Hui Zhu  <teawater@gmail.com>
>
>        * gdb.texinfo (can-use-displaced-stepping) Describe the auto mode
>        setting, and make it the default.
>
>
> Thanks,
> Hui
>
> On Thu, Oct 9, 2008 at 22:49, Pedro Alves <pedro@codesourcery.com> wrote:
>> Hi,
>>
>> Thanks for doing this.
>>
>> On Wednesday 08 October 2008 07:10:43, teawater wrote:
>>
>>> -int can_use_displaced_stepping = 1;
>>> +const char can_use_displaced_stepping_auto[] = "auto";
>>> +const char can_use_displaced_stepping_on[] = "on";
>>> +const char can_use_displaced_stepping_off[] = "off";
>>> +const char *can_use_displaced_stepping_enum[] =
>>> +{
>>> +  can_use_displaced_stepping_auto,
>>> +  can_use_displaced_stepping_on,
>>> +  can_use_displaced_stepping_off,
>>> +  NULL,
>>> +};
>>> +const char *can_use_displaced_stepping = can_use_displaced_stepping_auto;
>>
>> Could you make these static while you're at it, please?  It was just missed
>> in the old boolean setting.
>>
>>>  static void
>>>  show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
>>>                                  struct cmd_list_element *c,
>>> @@ -567,12 +579,16 @@ Debugger's willingness to use displaced
>>>  "breakpoints is %s.\n"), value);
>>
>> Could you update this function to show what effect the auto setting
>> is currently having?  See breakpoint.c:show_always_inserted_mode for
>> an example.
>>
>>>  }
>>>
>>> -/* Return non-zero if displaced stepping is enabled, and can be used
>>> +/* Return non-zero if displaced stepping is auto, and can be used
>>> +   with GDBARCH, and in non-stop mode.
>>> +   Return non-zero if displaced stepping is on, and can be used
>>>     with GDBARCH.  */
>>>  static int
>>>  use_displaced_stepping (struct gdbarch *gdbarch)
>>>  {
>>> -  return (can_use_displaced_stepping
>>> +  return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
>>> +           && non_stop)
>>> +          || can_use_displaced_stepping == can_use_displaced_stepping_on)
>>>           && gdbarch_displaced_step_copy_insn_p (gdbarch));
>>>  }
>>>
>>> @@ -4857,11 +4873,14 @@ function is skipped and the step command
>>>                            show_step_stop_if_no_debug,
>>>                            &setlist, &showlist);
>>>
>>> -  add_setshow_boolean_cmd ("can-use-displaced-stepping", class_maintenance,
>>> +  add_setshow_enum_cmd ("can-use-displaced-stepping", class_maintenance,
>>> +                          can_use_displaced_stepping_enum,
>>>                            &can_use_displaced_stepping, _("\
>>>  Set debugger's willingness to use displaced stepping."), _("\
>>>  Show debugger's willingness to use displaced stepping."), _("\
>>> -If zero, gdb will not use displaced stepping to step over\n\
>>> +If auto, gdb will auto use displaced stepping if it need (in non-stop mode).\n\
>>> +If on, gdb will use displaced stepping if such is supported by the target.\n\
>>> +If off, gdb will not use displaced stepping to step over\n\
>>>  breakpoints, even if such is supported by the target."),
>>
>> This also needs to be updated in the manual.  I'd suggest borrowing the text
>> from breakpoint.c's "always-inserted" command description to make it a
>> bit more descriptive, and mention which mode is the default.  Should mostly
>> be a matter of copy-paste, I think.
>>
>> --
>> Pedro Alves
>>
>

[-- Attachment #2: displaced_step_non_step_doc.txt --]
[-- Type: text/plain, Size: 1989 bytes --]

--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2008-10-10  Hui Zhu  <teawater@gmail.com>
+
+	* gdb.texinfo (can-use-displaced-stepping) Describe the auto mode
+	setting, and make it the default.
+
 2008-10-06  Doug Evans  <dje@google.com>
 
 	* gdb.texinfo (set debug dwarf2-die): Document it.
--- a/gdb.texinfo
+++ b/gdb.texinfo
@@ -23935,16 +23935,25 @@ Shared library events.
 
 @kindex maint set can-use-displaced-stepping
 @kindex maint show can-use-displaced-stepping
-@cindex displaced stepping support
-@cindex out-of-line single-stepping
-@item maint set can-use-displaced-stepping
-@itemx maint show can-use-displaced-stepping
-Control whether or not @value{GDBN} will do @dfn{displaced stepping}
-if the target supports it.  The default is on.  Displaced stepping is
-a way to single-step over breakpoints without removing them from the
-inferior, by executing an out-of-line copy of the instruction that was
-originally at the breakpoint location.  It is also known as
-out-of-line single-stepping.
+@table @code
+@item maint set can-use-displaced-stepping on
+If the target supports it.  Gdb behaves are single-step over
+breakpoints without removing them from the inferior, by executing
+an out-of-line copy of the instruction that was originally at the
+breakpoint location.  It is also known as out-of-line single-stepping.
+
+@item maint set can-use-displaced-stepping off
+Gdb behaves are single-step over breakpoints with removing them from
+the inferior.
+
+@cindex non-stop mode, and @samp{maint set can-use-displaced-stepping}
+@item maint set can-use-displaced-stepping auto
+This is the default mode.  If @value{GDBN} is controlling the inferior
+in non-stop mode (@pxref{Non-Stop Mode}), gdb behaves as if
+@code{can-use-displaced-stepping} mode is on.  If @value{GDBN} is
+controlling the inferior in all-stop mode, @value{GDBN} behaves as if
+@code{can-use-displaced-stepping} mode is off.
+@end table
 
 @kindex maint check-symtabs
 @item maint check-symtabs

Re: [RFA] Displaced stepping just enable in non-stop mode

[Pedro Alves]

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

Hi teawater,

Thanks again for doing this.  I believe your patch is close to being OK.  I
took the liberty of adjusting it a bit, mostly re-wording a bit the
comments and the documentation.

I think Joel's suggestion was a good one:  By gaining an "auto"
setting, this is as much a maintainer command as
"set breakpoint always-inserted" is.  So, I believe it's time we move
the command to the top level "set" command group.

 set can-use-displaced-stepping (auto|on|off)

Eli, can I ask you to go over the help strings and the documentation in
the patch below?  Do they look OK-ish?

-- 
Pedro Alves

[-- Attachment #2: displaced_step_auto.diff --]
[-- Type: text/x-diff, Size: 7490 bytes --]

2008-10-16  Hui Zhu  <teawater@gmail.com>
	    Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (can_use_displaced_stepping): Change type to const char
	pointer.
	(can_use_displaced_stepping_auto): New string.
	(can_use_displaced_stepping_on): New string.
	(can_use_displaced_stepping_off): New string.
	(can_use_displaced_stepping_enum): New array.
	(show_can_use_displaced_stepping): In auto mode, also show the
	current effect of the option.
	(use_displaced_stepping): Return non-zero if displaced stepping is
	auto, and can be used with GDBARCH, and in non-stop mode.  Return
	non-zero if displaced stepping is on, and can be used with
	GDBARCH.  Return zero otherwise.
	(_initialize_infrun): Make the "set can-use-displaced-stepping"
	command an enum command.  Change its class to class_run.  Place it
	in the top level set list.  Extend help to describe the auto mode.

2008-10-16  Hui Zhu  <teawater@gmail.com>
	    Pedro Alves  <pedro@codesourcery.com>

	* gdb.texinfo (can-use-displaced-stepping): Describe the auto mode
	setting, and say it's the default.  This is now a mainstream
	setting instead of a maintenance setting.

---
 gdb/doc/gdb.texinfo |   33 ++++++++++++++++++------
 gdb/infrun.c        |   69 ++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 75 insertions(+), 27 deletions(-)

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-10-07 19:08:53.000000000 +0100
+++ src/gdb/infrun.c	2008-10-16 00:58:31.000000000 +0100
@@ -553,26 +553,55 @@ static CORE_ADDR displaced_step_original
 /* Saved contents of copy area.  */
 static gdb_byte *displaced_step_saved_copy;
 
-/* When this is non-zero, we are allowed to use displaced stepping, if
-   the architecture supports it.  When this is zero, we use
-   traditional the hold-and-step approach.  */
-int can_use_displaced_stepping = 1;
+/* Enum strings for "set|show can-use-displaced-stepping".  */
+
+static const char can_use_displaced_stepping_auto[] = "auto";
+static const char can_use_displaced_stepping_on[] = "on";
+static const char can_use_displaced_stepping_off[] = "off";
+static const char *can_use_displaced_stepping_enum[] =
+{
+  can_use_displaced_stepping_auto,
+  can_use_displaced_stepping_on,
+  can_use_displaced_stepping_off,
+  NULL,
+};
+
+/* If ON, and the architecture supports it, GDB will use displaced
+   stepping to step over breakpoints.  If OFF, or if the architecture
+   doesn't support it, GDB will instead use the traditional
+   hold-and-step approach.  If AUTO (which is the default), GDB will
+   decide which technique to use to step over breakpoints depending on
+   which of all-stop or non-stop mode is active --- displaced stepping
+   in non-stop mode; hold-and-step in all-stop mode.  */
+
+static const char *can_use_displaced_stepping =
+  can_use_displaced_stepping_auto;
+
 static void
 show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
 				 struct cmd_list_element *c,
 				 const char *value)
 {
-  fprintf_filtered (file, _("\
-Debugger's willingness to use displaced stepping to step over "
-"breakpoints is %s.\n"), value);
+  if (can_use_displaced_stepping == can_use_displaced_stepping_auto)
+    fprintf_filtered (file, _("\
+Debugger's willingness to use displaced stepping to step over \
+breakpoints is %s (currently %s).\n"),
+		      value, non_stop ? "on" : "off");
+  else
+    fprintf_filtered (file, _("\
+Debugger's willingness to use displaced stepping to step over \
+breakpoints is %s.\n"), value);
 }
 
-/* Return non-zero if displaced stepping is enabled, and can be used
-   with GDBARCH.  */
+/* Return non-zero if displaced stepping can/should be used to step
+   over breakpoints.  */
+
 static int
 use_displaced_stepping (struct gdbarch *gdbarch)
 {
-  return (can_use_displaced_stepping
+  return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
+	    && non_stop)
+	   || can_use_displaced_stepping == can_use_displaced_stepping_on)
 	  && gdbarch_displaced_step_copy_insn_p (gdbarch));
 }
 
@@ -4857,16 +4886,20 @@ function is skipped and the step command
 			   show_step_stop_if_no_debug,
 			   &setlist, &showlist);
 
-  add_setshow_boolean_cmd ("can-use-displaced-stepping", class_maintenance,
-			   &can_use_displaced_stepping, _("\
+  add_setshow_enum_cmd ("can-use-displaced-stepping", class_run,
+			can_use_displaced_stepping_enum,
+			&can_use_displaced_stepping, _("\
 Set debugger's willingness to use displaced stepping."), _("\
 Show debugger's willingness to use displaced stepping."), _("\
-If zero, gdb will not use displaced stepping to step over\n\
-breakpoints, even if such is supported by the target."),
-			   NULL,
-			   show_can_use_displaced_stepping,
-			   &maintenance_set_cmdlist,
-			   &maintenance_show_cmdlist);
+If on, gdb will use displaced stepping to step over breakpoints if it is\n\
+supported by the target architecture.  If off, gdb will not use displaced\n\
+stepping to step over breakpoints, even if such is supported by the target\n\
+architecture.  If auto (which is the default), gdb will use displaced stepping\n\
+if the target architecture supports it and non-stop mode is active, but will not\n\
+use it in all-stop mode (see help set non-stop)."),
+			NULL,
+			show_can_use_displaced_stepping,
+			&setlist, &showlist);
 
   /* ptid initializations */
   null_ptid = ptid_build (0, 0, 0);
Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2008-10-15 23:33:25.000000000 +0100
+++ src/gdb/doc/gdb.texinfo	2008-10-16 01:05:33.000000000 +0100
@@ -23933,18 +23933,33 @@ Shared library events.
 
 @end table
 
-@kindex maint set can-use-displaced-stepping
-@kindex maint show can-use-displaced-stepping
+@kindex set can-use-displaced-stepping
+@kindex show can-use-displaced-stepping
 @cindex displaced stepping support
 @cindex out-of-line single-stepping
-@item maint set can-use-displaced-stepping
-@itemx maint show can-use-displaced-stepping
+@item set can-use-displaced-stepping
+@itemx show can-use-displaced-stepping
 Control whether or not @value{GDBN} will do @dfn{displaced stepping}
-if the target supports it.  The default is on.  Displaced stepping is
-a way to single-step over breakpoints without removing them from the
-inferior, by executing an out-of-line copy of the instruction that was
-originally at the breakpoint location.  It is also known as
-out-of-line single-stepping.
+if the target supports it.  Displaced stepping is a way to single-step
+over breakpoints without removing them from the inferior, by executing
+an out-of-line copy of the instruction that was originally at the
+breakpoint location.  It is also known as out-of-line single-stepping.
+
+@table @code
+@item set can-use-displaced-stepping on
+If the target architecture supports it, @value{GDBN} will use
+displaced stepping to step over breakpoints.
+
+@item set can-use-displaced-stepping off
+@value{GDBN} will not use displaced stepping to step over breakpoints,
+even if such is supported by the target architecture.
+
+@cindex non-stop mode, and @samp{set can-use-displaced-stepping}
+@item set can-use-displaced-stepping auto
+This is the default mode.  @value{GDBN} will use displaced stepping
+only if non-stop mode is active (@pxref{Non-Stop Mode}) and the target
+architecture supports it.
+@end table
 
 @kindex maint check-symtabs
 @item maint check-symtabs

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

Thanks for your help Pedro. :)

Hui

On Thu, Oct 16, 2008 at 08:07, Pedro Alves <pedro@codesourcery.com> wrote:
> Hi teawater,
>
> Thanks again for doing this.  I believe your patch is close to being OK.  I
> took the liberty of adjusting it a bit, mostly re-wording a bit the
> comments and the documentation.
>
> I think Joel's suggestion was a good one:  By gaining an "auto"
> setting, this is as much a maintainer command as
> "set breakpoint always-inserted" is.  So, I believe it's time we move
> the command to the top level "set" command group.
>
>  set can-use-displaced-stepping (auto|on|off)
>
> Eli, can I ask you to go over the help strings and the documentation in
> the patch below?  Do they look OK-ish?
>
> --
> Pedro Alves
>

Re: [RFA] Displaced stepping just enable in non-stop mode

[Daniel Jacobowitz]

On Thu, Oct 16, 2008 at 01:07:42AM +0100, Pedro Alves wrote:
> Hi teawater,
> 
> Thanks again for doing this.  I believe your patch is close to being OK.  I
> took the liberty of adjusting it a bit, mostly re-wording a bit the
> comments and the documentation.
> 
> I think Joel's suggestion was a good one:  By gaining an "auto"
> setting, this is as much a maintainer command as
> "set breakpoint always-inserted" is.  So, I believe it's time we move
> the command to the top level "set" command group.
> 
>  set can-use-displaced-stepping (auto|on|off)
> 
> Eli, can I ask you to go over the help strings and the documentation in
> the patch below?  Do they look OK-ish?

That's a pretty long name.  Do you think there are any other useful
stepping related settings?  If so, maybe we should have a "set step"
prefix.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Displaced stepping just enable in non-stop mode

[Pedro Alves]

On Thursday 16 October 2008 03:38:37, Daniel Jacobowitz wrote:
> On Thu, Oct 16, 2008 at 01:07:42AM +0100, Pedro Alves wrote:

> > I think Joel's suggestion was a good one:  By gaining an "auto"
> > setting, this is as much a maintainer command as
> > "set breakpoint always-inserted" is.  So, I believe it's time we move
> > the command to the top level "set" command group.
> > 
> >  set can-use-displaced-stepping (auto|on|off)
> > 
> > Eli, can I ask you to go over the help strings and the documentation in
> > the patch below?  Do they look OK-ish?
> 
> That's a pretty long name.  Do you think there are any other useful
> stepping related settings?  If so, maybe we should have a "set step"
> prefix.
> 

Hmm, not offhand.  I'd have to think about it.
What would you call the setting then, if not "set step can-use-displaced-stepping" ?

The name was actually modeled on "set can-use-hw-watchpoints" in the first place.

-- 
Pedro Alves

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

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

Hi all,

I make a new patch that change "can-use-displaced-stepping" to
"displaced-stepping" and change some part according to Eli's idea in
doc.

2008-10-22  Hui Zhu  <teawater@gmail.com>
	    Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (can_use_displaced_stepping): Change type to const char
	pointer.
	(can_use_displaced_stepping_auto): New string.
	(can_use_displaced_stepping_on): New string.
	(can_use_displaced_stepping_off): New string.
	(can_use_displaced_stepping_enum): New array.
	(show_can_use_displaced_stepping): In auto mode, also show the
	current effect of the option.
	(use_displaced_stepping): Return non-zero if displaced stepping is
	auto, and can be used with GDBARCH, and in non-stop mode.  Return
	non-zero if displaced stepping is on, and can be used with
	GDBARCH.  Return zero otherwise.
	(_initialize_infrun): Make the "set displaced-stepping"
	command an enum command.  Change its class to class_run.  Place it
	in the top level set list.  Extend help to describe the auto mode.

2008-10-22  Hui Zhu  <teawater@gmail.com>
	    Pedro Alves  <pedro@codesourcery.com>

	* gdb.texinfo (displaced-stepping): Describe the auto mode
	setting, and say it's the default.  This is now a mainstream
	setting instead of a maintenance setting.

Thanks,
Hui

On Thu, Oct 16, 2008 at 10:59, Pedro Alves <pedro@codesourcery.com> wrote:
> On Thursday 16 October 2008 03:38:37, Daniel Jacobowitz wrote:
>> On Thu, Oct 16, 2008 at 01:07:42AM +0100, Pedro Alves wrote:
>
>> > I think Joel's suggestion was a good one:  By gaining an "auto"
>> > setting, this is as much a maintainer command as
>> > "set breakpoint always-inserted" is.  So, I believe it's time we move
>> > the command to the top level "set" command group.
>> >
>> >  set can-use-displaced-stepping (auto|on|off)
>> >
>> > Eli, can I ask you to go over the help strings and the documentation in
>> > the patch below?  Do they look OK-ish?
>>
>> That's a pretty long name.  Do you think there are any other useful
>> stepping related settings?  If so, maybe we should have a "set step"
>> prefix.
>>
>
> Hmm, not offhand.  I'd have to think about it.
> What would you call the setting then, if not "set step can-use-displaced-stepping" ?
>
> The name was actually modeled on "set can-use-hw-watchpoints" in the first place.
>
> --
> Pedro Alves
>

[-- Attachment #2: displaced_step_non_step.txt --]
[-- Type: text/plain, Size: 5784 bytes --]

--- a/infrun.c
+++ b/infrun.c
@@ -553,26 +553,55 @@ static CORE_ADDR displaced_step_original
 /* Saved contents of copy area.  */
 static gdb_byte *displaced_step_saved_copy;
 
-/* When this is non-zero, we are allowed to use displaced stepping, if
-   the architecture supports it.  When this is zero, we use
-   traditional the hold-and-step approach.  */
-int can_use_displaced_stepping = 1;
+/* Enum strings for "set|show displaced-stepping".  */
+
+static const char can_use_displaced_stepping_auto[] = "auto";
+static const char can_use_displaced_stepping_on[] = "on";
+static const char can_use_displaced_stepping_off[] = "off";
+static const char *can_use_displaced_stepping_enum[] =
+{
+  can_use_displaced_stepping_auto,
+  can_use_displaced_stepping_on,
+  can_use_displaced_stepping_off,
+  NULL,
+};
+
+/* If ON, and the architecture supports it, GDB will use displaced
+   stepping to step over breakpoints.  If OFF, or if the architecture
+   doesn't support it, GDB will instead use the traditional
+   hold-and-step approach.  If AUTO (which is the default), GDB will
+   decide which technique to use to step over breakpoints depending on
+   which of all-stop or non-stop mode is active --- displaced stepping
+   in non-stop mode; hold-and-step in all-stop mode.  */
+
+static const char *can_use_displaced_stepping =
+  can_use_displaced_stepping_auto;
+
 static void
 show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
 				 struct cmd_list_element *c,
 				 const char *value)
 {
-  fprintf_filtered (file, _("\
-Debugger's willingness to use displaced stepping to step over "
-"breakpoints is %s.\n"), value);
+  if (can_use_displaced_stepping == can_use_displaced_stepping_auto)
+    fprintf_filtered (file, _("\
+Debugger's willingness to use displaced stepping to step over \
+breakpoints is %s (currently %s).\n"),
+		      value, non_stop ? "on" : "off");
+  else
+    fprintf_filtered (file, _("\
+Debugger's willingness to use displaced stepping to step over \
+breakpoints is %s.\n"), value);
 }
 
-/* Return non-zero if displaced stepping is enabled, and can be used
-   with GDBARCH.  */
+/* Return non-zero if displaced stepping can/should be used to step
+   over breakpoints.  */
+
 static int
 use_displaced_stepping (struct gdbarch *gdbarch)
 {
-  return (can_use_displaced_stepping
+  return (((can_use_displaced_stepping == can_use_displaced_stepping_auto
+	    && non_stop)
+	   || can_use_displaced_stepping == can_use_displaced_stepping_on)
 	  && gdbarch_displaced_step_copy_insn_p (gdbarch));
 }
 
@@ -5107,16 +5136,20 @@ function is skipped and the step command
 			   show_step_stop_if_no_debug,
 			   &setlist, &showlist);
 
-  add_setshow_boolean_cmd ("can-use-displaced-stepping", class_maintenance,
-			   &can_use_displaced_stepping, _("\
+  add_setshow_enum_cmd ("displaced-stepping", class_run,
+			can_use_displaced_stepping_enum,
+			&can_use_displaced_stepping, _("\
 Set debugger's willingness to use displaced stepping."), _("\
 Show debugger's willingness to use displaced stepping."), _("\
-If zero, gdb will not use displaced stepping to step over\n\
-breakpoints, even if such is supported by the target."),
-			   NULL,
-			   show_can_use_displaced_stepping,
-			   &maintenance_set_cmdlist,
-			   &maintenance_show_cmdlist);
+If on, gdb will use displaced stepping to step over breakpoints if it is\n\
+supported by the target architecture.  If off, gdb will not use displaced\n\
+stepping to step over breakpoints, even if such is supported by the target\n\
+architecture.  If auto (which is the default), gdb will use displaced stepping\n\
+if the target architecture supports it and non-stop mode is active, but will not\n\
+use it in all-stop mode (see help set non-stop)."),
+			NULL,
+			show_can_use_displaced_stepping,
+			&setlist, &showlist);
 
   add_setshow_enum_cmd ("exec-direction", class_run, exec_direction_names,
 			&exec_direction, _("Set direction of execution.\n\
--- a/gdb.texinfo
+++ b/gdb.texinfo
@@ -24113,18 +24113,33 @@ Shared library events.
 
 @end table
 
-@kindex maint set can-use-displaced-stepping
-@kindex maint show can-use-displaced-stepping
+@kindex set displaced-stepping
+@kindex show displaced-stepping
 @cindex displaced stepping support
 @cindex out-of-line single-stepping
-@item maint set can-use-displaced-stepping
-@itemx maint show can-use-displaced-stepping
+@item set displaced-stepping
+@itemx show displaced-stepping
 Control whether or not @value{GDBN} will do @dfn{displaced stepping}
-if the target supports it.  The default is on.  Displaced stepping is
-a way to single-step over breakpoints without removing them from the
-inferior, by executing an out-of-line copy of the instruction that was
-originally at the breakpoint location.  It is also known as
-out-of-line single-stepping.
+if the target supports it.  Displaced stepping is a way to single-step
+over breakpoints without removing them from the inferior, by executing
+an out-of-line copy of the instruction that was originally at the
+breakpoint location.  It is also known as out-of-line single-stepping.
+
+@table @code
+@item set displaced-stepping on
+If the target architecture supports it, @value{GDBN} will use
+displaced stepping to step over breakpoints.
+
+@item set displaced-stepping off
+@value{GDBN} will not use displaced stepping to step over breakpoints,
+even if such is supported by the target architecture.
+
+@cindex non-stop mode, and @samp{set displaced-stepping}
+@item set displaced-stepping auto
+This is the default mode.  @value{GDBN} will use displaced stepping
+only if non-stop mode is active (@pxref{Non-Stop Mode}) and the target
+architecture supports displaced stepping.
+@end table
 
 @kindex maint check-symtabs
 @item maint check-symtabs

Re: [RFA] Displaced stepping just enable in non-stop mode

[Eli Zaretskii]

> Date: Wed, 22 Oct 2008 11:16:02 +0800
> From: teawater <teawater@gmail.com>
> 
> I make a new patch that change "can-use-displaced-stepping" to
> "displaced-stepping" and change some part according to Eli's idea in
> doc.

Thank you, I'm now happy with the documentation part.

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

Thanks Eli.

Pedro, How do you think about code part?

Thanks,
Hui

On Thu, Oct 23, 2008 at 03:47, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Wed, 22 Oct 2008 11:16:02 +0800
>> From: teawater <teawater@gmail.com>
>>
>> I make a new patch that change "can-use-displaced-stepping" to
>> "displaced-stepping" and change some part according to Eli's idea in
>> doc.
>
> Thank you, I'm now happy with the documentation part.
>

Re: [RFA] Displaced stepping just enable in non-stop mode

[Pedro Alves]

On Thursday 23 October 2008 09:12:35, teawater wrote:
> Thanks Eli.
> 
> Pedro, How do you think about code part?

OK with me.  Thanks!

-- 
Pedro Alves

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

Thanks everybody. I have check it in.

Hui

On Thu, Oct 23, 2008 at 20:56, Pedro Alves <pedro@codesourcery.com> wrote:
> On Thursday 23 October 2008 09:12:35, teawater wrote:
>> Thanks Eli.
>>
>> Pedro, How do you think about code part?
>
> OK with me.  Thanks!
>
> --
> Pedro Alves
>

Re: [RFA] Displaced stepping just enable in non-stop mode

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Thu, 16 Oct 2008 01:07:42 +0100
> Cc: gdb-patches@sourceware.org,
>  "Joel Brobecker" <brobecker@adacore.com>,
>  "Michael Snyder" <msnyder@vmware.com>
> 
> Eli, can I ask you to go over the help strings and the documentation in
> the patch below?  Do they look OK-ish?

They are OK, but:

  . The doc strings use too long lines, which will look ugly on your
    garden-variety 80-column terminal.

  . In the gdb.texinfo, don't put "set" and "show" commands in the
    same @item/@itemx, because the text below them says "Control
    whether...", which is inappropriate for the "show" command.

  . "Displaced stepping" is really a bad name for this feature.  Is it
    widely accepted or did we invent it?

  . This text:

     +@cindex non-stop mode, and @samp{set can-use-displaced-stepping}
     +@item set can-use-displaced-stepping auto
     +This is the default mode.  @value{GDBN} will use displaced stepping
     +only if non-stop mode is active (@pxref{Non-Stop Mode}) and the target
     +architecture supports it.

    needs to be rephrased in the last sentence, to disambiguate the
    "supports it" part: a reader could be confused to think that "it"
    refers to the non-stop mode, not to displaced stepping.

  . Why isn't it better to use displaced stepping, if supported, even
    if non-stop mode is not in effect?  I think the linkage between
    the two is confusing and unnecessary.

  . I agree with Daniel's criticism of the can-use-displaced-stepping
    option's name.  An obvious first step towards making it shorter
    would be to remove the "can-" prefix.

Thanks.

Re: [RFA] Displaced stepping just enable in non-stop mode

[Daniel Jacobowitz]

On Thu, Oct 16, 2008 at 10:12:37AM +0200, Eli Zaretskii wrote:
>   . Why isn't it better to use displaced stepping, if supported, even
>     if non-stop mode is not in effect?  I think the linkage between
>     the two is confusing and unnecessary.

It is generally good to use displaced stepping.  But in some
circumstances it is slower, and in others it doesn't work at all.  It
requires we have a small scratchpad area on the target which is
writeable and executable.  By default we use the area at _start; this
doesn't work on some simulator targets, on targets which execute code
from ROM or flash memory, or during reverse debugging.

Some of those cases could be fixed by adding a user knob for where to
put the scratchpad, though others can't.

It's linked to non-stop because for non-stop it is required.

I'm not sure what else to call displaced stepping.  "Step around
breakpoints"?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Displaced stepping just enable in non-stop mode

[Eli Zaretskii]

> Date: Thu, 16 Oct 2008 08:34:22 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Pedro Alves <pedro@codesourcery.com>, teawater@gmail.com,
> 	gdb-patches@sourceware.org, brobecker@adacore.com,
> 	msnyder@vmware.com
> 
> On Thu, Oct 16, 2008 at 10:12:37AM +0200, Eli Zaretskii wrote:
> >   . Why isn't it better to use displaced stepping, if supported, even
> >     if non-stop mode is not in effect?  I think the linkage between
> >     the two is confusing and unnecessary.
> 
> It is generally good to use displaced stepping.  But in some
> circumstances it is slower, and in others it doesn't work at all.  It
> requires we have a small scratchpad area on the target which is
> writeable and executable.  By default we use the area at _start; this
> doesn't work on some simulator targets, on targets which execute code
> from ROM or flash memory, or during reverse debugging.
> 
> Some of those cases could be fixed by adding a user knob for where to
> put the scratchpad, though others can't.
> 
> It's linked to non-stop because for non-stop it is required.

Sorry, I don't get the logic of this decision.

Can we reliably use displaced stepping, or can't we?  If we can do
that reliably in vast majority of use-cases, we should do that even
without non-stop.  If we cannot do that reliably enough, we shouldn't
turn it on even with non-stop mode, or maybe refuse to turn on
non-stop, rather than risk screwing the users.

Since we are prepared to decide that turning non-stop turns on
displaced stepping, I understand that in most cases displaced stepping
does work, which brings me to the conclusion that we could use
displaced stepping even without non-stop.

We could also try to detect if it works, and display a warning if we
think it won't (RE the cases you described above).

> I'm not sure what else to call displaced stepping.  "Step around
> breakpoints"?

The text mentions "out-of-line stepping", which sounds better to me.

Re: [RFA] Displaced stepping just enable in non-stop mode

[Daniel Jacobowitz]

On Thu, Oct 16, 2008 at 08:25:13PM +0200, Eli Zaretskii wrote:
> Can we reliably use displaced stepping, or can't we?  If we can do
> that reliably in vast majority of use-cases, we should do that even
> without non-stop.  If we cannot do that reliably enough, we shouldn't
> turn it on even with non-stop mode, or maybe refuse to turn on
> non-stop, rather than risk screwing the users.

We do not know whether it's reliable or not.  It depends on properties
of the target system that GDB has no knowledge of.  But there's no way
to use non-stop without displaced stepping; so we require it to work
on any target that supports non-stop debugging.

> We could also try to detect if it works, and display a warning if we
> think it won't (RE the cases you described above).

Hmm, that's an interesting idea.  Pedro, what do you think - would
autodetection work for the cases we've seen trouble?  Something as
simple as "can we write to _start" is probably enough, but I don't
remember what the failure looked like with the record target; and in
that case it may be complicated by the fact that we're initially going
forwards and could write.

> > I'm not sure what else to call displaced stepping.  "Step around
> > breakpoints"?
> 
> The text mentions "out-of-line stepping", which sounds better to me.

I like "set step out-of-line"...

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Displaced stepping just enable in non-stop mode

[Michael Snyder]

Daniel Jacobowitz wrote:

>> We could also try to detect if it works, and display a warning if we
>> think it won't (RE the cases you described above).
> 
> Hmm, that's an interesting idea.  Pedro, what do you think - would
> autodetection work for the cases we've seen trouble?  Something as
> simple as "can we write to _start" is probably enough, but I don't
> remember what the failure looked like with the record target; and in
> that case it may be complicated by the fact that we're initially going
> forwards and could write.

What it looks like is that you try to write to memory
that's write-protected.

This is because most replay targets will treat all of memory
as write-protected when they are in replay mode.

Where this usually manifests is, you'll say "continue"
(probably for the first time since attaching to the target),
and it'll croak because it's trying to step over some
"invisible" breakpoint such as the one that handles
shared libraries.

> 
>>> I'm not sure what else to call displaced stepping.  "Step around
>>> breakpoints"?
>> The text mentions "out-of-line stepping", which sounds better to me.
> 
> I like "set step out-of-line"...

Arr arr...  step up-against-the-wall...

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

On Fri, Oct 17, 2008 at 05:15, Michael Snyder <msnyder@vmware.com> wrote:
> Daniel Jacobowitz wrote:
>
>>> We could also try to detect if it works, and display a warning if we
>>> think it won't (RE the cases you described above).
>>
>> Hmm, that's an interesting idea.  Pedro, what do you think - would
>> autodetection work for the cases we've seen trouble?  Something as
>> simple as "can we write to _start" is probably enough, but I don't
>> remember what the failure looked like with the record target; and in
>> that case it may be complicated by the fact that we're initially going
>> forwards and could write.
>
> What it looks like is that you try to write to memory
> that's write-protected.
>
> This is because most replay targets will treat all of memory
> as write-protected when they are in replay mode.
>
> Where this usually manifests is, you'll say "continue"
> (probably for the first time since attaching to the target),
> and it'll croak because it's trying to step over some
> "invisible" breakpoint such as the one that handles
> shared libraries.
>
>>
>>>> I'm not sure what else to call displaced stepping.  "Step around
>>>> breakpoints"?
>>>
>>> The text mentions "out-of-line stepping", which sounds better to me.
>>
>> I like "set step out-of-line"...
>
> Arr arr...  step up-against-the-wall...
>
>

How about "set step displace"

I think "displace" is clear.

Re: [RFA] Displaced stepping just enable in non-stop mode

[Eli Zaretskii]

> Date: Fri, 17 Oct 2008 13:45:36 +0800
> From: teawater <teawater@gmail.com>
> Cc: "Eli Zaretskii" <eliz@gnu.org>, pedro@codesourcery.com, 
> 	gdb-patches@sourceware.org, brobecker@adacore.com
> 
> How about "set step displace"
> 
> I think "displace" is clear.

It isn't clear to me.  "Displace" means simply "move forcefully".
What is actually being moved here is the breakpoint instruction, but
"set step displace" includes no hints for that.

Maybe "set displace-break-insn" or some such.

Re: [RFA] Displaced stepping just enable in non-stop mode

[Pedro Alves]

On Friday 17 October 2008 11:01:12, Eli Zaretskii wrote:
> > Date: Fri, 17 Oct 2008 13:45:36 +0800
> > From: teawater <teawater@gmail.com>
> > Cc: "Eli Zaretskii" <eliz@gnu.org>, pedro@codesourcery.com, 
> > 	gdb-patches@sourceware.org, brobecker@adacore.com
> > 
> > How about "set step displace"
> > 
> > I think "displace" is clear.
> 
> It isn't clear to me.  "Displace" means simply "move forcefully".
> What is actually being moved here is the breakpoint instruction, but
> "set step displace" includes no hints for that.
> 
> Maybe "set displace-break-insn" or some such.

I've thought a bit about the "set step ..." suggestion, and I
remembered Michael's "set/show enable-software-singlestep"
command, that could also fit there.  So, I guess we could have:

For displaced stepping:

 set step over-breakpoint auto (on if there's gdbarch + nonstop support)
 set step over-breakpoint displaced/ool/out-of-line
 set step over-breakpoint step-and-hold

 (or shorter versions)
 set step breakpoint out-of-line
 set step over-breakpoint ool
 set step breakpoint ool

For soft/hard single-stepping:

 set step single-step auto
 set step single-step hard
 set step single-step soft

 Or shorter versions:
 set step single hard
 set step single soft 

...

-- 
Pedro Alves

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

I think "set displace-break-insn" is not right.
It will make user think breakpoint insn move to another address.
Actually this function try to don't move breakpoint insn in step.

On Fri, Oct 17, 2008 at 18:01, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 17 Oct 2008 13:45:36 +0800
>> From: teawater <teawater@gmail.com>
>> Cc: "Eli Zaretskii" <eliz@gnu.org>, pedro@codesourcery.com,
>>       gdb-patches@sourceware.org, brobecker@adacore.com
>>
>> How about "set step displace"
>>
>> I think "displace" is clear.
>
> It isn't clear to me.  "Displace" means simply "move forcefully".
> What is actually being moved here is the breakpoint instruction, but
> "set step displace" includes no hints for that.
>
> Maybe "set displace-break-insn" or some such.
>

Re: [RFA] Displaced stepping just enable in non-stop mode

[Pedro Alves]

On Thursday 16 October 2008 22:15:30, Michael Snyder wrote:
> What it looks like is that you try to write to memory
> that's write-protected.
> 
> This is because most replay targets will treat all of memory
> as write-protected when they are in replay mode.
> 
> Where this usually manifests is, you'll say "continue"
> (probably for the first time since attaching to the target),
> and it'll croak because it's trying to step over some
> "invisible" breakpoint such as the one that handles
> shared libraries.

I see, then this means that all the reverse targets in existance
implement inserting breakpoints (target_insert_breakpoint, Z packet),
instead of relying on memory breakpoints.  Ah, the record target
just ignores breakpoint insertions, and relies on forcing single-stepping
and checking breakpoint hits itself.

-- 
Pedro Alves

Re: [RFA] Displaced stepping just enable in non-stop mode

[teawater]

P record can support displaced stepping but it don't support non-step.
 So we remove this code.

On Fri, Oct 17, 2008 at 22:48, Pedro Alves <pedro@codesourcery.com> wrote:
> On Thursday 16 October 2008 22:15:30, Michael Snyder wrote:
>> What it looks like is that you try to write to memory
>> that's write-protected.
>>
>> This is because most replay targets will treat all of memory
>> as write-protected when they are in replay mode.
>>
>> Where this usually manifests is, you'll say "continue"
>> (probably for the first time since attaching to the target),
>> and it'll croak because it's trying to step over some
>> "invisible" breakpoint such as the one that handles
>> shared libraries.
>
> I see, then this means that all the reverse targets in existance
> implement inserting breakpoints (target_insert_breakpoint, Z packet),
> instead of relying on memory breakpoints.  Ah, the record target
> just ignores breakpoint insertions, and relies on forcing single-stepping
> and checking breakpoint hits itself.
>
> --
> Pedro Alves
>

Re: [RFA] Displaced stepping just enable in non-stop mode

[Michael Snyder]

Pedro Alves wrote:
> On Thursday 16 October 2008 22:15:30, Michael Snyder wrote:
>> What it looks like is that you try to write to memory
>> that's write-protected.
>>
>> This is because most replay targets will treat all of memory
>> as write-protected when they are in replay mode.
>>
>> Where this usually manifests is, you'll say "continue"
>> (probably for the first time since attaching to the target),
>> and it'll croak because it's trying to step over some
>> "invisible" breakpoint such as the one that handles
>> shared libraries.
> 
> I see, then this means that all the reverse targets in existance
> implement inserting breakpoints (target_insert_breakpoint, Z packet),
> instead of relying on memory breakpoints. 

I believe that's true.  Certainly it's true for gdb-freeplay.

Teawater's "process replay" isn't a remote target, so it does
not use Z packet explicitly, but it does something similar --
in effect, it mimics the Z0 semantics.

VMware's development version uses Z0.

> Ah, the record target
> just ignores breakpoint insertions, and relies on forcing single-stepping
> and checking breakpoint hits itself.

That's right.

RE: [RFA] Displaced stepping just enable in non-stop mode

[Jakob Engblom]

> > I see, then this means that all the reverse targets in existance
> > implement inserting breakpoints (target_insert_breakpoint, Z packet),
> > instead of relying on memory breakpoints.
> 
> I believe that's true.  Certainly it's true for gdb-freeplay.

Do I understand to mean that the assumption is that the remote-capable target
system 
Implements breakpoints by NOT modifying the code, but rather using some other
mechanism that says that certain instructions in memory are breakpoints, and
triggers them appropriately? 

This is what we do in Simics, and this assumptions makes sense to us.
Breakpoints are implemented in the simulator, without any change to the target
system. 

/jakob

Re: [RFA] Displaced stepping just enable in non-stop mode

[Michael Snyder]

Jakob Engblom wrote:
>>> I see, then this means that all the reverse targets in existance
>>> implement inserting breakpoints (target_insert_breakpoint, Z packet),
>>> instead of relying on memory breakpoints.
>> I believe that's true.  Certainly it's true for gdb-freeplay.
> 
> Do I understand to mean that the assumption is that the remote-capable target
> system
> Implements breakpoints by NOT modifying the code, but rather using some other
> mechanism that says that certain instructions in memory are breakpoints, and
> triggers them appropriately?
> 
> This is what we do in Simics, and this assumptions makes sense to us.
> Breakpoints are implemented in the simulator, without any change to the target
> system.

That's right.

The serial protocol message is Z0/z0, and it instructs the target
to set/clear a breakpoint (rather than gdb inserting traps into
the text section explicitly).

Re: [RFA] Displaced stepping just enable in non-stop mode

[Pedro Alves]

On Thursday 16 October 2008 19:25:13, Eli Zaretskii wrote:

> Since we are prepared to decide that turning non-stop turns on
> displaced stepping, I understand that in most cases displaced stepping
> does work, which brings me to the conclusion that we could use
> displaced stepping even without non-stop.

The target itself could support stepping over breakpoints
without requiring us to use displaced stepping on the core
side.  DICOS falls in that category, I just haven't submitted
the patch yet to make the target report support for it (along with a
qSupported feature).  If the target doesn't report support for it, we
fallback to displaced stepping, and that requires gdbarch support.

> We could also try to detect if it works, and display a warning if we
> think it won't (RE the cases you described above).

That would be a warning that brings no value to the user in
all-stop mode.

If the user sees:

 "warning: you can not use displaced stepping on this platform".

He/she will think that something is wrong, while at least currently,
no functionality the user cares for is lost (in all-stop).

> > I'm not sure what else to call displaced stepping.  "Step around
> > breakpoints"?
> 
> The text mentions "out-of-line stepping", which sounds better to me.

I have no idea why "displaced stepping" was chosen in the first
place --- that pre-dates me.

-- 
Pedro Alves

Re: [RFA] Displaced stepping just enable in non-stop mode

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Fri, 17 Oct 2008 15:50:40 +0100
> Cc: Daniel Jacobowitz <drow@false.org>,  teawater@gmail.com,  brobecker@adacore.com,  msnyder@vmware.com
> 
> > We could also try to detect if it works, and display a warning if we
> > think it won't (RE the cases you described above).
> 
> That would be a warning that brings no value to the user in
> all-stop mode.
> 
> If the user sees:
> 
>  "warning: you can not use displaced stepping on this platform".
> 
> He/she will think that something is wrong, while at least currently,
> no functionality the user cares for is lost (in all-stop).

The warning should say that non-stop won't work, and we should refuse
to work in non-stop mode in this case.