Mirror of the gdb mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Doug Evans <dje@google.com>, gdb@sourceware.org
Subject: Re: vCont: optional packet or not?
Date: Wed, 02 Nov 2016 15:30:00 -0000	[thread overview]
Message-ID: <f81f5840-c9c8-3d26-75fd-72afc08ee090@redhat.com> (raw)
In-Reply-To: <94eb2c043598965907054043d1f7@google.com>

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


      reply	other threads:[~2016-11-02 15:30 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-01 21:15 Doug Evans
2016-11-02 15:30 ` Pedro Alves [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f81f5840-c9c8-3d26-75fd-72afc08ee090@redhat.com \
    --to=palves@redhat.com \
    --cc=dje@google.com \
    --cc=gdb@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox