[gdb/cli] Add maint info screen

Message ID 20230417140906.25341-1-tdevries@suse.de
State Committed
Headers
Series [gdb/cli] Add maint info screen |

Commit Message

Tom de Vries April 17, 2023, 2:09 p.m. UTC
  While working on PRs tui/30337 and cli/30346 I came across various notions of
width in gdb, as reported by gdb, readline, curses and the environment
variables.

As for gdb, readline and the environment variables, the way things work
is:
- Gdb asks readline to detect screen size,
- readline sets the actual screen size in the environment variables
  COLUMNS and LINES,
- readline reports back a screen size to gdb, which may have one column
  less than the actual screen size, to deal with lack of auto-wrap.
  This becomes gdb's notion of screen size (in other words the point where
  we can expect the gdb command line to wrap),
- Gdb then explicitly sets readline's screen size, which readline itself may
  adjust to deal with lack of auto-wrap.  This becomes readlines notion
  of screen size (well, internally the unadjusted one, but it'll report back
  the adjusted one).

Add a command "maint info screen" that prints these notions, both for width
and height.

For TERM=xterm we have:
...
$ TERM=xterm gdb -ex "maint info screen"
Number of characters gdb thinks are in a line is 118.
Number of characters readline reports are in a line is 118.
Number of characters curses thinks are in a line is 118.
Number of characters environment thinks are in a line is 118 (COLUMNS).
Number of lines gdb thinks are in a page is 27.
Number of lines readline reports are in a page is 27.
Number of lines curses thinks are in a page is 27.
Number of lines environment thinks are in a page is 27 (LINES).
...

And for TERM=ansi:
...
$ TERM=ansi gdb -ex "maint info screen"
Number of characters gdb thinks are in a line is 117.
Number of characters readline reports are in a line is 116.
Number of characters curses thinks are in a line is 118.
Number of characters environment thinks are in a line is 118 (COLUMNS).
Number of lines gdb thinks are in a page is 27.
Number of lines readline reports are in a page is 27.
Number of lines curses thinks are in a page is 27.
Number of lines environment thinks are in a page is 27 (LINES).
...

[ The fact that we have "characters readline reports are in a line is 116" is
is due to gdb making readline adjust twice for the lack of auto-wrap, this is
PR cli/30346.

Likewise we can detect tui/30337 by doing a resize in TUI mode and doing
"maint info screen":
...
Number of characters characters curses thinks are in a line is 110.
Number of characters environment thinks are in a line is 111 (COLUMNS). ]

And for TERM=ansi, with width and heigth set to 0:
...
Number of characters gdb thinks are in a line is 4294967295 (unlimited).
Number of characters readline reports are in a line is 32766 (unlimited - 1).
Number of characters curses thinks are in a line is 118.
Number of characters environment thinks are in a line is 118 (COLUMNS).
Number of lines gdb thinks are in a page is 4294967295 (unlimited).
Number of lines readline reports are in a page is 32767 (unlimited).
Number of lines curses thinks are in a page is 27.
Number of lines environment thinks are in a page is 27 (LINES).
...

[ Note that when doing a resize by say maximizing or de-maximizing a terminal,
all reported values are updated, except for curses when not in TUI mode.

Maybe that means there's a bug.  If not, then maybe we should not print
the curses lines unless in TUI mode, or annotate those lines such that it's
clear that the values may be not up-to-date. ]

I'd like to use this command in the regression test for PR cli/30346.

Tested on x86_64-linux.
---
 gdb/doc/gdb.texinfo |  7 +++++
 gdb/utils.c         | 70 +++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 75 insertions(+), 2 deletions(-)


base-commit: 55003828f4d81a3d857ccfdfe4841704004a6468
  

Comments

Eli Zaretskii April 17, 2023, 5:19 p.m. UTC | #1
> Date: Mon, 17 Apr 2023 16:09:06 +0200
> From: Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>
> 
>  gdb/doc/gdb.texinfo |  7 +++++
>  gdb/utils.c         | 70 +++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 75 insertions(+), 2 deletions(-)

