Assert that 'length' > 0 on infcmd.c:construct_inferior_arguments

  While testing a GCC 10 build of our git HEAD, I noticed an error
triggered by -Werror-stringop on
infcmd.c:construct_inferior_arguments.  One of the things the function
does is calculate the length of the string that will hold the
inferior's arguments.  GCC warns us that 'length' can be 0, which can
lead to undesired behaviour:

../../gdb/infcmd.c: In function 'char* construct_inferior_arguments(int, char**)':
../../gdb/infcmd.c:369:17: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
  369 |       result[0] = '\0';
      |       ~~~~~~~~~~^~~~~~
../../gdb/infcmd.c:368:33: note: at offset 0 to an object with size 0 allocated by 'xmalloc' here
  368 |       result = (char *) xmalloc (length);
      |                         ~~~~~~~~^~~~~~~~

The solution here is to explicit check that 'length' is greater than
0.  Since we're dealing with 'argc', I think it's pretty much
guaranteed that it's going to be at least 1.

Tested by rebuilding.

OK?

gdb/ChangeLog:
2020-01-29  Sergio Durigan Junior  <sergiodj@redhat.com>

	* infcmd.c (construct_inferior_arguments): Assert that
	'length' is greater than 0.

Change-Id: Ide8407cbedcb4921de1843a6a15bbcb7676c7d26
---
 gdb/infcmd.c | 5 +++++
 1 file changed, 5 insertions(+)
  

On 1/29/20 5:59 PM, Sergio Durigan Junior wrote:
> While testing a GCC 10 build of our git HEAD, I noticed an error
> triggered by -Werror-stringop on
> infcmd.c:construct_inferior_arguments.  One of the things the function
> does is calculate the length of the string that will hold the
> inferior's arguments.  GCC warns us that 'length' can be 0, which can
> lead to undesired behaviour:
> 
> ../../gdb/infcmd.c: In function 'char* construct_inferior_arguments(int, char**)':
> ../../gdb/infcmd.c:369:17: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
>   369 |       result[0] = '\0';
>       |       ~~~~~~~~~~^~~~~~
> ../../gdb/infcmd.c:368:33: note: at offset 0 to an object with size 0 allocated by 'xmalloc' here
>   368 |       result = (char *) xmalloc (length);
>       |                         ~~~~~~~~^~~~~~~~
> 

> The solution here is to explicit check that 'length' is greater than
> 0.  Since we're dealing with 'argc', I think it's pretty much
> guaranteed that it's going to be at least 1.

It's a certainly -- there's only one caller to construct_inferior_arguments,
which does:

const char *
get_inferior_args (void)
{
  if (current_inferior ()->argc != 0)
    {      
      char *n;

      n = construct_inferior_arguments (current_inferior ()->argc,
					current_inferior ()->argv);


(construct_inferior_arguments could be made static, btw.)


> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -365,6 +365,11 @@ construct_inferior_arguments (int argc, char **argv)
>  	  length += strlen (argv[i]) + 1;
>  	}
>  
> +      /* argc should always be at least 1, but we double check this

Uppercase ARGC.  But putting

 gdb_assert (argc > 0);

at the top of the function instead as I originally suggested also works
for me (tried current gcc master), which seems a bit better to me, as it
covers both branches at once.  Did it not work for you?  This makes gcc see
that the loops always run at least once.

Thanks,
Pedro Alves