Optionally print source code to gdb console window

Message ID CA+L9JkjGv1J546wQScPSnTVzqM7ai65Ye4XQELqMkSBdabTKKg@mail.gmail.com
State New
Headers
Series Optionally print source code to gdb console window |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm fail Testing failed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Testing failed

Commit Message

Bob Rossi May 5, 2024, 8:57 p.m. UTC
  A little history. When using --annotations, gdb did not print the source
code to the gdb console window. When using mi with new-ui it does. When I
reported this in the past, several people said it was a feature that gdb
printed the source code lines to the console.

I've had several users of cgdb say they do not want gdb to print the source
code to the gdb console window as they can see the code in the code view.

I've created and attached a patch that I hope makes it optional to have gdb
print the source code to the gdb console window. Could I have some feedback?

I've added a new print source option to control printing source code to the
gdb console.

(gdb) show print source
Printing of source code to gdb console is on.

You can turn the printing of the source code off as follows.
(gdb) set print source off
(gdb)

When the printing of source code is on,
(gdb) r
Starting program: /home/bob/rcs/git/gdb/gdb-build/main
....
Breakpoint 1, main (argc=1, argv=0x7fffffffe0c8) at test_main.cpp:42
42      {
(gdb) n
43          int i = 3;
(gdb) n
44          int j = 4;
(gdb) n
47          long_func();

When the printing of source code is off,
(gdb) r
Starting program: /home/bob/rcs/git/gdb/gdb-build/main
...
Breakpoint 1, main (argc=1, argv=0x7fffffffe098) at test_main.cpp:42
(gdb) n
(gdb) n
(gdb) n
(gdb)

I don't know gdb code well enough to understand if i've disabled
functionality
beyond what i was hoping to.

I'm not sure how to control this from cgdb when using old versions of gdb.
I get the following error when i run --ex "set print source off" when
starting gdb.
Undefined set print command: "source off".  Try "help set print".

Thanks,
Bob Rossi
  

Comments

Bob Rossi May 6, 2024, 10:18 p.m. UTC | #1
I got an automated email saying I broke a test.

This updated patch fixes the test and updates the code to the GNU standard,
hopefully.

Thanks,
Bob Rossi


On Sun, May 5, 2024 at 4:57 PM Robert Rossi <bob@brasko.net> wrote:

> A little history. When using --annotations, gdb did not print the source
> code to the gdb console window. When using mi with new-ui it does. When I
> reported this in the past, several people said it was a feature that gdb
> printed the source code lines to the console.
>
> I've had several users of cgdb say they do not want gdb to print the
> source code to the gdb console window as they can see the code in the code
> view.
>
> I've created and attached a patch that I hope makes it optional to have
> gdb print the source code to the gdb console window. Could I have some
> feedback?
>
> I've added a new print source option to control printing source code to
> the gdb console.
>
> (gdb) show print source
> Printing of source code to gdb console is on.
>
> You can turn the printing of the source code off as follows.
> (gdb) set print source off
> (gdb)
>
> When the printing of source code is on,
> (gdb) r
> Starting program: /home/bob/rcs/git/gdb/gdb-build/main
> ....
> Breakpoint 1, main (argc=1, argv=0x7fffffffe0c8) at test_main.cpp:42
> 42      {
> (gdb) n
> 43          int i = 3;
> (gdb) n
> 44          int j = 4;
> (gdb) n
> 47          long_func();
>
> When the printing of source code is off,
> (gdb) r
> Starting program: /home/bob/rcs/git/gdb/gdb-build/main
> ...
> Breakpoint 1, main (argc=1, argv=0x7fffffffe098) at test_main.cpp:42
> (gdb) n
> (gdb) n
> (gdb) n
> (gdb)
>
> I don't know gdb code well enough to understand if i've disabled
> functionality
> beyond what i was hoping to.
>
> I'm not sure how to control this from cgdb when using old versions of gdb.
> I get the following error when i run --ex "set print source off" when
> starting gdb.
> Undefined set print command: "source off".  Try "help set print".
>
> Thanks,
> Bob Rossi
>
  
Tom Tromey May 7, 2024, 4:52 p.m. UTC | #2
>>>>> "Robert" == Robert Rossi <bob@brasko.net> writes:

Thank you for the patch.

Robert> A little history. When using --annotations, gdb did not print
Robert> the source code to the gdb console window. When using mi with
Robert> new-ui it does.  When I reported this in the past, several
Robert> people said it was a feature that gdb printed the source code
Robert> lines to the console.

Robert> I've had several users of cgdb say they do not want gdb to print
Robert> the source code to the gdb console window as they can see the
Robert> code in the code view.

Robert> I've created and attached a patch that I hope makes it optional
Robert> to have gdb print the source code to the gdb console
Robert> window. Could I have some feedback?

I was reviewing your patch and I found is a pre-existing setting that
claims to do what you want: "set source open off".

Could you try this and see if it works?  I suspect this is the way to
go.

Otherwise, we can look at your patch.  From a first read, I think the
idea is totally fine, but the implementation would need to be changed.
Also it would need documentation and a test case.

thanks,
Tom
  
Bob Rossi May 7, 2024, 9:33 p.m. UTC | #3
Hi Tom,

Thanks for the feedback. This is wrong in a few directions unfortunately.

The first is that the output looks like this in the suggested case,
(gdb) n
43      in test_main.cpp
(gdb)
44      in test_main.cpp
(gdb) n
47      in test_main.cpp
(gdb) list
42      in test_main.cpp

Which is not what the user wants. They don't want to see the updates in the
console
regarding source positions.
In addition, you can see capabilities they may want, 'list', no longer work.

The reason this issue was noticed in the first place is because this was the
vision of the original gdb maintainers when using annotate=2. In that mode,
no source code would be printed to the console. However, in mi new-ui mode
gdb does print the source code to the console.

My personal opinion is that gdb shouldn't print code to the console in mi
mode
or in new-ui mode. However, there were some people from eclipse a few years
ago that said they disagreed and that gdb should behave identical in new-ui
mode and not in new-ui mode, so i've attempted to make an option.

I'm happy to add docs and a test case.
Can you give me some pointers on how the implementation should change?

I should also ask, is an option even necessary?
I think if we remove the source printing from the console when in new-ui
mode
or in mi-mode it would make the most sense. After all, there is a front end
displaying
this information, no?

Thanks,
Bob Rossi

On Tue, May 7, 2024 at 12:52 PM Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Robert" == Robert Rossi <bob@brasko.net> writes:
>
> Thank you for the patch.
>
> Robert> A little history. When using --annotations, gdb did not print
> Robert> the source code to the gdb console window. When using mi with
> Robert> new-ui it does.  When I reported this in the past, several
> Robert> people said it was a feature that gdb printed the source code
> Robert> lines to the console.
>
> Robert> I've had several users of cgdb say they do not want gdb to print
> Robert> the source code to the gdb console window as they can see the
> Robert> code in the code view.
>
> Robert> I've created and attached a patch that I hope makes it optional
> Robert> to have gdb print the source code to the gdb console
> Robert> window. Could I have some feedback?
>
> I was reviewing your patch and I found is a pre-existing setting that
> claims to do what you want: "set source open off".
>
> Could you try this and see if it works?  I suspect this is the way to
> go.
>
> Otherwise, we can look at your patch.  From a first read, I think the
> idea is totally fine, but the implementation would need to be changed.
> Also it would need documentation and a test case.
>
> thanks,
> Tom
>
  
Tom Tromey May 8, 2024, 12:52 a.m. UTC | #4
> The first is that the output looks like this in the suggested case,
> (gdb) n
> 43      in test_main.cpp

Yeah, that's no good.

> My personal opinion is that gdb shouldn't print code to the console in mi mode
> or in new-ui mode. However, there were some people from eclipse a few years
> ago that said they disagreed and that gdb should behave identical in new-ui
> mode and not in new-ui mode, so i've attempted to make an option.

Ok.

> Can you give me some pointers on how the implementation should change?

The new flag shouldn't be in value_print_options.  That struct is only
for things affecting value-printing.

Perhaps 'set source open' could be extended to have a new value.  Or
perhaps some other 'set source' sub-option could be made.  I'm afraid
that normally we bikeshed option names a bit.

> I should also ask, is an option even necessary?
> I think if we remove the source printing from the console when in new-ui mode
> or in mi-mode it would make the most sense. After all, there is a front end displaying
> this information, no?

I personally don't have much stake in it, but it seems to me that if the
Eclipse folks and/or the new-ui implementers already discussed it, then
an option is necessary.

Tom
  
Bob Rossi May 8, 2024, 1:29 a.m. UTC | #5
On Tue, May 07, 2024 at 06:52:57PM -0600, Tom Tromey wrote:
> > Can you give me some pointers on how the implementation should change?
> 
> The new flag shouldn't be in value_print_options.  That struct is only
> for things affecting value-printing.

That makes sense.

> Perhaps 'set source open' could be extended to have a new value.  Or
> perhaps some other 'set source' sub-option could be made.  I'm afraid
> that normally we bikeshed option names a bit.

I think it's this behavior of gdb i want to suppress,
https://sourceware.org/gdb/current/onlinedocs/gdb#Source
"When your program stops, GDB spontaneously prints the line where it
stopped"

I'm looking for a way to control printing source lines when printing
frame information? (I'm modifying do_print_frame_info)

Something like 'set frame print_source off' might make sense to me.
Just a thought.

Thanks,
Bob Rossi
  
Tom Tromey May 8, 2024, 3:46 p.m. UTC | #6
>>>>> "Bob" == Bob Rossi <bob@brasko.net> writes:

>> Perhaps 'set source open' could be extended to have a new value.  Or
>> perhaps some other 'set source' sub-option could be made.  I'm afraid
>> that normally we bikeshed option names a bit.

Bob> I think it's this behavior of gdb i want to suppress,
Bob> https://sourceware.org/gdb/current/onlinedocs/gdb#Source
Bob> "When your program stops, GDB spontaneously prints the line where it
Bob> stopped"

Bob> I'm looking for a way to control printing source lines when printing
Bob> frame information? (I'm modifying do_print_frame_info)

Bob> Something like 'set frame print_source off' might make sense to me.
Bob> Just a thought.

How about adding a new value to "set print frame-info"?
Like "set print frame-info nothing"?

Tom
  
Bob Rossi May 8, 2024, 11:07 p.m. UTC | #7
On Wed, May 08, 2024 at 09:46:16AM -0600, Tom Tromey wrote:
> >>>>> "Bob" == Bob Rossi <bob@brasko.net> writes:
> 
> >> Perhaps 'set source open' could be extended to have a new value.  Or
> >> perhaps some other 'set source' sub-option could be made.  I'm afraid
> >> that normally we bikeshed option names a bit.
> 
> Bob> I think it's this behavior of gdb i want to suppress,
> Bob> https://sourceware.org/gdb/current/onlinedocs/gdb#Source
> Bob> "When your program stops, GDB spontaneously prints the line where it
> Bob> stopped"
> 
> Bob> I'm looking for a way to control printing source lines when printing
> Bob> frame information? (I'm modifying do_print_frame_info)
> 
> Bob> Something like 'set frame print_source off' might make sense to me.
> Bob> Just a thought.
> 
> How about adding a new value to "set print frame-info"?
> Like "set print frame-info nothing"?

I saw in the NEWS file,
    show print frame-info
      This controls what frame information is printed by the commands printing
      a frame.  This setting will e.g. influence the behaviour of 'backtrace',
      'frame', 'stepi'.  The python frame filtering also respect this setting.
      The 'backtrace' '-frame-info' option can override this global setting.

So i tried out how frame-info impacts the bt command,
    (gdb) set print frame-info short-location
    (gdb) n
    main (argc=1, argv=0x7fffffffe098)
    (gdb) bt
    #0  main (argc=1, argv=0x7fffffffe098)
    (gdb) set print frame-info auto
    (gdb) bt
    #0  main (argc=1, argv=0x7fffffffe098) at test_main.cpp:47
    (gdb) 

The only request i've had, is not to print source lines automatically to
the console, because cgdb already displays them in a source view.
This is how cgdb worked with annotations, so people were familiar 
with that behavior in cgdb, until i upgraded to mi.

I don't think people want gdb's behavior changed under cgdb.
If they prefer special settings for 'print frame-info', they should
probably be allowed to keep them and not have cgdb hijack them.

A new option, but separate from the existing frame-info command, as to
not impact other areas of gdb.
  set print frame-info-source-line off
This is very similar to your approach, however, it lets the user choose
whatever they want with print frame-info, and then it lets cgdb "subtract" the
source-line printing from the console in only that case (and not bt case).

What do you think about this crazyiness?

Thanks,
Bob Rossi
  
Tom Tromey May 9, 2024, 1:58 p.m. UTC | #8
>>>>> "Bob" == Bob Rossi <bob@brasko.net> writes:

Bob> I saw in the NEWS file,
Bob>     show print frame-info
Bob>       This controls what frame information is printed by the commands printing
Bob>       a frame.  This setting will e.g. influence the behaviour of 'backtrace',
Bob>       'frame', 'stepi'.  The python frame filtering also respect this setting.
Bob>       The 'backtrace' '-frame-info' option can override this global setting.

Yeah, sorry.  I was hoping to find some simpler way but I think there
isn't one.

Bob> A new option, but separate from the existing frame-info command, as to
Bob> not impact other areas of gdb.
Bob>   set print frame-info-source-line off
Bob> This is very similar to your approach, however, it lets the user choose
Bob> whatever they want with print frame-info, and then it lets cgdb "subtract" the
Bob> source-line printing from the console in only that case (and not bt case).

Bob> What do you think about this crazyiness?

Works for me.

Tom
  
Andrew Burgess May 10, 2024, 10:19 a.m. UTC | #9
Bob Rossi <bob@brasko.net> writes:

> On Wed, May 08, 2024 at 09:46:16AM -0600, Tom Tromey wrote:
>> >>>>> "Bob" == Bob Rossi <bob@brasko.net> writes:
>> 
>> >> Perhaps 'set source open' could be extended to have a new value.  Or
>> >> perhaps some other 'set source' sub-option could be made.  I'm afraid
>> >> that normally we bikeshed option names a bit.
>> 
>> Bob> I think it's this behavior of gdb i want to suppress,
>> Bob> https://sourceware.org/gdb/current/onlinedocs/gdb#Source
>> Bob> "When your program stops, GDB spontaneously prints the line where it
>> Bob> stopped"
>> 
>> Bob> I'm looking for a way to control printing source lines when printing
>> Bob> frame information? (I'm modifying do_print_frame_info)
>> 
>> Bob> Something like 'set frame print_source off' might make sense to me.
>> Bob> Just a thought.
>> 
>> How about adding a new value to "set print frame-info"?
>> Like "set print frame-info nothing"?
>
> I saw in the NEWS file,
>     show print frame-info
>       This controls what frame information is printed by the commands printing
>       a frame.  This setting will e.g. influence the behaviour of 'backtrace',
>       'frame', 'stepi'.  The python frame filtering also respect this setting.
>       The 'backtrace' '-frame-info' option can override this global setting.
>
> So i tried out how frame-info impacts the bt command,
>     (gdb) set print frame-info short-location
>     (gdb) n
>     main (argc=1, argv=0x7fffffffe098)
>     (gdb) bt
>     #0  main (argc=1, argv=0x7fffffffe098)
>     (gdb) set print frame-info auto
>     (gdb) bt
>     #0  main (argc=1, argv=0x7fffffffe098) at test_main.cpp:47
>     (gdb) 
>
> The only request i've had, is not to print source lines automatically to
> the console, because cgdb already displays them in a source view.
> This is how cgdb worked with annotations, so people were familiar 
> with that behavior in cgdb, until i upgraded to mi.
>
> I don't think people want gdb's behavior changed under cgdb.
> If they prefer special settings for 'print frame-info', they should
> probably be allowed to keep them and not have cgdb hijack them.
>
> A new option, but separate from the existing frame-info command, as to
> not impact other areas of gdb.
>   set print frame-info-source-line off
> This is very similar to your approach, however, it lets the user choose
> whatever they want with print frame-info, and then it lets cgdb "subtract" the
> source-line printing from the console in only that case (and not bt case).

Have you tried 'set suppress-cli-notifications on'?

This:

  (a) Stops cli (even in new-ui) printing the location when inferior
  execution stops, and

  (b) Stops cli printing the context (stack frame) when the user changes
  the current stack frame (e.g. up, down, frame N, etc).

Now (a) seems like exactly what you want.

Not sure if (b) is what you want or not.  I don't think you mentioned
this, but if you have a GUI which is displaying the source location, you
might also be displaying the stack, in which case you might want (b)
too.

Internally, 'set suppress-cli-notifications on' sets two separate flags,
one each for (a) and (b), so you could potentially implement
sub-commands of 'set suppress-cli-notifications' to control each
component individually, if that is more helpful.

Maybe I've not understood the problem requirements though, so feel free
to ignore me if this isn't helpful :)

Thanks,
Andrew
  
Tom Tromey May 10, 2024, 6:07 p.m. UTC | #10
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> Have you tried 'set suppress-cli-notifications on'?

Somewhere in this zoo of options is a simpler, more orthogonal way to
configure gdb's behavior, waiting to get out.

Tom
  
Pedro Alves May 10, 2024, 7:23 p.m. UTC | #11
On 2024-05-07 22:33, Robert Rossi wrote:

> Which is not what the user wants. They don't want to see the updates in the console
> regarding source positions.
> In addition, you can see capabilities they may want, 'list', no longer work.
> 
> The reason this issue was noticed in the first place is because this was the
> vision of the original gdb maintainers when using annotate=2. In that mode,
> no source code would be printed to the console. However, in mi new-ui mode
> gdb does print the source code to the console.
> 
> My personal opinion is that gdb shouldn't print code to the console in mi mode
> or in new-ui mode. However, there were some people from eclipse a few years
> ago that said they disagreed and that gdb should behave identical in new-ui
> mode and not in new-ui mode, so i've attempted to make an option.

You may be thinking of me.  But yes, I disagree, and do think that gdb should
behave identical by default, that was part of the intent/design.  I think I
suggested making it an option, so I definitely agree with having one.
  
Pedro Alves May 10, 2024, 7:29 p.m. UTC | #12
On 2024-05-08 16:46, Tom Tromey wrote:
> How about adding a new value to "set print frame-info"?
> Like "set print frame-info nothing"?

This would affect commands invoked on the command line too, like "frame".  IIUC,
Bob wants to suppress the automatic location printing that you get when gdb
presents a stop, and affecting commands like "frame", etc. would be undesirable.

A setting somewhere around events may be more appropriate, like Andrew suggested
down thread.
  

Patch

diff -urNp gdb-14.2.orig/gdb/stack.c gdb-14.2/gdb/stack.c
--- gdb-14.2.orig/gdb/stack.c	2024-03-03 00:55:00.000000000 -0500
+++ gdb-14.2/gdb/stack.c	2024-05-05 11:27:07.735576624 -0400
@@ -1161,7 +1161,9 @@  print_frame_info (const frame_print_opti
 	      uiout->text ("\t");
 	    }
 
-	  print_source_lines (sal.symtab, sal.line, sal.line + 1, 0);
+      
+          if (opts.sourceprint)
+              print_source_lines (sal.symtab, sal.line, sal.line + 1, 0);
 	}
 
       /* If disassemble-next-line is set to on and there is line debug
diff -urNp gdb-14.2.orig/gdb/valprint.c gdb-14.2/gdb/valprint.c
--- gdb-14.2.orig/gdb/valprint.c	2024-03-03 00:55:00.000000000 -0500
+++ gdb-14.2/gdb/valprint.c	2024-05-05 11:24:32.447511834 -0400
@@ -113,6 +113,7 @@  struct value_print_options user_print_op
   true,				/* addressprint */
   false,			/* nibblesprint */
   false,			/* objectprint */
+  true,	    		/* sourceprint */
   PRINT_MAX_DEFAULT,		/* print_max */
   PRINT_MAX_CHARS_DEFAULT,	/* print_max_chars */
   10,				/* repeat_count_threshold */
@@ -2878,6 +2879,16 @@  Printing of C++ virtual function tables
 	      value);
 }
 
+/* Controls printing of source code.  */
+static void
+show_sourceprint (struct ui_file *file, int from_tty,
+		struct cmd_list_element *c, const char *value)
+{
+  gdb_printf (file, _("\
+Printing of source code to gdb console is %s.\n"),
+	      value);
+}
+
 /* Controls looking up an object's derived type using what we find in
    its vtables.  */
 static void
@@ -3088,6 +3099,14 @@  pretty-printers for that value.")
     N_("Show printing of C++ virtual function tables."),
     NULL, /* help_doc */
   },
+  boolean_option_def {
+    "source",
+    [] (value_print_options *opt) { return &opt->sourceprint; },
+    show_sourceprint, /* show_cmd_cb */
+    N_("Set printing of source code to gdb console."),
+    N_("Show printing of source code to gdb console."),
+    NULL, /* help_doc */
+  },
 };
 
 /* See valprint.h.  */
diff -urNp gdb-14.2.orig/gdb/valprint.h gdb-14.2/gdb/valprint.h
--- gdb-14.2.orig/gdb/valprint.h	2024-03-03 00:55:00.000000000 -0500
+++ gdb-14.2/gdb/valprint.h	2024-05-05 11:16:22.418856087 -0400
@@ -62,6 +62,9 @@  struct value_print_options
      in its vtables.  */
   bool objectprint;
 
+  /* Controls printing of source to console.  */
+  bool sourceprint;
+
   /* Maximum number of elements to print for vector contents, or UINT_MAX
      for no limit.  Note that "set print elements 0" stores UINT_MAX in
      print_max, which displays in a show command as "unlimited".  */