Thanks for working on this.

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -41363,6 +41363,13 @@ This option is @samp{on} by default for supported architectures.
>  This option is useful for debugging @value{GDBN}'s use of the Pygments
>  library when @value{GDBN} is built for an architecture that supports
>  styling with the builtin disassembler
> +
> +@kindex maint info screen
> +@cindex Show screen characteristics
           ^^^^
Please avoid capital letters in index entries as much as possible.
That's because differences in letter-case can cause the index entries
be sorted in different order depending on the locale where the manual
is built, and we want to avoid this locale dependency if possible.

> +@item maint info screen
> +Print various characteristics of the screen, such as various notions
> +of width and height.

Maybe say what will this be useful for?

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  
Tom Tromey April 21, 2023, 1:54 p.m. UTC | #2
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

[...]
Tom> I'd like to use this command in the regression test for PR cli/30346.

Seems reasonable to me.
Reviewed-By: Tom Tromey <tom@tromey.com>

Tom
  
Tom de Vries April 21, 2023, 3:21 p.m. UTC | #3
On 4/17/23 19:19, Eli Zaretskii wrote:
>> Date: Mon, 17 Apr 2023 16:09:06 +0200
>> From: Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>
>>
>>   gdb/doc/gdb.texinfo |  7 +++++
>>   gdb/utils.c         | 70 +++++++++++++++++++++++++++++++++++++++++++--
>>   2 files changed, 75 insertions(+), 2 deletions(-)
> 
> Thanks for working on this.
> 
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -41363,6 +41363,13 @@ This option is @samp{on} by default for supported architectures.
>>   This option is useful for debugging @value{GDBN}'s use of the Pygments
>>   library when @value{GDBN} is built for an architecture that supports
>>   styling with the builtin disassembler
>> +
>> +@kindex maint info screen
>> +@cindex Show screen characteristics
>             ^^^^
> Please avoid capital letters in index entries as much as possible.
> That's because differences in letter-case can cause the index entries
> be sorted in different order depending on the locale where the manual
> is built, and we want to avoid this locale dependency if possible.
> 

Ack, done.

>> +@item maint info screen
>> +Print various characteristics of the screen, such as various notions
>> +of width and height.
> 
> Maybe say what will this be useful for?
> 

I haven't managed to come up with something meaningful here, so I've 
left it as is.

I'm open to further suggestions, but I've committed to be able to use it 
in test-cases.

> Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Thanks for the review.

- Tom
  
