[remote protocol] Allow qSymbol response to continue packets

[remote protocol] Allow qSymbol response to continue packets

[Daniel Jacobowitz]

I'd like to propose the attached as an extension to the remote protocol.
The code implementing this change is here:
  http://sources.redhat.com/ml/gdb-patches/2004-02/msg00000.html

From my original post:
  As Amit Kale mentioned in December, to support NPTL gdbserver needs to
  look up symbols during remote_wait.  The existing qSymbol model assumes
  that only at objfile loads (i.e. during td_ta_new) do we need to look up
  symbols; NPTL looks up symbols lazily when it needs them, which includes
  at the creation of the first child thread.  This patch (which, I know,
  needs a matching change for the manual) allows qSymbol: queries as a
  response to remote_wait, in much the same way as the file I/O protocol.

So here's the manual page and a description of the change.  Thoughts?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-03-06  Daniel Jacobowitz  <drow@mvista.com>

	* gdb.texinfo (Stop Reply Packets): Define qSymbol response.

Index: gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.199
diff -u -p -r1.199 gdb.texinfo
--- gdb.texinfo	29 Feb 2004 02:57:24 -0000	1.199
+++ gdb.texinfo	6 Mar 2004 23:48:15 -0000
@@ -20267,6 +20267,15 @@ packet from the target.  The latest @sam
 @samp{s} action is expected to be continued.
 @xref{File-I/O remote protocol extension}, for more details.
 
+@item qSymbol:@var{sym_name}
+
+The target is requesting the address of a symbol.  @value{GDBN} replies with
+a @code{qSymbol} packet providing the address of @var{sym_name} if available
+(@pxref{General Query Packets}).
+
+As with @code{F}, this response does not terminate the current resume
+action.  @value{GDBN} continues waiting for another stop packet.
+
 @end table
 
 @node General Query Packets

Re: [remote protocol] Allow qSymbol response to continue packets

[Eli Zaretskii]

> Date: Sat, 6 Mar 2004 18:52:53 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> So here's the manual page and a description of the change.  Thoughts?

The manual patch is okay with me, thanks, but please add a @cindex
entry for "@code{qSymbol} packet" somewhere in that vicinity.

Re: [remote protocol] Allow qSymbol response to continue packets

[Andrew Cagney]

> I'd like to propose the attached as an extension to the remote protocol.
> The code implementing this change is here:
>   http://sources.redhat.com/ml/gdb-patches/2004-02/msg00000.html
> 
>>From my original post:
>   As Amit Kale mentioned in December, to support NPTL gdbserver needs to
>   look up symbols during remote_wait.  The existing qSymbol model assumes
>   that only at objfile loads (i.e. during td_ta_new) do we need to look up
>   symbols; NPTL looks up symbols lazily when it needs them, which includes
>   at the creation of the first child thread.  This patch (which, I know,
>   needs a matching change for the manual) allows qSymbol: queries as a
>   response to remote_wait, in much the same way as the file I/O protocol.
> 
> So here's the manual page and a description of the change.  Thoughts?

This isn't sufficient:

> +@item qSymbol:@var{sym_name}
> +
> +The target is requesting the address of a symbol.  @value{GDBN} replies with
> +a @code{qSymbol} packet providing the address of @var{sym_name} if available
> +(@pxref{General Query Packets}).
> +
> +As with @code{F}, this response does not terminate the current resume
> +action.  @value{GDBN} continues waiting for another stop packet.
> +

look through the File-I/O section that discuss cntrl-c.

I think something based on the existing F packet would be better.  At 
least that way we have a situtation where the clear intent is for 
identical semantics.

Andrew

Re: [remote protocol] Allow qSymbol response to continue packets

[Daniel Jacobowitz]

