Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Extending RSP with vCont;n and vCont;f
@ 2013-10-02 15:08 ILG.Robert
  2013-10-02 16:25 ` Eli Zaretskii
  2013-10-04  6:49 ` Yao Qi
  0 siblings, 2 replies; 9+ messages in thread
From: ILG.Robert @ 2013-10-02 15:08 UTC (permalink / raw)
  To: gdb-patches

Hallo,

we would like to improve the RSP command "vCont" for remote debugging. It seems that the range option has been introduced recently to get rid of time consuming, successive single-steps. Our intention is to get rid of further, unnecessary RSP packages being sent as our target is capable to do a step-over (called next by GDB) and a step-return (called finish by GDB). Therefore we propose to extend the vCont command with "vCont;n" and "vCont;f" as well.

You will find the necessary patch below. It has been tested on MinGW with a x86-Target. It's based on the todays trunk.

Thanks in advance. 
Robert Ilg

diff --git a/gdb/remote.c b/gdb/remote.c
index a9ef297..df31c3a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -256,11 +256,19 @@

   /* vCont;r */
   int r;
+
+  /* vCont;f */
+  int f;
+
+  /* vCont;n */
+  int n;
};

 /* Controls whether GDB is willing to use range stepping.  */

 static int use_range_stepping = 1;
+static int use_finish_stepping = 1;
+static int use_next_stepping = 1;

 #define OPAQUETHREADBYTES 8

@@ -4704,6 +4712,8 @@
       support_C = 0;
       rs->supports_vCont.t = 0;
       rs->supports_vCont.r = 0;
+      rs->supports_vCont.n = 0;
+      rs->supports_vCont.f = 0;
       while (p && *p == ';')
    {
      p++;
@@ -4719,6 +4729,10 @@
        rs->supports_vCont.t = 1;
      else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
        rs->supports_vCont.r = 1;
+      else if (*p == 'f' && (*(p + 1) == ';' || *(p + 1) == 0))
+        rs->supports_vCont.f = 1;
+      else if (*p == 'n' && (*(p + 1) == ';' || *(p + 1) == 0))
+        rs->supports_vCont.n = 1;

       p = strchr (p, ';');
    }
@@ -4747,9 +4761,29 @@
            ptid_t ptid, int step, enum gdb_signal siggnal)
{
   struct remote_state *rs = get_remote_state ();
+  struct thread_info *tp;
+
+  if (ptid_equal (ptid, minus_one_ptid))
+  {
+    /* If we don't know about the target thread's tid, then
+     * we're resuming magic_null_ptid (see caller).  */
+    tp = find_thread_ptid (magic_null_ptid);
+   }
+   else
+     tp = find_thread_ptid (ptid);

+   gdb_assert (tp != NULL);
+
   if (step && siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";S%02x", siggnal);
+  else if (
+       /* GDB is willing to step next and target supports it */
+       use_next_stepping && rs->supports_vCont.n &&
+       /* step next is suitable for this resumption */
+       step && (tp->control.step_over_calls == STEP_OVER_ALL))
+  {
+    p += xsnprintf (p, endp - p, ";n");
+  }
   else if (step
       /* GDB is willing to range step.  */
       && use_range_stepping
@@ -4761,18 +4795,6 @@
          it).  */
       && !(remote_multi_process_p (rs) && ptid_is_pid (ptid)))
     {
-      struct thread_info *tp;
-
-      if (ptid_equal (ptid, minus_one_ptid))
-    {
-      /* If we don't know about the target thread's tid, then
-         we're resuming magic_null_ptid (see caller).  */
-      tp = find_thread_ptid (magic_null_ptid);
-    }
-      else
-    tp = find_thread_ptid (ptid);
-      gdb_assert (tp != NULL);
-
       if (tp->control.may_range_step)
    {
      int addr_size = gdbarch_addr_bit (target_gdbarch ()) / 8;
@@ -4790,6 +4812,14 @@
     p += xsnprintf (p, endp - p, ";s");
   else if (siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";C%02x", siggnal);
+  else if (
+       /* GDB is willing to step next and target supports it */
+       use_finish_stepping && rs->supports_vCont.f &&
+       /* step finish is suitable for this resumption */
+       (tp->control.proceed_to_finish == 1))
+  {
+    p += xsnprintf (p, endp - p, ";f");
+  }
   else
     p += xsnprintf (p, endp - p, ";c");

@@ -11760,6 +11790,82 @@
     }
}

+/* The "set/show next-stepping" show hook.  */
+
+static void
+show_next_stepping (struct ui_file *file, int from_tty,
+             struct cmd_list_element *c,
+             const char *value)
+{
+  fprintf_filtered (file,
+            _("Debugger's willingness to use next stepping "
+              "is %s.\n"), value);
+}
+
+/* The "set/show next-stepping" set hook.  */
+
+static void
+set_next_stepping (char *ignore_args, int from_tty,
+            struct cmd_list_element *c)
+{
+  /* When enabling, check whether next stepping is actually
+     supported by the target, and warn if not.  */
+  if (use_next_stepping)
+    {
+      if (remote_desc != NULL)
+    {
+      struct remote_state *rs = get_remote_state ();
+
+      if (remote_protocol_packets[PACKET_vCont].support == PACKET_SUPPORT_UNKNOWN)
+        remote_vcont_probe (rs);
+
+      if (remote_protocol_packets[PACKET_vCont].support == PACKET_ENABLE
+          && rs->supports_vCont.n)
+        return;
+    }
+
+      warning (_("Next stepping is not supported by the current target"));
+    }
+}
+
+/* The "set/show finish-stepping" show hook.  */
+
+static void
+show_finish_stepping (struct ui_file *file, int from_tty,
+             struct cmd_list_element *c,
+             const char *value)
+{
+  fprintf_filtered (file,
+            _("Debugger's willingness to use finish stepping "
+              "is %s.\n"), value);
+}
+
+/* The "set/show finish-stepping" set hook.  */
+
+static void
+set_finish_stepping (char *ignore_args, int from_tty,
+            struct cmd_list_element *c)
+{
+  /* When enabling, check whether finish stepping is actually
+     supported by the target, and warn if not.  */
+  if (use_finish_stepping)
+    {
+      if (remote_desc != NULL)
+    {
+      struct remote_state *rs = get_remote_state ();
+
+      if (remote_protocol_packets[PACKET_vCont].support == PACKET_SUPPORT_UNKNOWN)
+        remote_vcont_probe (rs);
+
+      if (remote_protocol_packets[PACKET_vCont].support == PACKET_ENABLE
+          && rs->supports_vCont.f)
+        return;
+    }
+
+      warning (_("Finish stepping is not supported by the current target"));
+    }
+}
+
void
_initialize_remote (void)
{
@@ -12168,6 +12274,34 @@
                  &setlist,
                  &showlist);

+  add_setshow_boolean_cmd ("next-stepping", class_run,
+                 &use_next_stepping, _("\
+  Enable or disable next stepping."), _("\
+  Show whether target-assisted next stepping is enabled."), _("\
+  If on, and the target supports it, when skipping over a source line, GDB\n\
+  tells the target to skip the next instruction itself instead of\n\
+  of issuing multiple single-steps.  This speeds up source level\n\
+  stepping.  If off, GDB always issues slower stepping mechanisms\n\
+  instead, even if next stepping is supported by the target.  The default is on."),
+                 set_next_stepping,
+                 show_next_stepping,
+                 &setlist,
+                 &showlist);
+
+  add_setshow_boolean_cmd ("finish-stepping", class_run,
+                 &use_finish_stepping, _("\
+  Enable or disable finish stepping."), _("\
+  Show whether target-assisted finish stepping is enabled."), _("\
+  If on, and the target supports it, when stepping out of a function, GDB\n\
+  tells the target to step out of the corresponding stack frame itself.\n\
+  This speeds up source level stepping. If off, GDB issues slower\n\
+  stepping mechanisms instead, even if finish\n\
+  stepping is supported by the target.  The default is on."),
+                 set_finish_stepping,
+                 show_finish_stepping,
+                 &setlist,
+                 &showlist);
+
   /* Eventually initialize fileio.  See fileio.c */
  initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);




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

* Re: Extending RSP with vCont;n and vCont;f
  2013-10-02 15:08 Extending RSP with vCont;n and vCont;f ILG.Robert
@ 2013-10-02 16:25 ` Eli Zaretskii
  2013-10-04  6:49 ` Yao Qi
  1 sibling, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2013-10-02 16:25 UTC (permalink / raw)
  To: ILG.Robert; +Cc: gdb-patches

