[PATCH] Prevent overflow in rl_set_screen_size

[PATCH] Prevent overflow in rl_set_screen_size

[Saagar Jha]

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]

Hi,

I was running GDB under the undefined behavior sanitizer and I found a signed integer overflow in set_screen_size. I’ve attached a (IMO slightly clumsy, but I couldn’t think of a nicer way to solve this) patch that solves this issue. I couldn’t figure out how to formally test this code, but I can compile and run this on my computer running macOS Mojave 10.14.1. Would someone mind taking a look at this? This is my first set of contributions to GDB, so if there’s anything wrong (or you have general feedback) I’d love to hear about it!

Regards,
Saagar Jha

[-- Attachment #2: Prevent-overflow-in-rl_set_screen_size.patch --]
[-- Type: application/octet-stream, Size: 1600 bytes --]

From 9bf98bc3963d822e33f485067907b191420bb8e4 Mon Sep 17 00:00:00 2001
From: Saagar Jha <saagar@saagarjha.com>
Date: Tue, 22 May 2018 04:08:40 -0700
Subject: [PATCH 1/5] Prevent overflow in rl_set_screen_size

GDB calls rl_set_screen_size in readline with the current screen size,
measured in rows and columns. To represent "infinite" sizes, GDB passes
in INT_MAX; however, since rl_set_screen_size internally multiplies the
number of rows and columns, this causes a signed integer overflow. To
prevent this we can instead pass in the approximate square root of
INT_MAX (which is still reasonably large), so that even when the number
of rows and columns is "infinite" we don't overflow.

gdb/ChangeLog:
2018-05-22  Saagar Jha  <saagar@saagarjha.com>

	* utils.c: Reduce "infinite" rows and columns before calling
	rl_set_screen_size.
---
 gdb/utils.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index 8d4a744e71..56257c35cf 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1377,11 +1377,13 @@ set_screen_size (void)
   int rows = lines_per_page;
   int cols = chars_per_line;
 
+  // Use approximately sqrt(INT_MAX) instead of INT_MAX so that we don't
+  // overflow in rl_set_screen_size, which multiplies rows and columns
   if (rows <= 0)
-    rows = INT_MAX;
+    rows = INT_MAX >> (sizeof(int) * 8 / 2);
 
   if (cols <= 0)
-    cols = INT_MAX;
+    cols = INT_MAX >> (sizeof(int) * 8 / 2);
 
   /* Update Readline's idea of the terminal size.  */
   rl_set_screen_size (rows, cols);
-- 
2.19.1


[-- Attachment #3: Type: text/plain, Size: 1 bytes --]

Re: [PATCH] Prevent overflow in rl_set_screen_size

[Kevin Buettner]

On Fri, 26 Oct 2018 21:56:50 -0700
Saagar Jha <saagar@saagarjha.com> wrote:

> GDB calls rl_set_screen_size in readline with the current screen size,
> measured in rows and columns. To represent "infinite" sizes, GDB passes
> in INT_MAX; however, since rl_set_screen_size internally multiplies the
> number of rows and columns, this causes a signed integer overflow. To
> prevent this we can instead pass in the approximate square root of
> INT_MAX (which is still reasonably large), so that even when the number
> of rows and columns is "infinite" we don't overflow.

This seems like a reasonable approach to me.  (I couldn't think of a
better way to do it.)

> gdb/ChangeLog:
> 2018-05-22  Saagar Jha  <saagar@saagarjha.com>
> 
> 	* utils.c: Reduce "infinite" rows and columns before calling
> 	rl_set_screen_size.

When you check this in, make sure that you adjust the date.  Also,
the ChangeLog comment should include the name of the affected function.
So it'll look something like this:

 	* utils.c (set_screen_size): Reduce "infinite" rows and columns
	before calling rl_set_screen_size.

The actual content of the patch looks good to me.

Thanks,

Kevin

> ---
>  gdb/utils.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 8d4a744e71..56257c35cf 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1377,11 +1377,13 @@ set_screen_size (void)
>    int rows = lines_per_page;
>    int cols = chars_per_line;
>  
> +  // Use approximately sqrt(INT_MAX) instead of INT_MAX so that we don't
> +  // overflow in rl_set_screen_size, which multiplies rows and columns
>    if (rows <= 0)
> -    rows = INT_MAX;
> +    rows = INT_MAX >> (sizeof(int) * 8 / 2);
>  
>    if (cols <= 0)
> -    cols = INT_MAX;
> +    cols = INT_MAX >> (sizeof(int) * 8 / 2);
>  
>    /* Update Readline's idea of the terminal size.  */
>    rl_set_screen_size (rows, cols);
> -- 
> 2.19.1
> 
> 

Re: [PATCH] Prevent overflow in rl_set_screen_size

[Pedro Alves]

On 02/15/2019 01:52 AM, Kevin Buettner wrote:
> On Fri, 26 Oct 2018 21:56:50 -0700
> Saagar Jha <saagar@saagarjha.com> wrote:
> 
>> GDB calls rl_set_screen_size in readline with the current screen size,
>> measured in rows and columns. To represent "infinite" sizes, GDB passes
>> in INT_MAX; however, since rl_set_screen_size internally multiplies the
>> number of rows and columns, this causes a signed integer overflow. To
>> prevent this we can instead pass in the approximate square root of
>> INT_MAX (which is still reasonably large), so that even when the number
>> of rows and columns is "infinite" we don't overflow.
> 
> This seems like a reasonable approach to me.  (I couldn't think of a
> better way to do it.)

It might be reasonable to have this as workaround, but pedantically,
shouldn't this be fixed in readline?  The function's
documentation doesn't say anything about upper limits:

 "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."

so if the function takes int parameters without specifying an upper bound, it
seems like a readline bug to me to not consider large numbers.

A couple comments on formatting below.

>> diff --git a/gdb/utils.c b/gdb/utils.c
>> index 8d4a744e71..56257c35cf 100644
>> --- a/gdb/utils.c
>> +++ b/gdb/utils.c
>> @@ -1377,11 +1377,13 @@ set_screen_size (void)
>>    int rows = lines_per_page;
>>    int cols = chars_per_line;
>>  
>> +  // Use approximately sqrt(INT_MAX) instead of INT_MAX so that we don't
>> +  // overflow in rl_set_screen_size, which multiplies rows and columns

Please use /**/ for comments, and end the sentence with a period.

>>    if (rows <= 0)
>> -    rows = INT_MAX;
>> +    rows = INT_MAX >> (sizeof(int) * 8 / 2);

Space before parens in "sizeof(int)".

>>  
>>    if (cols <= 0)
>> -    cols = INT_MAX;
>> +    cols = INT_MAX >> (sizeof(int) * 8 / 2);

Ditto.

>>  
>>    /* Update Readline's idea of the terminal size.  */
>>    rl_set_screen_size (rows, cols);
>> -- 
>> 2.19.1
>>
>>


Thanks,
Pedro Alves

Re: [PATCH] Prevent overflow in rl_set_screen_size

[Saagar Jha]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

Here’s a new patch. I can try to see what readline is willing to do regarding documenting this limitation, but that might take a while because I’d have to figure out where to send contributions and the process to getting them merged in (I *think* I’m supposed to send an email to  bug-readline@gnu.org?) Hopefully this patch is useful on its own until readline is fixed.

Regards,
Saagar Jha


[-- Attachment #2: Prevent-overflow-in-rl_set_screen_size.patch --]
[-- Type: application/octet-stream, Size: 1622 bytes --]

From 7955608ce06dbd8944292b2318b90863d3a82ae1 Mon Sep 17 00:00:00 2001
From: Saagar Jha <saagar@saagarjha.com>
Date: Tue, 22 May 2018 04:08:40 -0700
Subject: [PATCH] Prevent overflow in rl_set_screen_size

GDB calls rl_set_screen_size in readline with the current screen size,
measured in rows and columns. To represent "infinite" sizes, GDB passes
in INT_MAX; however, since rl_set_screen_size internally multiplies the
number of rows and columns, this causes a signed integer overflow. To
prevent this we can instead pass in the approximate square root of
INT_MAX (which is still reasonably large), so that even when the number
of rows and columns is "infinite" we don't overflow.

gdb/ChangeLog:
2019-02-15  Saagar Jha  <saagar@saagarjha.com>

	* utils.c (set_screen_size): Reduce "infinite" rows and columns
	before calling rl_set_screen_size.
---
 gdb/utils.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index 6fb5736abb..abca7337ab 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1376,11 +1376,13 @@ set_screen_size (void)
   int rows = lines_per_page;
   int cols = chars_per_line;
 
+  /* Use approximately sqrt(INT_MAX) instead of INT_MAX so that we don't
+     overflow in rl_set_screen_size, which multiplies rows and columns. */
   if (rows <= 0)
-    rows = INT_MAX;
+    rows = INT_MAX >> (sizeof( int ) * 8 / 2);
 
   if (cols <= 0)
-    cols = INT_MAX;
+    cols = INT_MAX >> (sizeof( int ) * 8 / 2);
 
   /* Update Readline's idea of the terminal size.  */
   rl_set_screen_size (rows, cols);
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 2424 bytes --]


> On Feb 15, 2019, at 01:39, Pedro Alves <palves@redhat.com> wrote:
> 
> On 02/15/2019 01:52 AM, Kevin Buettner wrote:
>> On Fri, 26 Oct 2018 21:56:50 -0700
>> Saagar Jha <saagar@saagarjha.com> wrote:
>> 
>>> GDB calls rl_set_screen_size in readline with the current screen size,
>>> measured in rows and columns. To represent "infinite" sizes, GDB passes
>>> in INT_MAX; however, since rl_set_screen_size internally multiplies the
>>> number of rows and columns, this causes a signed integer overflow. To
>>> prevent this we can instead pass in the approximate square root of
>>> INT_MAX (which is still reasonably large), so that even when the number
>>> of rows and columns is "infinite" we don't overflow.
>> 
>> This seems like a reasonable approach to me.  (I couldn't think of a
>> better way to do it.)
> 
> It might be reasonable to have this as workaround, but pedantically,
> shouldn't this be fixed in readline?  The function's
> documentation doesn't say anything about upper limits:
> 
> "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."
> 
> so if the function takes int parameters without specifying an upper bound, it
> seems like a readline bug to me to not consider large numbers.
> 
> A couple comments on formatting below.
> 
>>> diff --git a/gdb/utils.c b/gdb/utils.c
>>> index 8d4a744e71..56257c35cf 100644
>>> --- a/gdb/utils.c
>>> +++ b/gdb/utils.c
>>> @@ -1377,11 +1377,13 @@ set_screen_size (void)
>>>   int rows = lines_per_page;
>>>   int cols = chars_per_line;
>>> 
>>> +  // Use approximately sqrt(INT_MAX) instead of INT_MAX so that we don't
>>> +  // overflow in rl_set_screen_size, which multiplies rows and columns
> 
> Please use /**/ for comments, and end the sentence with a period.
> 
>>>   if (rows <= 0)
>>> -    rows = INT_MAX;
>>> +    rows = INT_MAX >> (sizeof(int) * 8 / 2);
> 
> Space before parens in "sizeof(int)".
> 
>>> 
>>>   if (cols <= 0)
>>> -    cols = INT_MAX;
>>> +    cols = INT_MAX >> (sizeof(int) * 8 / 2);
> 
> Ditto.
> 
>>> 
>>>   /* Update Readline's idea of the terminal size.  */
>>>   rl_set_screen_size (rows, cols);
>>> -- 
>>> 2.19.1
>>> 
>>> 
> 
> 
> Thanks,
> Pedro Alves

Re: [PATCH] Prevent overflow in rl_set_screen_size

[Tom Tromey]

>>>>> "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.

What's funny is that readline *does* check for negative values:

  if (rows > 0)
    _rl_screenheight = rows;
  .. etc ..

So gdb's approach:

  if (rows <= 0)
    rows = INT_MAX;

... actively works around the existing checks in readline.

Tom

Re: [PATCH] Prevent overflow in rl_set_screen_size

[Pedro Alves]

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.

Thanks,
Pedro Alves

Re: [PATCH] Prevent overflow in rl_set_screen_size

[Pedro Alves]

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?

I've also extended the comment based on my previous email.

From ce69f10a95fea289676bfb5db58b096546befe4f Mon Sep 17 00:00:00 2001
From: Saagar Jha <saagar@saagarjha.com>
Date: Tue, 22 May 2018 04:08:40 -0700
Subject: [PATCH] Prevent overflow in rl_set_screen_size

GDB calls rl_set_screen_size in readline with the current screen size,
measured in rows and columns.  To represent "infinite" sizes, GDB
passes in INT_MAX; however, since rl_set_screen_size internally
multiplies the number of rows and columns, this causes a signed
integer overflow.  To prevent this we can instead pass in the
approximate square root of INT_MAX (which is still reasonably large),
so that even when the number of rows and columns is "infinite" we
don't overflow.

gdb/ChangeLog:
yyyy-mm-dd  Saagar Jha  <saagar@saagarjha.com>
	    Pedro Alves  <palves@redhat.com>

	* utils.c (set_screen_size): Reduce "infinite" rows and columns
	before calling rl_set_screen_size.
---
 gdb/utils.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index ec2619642a..069da23542 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1380,11 +1380,24 @@ set_screen_size (void)
   int rows = lines_per_page;
   int cols = chars_per_line;
 
-  if (rows <= 0)
-    rows = INT_MAX;
+  /* If we get 0 or negative ROWS or COLS, treat as "infinite" size.
+     A negative number can be seen here with the "set width/height"
+     commands and either:
 
-  if (cols <= 0)
-    cols = INT_MAX;
+     - the user specified "unlimited", which maps to UINT_MAX, or
+     - the user spedified some number between INT_MAX and UINT_MAX.
+
+     Cap "infinity" to approximately sqrt(INT_MAX) so that we don't
+     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;
+
+  if (cols <= 0 || cols > sqrt_int_max)
+    cols = sqrt_int_max;
 
   /* Update Readline's idea of the terminal size.  */
   rl_set_screen_size (rows, cols);
-- 
2.14.4

[PATCH 2/2] Make 'show width/height' display "unlimited" when capped, for readline (Re: [PATCH] Prevent overflow in rl_set_screen_size)

[Pedro Alves]

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

Re: [PATCH 2/2] Make 'show width/height' display "unlimited" when capped, for readline (Re: [PATCH] Prevent overflow in rl_set_screen_size)

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> So how about this version instead?

Pedro> And this as a follow up?

These both look good to me.
Thanks for following up on this.

Tom

Re: [PATCH 2/2] Make 'show width/height' display "unlimited" when capped, for readline (Re: [PATCH] Prevent overflow in rl_set_screen_size)

[Kevin Buettner]

On Wed, 20 Feb 2019 14:04:28 -0700
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:  
> 
> Pedro> So how about this version instead?  
> 
> Pedro> And this as a follow up?  
> 
> These both look good to me.
> Thanks for following up on this.

LGTM too.

Kevin

Re: [PATCH 2/2] Make 'show width/height' display "unlimited" when capped, for readline (Re: [PATCH] Prevent overflow in rl_set_screen_size)

[Pedro Alves]

On 02/20/2019 11:01 PM, Kevin Buettner wrote:
> On Wed, 20 Feb 2019 14:04:28 -0700
> Tom Tromey <tom@tromey.com> wrote:
> 
>>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:  
>>
>> Pedro> So how about this version instead?  
>>
>> Pedro> And this as a follow up?  
>>
>> These both look good to me.
>> Thanks for following up on this.
> 
> LGTM too.
Thanks.  I've checked these in now.

Thanks,
Pedro Alves

[PATCH] Test "set width/height -1" (Re: [PATCH 2/2] Make 'show width/height' display "unlimited" when capped for readline)

[Pedro Alves]

On 02/27/2019 06:50 PM, Pedro Alves wrote:
> On 02/20/2019 11:01 PM, Kevin Buettner wrote:
>> On Wed, 20 Feb 2019 14:04:28 -0700
>> Tom Tromey <tom@tromey.com> wrote:
>>
>>>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:  
>>>
>>> Pedro> So how about this version instead?  
>>>
>>> Pedro> And this as a follow up?  
>>>
>>> These both look good to me.
>>> Thanks for following up on this.
>>
>> LGTM too.
> Thanks.  I've checked these in now.

And this obvious follow up too.

From 5862844d0f443b9f65e8dd0d85c43f8818d3f355 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 27 Feb 2019 18:48:37 +0000
Subject: [PATCH] Test "set width/height -1"

As a follow up to the previous commit, add a test for "set
width/height -1", to make sure we don't overflow in readline with
negative values either.

gdb/testsuite/ChangeLog:
2019-02-27  Pedro Alves  <palves@redhat.com>

	* gdb.base/page.exp: Add tests for "set width/height -1".
---
 gdb/testsuite/ChangeLog         | 4 ++++
 gdb/testsuite/gdb.base/page.exp | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b5177c7e07..29b2832764 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2019-02-27  Pedro Alves  <palves@redhat.com>
+
+	* gdb.base/page.exp: Add tests for "set width/height -1".
+
 2019-02-27  Pedro Alves  <palves@redhat.com>
 
 	* gdb.base/page.exp: Add tests for "set/show width/height" with
diff --git a/gdb/testsuite/gdb.base/page.exp b/gdb/testsuite/gdb.base/page.exp
index 8f1698c26e..68d94b38a3 100644
--- a/gdb/testsuite/gdb.base/page.exp
+++ b/gdb/testsuite/gdb.base/page.exp
@@ -118,6 +118,9 @@ foreach_with_prefix size {"0" "0x80000000" "unlimited"} {
 	"Number of lines gdb thinks are in a page is unlimited\\."
 }
 
+gdb_test "set width -1" "integer -1 out of range"
+gdb_test "set height -1" "integer -1 out of range"
+
 gdb_exit
 return 0
 
-- 
2.14.4