This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: tracing broken if target doesn't do disconnected tracing


On Wednesday 07 April 2010 12:40:36, Pedro Alves wrote:
> On Wednesday 07 April 2010 02:34:12, Stan Shebs wrote:
> > Here's my suggested changes, and of course it implies some target-side 
> > changes as well.  Turns out that I generated some confusion for myself 
> > by auto-generating tstop commands, which I realized when I was unable to 
> > write a code comment that justified it. :-)
> > 
> > So the new behavior is that GDB always simply drops the connection.  If 
> > the target can do disconnected tracing, it just keeps going.  If it 
> > can't, or the query answer instructs it to not keep tracing, it stops, 
> > and records the stop reason as "tdisconnection"; you'll then see that 
> > when you reconnect to the target and do a tstatus.  Since tstatus 
> > reports the target's state consistently now, you'll also see that in a 
> > trace file.  Ditto for circularity of trace buffer.
> > 
> > If this behavior seems reasonable, I'll add appropriate bits to the 
> > manual and repost/commit.
> 
> Thanks!  This is a good improvement, IMO.

Actually, I have one further comment, before you go write documentation
bits:

> +   /* If running interactively, offer the user options for what to do
> +      with the run.  Scripts are just going to disconnect and let the
> +      target deal with it, according to how it's been instructed
> +      previously via disconnected-tracing.  */
>     if (current_trace_status ()->running && from_tty)
>       {
> !       if (current_trace_status ()->disconnected_tracing)
> !       {
> !         int cont = query (_("Trace is running.  Continue tracing after detach? "));
> ! 
> !         /* Note that we send the query result without affecting the
> !            user's setting of disconnected_tracing, so that the answer is
> !            a one-time-only.  */
> !         send_disconnected_tracing_value (cont);
> !       }
> !       else
> !       {
> !         if (!query (_("Trace is running but will stop on detach; detach anyway? ")))
> !           error (_("Not confirmed."));
> !       }
>       }

We've had people reporting these queries were confusing
before.

The user has asked to detach: in one branch (the else branch), gdb
would warn trace was running and gave you
the option of canceling the detach.  In the other branch, it
gave you no such option, it always detaches.  No simple way of
going "ooops, right, I have a trace tracing, didn't really
mean to detach yet."  I've been bitten by this before.  Yes, you can
reconnect to the target, but, it's just annoying, and I don't think
people will often want to chose to say "no" to the
"Trace is running.  Continue tracing after detach? " query.
I think they'll want to say "don't detach yet then" often instead.

I suggest we change that to simply:

  /* If running interactively, warn the user a trace run is ongoing.
     She may want to cancel detaching instead.  */
    if (current_trace_status ()->running && from_tty)
      {
        if (current_trace_status ()->disconnected_tracing)
          {
            if (!query (_("Trace is running and will continue after detach; detach anyway? ")))
              error (_("Not confirmed."));
          }
        else
          {
            if (!query (_("Trace is running but will stop on detach; detach anyway? ")))
             error (_("Not confirmed."));
          }
      }

  - simpler, more coherent, less explaining, less confusing.

> 
> > As for multi-process, I can see reasons to make these kinds of flags 
> > either per-process or global to the target, so didn't try to guess at 
> > the final decision here. :-)
> 
> The support for the feature is reported by qSupported, hence it's
> certainly target-wide noawadays.  It may or not be desirable to
> be able to select which processes keep tracing on disconnect, so
> a per-status state flag for that also sounds acceptable --- it
> could represent whether tracing will continue for a given process
> after disconnection.  The flag (trace_status->disconnected_tracing)
> being 0 doesn't mean the target doesn't support disconnected
> tracing, so there's still no way for the common code to know it.
> 
> With the current patch, against the gdbserver that doesn't
> support disconnected tracing, I still see this:
> 
>  (gdb) set disconnected-tracing on
>  warning: Target does not support disconnected tracing.
>  (gdb) show disconnected-tracing
>  Whether tracing continues after GDB disconnects is on.
> 
> (yes, I wrote the warning, but it was a hack needed before
> so that quitting wouldn't get stuck.)
> 
> With a user hat on, the "Whether tracing continues after
> GDB disconnects is on." string as above looks confusing
> to me.  Same, or worse for the circular-trace-buffer
> setting, as that one says "target's use of", which isn't
> true, it's solely GDB's intention.
> 
> Is there a way to make this pattern unconfusing?  We're
> going to grow more similar options in the future, and
> this will proliferate if unattended.
> 
> 


-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]