On Thu, Mar 11, 2004 at 03:05:50PM -0500, Andrew Cagney wrote:
> >I'd like to propose the attached as an extension to the remote protocol.
> >The code implementing this change is here:
> >  http://sources.redhat.com/ml/gdb-patches/2004-02/msg00000.html
> >
> >>From my original post:
> >  As Amit Kale mentioned in December, to support NPTL gdbserver needs to
> >  look up symbols during remote_wait.  The existing qSymbol model assumes
> >  that only at objfile loads (i.e. during td_ta_new) do we need to look up
> >  symbols; NPTL looks up symbols lazily when it needs them, which includes
> >  at the creation of the first child thread.  This patch (which, I know,
> >  needs a matching change for the manual) allows qSymbol: queries as a
> >  response to remote_wait, in much the same way as the file I/O protocol.
> >
> >So here's the manual page and a description of the change.  Thoughts?
> 
> This isn't sufficient:
> 
> >+@item qSymbol:@var{sym_name}
> >+
> >+The target is requesting the address of a symbol.  @value{GDBN} replies 
> >with
> >+a @code{qSymbol} packet providing the address of @var{sym_name} if 
> >available
> >+(@pxref{General Query Packets}).
> >+
> >+As with @code{F}, this response does not terminate the current resume
> >+action.  @value{GDBN} continues waiting for another stop packet.
> >+
> 
> look through the File-I/O section that discuss cntrl-c.
> 
> I think something based on the existing F packet would be better.  At 
> least that way we have a situtation where the clear intent is for 
> identical semantics.

Could you explain why you this is necessary?

I'm guessing in the File I/O case this handles the user hitting C-c
while the client is processing a request, and there is considerable
complexity involved in reporting whether the I/O has completed.  But
using errno doesn't make any sense in the symbol lookup context and it
seems to me easier to make GDB delay sending the C-c to the target
until the qSymbol has been processed:
  -> c
  <- qSymbol:AAAAAAAAAAAAA
  Control-C
  -> qSymbol:AAAAAAAAAAAAA:012131312
  -> \003

That keeps the stub implementation much simpler.  And the client
implementation is easy using blocked signals.

Implementing something as you describe would be a bit trickier in
gdbserver; I'd have to force-stop all threads and then fake a SIGINT
event.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [remote protocol] Allow qSymbol response to continue packets

[Andrew Cagney]

>>> look through the File-I/O section that discuss cntrl-c.
>>> 
>>> I think something based on the existing F packet would be better.  At 
>>> least that way we have a situtation where the clear intent is for 
>>> identical semantics.
> 
> 
> Could you explain why you this is necessary?

> I'm guessing in the File I/O case this handles the user hitting C-c
> while the client is processing a request, and there is considerable
> complexity involved in reporting whether the I/O has completed.  But
> using errno doesn't make any sense in the symbol lookup context and it
> seems to me easier to make GDB delay sending the C-c to the target
> until the qSymbol has been processed:
>   -> c
>   <- qSymbol:AAAAAAAAAAAAA
>   Control-C
>   -> qSymbol:AAAAAAAAAAAAA:012131312
>   -> \003

Here's the problem:

> @node The Ctrl-C message
> @subsection The Ctrl-C message
> @cindex ctrl-c message, in file-i/o protocol
> 
> A special case is, if the @var{Ctrl-C flag} is set in the @value{GDBN}
> reply packet.  In this case the target should behave, as if it had
> gotten a break message.  The meaning for the target is ``system call
> interupted by @code{SIGINT}''.  Consequentially, the target should actually stop
> (as with a break message) and return to @value{GDBN} with a @code{T02}
> packet.  In this case, it's important for the target to know, in which
> state the system call was interrupted.  Since this action is by design
> not an atomic operation, we have to differ between two cases:
> 
> @itemize @bullet
> @item
> The system call hasn't been performed on the host yet.
> 
> @item
> The system call on the host has been finished.
> 
> @end itemize
> 
> These two states can be distinguished by the target by the value of the
> returned @code{errno}.  If it's the protocol representation of @code{EINTR}, the
>  system
> call hasn't been performed.  This is equivalent to the @code{EINTR} handling
> on POSIX systems.  In any other case, the target may presume that the
> system call has been finished --- successful or not --- and should behave
> as if the break message arrived right after the system call.
> 
> @value{GDBN} must behave reliable.  If the system call has not been called
> yet, @value{GDBN} may send the @code{F} reply immediately, setting @code{EINTR} 
> as
> @code{errno} in the packet.  If the system call on the host has been finished
> before the user requests a break, the full action must be finshed by
> @value{GDBN}.  This requires sending @code{M} or @code{X} packets as they fit.
> The @code{F} packet may only be send when either nothing has happened
> or the full action has been completed.

