This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH 2/2] Make 'show width/height' display "unlimited" when capped, for readline (Re: [PATCH] Prevent overflow in rl_set_screen_size)
On 02/20/2019 05:22 PM, Pedro Alves wrote:
> On 02/20/2019 03:54 PM, Pedro Alves wrote:
>> On 02/15/2019 08:19 PM, Tom Tromey wrote:
>>>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>>>
>>> Pedro> so if the function takes int parameters without specifying an upper bound, it
>>> Pedro> seems like a readline bug to me to not consider large numbers.
>>>
>>> True, though it doesn't hurt to also check in gdb.
>>
>> Yeah, that's what I meant to imply with
>> "It might be reasonable to have this as workaround"
>> Maybe not the best choice of words.
>>
>>>
>>> What's funny is that readline *does* check for negative values:
>>>
>>> if (rows > 0)
>>> _rl_screenheight = rows;
>>> .. etc ..
>>
>> Yeah, it's implementing what is documented:
>>
>> "Function: void rl_set_screen_size (int rows, int cols)
>> Set Readline's idea of the terminal size to rows rows and cols columns.
>> If either rows or columns is less than or equal to 0, Readline's idea
>> ^^^^^^^^^^^^^^^^^^^^^^^
>> of that terminal dimension is unchanged."
>>
>> Note the "less than or equal to 0". I would assume that that
>> comes from a "it's obviously bogus to have negative sizes, so we'll
>> just ignore them" perspective.
>>
>>>
>>> So gdb's approach:
>>>
>>> if (rows <= 0)
>>> rows = INT_MAX;
>>>
>>> ... actively works around the existing checks in readline.
>>
>> I'd call it more like mapping different ranges/APIs. gdb
>> uses "0" or UINT_MAX for unlimited:
>>
>> (gdb) help set height
>> Set number of lines in a page for GDB output pagination.
>> This affects the number of lines after which GDB will pause
>> its output and ask you whether to continue.
>> Setting this to "unlimited" or zero causes GDB never pause during output.
>>
>> While negative numbers don't work on the command line:
>>
>> (gdb) set height -1
>> integer -1 out of range
>>
>> you end up with negative rows/cols in that quoted code if you
>> do "set height/width unlimited", because lines_per_page/chars_per_line
>> are unsigned integers and "unlimited" sets them to UINT_MAX. And
>> also, if you do
>> (gdb) set height 'unsigned number between INT_MAX and UINT_MAX'
>> like:
>> (gdb) set height 0x80000000
>>
>> then that code:
>>
>> if (rows <= 0)
>> rows = INT_MAX;
>>
>> maps the value to INT_MAX, which is basically the same thing in
>> practice -- a huge number is practically the same as "unlimited" here.
>
> Which makes me think that to be 100% correct wrt to avoiding overflow in
> readline, we should also treat numbers >= sqrt(INT_MAX) as "infinite".
> Because otherwise, with
>
> (gdb) set height 0x7ffff
> (gdb) set width 0x7ffff
>
> readline overflows too, even with Saagar's current patch, obviously
> because 0x7ffff x 0x7ffff overflows int:
>
> (gdb) p 0x7ffff * 0x7ffff
> $1 = -1048575
>
> So how about this version instead?
And this as a follow up?
>From f9124a178e0aeacfa63f18aa742f6b7c8762051b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 20 Feb 2019 17:12:03 +0000
Subject: [PATCH 2/2] Make 'show width/height' display "unlimited" when capped
for readline
When we cap the height/width sizes before passing to readline, tweak
the corresponding command variable to show "unlimited":
(gdb) set height 0x8000
(gdb) show height
Number of lines gdb thinks are in a page is unlimited.
Instead of the current output:
(gdb) set height 0x8000
(gdb) show height
Number of lines gdb thinks are in a page is 32768.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* utils.c (set_screen_size): When we cap the height/width sizes,
tweak the corresponding command variable to show "unlimited":
gdb/testsuite/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* gdb.base/page.exp: Add tests for "set/show width/height" with
"infinite" values.
---
gdb/testsuite/gdb.base/page.exp | 24 ++++++++++++++++++++++++
gdb/utils.c | 10 ++++++++--
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/gdb/testsuite/gdb.base/page.exp b/gdb/testsuite/gdb.base/page.exp
index 10ebf0d43b..8f1698c26e 100644
--- a/gdb/testsuite/gdb.base/page.exp
+++ b/gdb/testsuite/gdb.base/page.exp
@@ -94,6 +94,30 @@ gdb_expect_list "paged count for interrupt" \
gdb_test "q" "Quit" "quit while paging"
+# Check that width/height of sqrt(INT_MAX) is treated as unlimited, as
+# well as "0" and explicit "unlimited".
+foreach_with_prefix size {"0" "0x80000000" "unlimited"} {
+
+ # Alternate between "non-unlimited" values and "unlimited" values,
+ # to make sure we're not seeing stale internal state.
+
+ gdb_test "set width 200"
+ gdb_test "show width" \
+ "Number of characters gdb thinks are in a line is 200\\."
+
+ gdb_test "set height 200"
+ gdb_test "show height" \
+ "Number of lines gdb thinks are in a page is 200\\."
+
+ gdb_test "set width $size"
+ gdb_test "show width unlimited" \
+ "Number of characters gdb thinks are in a line is unlimited\\."
+
+ gdb_test "set height $size"
+ gdb_test "show height unlimited" \
+ "Number of lines gdb thinks are in a page is unlimited\\."
+}
+
gdb_exit
return 0
diff --git a/gdb/utils.c b/gdb/utils.c
index 069da23542..60af31f2e4 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1394,10 +1394,16 @@ set_screen_size (void)
const int sqrt_int_max = INT_MAX >> (sizeof (int) * 8 / 2);
if (rows <= 0 || rows > sqrt_int_max)
- rows = sqrt_int_max;
+ {
+ rows = sqrt_int_max;
+ lines_per_page = UINT_MAX;
+ }
if (cols <= 0 || cols > sqrt_int_max)
- cols = sqrt_int_max;
+ {
+ cols = sqrt_int_max;
+ chars_per_line = UINT_MAX;
+ }
/* Update Readline's idea of the terminal size. */
rl_set_screen_size (rows, cols);
--
2.14.4