Minor gdb-ui patches to make it a bit more robust

Minor gdb-ui patches to make it a bit more robust

[Stefan Monnier]

Here are 2 patches which help me deal with gdb-ui.

The first 2 hunks reset gdb-output-sink as it should after starting
a new process.  I think this is a plain bug fix, but I'll let
Nick decide.

The second hunk make it fallback on the old gud-gdb code in case the
prompt appears before we get to receive the expected annotations.
I've been using M-x gdb RET with "gdb --fullname emacs" for ever and it
took me a while to understand why it suddenly stopped working properly:
the behavior is pretty nasty: you get all the expected GDB output but
your input isn't sent to the gdb process (because gdb-ui thinks that
GDB is still initializing) and completion just hangs (because it sends
a command which isn't passed on to the process and then waits for the
process to reply).

Maybe my hack isn't such a great idea, but it seems to work OK for my
test and it brings back Emacs-21's M-x gdb behavior of automatically
choosing gdb-ui or plain gud-gdb.

Maybe Nick can suggest a more reliable way to detect when the
annotations are missing.


        Stefan


--- orig/lisp/progmodes/gdb-ui.el
+++ mod/lisp/progmodes/gdb-ui.el
@@ -150,7 +150,7 @@
 (defvar gdb-prompting nil
   "True when gdb is idle with no pending input.")
 