A user trying to cntrl-c GDB while GDB is taking its time looking up a 
symbol isn't theory.  There needs to be an error/abort mechanism and 
adopting "F" provides that.

The alternative is to specify some sort of customized q packet semantics 
- giving two callbacks and two different behaviors  - I'm really not 
interested in going there.

> That keeps the stub implementation much simpler.  And the client
> implementation is easy using blocked signals.

> Implementing something as you describe would be a bit trickier in
> gdbserver; I'd have to force-stop all threads and then fake a SIGINT
> event.

You need to handle such race conditions anyway.

-> c
<- qSymbol | cntrl-c ->

Andrew

Re: [remote protocol] Allow qSymbol response to continue packets

[Daniel Jacobowitz]

On Thu, Mar 11, 2004 at 04:27:16PM -0500, Andrew Cagney wrote:
> 
> >>>look through the File-I/O section that discuss cntrl-c.
> >>>
> >>>I think something based on the existing F packet would be better.  At 
> >>>least that way we have a situtation where the clear intent is for 
> >>>identical semantics.
> >
> >
> >Could you explain why you this is necessary?
> 
> >I'm guessing in the File I/O case this handles the user hitting C-c
> >while the client is processing a request, and there is considerable
> >complexity involved in reporting whether the I/O has completed.  But
> >using errno doesn't make any sense in the symbol lookup context and it
> >seems to me easier to make GDB delay sending the C-c to the target
> >until the qSymbol has been processed:
> >  -> c
> >  <- qSymbol:AAAAAAAAAAAAA
> >  Control-C
> >  -> qSymbol:AAAAAAAAAAAAA:012131312
> >  -> \003
> 
> Here's the problem:

I did read the manual when you referenced it, you don't need to paste
the whole thing :)

> A user trying to cntrl-c GDB while GDB is taking its time looking up a 
> symbol isn't theory.  There needs to be an error/abort mechanism and 
> adopting "F" provides that.
> 
> The alternative is to specify some sort of customized q packet semantics 
> - giving two callbacks and two different behaviors  - I'm really not 
> interested in going there.

Perhaps you've noticed that we have hashed minimal symbol table
lookups?  There is no excuse for symbol table lookups to take long
enough for there to need to be an abort mechanism.  This is in contrast
to file I/O which can block.

I don't think we need to use the heavy-weight mechanism which supports
interruption for operations that don't need to be interrupted, and I
can't see a reason to support interruption of this lookup.  If you do,
please enlighten me.


Race conditions obviously need to be handled, which they will be
automatically in the current code.  GDB will write an \003 to the
serial connection, which as I describe below, gdbserver will
immediately consume and process, and then report next time it's asked
to wait (i.e. when the qSymbol is done).


> >That keeps the stub implementation much simpler.  And the client
> >implementation is easy using blocked signals.
> 
> >Implementing something as you describe would be a bit trickier in
> >gdbserver; I'd have to force-stop all threads and then fake a SIGINT
> >event.
> 
> You need to handle such race conditions anyway.
> 
> -> c
> <- qSymbol | cntrl-c ->

That's a different problem, and it is already correctly handled by
gdbserver.  We'll write out the qSymbol, read in the Ctrl-C, signal the
inferior, look again for an ACK, eventually get the ACK.  Then we'd
wait for and get a qSymbol reply, resume the suspended thread that made
the lookup request, wait for it, and see the SIGINT we created.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [remote protocol] Allow qSymbol response to continue packets

