[PATCH] gcc-ar: Handle response files properly [PR77576]

[PATCH] gcc-ar: Handle response files properly [PR77576]

[Costas Argyris]

[-- Attachment #1.1: Type: text/plain, Size: 399 bytes --]

Basically implementing what Andrew said in the PR:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77576

If @file has been passed to gcc-ar, do the following:

1) Expand it to get an argv without any @files.
2) Then apply the plugin modifications to argv.
3) Create temporary response file.
4) Put the modified argv in the temporary file.
5) Call ar with @tmp.
6) Delete the temporary response file.

[-- Attachment #2: 0001-gcc-ar-Handle-response-files-properly-PR77576.patch --]
[-- Type: application/octet-stream, Size: 2896 bytes --]

From 1eb857daa1cdc75827efeb609f91c90b1dd539eb Mon Sep 17 00:00:00 2001
From: Costas Argyris <costas.argyris@gmail.com>
Date: Sat, 1 Jul 2023 22:00:44 +0100
Subject: [PATCH] gcc-ar: Handle response files properly [PR77576]

If @file has been passed to gcc-ar, do the following:

1) Expand it to get an argv without any @files.
2) Then apply the plugin modifications to argv.
3) Create temporary response file.
4) Put the modified argv in the temporary file.
5) Call ar with @tmp.
6) Delete the temporary response file.

Signed-off-by: Costas Argyris <costas.argyris@gmail.com>
---
 gcc/gcc-ar.cc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/gcc/gcc-ar.cc b/gcc/gcc-ar.cc
index 4e4c525927d..417c4913793 100644
--- a/gcc/gcc-ar.cc
+++ b/gcc/gcc-ar.cc
@@ -135,6 +135,10 @@ main (int ac, char **av)
   int k, status, err;
   const char *err_msg;
   const char **nargv;
+  char **old_argv;
+  const char *rsp_file = NULL;
+  const char *rsp_arg = NULL;
+  const char *rsp_argv[3];
   bool is_ar = !strcmp (PERSONALITY, "ar");
   int exit_code = FATAL_EXIT_CODE;
   int i;
@@ -209,6 +213,13 @@ main (int ac, char **av)
 	}
     }
 
+  /* Expand any @files before modifying the command line
+     and use a temporary response file if there were any.  */
+  old_argv = av;
+  expandargv (&ac, &av);
+  if (av != old_argv)
+    rsp_file = make_temp_file ("");
+
   /* Prepend - if necessary.  */
   if (is_ar && av[1] && av[1][0] != '-')
     av[1] = concat ("-", av[1], NULL);
@@ -225,6 +236,39 @@ main (int ac, char **av)
     nargv[j + k] = av[k];
   nargv[j + k] = NULL;
 
+  /* If @file was passed, put nargv into the temporary response
+     file and then change it to a single @FILE argument, where
+     FILE is the temporary filename.  */
+  if (rsp_file)
+    {
+      FILE *f;
+      int status;
+      f = fopen (rsp_file, "w");
+      if (f == NULL)
+        {
+          fprintf (stderr, "Cannot open temporary file %s\n", rsp_file);
+          exit (1);
+        }
+      status = writeargv (
+          CONST_CAST2 (char * const *, const char **, nargv) + 1, f);
+      if (status)
+        {
+          fprintf (stderr, "Cannot write to temporary file %s\n", rsp_file);
+          exit (1);
+        }
+      status = fclose (f);
+      if (EOF == status)
+        {
+          fprintf (stderr, "Cannot close temporary file %s\n", rsp_file);
+          exit (1);
+        }
+      rsp_arg = concat ("@", rsp_file, NULL);
+      rsp_argv[0] = nargv[0];
+      rsp_argv[1] = rsp_arg;
+      rsp_argv[2] = NULL;
+      nargv = rsp_argv;
+    }
+
   /* Run utility */
   /* ??? the const is misplaced in pex_one's argv? */
   err_msg = pex_one (PEX_LAST|PEX_SEARCH, 
@@ -249,5 +293,8 @@ main (int ac, char **av)
   else
     exit_code = SUCCESS_EXIT_CODE;
 
+  if (rsp_file)
+    unlink (rsp_file);
+
   return exit_code;
 }
-- 
2.30.2

Re: [PATCH] gcc-ar: Handle response files properly [PR77576]

[Costas Argyris]

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

I should also add that for a rsp file that contains just "--version":

gcc-ar @rsp

fails without the patch (current problem) and successfully prints
the version info with it.


On Sat, 1 Jul 2023 at 22:45, Costas Argyris <costas.argyris@gmail.com>
wrote:

> Basically implementing what Andrew said in the PR:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77576
>
> If @file has been passed to gcc-ar, do the following:
>
> 1) Expand it to get an argv without any @files.
> 2) Then apply the plugin modifications to argv.
> 3) Create temporary response file.
> 4) Put the modified argv in the temporary file.
> 5) Call ar with @tmp.
> 6) Delete the temporary response file.
>

Re: [PATCH] gcc-ar: Handle response files properly [PR77576]

[Costas Argyris]

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

Problem: gcc-ar fails when a @file is passed to it:

$ cat rsp
--version
$ gcc-ar @rsp
/usr/bin/ar: invalid option -- '@'

This is because a dash '-' is prepended to the first
argument if it doesn't start with one, resulting in
the wrong call 'ar -@rsp'.

Fix: Expand argv to get rid of any @files and if any
expansions were made, pass everything through a
temporary response file.

$ gcc-ar @rsp
GNU ar (GNU Binutils for Debian) 2.35.2
...


PR gcc-ar/77576
* gcc/gcc-ar.cc (main): Expand argv and use
temporary response file to call ar if any
expansions were made.
---
 gcc/gcc-ar.cc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/gcc/gcc-ar.cc b/gcc/gcc-ar.cc
index 4e4c525927d..417c4913793 100644
--- a/gcc/gcc-ar.cc
+++ b/gcc/gcc-ar.cc
@@ -135,6 +135,10 @@ main (int ac, char **av)
   int k, status, err;
   const char *err_msg;
   const char **nargv;
+  char **old_argv;
+  const char *rsp_file = NULL;
+  const char *rsp_arg = NULL;
+  const char *rsp_argv[3];
   bool is_ar = !strcmp (PERSONALITY, "ar");
   int exit_code = FATAL_EXIT_CODE;
   int i;
@@ -209,6 +213,13 @@ main (int ac, char **av)
  }
     }

+  /* Expand any @files before modifying the command line
+     and use a temporary response file if there were any.  */
+  old_argv = av;
+  expandargv (&ac, &av);
+  if (av != old_argv)
+    rsp_file = make_temp_file ("");
+
   /* Prepend - if necessary.  */
   if (is_ar && av[1] && av[1][0] != '-')
     av[1] = concat ("-", av[1], NULL);
@@ -225,6 +236,39 @@ main (int ac, char **av)
     nargv[j + k] = av[k];
   nargv[j + k] = NULL;

+  /* If @file was passed, put nargv into the temporary response
+     file and then change it to a single @FILE argument, where
+     FILE is the temporary filename.  */
+  if (rsp_file)
+    {
+      FILE *f;
+      int status;
+      f = fopen (rsp_file, "w");
+      if (f == NULL)
+        {
+          fprintf (stderr, "Cannot open temporary file %s\n", rsp_file);
+          exit (1);
+        }
+      status = writeargv (
+          CONST_CAST2 (char * const *, const char **, nargv) + 1, f);
+      if (status)
+        {
+          fprintf (stderr, "Cannot write to temporary file %s\n",
rsp_file);
+          exit (1);
+        }
+      status = fclose (f);
+      if (EOF == status)
+        {
+          fprintf (stderr, "Cannot close temporary file %s\n", rsp_file);
+          exit (1);
+        }
+      rsp_arg = concat ("@", rsp_file, NULL);
+      rsp_argv[0] = nargv[0];
+      rsp_argv[1] = rsp_arg;
+      rsp_argv[2] = NULL;
+      nargv = rsp_argv;
+    }
+
   /* Run utility */
   /* ??? the const is misplaced in pex_one's argv? */
   err_msg = pex_one (PEX_LAST|PEX_SEARCH,
@@ -249,5 +293,8 @@ main (int ac, char **av)
   else
     exit_code = SUCCESS_EXIT_CODE;

+  if (rsp_file)
+    unlink (rsp_file);
+
   return exit_code;
 }
-- 
2.30.2

Re: [PATCH] gcc-ar: Handle response files properly [PR77576]

[Costas Argyris]

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

Bootstrapped successfully on x86_64-pc-linux-gnu

On Fri, 7 Jul 2023 at 11:33, Costas Argyris <costas.argyris@gmail.com>
wrote:

> Problem: gcc-ar fails when a @file is passed to it:
>
> $ cat rsp
> --version
> $ gcc-ar @rsp
> /usr/bin/ar: invalid option -- '@'
>
> This is because a dash '-' is prepended to the first
> argument if it doesn't start with one, resulting in
> the wrong call 'ar -@rsp'.
>
> Fix: Expand argv to get rid of any @files and if any
> expansions were made, pass everything through a
> temporary response file.
>
> $ gcc-ar @rsp
> GNU ar (GNU Binutils for Debian) 2.35.2
> ...
>
>
> PR gcc-ar/77576
> * gcc/gcc-ar.cc (main): Expand argv and use
> temporary response file to call ar if any
> expansions were made.
> ---
>  gcc/gcc-ar.cc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/gcc/gcc-ar.cc b/gcc/gcc-ar.cc
> index 4e4c525927d..417c4913793 100644
> --- a/gcc/gcc-ar.cc
> +++ b/gcc/gcc-ar.cc
> @@ -135,6 +135,10 @@ main (int ac, char **av)
>    int k, status, err;
>    const char *err_msg;
>    const char **nargv;
> +  char **old_argv;
> +  const char *rsp_file = NULL;
> +  const char *rsp_arg = NULL;
> +  const char *rsp_argv[3];
>    bool is_ar = !strcmp (PERSONALITY, "ar");
>    int exit_code = FATAL_EXIT_CODE;
>    int i;
> @@ -209,6 +213,13 @@ main (int ac, char **av)
>   }
>      }
>
> +  /* Expand any @files before modifying the command line
> +     and use a temporary response file if there were any.  */
> +  old_argv = av;
> +  expandargv (&ac, &av);
> +  if (av != old_argv)
> +    rsp_file = make_temp_file ("");
> +
>    /* Prepend - if necessary.  */
>    if (is_ar && av[1] && av[1][0] != '-')
>      av[1] = concat ("-", av[1], NULL);
> @@ -225,6 +236,39 @@ main (int ac, char **av)
>      nargv[j + k] = av[k];
>    nargv[j + k] = NULL;
>
> +  /* If @file was passed, put nargv into the temporary response
> +     file and then change it to a single @FILE argument, where
> +     FILE is the temporary filename.  */
> +  if (rsp_file)
> +    {
> +      FILE *f;
> +      int status;
> +      f = fopen (rsp_file, "w");
> +      if (f == NULL)
> +        {
> +          fprintf (stderr, "Cannot open temporary file %s\n", rsp_file);
> +          exit (1);
> +        }
> +      status = writeargv (
> +          CONST_CAST2 (char * const *, const char **, nargv) + 1, f);
> +      if (status)
> +        {
> +          fprintf (stderr, "Cannot write to temporary file %s\n",
> rsp_file);
> +          exit (1);
> +        }
> +      status = fclose (f);
> +      if (EOF == status)
> +        {
> +          fprintf (stderr, "Cannot close temporary file %s\n", rsp_file);
> +          exit (1);
> +        }
> +      rsp_arg = concat ("@", rsp_file, NULL);
> +      rsp_argv[0] = nargv[0];
> +      rsp_argv[1] = rsp_arg;
> +      rsp_argv[2] = NULL;
> +      nargv = rsp_argv;
> +    }
> +
>    /* Run utility */
>    /* ??? the const is misplaced in pex_one's argv? */
>    err_msg = pex_one (PEX_LAST|PEX_SEARCH,
> @@ -249,5 +293,8 @@ main (int ac, char **av)
>    else
>      exit_code = SUCCESS_EXIT_CODE;
>
> +  if (rsp_file)
> +    unlink (rsp_file);
> +
>    return exit_code;
>  }
> --
> 2.30.2
>

Re: [PATCH] gcc-ar: Handle response files properly [PR77576]

[Costas Argyris]

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

Pinging to try and get this bug in gcc-ar fixed.

Note that the patch posted as an attachment in

https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623400.html

is exactly the same as the patch embedded in

https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623855.html

and the one posted in the PR itself

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77576

On Fri, 7 Jul 2023 at 13:00, Costas Argyris <costas.argyris@gmail.com>
wrote:

