Re: [PATCH] libiberty: On Windows pass a >32k cmdline through a response file.

2023-06-05 Thread Costas Argyris via Gcc-patches
Thanks, here is the follow up patch for a couple typos in the same file.

On Mon, 5 Jun 2023 at 09:12, Jonathan Yong <10wa...@gmail.com> wrote:

> On 5/23/23 08:21, Jonathan Yong wrote:
> > On 5/22/23 13:25, Costas Argyris wrote:
> >> Currently on Windows, when CreateProcess is called with a command-line
> >> that exceeds the 32k Windows limit, we get a very bad error:
> >>
> >> "CreateProcess: No such file or directory"
> >>
> >> This patch detects the case where this would happen and writes the
> >> long command-line to a temporary response file and calls CreateProcess
> >> with @file instead.
> >>
> >
> > Looks OK to me.
> >
> > I will commit it around next week if there are no objections.
> >
>
> Done, pushed to master, thanks.
>
>
From 45c18cf113585aa0b03512a459e757c7aaef69ce Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Mon, 5 Jun 2023 10:03:11 +0100
Subject: [PATCH] libiberty: pex-win32.c: Fix some typos.

Signed-off-by: Costas Argyris 
---
 libiberty/pex-win32.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libiberty/pex-win32.c b/libiberty/pex-win32.c
index 0fd8b38734c..f7fe306036b 100644
--- a/libiberty/pex-win32.c
+++ b/libiberty/pex-win32.c
@@ -351,7 +351,7 @@ argv_to_cmdline (char *const *argv)
 	 prevent wasting 2 chars per argument of the CreateProcess 32k char
 	 limit.  We need only escape embedded double-quotes and immediately
 	 preceeding backslash characters.  A sequence of backslach characters
-	 that is not follwed by a double quote character will not be
+	 that is not followed by a double quote character will not be
 	 escaped.  */
   needs_quotes = 0;
   for (j = 0; argv[i][j]; j++)
@@ -366,7 +366,7 @@ argv_to_cmdline (char *const *argv)
 	  /* Escape preceeding backslashes.  */
 	  for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
 		cmdline_len++;
-	  /* Escape the qote character.  */
+	  /* Escape the quote character.  */
 	  cmdline_len++;
 	}
 	}
-- 
2.30.2



[PATCH] libiberty: writeargv: Simplify function error mode.

2023-06-05 Thread Costas Argyris via Gcc-patches
writeargv can be simplified by getting rid of the error exit mode
that was only relevant many years ago when the function used
to open the file descriptor internally.
From 1271552baee5561fa61652f4ca7673c9667e4f8f Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Mon, 5 Jun 2023 15:02:06 +0100
Subject: [PATCH] libiberty: writeargv: Simplify function error mode.

The goto-based error mode was based on a previous version
of the function where it was responsible for opening the
file, so it had to close it upon any exit:

https://inbox.sourceware.org/gcc-patches/20070417200340.gm9...@sparrowhawk.codesourcery.com/

(thanks pinskia)

This is no longer the case though since now the function
takes the file descriptor as input, so the exit mode on
error can be just a simple return 1 statement.

Signed-off-by: Costas Argyris 
---
 libiberty/argv.c | 29 +
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/libiberty/argv.c b/libiberty/argv.c
index a95a10e14ff..1a18b4d8866 100644
--- a/libiberty/argv.c
+++ b/libiberty/argv.c
@@ -289,8 +289,8 @@ char **buildargv (const char *input)
 @deftypefn Extension int writeargv (char * const *@var{argv}, FILE *@var{file})
 
 Write each member of ARGV, handling all necessary quoting, to the file
-named by FILE, separated by whitespace.  Return 0 on success, non-zero
-if an error occurred while writing to FILE.
+associated with FILE, separated by whitespace.  Return 0 on success,
+non-zero if an error occurred while writing to FILE.
 
 @end deftypefn
 
@@ -314,36 +314,25 @@ writeargv (char * const *argv, FILE *f)
 
   if (ISSPACE(c) || c == '\\' || c == '\'' || c == '"')
 if (EOF == fputc ('\\', f))
-  {
-status = 1;
-goto done;
-  }
+  return 1;
 
   if (EOF == fputc (c, f))
-{
-  status = 1;
-  goto done;
-}
+return 1;
+	  
   arg++;
 }
 
   /* Write out a pair of quotes for an empty argument.  */
   if (arg == *argv)
-	if (EOF == fputs ("\"\"", f))
-	  {
-	status = 1;
-	goto done;
-	  }
+if (EOF == fputs ("\"\"", f))
+  return 1;
 
   if (EOF == fputc ('\n', f))
-{
-  status = 1;
-  goto done;
-}
+return 1;
+  
   argv++;
 }
 
- done:
   return status;
 }
 
-- 
2.30.2



Re: [PATCH] libiberty: writeargv: Simplify function error mode.

2023-06-06 Thread Costas Argyris via Gcc-patches
You are right, this is also a remnant of the old function design
that I completely missed.Here is the follow-up patch for that.

Thanks for pointing it out.

Costas

On Tue, 6 Jun 2023 at 04:12, Jeff Law  wrote:

>
>
> On 6/5/23 08:37, Costas Argyris via Gcc-patches wrote:
> > writeargv can be simplified by getting rid of the error exit mode
> > that was only relevant many years ago when the function used
> > to open the file descriptor internally.
> [ ... ]
> Thanks.  I've pushed this to the trunk.
>
> You could (as a follow-up) simplify it even further.  There's no need
> for the status variable as far as I can tell.  You could just have the
> final return be "return 0;" instead of "return status;".
>
> Jeff
>
From 13fdfea60eeac64e028315392614b955e998487d Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Tue, 6 Jun 2023 09:15:48 +0100
Subject: [PATCH] libiberty: writeargv: Remove unnecessary status variable.

Follow-up from 4d1e4ce986f pointed out by jlaw.

Signed-off-by: Costas Argyris 
---
 libiberty/argv.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libiberty/argv.c b/libiberty/argv.c
index 1a18b4d8866..c2823d3e4ba 100644
--- a/libiberty/argv.c
+++ b/libiberty/argv.c
@@ -299,8 +299,6 @@ non-zero if an error occurred while writing to FILE.
 int
 writeargv (char * const *argv, FILE *f)
 {
-  int status = 0;
-
   if (f == NULL)
 return 1;
 
@@ -333,7 +331,7 @@ writeargv (char * const *argv, FILE *f)
   argv++;
 }
 
-  return status;
+  return 0;
 }
 
 /*
-- 
2.30.2



[PATCH] libiberty: pex-unix.c: Make pex_unix_cleanup signature always match body.

2023-06-07 Thread Costas Argyris via Gcc-patches
I saw this while working on something else:

pex_unix_cleanup signature doesn't always match the
body of the function in terms of ATTRIBUTE_UNUSED.
If the conditional code in the body is compiled, then
ATTRIBUTE_UNUSED isn't correct.

This change makes it always match, thereby making it
a bit cleaner IMO.

Costas
From 4c84afd631ad09011b237790599e1c320852f82d Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Wed, 7 Jun 2023 10:34:14 +0100
Subject: [PATCH] libiberty: pex-unix.c: Make pex_unix_cleanup signature always
 match body.

Signed-off-by: Costas Argyris 
---
 libiberty/pex-unix.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libiberty/pex-unix.c b/libiberty/pex-unix.c
index 33b5bce31c2..10f8ddd2feb 100644
--- a/libiberty/pex-unix.c
+++ b/libiberty/pex-unix.c
@@ -814,9 +814,9 @@ pex_unix_fdopenw (struct pex_obj *obj ATTRIBUTE_UNUSED, int fd,
 }
 
 static void
-pex_unix_cleanup (struct pex_obj *obj ATTRIBUTE_UNUSED)
-{
 #if !defined (HAVE_WAIT4) && !defined (HAVE_WAITPID)
+pex_unix_cleanup (struct pex_obj *obj)
+{
   while (obj->sysdep != NULL)
 {
   struct status_list *this;
@@ -827,5 +827,9 @@ pex_unix_cleanup (struct pex_obj *obj ATTRIBUTE_UNUSED)
   free (this);
   obj->sysdep = (void *) next;
 }
-#endif
 }
+#else
+pex_unix_cleanup (struct pex_obj *obj ATTRIBUTE_UNUSED)
+{
+}
+#endif
-- 
2.30.2



Re: [PATCH] libiberty: pex-unix.c: Make pex_unix_cleanup signature always match body.

2023-06-07 Thread Costas Argyris via Gcc-patches
Oh OK, thanks for the clarification.

Costas

On Wed, 7 Jun 2023 at 13:59, Jeff Law  wrote:

>
>
> On 6/7/23 04:21, Costas Argyris via Gcc-patches wrote:
> > I saw this while working on something else:
> >
> > pex_unix_cleanup signature doesn't always match the
> > body of the function in terms of ATTRIBUTE_UNUSED.
> > If the conditional code in the body is compiled, then
> > ATTRIBUTE_UNUSED isn't correct.
> >
> > This change makes it always match, thereby making it
> > a bit cleaner IMO.
> ATTRIBUTE_UNUSED is meant to be a "maybe unused" decoration.   I'd just
> leave it as-is.
>
> jeff
>


[PATCH] gcc-ar: Remove code duplication.

2023-06-15 Thread Costas Argyris via Gcc-patches
Some refactoring I thought would be useful while looking at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77576

I think some duplicated code can go away by doing this,
while also saving a bit of memory.
From c3f3b2fd53291805b5d0be19df6d1a348c5889ec Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Thu, 15 Jun 2023 12:37:35 +0100
Subject: [PATCH] gcc-ar: Remove code duplication.

Preparatory refactoring that simplifies by eliminating
some duplicated code, before trying to fix 77576.
I believe this stands on its own regardless of the PR.
It also saves a nargv element when we have a plugin and
three when not.

Signed-off-by: Costas Argyris 
---
 gcc/gcc-ar.cc | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/gcc/gcc-ar.cc b/gcc/gcc-ar.cc
index 5e5b63e1988..4e4c525927d 100644
--- a/gcc/gcc-ar.cc
+++ b/gcc/gcc-ar.cc
@@ -128,6 +128,9 @@ main (int ac, char **av)
   const char *exe_name;
 #if HAVE_LTO_PLUGIN > 0
   char *plugin;
+  const int j = 2; /* Two extra args, --plugin   */
+#else
+  const int j = 0; /* No extra args.  */
 #endif
   int k, status, err;
   const char *err_msg;
@@ -206,25 +209,21 @@ main (int ac, char **av)
 	}
 }
 
+  /* Prepend - if necessary.  */
+  if (is_ar && av[1] && av[1][0] != '-')
+av[1] = concat ("-", av[1], NULL);
+  
   /* Create new command line with plugin - if we have one, otherwise just
  copy the command through.  */
-  nargv = XCNEWVEC (const char *, ac + 4);
+  nargv = XCNEWVEC (const char *, ac + j + 1); /* +j plugin args +1 for NULL.  */
   nargv[0] = exe_name;
 #if HAVE_LTO_PLUGIN > 0
   nargv[1] = "--plugin";
   nargv[2] = plugin;
-  if (is_ar && av[1] && av[1][0] != '-')
-av[1] = concat ("-", av[1], NULL);
-  for (k = 1; k < ac; k++)
-nargv[2 + k] = av[k];
-  nargv[2 + k] = NULL;
-#else
-  if (is_ar && av[1] && av[1][0] != '-')
-av[1] = concat ("-", av[1], NULL);
-  for (k = 1; k < ac; k++)
-nargv[k] = av[k];
-  nargv[k] = NULL;
 #endif
+  for (k = 1; k < ac; k++)
+nargv[j + k] = av[k];
+  nargv[j + k] = NULL;
 
   /* Run utility */
   /* ??? the const is misplaced in pex_one's argv? */
-- 
2.30.2



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

2023-07-14 Thread Costas Argyris via Gcc-patches
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 
wrote:

> Bootstrapped successfully on x86_64-pc-linux-gnu
>
> On Fri, 7 Jul 2023 at 11:33, Costas Argyris 
> 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]

2023-07-28 Thread Costas Argyris via Gcc-patches
ping

On Fri, 14 Jul 2023 at 09:05, Costas Argyris 
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 
> wrote:
>
>> Bootstrapped successfully on x86_64-pc-linux-gnu
>>
>> On Fri, 7 Jul 2023 at 11:33, Costas Argyris 
>> 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
>>>
>>


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

2023-07-01 Thread Costas Argyris via Gcc-patches
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.


0001-gcc-ar-Handle-response-files-properly-PR77576.patch
Description: Binary data


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

2023-07-03 Thread Costas Argyris via Gcc-patches
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 
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]

2023-07-07 Thread Costas Argyris via Gcc-patches
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]

2023-07-07 Thread Costas Argyris via Gcc-patches
Bootstrapped successfully on x86_64-pc-linux-gnu

On Fri, 7 Jul 2023 at 11:33, Costas Argyris 
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
>


[PATCH] libiberty: fix memory leak in pex-win32.c and refactor

2023-03-01 Thread Costas Argyris via Gcc-patches
Hi