-(defvar gdb-output-sink 'user
+(defvar gdb-output-sink nil
   "The disposition of the output of the current gdb command.
 Possible values are these symbols:
 
@@ -317,6 +317,7 @@
   (local-set-key "\C-i" 'gud-gdb-complete-command)
   (setq comint-prompt-regexp "^(.*gdb[+]?) *")
   (setq paragraph-start comint-prompt-regexp)
+  (setq gdb-output-sink 'user)
   (setq gdb-first-prompt t)
   (setq gud-running nil)
   (setq gdb-ready nil)
@@ -1673,6 +1674,16 @@
 	(progn
 	  (setq output (gdb-concat-output output gud-marker-acc))
 	  (setq gud-marker-acc "")))
+      (unless gdb-ready           ;Should we check gdb-first-prompt instead?
+        (when (string-match "\n(gdb) \\'" output)
+          ;; We just received a GDB prompt but haven't seen any of the
+          ;; annotations we expect to come along with it.  Apparently GDB
+          ;; was not run with "annotate=3".  Let's try to fallback on the
+          ;; old mode.
+          (message "Falling back to old GDB support")
+          (set (make-local-variable 'gud-minor-mode) 'gdb)
+          (kill-local-variable 'comint-input-sender)
+          (setq gud-marker-filter 'gud-gdb-marker-filter)))
       output)))
 
 (defun gdb-concat-output (so-far new)

Re: Minor gdb-ui patches to make it a bit more robust

[Nick Roberts]

 > Here are 2 patches which help me deal with gdb-ui.
 > 
 > The first 2 hunks reset gdb-output-sink as it should after starting
 > a new process.  I think this is a plain bug fix, but I'll let
 > Nick decide.

I don't know why you declare gdb-output-sink with a value of nil as it
has no meaning but it's harmless.  In practice, 've not found a need to reset
gdb-output-sink but you presumably have.  So I'm happy if you want to make
these changes.

 > The second hunk make it fallback on the old gud-gdb code in case the
 > prompt appears before we get to receive the expected annotations.
 > I've been using M-x gdb RET with "gdb --fullname emacs" for ever and it
 > took me a while to understand why it suddenly stopped working properly:
 > the behavior is pretty nasty: you get all the expected GDB output but
 > your input isn't sent to the gdb process (because gdb-ui thinks that
 > GDB is still initializing) and completion just hangs (because it sends
 > a command which isn't passed on to the process and then waits for the
 > process to reply).

Using the same startup function (M-x gdb) for graphical mode and text command
mode ("fullname") has given backward compatiblity but caused a lot of grief.

Previously (Emacs 22.1) M-x gdb assumed text command mode until it received
annotations to put it in graphical mode.  This led to error reports where
users wanted graphical mode and had commands in their .gdbinit which started
execution.  My advice was to use M-x gdba.

In EMACS_22_BASE, I switched things round so that M-x gdb assumed graphical
mode until it received a fullname annotation to put it in text command mode.
This solved the above problem.  I also removed M-x gdba and added M-x gud-gdb.

More recently, Robert Marshall made a bug report about start up with M-x gdb
where he entered Gdb user commands before Emacs was ready.  On the *trunk*,
I've solved this by deferring the input rather than discarding it.  I also made
a couple of other related changes, one of which was setting comint-input-sender
to gdb-send earlier.  I think this change has caused the problem you are
seeing.  I've not made this change on EMACS_22_BASE, so I think M-x gdb RET
with "gdb --fullname emacs" will STILL WORK there.

On the trunk, I want to migrate away slightly from text command mode so my
advice there is to use it via M-x gud-gdb now.  I have a patch for gdb-ui.el
which will mean that M-x gdb will only work in graphical mode and doc changes
to reflect this which I propose to commit shortly.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: Minor gdb-ui patches to make it a bit more robust

[Stefan Monnier]

>> Here are 2 patches which help me deal with gdb-ui.
>> The first 2 hunks reset gdb-output-sink as it should after starting
>> a new process.  I think this is a plain bug fix, but I'll let
>> Nick decide.

> I don't know why you declare gdb-output-sink with a value of nil as it
> has no meaning but it's harmless.

This variable should really be a process property, so it makes sense for
it to have an invalid value when no process is running.  Setting it to
an invalid default value therefore intends to help catch the case where
we forget to initialize the variable.

> In practice, I've not found a need to reset gdb-output-sink but you
> presumably have.

Indeed.  I think it's pretty easy to get into such a state if you're
edebugging gdba functions and abort them forcefully.

> So I'm happy if you want to make these changes.

Thanks.

>> The second hunk make it fallback on the old gud-gdb code in case the
>> prompt appears before we get to receive the expected annotations.
>> I've been using M-x gdb RET with "gdb --fullname emacs" for ever and it
>> took me a while to understand why it suddenly stopped working properly:
>> the behavior is pretty nasty: you get all the expected GDB output but
>> your input isn't sent to the gdb process (because gdb-ui thinks that
>> GDB is still initializing) and completion just hangs (because it sends
>> a command which isn't passed on to the process and then waits for the
>> process to reply).

> Using the same startup function (M-x gdb) for graphical mode and text command
> mode ("fullname") has given backward compatiblity but caused a lot of grief.

> Previously (Emacs 22.1) M-x gdb assumed text command mode until it received
> annotations to put it in graphical mode.  This led to error reports where
> users wanted graphical mode and had commands in their .gdbinit which started
> execution.  My advice was to use M-x gdba.

> In EMACS_22_BASE, I switched things round so that M-x gdb assumed graphical
> mode until it received a fullname annotation to put it in text command mode.
> This solved the above problem.  I also removed M-x gdba and added M-x gud-gdb.

Yes, I saw that.  You may want to keep the gdba alias, tho.  I do not
disagree with this change.

> More recently, Robert Marshall made a bug report about start up with
> M-x gdb where he entered Gdb user commands before Emacs was ready.
> On the *trunk*, I've solved this by deferring the input rather than
> discarding it.  I also made a couple of other related changes, one of
> which was setting comint-input-sender to gdb-send earlier.  I think
> this change has caused the problem you are seeing.  I've not made this
> change on EMACS_22_BASE, so I think M-x gdb RET with "gdb --fullname
> emacs" will STILL WORK there.

Indeed, the specific problem I encountered is recent.

> On the trunk, I want to migrate away slightly from text command mode so my
> advice there is to use it via M-x gud-gdb now.  I have a patch for gdb-ui.el
> which will mean that M-x gdb will only work in graphical mode and doc changes
> to reflect this which I propose to commit shortly.

That's OK.  But given the reliability problems of gdb-ui, I think it's
crucial that we take a defensive-coding approach there.  I.e. whenever
possible check our assumptions, and not wait for someone to come up with
a counter example before initializing the variables at the right place.

Detecting a (gdb) prompt when we don't expect it is part of
defensive coding.  Maybe falling back automatically to gud-gdb is not
a good idea, but detecting the situation and signalling it to the user
is important.  There might be all kinds of reasons why the user failed
to provide the --annotate=3 argument.

Also I think a good way to make it more reliable would be to make
it work with several gud buffers by moving most global vars to
process properties, so they're necessarily correctly initialized, and
we'd be forced to think a bit harder about what's going on where.


        Stefan


PS: Is there any hope for GDB to accept a command that puts it in
annotate=3 mode, rather than having to tweak the command line for it?
That would solve a lot of those problems.

Re: Minor gdb-ui patches to make it a bit more robust

[Andreas Schwab]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> PS: Is there any hope for GDB to accept a command that puts it in
> annotate=3 mode, rather than having to tweak the command line for it?
> That would solve a lot of those problems.

You can set the annotation level with set annotate.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Minor gdb-ui patches to make it a bit more robust

[Nick Roberts]

 > Yes, I saw that.  You may want to keep the gdba alias, tho.

That's a good idea, so I've done it in EMACS_22_BASE.

 > Detecting a (gdb) prompt when we don't expect it is part of
 > defensive coding.  Maybe falling back automatically to gud-gdb is not
 > a good idea, but detecting the situation and signalling it to the user
 > is important.  There might be all kinds of reasons why the user failed
 > to provide the --annotate=3 argument.

The flexibility of the command line means that there are always ways round
these types of checks.  For example, the prompt can be changed, e.g when
debugging gdb,

set prompt (top-gdb) 

is used.

 > Also I think a good way to make it more reliable would be to make
 > it work with several gud buffers by moving most global vars to
 > process properties, so they're necessarily correctly initialized, and
 > we'd be forced to think a bit harder about what's going on where.

I'm not familar with process properties but I trust your judgement to make
these changes.  (In fact if Emacs was a democracy, I'd vote for you as
maintainer!).  Bear in mind, though, that the (long term) plan is to move away
from annotations and fully use GDB/MI.

 > PS: Is there any hope for GDB to accept a command that puts it in
 > annotate=3 mode, rather than having to tweak the command line for it?
 > That would solve a lot of those problems.

Yes, if you mean "set annotate 3".

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: Minor gdb-ui patches to make it a bit more robust

[Stefan Monnier]

> The flexibility of the command line means that there are always ways round
> these types of checks.  For example, the prompt can be changed, e.g when

Of course, I'm not deluding myself: these are nothing more than sanity
checks, but they can come *real* handy to the user.  I wasted a good 10
minutes trying to understand why my GDB was not responsive.

>> Also I think a good way to make it more reliable would be to make
>> it work with several gud buffers by moving most global vars to
>> process properties, so they're necessarily correctly initialized, and
>> we'd be forced to think a bit harder about what's going on where.

> I'm not familar with process properties but I trust your judgement to
> make these changes.

Process properties are nothing magical: they're just a property-list
attached to processes, that you can set and read via process-put and
process-get.  Handy for variables which are really per-process
(e.g. the yet-to-be-processed process output that needs to be passed
from one invocation of the process filter to the next).

> Bear in mind, though, that the (long term) plan is to move away from
> annotations and fully use GDB/MI.

I don't see in what way that would make any difference: global variables
will still be a source of bugs (and will still prevent the co-existence
of multiple gdb-ui processes in the same Emacs instance).

>> PS: Is there any hope for GDB to accept a command that puts it in
>> annotate=3 mode, rather than having to tweak the command line for it?
>> That would solve a lot of those problems.

> Yes, if you mean "set annotate 3".

So is there any hope to make gdb-ui rely on that rather than
on --annotate=3?


        Stefan

Re: Minor gdb-ui patches to make it a bit more robust

[Nick Roberts]

Stefan Monnier writes:
 > > The flexibility of the command line means that there are always ways round
 > > these types of checks.  For example, the prompt can be changed, e.g when
 > 
 > Of course, I'm not deluding myself: these are nothing more than sanity
 > checks, but they can come *real* handy to the user.  I wasted a good 10
 > minutes trying to understand why my GDB was not responsive.

It's a reliability issue in't it?  Even if your patch works for 99% of users,
it still means the other 1% will be exasperated and go round saying that
gdb-ui is buggy.

 > >> Also I think a good way to make it more reliable would be to make
 > >> it work with several gud buffers by moving most global vars to
 > >> process properties, so they're necessarily correctly initialized, and
 > >> we'd be forced to think a bit harder about what's going on where.
 > 
 > > I'm not familar with process properties but I trust your judgement to
 > > make these changes.
 > 
 > Process properties are nothing magical: they're just a property-list
 > attached to processes, that you can set and read via process-put and
 > process-get.  Handy for variables which are really per-process
 > (e.g. the yet-to-be-processed process output that needs to be passed
 > from one invocation of the process filter to the next).

OK, I'm still not sure how it would all work.  Perhaps, if, on the trunk,
you initialise one or two variables this way I can do the rest by following
the idiom.

 > > Bear in mind, though, that the (long term) plan is to move away from
 > > annotations and fully use GDB/MI.
 > 
 > I don't see in what way that would make any difference: global variables
 > will still be a source of bugs (and will still prevent the co-existence
 > of multiple gdb-ui processes in the same Emacs instance).

I just mean that there will be a lot of churn, at some stage, so I didn't
want to anyone to waste energy on the annotation side of things.

 > >> PS: Is there any hope for GDB to accept a command that puts it in
 > >> annotate=3 mode, rather than having to tweak the command line for it?
 > >> That would solve a lot of those problems.
 > 
 > > Yes, if you mean "set annotate 3".
 > 
 > So is there any hope to make gdb-ui rely on that rather than
 > on --annotate=3?

Yes.  I guess that is another possible way to solve the initialisation
problems (and keep text command mode available to M-x gdb).

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: Minor gdb-ui patches to make it a bit more robust

[Stefan Monnier]

>> > The flexibility of the command line means that there are always ways round
>> > these types of checks.  For example, the prompt can be changed, e.g when
>> 
>> Of course, I'm not deluding myself: these are nothing more than sanity
>> checks, but they can come *real* handy to the user.  I wasted a good 10
>> minutes trying to understand why my GDB was not responsive.

> It's a reliability issue in't it?  Even if your patch works for 99% of
> users, it still means the other 1% will be exasperated and go round
> saying that gdb-ui is buggy.

No, it's not a reliability issue.  It's a convenience issue.  Similar to
the check we add to detect "you've loaded an old version of CUA-mode":
it won't catch all cases, but will help most victims of the change
figure out quickly what's going on.  So it's perfectly OK to only catch
99% of the cases.  As I said, we may prefer to signal an error (or even
just a (loud) message) rather than fallback automatically to the old
gud-gdb code.

> OK, I'm still not sure how it would all work.  Perhaps, if, on the trunk,
> you initialise one or two variables this way I can do the rest by following
> the idiom.

I'll see if I find time to do it.  Basically, you want to replace

     (setq gdb-foo bar)
with
     (process-put <proc> 'foo bar)

and

     gdb-foo
with
     (process-get <proc> 'foo)

Of course, at first this will be very awkward because <proc> is not
immediately available, so you'll need (get-buffer-process
(current-buffer)) or somesuch.  And worse yet, some of the code is not
always run in the *gud* buffer, so we'll need (with-current-buffer
<gudbuf> (get-buffer-process (current-buffer))).

We may also prefer to move some of the global vars to buffer-local
rather than process-local.  In that case some of the code will stay as
is, and other will need some (with-current-buffer <gudbuf> ...) wrappers.

>> I don't see in what way that would make any difference: global variables
>> will still be a source of bugs (and will still prevent the co-existence
>> of multiple gdb-ui processes in the same Emacs instance).

> I just mean that there will be a lot of churn, at some stage, so I didn't
> want to anyone to waste energy on the annotation side of things.

That makes sense, indeed.

>> >> PS: Is there any hope for GDB to accept a command that puts it in
>> >> annotate=3 mode, rather than having to tweak the command line for it?
>> >> That would solve a lot of those problems.
>> 
>> > Yes, if you mean "set annotate 3".
>> 
>> So is there any hope to make gdb-ui rely on that rather than
>> on --annotate=3?

> Yes.  I guess that is another possible way to solve the initialisation
> problems (and keep text command mode available to M-x gdb).

Then, I strongly encourage you to make such a change to gdb-ui.
If you could make a similar change to gud-gdb (so it sends
"set annotate 2" I guess), that would be great.


        Stefan