[PATCH 1/1] libiberty(argv.c): Fix memory leak in expandargv.

[Martin Sebor]

On 2/18/21 3:58 AM, Ayush Mittal via Gcc-patches wrote:
> Dynamic memory referenced by 'buffer' was allocated using xmalloc but fails to free it
> when jump to 'error' label.
> 
> Issue as per static analysis tool.

The change looks okay to me although I can't approve it.  Since GCC
is a regression fixing stage, unless the leak is a recent regression
the fix is (strictly speaking) out of scope for GCC 11.  Either
a libiberty or a global maintainer might be comfortable approving it 
regardless.

That said, rather than adding another call to free, I would suggest
to consider initializing buffer to null and moving the existing call
to free the buffer under the error: label.

Martin

> 
> Signed-off-by: Ayush Mittal <ayush.m@samsung.com>
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> ---
>   libiberty/ChangeLog | 4 ++++
>   libiberty/argv.c    | 5 ++++-
>   2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
> index e472553..96cacba 100644
> --- a/libiberty/ChangeLog
> +++ b/libiberty/ChangeLog
> @@ -1,3 +1,7 @@
> +2021-02-18  Ayush Mittal  <ayush.m@samsung.com>
> +
> +	* argv.c (expandargv): Fix memory leak for buffer allocated to read file contents.
> +
>   2021-02-01  Martin Sebor  <msebor@redhat.com>
>   
>   	* dyn-string.c (dyn_string_insert_cstr): Use memcpy instead of strncpy
> diff --git a/libiberty/argv.c b/libiberty/argv.c
> index cd97f90..7199c7d 100644
> --- a/libiberty/argv.c
> +++ b/libiberty/argv.c
> @@ -442,7 +442,10 @@ expandargv (int *argcp, char ***argvp)
>   	     due to CR/LF->CR translation when reading text files.
>   	     That does not in-and-of itself indicate failure.  */
>   	  && ferror (f))
> -	goto error;
> +	{
> +	  free(buffer);
> +	  goto error;
> +	}
>         /* Add a NUL terminator.  */
>         buffer[len] = '\0';
>         /* If the file is empty or contains only whitespace, buildargv would
>