It seems that the win32_spawn function in libiberty/pex-win32.c is leaking
the cmdline buffer in 2/3 exit scenarios (it is only free'd in 1/3).The
problem here is that the cleanup code is written 3 times, one at each exit
scenario.

The proposed attached refactoring has the cleanup code appearing just once
and is executed for all exit scenarios, reducing the likelihood of such
leaks in the future.

Thanks,
Costas
From ca1ff7b48db2e30963bd4c0e08ecd6653214a5db Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Sun, 26 Feb 2023 16:34:11 +
Subject: [PATCH] libiberty: fix memory leak in pex-win32.c and refactor

Fix memory leak of cmdline buffer and refactor to have
cleanup code appear once for all exit cases.
---
 libiberty/pex-win32.c | 33 +++--
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/libiberty/pex-win32.c b/libiberty/pex-win32.c
index 02d3a3e839b..82303947de4 100644
--- a/libiberty/pex-win32.c
+++ b/libiberty/pex-win32.c
@@ -577,14 +577,12 @@ win32_spawn (const char *executable,
 	 LPSTARTUPINFO si,
 	 LPPROCESS_INFORMATION pi)
 {
-  char *full_executable;
-  char *cmdline;
+  char *full_executable = NULL;
+  char *cmdline = NULL;
+  pid_t pid = (pid_t) -1;
   char **env_copy;
   char *env_block = NULL;
 
-  full_executable = NULL;
-  cmdline = NULL;
-
   if (env)
 {
   int env_size;
@@ -622,13 +620,13 @@ win32_spawn (const char *executable,
 
   full_executable = find_executable (executable, search);
   if (!full_executable)
-goto error;
+goto exit;
   cmdline = argv_to_cmdline (argv);
   if (!cmdline)
-goto error;
+goto exit;
 
   /* Create the child process.  */  
-  if (!CreateProcess (full_executable, cmdline, 
+  if (CreateProcess (full_executable, cmdline, 
 		  /*lpProcessAttributes=*/NULL,
 		  /*lpThreadAttributes=*/NULL,
 		  /*bInheritHandles=*/TRUE,
@@ -638,26 +636,17 @@ win32_spawn (const char *executable,
 		  si,
 		  pi))
 {
-  free (env_block);
-
-  free (full_executable);
-
-  return (pid_t) -1;
+  CloseHandle (pi->hThread);
+  pid = (pid_t) pi->hProcess;
 }
-
+  
+ exit:
   /* Clean up.  */
-  CloseHandle (pi->hThread);
-  free (full_executable);
-  free (env_block);
-
-  return (pid_t) pi->hProcess;
-
- error:
   free (env_block);
   free (cmdline);
   free (full_executable);
 
-  return (pid_t) -1;
+  return pid;
 }
 
 /* Spawn a script.  This simulates the Unix script execution mechanism.
-- 
2.30.2



Re: [PATCH] libiberty: fix memory leak in pex-win32.c and refactor

2023-03-02 Thread Costas Argyris via Gcc-patches
I forgot to mention that:

1) The CreateProcess documentation

https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa

doesn't mention anything about taking ownership of this or any other buffer
passed to it.

2) The cmdline buffer gets created by the argv_to_cmdline function

https://github.com/gcc-mirror/gcc/blob/master/libiberty/pex-win32.c#L339

which has this comment right above it:

/* Return a Windows command-line from ARGV.  It is the caller's
   responsibility to free the string returned.  */

Thanks,
Costas

On Thu, 2 Mar 2023 at 07:32, Richard Biener 
wrote:

> On Wed, Mar 1, 2023 at 7:14 PM Costas Argyris via Gcc-patches
>  wrote:
> >
> > Hi
> >
> > It seems that the win32_spawn function in libiberty/pex-win32.c is
> leaking
> > the cmdline buffer in 2/3 exit scenarios (it is only free'd in 1/3).
> The
> > problem here is that the cleanup code is written 3 times, one at each
> exit
> > scenario.
> >
> > The proposed attached refactoring has the cleanup code appearing just
> once
> > and is executed for all exit scenarios, reducing the likelihood of such
> > leaks in the future.
>
> One could imagine that CreateProcess in case of success takes ownership of
> the buffer pointed to by cmdline?  If you can confirm it is not then the
> patch
> looks OK to me.
>
> Thanks,
> Richard.
>
> > Thanks,
> > Costas
>


Re: [PATCH] libiberty: fix memory leak in pex-win32.c and refactor

2023-03-02 Thread Costas Argyris via Gcc-patches
Thanks for the review.

What is the next step please?

Thanks,
Costas

On Thu, 2 Mar 2023 at 10:08, Richard Biener 
wrote:

> On Thu, Mar 2, 2023 at 10:21 AM Costas Argyris 
> wrote:
> >
> > I forgot to mention that:
> >
> > 1) The CreateProcess documentation
> >
> >
> https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa
> >
> > doesn't mention anything about taking ownership of this or any other
> buffer passed to it.
>
> Thanks - thus the patch is OK.
>
> Thanks,
> Richard.
>
> > 2) The cmdline buffer gets created by the argv_to_cmdline function
> >
> > https://github.com/gcc-mirror/gcc/blob/master/libiberty/pex-win32.c#L339
> >
> > which has this comment right above it:
> >
> > /* Return a Windows command-line from ARGV.  It is the caller's
> >responsibility to free the string returned.  */
> >
> > Thanks,
> > Costas
> >
> > On Thu, 2 Mar 2023 at 07:32, Richard Biener 
> wrote:
> >>
> >> On Wed, Mar 1, 2023 at 7:14 PM Costas Argyris via Gcc-patches
> >>  wrote:
> >> >
> >> > Hi
> >> >
> >> > It seems that the win32_spawn function in libiberty/pex-win32.c is
> leaking
> >> > the cmdline buffer in 2/3 exit scenarios (it is only free'd in 1/3).
>   The
> >> > problem here is that the cleanup code is written 3 times, one at each
> exit
> >> > scenario.
> >> >
> >> > The proposed attached refactoring has the cleanup code appearing just
> once
> >> > and is executed for all exit scenarios, reducing the likelihood of
> such
> >> > leaks in the future.
> >>
> >> One could imagine that CreateProcess in case of success takes ownership
> of
> >> the buffer pointed to by cmdline?  If you can confirm it is not then
> the patch
> >> looks OK to me.
> >>
> >> Thanks,
> >> Richard.
> >>
> >> > Thanks,
> >> > Costas
>


[PATCH] driver: Treat include path args the same way between cpp_unique_options and asm_options. [PR71850]

2023-03-02 Thread Costas Argyris via Gcc-patches
This is a proposal to fix PR71850 by applying the existing logic for
passing include paths to cc1 to as.

Thanks,
Costas
From 393aff0d006ee9372cc8b9321c612c2dfb4b0a31 Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Thu, 2 Mar 2023 18:27:22 +
Subject: [PATCH] driver: Treat include path args the same way between
 cpp_unique_options and asm_options. [PR71850]

On Windows, when a @file with many include paths is passed to gcc, it forwards those include paths to cc1 through a temporary @file as well, so they don't end up in the command line.This is because cpp_unique_options has %@{I* which passes -I args in a temporary file, if a temporary file was passed to the driver in the first place.

The same logic is not applied in asm_options, and this leads to the include paths being passed as command line arguments to the assembler, which causes the failure on Windows seen in PR71850.

Treating the -I args to the assembler the same way as to the compiler (that is, through a @tempfile if @file was passed to gcc) solves the issue, allowing a large number of include paths to be passed to gcc on Windows through a @file.
---
 gcc/gcc.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index becc56051a8..b1fa80cde4f 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -1278,7 +1278,7 @@ static const char *asm_options =
 #if HAVE_GNU_AS
 /* If GNU AS is used, then convert -w (no warnings), -I, and -v
to the assembler equivalents.  */
-"%{v} %{w:-W} %{I*} "
+"%{v} %{w:-W} %@{I*} "
 #endif
 "%(asm_debug_option)"
 ASM_COMPRESS_DEBUG_SPEC
-- 
2.30.2



Enable UTF-8 code page in driver and compiler on 64-bit mingw host [PR108865]

2023-03-06 Thread Costas Argyris via Gcc-patches
Hi

This is a proposal for addressing

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

by integrating the UTF-8 manifest file into gcc's build process for the
64-bit mingw host.

The analysis and discussion leading up to the latest patch are written in
the bug report.

The patch attached in this email is exactly the same as the one posted last
in the bug report.

I should also mention that, in case of approval, I would need someone with
write access to make the commit for me please.

Thanks,
Costas


0001-Enable-UTF-8-code-page-on-Windows-64-bit-host-PR1088.patch
Description: Binary data


Re: Enable UTF-8 code page in driver and compiler on 64-bit mingw host [PR108865]

2023-03-07 Thread Costas Argyris via Gcc-patches
Hi Jacek,

"Is there a reason to make it specific to x86_64? It seems to me that all
mingw hosts could use it."

Are you referring to the 32-bit host?My concern here is that this
functionality (embedding the UTF-8
manifest file into the executable) is only truly supported in recent
versions of Windows.From:

https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page

It says that Windows Version 1903 (May 2019 Update) enables this, so we are
looking at the 64-bit
version of Windows.

I suppose you are referring to the scenario where one has a 32-bit gcc +
mingw running in a 64-bit
Windows that is recent enough to support this?It is not clear to me
based on the above doc what
would happen encoding-wise in that situation, and I haven't tried it either
because I assumed that
most people would want the 64-bit version of gcc since they are probably
running a 64-bit OS.

If you think it is useful, I could look into that as a separate task to try
and keep this one simple, if
that makes sense.

"I think that .manifest file should also be a dependency here."

Why is that?Windres takes only the .rc file as its input, as per its
own doc, and it successfully
compiles it into an object file.The .manifest file is only referenced
by the .rc file, and it doesn't
get passed to windres, so I don't see why it has to be listed as a
prerequisite in the make rule.

Thanks,
Costas

On Tue, 7 Mar 2023 at 12:02, Jacek Caban  wrote:

> Hi Costas,
>
> On 3/7/23 01:52, Costas Argyris via Gcc-patches wrote:
>
> This is a proposal for addressing
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108865
>
> by integrating the UTF-8 manifest file into gcc's build process for the
> 64-bit mingw host.
>
>
> Is there a reason to make it specific to x86_64? It seems to me that all
> mingw hosts could use it.
>
>
>
> +# The resource .rc file references the utf8 .manifest file.
> +# Compile it into an object file using windres.
> +# The resulting .o file gets added to host_extra_gcc_objs in
> +# config.host for x86_64-*-mingw* host and gets linked into
> +# the driver as a .o file, so it's lack of symbols is OK.
> +utf8rc-mingw32.o : $(srcdir)/config/i386/utf8-mingw32.rc
> + $(WINDRES) $< $@
>
>
> I think that .manifest file should also be a dependency here.
>
>
> Thanks,
>
> Jacek
>


Re: Enable UTF-8 code page in driver and compiler on 64-bit mingw host [PR108865]

2023-03-07 Thread Costas Argyris via Gcc-patches
Hi Jacek,

"but I think it should work just fine if you didn't explicitly limit the
patch to x86_64."

I would think so too.

Actually, even cygwin might benefit from this, assuming it has the same
problem, which I don't know if it's the case.

But I'm not experienced with that so I would like to explore these hosts
separately and just focus on the most common 64-bit Windows host with this
change, if possible.

"The point that when winnt-utf8.manifest is modified, utf8-mingw32.o should
be rebuilt."

Right, makes sense.

Just noting that winnt-utf8.manifest is really not meant to be modified,
because it is copied straight from:

https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page

and will probably remain like that, but I do get your point and I am happy
to make the change.

Thanks,
Costas

On Tue, 7 Mar 2023 at 14:18, Jacek Caban  wrote:

> Hi Costas,
>
> On 3/7/23 15:00, Costas Argyris wrote:
> > Hi Jacek,
> >
> > "Is there a reason to make it specific to x86_64? It seems to me that
> > all mingw hosts could use it."
> >
> > Are you referring to the 32-bit host?My concern here is that this
> > functionality (embedding the UTF-8
> > manifest file into the executable) is only truly supported in recent
> > versions of Windows.From:
> >
> >
> https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page
> >
> > It says that Windows Version 1903 (May 2019 Update) enables this, so
> > we are looking at the 64-bit
> > version of Windows.
> >
> > I suppose you are referring to the scenario where one has a 32-bit
> > gcc + mingw running in a 64-bit
> > Windows that is recent enough to support this?It is not clear to
> > me based on the above doc what
> > would happen encoding-wise in that situation, and I haven't tried it
> > either because I assumed that
> > most people would want the 64-bit version of gcc since they are
> > probably running a 64-bit OS.
> >
> > If you think it is useful, I could look into that as a separate task
> > to try and keep this one simple, if
> > that makes sense.
>
>
> Yes, realistically it's mostly about 32-bit gcc on 64-bit Windows
> (perhaps aarch64 as well at some point in the future). It's probably
> indeed not very popular configuration those days, but I think it should
> work just fine if you didn't explicitly limit the patch to x86_64.
>
>
> > "I think that .manifest file should also be a dependency here."
> >
> > Why is that?Windres takes only the .rc file as its input, as per
> > its own doc, and it successfully
> > compiles it into an object file.The .manifest file is only
> > referenced by the .rc file, and it doesn't
> > get passed to windres, so I don't see why it has to be listed as a
> > prerequisite in the make rule.
>
>
> The point that when winnt-utf8.manifest is modified, utf8-mingw32.o
> should be rebuilt. Anyway, it's probably not a big deal (I should
> disclaim that I'm not very familiar with gcc build system; I'm mostly on
> this ML due to mingw-w64 contributions).
>
>
> Thanks,
>
> Jacek
>
>


Re: Enable UTF-8 code page in driver and compiler on 64-bit mingw host [PR108865]

2023-03-08 Thread Costas Argyris via Gcc-patches
Added .manifest file to the make rule for utf8rc-mingw32.o, latest patch
attached.

On Tue, 7 Mar 2023 at 15:27, Costas Argyris 
wrote:

> Hi Jacek,
>
> "but I think it should work just fine if you didn't explicitly limit the
> patch to x86_64."
>
> I would think so too.
>
> Actually, even cygwin might benefit from this, assuming it has the same
> problem, which I don't know if it's the case.
>
> But I'm not experienced with that so I would like to explore these hosts
> separately and just focus on the most common 64-bit Windows host with this
> change, if possible.
>
> "The point that when winnt-utf8.manifest is modified, utf8-mingw32.o
> should be rebuilt."
>
> Right, makes sense.
>
> Just noting that winnt-utf8.manifest is really not meant to be modified,
> because it is copied straight from:
>
>
> https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page
>
> and will probably remain like that, but I do get your point and I am happy
> to make the change.
>
> Thanks,
> Costas
>
> On Tue, 7 Mar 2023 at 14:18, Jacek Caban  wrote:
>
>> Hi Costas,
>>
>> On 3/7/23 15:00, Costas Argyris wrote:
>> > Hi Jacek,
>> >
>> > "Is there a reason to make it specific to x86_64? It seems to me that
>> > all mingw hosts could use it."
>> >
>> > Are you referring to the 32-bit host?My concern here is that this
>> > functionality (embedding the UTF-8
>> > manifest file into the executable) is only truly supported in recent
>> > versions of Windows.From:
>> >
>> >
>> https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page
>> >
>> > It says that Windows Version 1903 (May 2019 Update) enables this, so
>> > we are looking at the 64-bit
>> > version of Windows.
>> >
>> > I suppose you are referring to the scenario where one has a 32-bit
>> > gcc + mingw running in a 64-bit
>> > Windows that is recent enough to support this?It is not clear to
>> > me based on the above doc what
>> > would happen encoding-wise in that situation, and I haven't tried it
>> > either because I assumed that
>> > most people would want the 64-bit version of gcc since they are
>> > probably running a 64-bit OS.
>> >
>> > If you think it is useful, I could look into that as a separate task
>> > to try and keep this one simple, if
>> > that makes sense.
>>
>>
>> Yes, realistically it's mostly about 32-bit gcc on 64-bit Windows
>> (perhaps aarch64 as well at some point in the future). It's probably
>> indeed not very popular configuration those days, but I think it should
>> work just fine if you didn't explicitly limit the patch to x86_64.
>>
>>
>> > "I think that .manifest file should also be a dependency here."
>> >
>> > Why is that?Windres takes only the .rc file as its input, as per
>> > its own doc, and it successfully
>> > compiles it into an object file.The .manifest file is only
>> > referenced by the .rc file, and it doesn't
>> > get passed to windres, so I don't see why it has to be listed as a
>> > prerequisite in the make rule.
>>
>>
>> The point that when winnt-utf8.manifest is modified, utf8-mingw32.o
>> should be rebuilt. Anyway, it's probably not a big deal (I should
>> disclaim that I'm not very familiar with gcc build system; I'm mostly on
>> this ML due to mingw-w64 contributions).
>>
>>
>> Thanks,
>>
>> Jacek
>>
>>
From 694d6f4860a08f690070df411f3f72d66a48a981 Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Tue, 28 Feb 2023 17:10:18 +
Subject: [PATCH] Enable UTF-8 code page on Windows 64-bit host [PR108865]

Compile a resource object that contains the utf8 manifest.

Then link that object into the driver and compiler proper.

For compiler proper the link has to be forced because the
resource object file gets into a static library (libbackend.a)
and gets eventually dropped because it has no symbols of
its own and nothing is referencing it inside the library.

Therefore, an artificial symbol is planted to force the link.
---
 gcc/config.host |  5 ++-
 gcc/config/i386/sym-mingw32.cc  |  1 +
 gcc/config/i386/utf8-mingw32.rc |  3 ++
 gcc/config/i386/winnt-utf8.manifest |  8 
 gcc/config/i386/x-mingw32   |  3 +-
 gcc/config/i386/x-mingw32-utf8  | 57 +
 6 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 gcc/config/i386/sym-mingw32.cc
 create mode 100644 gcc/config/i386/utf8-mingw32.rc
 create mode 100644 gcc/config/i386/winnt-utf8.manifest
 create mode 100644 gcc/config/i386/x-mingw32-utf8

diff --git a/gcc/config.host b/gcc/config.host
index a522c39658e..4abb32ad73d 100644
--- a/gcc/config.host
+++ b/gcc/config.host
@@ -241,10 +241,11 @@ case ${host} in
   x86_64-*-mingw*)
 use_long_long_for_widest_fast_int=yes
 host_xm_file=i386/xm-mingw32.h
-host_xmake_file="${host_xmake_file} i386/x-mingw32"
+host_xmake_file="${host_xmake_file} i386/x-mingw32 i386/x-mingw32-utf8"
 host_exeext=.exe
 out_host_hook_obj=host-mingw32.o
-host_extra_gcc_objs="${ho

Re: Enable UTF-8 code page in driver and compiler on 64-bit mingw host [PR108865]

2023-03-09 Thread Costas Argyris via Gcc-patches
Pinging the list and mingw maintainer.

Analysis and pre-approval here:

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

On Wed, 8 Mar 2023 at 10:52, Costas Argyris 
wrote:

> Added .manifest file to the make rule for utf8rc-mingw32.o, latest patch
> attached.
>
> On Tue, 7 Mar 2023 at 15:27, Costas Argyris 
> wrote:
>
>> Hi Jacek,
>>
>> "but I think it should work just fine if you didn't explicitly limit the
>> patch to x86_64."
>>
>> I would think so too.
>>
>> Actually, even cygwin might benefit from this, assuming it has the same
>> problem, which I don't know if it's the case.
>>
>> But I'm not experienced with that so I would like to explore these hosts
>> separately and just focus on the most common 64-bit Windows host with this
>> change, if possible.
>>
>> "The point that when winnt-utf8.manifest is modified, utf8-mingw32.o
>> should be rebuilt."
>>
>> Right, makes sense.
>>
>> Just noting that winnt-utf8.manifest is really not meant to be modified,
>> because it is copied straight from:
>>
>>
>> https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page
>>
>> and will probably remain like that, but I do get your point and I am
>> happy to make the change.
>>
>> Thanks,
>> Costas
>>
>> On Tue, 7 Mar 2023 at 14:18, Jacek Caban  wrote:
>>
>>> Hi Costas,
>>>
>>> On 3/7/23 15:00, Costas Argyris wrote:
>>> > Hi Jacek,
>>> >
>>> > "Is there a reason to make it specific to x86_64? It seems to me that
>>> > all mingw hosts could use it."
>>> >
>>> > Are you referring to the 32-bit host?My concern here is that this
>>> > functionality (embedding the UTF-8
>>> > manifest file into the executable) is only truly supported in recent
>>> > versions of Windows.From:
>>> >
>>> >
>>> https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page
>>> >
>>> > It says that Windows Version 1903 (May 2019 Update) enables this, so
>>> > we are looking at the 64-bit
>>> > version of Windows.
>>> >
>>> > I suppose you are referring to the scenario where one has a 32-bit
>>> > gcc + mingw running in a 64-bit
>>> > Windows that is recent enough to support this?It is not clear to
>>> > me based on the above doc what
>>> > would happen encoding-wise in that situation, and I haven't tried it
>>> > either because I assumed that
>>> > most people would want the 64-bit version of gcc since they are
>>> > probably running a 64-bit OS.
>>> >
>>> > If you think it is useful, I could look into that as a separate task
>>> > to try and keep this one simple, if
>>> > that makes sense.
>>>
>>>
>>> Yes, realistically it's mostly about 32-bit gcc on 64-bit Windows
>>> (perhaps aarch64 as well at some point in the future). It's probably
>>> indeed not very popular configuration those days, but I think it should
>>> work just fine if you didn't explicitly limit the patch to x86_64.
>>>
>>>
>>> > "I think that .manifest file should also be a dependency here."
>>> >
>>> > Why is that?Windres takes only the .rc file as its input, as per
>>> > its own doc, and it successfully
>>> > compiles it into an object file.The .manifest file is only
>>> > referenced by the .rc file, and it doesn't
>>> > get passed to windres, so I don't see why it has to be listed as a
>>> > prerequisite in the make rule.
>>>
>>>
>>> The point that when winnt-utf8.manifest is modified, utf8-mingw32.o
>>> should be rebuilt. Anyway, it's probably not a big deal (I should
>>> disclaim that I'm not very familiar with gcc build system; I'm mostly on
>>> this ML due to mingw-w64 contributions).
>>>
>>>
>>> Thanks,
>>>
>>> Jacek
>>>
>>>
From 694d6f4860a08f690070df411f3f72d66a48a981 Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Tue, 28 Feb 2023 17:10:18 +
Subject: [PATCH] Enable UTF-8 code page on Windows 64-bit host [PR108865]

Compile a resource object that contains the utf8 manifest.

Then link that object into the driver and compiler proper.

For compiler proper the link has to be forced because the
resource object file gets into a static library (libbackend.a)
and gets eventually dropped because it has no symbols of
its own and nothing is referencing it inside the library.

Therefore, an artificial symbol is planted to force the link.
---
 gcc/config.host |  5 ++-
 gcc/config/i386/sym-mingw32.cc  |  1 +
 gcc/config/i386/utf8-mingw32.rc |  3 ++
 gcc/config/i386/winnt-utf8.manifest |  8 
 gcc/config/i386/x-mingw32   |  3 +-
 gcc/config/i386/x-mingw32-utf8  | 57 +
 6 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 gcc/config/i386/sym-mingw32.cc
 create mode 100644 gcc/config/i386/utf8-mingw32.rc
 create mode 100644 gcc/config/i386/winnt-utf8.manifest
 create mode 100644 gcc/config/i386/x-mingw32-utf8

diff --git a/gcc/config.host b/gcc/config.host
index a522c39658e..4abb32ad73d 100644
--- a/gcc/config.host
+++ b/gcc/config.host
@@ -241,10 +241,11 @@ case ${host} in
   x86_64-*-mingw*)
 use_long_long_f

Re: [PATCH] driver: Treat include path args the same way between cpp_unique_options and asm_options. [PR71850]

2023-03-09 Thread Costas Argyris via Gcc-patches
Pinging list and driver reviewer.

Details here:

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

On Thu, 2 Mar 2023 at 19:25, Costas Argyris 
wrote:

> This is a proposal to fix PR71850 by applying the existing logic for
> passing include paths to cc1 to as.
>
> Thanks,
> Costas
>
From 393aff0d006ee9372cc8b9321c612c2dfb4b0a31 Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Thu, 2 Mar 2023 18:27:22 +
Subject: [PATCH] driver: Treat include path args the same way between
 cpp_unique_options and asm_options. [PR71850]

On Windows, when a @file with many include paths is passed to gcc, it forwards those include paths to cc1 through a temporary @file as well, so they don't end up in the command line.This is because cpp_unique_options has %@{I* which passes -I args in a temporary file, if a temporary file was passed to the driver in the first place.

The same logic is not applied in asm_options, and this leads to the include paths being passed as command line arguments to the assembler, which causes the failure on Windows seen in PR71850.

Treating the -I args to the assembler the same way as to the compiler (that is, through a @tempfile if @file was passed to gcc) solves the issue, allowing a large number of include paths to be passed to gcc on Windows through a @file.
---
 gcc/gcc.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index becc56051a8..b1fa80cde4f 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -1278,7 +1278,7 @@ static const char *asm_options =
 #if HAVE_GNU_AS
 /* If GNU AS is used, then convert -w (no warnings), -I, and -v
to the assembler equivalents.  */
-"%{v} %{w:-W} %{I*} "
+"%{v} %{w:-W} %@{I*} "
 #endif
 "%(asm_debug_option)"
 ASM_COMPRESS_DEBUG_SPEC
-- 
2.30.2



[PATCH] libiberty: On Windows pass a >32k cmdline through a response file.

2023-05-22 Thread Costas Argyris via Gcc-patches
Currently on Windows, when CreateProcess is called with a command-line
that exceeds the 32k Windows limit, we get a very bad error:

"CreateProcess: No such file or directory"

This patch detects the case where this would happen and writes the
long command-line to a temporary response file and calls CreateProcess
with @file instead.
From 5c7237c102cdaca34e5907cd25c31610bda51919 Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Mon, 22 May 2023 13:55:56 +0100
Subject: [PATCH] libiberty: On Windows, pass a >32k cmdline through a response
 file.

pex-win32.c (win32_spawn): If the command line for CreateProcess
exceeds the 32k Windows limit, try to store it in a temporary
response file and call CreateProcess with @file instead (PR71850).

Signed-off-by: Costas Argyris 
---
 libiberty/pex-win32.c | 57 +--
 1 file changed, 44 insertions(+), 13 deletions(-)

diff --git a/libiberty/pex-win32.c b/libiberty/pex-win32.c
index 23c6c190a2c..0fd8b38734c 100644
--- a/libiberty/pex-win32.c
+++ b/libiberty/pex-win32.c
@@ -569,7 +569,8 @@ env_compare (const void *a_ptr, const void *b_ptr)
  * target is not actually an executable, such as if it is a shell script. */
 
 static pid_t
-win32_spawn (const char *executable,
+win32_spawn (struct pex_obj *obj,
+ const char *executable,
 	 BOOL search,
 	 char *const *argv,
  char *const *env, /* array of strings of the form: VAR=VALUE */
@@ -624,8 +625,37 @@ win32_spawn (const char *executable,
   cmdline = argv_to_cmdline (argv);
   if (!cmdline)
 goto exit;
-
-  /* Create the child process.  */  
+  /* If cmdline is too large, CreateProcess will fail with a bad
+ 'No such file or directory' error. Try passing it through a
+ temporary response file instead.  */
+  if (strlen (cmdline) > 32767)
+{
+  char *response_file = make_temp_file ("");
+  /* Register the file for deletion by pex_free.  */
+  ++obj->remove_count;
+  obj->remove = XRESIZEVEC (char *, obj->remove, obj->remove_count);
+  obj->remove[obj->remove_count - 1] = response_file;
+  int fd = pex_win32_open_write (obj, response_file, 0, 0);
+  if (fd == -1)
+goto exit;
+  FILE *f = pex_win32_fdopenw (obj, fd, 0);
+  /* Don't write argv[0] (program name) to the response file.  */
+  if (writeargv (&argv[1], f))
+{
+  fclose (f);
+  goto exit;
+}
+  fclose (f); /* Also closes fd and the underlying OS handle.  */
+  char *response_arg = concat ("@", response_file, NULL);
+  char *response_argv[3] = {argv[0], response_arg, NULL};
+  free (cmdline);
+  cmdline = argv_to_cmdline (response_argv);
+  free (response_arg);
+  if (!cmdline)
+goto exit;
+}
+  
+  /* Create the child process.  */
   if (CreateProcess (full_executable, cmdline,
 		  /*lpProcessAttributes=*/NULL,
 		  /*lpThreadAttributes=*/NULL,
@@ -645,7 +675,7 @@ win32_spawn (const char *executable,
   free (env_block);
   free (cmdline);
   free (full_executable);
-
+  
   return pid;
 }
 
@@ -653,7 +683,8 @@ win32_spawn (const char *executable,
This function is called as a fallback if win32_spawn fails. */
 
 static pid_t
-spawn_script (const char *executable, char *const *argv,
+spawn_script (struct pex_obj *obj,
+  const char *executable, char *const *argv,
   char* const *env,
 	  DWORD dwCreationFlags,
 	  LPSTARTUPINFO si,
@@ -703,20 +734,20 @@ spawn_script (const char *executable, char *const *argv,
 	  executable = strrchr (executable1, '\\') + 1;
 	  if (!executable)
 		executable = executable1;
-	  pid = win32_spawn (executable, TRUE, argv, env,
+	  pid = win32_spawn (obj, executable, TRUE, argv, env,
  dwCreationFlags, si, pi);
 #else
 	  if (strchr (executable1, '\\') == NULL)
-		pid = win32_spawn (executable1, TRUE, argv, env,
+		pid = win32_spawn (obj, executable1, TRUE, argv, env,
    dwCreationFlags, si, pi);
 	  else if (executable1[0] != '\\')
-		pid = win32_spawn (executable1, FALSE, argv, env,
+		pid = win32_spawn (obj, executable1, FALSE, argv, env,
    dwCreationFlags, si, pi);
 	  else
 		{
 		  const char *newex = mingw_rootify (executable1);
 		  *avhere = newex;
-		  pid = win32_spawn (newex, FALSE, argv, env,
+		  pid = win32_spawn (obj, newex, FALSE, argv, env,
  dwCreationFlags, si, pi);
 		  if (executable1 != newex)
 		free ((char *) newex);
@@ -726,7 +757,7 @@ spawn_script (const char *executable, char *const *argv,
 		  if (newex != executable1)
 			{
 			  *avhere = newex;
-			  pid = win32_spawn (newex, FALSE, argv, env,
+			  pid = win32_spawn (obj, newex, FALSE, argv, env,
 	 dwCreationFlags, si, pi);
 			  free ((char *) newex);
 			}
@@ -745,7 +776,7 @@ spawn_script (const char *executable, char *const *argv,
 /* Execute a child.  */
 
 static pid_t
-pex_win32_exec_child (struct pex_obj *obj ATTRIBUTE_UNUSED, int 

Re: [PATCH] driver: Treat include path args the same way between cpp_unique_options and asm_options. [PR71850]

2023-03-20 Thread Costas Argyris via Gcc-patches
ping

On Thu, 9 Mar 2023 at 13:39, Costas Argyris 
wrote:

> Pinging list and driver reviewer.
>
> Details here:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71850
>
> On Thu, 2 Mar 2023 at 19:25, Costas Argyris 
> wrote:
>
>> This is a proposal to fix PR71850 by applying the existing logic for
>> passing include paths to cc1 to as.
>>
>> Thanks,
>> Costas
>>
>


[PATCH] Fix native MSYS2 build failure [PR108865, PR109188]

2023-03-23 Thread Costas Argyris via Gcc-patches
Patch to fix native Windows MSYS2 package build failures.

The patch has been confirmed by both original reporters in the
PRs, and myself.

cc'd mingw-w64 maintainer for mingw-w64-specific issue.


0001-Fix-native-MSYS2-build-failure-PR108865-PR109188.patch
Description: Binary data


Re: [PATCH] driver: Treat include path args the same way between cpp_unique_options and asm_options. [PR71850]

2023-03-27 Thread Costas Argyris via Gcc-patches
[ping^3]

This looks like it fixes the bug and also unifies the way include paths are
passed from the driver to the compiler and assembler (when a @file has
been passed to the driver in the first place).

That is, when @file has been passed to the driver, put the include paths
in a temp @file and pass them to the assembler.Note this is already
happening for the compiler, so this patch merely extends this logic to the
assembler.

Is there any reason not to go for it?

On Mon, 20 Mar 2023 at 09:47, Costas Argyris 
wrote:

> ping
>
> On Thu, 9 Mar 2023 at 13:39, Costas Argyris 
> wrote:
>
>> Pinging list and driver reviewer.
>>
>> Details here:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71850
>>
>> On Thu, 2 Mar 2023 at 19:25, Costas Argyris 
>> wrote:
>>
>>> This is a proposal to fix PR71850 by applying the existing logic for
>>> passing include paths to cc1 to as.
>>>
>>> Thanks,
>>> Costas
>>>
>>


Re: [PATCH] driver: Treat include path args the same way between cpp_unique_options and asm_options. [PR71850]

2023-03-27 Thread Costas Argyris via Gcc-patches
Would it be possible to make it version-dependent, then?

As in, if GNU assembler is greater or equal to the version that
supports @FILE, then pass @FILE to it, otherwise fall back to
the current behavior.

I assume most people nowadays would have a version of
Binutils later than 2005, but if we could make it conditional on
the version then even those with earlier version wouldn't break,
they would just get the current behavior.

On Mon, 27 Mar 2023 at 11:00, Xi Ruoyao  wrote:

> On Mon, 2023-03-27 at 10:36 +0100, Costas Argyris via Gcc-patches wrote:
> > [ping^3]
> >
> > This looks like it fixes the bug and also unifies the way include paths
> are
> > passed from the driver to the compiler and assembler (when a @file has
> > been passed to the driver in the first place).
> >
> > That is, when @file has been passed to the driver, put the include paths
> > in a temp @file and pass them to the assembler.Note this is already
> > happening for the compiler, so this patch merely extends this logic to
> the
> > assembler.
> >
> > Is there any reason not to go for it?
>
> It's not supported by all GNU assembler releases.  For example, GCC
> installation doc says we require Binutils >= 2.13.1 for i?86-*-linux*.
> Binutils 2.13.1 was released in 2002, but @FILE support was added into
> Binutils in 2005.
> > >
>
> --
> Xi Ruoyao 
> School of Aerospace Science and Technology, Xidian University
>


Re: Enable UTF-8 code page in driver and compiler on 64-bit mingw host [PR108865]

2023-03-27 Thread Costas Argyris via Gcc-patches
The patch attached to this email extends the UTF-8 support of the
driver and compiler processes to the 32-bit mingw host.Initially,
only the 64-bit host got it.

About the changes in sym-mingw32.cc:

Even though the 64-bit host was building fine with the symbol being
simply declared as a char, the 32-bit host was failing to find the
symbol at link time because a leading underscore was being added
to it by the compiler.The asm keyword ensures that the symbol
always appears with that exact name, such that the linker will
always find it.

The patch also includes Jacek's flag about adding the .manifest file
as a prerequisite for the object file (this was actually done from before
but an earlier version of the patch was pushed so it was missed).

Tested building from master for both 32 and 64-bit mingw hosts using:

1) cross-compilation from a Debian machine using configure + make
2) native-compilation from a Windows machine using MSYS2

On Thu, 9 Mar 2023 at 15:03, Jonathan Yong <10wa...@gmail.com> wrote:

> On 3/9/23 13:33, Costas Argyris wrote:
> > Pinging the list and mingw maintainer.
> >
> > Analysis and pre-approval here:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108865
> >
>
> Thanks, pushed to master branch.
>
>
>
From 0ed08739f44116eaef46b552df923959ba945afa Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Sun, 26 Mar 2023 11:32:13 +0100
Subject: [PATCH] Extend UTF-8 support to the 32-bit mingw host.

Prevent any name mangling in HOST_EXTRA_OBJS_SYMBOL
such that the linker always finds it by that name.

Also add the .manifest file as an explicit
dependency in the make rule such that the
object gets re-built if it changes.
---
 gcc/config.host| 5 +++--
 gcc/config/i386/sym-mingw32.cc | 4 +++-
 gcc/config/i386/x-mingw32-utf8 | 3 ++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/gcc/config.host b/gcc/config.host
index 4abb32ad73d..5df85752ed4 100644
--- a/gcc/config.host
+++ b/gcc/config.host
@@ -232,10 +232,11 @@ case ${host} in
 ;;
   i[34567]86-*-mingw32*)
 host_xm_file=i386/xm-mingw32.h
-host_xmake_file="${host_xmake_file} i386/x-mingw32"
+host_xmake_file="${host_xmake_file} i386/x-mingw32 i386/x-mingw32-utf8"
 host_exeext=.exe
 out_host_hook_obj=host-mingw32.o
-host_extra_gcc_objs="${host_extra_gcc_objs} driver-mingw32.o"
+host_extra_objs="${host_extra_objs} utf8-mingw32.o"
+host_extra_gcc_objs="${host_extra_gcc_objs} driver-mingw32.o utf8rc-mingw32.o"
 host_lto_plugin_soname=liblto_plugin.dll
 ;;
   x86_64-*-mingw*)
diff --git a/gcc/config/i386/sym-mingw32.cc b/gcc/config/i386/sym-mingw32.cc
index f369698abc4..2f8dee6c1ec 100644
--- a/gcc/config/i386/sym-mingw32.cc
+++ b/gcc/config/i386/sym-mingw32.cc
@@ -1 +1,3 @@
-char HOST_EXTRA_OBJS_SYMBOL;
+/* Prevent any name mangling to make sure that the linker
+   will always find the symbol. */
+char HOST_EXTRA_OBJS_SYMBOL asm ("HOST_EXTRA_OBJS_SYMBOL");
diff --git a/gcc/config/i386/x-mingw32-utf8 b/gcc/config/i386/x-mingw32-utf8
index 9de963d7965..cf5c3db3d8b 100644
--- a/gcc/config/i386/x-mingw32-utf8
+++ b/gcc/config/i386/x-mingw32-utf8
@@ -27,7 +27,8 @@
 # The resulting .o file gets added to host_extra_gcc_objs in
 # config.host for x86_64-*-mingw* host and gets linked into
 # the driver as a .o file, so it's lack of symbols is OK.
-utf8rc-mingw32.o : $(srcdir)/config/i386/utf8-mingw32.rc
+utf8rc-mingw32.o : $(srcdir)/config/i386/utf8-mingw32.rc \
+  $(srcdir)/config/i386/winnt-utf8.manifest
 	$(WINDRES) $< $@
 
 # Create an object file that just exports the global symbol
-- 
2.30.2



Re: Enable UTF-8 code page in driver and compiler on 64-bit mingw host [PR108865]

2023-03-28 Thread Costas Argyris via Gcc-patches
I forgot to update the relevant comments with the previous patch.

This is a comment-only patch that brings them up-to-date.

On Tue, 28 Mar 2023 at 09:05, Jonathan Yong <10wa...@gmail.com> wrote:

> On 3/27/23 17:17, Costas Argyris wrote:
> > The patch attached to this email extends the UTF-8 support of the
> > driver and compiler processes to the 32-bit mingw host.Initially,
> > only the 64-bit host got it.
> >
> > About the changes in sym-mingw32.cc:
> >
> > Even though the 64-bit host was building fine with the symbol being
> > simply declared as a char, the 32-bit host was failing to find the
> > symbol at link time because a leading underscore was being added
> > to it by the compiler.The asm keyword ensures that the symbol
> > always appears with that exact name, such that the linker will
> > always find it.
> >
> > The patch also includes Jacek's flag about adding the .manifest file
> > as a prerequisite for the object file (this was actually done from before
> > but an earlier version of the patch was pushed so it was missed).
> >
> > Tested building from master for both 32 and 64-bit mingw hosts using:
> >
> > 1) cross-compilation from a Debian machine using configure + make
> > 2) native-compilation from a Windows machine using MSYS2
> >
> Thanks, approved and pushed to master branch.
>
>
From 87da0323ec5b08d44f17713d8d4e19664d7a3aa6 Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Tue, 28 Mar 2023 11:29:06 +0100
Subject: [PATCH] mingw: Fix comments in x-mingw32-utf8

This is a comment-only change that I should have
done with the previous commit (304c7d44a) but
forgot to do so.
---
 gcc/config/i386/x-mingw32-utf8 | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/gcc/config/i386/x-mingw32-utf8 b/gcc/config/i386/x-mingw32-utf8
index cf5c3db3d8b..2783dd259a6 100644
--- a/gcc/config/i386/x-mingw32-utf8
+++ b/gcc/config/i386/x-mingw32-utf8
@@ -17,15 +17,15 @@
 # .
 #
 #
-# For 64-bit Windows host, embed a manifest that sets the active
+# For mingw Windows hosts, embed a manifest that sets the active
 # code page of the driver and compiler proper processes to utf8.
-# This only has an effect on Windows version 1903 (May 2019 Update)
-# or later.
+# This only has an effect when gcc is hosted on Windows version
+# 1903 (May 2019 Update) or later.
 
 # The resource .rc file references the utf8 .manifest file.
 # Compile it into an object file using windres.
 # The resulting .o file gets added to host_extra_gcc_objs in
-# config.host for x86_64-*-mingw* host and gets linked into
+# config.host for mingw hosts and gets linked into
 # the driver as a .o file, so it's lack of symbols is OK.
 utf8rc-mingw32.o : $(srcdir)/config/i386/utf8-mingw32.rc \
   $(srcdir)/config/i386/winnt-utf8.manifest
@@ -39,7 +39,7 @@ sym-mingw32.o : $(srcdir)/config/i386/sym-mingw32.cc
 # Combine the two object files into one which has both the
 # compiled utf8 resource and the HOST_EXTRA_OBJS_SYMBOL symbol.
 # The resulting .o file gets added to host_extra_objs in
-# config.host for x86_64-*-mingw* host and gets archived into
+# config.host for mingw hosts and gets archived into
 # libbackend.a which gets linked into the compiler proper.
 # If nothing references it into libbackend.a, it will not
 # get linked into the compiler proper eventually.
@@ -54,4 +54,8 @@ utf8-mingw32.o : utf8rc-mingw32.o sym-mingw32.o
 # This is expected because the resource object is not supposed
 # to have any symbols, it just has to be linked into the
 # executable in order for Windows to use the utf8 code page.
+# Some build environments are passing these flags to other
+# programs as well, so make the symbol definition optional
+# such that these programs don't fail to build when they
+# don't find it.
 $(COMPILERS) : override LDFLAGS += -Wl,--undefined=HOST_EXTRA_OBJS_SYMBOL
-- 
2.30.2



[PATCH] mingw: Support building with older gcc versions

2023-04-12 Thread Costas Argyris via Gcc-patches
This is proposed to fix PR109460 where an older version of
gcc (7.3) was used to build for windows (mingw) host.
From e5b608072f80a83cca65e88bb75ecc62ab0bbb87 Mon Sep 17 00:00:00 2001
From: Costas Argyris 
Date: Wed, 12 Apr 2023 08:48:18 +0100
Subject: [PATCH] mingw: Support building with older gcc versions

The $@ argument to the compiler is causing
only a warning in some gcc versions but an
error in others. In any case, $@ was never
necessary so remove it completely, just like
the rules in x-mingw32 where the object file
gets named after the source file.

This fixes both warnings and errors about
sym-mingw32.o appearing in the command line
unnecessarily.

The -nostdlib flag is required along with -r
for older gcc versions that don't apply it
automatically with -r, resulting in main
functions erroneously entering a partial link.
---
 gcc/config/i386/x-mingw32-utf8 | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/config/i386/x-mingw32-utf8 b/gcc/config/i386/x-mingw32-utf8
index 2783dd259a6..b5a6cfcf702 100644
--- a/gcc/config/i386/x-mingw32-utf8
+++ b/gcc/config/i386/x-mingw32-utf8
@@ -34,7 +34,7 @@ utf8rc-mingw32.o : $(srcdir)/config/i386/utf8-mingw32.rc \
 # Create an object file that just exports the global symbol
 # HOST_EXTRA_OBJS_SYMBOL
 sym-mingw32.o : $(srcdir)/config/i386/sym-mingw32.cc
-	$(COMPILER) -c $< $@
+	$(COMPILER) -c $<
 
 # Combine the two object files into one which has both the
 # compiled utf8 resource and the HOST_EXTRA_OBJS_SYMBOL symbol.
@@ -44,8 +44,10 @@ sym-mingw32.o : $(srcdir)/config/i386/sym-mingw32.cc
 # If nothing references it into libbackend.a, it will not
 # get linked into the compiler proper eventually.
 # Therefore we need to request the symbol at compiler link time.
+# -nostdlib is required for supporting old gcc versions that
+# don't apply it automatically with -r.
 utf8-mingw32.o : utf8rc-mingw32.o sym-mingw32.o
-	$(COMPILER) -r utf8rc-mingw32.o sym-mingw32.o -o $@
+	$(COMPILER) -r -nostdlib utf8rc-mingw32.o sym-mingw32.o -o $@
 
 # Force compilers to link against the utf8 resource by
 # requiring the symbol to be defined.
-- 
2.30.2