> Bootstrapped successfully on x86_64-pc-linux-gnu
>
> On Fri, 7 Jul 2023 at 11:33, Costas Argyris <costas.argyris@gmail.com>
> wrote:
>
>> Problem: gcc-ar fails when a @file is passed to it:
>>
>> $ cat rsp
>> --version
>> $ gcc-ar @rsp
>> /usr/bin/ar: invalid option -- '@'
>>
>> This is because a dash '-' is prepended to the first
>> argument if it doesn't start with one, resulting in
>> the wrong call 'ar -@rsp'.
>>
>> Fix: Expand argv to get rid of any @files and if any
>> expansions were made, pass everything through a
>> temporary response file.
>>
>> $ gcc-ar @rsp
>> GNU ar (GNU Binutils for Debian) 2.35.2
>> ...
>>
>>
>> PR gcc-ar/77576
>> * gcc/gcc-ar.cc (main): Expand argv and use
>> temporary response file to call ar if any
>> expansions were made.
>> ---
>>  gcc/gcc-ar.cc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 47 insertions(+)
>>
>> diff --git a/gcc/gcc-ar.cc b/gcc/gcc-ar.cc
>> index 4e4c525927d..417c4913793 100644
>> --- a/gcc/gcc-ar.cc
>> +++ b/gcc/gcc-ar.cc
>> @@ -135,6 +135,10 @@ main (int ac, char **av)
>>    int k, status, err;
>>    const char *err_msg;
>>    const char **nargv;
>> +  char **old_argv;
>> +  const char *rsp_file = NULL;
>> +  const char *rsp_arg = NULL;
>> +  const char *rsp_argv[3];
>>    bool is_ar = !strcmp (PERSONALITY, "ar");
>>    int exit_code = FATAL_EXIT_CODE;
>>    int i;
>> @@ -209,6 +213,13 @@ main (int ac, char **av)
>>   }
>>      }
>>
>> +  /* Expand any @files before modifying the command line
>> +     and use a temporary response file if there were any.  */
>> +  old_argv = av;
>> +  expandargv (&ac, &av);
>> +  if (av != old_argv)
>> +    rsp_file = make_temp_file ("");
>> +
>>    /* Prepend - if necessary.  */
>>    if (is_ar && av[1] && av[1][0] != '-')
>>      av[1] = concat ("-", av[1], NULL);
>> @@ -225,6 +236,39 @@ main (int ac, char **av)
>>      nargv[j + k] = av[k];
>>    nargv[j + k] = NULL;
>>
>> +  /* If @file was passed, put nargv into the temporary response
>> +     file and then change it to a single @FILE argument, where
>> +     FILE is the temporary filename.  */
>> +  if (rsp_file)
>> +    {
>> +      FILE *f;
>> +      int status;
>> +      f = fopen (rsp_file, "w");
>> +      if (f == NULL)
>> +        {
>> +          fprintf (stderr, "Cannot open temporary file %s\n", rsp_file);
>> +          exit (1);
>> +        }
>> +      status = writeargv (
>> +          CONST_CAST2 (char * const *, const char **, nargv) + 1, f);
>> +      if (status)
>> +        {
>> +          fprintf (stderr, "Cannot write to temporary file %s\n",
>> rsp_file);
>> +          exit (1);
>> +        }
>> +      status = fclose (f);
>> +      if (EOF == status)
>> +        {
>> +          fprintf (stderr, "Cannot close temporary file %s\n", rsp_file);
>> +          exit (1);
>> +        }
>> +      rsp_arg = concat ("@", rsp_file, NULL);
>> +      rsp_argv[0] = nargv[0];
>> +      rsp_argv[1] = rsp_arg;
>> +      rsp_argv[2] = NULL;
>> +      nargv = rsp_argv;
>> +    }
>> +
>>    /* Run utility */
>>    /* ??? the const is misplaced in pex_one's argv? */
>>    err_msg = pex_one (PEX_LAST|PEX_SEARCH,
>> @@ -249,5 +293,8 @@ main (int ac, char **av)
>>    else
>>      exit_code = SUCCESS_EXIT_CODE;
>>
>> +  if (rsp_file)
>> +    unlink (rsp_file);
>> +
>>    return exit_code;
>>  }
>> --
>> 2.30.2
>>
>

Re: [PATCH] gcc-ar: Handle response files properly [PR77576]

[Costas Argyris]

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

ping

On Fri, 14 Jul 2023 at 09:05, Costas Argyris <costas.argyris@gmail.com>
wrote:

> Pinging to try and get this bug in gcc-ar fixed.
>
> Note that the patch posted as an attachment in
>
> https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623400.html
>
> is exactly the same as the patch embedded in
>
> https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623855.html
>
> and the one posted in the PR itself
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77576
>
> On Fri, 7 Jul 2023 at 13:00, Costas Argyris <costas.argyris@gmail.com>
> wrote:
>
>> Bootstrapped successfully on x86_64-pc-linux-gnu
>>
>> On Fri, 7 Jul 2023 at 11:33, Costas Argyris <costas.argyris@gmail.com>
>> wrote:
>>
>>> Problem: gcc-ar fails when a @file is passed to it:
>>>
>>> $ cat rsp
>>> --version
>>> $ gcc-ar @rsp
>>> /usr/bin/ar: invalid option -- '@'
>>>
>>> This is because a dash '-' is prepended to the first
>>> argument if it doesn't start with one, resulting in
>>> the wrong call 'ar -@rsp'.
>>>
>>> Fix: Expand argv to get rid of any @files and if any
>>> expansions were made, pass everything through a
>>> temporary response file.
>>>
>>> $ gcc-ar @rsp
>>> GNU ar (GNU Binutils for Debian) 2.35.2
>>> ...
>>>
>>>
>>> PR gcc-ar/77576
>>> * gcc/gcc-ar.cc (main): Expand argv and use
>>> temporary response file to call ar if any
>>> expansions were made.
>>> ---
>>>  gcc/gcc-ar.cc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 47 insertions(+)
>>>
>>> diff --git a/gcc/gcc-ar.cc b/gcc/gcc-ar.cc
>>> index 4e4c525927d..417c4913793 100644
>>> --- a/gcc/gcc-ar.cc
>>> +++ b/gcc/gcc-ar.cc
>>> @@ -135,6 +135,10 @@ main (int ac, char **av)
>>>    int k, status, err;
>>>    const char *err_msg;
>>>    const char **nargv;
>>> +  char **old_argv;
>>> +  const char *rsp_file = NULL;
>>> +  const char *rsp_arg = NULL;
>>> +  const char *rsp_argv[3];
>>>    bool is_ar = !strcmp (PERSONALITY, "ar");
>>>    int exit_code = FATAL_EXIT_CODE;
>>>    int i;
>>> @@ -209,6 +213,13 @@ main (int ac, char **av)
>>>   }
>>>      }
>>>
>>> +  /* Expand any @files before modifying the command line
>>> +     and use a temporary response file if there were any.  */
>>> +  old_argv = av;
>>> +  expandargv (&ac, &av);
>>> +  if (av != old_argv)
>>> +    rsp_file = make_temp_file ("");
>>> +
>>>    /* Prepend - if necessary.  */
>>>    if (is_ar && av[1] && av[1][0] != '-')
>>>      av[1] = concat ("-", av[1], NULL);
>>> @@ -225,6 +236,39 @@ main (int ac, char **av)
>>>      nargv[j + k] = av[k];
>>>    nargv[j + k] = NULL;
>>>
>>> +  /* If @file was passed, put nargv into the temporary response
>>> +     file and then change it to a single @FILE argument, where
>>> +     FILE is the temporary filename.  */
>>> +  if (rsp_file)
>>> +    {
>>> +      FILE *f;
>>> +      int status;
>>> +      f = fopen (rsp_file, "w");
>>> +      if (f == NULL)
>>> +        {
>>> +          fprintf (stderr, "Cannot open temporary file %s\n", rsp_file);
>>> +          exit (1);
>>> +        }
>>> +      status = writeargv (
>>> +          CONST_CAST2 (char * const *, const char **, nargv) + 1, f);
>>> +      if (status)
>>> +        {
>>> +          fprintf (stderr, "Cannot write to temporary file %s\n",
>>> rsp_file);
>>> +          exit (1);
>>> +        }
>>> +      status = fclose (f);
>>> +      if (EOF == status)
>>> +        {
>>> +          fprintf (stderr, "Cannot close temporary file %s\n",
>>> rsp_file);
>>> +          exit (1);
>>> +        }
>>> +      rsp_arg = concat ("@", rsp_file, NULL);
>>> +      rsp_argv[0] = nargv[0];
>>> +      rsp_argv[1] = rsp_arg;
>>> +      rsp_argv[2] = NULL;
>>> +      nargv = rsp_argv;
>>> +    }
>>> +
>>>    /* Run utility */
>>>    /* ??? the const is misplaced in pex_one's argv? */
>>>    err_msg = pex_one (PEX_LAST|PEX_SEARCH,
>>> @@ -249,5 +293,8 @@ main (int ac, char **av)
>>>    else
>>>      exit_code = SUCCESS_EXIT_CODE;
>>>
>>> +  if (rsp_file)
>>> +    unlink (rsp_file);
>>> +
>>>    return exit_code;
>>>  }
>>> --
>>> 2.30.2
>>>
>>

Re: [PATCH] gcc-ar: Handle response files properly [PR77576]

[Joseph Myers]

This patch is OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] gcc-ar: Handle response files properly [PR77576]

[Jeff Law]

On 7/28/23 15:11, Joseph Myers wrote:
> This patch is OK.
I fixed the whitespace errors in the patch as well as a couple minor 
ChangeLog entry items and pushed Costas's patch to the trunk.
jeff