Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Matt Rice <ratmice@gmail.com>
To: Tom Tromey <tromey@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [patch] python prompt additions at first prompt.
Date: Tue, 09 Aug 2011 00:20:00 -0000	[thread overview]
Message-ID: <CACTLOFoRPLLH=bLyyv6RhVwpjzZ-s=NA0uj94guPjUvd+Z-H4A@mail.gmail.com> (raw)
In-Reply-To: <m3r5521i8d.fsf@fleche.redhat.com>

On Wed, Aug 3, 2011 at 11:07 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:
>
> Matt> I think it's ok anyways, the 'if (async_command_editing_p)' case in
> Matt> display_gdb_prompt seems to cover it,
> Matt> with the addition of rl_callback_handler_remove() which afaict seemed
> Matt> ok to call before a handler is installed
>
> It is really unclear to me whether or not this patch is ok.
>
> display_gdb_prompt can early exit in a couple of cases:
>
> 1:
>
>  if (!current_interp_display_prompt_p ())
>    return;
>
> (I don't think this one can happen here.)
>
> 2:
>
>  if (sync_execution && is_running (inferior_ptid))
>    {
> [...]
>      rl_callback_handler_remove ();
>      return;
>
> This seems like it could be possible, maybe with attach + cont from the
> command line?

argh, thanks for noticing this should have seen it.
yes it is possible,

./gdb/gdb -ex 'set target-async on' -ex 'attach 26359' -ex 'continue&'

I have yet to fully unfurl the state machine, but in this case,
display_gdb_prompt
now gets called twice, first hitting this sync_execution && is_running
and returning, secondly without returning early, and hitting
async_editing.   This leads to one prompt being displayed.

further, trying this same case without my patch leads me to believe
that the original code is not correct either.
precisely because it does call rl_callback_handler_install, debugging
gdb gives an infinite loop of these.
not debugging gdb you seem to wind up back at the shell needing to 'fg' gdb.

Program received signal SIGTTOU, Stopped (tty output).
0x000000394acd7f18 in tcsetattr () from /lib64/libc.so.6
(gdb) bt
#0  0x000000394acd7f18 in tcsetattr () from /lib64/libc.so.6
#1  0x00000000006c16db in _set_tty_settings (tty=0,
tiop=0x7fffffffdcf0) at rltty.c:476
#2  0x00000000006c1706 in set_tty_settings (tty=0,
tiop=0x7fffffffdcf0) at rltty.c:490
#3  0x00000000006c1a71 in rl_prep_terminal (meta_flag=1) at rltty.c:653
#4  0x00000000006d436e in _rl_callback_newline () at callback.c:82
#5  0x00000000006d43d3 in rl_callback_handler_install
(prompt=0x7fffffffdd70 "",
    linefunc=0x5b6236 <command_line_handler>) at callback.c:102
#6  0x00000000004fa86c in tui_command_loop (data=0x0) at ./tui/tui-interp.c:159

the patch at least suffers from one issue that the first 'current'
argument to gdb.prompt_hook
is both inconsistent and unsatisfactory.
None, or "" empty string with patch: (normal vs target-async/attach .. etc)
rather than the "(gdb) " it is without the patch/should be.
(by putting the observer mechanism before the first rl_callback_handler_install)

So, I guess neither the current patch or adding the observer calls to
the existing code
is correct and we at least need to split up prompt displaying from the
setting of the prompt parameter?

> If that one cannot be taken, then I agree the patch is correct.
> Could you try this?
>
> Tom
>


  reply	other threads:[~2011-08-09  0:20 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-30 22:20 Matt Rice
2011-08-01  9:51 ` Phil Muldoon
2011-08-01 14:08   ` Matt Rice
2011-08-01 14:13     ` Phil Muldoon
2011-08-01 17:44       ` Matt Rice
2011-08-02  9:07         ` Phil Muldoon
2011-08-02 17:59           ` Matt Rice
2011-08-02 20:37             ` Phil Muldoon
2011-08-03 18:08 ` Tom Tromey
2011-08-09  0:20   ` Matt Rice [this message]
2011-08-09  0:25     ` Matt Rice
2011-08-10 15:21     ` Tom Tromey
2011-08-11 12:03       ` Matt Rice
2011-08-12 13:10         ` Matt Rice
2011-08-12 14:44           ` Pedro Alves
2011-08-12 15:07             ` Matt Rice
2011-08-29 16:23               ` Matt Rice
2011-08-29 16:39                 ` Pedro Alves
2011-09-02 14:31                 ` Pedro Alves
2011-09-02 21:41                   ` Matt Rice

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='CACTLOFoRPLLH=bLyyv6RhVwpjzZ-s=NA0uj94guPjUvd+Z-H4A@mail.gmail.com' \
    --to=ratmice@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tromey@redhat.com \
    /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