Re: [PATCH] Prevent overflow in rl_set_screen_size

[Kevin Buettner <kevinb at redhat dot com>]

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