[Andrew Cagney]

>>>> >Could you explain why you this is necessary?
>>
>>> 
>>
>>>> >I'm guessing in the File I/O case this handles the user hitting C-c
>>>> >while the client is processing a request, and there is considerable
>>>> >complexity involved in reporting whether the I/O has completed.  But
>>>> >using errno doesn't make any sense in the symbol lookup context and it
>>>> >seems to me easier to make GDB delay sending the C-c to the target
>>>> >until the qSymbol has been processed:
>>>> >  -> c
>>>> >  <- qSymbol:AAAAAAAAAAAAA
>>>> >  Control-C
>>>> >  -> qSymbol:AAAAAAAAAAAAA:012131312
>>>> >  -> \003
>>
>>> 
>>> Here's the problem:
> 
> 
> I did read the manual when you referenced it, you don't need to paste
> the whole thing :)

I quoted the relevant section of the manual as it hopefully explained 
why it was necessary.  If that doesn't answer the question then we've a 
bug in the manual.

>>> A user trying to cntrl-c GDB while GDB is taking its time looking up a 
>>> symbol isn't theory.  There needs to be an error/abort mechanism and 
>>> adopting "F" provides that.
>>> 
>>> The alternative is to specify some sort of customized q packet semantics 
>>> - giving two callbacks and two different behaviors  - I'm really not 
>>> interested in going there.
> 
> 
> Perhaps you've noticed that we have hashed minimal symbol table
> lookups?  There is no excuse for symbol table lookups to take long
> enough for there to need to be an abort mechanism.  This is in contrast
> to file I/O which can block.

Protocol's can't make such assumptions.

> I don't think we need to use the heavy-weight mechanism which supports
> interruption for operations that don't need to be interrupted, and I
> can't see a reason to support interruption of this lookup.  If you do,
> please enlighten me.

I think we'll have to disagree on our definitions of heavy weight (if F 
it is too heavy weight then perhaphs we need to remove a few things from 
it).

The protocol needs to specify the failure states, the F packet provides 
that for free.  As I said, I'm really not interested in cooking up 
another callback packet with a different set of failure states.  One is 
enough.

>>> You need to handle such race conditions anyway.
>>> 
>>> -> c
>>> <- qSymbol | cntrl-c ->
> 
> 
> That's a different problem, and it is already correctly handled by
> gdbserver.  We'll write out the qSymbol, read in the Ctrl-C, signal the
> inferior, look again for an ACK, eventually get the ACK.  Then we'd
> wait for and get a qSymbol reply, resume the suspended thread that made
> the lookup request, wait for it, and see the SIGINT we created.

If you've code to handle that you've code to handle a packet containing:

- <retry><cntrl-C>
- <symbol><cntrl-C>

Andrew

Re: [remote protocol] Allow qSymbol response to continue packets

[Daniel Jacobowitz]

On Thu, Mar 11, 2004 at 06:20:21PM -0500, Andrew Cagney wrote:
> Protocol's can't make such assumptions.

I disagree.  I consider symbol lookup a _fundamentally non-blocking_
operation, and I'm sure there are other similar fundamentally
uninterruptible responses.  For instance, any notification events (for
things similar to tracepoints, though I don't know if it would apply to
our tracepoint implementation, since the remote protocol bits of
tracepoint support do not have documentation that I can find). Or a
revised version of the 'console output' response that doesn't have the
protocol-breaking problems you described to me the last time I tried to
model something after it.