> From: "ILG.Robert" <R.ILG@bachmann.info>
> Date: Wed, 2 Oct 2013 15:08:02 +0000
> 
> we would like to improve the RSP command "vCont" for remote debugging. It seems that the range option has been introduced recently to get rid of time consuming, successive single-steps. Our intention is to get rid of further, unnecessary RSP packages being sent as our target is capable to do a step-over (called next by GDB) and a step-return (called finish by GDB). Therefore we propose to extend the vCont command with "vCont;n" and "vCont;f" as well.

Thanks.

> +  add_setshow_boolean_cmd ("finish-stepping", class_run,
> +                 &use_finish_stepping, _("\
> +  Enable or disable finish stepping."), _("\
> +  Show whether target-assisted finish stepping is enabled."), _("\
> +  If on, and the target supports it, when stepping out of a function, GDB\n\
> +  tells the target to step out of the corresponding stack frame itself.\n\
> +  This speeds up source level stepping. If off, GDB issues slower\n\
                                         ^^
Two spaces between sentences, please.


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

* Re: Extending RSP with vCont;n and vCont;f
  2013-10-02 15:08 Extending RSP with vCont;n and vCont;f ILG.Robert
  2013-10-02 16:25 ` Eli Zaretskii
@ 2013-10-04  6:49 ` Yao Qi
  1 sibling, 0 replies; 9+ messages in thread
From: Yao Qi @ 2013-10-04  6:49 UTC (permalink / raw)
  To: ILG.Robert; +Cc: gdb-patches

On 10/02/2013 11:08 PM, ILG.Robert wrote:
> void
> _initialize_remote (void)
> {
> @@ -12168,6 +12274,34 @@
>                    &setlist,
>                    &showlist);
>
> +  add_setshow_boolean_cmd ("next-stepping", class_run,
> +                 &use_next_stepping, _("\
> +  Enable or disable next stepping."), _("\
> +  Show whether target-assisted next stepping is enabled."), _("\
> +  If on, and the target supports it, when skipping over a source line, GDB\n\
> +  tells the target to skip the next instruction itself instead of\n\
> +  of issuing multiple single-steps.  This speeds up source level\n\
> +  stepping.  If off, GDB always issues slower stepping mechanisms\n\
> +  instead, even if next stepping is supported by the target.  The default is on."),

Due to lack of description of these two packets, I have to understand
them from the help message of the command.

IIUC, it is similar to range-stepping, vCont;r except that start and 
stop address are not provided to the stub.  That means vCont;n requires 
a smarter stub to know enough line table information to decide where to 
stop.

An alternative to me is to teach your stub understand range stepping.

> +                 set_next_stepping,
> +                 show_next_stepping,
> +                 &setlist,
> +                 &showlist);
> +
> +  add_setshow_boolean_cmd ("finish-stepping", class_run,
> +                 &use_finish_stepping, _("\
> +  Enable or disable finish stepping."), _("\
> +  Show whether target-assisted finish stepping is enabled."), _("\
> +  If on, and the target supports it, when stepping out of a function, GDB\n\
> +  tells the target to step out of the corresponding stack frame itself.\n\
> +  This speeds up source level stepping. If off, GDB issues slower\n\
> +  stepping mechanisms instead, even if finish\n\
> +  stepping is supported by the target.  The default is on."),

GDB inserts a momentary breakpoint, resume the inferior and wait when
command "finish" is executed.  I don't think GDB emits a large number of 
RSP packets in this process.  Do you have an experiment that vCont;f
packet improves performance to some extent?

-- 
Yao (齐尧)


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

* Re: Extending RSP with vCont;n and vCont;f
@ 2013-11-08  8:18 ILG.Robert
  0 siblings, 0 replies; 9+ messages in thread
From: ILG.Robert @ 2013-11-08  8:18 UTC (permalink / raw)
  To: gdb-patches

First of all I have to say "thank you" and I have to withdraw the "vCont"-patch. 
All arguments and problems have been solved with your help!

The rest of this mail will explain the actual reason for our problem, will answer your questions
and sums up the solution of handling "read-only" breakpoints.

> Sorry for the delayed reply.
Now I'm late, but it took some time to trace down the problem.

It turned out that we have had some wrong settings concerning the variable "DECR_PC_AFTER_BREAK".
Normally this variable is set to -1 so that the EIP-Address is corrected to the actual breakpoint
address. Our GDB-Stub already delivers the correct EIP-Address so that GDB was misinterpreting the
breakpoints for some special cases. As a consequence GDB directly changed the EIP-Register which 
caused those random problems. One of this special case happened to be at the same place as our
"read-only" problem so that I accused GDB in vain. Sorry about that!

>> On 10/09/2013 07:37 PM, ILG.Robert wrote:
>> I haven't been able to trace back the exact problem. If the target denies to insert a 
>> breakpoint for "finish", GDB will crash later while debugging because it suddenly uses 
>> rotten addresses. If GDB is not informed about the problem of setting such a 
>> breakpoints, you can continue debugging without any problem. It looks like as GDB 
>> contains an incomplete error handling.

> I hacked GDB a little to force GDB setting momentary breakpoint on address 0x0 when 
> command 'finish' is typed. Then, I get: 
>
> (gdb) finish
> Run till exit from #0 break_me () at ../../../../git/gdb/testsuite/gdb.base/frame-args.c:35 
> Warning:
> Cannot insert breakpoint 0.
> Cannot access memory at address 0x0
>
> 0x080483c8 in break_me () at ../../../../git/gdb/testsuite/gdb.base/frame-args.c:35 
> 35      }
>
> Looks GDB handles this error well.  Can you see this warning in your target?
Yes, I can see the same output. After the problem of "DECR_PC_AFTER_BREAK" has been fixed 
the following output comes along (Hz-Packages and m-Packages removed for better readability):
    Sending packet: $Z0,7564605,1#84...Packet received: E02
    Sending packet: $z0,6a9d31f,1#31...Packet received: OK
    Sending packet: $vCont?#49...Packet received: vCont;c;C;s;S;t;f;n
  Packet vCont (verbose-resume) is supported
    Sending packet: $vCont;s:23e5af8#f0...Packet received: OK
    Notification received: Stop:T0505:785a3e02;04:5c5a3e02;08:d0e3a906;thread:23e5af8;
    Sending packet: $vStopped#55...Packet received: OK
    Sending packet: $Z0,6a9d31f,1#11...Packet received: OK
    Sending packet: $Z0,7564605,1#84...Packet received: E02
  Warning:
  Cannot insert breakpoint 0.
  Error accessing memory address 0x7564605: (undocumented errno -1).

  cycle () at ugn.c:81
  81      {

>
>> So the real questions are:
>> Here are my answers, and other people may have their answers too.
>> Is it intended to skip unknown/read-only code in GDB?
>
>IMO, it is not right to skip unknown/read-only code for command "finish".
>
>> If yes, has it to be solved within GDB or within the target?
> Generally, hardware breakpoints can be used for read-only regions. If your hardware 
> has hw breakpoints, GDB or your stub can switch breakpoint to hw breakpoint if the 
> region is read-only or the address is within your system code. Looks it is easier 
> to do it inside your stub. People who familiar with breakpoint can give comments too. 

Pedro Alves was so kind to point out the already existing solution to handle "read-only" breakpoints.
Unfortunately I've not been able to implement it yet, as it is too close for our version to be shipped.
".for the GDB side, see "set breakpoint auto-hw on" at:

  https://sourceware.org/gdb/onlinedocs/gdb/Set-Breaks.html
"

Thanks again,
Robert


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

* Re: Extending RSP with vCont;n and vCont;f
@ 2013-10-23  7:19 ILG.Robert
  0 siblings, 0 replies; 9+ messages in thread
From: ILG.Robert @ 2013-10-23  7:19 UTC (permalink / raw)
  To: yao, palves; +Cc: gdb-patches

As the question concerning the proposed patch has passed silently within the mass of requests, I dare to post it again.

How does GDB cope with read-only code, that does not allow to insert breakpoints?

Here the problem is described in more detail:
> All the same "finish" is needed in order to handle the GDB "finish" correctly. 
> Remember that our target cannot insert breakpoints to system code, like 
> other targets cannot alter read-only memory. So if you trigger the system 
> to call a call-back function of your code, you cannot step back. The Stack 
> would look like this: 
> MyCode1-->SystemCode2-->...-->Systemcode4-->MyCode5. 
> As you cannot insert a breakpoint to SystemCode4 there is no way to 
> "finish" until MyCode1. In such a case GDB or the target need to realize 
> that the code in between cannot be controlled with breakpoints and 
> therefore the breakpoint needs to be set to the return address of MyCode1 
> - skipping all code in between. GDB does not skip unknown code at the 
> moment, so the question is whether skipping unknown Code has to be done 
> by GDB or by the target (remote by using "finish").
>
> So the real questions are:
> Is it intended to skip unknown/read-only code in GDB?
> If yes, has it to be solved within GDB or within the target?

Thanks.

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

* Re: Extending RSP with vCont;n and vCont;f
  2013-10-06  7:58 ` Yao Qi
@ 2013-10-07 12:05   ` Pedro Alves
  0 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2013-10-07 12:05 UTC (permalink / raw)
  To: Yao Qi; +Cc: ILG.Robert, gdb-patches

On 10/06/2013 08:56 AM, Yao Qi wrote:
> On 10/04/2013 08:53 PM, ILG.Robert wrote:
>> Second our system contains some assembler functions which do not build a stack frame as usually done by compiled c-code. When debugging these assembler functions GDB reacts as usual: it detects the call of external code and does a "continue" to the return address of the current stack frame. Yet this return address is not the caller - it is the caller of the caller and therefore a "step-into" does not only skip undebugable code, it also steps out of the currently debugged function.
>>
> 
> IMO, it is not a problem specific to your target.  It needs a fix in GDB 
> rather than proposing a new rsp packet.

Or really, those assembly functions have wrong or missing unwind info.

-- 
Pedro Alves


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

* Re: Extending RSP with vCont;n and vCont;f
  2013-10-04 12:53 ILG.Robert
@ 2013-10-06  7:58 ` Yao Qi
  2013-10-07 12:05   ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2013-10-06  7:58 UTC (permalink / raw)
  To: ILG.Robert; +Cc: gdb-patches

On 10/04/2013 08:53 PM, ILG.Robert wrote:
> According to your comments, you got it completely right. Please let me know if there is another place where I have to describe these two packages in detail.
>

The mail body is a good place to describe these packets.  IWBN to give
the rsp traffic with and without the patch, and highlight the
difference.  gdb/doc/gdb.texinfo should be updated to include these two
new packets, but you can do it later.

>>> >>...
>> >IIUC, it is similar to range-stepping, vCont;r except that start and
>> >stop address are not provided to the stub.  That means vCont;n requires a smarter stub to know enough line table information to decide where to stop.
>> >
>> >An alternative to me is to teach your stub understand range stepping.
>>> >>...
>> >GDB inserts a momentary breakpoint, resume the inferior and wait when
>> >command "finish" is executed.  I don't think GDB emits a large number
>> >of RSP packets in this process.  Do you have an experiment that vCont;f packet improves performance to some extent?
> Indeed your comments are good points if only performance increase is considered. Yet we have other issues that require these packages to be used.
>
> First of all we are debugging on a target that has no memory management per task/thread. To be more specific the controller manages memory access block wise. So whenever GDB sets a breakpoint to a memory address within a protected block (that is not part of the application being debugged), an exception on the controller would result. We avoid this by ignoring breakpoints to these well-known, protected blocks within our gdb-stub. If our target does not act differently on "next" and "finish", debugging with GDB runs into a dead end. Also consider that some target have read-only memory, where this will cause problems as well.
>

I am not familiar with your target, so I don't understand why
"debugging with GDB runs into a dead end".  Sounds like breakpoint is 
supported on your stub, but why does "finish" not work properly?

> Second our system contains some assembler functions which do not build a stack frame as usually done by compiled c-code. When debugging these assembler functions GDB reacts as usual: it detects the call of external code and does a "continue" to the return address of the current stack frame. Yet this return address is not the caller - it is the caller of the caller and therefore a "step-into" does not only skip undebugable code, it also steps out of the currently debugged function.
>

IMO, it is not a problem specific to your target.  It needs a fix in GDB 
rather than proposing a new rsp packet.

> All in all our system has some limitations that are special for low-level targets. Our proposal is a  generalization on how GDB is currently working. Currently GDB does not expect the target to be able to do a step-over or a step-into. Yet a specific target should be able to override this behaviour by telling GDB that it is capable to do so. Such the target can do target-specific staff whatever is concerned (read-only memory, memory protection, timing, mutex - whatever comes into your mind). Step-range does not cover these scenarios and introducing specific code for different remote targets to GDB source seems to be wrong  to me.

Looks your motivation is for functional purpose, range stepping is not
suitable to you.  Your generalization doesn't look reasonable to me.
We can have a look at supported actions of vCont, 'c,C,s,S,t,r'.  They
are quite low-level and primitive.  However, in your proposal, vCont;n 
and vCont;f are about "step to a new line" and "step out of this 
function", why are quite high-level to me.

On the other hand, these two packets can't be exercised in other 
people's environment, because we don't have your stub and gdbserver 
doesn't support them.  I am afraid that these piece of code will be 
rotten in the tree as GDB evolves.

-- 
Yao (齐尧)


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

* RE: Extending RSP with vCont;n and vCont;f
@ 2013-10-04 12:53 ILG.Robert
  2013-10-06  7:58 ` Yao Qi
  0 siblings, 1 reply; 9+ messages in thread
From: ILG.Robert @ 2013-10-04 12:53 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2829 bytes --]

> Due to lack of description of these two packets, I have to understand them from the help message of the command.

According to your comments, you got it completely right. Please let me know if there is another place where I have to describe these two packages in detail.

>> ...
> IIUC, it is similar to range-stepping, vCont;r except that start and 
> stop address are not provided to the stub.  That means vCont;n requires a smarter stub to know enough line table information to decide where to stop.
>
> An alternative to me is to teach your stub understand range stepping.
>> ...
> GDB inserts a momentary breakpoint, resume the inferior and wait when 
> command "finish" is executed.  I don't think GDB emits a large number 
> of RSP packets in this process.  Do you have an experiment that vCont;f packet improves performance to some extent?

Indeed your comments are good points if only performance increase is considered. Yet we have other issues that require these packages to be used.

First of all we are debugging on a target that has no memory management per task/thread. To be more specific the controller manages memory access block wise. So whenever GDB sets a breakpoint to a memory address within a protected block (that is not part of the application being debugged), an exception on the controller would result. We avoid this by ignoring breakpoints to these well-known, protected blocks within our gdb-stub. If our target does not act differently on "next" and "finish", debugging with GDB runs into a dead end. Also consider that some target have read-only memory, where this will cause problems as well.

Second our system contains some assembler functions which do not build a stack frame as usually done by compiled c-code. When debugging these assembler functions GDB reacts as usual: it detects the call of external code and does a "continue" to the return address of the current stack frame. Yet this return address is not the caller - it is the caller of the caller and therefore a "step-into" does not only skip undebugable code, it also steps out of the currently debugged function. 

All in all our system has some limitations that are special for low-level targets. Our proposal is a  generalization on how GDB is currently working. Currently GDB does not expect the target to be able to do a step-over or a step-into. Yet a specific target should be able to override this behaviour by telling GDB that it is capable to do so. Such the target can do target-specific staff whatever is concerned (read-only memory, memory protection, timing, mutex - whatever comes into your mind). Step-range does not cover these scenarios and introducing specific code for different remote targets to GDB source seems to be wrong  to me.

Thanks,
Robert
\x16º&Öéj×!zÊÞ¶êç×N´ÛÙb²Ö«r\x18\x1dn–­r\x17¬

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

* RE: Extending RSP with vCont;n and vCont;f
@ 2013-10-03  5:04 ILG.Robert
  0 siblings, 0 replies; 9+ messages in thread
From: ILG.Robert @ 2013-10-03  5:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

Hello again,

Thank you for finding the documentation error. Below is the whole patch with the correction.

Thanks,
Robert

>> 
>> we would like to improve the RSP command "vCont" for remote debugging. It seems that the range option has been 
>>introduced recently to get rid of time consuming, successive single-steps. Our intention is to get rid of further, 
>>unnecessary RSP packages being sent as our target is capable to do a step-over (called next by GDB) and a step-return 
> > (called finish by GDB). Therefore we propose to extend the vCont command with "vCont;n" and "vCont;f" as well.
>
>Thanks.
>
>> +  add_setshow_boolean_cmd ("finish-stepping", class_run,
>> +                 &use_finish_stepping, _("\
>> +  Enable or disable finish stepping."), _("\
>> +  Show whether target-assisted finish stepping is enabled."), _("\
>> +  If on, and the target supports it, when stepping out of a function, 
>> +GDB\n\
>> +  tells the target to step out of the corresponding stack frame 
>> +itself.\n\
>> +  This speeds up source level stepping. If off, GDB issues slower\n\
>                                         ^^ Two spaces between sentences, please.

diff --git a/gdb/remote.c b/gdb/remote.c
index a9ef297..3522236 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -256,11 +256,19 @@
 
   /* vCont;r */
   int r;
+
+  /* vCont;f */
+  int f;
+
+  /* vCont;n */
+  int n;
 };
 
 /* Controls whether GDB is willing to use range stepping.  */
 
 static int use_range_stepping = 1;
+static int use_finish_stepping = 1;
+static int use_next_stepping = 1;
 
 #define OPAQUETHREADBYTES 8
 
@@ -4704,6 +4712,8 @@
       support_C = 0;
       rs->supports_vCont.t = 0;
       rs->supports_vCont.r = 0;
+      rs->supports_vCont.n = 0;
+      rs->supports_vCont.f = 0;
       while (p && *p == ';')
 	{
 	  p++;
@@ -4719,6 +4729,10 @@
 	    rs->supports_vCont.t = 1;
 	  else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
 	    rs->supports_vCont.r = 1;
+      else if (*p == 'f' && (*(p + 1) == ';' || *(p + 1) == 0))
+        rs->supports_vCont.f = 1;
+      else if (*p == 'n' && (*(p + 1) == ';' || *(p + 1) == 0))
+        rs->supports_vCont.n = 1;
 
 	  p = strchr (p, ';');
 	}
@@ -4747,9 +4761,29 @@
 		   ptid_t ptid, int step, enum gdb_signal siggnal)
 {
   struct remote_state *rs = get_remote_state ();
+  struct thread_info *tp;
+
+  if (ptid_equal (ptid, minus_one_ptid))
+  {
+    /* If we don't know about the target thread's tid, then
+     * we're resuming magic_null_ptid (see caller).  */
+    tp = find_thread_ptid (magic_null_ptid);
+   }
+   else
+     tp = find_thread_ptid (ptid);
 
+   gdb_assert (tp != NULL);
+
   if (step && siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";S%02x", siggnal);
+  else if (
+       /* GDB is willing to step next and target supports it */
+       use_next_stepping && rs->supports_vCont.n &&
+       /* step next is suitable for this resumption */
+       step && (tp->control.step_over_calls == STEP_OVER_ALL))
+  {
+    p += xsnprintf (p, endp - p, ";n");
+  }
   else if (step
 	   /* GDB is willing to range step.  */
 	   && use_range_stepping
@@ -4761,18 +4795,6 @@
 	      it).  */
 	   && !(remote_multi_process_p (rs) && ptid_is_pid (ptid)))
     {
-      struct thread_info *tp;
-
-      if (ptid_equal (ptid, minus_one_ptid))
-	{
-	  /* If we don't know about the target thread's tid, then
-	     we're resuming magic_null_ptid (see caller).  */
-	  tp = find_thread_ptid (magic_null_ptid);
-	}
-      else
-	tp = find_thread_ptid (ptid);
-      gdb_assert (tp != NULL);
-
       if (tp->control.may_range_step)
 	{
 	  int addr_size = gdbarch_addr_bit (target_gdbarch ()) / 8;
@@ -4790,6 +4812,14 @@
     p += xsnprintf (p, endp - p, ";s");
   else if (siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";C%02x", siggnal);
+  else if (
+       /* GDB is willing to step next and target supports it */
+       use_finish_stepping && rs->supports_vCont.f &&
+       /* step finish is suitable for this resumption */
+       (tp->control.proceed_to_finish == 1))
+  {
+    p += xsnprintf (p, endp - p, ";f");
+  }
   else
     p += xsnprintf (p, endp - p, ";c");
 
@@ -11760,6 +11790,82 @@
     }
 }
 
+/* The "set/show next-stepping" show hook.  */
+
+static void
+show_next_stepping (struct ui_file *file, int from_tty,
+             struct cmd_list_element *c,
+             const char *value)
+{
+  fprintf_filtered (file,
+            _("Debugger's willingness to use next stepping "
+              "is %s.\n"), value);
+}
+
+/* The "set/show next-stepping" set hook.  */
+
+static void
+set_next_stepping (char *ignore_args, int from_tty,
+            struct cmd_list_element *c)
+{
+  /* When enabling, check whether next stepping is actually
+     supported by the target, and warn if not.  */
+  if (use_next_stepping)
+    {
+      if (remote_desc != NULL)
+    {
+      struct remote_state *rs = get_remote_state ();
+
+      if (remote_protocol_packets[PACKET_vCont].support == PACKET_SUPPORT_UNKNOWN)
+        remote_vcont_probe (rs);
+
+      if (remote_protocol_packets[PACKET_vCont].support == PACKET_ENABLE
+          && rs->supports_vCont.n)
+        return;
+    }
+
+      warning (_("Next stepping is not supported by the current target"));
+    }
+}
+
+/* The "set/show finish-stepping" show hook.  */
+
+static void
+show_finish_stepping (struct ui_file *file, int from_tty,
+             struct cmd_list_element *c,
+             const char *value)
+{
+  fprintf_filtered (file,
+            _("Debugger's willingness to use finish stepping "
+              "is %s.\n"), value);
+}
+
+/* The "set/show finish-stepping" set hook.  */
+
+static void
+set_finish_stepping (char *ignore_args, int from_tty,
+            struct cmd_list_element *c)
+{
+  /* When enabling, check whether finish stepping is actually
+     supported by the target, and warn if not.  */
+  if (use_finish_stepping)
+    {
+      if (remote_desc != NULL)
+    {
+      struct remote_state *rs = get_remote_state ();
+
+      if (remote_protocol_packets[PACKET_vCont].support == PACKET_SUPPORT_UNKNOWN)
+        remote_vcont_probe (rs);
+
+      if (remote_protocol_packets[PACKET_vCont].support == PACKET_ENABLE
+          && rs->supports_vCont.f)
+        return;
+    }
+
+      warning (_("Finish stepping is not supported by the current target"));
+    }
+}
+
 void
 _initialize_remote (void)
 {
@@ -12168,6 +12274,34 @@
 			   &setlist,
 			   &showlist);
 
+  add_setshow_boolean_cmd ("next-stepping", class_run,
+                 &use_next_stepping, _("\
+  Enable or disable next stepping."), _("\
+  Show whether target-assisted next stepping is enabled."), _("\
+  If on, and the target supports it, when skipping over a source line, GDB\n\
+  tells the target to skip the next instruction itself instead of\n\
+  of issuing multiple single-steps.  This speeds up source level\n\
+  stepping.  If off, GDB always issues slower stepping mechanisms\n\
+  instead, even if next stepping is supported by the target.  The default is on."),
+                 set_next_stepping,
+                 show_next_stepping,
+                 &setlist,
+                 &showlist);
+
+  add_setshow_boolean_cmd ("finish-stepping", class_run,
+                 &use_finish_stepping, _("\
+  Enable or disable finish stepping."), _("\
+  Show whether target-assisted finish stepping is enabled."), _("\
+  If on, and the target supports it, when stepping out of a function, GDB\n\
+  tells the target to step out of the corresponding stack frame itself.\n\
+  This speeds up source level stepping.  If off, GDB issues slower\n\
+  stepping mechanisms instead, even if finish\n\
+  stepping is supported by the target.  The default is on."),
+                 set_finish_stepping,
+                 show_finish_stepping,
+                 &setlist,
+                 &showlist);
+
   /* Eventually initialize fileio.  See fileio.c */
   initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);


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

end of thread, other threads:[~2013-11-08  8:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-02 15:08 Extending RSP with vCont;n and vCont;f ILG.Robert
2013-10-02 16:25 ` Eli Zaretskii
2013-10-04  6:49 ` Yao Qi
2013-10-03  5:04 ILG.Robert
2013-10-04 12:53 ILG.Robert
2013-10-06  7:58 ` Yao Qi
2013-10-07 12:05   ` Pedro Alves
2013-10-23  7:19 ILG.Robert
2013-11-08  8:18 ILG.Robert

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