vCont: optional packet or not?

vCont: optional packet or not?

[Doug Evans]

Hi.

Two main questions/issues:
1) In non-stop mode is vCont a required or optional packet
a stub needs to provide?
2) Trunk seems to have regressed (could be wrong) with respect
to first checking whether vCont is supported before using it.

In 7.12 it seems optional. I don't remember exactly, but
the 7.12 code does do some checking whether vCont is supported
before sending vCont packets.

Things are further confusing :-( :-( :-( because
there's also vContSupported which the docs say indicates whether
vCont? is supported, not whether vCont is supported.
[The name could be a teensy bit better ...]
vCont? will tell gdb whether vCont is supported (including
what flavors (c,C,s,S,t,...), but gdb only uses this to check
whether vCont;[sS] (singlestepping) is supported, not whether e.g.,
vCont;[cC] (continue) is supported.

So where's the check for whether vCont;[cC] is supported?

Things have changed from 7.12, I think, in trunk.
I'm seeing gdb issue vCont;c packets and complaining
about bad replies from my stub, but it has never asked
or even checked whether the packet is supported.
[e.g., remote_commit_resume just blindly sends vCont.]

What's the story here?

If someone can describe how things are intended to work,
I'll volunteer to fix/enhance the remote protocol spec
in the docs. [Or you can directly.]

Re: vCont: optional packet or not?

[Pedro Alves]

On 11/01/2016 09:15 PM, Doug Evans wrote:
> Hi.
> 
> Two main questions/issues:
> 1) In non-stop mode is vCont a required or optional packet
> a stub needs to provide?

I thought we already documented that it's required, but I could
only find this comment in gdb/remote.c:

...
     only to the base all-stop protocol, however.  In non-stop (which
     only supports vCont), the stub replies with an "OK", and is
     immediate able to process further serial input.  */
  if (!target_is_non_stop_p ())
    rs->waiting_for_stop_reply = 1;

All along I though it was mentioned in the docs somewhere.  :-(

Since only vCont is asynchronous (s/c/S/C are not), it seems
useless to support non-stop without vCont ?  

We do still send the reverse variants of s/c in non-stop mode,
since there are no vCont equivalents...  So maybe we should
instead say vCont is recommended instead of required?

> 2) Trunk seems to have regressed (could be wrong) with respect
> to first checking whether vCont is supported before using it.
> In 7.12 it seems optional. I don't remember exactly, but
> the 7.12 code does do some checking whether vCont is supported
> before sending vCont packets.
> 
> Things are further confusing :-( :-( :-( because
> there's also vContSupported which the docs say indicates whether
> vCont? is supported, not whether vCont is supported.
> [The name could be a teensy bit better ...]
> vCont? will tell gdb whether vCont is supported (including
> what flavors (c,C,s,S,t,...), but gdb only uses this to check
> whether vCont;[sS] (singlestepping) is supported, not whether e.g.,
> vCont;[cC] (continue) is supported.
> 
> So where's the check for whether vCont;[cC] is supported?

I agree it's confusing
<https://sourceware.org/ml/gdb-patches/2015-09/msg00324.html>.

> Things have changed from 7.12, I think, in trunk.
> I'm seeing gdb issue vCont;c packets and complaining
> about bad replies from my stub, but it has never asked
> or even checked whether the packet is supported.
> [e.g., remote_commit_resume just blindly sends vCont.]
> 
> What's the story here?

Right, I didn't add any check to see if vCont was supported
in that code path, with the assumption that no non-stop stub
would skip implementing vCont.  The coordination between
remote_resume and remote_commit_resume is done at the top
of each of those functions:

static void
remote_resume (struct target_ops *ops,
	       ptid_t ptid, int step, enum gdb_signal siggnal)
{
  struct remote_state *rs = get_remote_state ();

  /* When connected in non-stop mode, the core resumes threads
     individually.  Resuming remote threads directly in target_resume
     would thus result in sending one packet per thread.  Instead, to
     minimize roundtrip latency, here we just store the resume
     request; the actual remote resumption will be done in
     target_commit_resume / remote_commit_resume, where we'll be able
     to do vCont action coalescing.  */
  if (target_is_non_stop_p () && execution_direction != EXEC_REVERSE)
...

/* to_commit_resume implementation.  */

static void
remote_commit_resume (struct target_ops *ops)
{
  struct remote_state *rs = get_remote_state ();
  struct inferior *inf;
  struct thread_info *tp;
  int any_process_wildcard;
  int may_global_wildcard_vcont;
  struct vcont_builder vcont_builder;

  /* If connected in all-stop mode, we'd send the remote resume
     request directly from remote_resume.  Likewise if
     reverse-debugging, as there are no defined vCont actions for
     reverse execution.  */
  if (!target_is_non_stop_p () || execution_direction == EXEC_REVERSE)
    return;
...


It should be doable to include a "vCont is supported" check here.
Not sure it's worth it though.

> If someone can describe how things are intended to work,
> I'll volunteer to fix/enhance the remote protocol spec
> in the docs. [Or you can directly.]

Thanks,
Pedro Alves