> >I don't think we need to use the heavy-weight mechanism which supports
> >interruption for operations that don't need to be interrupted, and I
> >can't see a reason to support interruption of this lookup.  If you do,
> >please enlighten me.
> 
> I think we'll have to disagree on our definitions of heavy weight (if F 
> it is too heavy weight then perhaphs we need to remove a few things from 
> it).
> 
> The protocol needs to specify the failure states, the F packet provides 
> that for free.  As I said, I'm really not interested in cooking up 
> another callback packet with a different set of failure states.  One is 
> enough.
> 
> >>>You need to handle such race conditions anyway.
> >>>
> >>>-> c
> >>><- qSymbol | cntrl-c ->
> >
> >
> >That's a different problem, and it is already correctly handled by
> >gdbserver.  We'll write out the qSymbol, read in the Ctrl-C, signal the
> >inferior, look again for an ACK, eventually get the ACK.  Then we'd
> >wait for and get a qSymbol reply, resume the suspended thread that made
> >the lookup request, wait for it, and see the SIGINT we created.
> 
> If you've code to handle that you've code to handle a packet containing:
> 
> - <retry><cntrl-C>
> - <symbol><cntrl-C>

That's not correct, unfortunately.  A packet containing
<symbol><cntrl-C>, sure, that's easy.  But <retry><cntrl-C> is pretty
hard.  The call stack at this point goes through the middle of
libthread_db; I can't resume the inferior to give it a real SIGINT
without have to jump through hoops to re-initialize libthread_db after
processing the SIGINT, so I would have to:

 - stop all other running threads, if there are any; there could be
   depending on what libthread_db is trying to look up
 - forge a stop-with-SIGINT packet
 - handle whatever packets GDB sends me while I'm stopped
 - wait for GDB to resume
 - cause the resume to trigger re-issue of the queued qSymbol "stop
   response"
 - repeat until this succeeds
 - return that result to libthread_db

The long and short of it is that I'd have to duplicate or considerably
modularize the packet processing, to make the main loop re-entrant, to
support interruption of an otherwise non-blocking operation.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [remote protocol] Allow qSymbol response to continue packets

[Daniel Jacobowitz]

On Thu, Mar 11, 2004 at 06:38:33PM -0500, Daniel Jacobowitz wrote:
> On Thu, Mar 11, 2004 at 06:20:21PM -0500, Andrew Cagney wrote:
> > Protocol's can't make such assumptions.
> 
> I disagree.

Anyway, let me take a stab at implementing it the way you described.
I still think it's overkill, but I may be able to work out a way to do
it.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [remote protocol] Allow qSymbol response to continue packets

[Andrew Cagney]

> That's not correct, unfortunately.  A packet containing
> <symbol><cntrl-C>, sure, that's easy.  But <retry><cntrl-C> is pretty
> hard.  The call stack at this point goes through the middle of
> libthread_db; I can't resume the inferior to give it a real SIGINT
> without have to jump through hoops to re-initialize libthread_db after
> processing the SIGINT, so I would have to:
> 
>  - stop all other running threads, if there are any; there could be
>    depending on what libthread_db is trying to look up
>  - forge a stop-with-SIGINT packet
>  - handle whatever packets GDB sends me while I'm stopped
>  - wait for GDB to resume
>  - cause the resume to trigger re-issue of the queued qSymbol "stop
>    response"
>  - repeat until this succeeds
>  - return that result to libthread_db
> 
> The long and short of it is that I'd have to duplicate or considerably
> modularize the packet processing, to make the main loop re-entrant, to
> support interruption of an otherwise non-blocking operation.

Unfortunatly, an event driven state machine is the "text book correct" 
way of implementing a protocol (anything less is a crock).  Thats why I 
wrote RDA the way I did, and thats why gdb's remote code continues to 
need an overhaul (hopefully eventually sharing code with gdbserver).  If 
the original author of that code had thought to read s/w telco text, 
your and my life today would have been a lot easier, sigh.

Anyway, if F is used as the interaction model, should F also be used as 
the packet format?  Can I suggest, for queries to GDB, 'f' be used as 
the packet prefix letter but use 'F' for the rest of the specification. 
  That would give us:

<- fSymbol,<symbol-name>

-> f0,0,;<symbol-value>
or
-> f0,EINTR,;
or
-> f0,EINTR,C;

I suspect that is sufficient (looking at the code I think "F" can be 
simplified somewhat - time permitting :-().

The other option would be to generate T00 packets - cause GDB to go 
through the motions but then resume it?

Andrew