public inbox for gdb@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Bob Rossi <bob@brasko.net>
Cc: gdb@sourceware.org
Subject: Re: source annotation now prints source line
Date: Tue, 14 Apr 2020 13:57:13 +0100	[thread overview]
Message-ID: <20200414125713.GE2366@embecosm.com> (raw)
In-Reply-To: <20200414121705.GD2366@embecosm.com>

* Andrew Burgess <andrew.burgess@embecosm.com> [2020-04-14 13:17:05 +0100]:

> * Bob Rossi <bob@brasko.net> [2020-04-14 07:23:04 -0400]:
>
> > On Sat, Apr 04, 2020 at 07:54:24PM -0400, Bob Rossi wrote:
> > > When the source annotation is sent to the front end, the source line
> > > is now also sent to the front end console. I believe this commit
> > > introduced it,
> > >   https://github.com/bminor/binutils-gdb/commit/ec8e2b6d3051f0b4b2a8eee9917898e95046c62f
> > >
> > > Now CGDB displays,
> > >     (gdb) n
> > >     43          int i = 3;
> > >     (gdb)
> > >
> > > Instead of,
> > >     (gdb) n
> > >     (gdb)
> > >
> > > CGDB is a front end, and so it has a source view to display the code to
> > > the user. Why did GDB decide to also print the line of code to the front
> > > end's console window as well? This is confusing.
> >
> > The concept behind this commit seems incorrect.
> >
> > The motivation for the patch isn't clearly explained in
> > the commit message. I believe I've given a reasonable explanation
> > on why this patch makes no sense for front ends.
> >
> > Should I submit a patch reverting it?
>
> The patch in context is discussed here:
>
>   https://sourceware.org/pipermail/gdb-patches/2019-June/158310.html
>   https://sourceware.org/pipermail/gdb-patches/2019-June/158350.html
>   https://sourceware.org/pipermail/gdb-patches/2019-June/158351.html
>   https://sourceware.org/pipermail/gdb-patches/2019-June/158352.html
>   https://sourceware.org/pipermail/gdb-patches/2019-June/158353.html
>
> I'm not sure you've convinced me yet that the idea behind the patch is
> incorrect.  Annotations should be a (deprecated) way for F/Es to parse
> GDB's output, but they shouldn't impact _what_ GDB prints.
>
> In this particular case, printing the source line actually updates
> some internal state, which impacts how later commands operate.  What
> this means is that the users session will behave differently if they
> have annotations on than when annotations are off.
>
> I guess, what I don't understand is that if a F/E wants to hide a
> particular piece of the output, why can't it just strip that from the
> output stream?  The F/E must already be removing the annotation
> markers, so all the output must be going through the F/E anyway.
>
> Further, removing this particular piece of output makes sense for this
> F/E, but is it always going to be true for all F/Es?
>
> I haven't gone back and looked at the old behaviour, maybe I'll have
> more thoughts once I've looked at that again.

For the record, here's GDB's output before the patch (8.3.1):

  ## START ##
  Temporary breakpoint 1, main () at hello.c:6
  6	  printf ("Hello World\n");
  (gdb) set annotate 0xff

  ��pre-prompt
  (gdb)
  ��prompt
  n

  ��post-prompt

  ��starting
  Hello World

  ��source /home/andrew/tmp/hello.c:7:62:beg:0x401134

  ��stopped

  ��pre-prompt
  (gdb)
  ��prompt
  ## END ##

And here's the output after the patch (9.1):

  ## START ##
  Temporary breakpoint 1, main () at hello.c:6
  6	  printf ("Hello World\n");
  (gdb) set annotate 0xff

  ��pre-prompt
  (gdb)
  ��prompt
  n

  ��post-prompt

  ��starting
  Hello World

  ��source /home/andrew/tmp/hello.c:7:62:beg:0x401134
  7	  return 0;

  ��stopped

  ��pre-prompt
  (gdb)
  ��prompt
  ## END ##

I guess I'd still suggest that the "right" thing would be to strip the
output when processing GDB's output.  But, if you strongly disagree
then you could put forward a patch for discussion.  However, I think
you'd need to do more than revert the original patch.

If you look inside source.c:print_source_lines_base then you'll see
the current source line and symtab being updated.  This is done as
part of printing the '7  ..... return 0' line.  If you'd going to stop
this being printed, then you'd need to duplicate this behaviour
somewhere else, so that the current line/symtab are updated when
annotations are on.

Thanks,
Andrew

  reply	other threads:[~2020-04-14 12:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-04 23:54 Bob Rossi
2020-04-14 11:23 ` Bob Rossi
2020-04-14 12:17   ` Andrew Burgess
2020-04-14 12:57     ` Andrew Burgess [this message]
2020-04-15  2:13     ` Bob Rossi
2020-04-16 17:41       ` Andrew Burgess
2021-03-13 17:01         ` Bob Rossi
2021-03-15 13:10           ` Pedro Alves
2024-05-04 20:09             ` Robert Rossi
2024-05-05 15:36               ` Robert Rossi
2024-05-05 16:06                 ` Eli Zaretskii

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=20200414125713.GE2366@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=bob@brasko.net \
    --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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).