John Baldwin April 27, 2023, 8:49 p.m. UTC | #4
On 4/17/23 7:09 AM, Tom de Vries via Gdb-patches wrote:
> While working on PRs tui/30337 and cli/30346 I came across various notions of
> width in gdb, as reported by gdb, readline, curses and the environment
> variables.
> 
> As for gdb, readline and the environment variables, the way things work
> is:
> - Gdb asks readline to detect screen size,
> - readline sets the actual screen size in the environment variables
>    COLUMNS and LINES,
> - readline reports back a screen size to gdb, which may have one column
>    less than the actual screen size, to deal with lack of auto-wrap.
>    This becomes gdb's notion of screen size (in other words the point where
>    we can expect the gdb command line to wrap),
> - Gdb then explicitly sets readline's screen size, which readline itself may
>    adjust to deal with lack of auto-wrap.  This becomes readlines notion
>    of screen size (well, internally the unadjusted one, but it'll report back
>    the adjusted one).
> 
> Add a command "maint info screen" that prints these notions, both for width
> and height.
> 
> For TERM=xterm we have:
> ...
> $ TERM=xterm gdb -ex "maint info screen"
> Number of characters gdb thinks are in a line is 118.
> Number of characters readline reports are in a line is 118.
> Number of characters curses thinks are in a line is 118.
> Number of characters environment thinks are in a line is 118 (COLUMNS).
> Number of lines gdb thinks are in a page is 27.
> Number of lines readline reports are in a page is 27.
> Number of lines curses thinks are in a page is 27.
> Number of lines environment thinks are in a page is 27 (LINES).
> ...
> 
> And for TERM=ansi:
> ...
> $ TERM=ansi gdb -ex "maint info screen"
> Number of characters gdb thinks are in a line is 117.
> Number of characters readline reports are in a line is 116.
> Number of characters curses thinks are in a line is 118.
> Number of characters environment thinks are in a line is 118 (COLUMNS).
> Number of lines gdb thinks are in a page is 27.
> Number of lines readline reports are in a page is 27.
> Number of lines curses thinks are in a page is 27.
> Number of lines environment thinks are in a page is 27 (LINES).
> ...
> 
> [ The fact that we have "characters readline reports are in a line is 116" is
> is due to gdb making readline adjust twice for the lack of auto-wrap, this is
> PR cli/30346.
> 
> Likewise we can detect tui/30337 by doing a resize in TUI mode and doing
> "maint info screen":
> ...
> Number of characters characters curses thinks are in a line is 110.
> Number of characters environment thinks are in a line is 111 (COLUMNS). ]
> 
> And for TERM=ansi, with width and heigth set to 0:
> ...
> Number of characters gdb thinks are in a line is 4294967295 (unlimited).
> Number of characters readline reports are in a line is 32766 (unlimited - 1).
> Number of characters curses thinks are in a line is 118.
> Number of characters environment thinks are in a line is 118 (COLUMNS).
> Number of lines gdb thinks are in a page is 4294967295 (unlimited).
> Number of lines readline reports are in a page is 32767 (unlimited).
> Number of lines curses thinks are in a page is 27.
> Number of lines environment thinks are in a page is 27 (LINES).
> ...
> 
> [ Note that when doing a resize by say maximizing or de-maximizing a terminal,
> all reported values are updated, except for curses when not in TUI mode.
> 
> Maybe that means there's a bug.  If not, then maybe we should not print
> the curses lines unless in TUI mode, or annotate those lines such that it's
> clear that the values may be not up-to-date. ]
> 
> I'd like to use this command in the regression test for PR cli/30346.
> 
> Tested on x86_64-linux.

I was building GDB in a Linux x86-64 VM that didn't have curses-dev installed and
this change causes GDB to no longer build:

   CXX    utils.o
/git/gdb/gdb/utils.c: In function ‘void maintenance_info_screen(const char*, int)’:
/git/gdb/gdb/utils.c:1310:14: error: ‘COLS’ was not declared in this scope
  1310 |              COLS);
       |              ^~~~
/git/gdb/gdb/utils.c:1331:15: error: ‘LINES’ was not declared in this scope; did you mean ‘LONGEST’?
  1331 |               LINES);
       |               ^~~~~
       |               LONGEST
make[2]: *** [Makefile:1920: utils.o] Error 1
  
