Re: [PATCH] Assert that 'length' > 0 on infcmd.c:construct_inferior_arguments

[Pedro Alves <palves@redhat.com>]

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