Tom de Vries April 27, 2023, 9:06 p.m. UTC | #5
On 4/27/23 22:49, John Baldwin wrote:
> On 4/17/23 7:09 AM, Tom de Vries via Gdb-patches wrote:
>> While working on PRs tui/30337 and cli/30346 I came across various 
>> notions of
>> width in gdb, as reported by gdb, readline, curses and the environment
>> variables.
>>
>> As for gdb, readline and the environment variables, the way things work
>> is:
>> - Gdb asks readline to detect screen size,
>> - readline sets the actual screen size in the environment variables
>>    COLUMNS and LINES,
>> - readline reports back a screen size to gdb, which may have one column
>>    less than the actual screen size, to deal with lack of auto-wrap.
>>    This becomes gdb's notion of screen size (in other words the point 
>> where
>>    we can expect the gdb command line to wrap),
>> - Gdb then explicitly sets readline's screen size, which readline 
>> itself may
>>    adjust to deal with lack of auto-wrap.  This becomes readlines notion
>>    of screen size (well, internally the unadjusted one, but it'll 
>> report back
>>    the adjusted one).
>>
>> Add a command "maint info screen" that prints these notions, both for 
>> width
>> and height.
>>
>> For TERM=xterm we have:
>> ...
>> $ TERM=xterm gdb -ex "maint info screen"
>> Number of characters gdb thinks are in a line is 118.
>> Number of characters readline reports are in a line is 118.
>> Number of characters curses thinks are in a line is 118.
>> Number of characters environment thinks are in a line is 118 (COLUMNS).
>> Number of lines gdb thinks are in a page is 27.
>> Number of lines readline reports are in a page is 27.
>> Number of lines curses thinks are in a page is 27.
>> Number of lines environment thinks are in a page is 27 (LINES).
>> ...
>>
>> And for TERM=ansi:
>> ...
>> $ TERM=ansi gdb -ex "maint info screen"
>> Number of characters gdb thinks are in a line is 117.
>> Number of characters readline reports are in a line is 116.
>> Number of characters curses thinks are in a line is 118.
>> Number of characters environment thinks are in a line is 118 (COLUMNS).
>> Number of lines gdb thinks are in a page is 27.
>> Number of lines readline reports are in a page is 27.
>> Number of lines curses thinks are in a page is 27.
>> Number of lines environment thinks are in a page is 27 (LINES).
>> ...
>>
>> [ The fact that we have "characters readline reports are in a line is 
>> 116" is
>> is due to gdb making readline adjust twice for the lack of auto-wrap, 
>> this is
>> PR cli/30346.
>>
>> Likewise we can detect tui/30337 by doing a resize in TUI mode and doing
>> "maint info screen":
>> ...
>> Number of characters characters curses thinks are in a line is 110.
>> Number of characters environment thinks are in a line is 111 (COLUMNS). ]
>>
>> And for TERM=ansi, with width and heigth set to 0:
>> ...
>> Number of characters gdb thinks are in a line is 4294967295 (unlimited).
>> Number of characters readline reports are in a line is 32766 
>> (unlimited - 1).
>> Number of characters curses thinks are in a line is 118.
>> Number of characters environment thinks are in a line is 118 (COLUMNS).
>> Number of lines gdb thinks are in a page is 4294967295 (unlimited).
>> Number of lines readline reports are in a page is 32767 (unlimited).
>> Number of lines curses thinks are in a page is 27.
>> Number of lines environment thinks are in a page is 27 (LINES).
>> ...
>>
>> [ Note that when doing a resize by say maximizing or de-maximizing a 
>> terminal,
>> all reported values are updated, except for curses when not in TUI mode.
>>
>> Maybe that means there's a bug.  If not, then maybe we should not print
>> the curses lines unless in TUI mode, or annotate those lines such that 
>> it's
>> clear that the values may be not up-to-date. ]
>>
>> I'd like to use this command in the regression test for PR cli/30346.
>>
>> Tested on x86_64-linux.
> 
> I was building GDB in a Linux x86-64 VM that didn't have curses-dev 
> installed and
> this change causes GDB to no longer build:

Hi, this was already reported before ( 
https://sourceware.org/bugzilla/show_bug.cgi?id=30391 ), but using your 
report I can now also reproduce it after doing "sudo zypper remove 
ncurses-devel", thanks.

- Tom
  

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 940315b2713..a170d895931 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -41363,6 +41363,13 @@  This option is @samp{on} by default for supported architectures.
 This option is useful for debugging @value{GDBN}'s use of the Pygments
 library when @value{GDBN} is built for an architecture that supports
 styling with the builtin disassembler
+
+@kindex maint info screen
+@cindex Show screen characteristics
+@item maint info screen
+Print various characteristics of the screen, such as various notions
+of width and height.
+
 @kindex maint space
 @cindex memory used by commands
 @item maint space @var{value}
diff --git a/gdb/utils.c b/gdb/utils.c
index 6ec1cc0d48d..b5bb84ce85d 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1200,6 +1200,15 @@  set_batch_flag_and_restore_page_info::~set_batch_flag_and_restore_page_info ()
   set_width ();
 }
 
+/* An approximation of SQRT(INT_MAX) that is:
+   - cheap to calculate,
+   - guaranteed to be smaller than SQRT(INT_MAX), such that
+     sqrt_int_max * sqrt_int_max doesn't overflow, and
+   - "close enough" to SQRT(INT_MAX), for instance for INT_MAX == 2147483647,
+     SQRT(INT_MAX) is ~46341 and sqrt_int_max == 32767.  */
+
+static const int sqrt_int_max = INT_MAX >> (sizeof (int) * 8 / 2);
+
 /* Set the screen size based on LINES_PER_PAGE and CHARS_PER_LINE.  */
 
 static void
@@ -1219,8 +1228,6 @@  set_screen_size (void)
      overflow in rl_set_screen_size, which multiplies rows and columns
      to compute the number of characters on the screen.  */
 
-  const int sqrt_int_max = INT_MAX >> (sizeof (int) * 8 / 2);
-
   if (rows <= 0 || rows > sqrt_int_max)
     {
       rows = sqrt_int_max;
@@ -1273,6 +1280,62 @@  set_screen_width_and_height (int width, int height)
   set_width ();
 }
 
+/* Implement "maint info screen".  */
+
+static void
+maintenance_info_screen (const char *args, int from_tty)
+{
+  int rows, cols;
+  rl_get_screen_size (&rows, &cols);
+
+  gdb_printf (gdb_stdout,
+	      _("Number of characters gdb thinks "
+		"are in a line is %u%s.\n"),
+	      chars_per_line,
+	      chars_per_line == UINT_MAX ? " (unlimited)" : "");
+
+  gdb_printf (gdb_stdout,
+	      _("Number of characters readline reports "
+		"are in a line is %d%s.\n"),
+	      cols,
+	      (cols == sqrt_int_max
+	       ? " (unlimited)"
+	       : (cols == sqrt_int_max - 1
+		  ? " (unlimited - 1)"
+		  : "")));
+
+  gdb_printf (gdb_stdout,
+	     _("Number of characters curses thinks "
+	       "are in a line is %d.\n"),
+	     COLS);
+
+  gdb_printf (gdb_stdout,
+	      _("Number of characters environment thinks "
+		"are in a line is %s (COLUMNS).\n"),
+	      getenv ("COLUMNS"));
+
+  gdb_printf (gdb_stdout,
+	      _("Number of lines gdb thinks are in a page is %u%s.\n"),
+	      lines_per_page,
+	      lines_per_page == UINT_MAX ? " (unlimited)" : "");
+
+  gdb_printf (gdb_stdout,
+	      _("Number of lines readline reports "
+		"are in a page is %d%s.\n"),
+	      rows,
+	      rows == sqrt_int_max ? " (unlimited)" : "");
+
+  gdb_printf (gdb_stdout,
+	     _("Number of lines curses thinks "
+	       "are in a page is %d.\n"),
+	      LINES);
+
+  gdb_printf (gdb_stdout,
+	      _("Number of lines environment thinks "
+		"are in a page is %s (LINES).\n"),
+	      getenv ("LINES"));
+}
+
 void
 pager_file::emit_style_escape (const ui_file_style &style)
 {
@@ -3648,6 +3711,9 @@  When set, debugging messages will be marked with seconds and microseconds."),
   add_internal_problem_command (&internal_warning_problem);
   add_internal_problem_command (&demangler_warning_problem);
 
+  add_cmd ("screen", class_maintenance, &maintenance_info_screen,
+	 _("Show screen characteristics."), &maintenanceinfolist);
+
 #if GDB_SELF_TEST
   selftests::register_test ("gdb_realpath", gdb_realpath_tests);
   selftests::register_test ("gdb_argv_array_view", gdb_argv_as_array_view_test);